Added testcase for schduler segfault.

Original commit message from CVS:
* gst/schedulers/gstoptimalscheduler.c: (add_to_group),
(remove_from_group), (normalize_group), (group_migrate_connected),
(gst_opt_scheduler_iterate):
* testsuite/schedulers/.cvsignore:
* testsuite/schedulers/Makefile.am:
* testsuite/schedulers/queue_link.c: (main):
Added testcase for schduler segfault.
Fix scheduler segfault when removing a decoupled
entry point as the last element from a group.

ChangeLog

@@ -1,3 +1,15 @@
+2004-11-04  Wim Taymans  <wim@fluendo.com>
+
+	* gst/schedulers/gstoptimalscheduler.c: (add_to_group),
+	(remove_from_group), (normalize_group), (group_migrate_connected),
+	(gst_opt_scheduler_iterate):
+	* testsuite/schedulers/.cvsignore:
+	* testsuite/schedulers/Makefile.am:
+	* testsuite/schedulers/queue_link.c: (main):
+	Added testcase for schduler segfault.
+	Fix scheduler segfault when removing a decoupled
+	entry point as the last element from a group.
+
 2004-11-03  Christophe Fergeau  <teuf@gnome.org>
 
 	* gst/gstmarshal.list: add missing marshaller, fixes build

gst/schedulers/gstoptimalscheduler.c

@@ -904,6 +904,7 @@ remove_from_group (GstOptSchedulerGroup * group, GstElement * element)
 
   g_assert (group != NULL);
   g_assert (element != NULL);
+  /* this assert also catches the decoupled elements */
   g_assert (GST_ELEMENT_SCHED_GROUP (element) == group);
 
   /* first decrement the links that this group has with other groups through
@@ -942,25 +943,6 @@ remove_from_group (GstOptSchedulerGroup * group, GstElement * element)
   return group;
 }
 
-/* count number of elements in the group. Have to be careful because
- * decoupled elements are added as entry point but are not added to
- * the elements list */
-static gint
-group_num_elements (GstOptSchedulerGroup * group)
-{
-  gint num;
-
-  num = group->num_elements;
-  /* decoupled elements are not added to the group but are
-   * added as an entry */
-  if (group->entry) {
-    if (GST_ELEMENT_IS_DECOUPLED (group->entry)) {
-      num++;
-    }
-  }
-  return num;
-}
-
 /* check if an element is part of the given group. We have to be carefull
  * as decoupled elements are added as entry but are not added to the elements 
  * list */
@@ -2371,6 +2353,42 @@ rechain_group (GstOptSchedulerGroup * group)
   }
 }
 
+/* make sure that the group does not contain only single element.
+ * Only loop-based groups can contain a single element. */
+static GstOptSchedulerGroup *
+normalize_group (GstOptSchedulerGroup * group)
+{
+  gint num;
+  gboolean have_decoupled = FALSE;
+
+  if (group == NULL)
+    return NULL;
+
+  num = group->num_elements;
+  /* decoupled elements are not added to the group but are
+   * added as an entry */
+  if (group->entry && GST_ELEMENT_IS_DECOUPLED (group->entry)) {
+    num++;
+    have_decoupled = TRUE;
+  }
+
+  if (num == 1 && group->type != GST_OPT_SCHEDULER_GROUP_LOOP) {
+    GST_LOG ("removing last element from group %p", group);
+    if (have_decoupled) {
+      group->entry = NULL;
+      if (group->chain) {
+        GST_LOG ("removing group %p from its chain", group);
+        chain_group_set_enabled (group->chain, group, FALSE);
+        remove_from_chain (group->chain, group);
+      }
+      group = unref_group (group);
+    } else {
+      group = remove_from_group (group, GST_ELEMENT (group->elements->data));
+    }
+  }
+  return group;
+}
+
 /* migrate the element and all connected elements to a new group without looking at
  * the brokenpad */
 static GstOptSchedulerGroup *
