On Tue, 16 Dec 2014 13:19:53 -0800 Bill Spitzak <spit...@gmail.com> wrote:
> It looks like the purpose of "dmabuf_batch" is to send a more complex > set of arguments to the dmabuf::create_buffer request. This is a > variable-sized list of fd's, each containing a variable-sized list of > planes. The Wayland protocol does not have a method of passing this as a > request argument, so this object is used to build it with multiple > requests. Is this correct? And is this considered the correct way to do > this sort of thing in Wayland? We could use wl_array, except it does not work with fd's. So yes, this is the only way to get a variable number of fd's into a single final request. > It is not clear but I assume you must add at least one fd to the > dmabuf_batch for this to work, right? Yes. > Assuming this is correct, I think some consolidation of the objects > would help. A few ideas: > > - Make create_buffer a method on dmabuf_batch, not on dmabuf. I'm not sure what would be the difference. You can't do it without a dmabuf_batch anyway (the spec does not have allow-null="true"). > - Get rid of dmabuf_create_feedback object, and just reuse the > already-existing dmabuf_batch object to deliver the success/failure event. Then it's not a dmabuf_batch anymore, you need to think of a new name that describes it. Using a one-shot feedback object is a standard Wayland design pattern. > - I am a bit suspicious of the exactly 3 planes, there are 4:2:2 formats > with alpha channels. I think it would be better if there was a request > per-plane. This is modelled after https://www.khronos.org/registry/egl/extensions/EXT/EGL_EXT_image_dma_buf_import.txt The EGL extension could be extended to fourth plane though, so maybe we could use wl_array here to achieve easier extendability. The DRM user ABI seems to use 4 indeed. > - Isn't there a problem with having an event deliver a new object (ie > the wl_buffer in the create_successful event)? Can the dmabuf_batch > object just "be" a wl_buffer, but you cannot use it as a wl_buffer until > you get the success event? There is no problem in creating a new object by an event. It is rarely used, but it must work. > So in the most-cleaned-up version I can come up with the api is more > like this: > > dmabuf - singleton factory used to create dmabuf_buffer. > > dmabuf::create_buffer - create a new dmabuf_buffer. You must then do > some requests on it before it can be used as a wl_buffer. > > dmabuf::format event - same as you describe it > > dmabuf_buffer - A subclass of wl_buffer representing a (potential) dma > buffer. There is no such thing as sub-classing, all object types are strict and must match exactly in the protocol (no inheritance or is-a). The wl_buffer object must be created at some point. > dmabuf_buffer::add_fd - Add a new fd describing a dmabuf > > dmabuf_buffer::add_plane - Add a plane (offset + stride) to map from the > most recent fd. > > dmabuf_buffer::create - try to create the dma buffer. After this you > cannot do the add_fd/plane requests. > > dmabuf_buffer::create_successful event - you can now use the > dmabuf_buffer as a wl_buffer (for instance to attach it to a wl_surface). > > dmabuf_buffer::create_failed event - It did not work. The only thing you > are allowed to do is destroy the dmabuf_buffer. I wonder if that is simpler of just different, assuming you fix the sub-classing. I'd rather not do "cosmetic" changes yet, because we are likely going to need more parameters as the dma_buf kernel interfaces are being developed. Thanks, pq _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel