diff --git a/ChangeLog b/ChangeLog index 2758fa9f88..ed9f10d733 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,17 @@ +2007-10-08 Wim Taymans + + * libs/gst/base/gstbasesink.c: (gst_base_sink_preroll_queue_flush), + (gst_base_sink_queue_object_unlocked), + (gst_base_sink_queue_object), (gst_base_sink_event), + (gst_base_sink_needs_preroll), (gst_base_sink_chain_unlocked): + When we received EOS and are waiting for when to post the EOS message, + our state is prerolled and we should not return ASYNC. + Reorganize some code paths to implement this behavior. + + * tests/check/generic/sinks.c: (send_eos), (GST_START_TEST), + (gst_sinks_suite): + Add unit test to verify above EOS fix. + 2007-10-08 Wim Taymans * plugins/elements/gsttypefindelement.c: diff --git a/libs/gst/base/gstbasesink.c b/libs/gst/base/gstbasesink.c index c93ddb1190..2f446c2b9a 100644 --- a/libs/gst/base/gstbasesink.c +++ b/libs/gst/base/gstbasesink.c @@ -1003,6 +1003,7 @@ gst_base_sink_preroll_queue_flush (GstBaseSink * basesink, GstPad * pad) } /* we can't have EOS anymore now */ basesink->eos = FALSE; + basesink->priv->received_eos = FALSE; basesink->have_preroll = FALSE; basesink->eos_queued = FALSE; basesink->preroll_queued = 0; @@ -2025,9 +2026,6 @@ gst_base_sink_queue_object_unlocked (GstBaseSink * basesink, GstPad * pad, gint length; GQueue *q; - if (G_UNLIKELY (basesink->priv->received_eos)) - goto was_eos; - if (G_UNLIKELY (basesink->need_preroll)) { if (G_LIKELY (prerollable)) basesink->preroll_queued++; @@ -2073,13 +2071,6 @@ gst_base_sink_queue_object_unlocked (GstBaseSink * basesink, GstPad * pad, return ret; /* special cases */ -was_eos: - { - GST_DEBUG_OBJECT (basesink, - "we are EOS, dropping object, return UNEXPECTED"); - gst_mini_object_unref (obj); - return GST_FLOW_UNEXPECTED; - } preroll_failed: { GST_DEBUG_OBJECT (basesink, "preroll failed, reason %s", @@ -2121,6 +2112,9 @@ gst_base_sink_queue_object (GstBaseSink * basesink, GstPad * pad, if (G_UNLIKELY (basesink->flushing)) goto flushing; + if (G_UNLIKELY (basesink->priv->received_eos)) + goto was_eos; + ret = gst_base_sink_queue_object_unlocked (basesink, pad, obj, prerollable); GST_PAD_PREROLL_UNLOCK (pad); @@ -2134,6 +2128,14 @@ flushing: gst_mini_object_unref (obj); return GST_FLOW_WRONG_STATE; } +was_eos: + { + GST_DEBUG_OBJECT (basesink, + "we are EOS, dropping object, return UNEXPECTED"); + GST_PAD_PREROLL_UNLOCK (pad); + gst_mini_object_unref (obj); + return GST_FLOW_UNEXPECTED; + } } static gboolean @@ -2155,16 +2157,27 @@ gst_base_sink_event (GstPad * pad, GstEvent * event) { GstFlowReturn ret; - /* EOS is a prerollable object */ - ret = - gst_base_sink_queue_object (basesink, pad, - GST_MINI_OBJECT_CAST (event), TRUE); + GST_PAD_PREROLL_LOCK (pad); + if (G_UNLIKELY (basesink->flushing)) + goto flushing; - if (G_UNLIKELY (ret != GST_FLOW_OK)) + if (G_UNLIKELY (basesink->priv->received_eos)) { + /* we can't accept anything when we are EOS */ result = FALSE; - else - /* we are now EOS, and refuse more buffers */ + gst_event_unref (event); + } else { + /* we set the received EOS flag here so that we can use it when testing if + * we are prerolled and to refure more buffers. */ basesink->priv->received_eos = TRUE; + + /* EOS is a prerollable object, we call the unlocked version because it + * does not check the received_eos flag. */ + ret = gst_base_sink_queue_object_unlocked (basesink, pad, + GST_MINI_OBJECT_CAST (event), TRUE); + if (G_UNLIKELY (ret != GST_FLOW_OK)) + result = FALSE; + } + GST_PAD_PREROLL_UNLOCK (pad); break; } case GST_EVENT_NEWSEGMENT: @@ -2173,6 +2186,10 @@ gst_base_sink_event (GstPad * pad, GstEvent * event) GST_DEBUG_OBJECT (basesink, "newsegment %p", event); + GST_PAD_PREROLL_LOCK (pad); + if (G_UNLIKELY (basesink->flushing)) + goto flushing; + if (G_UNLIKELY (basesink->priv->received_eos)) { /* we can't accept anything when we are EOS */ result = FALSE; @@ -2185,13 +2202,14 @@ gst_base_sink_event (GstPad * pad, GstEvent * event) basesink->abidata.ABI.clip_segment); ret = - gst_base_sink_queue_object (basesink, pad, + gst_base_sink_queue_object_unlocked (basesink, pad, GST_MINI_OBJECT_CAST (event), FALSE); if (G_UNLIKELY (ret != GST_FLOW_OK)) result = FALSE; else basesink->have_newsegment = TRUE; } + GST_PAD_PREROLL_UNLOCK (pad); break; } case GST_EVENT_FLUSH_START: @@ -2228,7 +2246,8 @@ gst_base_sink_event (GstPad * pad, GstEvent * event) GST_DEBUG_OBJECT (basesink, "flush-stop %p", event); - /* unset flushing so we can accept new data */ + /* unset flushing so we can accept new data, this also flushes out any EOS + * event. */ gst_base_sink_set_flushing (basesink, pad, FALSE); /* we need new segment info after the flush. */ @@ -2236,8 +2255,6 @@ gst_base_sink_event (GstPad * pad, GstEvent * event) gst_segment_init (basesink->abidata.ABI.clip_segment, GST_FORMAT_UNDEFINED); basesink->have_newsegment = FALSE; - /* we flush out the EOS event as well now */ - basesink->priv->received_eos = FALSE; gst_event_unref (event); break; @@ -2254,9 +2271,20 @@ gst_base_sink_event (GstPad * pad, GstEvent * event) } break; } +done: gst_object_unref (basesink); return result; + + /* ERRORS */ +flushing: + { + GST_DEBUG_OBJECT (basesink, "we are flushing"); + GST_PAD_PREROLL_UNLOCK (pad); + result = FALSE; + gst_event_unref (event); + goto done; + } } /* default implementation to calculate the start and end @@ -2286,10 +2314,14 @@ gst_base_sink_needs_preroll (GstBaseSink * basesink) { gboolean is_prerolled, res; - is_prerolled = basesink->have_preroll || basesink->eos; + /* we have 2 cases where the PREROLL_LOCK is released: + * 1) we are blocking in the PREROLL_LOCK and thus are prerolled. + * 2) we are syncing on the clock + */ + is_prerolled = basesink->have_preroll || basesink->priv->received_eos; res = !is_prerolled && basesink->pad_mode != GST_ACTIVATE_PULL; GST_DEBUG_OBJECT (basesink, "have_preroll: %d, EOS: %d => needs preroll: %d", - basesink->have_preroll, basesink->eos, res); + basesink->have_preroll, basesink->priv->received_eos, res); return res; } @@ -2313,6 +2345,9 @@ gst_base_sink_chain_unlocked (GstBaseSink * basesink, GstPad * pad, if (G_UNLIKELY (basesink->flushing)) goto flushing; + if (G_UNLIKELY (basesink->priv->received_eos)) + goto was_eos; + /* for code clarity */ clip_segment = basesink->abidata.ABI.clip_segment; @@ -2364,6 +2399,13 @@ flushing: gst_buffer_unref (buf); return GST_FLOW_WRONG_STATE; } +was_eos: + { + GST_DEBUG_OBJECT (basesink, + "we are EOS, dropping object, return UNEXPECTED"); + gst_buffer_unref (buf); + return GST_FLOW_UNEXPECTED; + } out_of_segment: { GST_DEBUG_OBJECT (basesink, "dropping buffer, out of clipping segment"); diff --git a/tests/check/generic/sinks.c b/tests/check/generic/sinks.c index 9dab91941b..6cb4826253 100644 --- a/tests/check/generic/sinks.c +++ b/tests/check/generic/sinks.c @@ -896,13 +896,95 @@ GST_START_TEST (test_bin_live) gst_object_unref (pipeline); } -GST_END_TEST +GST_END_TEST static void +send_eos (GstPad * sinkpad) +{ + gst_pad_send_event (sinkpad, gst_event_new_eos ()); +} + +/* push a buffer with a very long duration in a fakesink, then push an EOS + * event. fakesink should emit EOS after the duration of the buffer expired. + * Going to PAUSED, however should not return ASYNC while processing the + * buffer. */ +GST_START_TEST (test_fake_eos) +{ + GstElement *sink, *pipeline; + GstBuffer *buffer; + GstStateChangeReturn ret; + GstPad *sinkpad; + GstFlowReturn res; + GThread *thread; + + pipeline = gst_pipeline_new ("pipeline"); + + sink = gst_element_factory_make ("fakesink", "sink"); + g_object_set (G_OBJECT (sink), "sync", TRUE, NULL); + + sinkpad = gst_element_get_pad (sink, "sink"); + + gst_bin_add (GST_BIN_CAST (pipeline), sink); + + ret = gst_element_set_state (pipeline, GST_STATE_PLAYING); + fail_unless (ret == GST_STATE_CHANGE_ASYNC, "no ASYNC state return"); + + /* push buffer of 100 seconds, since it has a timestamp of 0, it should be + * rendered immediatly and the chain function should return immediatly */ + buffer = gst_buffer_new_and_alloc (10); + GST_BUFFER_TIMESTAMP (buffer) = 0; + GST_BUFFER_DURATION (buffer) = 100 * GST_SECOND; + res = gst_pad_chain (sinkpad, buffer); + fail_unless (res == GST_FLOW_OK, "no OK flow return"); + + /* wait for preroll, this should happen really soon. */ + ret = gst_element_get_state (pipeline, NULL, NULL, -1); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "no SUCCESS state return"); + + /* push EOS, this will block for up to 100 seconds, until the previous + * buffer has finished. We therefore push it in another thread so we can do + * something else while it blocks. */ + thread = g_thread_create ((GThreadFunc) send_eos, sinkpad, TRUE, NULL); + fail_if (thread == NULL, "no thread"); + + /* wait a while so that the thread manages to start and push the EOS */ + g_usleep (G_USEC_PER_SEC); + + /* this should cancel rendering of the EOS event and should return SUCCESS + * because the sink is now prerolled on the EOS. */ + ret = gst_element_set_state (pipeline, GST_STATE_PAUSED); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "no SUCCESS state return"); + + /* we can unref the sinkpad now, we're done with it */ + gst_object_unref (sinkpad); + + /* wait for a second, use the debug log to see that basesink does not discard + * the EOS */ + g_usleep (G_USEC_PER_SEC); + + /* go back to PLAYING, which means waiting some more in EOS, check debug log + * to see this happen. */ + ret = gst_element_set_state (pipeline, GST_STATE_PLAYING); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "no SUCCESS state return"); + g_usleep (G_USEC_PER_SEC); + + /* teardown and cleanup */ + ret = gst_element_set_state (pipeline, GST_STATE_NULL); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "no SUCCESS state return"); + + gst_object_unref (pipeline); +} + +GST_END_TEST; + /* test: try changing state of sinks */ - Suite * gst_sinks_suite (void) +Suite * +gst_sinks_suite (void) { Suite *s = suite_create ("Sinks"); TCase *tc_chain = tcase_create ("general"); + /* time out after 10s, not the default 3, we need this for the last test. */ + tcase_set_timeout (tc_chain, 10); + suite_add_tcase (s, tc_chain); tcase_add_test (tc_chain, test_sink); tcase_add_test (tc_chain, test_sink_completion); @@ -919,6 +1001,7 @@ GST_END_TEST tcase_add_test (tc_chain, test_add_live); tcase_add_test (tc_chain, test_add_live2); tcase_add_test (tc_chain, test_bin_live); + tcase_add_test (tc_chain, test_fake_eos); return s; }