alphacombine: Fix for early allocation queries
authorNicolas Dufresne <nicolas.dufresne@collabora.com>
Fri, 10 Dec 2021 20:18:56 +0000 (15:18 -0500)
committerGStreamer Marge Bot <gitlab-merge-bot@gstreamer-foundation.org>
Fri, 10 Dec 2021 21:37:14 +0000 (21:37 +0000)
When using playbin3, it seems that the alpha decode is always first to
push caps and run an allocation query. As the format change from sink
and alpha were not synchronized, the allocation query could endup
being run before the caps are pushed. That may lead to failing query,
which makes the decoder thinks there is no GstVideoMeta downstream and
most likely CPU copy the frame.

This patch implements a format cookie to track and synchronize the
format changes on both pads fixing the racy performance issue.

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

subprojects/gst-plugins-bad/gst/codecalpha/gstalphacombine.c

index aeefeb9..b5f2bc8 100644 (file)
@@ -109,6 +109,9 @@ struct _GstAlphaCombine
   GstVideoInfo sink_vinfo;
   GstVideoInfo alpha_vinfo;
   GstVideoFormat src_format;
+
+  guint sink_format_cookie;
+  guint alpha_format_cookie;
 };
 
 #define gst_alpha_combine_parent_class parent_class
@@ -156,6 +159,13 @@ gst_alpha_combine_unlock_stop (GstAlphaCombine * self)
   g_mutex_lock (&self->buffer_lock);
   g_assert (self->flushing);
   self->flushing--;
+
+  /* Reset the format cookies to ensure they are equal */
+  if (!self->flushing) {
+    self->sink_format_cookie = 0;
+    self->alpha_format_cookie = 0;
+  }
+
   g_mutex_unlock (&self->buffer_lock);
 }
 
@@ -382,6 +392,7 @@ gst_alpha_combine_set_sink_format (GstAlphaCombine * self, GstCaps * caps)
   GstVideoFormat sink_format, src_format = GST_VIDEO_FORMAT_UNKNOWN;
   GstEvent *event;
   gint i;
+  gboolean ret;
 
   if (!gst_video_info_from_caps (&self->sink_vinfo, caps)) {
     GST_ELEMENT_ERROR (self, STREAM, FORMAT, ("Invalid video format"), (NULL));
@@ -412,7 +423,15 @@ gst_alpha_combine_set_sink_format (GstAlphaCombine * self, GstCaps * caps)
   event = gst_event_new_caps (caps);
   gst_caps_unref (caps);
 
-  return gst_pad_push_event (self->src_pad, event);
+  ret = gst_pad_push_event (self->src_pad, event);
+
+  /* signal the sink format change */
+  g_mutex_lock (&self->buffer_lock);
+  self->sink_format_cookie++;
+  g_cond_signal (&self->buffer_cond);
+  g_mutex_unlock (&self->buffer_lock);
+
+  return ret;
 }
 
 static gboolean
@@ -438,6 +457,13 @@ gst_alpha_combine_set_alpha_format (GstAlphaCombine * self, GstCaps * caps)
     return FALSE;
   }
 
+  self->alpha_format_cookie++;
+
+  /* wait for the matching format change on the sink pad */
+  while (self->alpha_format_cookie != self->sink_format_cookie &&
+      !self->flushing)
+    g_cond_wait (&self->buffer_cond, &self->buffer_lock);
+
   g_mutex_unlock (&self->buffer_lock);
 
   return TRUE;
@@ -560,6 +586,8 @@ gst_alpha_combine_change_state (GstElement * element, GstStateChange transition)
       self->src_format = GST_VIDEO_FORMAT_UNKNOWN;
       gst_video_info_init (&self->sink_vinfo);
       gst_video_info_init (&self->alpha_vinfo);
+      self->sink_format_cookie = 0;
+      self->alpha_format_cookie = 0;
       break;
     default:
       break;