mirror of
https://gitlab.freedesktop.org/gstreamer/gstreamer.git
synced 2024-06-29 03:10:40 +00:00
bin: try harder to avoid state changes in wrong direction
When the bin does an upward state change, try to avoid doing a downward state change on the child and vice versa. Add some more unit tests for this fix. Fixes https://bugzilla.gnome.org/show_bug.cgi?id=621833
This commit is contained in:
Wim Taymans
parent
57564ed276
commit
554b81ed24
94
gst/gstbin.c
94
gst/gstbin.c
|
|
@ -2129,7 +2129,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;
|
||||
|
||||
|
|
@ -2159,17 +2159,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 =
|
||||
|
|
@ -2220,6 +2279,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");
|
||||
|
|
@ -2227,18 +2295,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
|
||||
|
|
|
|||
|
|
@ -1155,6 +1155,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)
|
||||
{
|
||||
|
|
@ -1178,6 +1297,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)
|
||||
|
|
|
|||
Loading…
Reference in a new issue