From 7b7562a713e6059a960f2e0b23a41f8c0028dd53 Mon Sep 17 00:00:00 2001 From: Pau Espin Pedrol Date: Tue, 15 May 2018 16:47:52 +0200 Subject: jibuf: Fix out-of-order seq queue around syncpoints Fixes: OS#3262 Change-Id: Ib8c61dbe6261cf73d6efcd7873e23b7656117556 --- src/jibuf.c | 36 +++++++++++++++++++++++++++++++++--- tests/jibuf/jibuf_test.c | 3 +-- tests/jibuf/jibuf_test.ok | 4 ++-- 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/src/jibuf.c b/src/jibuf.c index 45019ae..2632a57 100644 --- a/src/jibuf.c +++ b/src/jibuf.c @@ -119,6 +119,35 @@ static void llist_add_sorted(struct msgb *msg, struct llist_head *msg_list) } +static void enqueue_pkt(struct osmo_jibuf *jb, struct msgb *msg, bool is_syncpoint) +{ + struct msgb *cur; + struct timeval *msg_ts; + + if (!is_syncpoint) { + llist_add_sorted(msg, &jb->msg_list); + return; + } + + /* syncpoints change the reference timings, and as such they can provoke + out of order enqueuing of this packet and its followups with regards + to the already stored packets which may be scheduled for later times. + We thus need to adapt dequeue time for the already stored pkts to be + dequeued before the syncpoint pkt. See OS#3262 for related scenarios. + */ + + msg_ts = msgb_scheduled_ts(msg); + + llist_for_each_entry(cur, &jb->msg_list, list) { + struct timeval *cur_ts = msgb_scheduled_ts(cur); + if (timercmp(msg_ts, cur_ts, <)) + *cur_ts = *msg_ts; + } + /* syncpoint goes always to the end since we moved all older packets + before it */ + llist_add_tail(&msg->list, &jb->msg_list); +} + static bool msg_get_marker(struct msgb *msg) { /* TODO: make it more generic as a callback so that different types of @@ -314,11 +343,13 @@ int osmo_jibuf_enqueue(struct osmo_jibuf *jb, struct msgb *msg) { int rel_delay, delay; struct timeval delay_ts, sched_ts; + bool is_syncpoint; clock_gettime_timeval(CLOCK_MONOTONIC, &jb->last_enqueue_time); /* Check if it's time to sync, ie. start of talkspurt */ - if (!jb->started || msg_is_syncpoint(jb, msg)) { + is_syncpoint = !jb->started || msg_is_syncpoint(jb, msg); + if (is_syncpoint) { jb->started = true; msg_set_as_reference(jb, msg); rel_delay = 0; @@ -365,8 +396,7 @@ int osmo_jibuf_enqueue(struct osmo_jibuf *jb, struct msgb *msg) jbcb->ts = sched_ts; jbcb->old_cb = old_cb; - llist_add_sorted(msg, &jb->msg_list); - + enqueue_pkt(jb, msg, is_syncpoint); /* See if updating the timer is needed: */ if (!osmo_timer_pending(&jb->timer) || diff --git a/tests/jibuf/jibuf_test.c b/tests/jibuf/jibuf_test.c index 7993a65..ba1bb01 100644 --- a/tests/jibuf/jibuf_test.c +++ b/tests/jibuf/jibuf_test.c @@ -673,8 +673,7 @@ static void test_rtp_marker_queue_order() clock_override_add(0, min_delay*1000); osmo_select_main(0); - /* This assert shows that packets are queued out of order in this case:*/ - OSMO_ASSERT(!osmo_jibuf_empty(jb)); + OSMO_ASSERT(osmo_jibuf_empty(jb)); osmo_jibuf_delete(jb); } diff --git a/tests/jibuf/jibuf_test.ok b/tests/jibuf/jibuf_test.ok index 6bf52fb..b92f73c 100644 --- a/tests/jibuf/jibuf_test.ok +++ b/tests/jibuf/jibuf_test.ok @@ -383,10 +383,10 @@ sys={0.140000}, mono={0.140000}: clock_override_add sys={0.140000}, mono={0.140000}: dequeue: seq=34 ts=720 INTERMEDIATE sys={0.140000}, mono={0.140000}: dequeue: seq=35 ts=880 INTERMEDIATE sys={0.140000}, mono={0.140000}: dequeue: seq=36 ts=1040 INTERMEDIATE -sys={0.140000}, mono={0.140000}: dequeue: seq=39 ts=1520 INTERMEDIATE sys={0.140000}, mono={0.140000}: dequeue: seq=37 ts=1200 INTERMEDIATE -sys={0.140000}, mono={0.140000}: dequeue: seq=40 ts=1680 LATEST sys={0.140000}, mono={0.140000}: dequeue: seq=38 ts=1360 INTERMEDIATE +sys={0.140000}, mono={0.140000}: dequeue: seq=39 ts=1520 INTERMEDIATE +sys={0.140000}, mono={0.140000}: dequeue: seq=40 ts=1680 LATEST ===test_rtp_out_of_sync(1600, 5, 800, 1)=== sys={0.000000}, mono={0.000000}: clock_override_set sys={0.000000}, mono={0.000000}: enqueue 1st packet (seq=33, ts=560) -- cgit v1.2.3