diff --git a/gst/gstbin.c b/gst/gstbin.c index 665259e6c2..d5ce5f6a33 100644 --- a/gst/gstbin.c +++ b/gst/gstbin.c @@ -2152,7 +2152,7 @@ gst_bin_element_set_state (GstBin * bin, GstElement * element, GstState next) { GstStateChangeReturn ret; - GstState pending, child_current, child_pending; + GstState child_current, child_pending; gboolean locked; GList *found; @@ -2182,17 +2182,76 @@ gst_bin_element_set_state (GstBin * bin, GstElement * element, goto no_preroll; } - GST_OBJECT_LOCK (bin); - pending = GST_STATE_PENDING (bin); + GST_CAT_INFO_OBJECT (GST_CAT_STATES, element, + "current %s pending %s, desired next %s", + gst_element_state_get_name (child_current), + gst_element_state_get_name (child_pending), + gst_element_state_get_name (next)); /* Try not to change the state of elements that are already in the state we're * going to */ - if (!(next == GST_STATE_PLAYING || child_pending != GST_STATE_VOID_PENDING || - (child_pending == GST_STATE_VOID_PENDING && - ((pending > child_current && next > child_current) || - (pending < child_current && next < child_current))))) + if (child_current == next && child_pending == GST_STATE_VOID_PENDING) { + /* child is already at the requested state, return previous return. Note that + * if the child has a pending state to next, we will still call the + * set_state function */ goto unneeded; + } else if (next > current) { + /* upward state change */ + if (child_pending == GST_STATE_VOID_PENDING) { + /* .. and the child is not busy doing anything */ + if (child_current > next) { + /* .. and is already past the requested state, assume it got there + * without error */ + ret = GST_STATE_CHANGE_SUCCESS; + goto unneeded; + } + } else if (child_pending > child_current) { + /* .. and the child is busy going upwards */ + if (child_current >= next) { + /* .. and is already past the requested state, assume it got there + * without error */ + ret = GST_STATE_CHANGE_SUCCESS; + goto unneeded; + } + } else { + /* .. and the child is busy going downwards */ + if (child_current > next) { + /* .. and is already past the requested state, assume it got there + * without error */ + ret = GST_STATE_CHANGE_SUCCESS; + goto unneeded; + } + } + } else if (next < current) { + /* downward state change */ + if (child_pending == GST_STATE_VOID_PENDING) { + /* .. and the child is not busy doing anything */ + if (child_current < next) { + /* .. and is already past the requested state, assume it got there + * without error */ + ret = GST_STATE_CHANGE_SUCCESS; + goto unneeded; + } + } else if (child_pending < child_current) { + /* .. and the child is busy going downwards */ + if (child_current <= next) { + /* .. and is already past the requested state, assume it got there + * without error */ + ret = GST_STATE_CHANGE_SUCCESS; + goto unneeded; + } + } else { + /* .. and the child is busy going upwards */ + if (child_current < next) { + /* .. and is already past the requested state, assume it got there + * without error */ + ret = GST_STATE_CHANGE_SUCCESS; + goto unneeded; + } + } + } + GST_OBJECT_LOCK (bin); /* the element was busy with an upwards async state change, we must wait for * an ASYNC_DONE message before we attemp to change the state. */ if ((found = @@ -2234,6 +2293,15 @@ locked: GST_STATE_UNLOCK (element); return ret; } +unneeded: + { + GST_CAT_INFO_OBJECT (GST_CAT_STATES, element, + "skipping transition from %s to %s", + gst_element_state_get_name (child_current), + gst_element_state_get_name (next)); + GST_STATE_UNLOCK (element); + return ret; + } was_busy: { GST_DEBUG_OBJECT (element, "element was busy, delaying state change"); @@ -2241,18 +2309,6 @@ was_busy: GST_STATE_UNLOCK (element); return GST_STATE_CHANGE_ASYNC; } -unneeded: - { - GST_CAT_INFO_OBJECT (GST_CAT_STATES, element, - "skipping transition from %s to %s, since bin pending" - " is %s : last change state return follows", - gst_element_state_get_name (child_current), - gst_element_state_get_name (next), - gst_element_state_get_name (pending)); - GST_OBJECT_UNLOCK (bin); - GST_STATE_UNLOCK (element); - return ret; - } } /* gst_iterator_fold functions for pads_activate diff --git a/tests/check/gst/gstbin.c b/tests/check/gst/gstbin.c index b02895814d..9fb8208ab4 100644 --- a/tests/check/gst/gstbin.c +++ b/tests/check/gst/gstbin.c @@ -1158,6 +1158,125 @@ GST_START_TEST (test_many_bins) GST_END_TEST; +static void +fakesrc_pad_blocked_cb (GstPad * pad, gboolean blocked, void *arg) +{ + GstPipeline *pipeline = (GstPipeline *) arg; + GstElement *src, *sink; + GstPad *srcpad; + + if (!blocked) { + /* Not interested in unblocking, ignore that... */ + return; + } + + src = gst_bin_get_by_name (GST_BIN (pipeline), "fakesrc"); + fail_unless (src != NULL, "Could not get fakesrc"); + + sink = gst_element_factory_make ("fakesink", "fakesink"); + fail_unless (sink != NULL, "Could not create fakesink"); + + g_object_set (sink, "state-error", 1, NULL); + gst_bin_add (GST_BIN (pipeline), sink); + + gst_element_link (src, sink); + gst_element_sync_state_with_parent (sink); + + srcpad = gst_element_get_static_pad (src, "src"); + gst_pad_set_blocked_async (srcpad, FALSE, fakesrc_pad_blocked_cb, pipeline); + gst_object_unref (srcpad); + + gst_object_unref (src); +} + +GST_START_TEST (test_state_failure_unref) +{ + GstElement *src, *pipeline; + GstPad *srcpad; + GstBus *bus; + GstStateChangeReturn ret; + GstMessage *msg; + + pipeline = gst_pipeline_new (NULL); + fail_unless (pipeline != NULL, "Could not create pipeline"); + + src = gst_element_factory_make ("fakesrc", "fakesrc"); + fail_unless (src != NULL, "Could not create fakesrc"); + + srcpad = gst_element_get_static_pad (src, "src"); + fail_unless (srcpad != NULL, "Could not get fakesrc srcpad"); + + gst_pad_set_blocked_async (srcpad, TRUE, fakesrc_pad_blocked_cb, pipeline); + gst_object_unref (srcpad); + + gst_bin_add (GST_BIN (pipeline), src); + + bus = gst_pipeline_get_bus (GST_PIPELINE (pipeline)); + fail_unless (bus != NULL, "Could not get bus"); + + gst_element_set_state (pipeline, GST_STATE_PLAYING); + + /* Wait for an error message from our fakesink (added from the + pad block callback). */ + msg = gst_bus_poll (bus, GST_MESSAGE_ERROR, GST_SECOND); + fail_if (msg == NULL, "No error message within 1 second"); + gst_message_unref (msg); + + /* Check that after this failure, we can still stop, and then unref, the + pipeline. This should always be possible. */ + ret = gst_element_set_state (pipeline, GST_STATE_NULL); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "downward state change failed"); + + gst_object_unref (bus); + gst_object_unref (pipeline); +} + +GST_END_TEST; + +static void +on_sync_bus_error (GstBus * bus, GstMessage * msg) +{ + fail_if (msg != NULL); +} + +GST_START_TEST (test_state_change_skip) +{ + GstElement *sink, *pipeline; + GstStateChangeReturn ret; + GstBus *bus; + + pipeline = gst_pipeline_new (NULL); + fail_unless (pipeline != NULL, "Could not create pipeline"); + + bus = gst_pipeline_get_bus (GST_PIPELINE (pipeline)); + fail_unless (bus != NULL, "Could not get bus"); + + /* no errors */ + gst_bus_enable_sync_message_emission (bus); + g_signal_connect (bus, "sync-message::error", (GCallback) on_sync_bus_error, + NULL); + + sink = gst_element_factory_make ("fakesink", "fakesink"); + fail_unless (sink != NULL, "Could not create fakesink"); + gst_element_set_state (sink, GST_STATE_PAUSED); + + g_object_set (sink, "state-error", 5, NULL); + + gst_bin_add (GST_BIN (pipeline), sink); + gst_element_set_state (pipeline, GST_STATE_PLAYING); + + g_object_set (sink, "state-error", 0, NULL); + + /* Check that after this failure, we can still stop, and then unref, the + pipeline. This should always be possible. */ + ret = gst_element_set_state (pipeline, GST_STATE_NULL); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "downward state change failed"); + + gst_object_unref (pipeline); +} + +GST_END_TEST; + static Suite * gst_bin_suite (void) { @@ -1181,6 +1300,8 @@ gst_bin_suite (void) tcase_add_test (tc_chain, test_iterate_sorted); tcase_add_test (tc_chain, test_link_structure_change); tcase_add_test (tc_chain, test_state_failure_remove); + tcase_add_test (tc_chain, test_state_failure_unref); + tcase_add_test (tc_chain, test_state_change_skip); /* fails on OSX build bot for some reason, and is a bit silly anyway */ if (0)