waylandsink: Move buffer commits to the display thread

Syncrhonizing buffer commits to the streaming thread can lead to
dropped frames when frame callbacks are not processed before the
next frame is ready for rendering.  Depending on the drift between
the wayland compositor and buffer source timings, this can lead to
periods of significant frame drop, especially when the media frame
rate is close to the display frame rate.

Cache buffers in the streaming thread and peform commits on the
display thread to eliminate the buffer commit racing.

The implementation is the same for both waylandsink and gtkwaylandsink,
so move it to the common wayland library under gst-lib.

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

diff --git a/subprojects/gst-plugins-bad/ext/gtk/gstgtkwaylandsink.c b/subprojects/gst-plugins-bad/ext/gtk/gstgtkwaylandsink.c
index cc51b0328bbeb14ae22f84ad600695fa334d8d21..fd6989cf49b3feaf3f727574ee1306a0d7813a89 100644 (file)
--- a/subprojects/gst-plugins-bad/ext/gtk/gstgtkwaylandsink.c
+++ b/subprojects/gst-plugins-bad/ext/gtk/gstgtkwaylandsink.c
@@ -107,15 +107,12 @@ typedef struct _GstGtkWaylandSinkPrivate
   GstVideoInfoDmaDrm drm_info;
   GstCaps *caps;
 
-  gboolean redraw_pending;
   GMutex render_lock;
 
   GstVideoOrientationMethod sink_rotate_method;
   GstVideoOrientationMethod tag_rotate_method;
   GstVideoOrientationMethod current_rotate_method;
 
-  struct wl_callback *callback;
-
   gchar *drm_device;
   gboolean skip_dumb_buffer_copy;
 } GstGtkWaylandSinkPrivate;
@@ -761,14 +758,6 @@ gst_gtk_wayland_sink_change_state (GstElement * element,
         /* remove buffer from surface, show nothing */
         gst_wl_window_render (priv->wl_window, NULL, NULL);
       }
-
-      g_mutex_lock (&priv->render_lock);
-      if (priv->callback) {
-        wl_callback_destroy (priv->callback);
-        priv->callback = NULL;
-      }
-      priv->redraw_pending = FALSE;
-      g_mutex_unlock (&priv->render_lock);
       break;
     default:
       break;
@@ -1075,56 +1064,25 @@ gst_gtk_wayland_sink_propose_allocation (GstBaseSink * bsink, GstQuery * query)
   return TRUE;
 }
 
-static void
-frame_redraw_callback (void *data, struct wl_callback *callback, uint32_t time)
-{
-  GstGtkWaylandSink *self = data;
-  GstGtkWaylandSinkPrivate *priv =
-      gst_gtk_wayland_sink_get_instance_private (self);
-
-  GST_LOG_OBJECT (self, "frame_redraw_cb");
-
-  g_mutex_lock (&priv->render_lock);
-  priv->redraw_pending = FALSE;
-
-  if (priv->callback) {
-    wl_callback_destroy (callback);
-    priv->callback = NULL;
-  }
-  g_mutex_unlock (&priv->render_lock);
-}
-
-static const struct wl_callback_listener frame_callback_listener = {
-  frame_redraw_callback
-};
-
 /* must be called with the render lock */
-static void
+static gboolean
 render_last_buffer (GstGtkWaylandSink * self, gboolean redraw)
 {
   GstGtkWaylandSinkPrivate *priv =
       gst_gtk_wayland_sink_get_instance_private (self);
   GstWlBuffer *wlbuffer;
   const GstVideoInfo *info = NULL;
-  struct wl_surface *surface;
-  struct wl_callback *callback;
 
   if (!priv->wl_window)
-    return;
+    return FALSE;
 
   wlbuffer = gst_buffer_get_wl_buffer (priv->display, priv->last_buffer);
-  surface = gst_wl_window_get_wl_surface (priv->wl_window);
-
-  priv->redraw_pending = TRUE;
-  callback = wl_surface_frame (surface);
-  priv->callback = callback;
-  wl_callback_add_listener (callback, &frame_callback_listener, self);
 
   if (G_UNLIKELY (priv->video_info_changed && !redraw)) {
     info = &priv->video_info;
     priv->video_info_changed = FALSE;
   }
-  gst_wl_window_render (priv->wl_window, wlbuffer, info);
+  return gst_wl_window_render (priv->wl_window, wlbuffer, info);
 }
 
 static GstFlowReturn
