diff --git a/ChangeLog b/ChangeLog index 0234c0bd3d..37de435565 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,26 @@ +2005-11-03 Wim Taymans + + * check/states/sinks.c: (GST_START_TEST), (gst_object_suite): + Added some more checks. Specifically the case where NO_PREROLL + elements are in the pipeline. + + * gst/base/gstbasesink.c: (gst_base_sink_commit_state), + (gst_base_sink_handle_object), (gst_base_sink_do_sync), + (gst_base_sink_get_position): + Post READY->PAUSED state change messages too. + Fix bug where VOID was posted as pending state... + + * gst/gstbin.c: (gst_bin_recalc_state): + use _element_continue_state() to continue the state change. + + * gst/gstelement.c: (gst_element_continue_state), + (gst_element_commit_state), (gst_element_set_state_func), + (gst_element_change_state), (gst_element_change_state_func): + Lots of state change cleanups, assign the STATE_RETURN in + a new continue_state() function that also propagates the + last return value from a state change to the app. + Update some debug statements with proper category. + 2005-11-03 Wim Taymans * docs/design/part-events.txt: diff --git a/check/states/sinks.c b/check/states/sinks.c index 7c97353ee2..80b131fa74 100644 --- a/check/states/sinks.c +++ b/check/states/sinks.c @@ -181,8 +181,11 @@ GST_START_TEST (test_livesrc_remove) } GST_END_TEST -/* a sink should go ASYNC to PAUSE. PAUSE does not complete - * since we have a live source. */ +/* the sink should go ASYNC to PAUSE. The live source should go + * NO_PREROLL to PAUSE. the pipeline returns NO_PREROLL. An + * attempt to go to PLAYING will return ASYNC. polling state + * completion should return SUCCESS when the sink is gone to + * PLAYING. */ GST_START_TEST (test_livesrc_sink) { GstElement *sink, *src, *pipeline; @@ -208,6 +211,10 @@ GST_START_TEST (test_livesrc_sink) fail_unless (ret == GST_STATE_CHANGE_NO_PREROLL, "no no_preroll state return"); + ret = gst_element_set_state (pipeline, GST_STATE_PAUSED); + fail_unless (ret == GST_STATE_CHANGE_NO_PREROLL, + "no no_preroll state return the second time"); + ret = gst_element_get_state (src, ¤t, &pending, GST_CLOCK_TIME_NONE); fail_unless (ret == GST_STATE_CHANGE_NO_PREROLL, "not paused"); fail_unless (current == GST_STATE_PAUSED, "not paused"); @@ -236,6 +243,108 @@ GST_START_TEST (test_livesrc_sink) GST_END_TEST; +/* The sink should go ASYNC to PLAYING. The source should go + * to PLAYING with SUCCESS. The pipeline returns ASYNC. */ +GST_START_TEST (test_livesrc2_sink) +{ + GstElement *sink, *src, *pipeline; + GstStateChangeReturn ret; + GstState current, pending; + GstPad *srcpad, *sinkpad; + + pipeline = gst_pipeline_new ("pipeline"); + src = gst_element_factory_make ("fakesrc", "src"); + g_object_set (G_OBJECT (src), "is-live", TRUE, NULL); + sink = gst_element_factory_make ("fakesink", "sink"); + + gst_bin_add (GST_BIN (pipeline), src); + gst_bin_add (GST_BIN (pipeline), sink); + + srcpad = gst_element_get_pad (src, "src"); + sinkpad = gst_element_get_pad (sink, "sink"); + gst_pad_link (srcpad, sinkpad); + gst_object_unref (srcpad); + gst_object_unref (sinkpad); + + ret = gst_element_set_state (pipeline, GST_STATE_PLAYING); + fail_unless (ret == GST_STATE_CHANGE_ASYNC, "no async state return"); + + ret = gst_element_get_state (src, ¤t, &pending, GST_CLOCK_TIME_NONE); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "not playing"); + fail_unless (current == GST_STATE_PLAYING, "not playing"); + fail_unless (pending == GST_STATE_VOID_PENDING, "not playing"); + + ret = + gst_element_get_state (pipeline, ¤t, &pending, GST_CLOCK_TIME_NONE); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "not playing"); + fail_unless (current == GST_STATE_PLAYING, "not playing"); + fail_unless (pending == GST_STATE_VOID_PENDING, "not playing"); + + /* and back down */ + ret = gst_element_set_state (pipeline, GST_STATE_PAUSED); + fail_unless (ret == GST_STATE_CHANGE_NO_PREROLL, + "no no_preroll state return"); + + ret = gst_element_get_state (src, ¤t, &pending, GST_CLOCK_TIME_NONE); + fail_unless (ret == GST_STATE_CHANGE_NO_PREROLL, "not no_preroll"); + fail_unless (current == GST_STATE_PAUSED, "not paused"); + fail_unless (pending == GST_STATE_VOID_PENDING, "not paused"); + + /* sink state is not known.. it might be prerolled or not */ + + /* and to READY */ + ret = gst_element_set_state (pipeline, GST_STATE_READY); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "no success state return"); + + ret = gst_element_get_state (src, ¤t, &pending, GST_CLOCK_TIME_NONE); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "not success"); + fail_unless (current == GST_STATE_READY, "not ready"); + fail_unless (pending == GST_STATE_VOID_PENDING, "not ready"); + + ret = gst_element_get_state (sink, ¤t, &pending, GST_CLOCK_TIME_NONE); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "not success"); + fail_unless (current == GST_STATE_READY, "not ready"); + fail_unless (pending == GST_STATE_VOID_PENDING, "not ready"); +} + +GST_END_TEST; + +GST_START_TEST (test_livesrc3_sink) +{ + GstElement *sink, *src, *pipeline; + GstStateChangeReturn ret; + GstState current, pending; + GstPad *srcpad, *sinkpad; + + pipeline = gst_pipeline_new ("pipeline"); + src = gst_element_factory_make ("fakesrc", "src"); + g_object_set (G_OBJECT (src), "is-live", TRUE, NULL); + sink = gst_element_factory_make ("fakesink", "sink"); + + gst_bin_add (GST_BIN (pipeline), src); + gst_bin_add (GST_BIN (pipeline), sink); + + srcpad = gst_element_get_pad (src, "src"); + sinkpad = gst_element_get_pad (sink, "sink"); + gst_pad_link (srcpad, sinkpad); + gst_object_unref (srcpad); + gst_object_unref (sinkpad); + + ret = gst_element_set_state (pipeline, GST_STATE_PLAYING); + fail_unless (ret == GST_STATE_CHANGE_ASYNC, "no async state return"); + + ret = + gst_element_get_state (pipeline, ¤t, &pending, GST_CLOCK_TIME_NONE); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "not playing"); + fail_unless (current == GST_STATE_PLAYING, "not playing"); + fail_unless (pending == GST_STATE_VOID_PENDING, "not playing"); + + /* and back down */ + ret = gst_element_set_state (pipeline, GST_STATE_NULL); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "no success state return"); +} + +GST_END_TEST; /* test: try changing state of sinks */ Suite * @@ -250,6 +359,8 @@ gst_object_suite (void) tcase_add_test (tc_chain, test_src_sink); tcase_add_test (tc_chain, test_livesrc_remove); tcase_add_test (tc_chain, test_livesrc_sink); + tcase_add_test (tc_chain, test_livesrc2_sink); + tcase_add_test (tc_chain, test_livesrc3_sink); return s; } diff --git a/gst/base/gstbasesink.c b/gst/base/gstbasesink.c index d6f30c9352..4b4315bb31 100644 --- a/gst/base/gstbasesink.c +++ b/gst/base/gstbasesink.c @@ -462,6 +462,9 @@ gst_base_sink_commit_state (GstBaseSink * basesink) case GST_STATE_PLAYING: basesink->need_preroll = FALSE; post_playing = TRUE; + /* post PAUSED too when we were READY */ + if (current == GST_STATE_READY) + post_paused = TRUE; break; case GST_STATE_PAUSED: basesink->need_preroll = TRUE; @@ -478,8 +481,6 @@ gst_base_sink_commit_state (GstBaseSink * basesink) GST_STATE_NEXT (basesink) = GST_STATE_VOID_PENDING; GST_STATE_PENDING (basesink) = GST_STATE_VOID_PENDING; GST_STATE_RETURN (basesink) = GST_STATE_CHANGE_SUCCESS; - - pending = GST_STATE_VOID_PENDING; } GST_UNLOCK (basesink); diff --git a/gst/gstbin.c b/gst/gstbin.c index cd701c7c26..54f04209b5 100644 --- a/gst/gstbin.c +++ b/gst/gstbin.c @@ -1093,6 +1093,10 @@ gst_bin_get_state_func (GstElement * element, GstState * state, return ret; } +/* FIXME, defined in gstelement.c but not yet in the header */ +GstStateChangeReturn +gst_element_continue_state (GstElement * element, GstStateChangeReturn ret); + static void gst_bin_recalc_state (GstBin * bin, gboolean force) { @@ -1209,7 +1213,7 @@ done: switch (ret) { case GST_STATE_CHANGE_SUCCESS: case GST_STATE_CHANGE_NO_PREROLL: - gst_element_commit_state (GST_ELEMENT_CAST (bin)); + ret = gst_element_continue_state (GST_ELEMENT_CAST (bin), ret); break; case GST_STATE_CHANGE_ASYNC: gst_element_lost_state (GST_ELEMENT_CAST (bin)); @@ -1221,8 +1225,7 @@ done: goto unknown_state; } - GST_CAT_INFO_OBJECT (GST_CAT_STATES, bin, "return now %d", - GST_STATE_RETURN (bin)); + GST_CAT_INFO_OBJECT (GST_CAT_STATES, bin, "bin RETURN is now %d", ret); return; diff --git a/gst/gstelement.c b/gst/gstelement.c index 4df21bf8de..12dc6ee86a 100644 --- a/gst/gstelement.c +++ b/gst/gstelement.c @@ -1811,40 +1811,19 @@ nothing_aborted: } } -/** - * gst_element_commit_state: - * @element: a #GstElement to commit the state of. - * - * Commit the state change of the element. This function is used - * by elements that do asynchronous state changes. - * The core will normally call this method automatically when an - * element returned SUCCESS from the state change function. - * Elements that return ASYNC from the change_state function should - * eventually call this method from the streaming thread to signal - * successfull state change completion. - * - * If after calling this method the element still has not reached - * the pending state, the next state change is performed. - * - * This function can only be called with the STATE_LOCK held. - * - * Returns: The result of the commit state change. - * - * MT safe. - */ +/* FIXME, this function is to be put in the .h file, + * gst_element_commit_state() should go away. */ GstStateChangeReturn -gst_element_commit_state (GstElement * element) +gst_element_continue_state (GstElement * element, GstStateChangeReturn ret) { GstState pending; - GstStateChangeReturn ret; GstState old_state, old_next; GstState current, next; GstMessage *message; GstStateChange transition; - g_return_val_if_fail (GST_IS_ELEMENT (element), GST_STATE_CHANGE_FAILURE); - GST_LOCK (element); + GST_STATE_RETURN (element) = ret; pending = GST_STATE_PENDING (element); /* check if there is something to commit */ @@ -1888,17 +1867,16 @@ gst_element_commit_state (GstElement * element) nothing_pending: { - GST_DEBUG_OBJECT (element, "nothing pending"); + GST_CAT_INFO_OBJECT (GST_CAT_STATES, element, "nothing pending"); GST_UNLOCK (element); - return GST_STATE_CHANGE_SUCCESS; + return ret; } complete: { GST_STATE_PENDING (element) = GST_STATE_VOID_PENDING; GST_STATE_NEXT (element) = GST_STATE_VOID_PENDING; - ret = GST_STATE_RETURN (element) = GST_STATE_CHANGE_SUCCESS; - GST_DEBUG_OBJECT (element, "completed state change"); + GST_CAT_INFO_OBJECT (GST_CAT_STATES, element, "completed state change"); GST_UNLOCK (element); message = gst_message_new_state_changed (GST_OBJECT (element), @@ -1911,6 +1889,35 @@ complete: } } +/** + * gst_element_commit_state: + * @element: a #GstElement to commit the state of. + * + * Commit the state change of the element. This function is used + * by elements that do asynchronous state changes. + * The core will normally call this method automatically when an + * element returned SUCCESS from the state change function. + * Elements that return ASYNC from the change_state function should + * eventually call this method from the streaming thread to signal + * successfull state change completion. + * + * If after calling this method the element still has not reached + * the pending state, the next state change is performed. + * + * This function can only be called with the STATE_LOCK held. + * + * Returns: The result of the commit state change. + * + * MT safe. + */ +GstStateChangeReturn +gst_element_commit_state (GstElement * element) +{ + g_return_val_if_fail (GST_IS_ELEMENT (element), GST_STATE_CHANGE_FAILURE); + + return gst_element_continue_state (element, GST_STATE_CHANGE_SUCCESS); +} + /** * gst_element_lost_state: * @element: a #GstElement the state is lost of @@ -2085,14 +2092,15 @@ gst_element_set_state_func (GstElement * element, GstState state) GST_STATE_UNLOCK (element); - GST_DEBUG_OBJECT (element, "returned %d", ret); + GST_CAT_DEBUG_OBJECT (GST_CAT_STATES, element, "returned %d", ret); return ret; was_busy: { GST_STATE_RETURN (element) = GST_STATE_CHANGE_ASYNC; - GST_DEBUG_OBJECT (element, "element was busy with async state change"); + GST_CAT_DEBUG_OBJECT (GST_CAT_STATES, element, + "element was busy with async state change"); GST_UNLOCK (element); GST_STATE_UNLOCK (element); @@ -2136,7 +2144,7 @@ gst_element_change_state (GstElement * element, GstStateChange transition) /* if we go upwards, we give the app a change to wait for * completion */ if (current < next) - goto exit; + goto async; /* else we just continue the state change downwards */ GST_CAT_INFO_OBJECT (GST_CAT_STATES, element, @@ -2144,50 +2152,51 @@ gst_element_change_state (GstElement * element, GstStateChange transition) gst_element_state_get_name (current), gst_element_state_get_name (next)); - GST_LOCK (element); - GST_STATE_RETURN (element) = GST_STATE_CHANGE_SUCCESS; - GST_UNLOCK (element); - - ret = gst_element_commit_state (element); + ret = gst_element_continue_state (element, GST_STATE_CHANGE_SUCCESS); break; case GST_STATE_CHANGE_SUCCESS: GST_CAT_DEBUG_OBJECT (GST_CAT_STATES, element, "element changed state SUCCESS"); /* we can commit the state now which will proceeed to * the next state */ - ret = gst_element_commit_state (element); + ret = gst_element_continue_state (element, ret); break; case GST_STATE_CHANGE_NO_PREROLL: GST_CAT_DEBUG_OBJECT (GST_CAT_STATES, element, "element changed state NO_PREROLL"); /* we can commit the state now which will proceeed to * the next state */ - gst_element_commit_state (element); - ret = GST_STATE_CHANGE_NO_PREROLL; + ret = gst_element_continue_state (element, ret); break; default: - ret = GST_STATE_CHANGE_FAILURE; goto invalid_return; } -exit: + GST_CAT_LOG_OBJECT (GST_CAT_STATES, element, "exit state change %d", ret); + + return ret; + +async: GST_LOCK (element); GST_STATE_RETURN (element) = ret; + GST_CAT_LOG_OBJECT (GST_CAT_STATES, element, "exit async state change %d", + ret); GST_UNLOCK (element); - GST_CAT_LOG_OBJECT (GST_CAT_STATES, element, "exit state change %d", ret); - return ret; /* ERROR */ invalid_return: { GST_LOCK (element); + /* somebody added a GST_STATE_ and forgot to do stuff here ! */ + g_critical ("%s: unknown return value %d from a state change function", + GST_ELEMENT_NAME (element), ret); + + ret = GST_STATE_CHANGE_FAILURE; GST_STATE_RETURN (element) = ret; GST_UNLOCK (element); - /* somebody added a GST_STATE_ and forgot to do stuff here ! */ - g_critical ("unknown return value %d from a state change function", ret); return ret; } } @@ -2328,10 +2337,14 @@ gst_element_change_state_func (GstElement * element, GstStateChange transition) was_ok: { + GST_LOCK (element); + result = GST_STATE_RETURN (element); GST_CAT_DEBUG_OBJECT (GST_CAT_STATES, element, "element is already in the %s state", gst_element_state_get_name (state)); - return GST_STATE_RETURN (element); + GST_UNLOCK (element); + + return result; } } diff --git a/libs/gst/base/gstbasesink.c b/libs/gst/base/gstbasesink.c index d6f30c9352..4b4315bb31 100644 --- a/libs/gst/base/gstbasesink.c +++ b/libs/gst/base/gstbasesink.c @@ -462,6 +462,9 @@ gst_base_sink_commit_state (GstBaseSink * basesink) case GST_STATE_PLAYING: basesink->need_preroll = FALSE; post_playing = TRUE; + /* post PAUSED too when we were READY */ + if (current == GST_STATE_READY) + post_paused = TRUE; break; case GST_STATE_PAUSED: basesink->need_preroll = TRUE; @@ -478,8 +481,6 @@ gst_base_sink_commit_state (GstBaseSink * basesink) GST_STATE_NEXT (basesink) = GST_STATE_VOID_PENDING; GST_STATE_PENDING (basesink) = GST_STATE_VOID_PENDING; GST_STATE_RETURN (basesink) = GST_STATE_CHANGE_SUCCESS; - - pending = GST_STATE_VOID_PENDING; } GST_UNLOCK (basesink); diff --git a/tests/check/generic/sinks.c b/tests/check/generic/sinks.c index 7c97353ee2..80b131fa74 100644 --- a/tests/check/generic/sinks.c +++ b/tests/check/generic/sinks.c @@ -181,8 +181,11 @@ GST_START_TEST (test_livesrc_remove) } GST_END_TEST -/* a sink should go ASYNC to PAUSE. PAUSE does not complete - * since we have a live source. */ +/* the sink should go ASYNC to PAUSE. The live source should go + * NO_PREROLL to PAUSE. the pipeline returns NO_PREROLL. An + * attempt to go to PLAYING will return ASYNC. polling state + * completion should return SUCCESS when the sink is gone to + * PLAYING. */ GST_START_TEST (test_livesrc_sink) { GstElement *sink, *src, *pipeline; @@ -208,6 +211,10 @@ GST_START_TEST (test_livesrc_sink) fail_unless (ret == GST_STATE_CHANGE_NO_PREROLL, "no no_preroll state return"); + ret = gst_element_set_state (pipeline, GST_STATE_PAUSED); + fail_unless (ret == GST_STATE_CHANGE_NO_PREROLL, + "no no_preroll state return the second time"); + ret = gst_element_get_state (src, ¤t, &pending, GST_CLOCK_TIME_NONE); fail_unless (ret == GST_STATE_CHANGE_NO_PREROLL, "not paused"); fail_unless (current == GST_STATE_PAUSED, "not paused"); @@ -236,6 +243,108 @@ GST_START_TEST (test_livesrc_sink) GST_END_TEST; +/* The sink should go ASYNC to PLAYING. The source should go + * to PLAYING with SUCCESS. The pipeline returns ASYNC. */ +GST_START_TEST (test_livesrc2_sink) +{ + GstElement *sink, *src, *pipeline; + GstStateChangeReturn ret; + GstState current, pending; + GstPad *srcpad, *sinkpad; + + pipeline = gst_pipeline_new ("pipeline"); + src = gst_element_factory_make ("fakesrc", "src"); + g_object_set (G_OBJECT (src), "is-live", TRUE, NULL); + sink = gst_element_factory_make ("fakesink", "sink"); + + gst_bin_add (GST_BIN (pipeline), src); + gst_bin_add (GST_BIN (pipeline), sink); + + srcpad = gst_element_get_pad (src, "src"); + sinkpad = gst_element_get_pad (sink, "sink"); + gst_pad_link (srcpad, sinkpad); + gst_object_unref (srcpad); + gst_object_unref (sinkpad); + + ret = gst_element_set_state (pipeline, GST_STATE_PLAYING); + fail_unless (ret == GST_STATE_CHANGE_ASYNC, "no async state return"); + + ret = gst_element_get_state (src, ¤t, &pending, GST_CLOCK_TIME_NONE); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "not playing"); + fail_unless (current == GST_STATE_PLAYING, "not playing"); + fail_unless (pending == GST_STATE_VOID_PENDING, "not playing"); + + ret = + gst_element_get_state (pipeline, ¤t, &pending, GST_CLOCK_TIME_NONE); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "not playing"); + fail_unless (current == GST_STATE_PLAYING, "not playing"); + fail_unless (pending == GST_STATE_VOID_PENDING, "not playing"); + + /* and back down */ + ret = gst_element_set_state (pipeline, GST_STATE_PAUSED); + fail_unless (ret == GST_STATE_CHANGE_NO_PREROLL, + "no no_preroll state return"); + + ret = gst_element_get_state (src, ¤t, &pending, GST_CLOCK_TIME_NONE); + fail_unless (ret == GST_STATE_CHANGE_NO_PREROLL, "not no_preroll"); + fail_unless (current == GST_STATE_PAUSED, "not paused"); + fail_unless (pending == GST_STATE_VOID_PENDING, "not paused"); + + /* sink state is not known.. it might be prerolled or not */ + + /* and to READY */ + ret = gst_element_set_state (pipeline, GST_STATE_READY); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "no success state return"); + + ret = gst_element_get_state (src, ¤t, &pending, GST_CLOCK_TIME_NONE); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "not success"); + fail_unless (current == GST_STATE_READY, "not ready"); + fail_unless (pending == GST_STATE_VOID_PENDING, "not ready"); + + ret = gst_element_get_state (sink, ¤t, &pending, GST_CLOCK_TIME_NONE); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "not success"); + fail_unless (current == GST_STATE_READY, "not ready"); + fail_unless (pending == GST_STATE_VOID_PENDING, "not ready"); +} + +GST_END_TEST; + +GST_START_TEST (test_livesrc3_sink) +{ + GstElement *sink, *src, *pipeline; + GstStateChangeReturn ret; + GstState current, pending; + GstPad *srcpad, *sinkpad; + + pipeline = gst_pipeline_new ("pipeline"); + src = gst_element_factory_make ("fakesrc", "src"); + g_object_set (G_OBJECT (src), "is-live", TRUE, NULL); + sink = gst_element_factory_make ("fakesink", "sink"); + + gst_bin_add (GST_BIN (pipeline), src); + gst_bin_add (GST_BIN (pipeline), sink); + + srcpad = gst_element_get_pad (src, "src"); + sinkpad = gst_element_get_pad (sink, "sink"); + gst_pad_link (srcpad, sinkpad); + gst_object_unref (srcpad); + gst_object_unref (sinkpad); + + ret = gst_element_set_state (pipeline, GST_STATE_PLAYING); + fail_unless (ret == GST_STATE_CHANGE_ASYNC, "no async state return"); + + ret = + gst_element_get_state (pipeline, ¤t, &pending, GST_CLOCK_TIME_NONE); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "not playing"); + fail_unless (current == GST_STATE_PLAYING, "not playing"); + fail_unless (pending == GST_STATE_VOID_PENDING, "not playing"); + + /* and back down */ + ret = gst_element_set_state (pipeline, GST_STATE_NULL); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "no success state return"); +} + +GST_END_TEST; /* test: try changing state of sinks */ Suite * @@ -250,6 +359,8 @@ gst_object_suite (void) tcase_add_test (tc_chain, test_src_sink); tcase_add_test (tc_chain, test_livesrc_remove); tcase_add_test (tc_chain, test_livesrc_sink); + tcase_add_test (tc_chain, test_livesrc2_sink); + tcase_add_test (tc_chain, test_livesrc3_sink); return s; }