bin: When removing a sink, check if the EOS status changed.
authorJan Schmidt <jan@centricular.com>
Thu, 29 Oct 2020 13:45:42 +0000 (00:45 +1100)
committerJan Schmidt <jan@centricular.com>
Thu, 29 Oct 2020 16:56:02 +0000 (03:56 +1100)
Removing a sink that hasn't posted EOS might change the bin itself
to EOS if it's the last remaining non-EOSed sink.

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

gst/gstbin.c
tests/check/gst/gstbin.c

index 799b0ae..b0326f4 100644 (file)
@@ -1576,7 +1576,7 @@ gst_bin_remove_func (GstBin * bin, GstElement * element)
   GstClock **provided_clock_p;
   GstElement **clock_provider_p;
   GList *walk, *next;
-  gboolean other_async, this_async, have_no_preroll;
+  gboolean other_async, this_async, have_no_preroll, removed_eos;
   GstStateChangeReturn ret;
 
   GST_DEBUG_OBJECT (bin, "element :%s", GST_ELEMENT_NAME (element));
@@ -1703,6 +1703,8 @@ gst_bin_remove_func (GstBin * bin, GstElement * element)
    * async state. */
   this_async = FALSE;
   other_async = FALSE;
+  /* If we remove an EOSed element, the bin might go EOS */
+  removed_eos = FALSE;
   for (walk = bin->messages; walk; walk = next) {
     GstMessage *message = (GstMessage *) walk->data;
     GstElement *src = GST_ELEMENT_CAST (GST_MESSAGE_SRC (message));
@@ -1736,6 +1738,10 @@ gst_bin_remove_func (GstBin * bin, GstElement * element)
           remove = TRUE;
         break;
       }
+      case GST_MESSAGE_EOS:
+        if (src == element)
+          removed_eos = TRUE;
+        break;
       default:
         break;
     }
@@ -1798,6 +1804,15 @@ no_state_recalc:
   gst_element_set_clock (element, NULL);
   GST_OBJECT_UNLOCK (bin);
 
+  /* If the element was a sink that had not posted EOS,
+   * it might have been the last one we were waiting for,
+   * so check if it's time to send EOS now */
+  if (is_sink && !removed_eos) {
+    GST_DEBUG_OBJECT (bin,
+        "Removing sink that had not EOSed. Re-checking overall EOS status");
+    bin_do_eos (bin);
+  }
+
   if (clock_message)
     gst_element_post_message (GST_ELEMENT_CAST (bin), clock_message);
 
index 8a57f89..1a88426 100644 (file)
@@ -286,6 +286,68 @@ GST_START_TEST (test_eos)
 
 GST_END_TEST;
 
+GST_START_TEST (test_eos_recheck)
+{
+  GstBus *bus;
+  GstElement *pipeline, *sink1, *sink2;
+  GstMessage *message;
+  GstPad *pad1;
+  GThread *thread1;
+
+  pipeline = gst_pipeline_new ("test_eos_recheck");
+  bus = gst_pipeline_get_bus (GST_PIPELINE (pipeline));
+
+  sink1 = gst_element_factory_make ("fakesink", "sink1");
+  sink2 = gst_element_factory_make ("fakesink", "sink2");
+
+  gst_bin_add_many (GST_BIN (pipeline), sink1, sink2, NULL);
+
+  /* Set async=FALSE so we don't wait for preroll */
+  g_object_set (sink1, "async", FALSE, NULL);
+  g_object_set (sink2, "async", FALSE, NULL);
+
+  pad1 = gst_check_setup_src_pad_by_name (sink1, &srctemplate, "sink");
+
+  gst_pad_set_active (pad1, TRUE);
+
+  fail_if (gst_element_set_state (GST_ELEMENT (pipeline),
+          GST_STATE_PLAYING) == GST_STATE_CHANGE_FAILURE);
+  fail_unless (gst_element_get_state (GST_ELEMENT (pipeline), NULL, NULL,
+          GST_CLOCK_TIME_NONE) == GST_STATE_CHANGE_SUCCESS);
+
+  /* Send one EOS to sink1 */
+  thread1 = g_thread_new ("thread1", (GThreadFunc) push_one_eos, pad1);
+
+  /* Make sure the EOS message is not sent */
+  message =
+      gst_bus_poll (bus, GST_MESSAGE_ERROR | GST_MESSAGE_EOS, 2 * GST_SECOND);
+  fail_if (message != NULL);
+
+  /* Remove sink2 without it EOSing, which should trigger an EOS re-check */
+  gst_object_ref (sink2);
+  gst_bin_remove (GST_BIN (pipeline), sink2);
+  gst_element_set_state (GST_ELEMENT (sink2), GST_STATE_NULL);
+
+  /* Make sure the EOS message is sent then */
+  message =
+      gst_bus_poll (bus, GST_MESSAGE_ERROR | GST_MESSAGE_EOS, 20 * GST_SECOND);
+  fail_if (message == NULL);
+  fail_unless (GST_MESSAGE_TYPE (message) == GST_MESSAGE_EOS);
+  gst_message_unref (message);
+
+  /* Cleanup */
+  g_thread_join (thread1);
+
+  gst_element_set_state (GST_ELEMENT (pipeline), GST_STATE_NULL);
+  gst_pad_set_active (pad1, FALSE);
+  gst_check_teardown_src_pad (sink1);
+  gst_object_unref (sink2);
+  gst_object_unref (bus);
+  gst_object_unref (pipeline);
+}
+
+GST_END_TEST;
+
 GST_START_TEST (test_stream_start)
 {
   GstBus *bus;
@@ -1867,6 +1929,7 @@ gst_bin_suite (void)
   tcase_add_test (tc_chain, test_interface);
   tcase_add_test (tc_chain, test_iterate_all_by_element_factory_name);
   tcase_add_test (tc_chain, test_eos);
+  tcase_add_test (tc_chain, test_eos_recheck);
   tcase_add_test (tc_chain, test_stream_start);
   tcase_add_test (tc_chain, test_children_state_change_order_flagged_sink);
   tcase_add_test (tc_chain, test_children_state_change_order_semi_sink);