diff --git a/ChangeLog b/ChangeLog index a774d9093e..4e783b6938 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,23 @@ +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): diff --git a/gst/gstclock.c b/gst/gstclock.c index a2924775cc..4ef7b9d4e1 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 6f4cd905a3..57d8e89214 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 059ddd01c1..ecba0d1c6b 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; @@ -189,6 +212,78 @@ GST_START_TEST (test_periodic_shot) gst_clock_id_unref (id); } +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) { @@ -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;