diff --git a/ChangeLog b/ChangeLog index 224b5b19ee..e570aff2e9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,15 @@ +2004-11-04 Wim Taymans + + * 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 * gst/gstmarshal.list: add missing marshaller, fixes build diff --git a/gst/schedulers/gstoptimalscheduler.c b/gst/schedulers/gstoptimalscheduler.c index 398e47dd9e..66544c3b1b 100644 --- a/gst/schedulers/gstoptimalscheduler.c +++ b/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->type != GST_OPT_SCHEDULER_GROUP_LOOP) { - GST_LOG ("removing last element from old group"); - group = remove_from_group (group, GST_ELEMENT (group->elements->data)); - } + group = normalize_group (group); } } if (new_group != NULL) { - if (group_num_elements (new_group) == 1 && - new_group->type != GST_OPT_SCHEDULER_GROUP_LOOP) { - GST_LOG ("removing last element from new group"); - new_group = - remove_from_group (new_group, - GST_ELEMENT (new_group->elements->data)); + new_group = normalize_group (new_group); + if (new_group == NULL) 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 */ diff --git a/tests/old/testsuite/schedulers/.gitignore b/tests/old/testsuite/schedulers/.gitignore index 61ee5ac546..af15adb8b0 100644 --- a/tests/old/testsuite/schedulers/.gitignore +++ b/tests/old/testsuite/schedulers/.gitignore @@ -14,3 +14,4 @@ useless_iteration 147819 147894 147894-2 +queue_link diff --git a/tests/old/testsuite/schedulers/Makefile.am b/tests/old/testsuite/schedulers/Makefile.am index 31e60ab322..391ac40476 100644 --- a/tests/old/testsuite/schedulers/Makefile.am +++ b/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 diff --git a/tests/old/testsuite/schedulers/queue_link.c b/tests/old/testsuite/schedulers/queue_link.c new file mode 100644 index 0000000000..f14d7b53e8 --- /dev/null +++ b/tests/old/testsuite/schedulers/queue_link.c @@ -0,0 +1,68 @@ +/* GStreamer + * Copyright (C) 2004 Wim Taymans + * + * 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 + +#include + +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; +} diff --git a/testsuite/schedulers/.gitignore b/testsuite/schedulers/.gitignore index 61ee5ac546..af15adb8b0 100644 --- a/testsuite/schedulers/.gitignore +++ b/testsuite/schedulers/.gitignore @@ -14,3 +14,4 @@ useless_iteration 147819 147894 147894-2 +queue_link diff --git a/testsuite/schedulers/Makefile.am b/testsuite/schedulers/Makefile.am index 31e60ab322..391ac40476 100644 --- a/testsuite/schedulers/Makefile.am +++ b/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 diff --git a/testsuite/schedulers/queue_link.c b/testsuite/schedulers/queue_link.c new file mode 100644 index 0000000000..f14d7b53e8 --- /dev/null +++ b/testsuite/schedulers/queue_link.c @@ -0,0 +1,68 @@ +/* GStreamer + * Copyright (C) 2004 Wim Taymans + * + * 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 + +#include + +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; +}