[Bug 711155] wayland: add wl_drm support to wayland sink

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=711155

--- Comment #42 from Nicolas Dufresne (stormer) <nicolas.dufresne at collabora.co.uk> ---
Review of attachment 310472:
 --> (https://bugzilla.gnome.org/review?bug=711155&attachment=310472)

I don't think the approach is right. This seem to track GstBuffer instead of
GstMemory. About the ifdef, I might have miss-read (and be wrong), but I
believe it's not because you have a release that support dmabuf that this
protocol is implement in the compositor. It also seems to assume that all
DMABUF will be backed by DRM Dumb.

::: ext/wayland/gstwaylandsink.c
@@ +586,3 @@
     config = gst_buffer_pool_get_config (pool);
     gst_buffer_pool_config_set_params (config, caps, size, 2, 0);
+    gst_buffer_pool_config_set_allocator (config, NULL, &params);

Why bother passing allocation params if you don't have any alignment
requirement ?

::: ext/wayland/waylandpool.c
@@ +38,3 @@
 #include <sys/types.h>

+#ifdef HAVE_WAYLAND_DMABUF

I think this should mostly be run-time. It would be sad if we cannot use the
element at all on a compositor that does not implement this protocol.

@@ +78,3 @@
+    memset (&destroy_arg, 0, sizeof destroy_arg);
+    drmPrimeFDToHandle (meta->drm_fd, prime_fd, &destroy_arg.handle);
+    drmIoctl (meta->drm_fd, DRM_IOCTL_MODE_DESTROY_DUMB, &destroy_arg);

What if the dmabuf was not DRM dumb ?

::: ext/wayland/wlvideoformat.c
@@ +63,3 @@
+#ifdef HAVE_WAYLAND_DMABUF
+  {WL_DMABUF_FORMAT_XRGB8888, GST_VIDEO_FORMAT_BGRx},
+  {WL_DMABUF_FORMAT_ARGB8888, GST_VIDEO_FORMAT_BGRA},

And no YUV format ? Without any YUV format, it's a bit pointless to have this
in GStreamer.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.