waylandsink: Improve DMA DRM integration

Pass GstVideoInfoDmaDrm or GstVideoInfo whenever possible, avoiding passing
strange combination of GstVieoFormat + modifier. Even though we don't have any
at the moment, this also allow supporting GstVideoFormat that are not supported
in our DRM integration.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/5120>

subprojects/gst-plugins-bad/ext/gtk/gstgtkwaylandsink.c

@@ -28,6 +28,7 @@
 #include "gstgtkutils.h"
 #include "gtkgstwaylandwidget.h"
 
+#include <drm_fourcc.h>
 #include <gdk/gdk.h>
 #include <gst/allocators/allocators.h>
 #include <gst/wayland/wayland.h>
@@ -108,7 +109,7 @@ typedef struct _GstGtkWaylandSinkPrivate
 
   gboolean video_info_changed;
   GstVideoInfo video_info;
-  guint64 modifier;
+  GstVideoInfoDmaDrm drm_info;
   GstCaps *caps;
 
   gboolean redraw_pending;
@@ -950,27 +951,26 @@ gst_gtk_wayland_sink_set_caps (GstBaseSink * bsink, GstCaps * caps)
   GstGtkWaylandSinkPrivate *priv =
       gst_gtk_wayland_sink_get_instance_private (self);
   gboolean use_dmabuf;
-  GstVideoFormat format;
-  GstVideoInfoDmaDrm drm_info;
 
   GST_DEBUG_OBJECT (self, "set caps %" GST_PTR_FORMAT, caps);
 
-  gst_video_info_dma_drm_init (&drm_info);
-
   if (gst_video_is_dma_drm_caps (caps)) {
-    if (!gst_video_info_dma_drm_from_caps (&drm_info, caps))
+    if (!gst_video_info_dma_drm_from_caps (&priv->drm_info, caps))
       goto invalid_format;
 
-    if (!gst_video_info_dma_drm_to_video_info (&drm_info, &priv->video_info))
+    if (!gst_video_info_dma_drm_to_video_info (&priv->drm_info,
+            &priv->video_info))
       goto invalid_format;
   } else {
     /* extract info from caps */
     if (!gst_video_info_from_caps (&priv->video_info, caps))
       goto invalid_format;
+
+    if (!gst_video_info_dma_drm_from_video_info (&priv->drm_info,
+            &priv->video_info, DRM_FORMAT_MOD_LINEAR))
+      gst_video_info_dma_drm_init (&priv->drm_info);
   }
 
-  format = GST_VIDEO_INFO_FORMAT (&priv->video_info);
-  priv->modifier = drm_info.drm_modifier;
   priv->video_info_changed = TRUE;
   priv->skip_dumb_buffer_copy = FALSE;
 
@@ -985,11 +985,11 @@ gst_gtk_wayland_sink_set_caps (GstBaseSink * bsink, GstCaps * caps)
 
   /* validate the format base on the memory type. */
   if (use_dmabuf) {
-    if (!gst_wl_display_check_format_for_dmabuf (priv->display, format,
+    if (!gst_wl_display_check_format_for_dmabuf (priv->display,
-            drm_info.drm_modifier))
+            &priv->drm_info))
-      goto unsupported_format;
+      goto unsupported_drm_format;
-  } else if (!gst_wl_display_check_format_for_shm (priv->display, format,
+  } else if (!gst_wl_display_check_format_for_shm (priv->display,
-          drm_info.drm_modifier)) {
+          &priv->video_info)) {
     /* Note: we still support dmabuf in this case, but formats must also be
      * supported on SHM interface to ensure a fallback is possible as we are
      * not guarantied we'll get dmabuf in the buffers. */
@@ -1029,10 +1029,17 @@ invalid_format:
         "Could not locate image format from caps %" GST_PTR_FORMAT, caps);
     return FALSE;
   }