@@ -2432,23 +2450,14 @@ group_migrate_connected (GstOptScheduler * osched, GstElement * element,
     /* remove last element from the group if any. Make sure not to remove
      * the loop based entry point of a group as this always needs one group */
     if (group != NULL) {
-      if (group_num_elements (group) == 1 &&
+      group = normalize_group (group);
-          group->type != GST_OPT_SCHEDULER_GROUP_LOOP) {
-        GST_LOG ("removing last element from old group");
-        group = remove_from_group (group, GST_ELEMENT (group->elements->data));
-      }
     }
   }
 
   if (new_group != NULL) {
-    if (group_num_elements (new_group) == 1 &&
+    new_group = normalize_group (new_group);
-        new_group->type != GST_OPT_SCHEDULER_GROUP_LOOP) {
+    if (new_group == NULL)
-      GST_LOG ("removing last element from new group");
-      new_group =
-          remove_from_group (new_group,
-          GST_ELEMENT (new_group->elements->data));
       return NULL;
-    }
     /* at this point the new group lives in its own chain but might
      * have to be merged with another chain, this happens when the new
      * group has a link with another group in another chain */

tests/old/testsuite/schedulers/.gitignore

@@ -14,3 +14,4 @@ useless_iteration
 147819
 147894
 147894-2
+queue_link

tests/old/testsuite/schedulers/Makefile.am

@@ -8,7 +8,8 @@ tests_pass = \
 	143777 143777-2 \
         147713 \
         147819 \
-        147894 147894-2 group_link
+        147894 147894-2 group_link \
+	queue_link
 
 # don't enable this one unless it actually works.
 #	useless_iteration

tests/old/testsuite/schedulers/queue_link.c

@@ -0,0 +1,68 @@
+/* GStreamer
+ * Copyright (C) 2004 Wim Taymans <wim@fluendo.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this library; if not, write to the Free
+ * Software Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <unistd.h>
+
+#include <gst/gst.h>
+
+gint
+main (gint argc, gchar ** argv)
+{
+  GstElement *pipeline, *thread, *bin, *src, *queue, *id1, *sink;
+
+  gst_init (&argc, &argv);
+
+  g_print ("setting up...\n");
+  /* setup pipeline */
+  pipeline = gst_element_factory_make ("pipeline", NULL);
+  g_assert (pipeline);
+  src = gst_element_factory_make ("fakesrc", NULL);
+  g_assert (src);
+  queue = gst_element_factory_make ("queue", NULL);
+  g_assert (queue);
+
+  thread = gst_element_factory_make ("thread", NULL);
+  g_assert (thread);
+  bin = gst_element_factory_make ("bin", NULL);
+  g_assert (bin);
+  id1 = gst_element_factory_make ("identity", NULL);
+  g_assert (id1);
+  sink = gst_element_factory_make ("fakesink", NULL);
+  g_assert (sink);
+
+  gst_bin_add_many (GST_BIN (bin), id1, sink, NULL);
+  gst_bin_add_many (GST_BIN (thread), bin, NULL);
+  gst_bin_add_many (GST_BIN (pipeline), src, queue, thread, NULL);
+
+  gst_element_link_pads (src, "src", queue, "sink");
+  gst_element_link_pads (queue, "src", id1, "sink");
+  gst_element_link_pads (id1, "src", sink, "sink");
+
+  if (gst_element_set_state (pipeline, GST_STATE_PLAYING) != GST_STATE_SUCCESS)
+    g_assert_not_reached ();
+
+  g_print ("unlinking...\n");
+
+  gst_object_ref (GST_OBJECT (queue));
+  gst_bin_remove (GST_BIN (pipeline), queue);
+  gst_object_ref (GST_OBJECT (bin));
+  gst_bin_remove (GST_BIN (thread), bin);
+
+  g_print ("done.\n");
+  return 0;
+}

testsuite/schedulers/.gitignore

@@ -14,3 +14,4 @@ useless_iteration
 147819
 147894
 147894-2
+queue_link

testsuite/schedulers/Makefile.am

@@ -8,7 +8,8 @@ tests_pass = \
 	143777 143777-2 \
         147713 \
         147819 \
-        147894 147894-2 group_link
+        147894 147894-2 group_link \
+	queue_link
 
 # don't enable this one unless it actually works.
 #	useless_iteration

testsuite/schedulers/queue_link.c

@@ -0,0 +1,68 @@
+/* GStreamer
+ * Copyright (C) 2004 Wim Taymans <wim@fluendo.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this library; if not, write to the Free
+ * Software Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <unistd.h>
+
+#include <gst/gst.h>
+
+gint
+main (gint argc, gchar ** argv)
+{
+  GstElement *pipeline, *thread, *bin, *src, *queue, *id1, *sink;
+
+  gst_init (&argc, &argv);
+
+  g_print ("setting up...\n");
+  /* setup pipeline */
+  pipeline = gst_element_factory_make ("pipeline", NULL);
+  g_assert (pipeline);
+  src = gst_element_factory_make ("fakesrc", NULL);
+  g_assert (src);
+  queue = gst_element_factory_make ("queue", NULL);
+  g_assert (queue);
+
+  thread = gst_element_factory_make ("thread", NULL);
+  g_assert (thread);
+  bin = gst_element_factory_make ("bin", NULL);
+  g_assert (bin);
+  id1 = gst_element_factory_make ("identity", NULL);
+  g_assert (id1);
+  sink = gst_element_factory_make ("fakesink", NULL);
+  g_assert (sink);
+
+  gst_bin_add_many (GST_BIN (bin), id1, sink, NULL);
+  gst_bin_add_many (GST_BIN (thread), bin, NULL);
+  gst_bin_add_many (GST_BIN (pipeline), src, queue, thread, NULL);
+
+  gst_element_link_pads (src, "src", queue, "sink");
+  gst_element_link_pads (queue, "src", id1, "sink");
+  gst_element_link_pads (id1, "src", sink, "sink");
+
+  if (gst_element_set_state (pipeline, GST_STATE_PLAYING) != GST_STATE_SUCCESS)
+    g_assert_not_reached ();
+
+  g_print ("unlinking...\n");
+
+  gst_object_ref (GST_OBJECT (queue));
+  gst_bin_remove (GST_BIN (pipeline), queue);
+  gst_object_ref (GST_OBJECT (bin));
+  gst_bin_remove (GST_BIN (thread), bin);
+
+  g_print ("done.\n");
+  return 0;
+}