@@ -1151,14 +1109,6 @@ gst_gtk_wayland_sink_show_frame (GstVideoSink * vsink, GstBuffer * buffer)
     goto done;
   }
 
-  /* drop buffers until we get a frame callback */
-  if (priv->redraw_pending) {
-    GST_LOG_OBJECT (self, "buffer %" GST_PTR_FORMAT " dropped (redraw pending)",
-        buffer);
-    ret = GST_BASE_SINK_FLOW_DROPPED;
-    goto done;
-  }
-
   /* make sure that the application has called set_render_rectangle() */
   if (G_UNLIKELY (gst_wl_window_get_render_rectangle (priv->wl_window)->w == 0))
     goto no_window_size;
@@ -1319,7 +1269,8 @@ render:
   }
 
   gst_buffer_replace (&priv->last_buffer, to_render);
-  render_last_buffer (self, FALSE);
+  if (!render_last_buffer (self, FALSE))
+    ret = GST_BASE_SINK_FLOW_DROPPED;
 
   if (buffer != to_render)
     gst_buffer_unref (to_render);

diff --git a/subprojects/gst-plugins-bad/ext/wayland/gstwaylandsink.c b/subprojects/gst-plugins-bad/ext/wayland/gstwaylandsink.c
index a9c6f670ea1f1854ad20a151211b7896522b4083..c2620e472f93b1fb0bab326549c279da5e75db34 100644 (file)
--- a/subprojects/gst-plugins-bad/ext/wayland/gstwaylandsink.c
+++ b/subprojects/gst-plugins-bad/ext/wayland/gstwaylandsink.c
@@ -448,13 +448,6 @@ gst_wayland_sink_change_state (GstElement * element, GstStateChange transition)
         }
       }
 
-      g_mutex_lock (&self->render_lock);
-      if (self->callback) {
-        wl_callback_destroy (self->callback);
-        self->callback = NULL;
-      }
-      self->redraw_pending = FALSE;
-      g_mutex_unlock (&self->render_lock);
       break;
     case GST_STATE_CHANGE_READY_TO_NULL:
       g_mutex_lock (&self->display_lock);
@@ -795,49 +788,20 @@ gst_wayland_sink_propose_allocation (GstBaseSink * bsink, GstQuery * query)
   return TRUE;
 }
 
-static void
-frame_redraw_callback (void *data, struct wl_callback *callback, uint32_t time)
-{
-  GstWaylandSink *self = data;
-
-  GST_LOG_OBJECT (self, "frame_redraw_cb");
-
-  g_mutex_lock (&self->render_lock);
-  self->redraw_pending = FALSE;
-
-  if (self->callback) {
-    wl_callback_destroy (callback);
-    self->callback = NULL;
-  }
-  g_mutex_unlock (&self->render_lock);
-}
-
-static const struct wl_callback_listener frame_callback_listener = {
-  frame_redraw_callback
-};
-
 /* must be called with the render lock */
-static void
+static gboolean
 render_last_buffer (GstWaylandSink * self, gboolean redraw)
 {
   GstWlBuffer *wlbuffer;
   const GstVideoInfo *info = NULL;
-  struct wl_surface *surface;
-  struct wl_callback *callback;
 
   wlbuffer = gst_buffer_get_wl_buffer (self->display, self->last_buffer);
-  surface = gst_wl_window_get_wl_surface (self->window);
-
-  self->redraw_pending = TRUE;
-  callback = wl_surface_frame (surface);
-  self->callback = callback;
-  wl_callback_add_listener (callback, &frame_callback_listener, self);
 
   if (G_UNLIKELY (self->video_info_changed && !redraw)) {
     info = &self->video_info;
     self->video_info_changed = FALSE;
   }
-  gst_wl_window_render (self->window, wlbuffer, info);
+  return gst_wl_window_render (self->window, wlbuffer, info);
 }
 
 static void