+unsupported_drm_format:
+  {
+    GST_ERROR_OBJECT (self, "DRM format %" GST_FOURCC_FORMAT
+        " is not available on the display",
+        GST_FOURCC_ARGS (priv->drm_info.drm_fourcc));
+    return FALSE;
+  }
 unsupported_format:
   {
     GST_ERROR_OBJECT (self, "Format %s is not available on the display",
-        gst_video_format_to_string (format));
+        gst_video_format_to_string (GST_VIDEO_INFO_FORMAT (&priv->video_info)));
     return FALSE;
   }
 }
@@ -1133,10 +1140,8 @@ gst_gtk_wayland_sink_show_frame (GstVideoSink * vsink, GstBuffer * buffer)
       gst_gtk_wayland_sink_get_instance_private (self);
   GstBuffer *to_render;
   GstWlBuffer *wlbuffer;
-  GstVideoFormat format;
   GstMemory *mem;
   struct wl_buffer *wbuf = NULL;
-  guint64 modifier;
 
   GstFlowReturn ret = GST_FLOW_OK;
 
@@ -1181,9 +1186,7 @@ gst_gtk_wayland_sink_show_frame (GstVideoSink * vsink, GstBuffer * buffer)
       "buffer %" GST_PTR_FORMAT " does not have a wl_buffer from our "
       "display, creating it", buffer);
 
