diff --git a/ChangeLog b/ChangeLog index 4d85ec67d5..9b0d740f14 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,23 @@ +2005-10-12 Wim Taymans + + * check/Makefile.am: + * check/states/sinks.c: (GST_START_TEST), (gst_object_suite): + * check/states/sinks2.c: + Moved sinks2 testcode in sinks check. + + * gst/gstbin.c: (gst_bin_provide_clock_func), (gst_bin_add_func), + (gst_bin_remove_func), (gst_bin_recalc_state), + (gst_bin_change_state_func), (bin_bus_handler): + Fix potential race condition when _get_state() iterated over an + ASYNC element right before it posted a state completion. + + * gst/gstclock.h: + Do proper cast here. + + * gst/gstevent.c: (gst_event_new_newsegment), + (gst_event_parse_newsegment): + A playback rate of 0.0 is not allowed. + 2005-10-11 Thomas Vander Stichele * win32/common/config.h: diff --git a/check/Makefile.am b/check/Makefile.am index 242bb27e57..53cedc786f 100644 --- a/check/Makefile.am +++ b/check/Makefile.am @@ -54,7 +54,6 @@ check_PROGRAMS = \ pipelines/simple_launch_lines \ pipelines/cleanup \ states/sinks \ - states/sinks2 \ gst-libs/controller \ gst-libs/gdp diff --git a/check/states/sinks.c b/check/states/sinks.c index e63abfcc42..1add001a1b 100644 --- a/check/states/sinks.c +++ b/check/states/sinks.c @@ -57,6 +57,50 @@ GST_START_TEST (test_sink) gst_object_unref (sink); } +GST_END_TEST +/* a sink should go ASYNC to PAUSE and PLAYING, when linking a src, it + * should complete the state change. */ +GST_START_TEST (test_sink_completion) +{ + GstElement *sink, *src; + GstStateChangeReturn ret; + GstState current, pending; + GTimeVal tv; + + sink = gst_element_factory_make ("fakesink", "sink"); + + ret = gst_element_set_state (sink, GST_STATE_PLAYING); + fail_unless (ret == GST_STATE_CHANGE_ASYNC, "no async state return"); + + GST_TIME_TO_TIMEVAL ((GstClockTime) 0, tv); + + ret = gst_element_get_state (sink, ¤t, &pending, &tv); + fail_unless (ret == GST_STATE_CHANGE_ASYNC, "not changing state async"); + fail_unless (current == GST_STATE_READY, "bad current state"); + fail_unless (pending == GST_STATE_PLAYING, "bad pending state"); + + src = gst_element_factory_make ("fakesrc", "src"); + gst_element_link (src, sink); + + ret = gst_element_set_state (src, GST_STATE_PLAYING); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "no success state return"); + + /* now wait for final state */ + ret = gst_element_get_state (sink, ¤t, &pending, NULL); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "failed to change state"); + fail_unless (current == GST_STATE_PLAYING, "bad current state"); + fail_unless (pending == GST_STATE_VOID_PENDING, "bad pending state"); + + ret = gst_element_set_state (sink, GST_STATE_NULL); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "failed to go to null"); + + ret = gst_element_set_state (src, GST_STATE_NULL); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "failed to go to null"); + + gst_object_unref (sink); + gst_object_unref (src); +} + GST_END_TEST /* a sink should go ASYNC to PAUSE. PAUSE should complete when * prerolled. */ @@ -152,6 +196,7 @@ GST_START_TEST (test_livesrc_sink) GstStateChangeReturn ret; GstState current, pending; GstPad *srcpad, *sinkpad; + GTimeVal tv; pipeline = gst_pipeline_new ("pipeline"); src = gst_element_factory_make ("fakesrc", "src"); @@ -176,16 +221,20 @@ GST_START_TEST (test_livesrc_sink) fail_unless (current == GST_STATE_PAUSED, "not paused"); fail_unless (pending == GST_STATE_VOID_PENDING, "not playing"); + /* don't block here */ + GST_TIME_TO_TIMEVAL (0, tv); + ret = gst_element_get_state (sink, ¤t, &pending, &tv); + fail_unless (ret == GST_STATE_CHANGE_ASYNC, "not async"); + fail_unless (current == GST_STATE_READY, "not ready"); + fail_unless (pending == GST_STATE_PAUSED, "not paused"); + ret = gst_element_get_state (pipeline, ¤t, &pending, NULL); fail_unless (ret == GST_STATE_CHANGE_NO_PREROLL, "not paused"); fail_unless (current == GST_STATE_PAUSED, "not paused"); fail_unless (pending == GST_STATE_VOID_PENDING, "not playing"); ret = gst_element_set_state (pipeline, GST_STATE_PLAYING); - ret = gst_element_get_state (pipeline, NULL, NULL, NULL); - fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "cannot force play got %d", - ret); - + fail_unless (ret == GST_STATE_CHANGE_ASYNC, "not async"); ret = gst_element_get_state (pipeline, ¤t, &pending, NULL); fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "not playing"); fail_unless (current == GST_STATE_PLAYING, "not playing"); @@ -204,6 +253,7 @@ gst_object_suite (void) suite_add_tcase (s, tc_chain); tcase_add_test (tc_chain, test_sink); + tcase_add_test (tc_chain, test_sink_completion); tcase_add_test (tc_chain, test_src_sink); tcase_add_test (tc_chain, test_livesrc_remove); tcase_add_test (tc_chain, test_livesrc_sink); diff --git a/check/states/sinks2.c b/check/states/sinks2.c deleted file mode 100644 index 433a222fcb..0000000000 --- a/check/states/sinks2.c +++ /dev/null @@ -1,95 +0,0 @@ -/* GStreamer - * - * unit test for sinks - * - * Copyright (C) <2005> Wim Taymans - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Library 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 - * Library General Public License for more details. - * - * You should have received a copy of the GNU Library General Public - * License along with this library; if not, write to the - * Free Software Foundation, Inc., 59 Temple Place - Suite 330, - * Boston, MA 02111-1307, USA. - */ - -#include - -/* a sink should go ASYNC to PAUSE and PLAYING. */ -GST_START_TEST (test_sink) -{ - GstElement *sink, *src; - GstStateChangeReturn ret; - GstState current, pending; - GTimeVal tv; - - sink = gst_element_factory_make ("fakesink", "sink"); - - ret = gst_element_set_state (sink, GST_STATE_PLAYING); - fail_unless (ret == GST_STATE_CHANGE_ASYNC, "no async state return"); - - GST_TIME_TO_TIMEVAL ((GstClockTime) 0, tv); - - ret = gst_element_get_state (sink, ¤t, &pending, &tv); - fail_unless (ret == GST_STATE_CHANGE_ASYNC, "not changing state async"); - fail_unless (current == GST_STATE_READY, "bad current state"); - fail_unless (pending == GST_STATE_PLAYING, "bad pending state"); - - src = gst_element_factory_make ("fakesrc", "src"); - gst_element_link (src, sink); - - ret = gst_element_set_state (src, GST_STATE_PLAYING); - fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "no success state return"); - - /* now wait for final state */ - ret = gst_element_get_state (sink, ¤t, &pending, NULL); - fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "failed to change state"); - fail_unless (current == GST_STATE_PLAYING, "bad current state"); - fail_unless (pending == GST_STATE_VOID_PENDING, "bad pending state"); - - ret = gst_element_set_state (sink, GST_STATE_NULL); - fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "failed to go to null"); - - ret = gst_element_set_state (src, GST_STATE_NULL); - fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "failed to go to null"); - - gst_object_unref (sink); - gst_object_unref (src); -} - -GST_END_TEST -/* test: try changing state of sinks */ - Suite * gst_object_suite (void) -{ - Suite *s = suite_create ("Sinks"); - TCase *tc_chain = tcase_create ("general"); - - suite_add_tcase (s, tc_chain); - tcase_add_test (tc_chain, test_sink); - - return s; -} - -int -main (int argc, char **argv) -{ - int nf; - - Suite *s = gst_object_suite (); - SRunner *sr = srunner_create (s); - - gst_check_init (&argc, &argv); - - srunner_run_all (sr, CK_NORMAL); - nf = srunner_ntests_failed (sr); - srunner_free (sr); - - return nf; -} diff --git a/common b/common index 615cf4d450..37ed26e33b 160000 --- a/common +++ b/common @@ -1 +1 @@ -Subproject commit 615cf4d4506ef1ffb1f600c434fced1fa26b0f44 +Subproject commit 37ed26e33bae9a6ab256c62ebbb9d711374a0abb diff --git a/gst/gstbin.c b/gst/gstbin.c index fd2e0cfa47..0366713365 100644 --- a/gst/gstbin.c +++ b/gst/gstbin.c @@ -934,8 +934,12 @@ gst_bin_recalc_state (GstBin * bin, gboolean force) /* lock bin, no element can be added or removed while we have this lock */ GST_LOCK (bin); + /* forced recalc, make state dirty again */ + if (force) + bin->state_dirty = TRUE; + /* no point in scanning if nothing changed and it's no forced recalc */ - if (!force && !bin->state_dirty) + if (!bin->state_dirty) goto not_dirty; /* no point in having two scans run concurrently */ @@ -947,6 +951,10 @@ gst_bin_recalc_state (GstBin * bin, gboolean force) GST_CAT_INFO_OBJECT (GST_CAT_STATES, bin, "recalc state"); restart: + /* when we leave this function, the state must not be dirty, whenever + * we are scanning and the state bemoces dirty again, we restart. */ + bin->state_dirty = FALSE; + have_no_preroll = FALSE; have_async = FALSE; @@ -975,6 +983,11 @@ restart: /* child added/removed during state change, restart. We need * to restart with the quick check as a no-preroll element could * have been added here and we don't want to block on sinks then.*/ + GST_DEBUG_OBJECT (bin, "children added or removed, restarting recalc"); + goto restart; + } + if (bin->state_dirty) { + GST_DEBUG_OBJECT (bin, "state dirty again, restarting recalc"); goto restart; } @@ -1009,7 +1022,6 @@ restart: } done: - bin->state_dirty = FALSE; bin->polling = FALSE; GST_UNLOCK (bin); diff --git a/gst/gstclock.h b/gst/gstclock.h index 4d8fb08409..dd6438dcea 100644 --- a/gst/gstclock.h +++ b/gst/gstclock.h @@ -123,8 +123,8 @@ typedef gpointer GstClockID; */ #define GST_TIME_TO_TIMEVAL(t,tv) \ G_STMT_START { \ - (tv).tv_sec = (t) / GST_SECOND; \ - (tv).tv_usec = ((t) - (tv).tv_sec * GST_SECOND) / GST_USECOND; \ + (tv).tv_sec = ((GstClockTime)(t)) / GST_SECOND; \ + (tv).tv_usec = (((GstClockTime)(t)) - (tv).tv_sec * GST_SECOND) / GST_USECOND; \ } G_STMT_END /** diff --git a/gst/gstevent.c b/gst/gstevent.c index c5d23f7a7b..6685e3b13a 100644 --- a/gst/gstevent.c +++ b/gst/gstevent.c @@ -384,6 +384,8 @@ GstEvent * gst_event_new_newsegment (gboolean update, gdouble rate, GstFormat format, gint64 start_value, gint64 stop_value, gint64 stream_time) { + g_return_val_if_fail (rate != 0.0, NULL); + if (format == GST_FORMAT_TIME) { GST_CAT_INFO (GST_CAT_EVENT, "creating newsegment update %d, rate %lf, format GST_FORMAT_TIME, " diff --git a/tests/check/Makefile.am b/tests/check/Makefile.am index 242bb27e57..53cedc786f 100644 --- a/tests/check/Makefile.am +++ b/tests/check/Makefile.am @@ -54,7 +54,6 @@ check_PROGRAMS = \ pipelines/simple_launch_lines \ pipelines/cleanup \ states/sinks \ - states/sinks2 \ gst-libs/controller \ gst-libs/gdp diff --git a/tests/check/generic/sinks.c b/tests/check/generic/sinks.c index e63abfcc42..1add001a1b 100644 --- a/tests/check/generic/sinks.c +++ b/tests/check/generic/sinks.c @@ -57,6 +57,50 @@ GST_START_TEST (test_sink) gst_object_unref (sink); } +GST_END_TEST +/* a sink should go ASYNC to PAUSE and PLAYING, when linking a src, it + * should complete the state change. */ +GST_START_TEST (test_sink_completion) +{ + GstElement *sink, *src; + GstStateChangeReturn ret; + GstState current, pending; + GTimeVal tv; + + sink = gst_element_factory_make ("fakesink", "sink"); + + ret = gst_element_set_state (sink, GST_STATE_PLAYING); + fail_unless (ret == GST_STATE_CHANGE_ASYNC, "no async state return"); + + GST_TIME_TO_TIMEVAL ((GstClockTime) 0, tv); + + ret = gst_element_get_state (sink, ¤t, &pending, &tv); + fail_unless (ret == GST_STATE_CHANGE_ASYNC, "not changing state async"); + fail_unless (current == GST_STATE_READY, "bad current state"); + fail_unless (pending == GST_STATE_PLAYING, "bad pending state"); + + src = gst_element_factory_make ("fakesrc", "src"); + gst_element_link (src, sink); + + ret = gst_element_set_state (src, GST_STATE_PLAYING); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "no success state return"); + + /* now wait for final state */ + ret = gst_element_get_state (sink, ¤t, &pending, NULL); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "failed to change state"); + fail_unless (current == GST_STATE_PLAYING, "bad current state"); + fail_unless (pending == GST_STATE_VOID_PENDING, "bad pending state"); + + ret = gst_element_set_state (sink, GST_STATE_NULL); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "failed to go to null"); + + ret = gst_element_set_state (src, GST_STATE_NULL); + fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "failed to go to null"); + + gst_object_unref (sink); + gst_object_unref (src); +} + GST_END_TEST /* a sink should go ASYNC to PAUSE. PAUSE should complete when * prerolled. */ @@ -152,6 +196,7 @@ GST_START_TEST (test_livesrc_sink) GstStateChangeReturn ret; GstState current, pending; GstPad *srcpad, *sinkpad; + GTimeVal tv; pipeline = gst_pipeline_new ("pipeline"); src = gst_element_factory_make ("fakesrc", "src"); @@ -176,16 +221,20 @@ GST_START_TEST (test_livesrc_sink) fail_unless (current == GST_STATE_PAUSED, "not paused"); fail_unless (pending == GST_STATE_VOID_PENDING, "not playing"); + /* don't block here */ + GST_TIME_TO_TIMEVAL (0, tv); + ret = gst_element_get_state (sink, ¤t, &pending, &tv); + fail_unless (ret == GST_STATE_CHANGE_ASYNC, "not async"); + fail_unless (current == GST_STATE_READY, "not ready"); + fail_unless (pending == GST_STATE_PAUSED, "not paused"); + ret = gst_element_get_state (pipeline, ¤t, &pending, NULL); fail_unless (ret == GST_STATE_CHANGE_NO_PREROLL, "not paused"); fail_unless (current == GST_STATE_PAUSED, "not paused"); fail_unless (pending == GST_STATE_VOID_PENDING, "not playing"); ret = gst_element_set_state (pipeline, GST_STATE_PLAYING); - ret = gst_element_get_state (pipeline, NULL, NULL, NULL); - fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "cannot force play got %d", - ret); - + fail_unless (ret == GST_STATE_CHANGE_ASYNC, "not async"); ret = gst_element_get_state (pipeline, ¤t, &pending, NULL); fail_unless (ret == GST_STATE_CHANGE_SUCCESS, "not playing"); fail_unless (current == GST_STATE_PLAYING, "not playing"); @@ -204,6 +253,7 @@ gst_object_suite (void) suite_add_tcase (s, tc_chain); tcase_add_test (tc_chain, test_sink); + tcase_add_test (tc_chain, test_sink_completion); tcase_add_test (tc_chain, test_src_sink); tcase_add_test (tc_chain, test_livesrc_remove); tcase_add_test (tc_chain, test_livesrc_sink);