From ebf770154f60f1ce1615b3553f4ae0eaf962f103 Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Thu, 16 Aug 2007 11:04:40 +0000 Subject: [PATCH] gst/gstbin.c: Improve debugging. Original commit message from CVS: * gst/gstbin.c: (is_eos), (gst_bin_add_func), (bin_handle_async_start), (gst_bin_handle_message_func): Improve debugging. When adding elements, insert messages into the bus of the newly added element and make sure the element is the source of the message. This allows the parent bin to intercept the message and do the right thing. It also avoids us posting ASYNC_START and CLOCK_PROVIDE messages to the app (which is not allowed). Update some docs. * tests/check/gst/gstghostpad.c: (GST_START_TEST): Fix testsuite so that is does not work around messages that should not have been posted in the first place. --- ChangeLog | 17 ++++++++ gst/gstbin.c | 73 ++++++++++++++++++++--------------- tests/check/gst/gstghostpad.c | 15 ++----- 3 files changed, 63 insertions(+), 42 deletions(-) diff --git a/ChangeLog b/ChangeLog index 038b0c5012..03a0309f27 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,19 @@ +2007-08-16 Wim Taymans + + * gst/gstbin.c: (is_eos), (gst_bin_add_func), + (bin_handle_async_start), (gst_bin_handle_message_func): + Improve debugging. + When adding elements, insert messages into the bus of the newly added + element and make sure the element is the source of the message. This + allows the parent bin to intercept the message and do the + right thing. It also avoids us posting ASYNC_START and CLOCK_PROVIDE + messages to the app (which is not allowed). + Update some docs. + + * tests/check/gst/gstghostpad.c: (GST_START_TEST): + Fix testsuite so that is does not work around messages that should not + have been posted in the first place. + 2007-08-16 Wim Taymans * gst/gstbin.c: (add_to_queue), (remove_from_queue), (clear_queue), @@ -22,6 +38,7 @@ 2007-08-14 Wim Taymans + * gst/gstbin.c: (gst_bin_add_func), (gst_bin_element_set_state), (bin_handle_async_start), (gst_bin_handle_message_func): Move ASYNC_START message posting to where it belongs, similar to diff --git a/gst/gstbin.c b/gst/gstbin.c index 6e736809bd..0909990e49 100644 --- a/gst/gstbin.c +++ b/gst/gstbin.c @@ -829,9 +829,10 @@ is_eos (GstBin * bin) if (bin_element_is_sink (element, bin) == 0) { /* check if element posted EOS */ if (find_message (bin, GST_OBJECT_CAST (element), GST_MESSAGE_EOS)) { - GST_DEBUG ("element posted EOS"); + GST_DEBUG ("sink '%s' posted EOS", GST_ELEMENT_NAME (element)); } else { - GST_DEBUG ("element did not post EOS yet"); + GST_DEBUG ("sink '%s' did not post EOS yet", + GST_ELEMENT_NAME (element)); result = FALSE; break; } @@ -865,7 +866,7 @@ gst_bin_add_func (GstBin * bin, GstElement * element) gchar *elem_name; GstIterator *it; gboolean is_sink; - GstMessage *clock_message = NULL; + GstMessage *clock_message = NULL, *async_message = NULL; GstStateChangeReturn ret; GST_DEBUG_OBJECT (bin, "element :%s", GST_ELEMENT_NAME (element)); @@ -902,9 +903,8 @@ gst_bin_add_func (GstBin * bin, GstElement * element) } if (gst_element_provides_clock (element)) { GST_DEBUG_OBJECT (bin, "element \"%s\" can provide a clock", elem_name); - bin->clock_dirty = TRUE; clock_message = - gst_message_new_clock_provide (GST_OBJECT_CAST (bin), NULL, TRUE); + gst_message_new_clock_provide (GST_OBJECT_CAST (element), NULL, TRUE); } bin->children = g_list_prepend (bin->children, element); @@ -917,26 +917,6 @@ gst_bin_add_func (GstBin * bin, GstElement * element) if (ret == GST_STATE_CHANGE_FAILURE) goto no_state_recalc; - /* update the bin state, the new element could have been an ASYNC or - * NO_PREROLL element */ - ret = GST_STATE_RETURN (element); - GST_DEBUG_OBJECT (bin, "added %s element", - gst_element_state_change_return_get_name (ret)); - - switch (ret) { - case GST_STATE_CHANGE_ASYNC: - bin_handle_async_start (bin, FALSE); - break; - case GST_STATE_CHANGE_NO_PREROLL: - bin_handle_async_done (bin, ret); - break; - case GST_STATE_CHANGE_FAILURE: - break; - default: - break; - } - -no_state_recalc: /* distribute the bus */ gst_element_set_bus (element, bin->child_bus); @@ -946,10 +926,42 @@ no_state_recalc: * that is not important right now. When the pipeline goes to PLAYING, * a new clock will be selected */ gst_element_set_clock (element, GST_ELEMENT_CLOCK (bin)); + + /* update the bin state, the new element could have been an ASYNC or + * NO_PREROLL element */ + ret = GST_STATE_RETURN (element); + GST_DEBUG_OBJECT (bin, "added %s element", + gst_element_state_change_return_get_name (ret)); + + switch (ret) { + case GST_STATE_CHANGE_ASYNC: + { + /* create message to track this aync element when it posts an async-done + * message */ + async_message = + gst_message_new_async_start (GST_OBJECT_CAST (element), FALSE); + break; + } + case GST_STATE_CHANGE_NO_PREROLL: + /* ignore all async elements we might have and commit our state */ + bin_handle_async_done (bin, ret); + break; + case GST_STATE_CHANGE_FAILURE: + break; + default: + break; + } + +no_state_recalc: GST_OBJECT_UNLOCK (bin); + /* post the messages on the bus of the element so that the bin can handle + * them */ if (clock_message) - gst_element_post_message (GST_ELEMENT_CAST (bin), clock_message); + gst_element_post_message (GST_ELEMENT_CAST (element), clock_message); + + if (async_message) + gst_element_post_message (GST_ELEMENT_CAST (element), async_message); /* unlink all linked pads */ it = gst_element_iterate_pads (element); @@ -2366,6 +2378,7 @@ bin_handle_async_start (GstBin * bin, gboolean new_base_time) if (GST_STATE_RETURN (bin) == GST_STATE_CHANGE_NO_PREROLL) goto was_no_preroll; + old_state = GST_STATE (bin); /* when we PLAYING we go back to PAUSED, when preroll happens, we go back to @@ -2554,10 +2567,7 @@ nothing_pending: * in the PLAYING state. If all sinks posted the EOS message, post * one upwards. * - * GST_MESSAGE_STATE_DIRTY: if we are the toplevel bin we do a state - * recalc. If we are not toplevel (we have a parent) we just post - * the message upwards. This makes sure only the toplevel bin will - * run over all its children to check if a state change happened. + * GST_MESSAGE_STATE_DIRTY: Deprecated * * GST_MESSAGE_SEGMENT_START: just collect, never forward upwards. If an * element posts segment_start twice, only the last message is kept. @@ -2594,7 +2604,7 @@ nothing_pending: * * GST_MESSAGE_ASYNC_START: Create an internal ELEMENT message that stores * the state of the element and the fact that the element will need a - * new base_time. + * new base_time. This message is not forwarded to the application. * * GST_MESSAGE_ASYNC_DONE: Find the internal ELEMENT message we kept for the * element when it posted ASYNC_START. If all elements are done, post a @@ -2793,6 +2803,7 @@ gst_bin_handle_message_func (GstBin * bin, GstMessage * message) /* nothing found, remove all old ASYNC_DONE messages */ bin_remove_messages (bin, NULL, GST_MESSAGE_ASYNC_DONE); + GST_DEBUG_OBJECT (bin, "async elements commited"); bin_handle_async_done (bin, GST_STATE_CHANGE_SUCCESS); } GST_OBJECT_UNLOCK (bin); diff --git a/tests/check/gst/gstghostpad.c b/tests/check/gst/gstghostpad.c index 14f6d11b5c..1e8ba08684 100644 --- a/tests/check/gst/gstghostpad.c +++ b/tests/check/gst/gstghostpad.c @@ -41,8 +41,7 @@ GST_START_TEST (test_remove1) ASSERT_OBJECT_REFCOUNT (b1, "pipeline", 1); ASSERT_OBJECT_REFCOUNT (b2, "bin", 1); fail_unless (gst_bin_add (GST_BIN (b1), b2)); - /* adding the bin creates a clock provide message with a ref to pipeline */ - ASSERT_OBJECT_REFCOUNT (b1, "pipeline", 2); + ASSERT_OBJECT_REFCOUNT (b1, "pipeline", 1); ASSERT_OBJECT_REFCOUNT (b2, "bin", 1); sinkpad = gst_element_get_pad (sink, "sink"); @@ -59,7 +58,7 @@ GST_START_TEST (test_remove1) gst_object_unref (sinkpad); /* now remove the bin with the ghostpad, b2 is disposed now. */ - ASSERT_OBJECT_REFCOUNT (b1, "pipeline", 2); + ASSERT_OBJECT_REFCOUNT (b1, "pipeline", 1); ASSERT_OBJECT_REFCOUNT (b2, "bin", 1); gst_bin_remove (GST_BIN (b1), b2); @@ -68,9 +67,6 @@ GST_START_TEST (test_remove1) fail_if (gst_pad_is_linked (srcpad)); gst_object_unref (srcpad); - /* flush the message, dropping the b1 refcount to 1 */ - gst_element_set_state (b1, GST_STATE_READY); - gst_element_set_state (b1, GST_STATE_NULL); ASSERT_OBJECT_REFCOUNT (b1, "pipeline", 1); gst_object_unref (b1); } @@ -389,11 +385,11 @@ GST_START_TEST (test_ghost_pads_bin) srcbin = GST_BIN (gst_bin_new ("srcbin")); gst_bin_add (pipeline, GST_ELEMENT (srcbin)); - ASSERT_OBJECT_REFCOUNT (pipeline, "pipeline", 2); /* provide-clock msg */ + ASSERT_OBJECT_REFCOUNT (pipeline, "pipeline", 1); sinkbin = GST_BIN (gst_bin_new ("sinkbin")); gst_bin_add (pipeline, GST_ELEMENT (sinkbin)); - ASSERT_OBJECT_REFCOUNT (pipeline, "pipeline", 3); /* provide-clock msg */ + ASSERT_OBJECT_REFCOUNT (pipeline, "pipeline", 1); src = gst_element_factory_make ("fakesrc", "src"); gst_bin_add (srcbin, src); @@ -420,9 +416,6 @@ GST_START_TEST (test_ghost_pads_bin) fail_unless (GST_PAD_PEER (target) != NULL); gst_object_unref (target); - /* flush the message, dropping the b1 refcount to 1 */ - gst_element_set_state (GST_ELEMENT (pipeline), GST_STATE_READY); - gst_element_set_state (GST_ELEMENT (pipeline), GST_STATE_NULL); ASSERT_OBJECT_REFCOUNT (pipeline, "pipeline", 1); gst_object_unref (pipeline);