From 8f0fda26d09da90e435fbd97816b0de23c856749 Mon Sep 17 00:00:00 2001 From: Thomas Vander Stichele Date: Thu, 29 Jul 2004 20:33:49 +0000 Subject: [PATCH] revert state change changes as agreed so we can rework them gradually Original commit message from CVS: revert state change changes as agreed so we can rework them gradually --- ChangeLog | 15 ++ gst/gstbin.c | 286 +++++++++++-------------- gst/gstbin.h | 3 - gst/gstthread.c | 44 ++-- tests/old/testsuite/states/Makefile.am | 4 +- testsuite/states/Makefile.am | 4 +- 6 files changed, 167 insertions(+), 189 deletions(-) diff --git a/ChangeLog b/ChangeLog index 1fd5d6fa86..6eaef66bea 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,18 @@ +2004-07-29 Thomas Vander Stichele + + * gst/gstbin.c: (gst_bin_get_type), (gst_bin_class_init), + (gst_bin_add_func), (gst_bin_remove_func), + (gst_bin_child_state_change), (gst_bin_child_state_change_func), + (set_kid_state_func), (gst_bin_change_state), (gst_bin_set_state), + (gst_bin_change_state_norecurse), (gst_bin_dispose), + (gst_bin_sync_children_state): + * gst/gstbin.h: + * gst/gstthread.c: (gst_thread_class_init), (gst_thread_release), + (gst_thread_change_state): + * testsuite/states/Makefile.am: + revert state change patches as agreed so we can rework them + gradually + 2004-07-29 Benjamin Otte * libs/gst/control/Makefile.am: diff --git a/gst/gstbin.c b/gst/gstbin.c index f129af681e..3810235acd 100644 --- a/gst/gstbin.c +++ b/gst/gstbin.c @@ -34,14 +34,6 @@ #include "gstindex.h" #include "gstutils.h" -GST_DEBUG_CATEGORY_STATIC (bin_debug); -#define GST_CAT_DEFAULT bin_debug -#define GST_LOG_BIN_CONTENTS(bin, text) GST_LOG_OBJECT ((bin), \ - text ": %d elements: %u PLAYING, %u PAUSED, %u READY, %u NULL, own state: %s", \ - (bin)->numchildren, (guint) (bin)->child_states[3], \ - (guint) (bin)->child_states[2], (bin)->child_states[1], \ - (bin)->child_states[0], gst_element_state_get_name (GST_STATE (bin))) - static GstElementDetails gst_bin_details = GST_ELEMENT_DETAILS ("Generic bin", "Generic/Bin", "Simple container object", @@ -54,6 +46,7 @@ static gboolean _gst_boolean_did_something_accumulator (GSignalInvocationHint * static void gst_bin_dispose (GObject * object); +static GstElementStateReturn gst_bin_change_state (GstElement * element); static GstElementStateReturn gst_bin_change_state_norecurse (GstBin * bin); #ifndef GST_DISABLE_INDEX @@ -123,8 +116,6 @@ gst_bin_get_type (void) _gst_bin_type = g_type_register_static (GST_TYPE_ELEMENT, "GstBin", &bin_info, 0); - GST_DEBUG_CATEGORY_INIT (bin_debug, "bin", GST_DEBUG_BOLD, - "debugging info for the 'bin' container element"); } return _gst_bin_type; } @@ -172,6 +163,7 @@ gst_bin_class_init (GstBinClass * klass) GST_DEBUG_FUNCPTR (gst_bin_restore_thyself); #endif + gstelement_class->change_state = GST_DEBUG_FUNCPTR (gst_bin_change_state); gstelement_class->set_state = GST_DEBUG_FUNCPTR (gst_bin_set_state); #ifndef GST_DISABLE_INDEX gstelement_class->set_index = GST_DEBUG_FUNCPTR (gst_bin_set_index); @@ -449,34 +441,6 @@ gst_bin_add_many (GstBin * bin, GstElement * element_1, ...) va_end (args); } -/* after adding or removing elements, we need to fix the state of the bin - * in a reentrant way. This is done here */ -static void -gst_bin_fix_state (GstBin * bin) -{ - gint i; - GstElementState desired; - - while (TRUE) { - /* find the highest child state */ - for (i = GST_NUM_STATES - 1; i > 0; i--) { - if (bin->child_states[i] > 0) - break; - } - g_assert (i < GST_NUM_STATES && i >= 0); - desired = 1 << i; - if (desired == GST_STATE (bin)) { - break; - } else if (desired < GST_STATE (bin)) { - GST_STATE_PENDING (bin) = GST_STATE (bin) >> 1; - } else { /* if (desired > GST_STATE (bin)) */ - GST_STATE_PENDING (bin) = GST_STATE (bin) << 1; - } - /* this part is reentrant */ - gst_bin_change_state_norecurse (bin); - } -} - static void gst_bin_add_func (GstBin * bin, GstElement * element) { @@ -495,14 +459,15 @@ gst_bin_add_func (GstBin * bin, GstElement * element) return; } - /* ref to guard unrefs in callbacks */ - gst_object_ref (GST_OBJECT (bin)); - gst_object_ref (GST_OBJECT (element)); + if (GST_STATE (element) > GST_STATE (bin)) { + GST_CAT_DEBUG_OBJECT (GST_CAT_STATES, bin, + "setting state to receive element \"%s\"", GST_OBJECT_NAME (element)); + gst_element_set_state ((GstElement *) bin, GST_STATE (element)); + } /* set the element's parent and add the element to the bin's list of children */ gst_object_set_parent (GST_OBJECT (element), GST_OBJECT (bin)); - GST_LOG_BIN_CONTENTS (bin, "before adding element"); bin->children = g_list_append (bin->children, element); bin->numchildren++; @@ -520,16 +485,10 @@ gst_bin_add_func (GstBin * bin, GstElement * element) gst_bin_set_element_sched (element, sched); } - /* check if we need to bump state because a high state element was added */ - gst_bin_fix_state (bin); GST_CAT_DEBUG_OBJECT (GST_CAT_PARENTAGE, bin, "added element \"%s\"", GST_OBJECT_NAME (element)); - GST_LOG_BIN_CONTENTS (bin, "after adding element"); g_signal_emit (G_OBJECT (bin), gst_bin_signals[ELEMENT_ADDED], 0, element); - /* we reffed above */ - gst_object_unref (GST_OBJECT (element)); - gst_object_unref (GST_OBJECT (bin)); } /** @@ -578,13 +537,9 @@ gst_bin_remove_func (GstBin * bin, GstElement * element) return; } - /* ref to guard unrefs in callbacks */ - gst_object_ref (GST_OBJECT (bin)); - /* remove this element from the list of managed elements */ gst_bin_unset_element_sched (element, GST_ELEMENT_SCHED (bin)); - GST_LOG_BIN_CONTENTS (bin, "before removing element"); /* now remove the element from the list of elements */ bin->children = g_list_remove (bin->children, element); bin->numchildren--; @@ -595,22 +550,30 @@ gst_bin_remove_func (GstBin * bin, GstElement * element) state_idx++; bin->child_states[state_idx]--; - GST_CAT_INFO_OBJECT (GST_CAT_PARENTAGE, bin, - "removed child \"%s\", %d children left", GST_OBJECT_NAME (element), - bin->numchildren); + GST_CAT_INFO_OBJECT (GST_CAT_PARENTAGE, bin, "removed child \"%s\"", + GST_OBJECT_NAME (element)); /* ref as we're going to emit a signal */ gst_object_ref (GST_OBJECT (element)); gst_object_unparent (GST_OBJECT (element)); - /* check the state */ - gst_bin_fix_state (bin); - GST_LOG_BIN_CONTENTS (bin, "after removing element"); + /* if we're down to zero children, force state to NULL */ + while (bin->numchildren == 0 && GST_ELEMENT_SCHED (bin) != NULL && + GST_STATE (bin) > GST_STATE_NULL) { + GstElementState next = GST_STATE (bin) >> 1; + + GST_STATE_PENDING (bin) = next; + gst_bin_change_state_norecurse (bin); + if (!GST_STATE (bin) == next) { + g_warning ("bin %s failed state change to %d", GST_ELEMENT_NAME (bin), + next); + break; + } + } g_signal_emit (G_OBJECT (bin), gst_bin_signals[ELEMENT_REMOVED], 0, element); /* element is really out of our control now */ gst_object_unref (GST_OBJECT (element)); - gst_object_unref (GST_OBJECT (bin)); } /** @@ -693,8 +656,7 @@ gst_bin_child_state_change (GstBin * bin, GstElementState oldstate, g_return_if_fail (GST_IS_ELEMENT (child)); GST_CAT_LOG (GST_CAT_STATES, "child %s changed state in bin %s from %s to %s", - GST_ELEMENT_NAME (child) ? GST_ELEMENT_NAME (child) : "(null)", - GST_ELEMENT_NAME (bin) ? GST_ELEMENT_NAME (bin) : "(null)", + GST_ELEMENT_NAME (child), GST_ELEMENT_NAME (bin), gst_element_state_get_name (oldstate), gst_element_state_get_name (newstate)); @@ -722,7 +684,7 @@ gst_bin_child_state_change_func (GstBin * bin, GstElementState oldstate, while (new >>= 1) new_idx++; - GST_LOG_BIN_CONTENTS (bin, "before child state change"); + GST_LOCK (bin); bin->child_states[old_idx]--; bin->child_states[new_idx]++; @@ -735,17 +697,18 @@ gst_bin_child_state_change_func (GstBin * bin, GstElementState oldstate, "highest child state is %s, changing bin state accordingly", gst_element_state_get_name (state)); GST_STATE_PENDING (bin) = state; + GST_UNLOCK (bin); gst_bin_change_state_norecurse (bin); if (state != GST_STATE (bin)) { - GST_INFO_OBJECT (bin, "state change in callback %d %d", - state, GST_STATE (bin)); + g_warning ("%s: state change in callback %d %d", + GST_ELEMENT_NAME (bin), state, GST_STATE (bin)); } return; } break; } } - GST_LOG_BIN_CONTENTS (bin, "after child state change"); + GST_UNLOCK (bin); } typedef gboolean (*GstBinForeachFunc) (GstBin * bin, GstElement * element, @@ -794,12 +757,10 @@ gst_bin_foreach (GstBin * bin, GstBinForeachFunc func, gpointer data) typedef struct { - GstElementState intermediate; GstElementState pending; GstElementStateReturn result; } SetKidStateData; - static int set_kid_state_func (GstBin * bin, GstElement * child, gpointer user_data) { @@ -812,49 +773,17 @@ set_kid_state_func (GstBin * bin, GstElement * child, gpointer user_data) old_child_state = GST_STATE (child); - /* are we moving up or down? */ - if (data->intermediate < data->pending) { - /* up, check if we are already closer to target */ - if (old_child_state >= data->intermediate && - old_child_state < data->pending) { - GST_CAT_DEBUG_OBJECT (GST_CAT_STATES, bin, - "state of child %s is %s, inbetween intermediate %s and pending %s", - GST_ELEMENT_NAME (child), - gst_element_state_get_name (old_child_state), - gst_element_state_get_name (data->intermediate), - gst_element_state_get_name (data->pending)); - return TRUE; - } - } else if (data->intermediate > data->pending) { - /* down, check if we are already closer to target */ - if (old_child_state < data->intermediate) { - GST_CAT_DEBUG_OBJECT (GST_CAT_STATES, bin, - "state of child %s is %s, inbetween intermediate %s and pending %s", - GST_ELEMENT_NAME (child), - gst_element_state_get_name (old_child_state), - gst_element_state_get_name (data->intermediate), - gst_element_state_get_name (data->pending)); - return TRUE; - } - } else { - /* same state */ - GST_CAT_DEBUG_OBJECT (GST_CAT_STATES, bin, - "setting final state on child %s, now in %s, going to %s", - GST_ELEMENT_NAME (child), gst_element_state_get_name (old_child_state), - gst_element_state_get_name (data->intermediate)); - } - GST_CAT_DEBUG_OBJECT (GST_CAT_STATES, bin, - "changing state of child %s from current %s to intermediate %s", + "changing state of child %s from current %s to pending %s", GST_ELEMENT_NAME (child), gst_element_state_get_name (old_child_state), - gst_element_state_get_name (data->intermediate)); + gst_element_state_get_name (data->pending)); - switch (gst_element_set_state (child, data->intermediate)) { + switch (gst_element_set_state (child, data->pending)) { case GST_STATE_FAILURE: GST_CAT_INFO_OBJECT (GST_CAT_STATES, bin, "child '%s' failed to go to state %d(%s)", GST_ELEMENT_NAME (child), - data->intermediate, gst_element_state_get_name (data->intermediate)); + data->pending, gst_element_state_get_name (data->pending)); gst_element_set_state (child, old_child_state); return FALSE; /* error out to the caller */ @@ -863,14 +792,14 @@ set_kid_state_func (GstBin * bin, GstElement * child, gpointer user_data) GST_CAT_INFO_OBJECT (GST_CAT_STATES, bin, "child '%s' is changing state asynchronously", GST_ELEMENT_NAME (child)); + data->result = GST_STATE_ASYNC; return TRUE; case GST_STATE_SUCCESS: GST_CAT_DEBUG (GST_CAT_STATES, "child '%s' changed state to %d(%s) successfully", - GST_ELEMENT_NAME (child), data->intermediate, - gst_element_state_get_name (data->intermediate)); - data->result = GST_STATE_SUCCESS; + GST_ELEMENT_NAME (child), data->pending, + gst_element_state_get_name (data->pending)); return TRUE; default: @@ -879,66 +808,87 @@ set_kid_state_func (GstBin * bin, GstElement * child, gpointer user_data) } } +static GstElementStateReturn +gst_bin_change_state (GstElement * element) +{ + GstBin *bin; + GstElementStateReturn ret; + GstElementState old_state, pending; + SetKidStateData data; + + g_return_val_if_fail (GST_IS_BIN (element), GST_STATE_FAILURE); + + bin = GST_BIN (element); + + old_state = GST_STATE (element); + pending = GST_STATE_PENDING (element); + + GST_CAT_DEBUG_OBJECT (GST_CAT_STATES, element, + "changing state of children from %s to %s", + gst_element_state_get_name (old_state), + gst_element_state_get_name (pending)); + + if (pending == GST_STATE_VOID_PENDING) + return GST_STATE_SUCCESS; + + data.pending = pending; + data.result = GST_STATE_SUCCESS; + if (!gst_bin_foreach (bin, set_kid_state_func, &data)) { + GST_STATE_PENDING (element) = old_state; + return GST_STATE_FAILURE; + } + + GST_CAT_DEBUG_OBJECT (GST_CAT_STATES, element, + "done changing bin's state from %s to %s, now in %s", + gst_element_state_get_name (old_state), + gst_element_state_get_name (pending), + gst_element_state_get_name (GST_STATE (element))); + + if (data.result == GST_STATE_ASYNC) + ret = GST_STATE_ASYNC; + else { + /* FIXME: this should have been done by the children already, no? */ + if (parent_class->change_state) { + ret = parent_class->change_state (element); + } else + ret = GST_STATE_SUCCESS; + } + return ret; +} + GstElementStateReturn gst_bin_set_state (GstElement * element, GstElementState state) { GstBin *bin = GST_BIN (element); - SetKidStateData data; - GstElementStateReturn ret = GST_STATE_FAILURE; - GstElementState intermediate; - GstElementState pending; - /* we start with the state of the bin */ - intermediate = GST_STATE (element); - pending = state; - - GST_CAT_DEBUG_OBJECT (GST_CAT_STATES, bin, - "setting state of bin %s from current %s to pending %s", - GST_ELEMENT_NAME (element), gst_element_state_get_name (intermediate), - gst_element_state_get_name (pending)); - - do { - data.intermediate = intermediate; - data.pending = pending; - data.result = GST_STATE_ASYNC; - - GST_CAT_DEBUG_OBJECT (GST_CAT_STATES, bin, - "setting state of children to intermediate %s", - gst_element_state_get_name (intermediate)); + if (GST_STATE (bin) == state) { + SetKidStateData data; + data.pending = state; + data.result = GST_STATE_SUCCESS; if (!gst_bin_foreach (bin, set_kid_state_func, &data)) { - ret = GST_STATE_FAILURE; - /* break out of the loop after failure */ - break; + return GST_STATE_FAILURE; } else { - ret = data.result; + return data.result; } - - /* if we have reached the target state, we can stop */ - if (intermediate == pending) - break; - - /* move intermediate state closer to target state */ - if (intermediate < pending) - intermediate <<= 1; - else - intermediate >>= 1; + } else { + return GST_CALL_PARENT_WITH_DEFAULT (GST_ELEMENT_CLASS, set_state, (element, + state), GST_STATE_FAILURE); } - while (TRUE); - - return ret; } static GstElementStateReturn gst_bin_change_state_norecurse (GstBin * bin) { - GstElementClass *klass; + GstElementStateReturn ret; - klass = GST_ELEMENT_GET_CLASS (bin); - g_return_val_if_fail (klass->change_state, GST_STATE_FAILURE); + if (parent_class->change_state) { + GST_CAT_LOG_OBJECT (GST_CAT_STATES, bin, "setting bin's own state"); + ret = parent_class->change_state (GST_ELEMENT (bin)); - GST_CAT_LOG_OBJECT (GST_CAT_STATES, bin, "setting bin's own state"); - return klass->change_state (GST_ELEMENT (bin)); + return ret; + } else + return GST_STATE_FAILURE; } static void @@ -955,10 +905,6 @@ gst_bin_dispose (GObject * object) } g_assert (bin->children == NULL); g_assert (bin->numchildren == 0); - g_assert (bin->child_states[3] == 0); /* playing */ - g_assert (bin->child_states[2] == 0); /* paused */ - g_assert (bin->child_states[1] == 0); /* ready */ - g_assert (bin->child_states[0] == 0); /* null */ G_OBJECT_CLASS (parent_class)->dispose (object); } @@ -1141,10 +1087,40 @@ gst_bin_get_all_by_interface (GstBin * bin, GType interface) GstElementStateReturn gst_bin_sync_children_state (GstBin * bin) { + GList *children; + GstElement *element; + GstElementState state; + GstElementStateReturn ret = GST_STATE_SUCCESS; + g_return_val_if_fail (GST_IS_BIN (bin), GST_STATE_FAILURE); - return gst_element_set_state (GST_ELEMENT (bin), - gst_element_get_state (GST_ELEMENT (bin))); + state = GST_STATE (bin); + children = bin->children; + GST_CAT_INFO (GST_CAT_STATES, + "syncing state of children with bin \"%s\"'s state %s", + GST_ELEMENT_NAME (bin), gst_element_state_get_name (state)); + + while (children) { + element = GST_ELEMENT (children->data); + children = children->next; + if (GST_STATE (element) != state) { + switch (gst_element_set_state (element, state)) { + case GST_STATE_SUCCESS: + break; + case GST_STATE_ASYNC: + if (ret == GST_STATE_SUCCESS) + ret = GST_STATE_ASYNC; + break; + case GST_STATE_FAILURE: + ret = GST_STATE_FAILURE; + default: + /* make sure gst_element_set_state never returns this */ + g_assert_not_reached (); + } + } + } + + return ret; } #ifndef GST_DISABLE_LOADSAVE diff --git a/gst/gstbin.h b/gst/gstbin.h index e23df3a196..b0c3f538f5 100644 --- a/gst/gstbin.h +++ b/gst/gstbin.h @@ -63,7 +63,6 @@ struct _GstBin { gint numchildren; GList *children; -/* FIXME 0.9: important!! make this guint instead of GstElementState */ GstElementState child_states[GST_NUM_STATES]; gpointer _gst_reserved[GST_PADDING]; @@ -111,9 +110,7 @@ void gst_bin_use_clock (GstBin *bin, GstClock *clock); GstClock* gst_bin_get_clock (GstBin *bin); void gst_bin_auto_clock (GstBin *bin); -#ifndef GST_DISABLE_DEPRECATED GstElementStateReturn gst_bin_sync_children_state (GstBin *bin); -#endif /* internal */ /* one of our childs signaled a state change */ diff --git a/gst/gstthread.c b/gst/gstthread.c index 0997ed17d9..f5f131feb0 100644 --- a/gst/gstthread.c +++ b/gst/gstthread.c @@ -27,7 +27,6 @@ #include "gstmarshal.h" #include "gstscheduler.h" #include "gstinfo.h" -#include "gstutils.h" #define GST_CAT_DEFAULT GST_CAT_THREAD #define STACK_SIZE 0x200000 @@ -72,8 +71,6 @@ static void gst_thread_set_property (GObject * object, guint prop_id, static void gst_thread_get_property (GObject * object, guint prop_id, GValue * value, GParamSpec * pspec); static GstElementStateReturn gst_thread_change_state (GstElement * element); -static GstElementStateReturn gst_thread_set_state (GstElement * element, - GstElementState state); static void gst_thread_child_state_change (GstBin * bin, GstElementState oldstate, GstElementState newstate, GstElement * element); @@ -184,7 +181,6 @@ gst_thread_class_init (gpointer g_class, gpointer class_data) #endif gstelement_class->change_state = GST_DEBUG_FUNCPTR (gst_thread_change_state); - gstelement_class->set_state = GST_DEBUG_FUNCPTR (gst_thread_set_state); gobject_class->set_property = GST_DEBUG_FUNCPTR (gst_thread_set_property); gobject_class->get_property = GST_DEBUG_FUNCPTR (gst_thread_get_property); @@ -406,34 +402,11 @@ static void gst_thread_release (GstThread * thread) { if (thread != gst_thread_get_current ()) { - GST_DEBUG_OBJECT (thread, "releasing lock"); g_cond_signal (thread->cond); g_mutex_unlock (thread->lock); } } -static GstElementStateReturn -gst_thread_set_state (GstElement * element, GstElementState state) -{ - GstElementStateReturn result; - GstThread *thread = GST_THREAD (element); - - if (thread != gst_thread_get_current ()) { - gst_thread_catch (thread); - GST_DEBUG_OBJECT (thread, "releasing lock"); - g_mutex_unlock (thread->lock); - } - result = - GST_CALL_PARENT_WITH_DEFAULT (GST_ELEMENT_CLASS, set_state, (element, - state), GST_STATE_FAILURE); - if (thread != gst_thread_get_current ()) { - GST_DEBUG_OBJECT (thread, "grabbing lock"); - g_mutex_lock (thread->lock); - gst_thread_release (thread); - } - return result; -} - static GstElementStateReturn gst_thread_change_state (GstElement * element) { @@ -478,13 +451,30 @@ gst_thread_change_state (GstElement * element) break; case GST_STATE_PAUSED_TO_PLAYING: { + /* FIXME: recurse into sub-bins */ + GList *elements = (GList *) gst_bin_get_list (GST_BIN (thread)); + + while (elements) { + gst_element_enable_threadsafe_properties ((GstElement *) elements-> + data); + elements = g_list_next (elements); + } /* reset self to spinning */ if (thread == gst_thread_get_current ()) GST_FLAG_SET (thread, GST_THREAD_STATE_SPINNING); break; } case GST_STATE_PLAYING_TO_PAUSED: + { + GList *elements = (GList *) gst_bin_get_list (GST_BIN (thread)); + + while (elements) { + gst_element_disable_threadsafe_properties ((GstElement *) elements-> + data); + elements = g_list_next (elements); + } break; + } case GST_STATE_PAUSED_TO_READY: break; case GST_STATE_READY_TO_NULL: diff --git a/tests/old/testsuite/states/Makefile.am b/tests/old/testsuite/states/Makefile.am index 0b9ce6612b..2a01e7553e 100644 --- a/tests/old/testsuite/states/Makefile.am +++ b/tests/old/testsuite/states/Makefile.am @@ -1,5 +1,5 @@ include ../Rules -tests_pass = locked parent bin +tests_pass = locked parent tests_fail = -tests_ignore = +tests_ignore = bin diff --git a/testsuite/states/Makefile.am b/testsuite/states/Makefile.am index 0b9ce6612b..2a01e7553e 100644 --- a/testsuite/states/Makefile.am +++ b/testsuite/states/Makefile.am @@ -1,5 +1,5 @@ include ../Rules -tests_pass = locked parent bin +tests_pass = locked parent tests_fail = -tests_ignore = +tests_ignore = bin