@@ -883,14 +847,6 @@ gst_wayland_sink_show_frame (GstVideoSink * vsink, GstBuffer * buffer)
     }
   }
 
-  /* drop buffers until we get a frame callback */
-  if (self->redraw_pending) {
-    GST_LOG_OBJECT (self, "buffer %" GST_PTR_FORMAT " dropped (redraw pending)",
-        buffer);
-    ret = GST_BASE_SINK_FLOW_DROPPED;
-    goto done;
-  }
-
   /* make sure that the application has called set_render_rectangle() */
   if (G_UNLIKELY (gst_wl_window_get_render_rectangle (self->window)->w == 0))
     goto no_window_size;
@@ -1050,7 +1006,8 @@ render:
   }
 
   gst_buffer_replace (&self->last_buffer, to_render);
-  render_last_buffer (self, FALSE);
+  if (!render_last_buffer (self, FALSE))
+    ret = GST_BASE_SINK_FLOW_DROPPED;
 
   if (buffer != to_render)
     gst_buffer_unref (to_render);
@@ -1194,7 +1151,7 @@ gst_wayland_sink_expose (GstVideoOverlay * overlay)
   GST_DEBUG_OBJECT (self, "expose");
 
   g_mutex_lock (&self->render_lock);
-  if (self->last_buffer && !self->redraw_pending) {
+  if (self->last_buffer) {
     GST_DEBUG_OBJECT (self, "redrawing last buffer");
     render_last_buffer (self, TRUE);
   }

diff --git a/subprojects/gst-plugins-bad/ext/wayland/gstwaylandsink.h b/subprojects/gst-plugins-bad/ext/wayland/gstwaylandsink.h
index f5c56015c366da7a947c518c639c1dc3d61643be..5bbb10a8e35eac2a3dc2f37c078574cad5069551 100644 (file)
--- a/subprojects/gst-plugins-bad/ext/wayland/gstwaylandsink.h
+++ b/subprojects/gst-plugins-bad/ext/wayland/gstwaylandsink.h
@@ -60,7 +60,6 @@ struct _GstWaylandSink
 
   gchar *display_name;
 
-  gboolean redraw_pending;
   GMutex render_lock;
   GstBuffer *last_buffer;
 
@@ -68,8 +67,6 @@ struct _GstWaylandSink
   GstVideoOrientationMethod tag_rotate_method;
   GstVideoOrientationMethod current_rotate_method;
 
-  struct wl_callback *callback;
-
   gchar *drm_device;
   gboolean skip_dumb_buffer_copy;
 };

diff --git a/subprojects/gst-plugins-bad/gst-libs/gst/wayland/gstwlwindow.c b/subprojects/gst-plugins-bad/gst-libs/gst/wayland/gstwlwindow.c
index 1a129bc44f49c6f02c937bbed8b53ec09e3f3b7c..6a662be26efc3fb0aece8173f4ac4a29265aa378 100644 (file)
--- a/subprojects/gst-plugins-bad/gst-libs/gst/wayland/gstwlwindow.c
+++ b/subprojects/gst-plugins-bad/gst-libs/gst/wayland/gstwlwindow.c
@@ -72,6 +72,14 @@ typedef struct _GstWlWindowPrivate
   /* when this is not set both the area_surface and the video_surface are not
    * visible and certain steps should be skipped */
   gboolean is_area_surface_mapped;
