From ebad8c0094597c98447d8afb4ba73a757621359f Mon Sep 17 00:00:00 2001
From: Edward Hervey <edward@centricular.com>
Date: Mon, 2 Oct 2017 17:59:17 +0200
Subject: [PATCH] bin: iterate_sorted: Ensure sources are always returned last

For linked elements, the resulting gst_bin_iterate_sorted() will
properly return elements from sink to sources.

If we have some elements that are not linked, we *still* want to
ensure that we return:
* In priority any sinks
* Last of all any sources
* And in between any element which is neither source nor sink

For this to work, when looking for the next candidate element,
not only check the degree order, but if there are two candidates
with the same degree order, prefer the non-source one.

Amongst other things, this fixes the case where we activating a
bin containing unlinked sources and other elements. Without this
we could end up activating sources (which might start adding pads
to be linked) before other (to which those new source element pads
might be linked) are not activated

https://bugzilla.gnome.org/show_bug.cgi?id=788434
---
 gst/gstbin.c             |  6 +++++
 tests/check/gst/gstbin.c | 49 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)

diff --git a/gst/gstbin.c b/gst/gstbin.c
index 26ce15595a..d6ec582f95 100644
--- a/gst/gstbin.c
+++ b/gst/gstbin.c
@@ -2320,6 +2320,12 @@ find_element (GstElement * element, GstBinSortIterator * bit)
   if (bit->best == NULL || bit->best_deg > degree) {
     bit->best = element;
     bit->best_deg = degree;
+  } else if (bit->best_deg == degree
+      && GST_OBJECT_FLAG_IS_SET (bit->best, GST_ELEMENT_FLAG_SOURCE)
+      && !GST_OBJECT_FLAG_IS_SET (element, GST_ELEMENT_FLAG_SOURCE)) {
+    /* If two elements have the same degree, we want to ensure we
+     * return non-source elements first. */
+    bit->best = element;
   }
 }
 
diff --git a/tests/check/gst/gstbin.c b/tests/check/gst/gstbin.c
index fe41ac0157..9ac461ea52 100644
--- a/tests/check/gst/gstbin.c
+++ b/tests/check/gst/gstbin.c
@@ -1098,6 +1098,54 @@ GST_START_TEST (test_iterate_sorted)
 
 GST_END_TEST;
 
+GST_START_TEST (test_iterate_sorted_unlinked)
+{
+  GstElement *pipeline, *src, *sink, *identity;
+  GstIterator *it;
+  GValue elem = { 0, };
+
+  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 (sink == NULL, "Could not create fakesink1");
+
+  identity = gst_element_factory_make ("identity", NULL);
+  fail_if (identity == NULL, "Could not create identity");
+
+  gst_bin_add_many (GST_BIN (pipeline), sink, identity, src, NULL);
+
+  /* If elements aren't linked, we should end up with:
+   * * sink elements always first
+   * * source elements always last
+   * * any other elements in between
+   */
+
+  it = gst_bin_iterate_sorted (GST_BIN (pipeline));
+  fail_unless (gst_iterator_next (it, &elem) == GST_ITERATOR_OK);
+  fail_unless (g_value_get_object (&elem) == (gpointer) sink);
+  g_value_reset (&elem);
+
+  fail_unless (gst_iterator_next (it, &elem) == GST_ITERATOR_OK);
+  fail_unless (g_value_get_object (&elem) == (gpointer) identity);
+  g_value_reset (&elem);
+
+  fail_unless (gst_iterator_next (it, &elem) == GST_ITERATOR_OK);
+  fail_unless (g_value_get_object (&elem) == (gpointer) src);
+  g_value_reset (&elem);
+
+  g_value_unset (&elem);
+  gst_iterator_free (it);
+
+  ASSERT_OBJECT_REFCOUNT (pipeline, "pipeline", 1);
+  gst_object_unref (pipeline);
+}
+
+GST_END_TEST;
+
 static void
 test_link_structure_change_state_changed_sync_cb (GstBus * bus,
     GstMessage * message, gpointer data)
@@ -1751,6 +1799,7 @@ gst_bin_suite (void)
   tcase_add_test (tc_chain, test_add_linked);
   tcase_add_test (tc_chain, test_add_self);
   tcase_add_test (tc_chain, test_iterate_sorted);
+  tcase_add_test (tc_chain, test_iterate_sorted_unlinked);
   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);