-  format = GST_VIDEO_INFO_FORMAT (&priv->video_info);
+  if (gst_wl_display_check_format_for_dmabuf (priv->display, &priv->drm_info)) {
-  modifier = priv->modifier;
-  if (gst_wl_display_check_format_for_dmabuf (priv->display, format, modifier)) {
     guint i, nb_dmabuf = 0;
 
     for (i = 0; i < gst_buffer_n_memory (buffer); i++)
@@ -1192,7 +1195,7 @@ gst_gtk_wayland_sink_show_frame (GstVideoSink * vsink, GstBuffer * buffer)
 
     if (nb_dmabuf && (nb_dmabuf == gst_buffer_n_memory (buffer)))
       wbuf = gst_wl_linux_dmabuf_construct_wl_buffer (buffer, priv->display,
-          &priv->video_info, modifier);
+          &priv->drm_info);
 
     /* DMABuf did not work, let try and make this a dmabuf, it does not matter
      * if it was a SHM since the compositor needs to copy that anyway, and
@@ -1216,7 +1219,7 @@ gst_gtk_wayland_sink_show_frame (GstVideoSink * vsink, GstBuffer * buffer)
       /* attach a wl_buffer if there isn't one yet */
       if (G_UNLIKELY (!wlbuffer)) {
         wbuf = gst_wl_linux_dmabuf_construct_wl_buffer (to_render,
-            priv->display, &priv->video_info, modifier);
+            priv->display, &priv->drm_info);
 
         if (G_UNLIKELY (!wbuf)) {
           GST_WARNING_OBJECT (self, "failed to import DRM Dumb dmabuf");
@@ -1247,9 +1250,8 @@ gst_gtk_wayland_sink_show_frame (GstVideoSink * vsink, GstBuffer * buffer)
   }
 
 handle_shm:
-  if (!wbuf
+  if (!wbuf && gst_wl_display_check_format_for_shm (priv->display,
-      && gst_wl_display_check_format_for_shm (priv->display, format,
+          &priv->video_info)) {
-          modifier)) {
     if (gst_buffer_n_memory (buffer) == 1 && gst_is_fd_memory (mem))
       wbuf = gst_wl_shm_memory_construct_wl_buffer (mem, priv->display,
           &priv->video_info);

subprojects/gst-plugins-bad/ext/wayland/gstwaylandsink.c

@@ -43,8 +43,9 @@
 #endif
 
 #include "gstwaylandsink.h"
-#include <gst/allocators/allocators.h>
 
+#include <drm_fourcc.h>
+#include <gst/allocators/allocators.h>
 #include <gst/video/videooverlay.h>
 
 /* signals */
@@ -693,27 +694,26 @@ gst_wayland_sink_set_caps (GstBaseSink * bsink, GstCaps * caps)
 {
   GstWaylandSink *self = GST_WAYLAND_SINK (bsink);;
   gboolean use_dmabuf;
-  GstVideoFormat format;
-  GstVideoInfoDmaDrm drm_info;
 
   GST_DEBUG_OBJECT (self, "set caps %" GST_PTR_FORMAT, caps);
 
-  gst_video_info_dma_drm_init (&drm_info);
-
   if (gst_video_is_dma_drm_caps (caps)) {
-    if (!gst_video_info_dma_drm_from_caps (&drm_info, caps))
+    if (!gst_video_info_dma_drm_from_caps (&self->drm_info, caps))
       goto invalid_format;
 
-    if (!gst_video_info_dma_drm_to_video_info (&drm_info, &self->video_info))
+    if (!gst_video_info_dma_drm_to_video_info (&self->drm_info,
+            &self->video_info))
       goto invalid_format;
   } else {
     /* extract info from caps */
     if (!gst_video_info_from_caps (&self->video_info, caps))
       goto invalid_format;
+
+    if (!gst_video_info_dma_drm_from_video_info (&self->drm_info,
+            &self->video_info, DRM_FORMAT_MOD_LINEAR))
+      gst_video_info_dma_drm_init (&self->drm_info);
   }
 
-  format = GST_VIDEO_INFO_FORMAT (&self->video_info);
-  self->modifier = drm_info.drm_modifier;
   self->video_info_changed = TRUE;
   self->skip_dumb_buffer_copy = FALSE;
 
@@ -728,11 +728,11 @@ gst_wayland_sink_set_caps (GstBaseSink * bsink, GstCaps * caps)
 
   /* validate the format base on the memory type. */
   if (use_dmabuf) {
-    if (!gst_wl_display_check_format_for_dmabuf (self->display, format,
+    if (!gst_wl_display_check_format_for_dmabuf (self->display,
-            drm_info.drm_modifier))
+            &self->drm_info))
-      goto unsupported_format;
+      goto unsupported_drm_format;
-  } else if (!gst_wl_display_check_format_for_shm (self->display, format,
+  } else if (!gst_wl_display_check_format_for_shm (self->display,
-          drm_info.drm_modifier)) {
+          &self->video_info)) {
     /* Note: we still support dmabuf in this case, but formats must also be
      * supported on SHM interface to ensure a fallback is possible as we are
      * not guarantied we'll get dmabuf in the buffers. */
@@ -750,10 +750,17 @@ invalid_format:
         "Could not locate image format from caps %" GST_PTR_FORMAT, caps);
     return FALSE;
   }
+unsupported_drm_format:
+  {
+    GST_ERROR_OBJECT (self, "DRM format %" GST_FOURCC_FORMAT
+        " is not available on the display",
+        GST_FOURCC_ARGS (self->drm_info.drm_fourcc));
+    return FALSE;
+  }
 unsupported_format:
   {
     GST_ERROR_OBJECT (self, "Format %s is not available on the display",
-        gst_video_format_to_string (format));
+        gst_video_format_to_string (GST_VIDEO_INFO_FORMAT (&self->video_info)));
     return FALSE;
   }
 }
@@ -853,10 +860,8 @@ gst_wayland_sink_show_frame (GstVideoSink * vsink, GstBuffer * buffer)
   GstWaylandSink *self = GST_WAYLAND_SINK (vsink);
   GstBuffer *to_render;
   GstWlBuffer *wlbuffer;
-  GstVideoFormat format;
   GstMemory *mem;
   struct wl_buffer *wbuf = NULL;
-  guint64 modifier;
 
   GstFlowReturn ret = GST_FLOW_OK;
 
@@ -912,9 +917,7 @@ gst_wayland_sink_show_frame (GstVideoSink * vsink, GstBuffer * buffer)
       "buffer %" GST_PTR_FORMAT " does not have a wl_buffer from our "
       "display, creating it", buffer);
 
