diff --git a/ChangeLog b/ChangeLog index 34bef0264b..daf048f805 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,20 @@ +2005-07-28 Wim Taymans + + * check/gst/gstbin.c: (GST_START_TEST), (gst_bin_suite): + Added checks for hierarchy consistency whan adding linked + elements to bins. + + * check/gst/gstelement.c: (GST_START_TEST), (gst_element_suite): + Added check to test element scheduling without bin/pipeline. + + * check/pipelines/simple_launch_lines.c: (GST_START_TEST): + First add elements to bin, then link. + + * gst/gstbin.c: (unlink_pads), (gst_bin_add_func), + (gst_bin_remove_func): + Unlink pads from elements added/removed from bin to maintain + hierarchy consistency. + 2005-07-28 Ronald S. Bultje * gst/base/gstbasetransform.c: (gst_base_transform_setcaps), diff --git a/check/gst/gstbin.c b/check/gst/gstbin.c index dd064991c1..0fe08140c0 100644 --- a/check/gst/gstbin.c +++ b/check/gst/gstbin.c @@ -270,6 +270,56 @@ GST_START_TEST (test_message_state_changed_children) GST_END_TEST; +/* adding an element with linked pads to a bin unlinks the + * pads */ +GST_START_TEST (test_add_linked) +{ + GstElement *src, *sink; + GstPad *srcpad, *sinkpad; + GstElement *pipeline; + + pipeline = gst_pipeline_new (NULL); + fail_unless (pipeline != NULL, "Could not create pipeline"); + + src = gst_element_factory_make ("fakesrc", NULL); + fail_if (src == NULL, "Could not create fakesrc"); + sink = gst_element_factory_make ("fakesink", NULL); + fail_if (src == NULL, "Could not create fakesink"); + + srcpad = gst_element_get_pad (src, "src"); + fail_unless (srcpad != NULL); + sinkpad = gst_element_get_pad (sink, "sink"); + fail_unless (sinkpad != NULL); + + fail_unless (gst_pad_link (srcpad, sinkpad) == GST_PAD_LINK_OK); + + /* pads are linked now */ + fail_unless (gst_pad_is_linked (srcpad)); + fail_unless (gst_pad_is_linked (sinkpad)); + + /* adding element to bin voids hierarchy so pads are unlinked */ + gst_bin_add (GST_BIN (pipeline), src); + + /* check if pads really are unlinked */ + fail_unless (!gst_pad_is_linked (srcpad)); + fail_unless (!gst_pad_is_linked (sinkpad)); + + /* cannot link pads in wrong hierarchy */ + fail_unless (gst_pad_link (srcpad, sinkpad) == GST_PAD_LINK_WRONG_HIERARCHY); + + /* adding other element to bin as well */ + gst_bin_add (GST_BIN (pipeline), sink); + + /* now we can link again */ + fail_unless (gst_pad_link (srcpad, sinkpad) == GST_PAD_LINK_OK); + + /* check if pads really are linked */ + fail_unless (gst_pad_is_linked (srcpad)); + fail_unless (gst_pad_is_linked (sinkpad)); +} + +GST_END_TEST; + Suite * gst_bin_suite (void) { @@ -281,6 +331,7 @@ gst_bin_suite (void) tcase_add_test (tc_chain, test_message_state_changed); tcase_add_test (tc_chain, test_message_state_changed_child); tcase_add_test (tc_chain, test_message_state_changed_children); + tcase_add_test (tc_chain, test_add_linked); return s; } diff --git a/check/gst/gstelement.c b/check/gst/gstelement.c index 20e37ac0a3..27bd84e010 100644 --- a/check/gst/gstelement.c +++ b/check/gst/gstelement.c @@ -94,6 +94,43 @@ GST_START_TEST (test_error_no_bus) GST_END_TEST; +/* link and run two elements without putting them in a + * pipeline */ +GST_START_TEST (test_link) +{ + GstElement *src, *sink; + + src = gst_element_factory_make ("fakesrc", "source"); + sink = gst_element_factory_make ("fakesink", "sink"); + + fail_unless (gst_element_link_pads (src, "src", sink, "sink")); + + /* do sink to source state change */ + gst_element_set_state (sink, GST_STATE_PAUSED); + gst_element_set_state (src, GST_STATE_PAUSED); + + /* wait for preroll */ + gst_element_get_state (sink, NULL, NULL, NULL); + + /* play some more */ + gst_element_set_state (sink, GST_STATE_PLAYING); + gst_element_set_state (src, GST_STATE_PLAYING); + + g_usleep (G_USEC_PER_SEC); + + /* and stop */ + gst_element_set_state (sink, GST_STATE_PAUSED); + gst_element_set_state (src, GST_STATE_PAUSED); + + /* wait for preroll */ + gst_element_get_state (sink, NULL, NULL, NULL); + + gst_element_set_state (sink, GST_STATE_NULL); + gst_element_set_state (src, GST_STATE_NULL); +} + +GST_END_TEST; + Suite * gst_element_suite (void) { @@ -104,6 +141,7 @@ gst_element_suite (void) tcase_add_test (tc_chain, test_add_remove_pad); tcase_add_test (tc_chain, test_add_pad_unref_element); tcase_add_test (tc_chain, test_error_no_bus); + tcase_add_test (tc_chain, test_link); return s; } diff --git a/check/pipelines/simple_launch_lines.c b/check/pipelines/simple_launch_lines.c index e031482ab4..d29c88efb3 100644 --- a/check/pipelines/simple_launch_lines.c +++ b/check/pipelines/simple_launch_lines.c @@ -139,8 +139,8 @@ GST_START_TEST (test_stop_from_app) g_return_if_fail (fakesrc && fakesink && pipeline); - gst_element_link (fakesrc, fakesink); gst_bin_add_many (GST_BIN (pipeline), fakesrc, fakesink, NULL); + gst_element_link (fakesrc, fakesink); g_object_set (fakesink, "signal-handoffs", (gboolean) TRUE, NULL); g_signal_connect (fakesink, "handoff", G_CALLBACK (got_handoff), NULL); diff --git a/gst/gstbin.c b/gst/gstbin.c index 6d151e2d8b..9ebfe6481e 100644 --- a/gst/gstbin.c +++ b/gst/gstbin.c @@ -336,6 +336,21 @@ is_eos (GstBin * bin) return result; } +static void +unlink_pads (GstPad * pad) +{ + GstPad *peer; + + if ((peer = gst_pad_get_peer (pad))) { + if (gst_pad_get_direction (pad) == GST_PAD_SRC) + gst_pad_unlink (pad, peer); + else + gst_pad_unlink (peer, pad); + gst_object_unref (peer); + } + gst_object_unref (pad); +} + /* add an element to this bin * * MT safe @@ -344,13 +359,13 @@ static gboolean gst_bin_add_func (GstBin * bin, GstElement * element) { gchar *elem_name; + GstIterator *it; /* we obviously can't add ourself to ourself */ if (G_UNLIKELY (GST_ELEMENT_CAST (element) == GST_ELEMENT_CAST (bin))) goto adding_itself; - /* get the element name to make sure it is unique in this bin, FIXME, another - * thread can change the name after the unlock. */ + /* get the element name to make sure it is unique in this bin. */ GST_LOCK (element); elem_name = g_strdup (GST_ELEMENT_NAME (element)); GST_UNLOCK (element); @@ -359,9 +374,9 @@ gst_bin_add_func (GstBin * bin, GstElement * element) /* then check to see if the element's name is already taken in the bin, * we can safely take the lock here. This check is probably bogus because - * you can safely change the element name after adding it to the bin. */ - if (G_UNLIKELY (gst_object_check_uniqueness (bin->children, - elem_name) == FALSE)) + * you can safely change the element name after this check and before setting + * the object parent. The window is very small though... */ + if (G_UNLIKELY (!gst_object_check_uniqueness (bin->children, elem_name))) goto duplicate_name; /* set the element's parent and add the element to the bin's list of children */ @@ -369,15 +384,23 @@ gst_bin_add_func (GstBin * bin, GstElement * element) GST_OBJECT_CAST (bin)))) goto had_parent; + /* if we add a sink we become a sink */ if (GST_FLAG_IS_SET (element, GST_ELEMENT_IS_SINK)) GST_FLAG_SET (bin, GST_ELEMENT_IS_SINK); + /* unlink all linked pads */ + it = gst_element_iterate_pads (element); + gst_iterator_foreach (it, (GFunc) unlink_pads, element); + gst_iterator_free (it); + bin->children = g_list_prepend (bin->children, element); bin->numchildren++; bin->children_cookie++; + /* distribute the bus */ gst_element_set_bus (element, bin->child_bus); + /* propagate the current base time and clock */ gst_element_set_base_time (element, GST_ELEMENT (bin)->base_time); gst_element_set_clock (element, GST_ELEMENT_CLOCK (bin)); @@ -416,6 +439,7 @@ had_parent: } } + /** * gst_bin_add: * @bin: #GstBin to add element to @@ -424,6 +448,9 @@ had_parent: * Adds the given element to the bin. Sets the element's parent, and thus * takes ownership of the element. An element can only be added to one bin. * + * If the element's pads are linked to other pads, the pads will be unlinked + * before the element is added to the bin. + * * MT safe. * * Returns: TRUE if the element could be added, FALSE on wrong parameters or @@ -467,6 +494,7 @@ static gboolean gst_bin_remove_func (GstBin * bin, GstElement * element) { gchar *elem_name; + GstIterator *it; /* grab element name so we can print it */ GST_LOCK (element); @@ -478,6 +506,11 @@ gst_bin_remove_func (GstBin * bin, GstElement * element) if (G_UNLIKELY (g_list_find (bin->children, element) == NULL)) goto not_in_bin; + /* unlink all linked pads */ + it = gst_element_iterate_pads (element); + gst_iterator_foreach (it, (GFunc) unlink_pads, element); + gst_iterator_free (it); + /* now remove the element from the list of elements */ bin->children = g_list_remove (bin->children, element); bin->numchildren--; @@ -546,6 +579,9 @@ not_in_bin: * want the element to still exist after removing, you need to call * #gst_object_ref before removing it from the bin. * + * If the element's pads are linked to other pads, the pads will be unlinked + * before the element is removed from the bin. + * * MT safe. * * Returns: TRUE if the element could be removed, FALSE on wrong parameters or diff --git a/tests/check/gst/gstbin.c b/tests/check/gst/gstbin.c index dd064991c1..0fe08140c0 100644 --- a/tests/check/gst/gstbin.c +++ b/tests/check/gst/gstbin.c @@ -270,6 +270,56 @@ GST_START_TEST (test_message_state_changed_children) GST_END_TEST; +/* adding an element with linked pads to a bin unlinks the + * pads */ +GST_START_TEST (test_add_linked) +{ + GstElement *src, *sink; + GstPad *srcpad, *sinkpad; + GstElement *pipeline; + + pipeline = gst_pipeline_new (NULL); + fail_unless (pipeline != NULL, "Could not create pipeline"); + + src = gst_element_factory_make ("fakesrc", NULL); + fail_if (src == NULL, "Could not create fakesrc"); + sink = gst_element_factory_make ("fakesink", NULL); + fail_if (src == NULL, "Could not create fakesink"); + + srcpad = gst_element_get_pad (src, "src"); + fail_unless (srcpad != NULL); + sinkpad = gst_element_get_pad (sink, "sink"); + fail_unless (sinkpad != NULL); + + fail_unless (gst_pad_link (srcpad, sinkpad) == GST_PAD_LINK_OK); + + /* pads are linked now */ + fail_unless (gst_pad_is_linked (srcpad)); + fail_unless (gst_pad_is_linked (sinkpad)); + + /* adding element to bin voids hierarchy so pads are unlinked */ + gst_bin_add (GST_BIN (pipeline), src); + + /* check if pads really are unlinked */ + fail_unless (!gst_pad_is_linked (srcpad)); + fail_unless (!gst_pad_is_linked (sinkpad)); + + /* cannot link pads in wrong hierarchy */ + fail_unless (gst_pad_link (srcpad, sinkpad) == GST_PAD_LINK_WRONG_HIERARCHY); + + /* adding other element to bin as well */ + gst_bin_add (GST_BIN (pipeline), sink); + + /* now we can link again */ + fail_unless (gst_pad_link (srcpad, sinkpad) == GST_PAD_LINK_OK); + + /* check if pads really are linked */ + fail_unless (gst_pad_is_linked (srcpad)); + fail_unless (gst_pad_is_linked (sinkpad)); +} + +GST_END_TEST; + Suite * gst_bin_suite (void) { @@ -281,6 +331,7 @@ gst_bin_suite (void) tcase_add_test (tc_chain, test_message_state_changed); tcase_add_test (tc_chain, test_message_state_changed_child); tcase_add_test (tc_chain, test_message_state_changed_children); + tcase_add_test (tc_chain, test_add_linked); return s; } diff --git a/tests/check/gst/gstelement.c b/tests/check/gst/gstelement.c index 20e37ac0a3..27bd84e010 100644 --- a/tests/check/gst/gstelement.c +++ b/tests/check/gst/gstelement.c @@ -94,6 +94,43 @@ GST_START_TEST (test_error_no_bus) GST_END_TEST; +/* link and run two elements without putting them in a + * pipeline */ +GST_START_TEST (test_link) +{ + GstElement *src, *sink; + + src = gst_element_factory_make ("fakesrc", "source"); + sink = gst_element_factory_make ("fakesink", "sink"); + + fail_unless (gst_element_link_pads (src, "src", sink, "sink")); + + /* do sink to source state change */ + gst_element_set_state (sink, GST_STATE_PAUSED); + gst_element_set_state (src, GST_STATE_PAUSED); + + /* wait for preroll */ + gst_element_get_state (sink, NULL, NULL, NULL); + + /* play some more */ + gst_element_set_state (sink, GST_STATE_PLAYING); + gst_element_set_state (src, GST_STATE_PLAYING); + + g_usleep (G_USEC_PER_SEC); + + /* and stop */ + gst_element_set_state (sink, GST_STATE_PAUSED); + gst_element_set_state (src, GST_STATE_PAUSED); + + /* wait for preroll */ + gst_element_get_state (sink, NULL, NULL, NULL); + + gst_element_set_state (sink, GST_STATE_NULL); + gst_element_set_state (src, GST_STATE_NULL); +} + +GST_END_TEST; + Suite * gst_element_suite (void) { @@ -104,6 +141,7 @@ gst_element_suite (void) tcase_add_test (tc_chain, test_add_remove_pad); tcase_add_test (tc_chain, test_add_pad_unref_element); tcase_add_test (tc_chain, test_error_no_bus); + tcase_add_test (tc_chain, test_link); return s; } diff --git a/tests/check/pipelines/simple-launch-lines.c b/tests/check/pipelines/simple-launch-lines.c index e031482ab4..d29c88efb3 100644 --- a/tests/check/pipelines/simple-launch-lines.c +++ b/tests/check/pipelines/simple-launch-lines.c @@ -139,8 +139,8 @@ GST_START_TEST (test_stop_from_app) g_return_if_fail (fakesrc && fakesink && pipeline); - gst_element_link (fakesrc, fakesink); gst_bin_add_many (GST_BIN (pipeline), fakesrc, fakesink, NULL); + gst_element_link (fakesrc, fakesink); g_object_set (fakesink, "signal-handoffs", (gboolean) TRUE, NULL); g_signal_connect (fakesink, "handoff", G_CALLBACK (got_handoff), NULL);