On 03/12/2013, benjamin.gaignard@linaro.org wrote :
Here are a few comments:
- <!-- dmabuf support. This object is created by the server and published
using the display's global event. --><interface name="wl_dmabuf" version="2">
What's the point of creating a new interface, and skipping version 1? You should have removed too the since="2" you have in some functions.
<enum name="capability" since="2">
<description summary="wl_dmabuf capability bitmask">Bitmask of capabilities.</description><entry name="prime" value="1" summary="wl_dmabuf prime available"/></enum>
What's the point of advertising if Prime is available or not? If Prime isn't available, then the extension shouldn't be advertised at all.
- wl_resource_post_event(resource, WL_DMABUF_FORMAT, WL_DMABUF_FORMAT_ARGB8888);
- wl_resource_post_event(resource, WL_DMABUF_FORMAT, WL_DMABUF_FORMAT_XRGB8888);
- wl_resource_post_event(resource, WL_DMABUF_FORMAT, WL_DMABUF_FORMAT_YUV410);
- wl_resource_post_event(resource, WL_DMABUF_FORMAT, WL_DMABUF_FORMAT_YUV411);
- wl_resource_post_event(resource, WL_DMABUF_FORMAT, WL_DMABUF_FORMAT_YUV420);
- wl_resource_post_event(resource, WL_DMABUF_FORMAT, WL_DMABUF_FORMAT_YUV422);
- wl_resource_post_event(resource, WL_DMABUF_FORMAT, WL_DMABUF_FORMAT_YUV444);
- wl_resource_post_event(resource, WL_DMABUF_FORMAT, WL_DMABUF_FORMAT_NV12);
- wl_resource_post_event(resource, WL_DMABUF_FORMAT, WL_DMABUF_FORMAT_NV16);
- wl_resource_post_event(resource, WL_DMABUF_FORMAT, WL_DMABUF_FORMAT_YUYV);
These formats can potentially not be all supported by the server. You should find a way to check available formats.
- capabilities = 0;
- if (dmabuf->flags & WAYLAND_DMABUF_PRIME)
capabilities |= WL_DMABUF_CAPABILITY_PRIME;
again, I don't understand why you would advertise PRIME support
If you want a generic interface to share dma-bufs buffers, you should make this interface much more generic, to adapt to what the server can do. For example you could advertise if the server supports mmap, etc.
Axel Davy