+
+  GMutex window_lock;
+  GstWlBuffer *next_buffer;
+  GstVideoInfo *next_video_info;
+  GstWlBuffer *staged_buffer;
+  gboolean clear_window;
+  struct wl_callback *frame_callback;
+  struct wl_callback *commit_callback;
 } GstWlWindowPrivate;
 
 G_DEFINE_TYPE_WITH_CODE (GstWlWindow, gst_wl_window, G_TYPE_OBJECT,
@@ -93,6 +101,9 @@ static void gst_wl_window_finalize (GObject * gobject);
 
 static void gst_wl_window_update_borders (GstWlWindow * self);
 
+static void gst_wl_window_commit_buffer (GstWlWindow * self,
+    GstWlBuffer * buffer);
+
 static void
 handle_xdg_toplevel_close (void *data, struct xdg_toplevel *xdg_toplevel)
 {
@@ -173,6 +184,7 @@ gst_wl_window_init (GstWlWindow * self)
   priv->configured = TRUE;
   g_cond_init (&priv->configure_cond);
   g_mutex_init (&priv->configure_mutex);
+  g_mutex_init (&priv->window_lock);
 }
 
 static void
@@ -181,6 +193,9 @@ gst_wl_window_finalize (GObject * gobject)
   GstWlWindow *self = GST_WL_WINDOW (gobject);
   GstWlWindowPrivate *priv = gst_wl_window_get_instance_private (self);
 
+  gst_wl_display_callback_destroy (priv->display, &priv->frame_callback);
+  gst_wl_display_callback_destroy (priv->display, &priv->commit_callback);
+
   if (priv->xdg_toplevel)
     xdg_toplevel_destroy (priv->xdg_toplevel);
   if (priv->xdg_surface)
@@ -499,11 +514,40 @@ gst_wl_window_set_opaque (GstWlWindow * self, const GstVideoInfo * info)
   }
 }
 