-  format = GST_VIDEO_INFO_FORMAT (&self->video_info);
+  if (gst_wl_display_check_format_for_dmabuf (self->display, &self->drm_info)) {
-  modifier = self->modifier;
-  if (gst_wl_display_check_format_for_dmabuf (self->display, format, modifier)) {
     guint i, nb_dmabuf = 0;
 
     for (i = 0; i < gst_buffer_n_memory (buffer); i++)
@@ -923,7 +926,7 @@ gst_wayland_sink_show_frame (GstVideoSink * vsink, GstBuffer * buffer)
 
     if (nb_dmabuf && (nb_dmabuf == gst_buffer_n_memory (buffer)))
       wbuf = gst_wl_linux_dmabuf_construct_wl_buffer (buffer, self->display,
-          &self->video_info, modifier);
+          &self->drm_info);
 
     /* DMABuf did not work, let try and make this a dmabuf, it does not matter
      * if it was a SHM since the compositor needs to copy that anyway, and
@@ -947,7 +950,7 @@ gst_wayland_sink_show_frame (GstVideoSink * vsink, GstBuffer * buffer)
       /* attach a wl_buffer if there isn't one yet */
       if (G_UNLIKELY (!wlbuffer)) {
         wbuf = gst_wl_linux_dmabuf_construct_wl_buffer (to_render,
-            self->display, &self->video_info, modifier);
+            self->display, &self->drm_info);
 
         if (G_UNLIKELY (!wbuf)) {
           GST_WARNING_OBJECT (self, "failed to import DRM Dumb dmabuf");
@@ -978,9 +981,8 @@ gst_wayland_sink_show_frame (GstVideoSink * vsink, GstBuffer * buffer)
   }
 
 handle_shm:
-  if (!wbuf
+  if (!wbuf && gst_wl_display_check_format_for_shm (self->display,
-      && gst_wl_display_check_format_for_shm (self->display, format,
+          &self->video_info)) {
-          modifier)) {
     if (gst_buffer_n_memory (buffer) == 1 && gst_is_fd_memory (mem))
       wbuf = gst_wl_shm_memory_construct_wl_buffer (mem, self->display,
           &self->video_info);

subprojects/gst-plugins-bad/ext/wayland/gstwaylandsink.h

@@ -54,7 +54,7 @@ struct _GstWaylandSink
 
   gboolean video_info_changed;
   GstVideoInfo video_info;
-  guint64 modifier;
+  GstVideoInfoDmaDrm drm_info;
   gboolean fullscreen;
   GstCaps *caps;
 

subprojects/gst-plugins-bad/gst-libs/gst/wayland/gstwldisplay.c

@@ -235,18 +235,14 @@ static const struct zwp_linux_dmabuf_v1_listener dmabuf_listener = {
 
 gboolean
 gst_wl_display_check_format_for_shm (GstWlDisplay * self,
-    GstVideoFormat format, guint64 modifier)
+    const GstVideoInfo * video_info)
 {
   GstWlDisplayPrivate *priv = gst_wl_display_get_instance_private (self);
+  GstVideoFormat format = GST_VIDEO_INFO_FORMAT (video_info);
   enum wl_shm_format shm_fmt;
   GArray *formats;
   guint i;
 
-  if (modifier != DRM_FORMAT_MOD_INVALID && modifier != DRM_FORMAT_MOD_LINEAR) {
-    GST_ERROR ("SHM interface does not support modifiers");
-    return FALSE;
-  }
-
   shm_fmt = gst_video_format_to_wl_shm_format (format);
   if (shm_fmt == (enum wl_shm_format) -1)
     return FALSE;
@@ -262,23 +258,21 @@ gst_wl_display_check_format_for_shm (GstWlDisplay * self,
 
 gboolean
 gst_wl_display_check_format_for_dmabuf (GstWlDisplay * self,
-    GstVideoFormat format, guint64 modifier)
+    const GstVideoInfoDmaDrm * drm_info)
 {
   GstWlDisplayPrivate *priv = gst_wl_display_get_instance_private (self);
+  guint64 modifier = drm_info->drm_modifier;
+  guint fourcc = drm_info->drm_fourcc;
   GArray *formats, *modifiers;
-  guint i, dmabuf_fmt;
+  guint i;
 
   if (!priv->dmabuf)
     return FALSE;
 
-  dmabuf_fmt = gst_video_format_to_wl_dmabuf_format (format);
-  if (!dmabuf_fmt)
-    return FALSE;
-
   formats = priv->dmabuf_formats;
   modifiers = priv->dmabuf_modifiers;
   for (i = 0; i < formats->len; i++) {
-    if (g_array_index (formats, uint32_t, i) == dmabuf_fmt) {
+    if (g_array_index (formats, uint32_t, i) == fourcc) {
       if (g_array_index (modifiers, guint64, i) == modifier) {
         return TRUE;
       }

subprojects/gst-plugins-bad/gst-libs/gst/wayland/gstwldisplay.h

@@ -57,11 +57,11 @@ gpointer gst_wl_display_lookup_buffer (GstWlDisplay * self, gpointer gstmem);
 
 GST_WL_API
 gboolean gst_wl_display_check_format_for_shm (GstWlDisplay * self,
-    GstVideoFormat format, guint64 modifier);
+    const GstVideoInfo *video_info);
 
 GST_WL_API
 gboolean gst_wl_display_check_format_for_dmabuf (GstWlDisplay * self,
-    GstVideoFormat format, guint64 modifier);
+    const GstVideoInfoDmaDrm *drm_info);
 
 GST_WL_API
 struct wl_display *gst_wl_display_get_display (GstWlDisplay * self);

subprojects/gst-plugins-bad/gst-libs/gst/wayland/gstwllinuxdmabuf.c

@@ -83,44 +83,55 @@ static const struct zwp_linux_buffer_params_v1_listener params_listener = {
 
 struct wl_buffer *
 gst_wl_linux_dmabuf_construct_wl_buffer (GstBuffer * buf,
-    GstWlDisplay * display, const GstVideoInfo * info, guint64 modifier)
+    GstWlDisplay * display, const GstVideoInfoDmaDrm * drm_info)
 {
   GstMemory *mem;
-  int format;
+  guint fourcc;
-  guint i, width, height;
+  guint64 modifier;
-  const gsize *offsets = info->offset;
+  guint i, width = 0, height = 0;
-  const gint *strides = info->stride;
+  GstVideoInfo info;
+  const gsize *offsets = NULL;
+  const gint *strides = NULL;
   GstVideoMeta *vmeta;
-  guint nplanes, flags = 0;
+  guint nplanes = 0, flags = 0;
   struct zwp_linux_buffer_params_v1 *params;
   gint64 timeout;
   ConstructBufferData data;
 
   g_return_val_if_fail (gst_wl_display_check_format_for_dmabuf (display,
-          GST_VIDEO_INFO_FORMAT (info), modifier), NULL);
+          drm_info), NULL);
 
   mem = gst_buffer_peek_memory (buf, 0);
-  format = gst_video_format_to_wl_dmabuf_format (GST_VIDEO_INFO_FORMAT (info));
+  fourcc = drm_info->drm_fourcc;
+  modifier = drm_info->drm_modifier;
 
   g_cond_init (&data.cond);
   g_mutex_init (&data.lock);
   g_mutex_lock (&data.lock);
 
-  width = GST_VIDEO_INFO_WIDTH (info);
-  height = GST_VIDEO_INFO_HEIGHT (info);
-  nplanes = GST_VIDEO_INFO_N_PLANES (info);
-
   vmeta = gst_buffer_get_video_meta (buf);
   if (vmeta) {
+    width = vmeta->width;
+    height = vmeta->height;
+    nplanes = vmeta->n_planes;
     offsets = vmeta->offset;
     strides = vmeta->stride;
+  } else if (gst_video_info_dma_drm_to_video_info (drm_info, &info)) {
+    nplanes = GST_VIDEO_INFO_N_PLANES (&info);
+    width = info.width;
+    height = info.height;
+    offsets = info.offset;
+    strides = info.stride;
+  } else {
+    GST_ERROR_OBJECT (display, "GstVideoMeta is needed to carry DMABuf using "
+        "'memory:DMABuf' caps feature.");
+    goto out;
   }
 
-  const gchar *format_string =
-      gst_wl_dmabuf_format_to_string (format, modifier);
   GST_DEBUG_OBJECT (display,
       "Creating wl_buffer from DMABuf of size %" G_GSSIZE_FORMAT
-      " (%d x %d), format %s", info->size, width, height, format_string);
+      " (%d x %d), DRM fourcc %" GST_FOURCC_FORMAT, gst_buffer_get_size (buf),
+      width, height, GST_FOURCC_ARGS (fourcc));
 
   /* Creation and configuration of planes  */
   params = zwp_linux_dmabuf_v1_create_params (gst_wl_display_get_dmabuf_v1
@@ -158,7 +169,7 @@ gst_wl_linux_dmabuf_construct_wl_buffer (GstBuffer * buf,
 
   /* Request buffer creation */
   zwp_linux_buffer_params_v1_add_listener (params, &params_listener, &data);
-  zwp_linux_buffer_params_v1_create (params, width, height, format, flags);
+  zwp_linux_buffer_params_v1_create (params, width, height, fourcc, flags);
 
   /* Wait for the request answer */
   wl_display_flush (gst_wl_display_get_display (display));
@@ -178,7 +189,7 @@ out:
   } else {
     GST_DEBUG_OBJECT (mem->allocator, "created linux_dmabuf wl_buffer (%p):"
         "%dx%d, fmt=%" GST_FOURCC_FORMAT ", %d planes",
-        data.wbuf, width, height, GST_FOURCC_ARGS (format), nplanes);
+        data.wbuf, width, height, GST_FOURCC_ARGS (fourcc), nplanes);
   }
 
   g_mutex_unlock (&data.lock);

subprojects/gst-plugins-bad/gst-libs/gst/wayland/gstwllinuxdmabuf.h

@@ -37,6 +37,6 @@ void gst_wl_linux_dmabuf_init_once (void);
 
 GST_WL_API
 struct wl_buffer *gst_wl_linux_dmabuf_construct_wl_buffer (GstBuffer * buf,
-GstWlDisplay * display, const GstVideoInfo * info, guint64 modifier);
+    GstWlDisplay * display, const GstVideoInfoDmaDrm * drm_info);
 
 G_END_DECLS

subprojects/gst-plugins-bad/gst-libs/gst/wayland/gstwlshmallocator.c

@@ -219,8 +219,8 @@ gst_wl_shm_memory_construct_wl_buffer (GstMemory * mem, GstWlDisplay * display,
 
   g_return_val_if_fail (gst_is_fd_memory (mem), NULL);
   g_return_val_if_fail (size <= memsize, NULL);
-  g_return_val_if_fail (gst_wl_display_check_format_for_shm (display,
+  g_return_val_if_fail (gst_wl_display_check_format_for_shm (display, info),
-          GST_VIDEO_INFO_FORMAT (info), DRM_FORMAT_MOD_INVALID), NULL);
+      NULL);
 
   GST_DEBUG_OBJECT (display, "Creating wl_buffer from SHM of size %"
       G_GSSIZE_FORMAT " (%d x %d, stride %d), format %s", size, width, height,