proxysink: Make sure stream-start and caps events are forwarded
authorSeungha Yang <seungha@centricular.com>
Tue, 5 Jul 2022 18:14:25 +0000 (03:14 +0900)
committerSeungha Yang <seungha@centricular.com>
Wed, 6 Jul 2022 20:42:21 +0000 (05:42 +0900)
There might be a sequence of event and buffer flow:
- Got stream-start/caps/segment events
- Got flush events
- And then buffers with a new segment event

In the above case, stream-start and caps event might not be reached to
peer proxysrc if peer proxysrc is not ready to receive them.

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

subprojects/gst-plugins-bad/gst/proxy/gstproxysink.c
subprojects/gst-plugins-bad/gst/proxy/gstproxysink.h
subprojects/gst-plugins-bad/tests/check/elements/proxysink.c [new file with mode: 0644]
subprojects/gst-plugins-bad/tests/check/meson.build

index 9425dcc..9b5b805 100644 (file)
@@ -120,6 +120,8 @@ gst_proxy_sink_change_state (GstElement * element, GstStateChange transition)
   switch (transition) {
     case GST_STATE_CHANGE_READY_TO_PAUSED:
       self->pending_sticky_events = FALSE;
+      self->sent_stream_start = FALSE;
+      self->sent_caps = FALSE;
       break;
     default:
       break;
@@ -180,6 +182,7 @@ gst_proxy_sink_sink_query (GstPad * pad, GstObject * parent, GstQuery * query)
 
 typedef struct
 {
+  GstProxySink *self;
   GstPad *otherpad;
   GstFlowReturn ret;
 } CopyStickyEventsData;
@@ -189,12 +192,46 @@ copy_sticky_events (G_GNUC_UNUSED GstPad * pad, GstEvent ** event,
     gpointer user_data)
 {
   CopyStickyEventsData *data = user_data;
+  GstProxySink *self = data->self;
 
   data->ret = gst_pad_store_sticky_event (data->otherpad, *event);
+  switch (GST_EVENT_TYPE (*event)) {
+    case GST_EVENT_STREAM_START:
+      if (data->ret != GST_FLOW_OK)
+        self->sent_stream_start = FALSE;
+      else
+        self->sent_stream_start = TRUE;
+      break;
+    case GST_EVENT_CAPS:
+      if (data->ret != GST_FLOW_OK)
+        self->sent_caps = FALSE;
+      else
+        self->sent_caps = TRUE;
+      break;
+    default:
+      break;
+  }
 
   return data->ret == GST_FLOW_OK;
 }
 
+static void
+gst_proxy_sink_send_sticky_events (GstProxySink * self, GstPad * pad,
+    GstPad * otherpad)
+{
+  if (self->pending_sticky_events || !self->sent_stream_start ||
+      !self->sent_caps) {
+    CopyStickyEventsData data;
+
+    data.self = self;
+    data.otherpad = otherpad;
+    data.ret = GST_FLOW_OK;
+
+    gst_pad_sticky_events_foreach (pad, copy_sticky_events, &data);
+    self->pending_sticky_events = data.ret != GST_FLOW_OK;
+  }
+}
+
 static gboolean
 gst_proxy_sink_sink_event (GstPad * pad, GstObject * parent, GstEvent * event)
 {
@@ -202,10 +239,11 @@ gst_proxy_sink_sink_event (GstPad * pad, GstObject * parent, GstEvent * event)
   GstProxySrc *src;
   gboolean ret = FALSE;
   gboolean sticky = GST_EVENT_IS_STICKY (event);
+  GstEventType event_type = GST_EVENT_TYPE (event);
 
   GST_LOG_OBJECT (pad, "Got %s event", GST_EVENT_TYPE_NAME (event));
 
-  if (GST_EVENT_TYPE (event) == GST_EVENT_FLUSH_STOP)
+  if (event_type == GST_EVENT_FLUSH_STOP)
     self->pending_sticky_events = FALSE;
 
   src = g_weak_ref_get (&self->proxysrc);
@@ -213,17 +251,24 @@ gst_proxy_sink_sink_event (GstPad * pad, GstObject * parent, GstEvent * event)
     GstPad *srcpad;
     srcpad = gst_proxy_src_get_internal_srcpad (src);
 
-    if (sticky && self->pending_sticky_events) {
-      CopyStickyEventsData data = { srcpad, GST_FLOW_OK };
-
-      gst_pad_sticky_events_foreach (pad, copy_sticky_events, &data);
-      self->pending_sticky_events = data.ret != GST_FLOW_OK;
-    }
+    if (sticky)
+      gst_proxy_sink_send_sticky_events (self, pad, srcpad);
 
     ret = gst_pad_push_event (srcpad, event);
     gst_object_unref (srcpad);
     gst_object_unref (src);
 
+    switch (event_type) {
+      case GST_EVENT_STREAM_START:
+        self->sent_stream_start = ret;
+        break;
+      case GST_EVENT_CAPS:
+        self->sent_caps = ret;
+        break;
+      default:
+        break;
+    }
+
     if (!ret && sticky) {
       self->pending_sticky_events = TRUE;
       ret = TRUE;
@@ -250,12 +295,7 @@ gst_proxy_sink_sink_chain (GstPad * pad, GstObject * parent, GstBuffer * buffer)
     GstPad *srcpad;
     srcpad = gst_proxy_src_get_internal_srcpad (src);
 
-    if (self->pending_sticky_events) {
-      CopyStickyEventsData data = { srcpad, GST_FLOW_OK };
-
-      gst_pad_sticky_events_foreach (pad, copy_sticky_events, &data);
-      self->pending_sticky_events = data.ret != GST_FLOW_OK;
-    }
+    gst_proxy_sink_send_sticky_events (self, pad, srcpad);
 
     ret = gst_pad_push (srcpad, buffer);
     gst_object_unref (srcpad);
@@ -286,12 +326,7 @@ gst_proxy_sink_sink_chain_list (GstPad * pad, GstObject * parent,
     GstPad *srcpad;
     srcpad = gst_proxy_src_get_internal_srcpad (src);
 
-    if (self->pending_sticky_events) {
-      CopyStickyEventsData data = { srcpad, GST_FLOW_OK };
-
-      gst_pad_sticky_events_foreach (pad, copy_sticky_events, &data);
-      self->pending_sticky_events = data.ret != GST_FLOW_OK;
-    }
+    gst_proxy_sink_send_sticky_events (self, pad, srcpad);
 
     ret = gst_pad_push_list (srcpad, list);
     gst_object_unref (srcpad);
index 9bd382c..4bbb9fc 100644 (file)
@@ -48,6 +48,8 @@ struct _GstProxySink {
 
   /* Whether there are sticky events pending */
   gboolean pending_sticky_events;
+  gboolean sent_stream_start;
+  gboolean sent_caps;
 };
 
 struct _GstProxySinkClass {
diff --git a/subprojects/gst-plugins-bad/tests/check/elements/proxysink.c b/subprojects/gst-plugins-bad/tests/check/elements/proxysink.c
new file mode 100644 (file)
index 0000000..44f84a5
--- /dev/null
@@ -0,0 +1,110 @@
+/* GStreamer
+ * Copyright (C) 2022 Seungha Yang <seungha@centricular.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public
+ * License along with this library; if not, write to the
+ * Free Software Foundation, Inc., 51 Franklin St, Fifth Floor,
+ * Boston, MA 02110-1301, USA.
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include <gst/gst.h>
+#include <gst/check/gstcheck.h>
+#include <gst/check/gstharness.h>
+
+GST_START_TEST (test_flush_before_buffer)
+{
+  GstElement *sink, *src;
+  GstHarness *h_in;
+  GstHarness *h_out;
+  GstEvent *event;
+  GstSegment segment;
+  GstCaps *caps;
+  GstBuffer *buf;
+
+  sink = gst_element_factory_make ("proxysink", NULL);
+  src = gst_element_factory_make ("proxysrc", NULL);
+
+  g_object_set (src, "proxysink", sink, NULL);
+
+  h_in = gst_harness_new_with_element (sink, "sink", NULL);
+  h_out = gst_harness_new_with_element (src, NULL, "src");
+  gst_object_unref (sink);
+  gst_object_unref (src);
+
+  /* Activate only input side first, then push sticky events
+   * without buffer */
+  gst_harness_play (h_in);
+
+  event = gst_event_new_stream_start ("proxy-test-stream-start");
+  fail_unless (gst_harness_push_event (h_in, event));
+
+  caps = gst_caps_from_string ("foo/bar");
+  event = gst_event_new_caps (caps);
+  gst_caps_unref (caps);
+  fail_unless (gst_harness_push_event (h_in, event));
+
+  gst_segment_init (&segment, GST_FORMAT_TIME);
+  event = gst_event_new_segment (&segment);
+  fail_unless (gst_harness_push_event (h_in, event));
+
+  /* Now activate output side, sticky event and buffers should be
+   * serialized */
+  gst_harness_play (h_out);
+
+  event = gst_event_new_flush_start ();
+  fail_unless (gst_harness_push_event (h_in, event));
+
+  event = gst_event_new_flush_stop (TRUE);
+  fail_unless (gst_harness_push_event (h_in, event));
+
+  event = gst_event_new_segment (&segment);
+  fail_unless (gst_harness_push_event (h_in, event));
+
+  buf = gst_buffer_new_and_alloc (4);
+  GST_BUFFER_PTS (buf) = 0;
+  GST_BUFFER_DTS (buf) = 0;
+
+  /* There must be no critical warning regarding
+   * sticky-event and buffer flow order*/
+  fail_unless_equals_int (gst_harness_push (h_in, buf), GST_FLOW_OK);
+
+  event = gst_event_new_eos ();
+  fail_unless (gst_harness_push_event (h_in, event));
+
+  /* make sure everything has been forwarded */
+  fail_unless (gst_harness_pull_until_eos (h_out, &buf));
+  gst_buffer_unref (buf);
+
+  gst_harness_teardown (h_in);
+  gst_harness_teardown (h_out);
+}
+
+GST_END_TEST;
+
+static Suite *
+proxysink_suite (void)
+{
+  Suite *s = suite_create ("proxysink");
+  TCase *tc_basic = tcase_create ("general");
+
+  suite_add_tcase (s, tc_basic);
+  tcase_add_test (tc_basic, test_flush_before_buffer);
+
+  return s;
+}
+
+GST_CHECK_MAIN (proxysink);
index 55b3087..2e78d3e 100644 (file)
@@ -59,6 +59,7 @@ base_tests = [
    [['elements/openjpeg.c'], not openjpeg_dep.found(), [openjpeg_dep]],
   [['elements/pcapparse.c'], false, [libparser_dep]],
   [['elements/pnm.c'], get_option('pnm').disabled()],
+  [['elements/proxysink.c'], get_option('proxy').disabled()],
   [['elements/ristrtpext.c']],
   [['elements/rtponvifparse.c'], get_option('onvif').disabled()],
   [['elements/rtponviftimestamp.c'], get_option('onvif').disabled()],