From a258e873426aff3cf1381cca197cbab284666db4 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Ren=C3=A9=20Stadler?= Date: Thu, 7 Dec 2006 10:51:36 +0000 Subject: [PATCH] gst/gstclock.c: Make period ids add the interval to the origial requested time instead of the possibly updated time w... MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Original commit message from CVS: Patch by: René Stadler * gst/gstclock.c: (gst_clock_id_wait): Make period ids add the interval to the origial requested time instead of the possibly updated time which can be wrong when there are multiple waiters for the same id. Fixes #382592. * gst/gstsystemclock.c: (gst_system_clock_async_thread), (gst_system_clock_id_wait_jitter_unlocked), (gst_system_clock_id_wait_jitter): Fix restart in the async notify thread when an async entry is added to the front of the list. Fixes #381492. * tests/check/gst/gstsystemclock.c: (store_callback), (notify_callback), (GST_START_TEST), (gst_systemclock_suite): Added test for multiple async waits. Added test for async wait order. --- ChangeLog | 20 +++++++++ gst/gstclock.c | 5 +-- gst/gstsystemclock.c | 17 ++++--- tests/check/gst/gstsystemclock.c | 97 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 131 insertions(+), 8 deletions(-) diff --git a/ChangeLog b/ChangeLog index a774d90..4e783b6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,25 @@ 2006-12-07 Wim Taymans + Patch by: René Stadler + + * gst/gstclock.c: (gst_clock_id_wait): + Make period ids add the interval to the origial requested time instead + of the possibly updated time which can be wrong when there are multiple + waiters for the same id. Fixes #382592. + + * gst/gstsystemclock.c: (gst_system_clock_async_thread), + (gst_system_clock_id_wait_jitter_unlocked), + (gst_system_clock_id_wait_jitter): + Fix restart in the async notify thread when an async entry is added to + the front of the list. Fixes #381492. + + * tests/check/gst/gstsystemclock.c: (store_callback), + (notify_callback), (GST_START_TEST), (gst_systemclock_suite): + Added test for multiple async waits. + Added test for async wait order. + +2006-12-07 Wim Taymans + * gst/gstbin.c: (gst_bin_query): Add some more docs about the POSITION query. diff --git a/gst/gstclock.c b/gst/gstclock.c index a292477..4ef7b9d 100644 --- a/gst/gstclock.c +++ b/gst/gstclock.c @@ -412,9 +412,8 @@ gst_clock_id_wait (GstClockID id, GstClockTimeDiff * jitter) GST_CAT_DEBUG_OBJECT (GST_CAT_CLOCK, clock, "done waiting entry %p, res: %d", id, res); - if (entry->type == GST_CLOCK_ENTRY_PERIODIC) { - entry->time += entry->interval; - } + if (entry->type == GST_CLOCK_ENTRY_PERIODIC) + entry->time = requested + entry->interval; if (clock->stats) gst_clock_update_stats (clock); diff --git a/gst/gstsystemclock.c b/gst/gstsystemclock.c index 6f4cd90..57d8e89 100644 --- a/gst/gstsystemclock.c +++ b/gst/gstsystemclock.c @@ -58,7 +58,8 @@ static guint64 gst_system_clock_get_resolution (GstClock * clock); static GstClockReturn gst_system_clock_id_wait_jitter (GstClock * clock, GstClockEntry * entry, GstClockTimeDiff * jitter); static GstClockReturn gst_system_clock_id_wait_jitter_unlocked - (GstClock * clock, GstClockEntry * entry, GstClockTimeDiff * jitter); + (GstClock * clock, GstClockEntry * entry, GstClockTimeDiff * jitter, + gboolean restart); static GstClockReturn gst_system_clock_id_wait_async (GstClock * clock, GstClockEntry * entry); static void gst_system_clock_id_unschedule (GstClock * clock, @@ -249,6 +250,7 @@ gst_system_clock_async_thread (GstClock * clock) /* now enter our (almost) infinite loop */ while (!sysclock->stopping) { GstClockEntry *entry; + GstClockTime requested; GstClockReturn res; /* check if something to be done */ @@ -270,10 +272,12 @@ gst_system_clock_async_thread (GstClock * clock) goto next_entry; } + requested = entry->time; + /* now wait for the entry, we already hold the lock */ res = gst_system_clock_id_wait_jitter_unlocked (clock, (GstClockID) entry, - NULL); + NULL, FALSE); switch (res) { case GST_CLOCK_UNSCHEDULED: @@ -295,7 +299,7 @@ gst_system_clock_async_thread (GstClock * clock) } if (entry->type == GST_CLOCK_ENTRY_PERIODIC) { /* adjust time now */ - entry->time += entry->interval; + entry->time = requested + entry->interval; /* and resort the list now */ clock->entries = g_list_sort (clock->entries, gst_clock_id_compare_func); @@ -366,7 +370,7 @@ gst_system_clock_get_resolution (GstClock * clock) */ static GstClockReturn gst_system_clock_id_wait_jitter_unlocked (GstClock * clock, - GstClockEntry * entry, GstClockTimeDiff * jitter) + GstClockEntry * entry, GstClockTimeDiff * jitter, gboolean restart) { GstClockTime entryt, real, now, target; GstClockTimeDiff diff; @@ -429,6 +433,9 @@ gst_system_clock_id_wait_jitter_unlocked (GstClock * clock, * continue our while loop. */ if (entry->status == GST_CLOCK_UNSCHEDULED) break; + /* else restart if we must */ + if (!restart) + break; } } } else if (diff == 0) { @@ -446,7 +453,7 @@ gst_system_clock_id_wait_jitter (GstClock * clock, GstClockEntry * entry, GstClockReturn ret; GST_OBJECT_LOCK (clock); - ret = gst_system_clock_id_wait_jitter_unlocked (clock, entry, jitter); + ret = gst_system_clock_id_wait_jitter_unlocked (clock, entry, jitter, FALSE); GST_OBJECT_UNLOCK (clock); return ret; diff --git a/tests/check/gst/gstsystemclock.c b/tests/check/gst/gstsystemclock.c index 059ddd0..ecba0d1 100644 --- a/tests/check/gst/gstsystemclock.c +++ b/tests/check/gst/gstsystemclock.c @@ -65,6 +65,29 @@ error_callback (GstClock * clock, GstClockTime time, return FALSE; } +static gboolean +store_callback (GstClock * clock, GstClockTime time, + GstClockID id, gpointer user_data) +{ + GstClockID *store_id = user_data; + + g_message ("unlocked async id %p\n", id); + *store_id = id; + return FALSE; +} + +static gboolean +notify_callback (GstClock * clock, GstClockTime time, + GstClockID id, gpointer user_data) +{ + gboolean *ret = (gboolean *) user_data; + + if (ret != NULL) + *ret = TRUE; + + return FALSE; +} + GST_START_TEST (test_single_shot) { GstClock *clock; @@ -190,6 +213,78 @@ GST_START_TEST (test_periodic_shot) } GST_END_TEST +GST_START_TEST (test_async_order) +{ + GstClock *clock; + GstClockID id1, id2, last_id = NULL; + GstClockTime base; + GstClockReturn result; + + clock = gst_system_clock_obtain (); + fail_unless (clock != NULL, "Could not create instance of GstSystemClock"); + + gst_clock_debug (clock); + base = gst_clock_get_time (clock); + + id1 = gst_clock_new_single_shot_id (clock, base + 2 * TIME_UNIT); + id2 = gst_clock_new_single_shot_id (clock, base + 1 * TIME_UNIT); + result = gst_clock_id_wait_async (id1, store_callback, &last_id); + fail_unless (result == GST_CLOCK_OK, "Waiting did not return OK"); + g_usleep (TIME_UNIT / (2 * 1000)); + result = gst_clock_id_wait_async (id2, store_callback, &last_id); + fail_unless (result == GST_CLOCK_OK, "Waiting did not return OK"); + g_usleep (TIME_UNIT / 1000); + fail_unless (last_id == id2, "Expected notification for id2 to come first"); + g_usleep (TIME_UNIT / 1000); + fail_unless (last_id == id1, "Missing notification for id1"); + gst_clock_id_unref (id1); + gst_clock_id_unref (id2); +} + +GST_END_TEST +GST_START_TEST (test_periodic_multi) +{ + GstClock *clock; + GstClockID clock_id; + GstClockTime base; + GstClockReturn result; + gboolean got_callback = FALSE; + + clock = gst_system_clock_obtain (); + fail_unless (clock != NULL, "Could not create instance of GstSystemClock"); + + gst_clock_debug (clock); + base = gst_clock_get_time (clock); + + clock_id = gst_clock_new_periodic_id (clock, base + TIME_UNIT, TIME_UNIT); + gst_clock_id_wait (clock_id, NULL); + fail_unless (gst_clock_get_time (clock) >= base + TIME_UNIT); + fail_unless (gst_clock_get_time (clock) < base + 2 * TIME_UNIT); + + /* now perform a concurrent wait and wait_async */ + + result = gst_clock_id_wait_async (clock_id, notify_callback, &got_callback); + fail_unless (result == GST_CLOCK_OK, "Async waiting did not return OK"); + fail_unless (got_callback == FALSE); + result = gst_clock_id_wait (clock_id, NULL); + fail_unless (result == GST_CLOCK_OK, "Waiting did not return OK"); + fail_unless (gst_clock_get_time (clock) >= base + 2 * TIME_UNIT); + /* give the async thread some time to call our callback: */ + g_usleep (TIME_UNIT / (10 * 1000)); + fail_unless (got_callback == TRUE, "got no async callback (1)"); + fail_unless (gst_clock_get_time (clock) < base + 3 * TIME_UNIT); + got_callback = FALSE; + + result = gst_clock_id_wait (clock_id, NULL); + fail_unless (result == GST_CLOCK_OK, "Waiting did not return OK"); + fail_unless (gst_clock_get_time (clock) >= base + 3 * TIME_UNIT); + /* give the async thread some time to call our callback: */ + g_usleep (TIME_UNIT / (10 * 1000)); + fail_unless (got_callback == TRUE, "got no async callback (2)"); + fail_unless (gst_clock_get_time (clock) < base + 4 * TIME_UNIT); +} + +GST_END_TEST GST_START_TEST (test_diff) { GstClockTime time1[] = @@ -213,6 +308,8 @@ gst_systemclock_suite (void) tcase_add_test (tc_chain, test_signedness); tcase_add_test (tc_chain, test_single_shot); tcase_add_test (tc_chain, test_periodic_shot); + tcase_add_test (tc_chain, test_periodic_multi); + tcase_add_test (tc_chain, test_async_order); tcase_add_test (tc_chain, test_diff); return s; -- 2.7.4