From 5cf0b1eeac5334e5ce797181c7dedd56e34abb16 Mon Sep 17 00:00:00 2001 From: Seungha Yang Date: Wed, 6 Jul 2022 03:14:25 +0900 Subject: [PATCH] proxysink: Make sure stream-start and caps events are forwarded 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: --- .../gst-plugins-bad/gst/proxy/gstproxysink.c | 73 +++++++++--- .../gst-plugins-bad/gst/proxy/gstproxysink.h | 2 + .../tests/check/elements/proxysink.c | 110 ++++++++++++++++++ .../gst-plugins-bad/tests/check/meson.build | 1 + 4 files changed, 167 insertions(+), 19 deletions(-) create mode 100644 subprojects/gst-plugins-bad/tests/check/elements/proxysink.c diff --git a/subprojects/gst-plugins-bad/gst/proxy/gstproxysink.c b/subprojects/gst-plugins-bad/gst/proxy/gstproxysink.c index 9425dccc49..9b5b80507a 100644 --- a/subprojects/gst-plugins-bad/gst/proxy/gstproxysink.c +++ b/subprojects/gst-plugins-bad/gst/proxy/gstproxysink.c @@ -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); diff --git a/subprojects/gst-plugins-bad/gst/proxy/gstproxysink.h b/subprojects/gst-plugins-bad/gst/proxy/gstproxysink.h index 9bd382c451..4bbb9fc657 100644 --- a/subprojects/gst-plugins-bad/gst/proxy/gstproxysink.h +++ b/subprojects/gst-plugins-bad/gst/proxy/gstproxysink.h @@ -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 index 0000000000..44f84a5aba --- /dev/null +++ b/subprojects/gst-plugins-bad/tests/check/elements/proxysink.c @@ -0,0 +1,110 @@ +/* GStreamer + * Copyright (C) 2022 Seungha Yang + * + * 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 +#include +#include + +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); diff --git a/subprojects/gst-plugins-bad/tests/check/meson.build b/subprojects/gst-plugins-bad/tests/check/meson.build index 71cc100fae..cc0d7ab7f6 100644 --- a/subprojects/gst-plugins-bad/tests/check/meson.build +++ b/subprojects/gst-plugins-bad/tests/check/meson.build @@ -57,6 +57,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()],