From 5059e9f8bdf7b1e3c899de8c627f11d9d14276a9 Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Thu, 16 Aug 2007 10:27:16 +0000 Subject: [PATCH] gst/gstbin.c: Fix annoying bug in the sorted iterator where a sink that is not really a sink (when it has downstream ... Original commit message from CVS: * gst/gstbin.c: (add_to_queue), (remove_from_queue), (clear_queue), (update_degree), (gst_bin_sort_iterator_next): Fix annoying bug in the sorted iterator where a sink that is not really a sink (when it has downstream links) screwed up the iterator. * tests/check/gst/gstbin.c: (GST_START_TEST), (gst_bin_suite): Unit test to verify the fix. --- ChangeLog | 10 ++++++++ gst/gstbin.c | 29 ++++++++++++++++++++--- tests/check/gst/gstbin.c | 51 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index ec9572ed68..038b0c5012 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2007-08-16 Wim Taymans + + * gst/gstbin.c: (add_to_queue), (remove_from_queue), (clear_queue), + (update_degree), (gst_bin_sort_iterator_next): + Fix annoying bug in the sorted iterator where a sink that is not really + a sink (when it has downstream links) screwed up the iterator. + + * tests/check/gst/gstbin.c: (GST_START_TEST), (gst_bin_suite): + Unit test to verify the fix. + 2007-08-16 Wim Taymans * gst/gstmessage.h: diff --git a/gst/gstbin.c b/gst/gstbin.c index 547e7e9e51..6e736809bd 100644 --- a/gst/gstbin.c +++ b/gst/gstbin.c @@ -1566,12 +1566,22 @@ typedef struct _GstBinSortIterator static void add_to_queue (GstBinSortIterator * bit, GstElement * element) { - GST_DEBUG_OBJECT (bit->bin, "%s add to queue", GST_ELEMENT_NAME (element)); + GST_DEBUG_OBJECT (bit->bin, "adding '%s' to queue", + GST_ELEMENT_NAME (element)); gst_object_ref (element); g_queue_push_tail (bit->queue, element); HASH_SET_DEGREE (bit, element, -1); } +static void +remove_from_queue (GstBinSortIterator * bit, GstElement * element) +{ + GST_DEBUG_OBJECT (bit->bin, "removing '%s' from queue", + GST_ELEMENT_NAME (element)); + g_queue_remove (bit->queue, element); + gst_object_unref (element); +} + /* clear the queue, unref all objects as we took a ref when * we added them to the queue */ static void @@ -1581,10 +1591,14 @@ clear_queue (GQueue * queue) while ((p = g_queue_pop_head (queue))) gst_object_unref (p); + } /* set all degrees to 0. Elements marked as a sink are - * added to the queue immediatly. */ + * added to the queue immediatly. Since we only look at the SINK flag of the + * element, it is possible that we add non-sinks to the queue. These will be + * removed from the queue again when we can prove that it provides data for some + * other element. */ static void reset_degree (GstElement * element, GstBinSortIterator * bit) { @@ -1636,6 +1650,13 @@ update_degree (GstElement * element, GstBinSortIterator * bit) gint old_deg, new_deg; old_deg = HASH_GET_DEGREE (bit, peer_element); + + /* check to see if we added an element as sink that was not really a + * sink because it was connected to some other element. */ + if (old_deg == -1) { + remove_from_queue (bit, peer_element); + old_deg = 0; + } new_deg = old_deg + bit->mode; GST_DEBUG_OBJECT (bit->bin, @@ -1701,7 +1722,9 @@ gst_bin_sort_iterator_next (GstBinSortIterator * bit, gpointer * result) if ((best = bit->best)) { if (bit->best_deg != 0) { /* we don't fail on this one yet */ - g_warning ("loop detected in the graph !!"); + GST_WARNING_OBJECT (bin, "loop dected in graph"); + g_warning ("loop detected in the graph of bin %s!!", + GST_ELEMENT_NAME (bin)); } /* best unhandled element, schedule as next element */ GST_DEBUG_OBJECT (bin, "queue empty, next best: %s", diff --git a/tests/check/gst/gstbin.c b/tests/check/gst/gstbin.c index fd5af7286e..3bf39dcbea 100644 --- a/tests/check/gst/gstbin.c +++ b/tests/check/gst/gstbin.c @@ -834,6 +834,56 @@ GST_START_TEST (test_children_state_change_order_two_sink) GST_END_TEST; +GST_START_TEST (test_iterate_sorted) +{ + GstElement *src, *tee, *identity, *sink1, *sink2, *pipeline, *bin; + GstIterator *it; + gpointer elem; + + pipeline = gst_pipeline_new (NULL); + fail_unless (pipeline != NULL, "Could not create pipeline"); + + bin = gst_bin_new (NULL); + fail_unless (bin != NULL, "Could not create bin"); + + src = gst_element_factory_make ("fakesrc", NULL); + fail_if (src == NULL, "Could not create fakesrc"); + + tee = gst_element_factory_make ("tee", NULL); + fail_if (tee == NULL, "Could not create tee"); + + sink1 = gst_element_factory_make ("fakesink", NULL); + fail_if (sink1 == NULL, "Could not create fakesink1"); + + gst_bin_add_many (GST_BIN (bin), src, tee, sink1, NULL); + + fail_unless (gst_element_link (src, tee) == TRUE); + fail_unless (gst_element_link (tee, sink1) == TRUE); + + identity = gst_element_factory_make ("identity", NULL); + fail_if (identity == NULL, "Could not create identity"); + + sink2 = gst_element_factory_make ("fakesink", NULL); + fail_if (sink2 == NULL, "Could not create fakesink2"); + + gst_bin_add_many (GST_BIN (pipeline), bin, identity, sink2, NULL); + + fail_unless (gst_element_link (tee, identity) == TRUE); + fail_unless (gst_element_link (identity, sink2) == TRUE); + + it = gst_bin_iterate_sorted (GST_BIN (pipeline)); + fail_unless (gst_iterator_next (it, &elem) == GST_ITERATOR_OK); + fail_unless (elem == sink2); + fail_unless (gst_iterator_next (it, &elem) == GST_ITERATOR_OK); + fail_unless (elem == identity); + fail_unless (gst_iterator_next (it, &elem) == GST_ITERATOR_OK); + fail_unless (elem == bin); + + gst_object_unref (pipeline); +} + +GST_END_TEST; + Suite * gst_bin_suite (void) { @@ -853,6 +903,7 @@ gst_bin_suite (void) tcase_add_test (tc_chain, test_watch_for_state_change); tcase_add_test (tc_chain, test_add_linked); tcase_add_test (tc_chain, test_add_self); + tcase_add_test (tc_chain, test_iterate_sorted); return s; }