-void
-gst_wl_window_render (GstWlWindow * self, GstWlBuffer * buffer,
-    const GstVideoInfo * info)
+static void
+frame_redraw_callback (void *data, struct wl_callback *callback, uint32_t time)
+{
+  GstWlWindow *self = data;
+  GstWlWindowPrivate *priv = gst_wl_window_get_instance_private (self);
+  GstWlBuffer *next_buffer;
+
+  GST_INFO ("frame_redraw_cb ");
+
+  wl_callback_destroy (callback);
+  priv->frame_callback = NULL;
+
+  g_mutex_lock (&priv->window_lock);
+  next_buffer = priv->next_buffer = priv->staged_buffer;
+  priv->staged_buffer = NULL;
+  g_mutex_unlock (&priv->window_lock);
+
+  if (next_buffer || priv->clear_window)
+    gst_wl_window_commit_buffer (self, next_buffer);
+
+  if (next_buffer)
+    gst_wl_buffer_unref_buffer (next_buffer);
+}
+
+static const struct wl_callback_listener frame_callback_listener = {
+  frame_redraw_callback
+};
+
+static void
+gst_wl_window_commit_buffer (GstWlWindow * self, GstWlBuffer * buffer)
 {
   GstWlWindowPrivate *priv = gst_wl_window_get_instance_private (self);
+  GstVideoInfo *info = priv->next_video_info;
+  struct wl_callback *callback;
 
   if (G_UNLIKELY (info)) {
     priv->scaled_width =
@@ -517,6 +561,9 @@ gst_wl_window_render (GstWlWindow * self, GstWlBuffer * buffer,
   }
 
   if (G_LIKELY (buffer)) {
+    callback = wl_surface_frame (priv->video_surface_wrapper);
+    priv->frame_callback = callback;
+    wl_callback_add_listener (callback, &frame_callback_listener, self);
     gst_wl_buffer_attach (buffer, priv->video_surface_wrapper);
     wl_surface_damage_buffer (priv->video_surface_wrapper, 0, 0, G_MAXINT32,
         G_MAXINT32);
@@ -535,6 +582,7 @@ gst_wl_window_render (GstWlWindow * self, GstWlBuffer * buffer,
     wl_surface_attach (priv->area_surface_wrapper, NULL, 0, 0);
     wl_surface_commit (priv->area_surface_wrapper);
     priv->is_area_surface_mapped = FALSE;
+    priv->clear_window = FALSE;
   }
 
   if (G_UNLIKELY (info)) {
@@ -542,9 +590,70 @@ gst_wl_window_render (GstWlWindow * self, GstWlBuffer * buffer,
      * the position of the video_subsurface */
     wl_surface_commit (priv->area_surface_wrapper);
     wl_subsurface_set_desync (priv->video_subsurface);
+    gst_video_info_free (priv->next_video_info);
+    priv->next_video_info = NULL;
+  }
+
+}
+
+static void
+commit_callback (void *data, struct wl_callback *callback, uint32_t serial)
+{
+  GstWlWindow *self = data;
+  GstWlWindowPrivate *priv = gst_wl_window_get_instance_private (self);
+  GstWlBuffer *next_buffer;
+
+  wl_callback_destroy (callback);
+  priv->commit_callback = NULL;
+
+  g_mutex_lock (&priv->window_lock);
+  next_buffer = priv->next_buffer;
+  g_mutex_unlock (&priv->window_lock);
+
+  gst_wl_window_commit_buffer (self, next_buffer);
+
+  if (next_buffer)
+    gst_wl_buffer_unref_buffer (next_buffer);
+}
+
+static const struct wl_callback_listener commit_listener = {
+  commit_callback
+};
+
+gboolean
+gst_wl_window_render (GstWlWindow * self, GstWlBuffer * buffer,
+    const GstVideoInfo * info)
+{
+  GstWlWindowPrivate *priv = gst_wl_window_get_instance_private (self);
+  gboolean ret = TRUE;
+
+  if (G_LIKELY (buffer))
+    gst_wl_buffer_ref_gst_buffer (buffer);
+
+  g_mutex_lock (&priv->window_lock);
+  if (G_UNLIKELY (info))
+    priv->next_video_info = gst_video_info_copy (info);
+
+  if (priv->next_buffer && priv->staged_buffer) {
+    GST_LOG_OBJECT (self, "buffer %p dropped (replaced)", priv->staged_buffer);
+    gst_wl_buffer_unref_buffer (priv->staged_buffer);
+    ret = FALSE;
+  }
+
+  if (!priv->next_buffer) {
+    priv->next_buffer = buffer;
+    priv->commit_callback =
+        gst_wl_display_sync (priv->display, &commit_listener, self);
+    wl_display_flush (gst_wl_display_get_display (priv->display));
+  } else {
+    priv->staged_buffer = buffer;
   }
+  if (!buffer)
+    priv->clear_window = TRUE;
+
+  g_mutex_unlock (&priv->window_lock);
 
-  wl_display_flush (gst_wl_display_get_display (priv->display));
+  return ret;
 }
 
 /* Update the buffer used to draw black borders. When we have viewporter

diff --git a/subprojects/gst-plugins-bad/gst-libs/gst/wayland/gstwlwindow.h b/subprojects/gst-plugins-bad/gst-libs/gst/wayland/gstwlwindow.h
index f3b338202b1b848517818389ade5a9d16f45a61d..4cd85ac36a4cb91c640a50aeaa6ccfe614d48633 100644 (file)
--- a/subprojects/gst-plugins-bad/gst-libs/gst/wayland/gstwlwindow.h
+++ b/subprojects/gst-plugins-bad/gst-libs/gst/wayland/gstwlwindow.h
@@ -60,7 +60,7 @@ GST_WL_API
 gboolean gst_wl_window_is_toplevel (GstWlWindow * self);
 
 GST_WL_API
-void gst_wl_window_render (GstWlWindow * self, GstWlBuffer * buffer,
+gboolean gst_wl_window_render (GstWlWindow * self, GstWlBuffer * buffer,
         const GstVideoInfo * info);
 
 GST_WL_API