jitterbuffer: Speed up lost timeout handling

When downstream blocks, "lost" timers are created to notify the
outgoing thread that packets are lost.

The problem is that for high packet-rate streams, we might end up with
a big list of lost timeouts (had a use-case with ~1000...).

The problem isn't so much the amount of lost timeouts to handle, but
rather the way they were handled. All timers would first be iterated,
then the one selected would be handled ... to re-iterate the list again.

All of this is being done while the jbuf lock is taken, which in some use-cases
would return in holding that lock for 10s... blocking any buffers from
being accepted in input... which would then arrive late ... which would
create plenty of lost timers ... which would cause the same issue.

In order to avoid that situation, handle the lost timers immediately when
iterating the list of pending timers. This modifies the complexity from
a quadratic to a linear complexity.

https://bugzilla.gnome.org/show_bug.cgi?id=762988
This commit is contained in:
Edward Hervey 2016-03-02 14:25:24 +01:00 committed by Edward Hervey
parent d656fe8d54
commit b82da62922

View file

@ -3522,40 +3522,56 @@ wait_next_timeout (GstRtpJitterBuffer * jitterbuffer)
GST_TIME_ARGS (now)); GST_TIME_ARGS (now));
len = priv->timers->len; len = priv->timers->len;
for (i = 0; i < len; i++) { for (i = 0; i < len;) {
TimerData *test = &g_array_index (priv->timers, TimerData, i); TimerData *test = &g_array_index (priv->timers, TimerData, i);
GstClockTime test_timeout = get_timeout (jitterbuffer, test); GstClockTime test_timeout = get_timeout (jitterbuffer, test);
gboolean save_best = FALSE; gboolean save_best = FALSE;
GST_DEBUG_OBJECT (jitterbuffer, "%d, %d, %d, %" GST_TIME_FORMAT, GST_DEBUG_OBJECT (jitterbuffer,
i, test->type, test->seqnum, GST_TIME_ARGS (test_timeout)); "%d, %d, %d, %" GST_TIME_FORMAT " diff:%" GST_STIME_FORMAT, i,
test->type, test->seqnum, GST_TIME_ARGS (test_timeout),
GST_STIME_ARGS ((gint64) (test_timeout - now)));
/* find the smallest timeout */ /* Weed out anything too late */
if (timer == NULL) { if (test->type == TIMER_TYPE_LOST &&
save_best = TRUE; (test_timeout == -1 || test_timeout <= now)) {
} else if (timer_timeout == -1) { GST_DEBUG_OBJECT (jitterbuffer, "Weeding out late entry");
/* we already have an immediate timeout, the new timer must be an do_lost_timeout (jitterbuffer, test, now);
* immediate timer with smaller seqnum to become the best */ if (!priv->timer_running)
if (test_timeout == -1 break;
&& (gst_rtp_buffer_compare_seqnum (test->seqnum, /* We don't move the iterator forward since we just removed the current entry,
timer->seqnum) > 0)) * but we update the termination condition */
len = priv->timers->len;
} else {
/* find the smallest timeout */
if (timer == NULL) {
save_best = TRUE; save_best = TRUE;
} else if (test_timeout == -1) { } else if (timer_timeout == -1) {
/* first immediate timer */ /* we already have an immediate timeout, the new timer must be an
save_best = TRUE; * immediate timer with smaller seqnum to become the best */
} else if (test_timeout < timer_timeout) { if (test_timeout == -1
/* earlier timer */ && (gst_rtp_buffer_compare_seqnum (test->seqnum,
save_best = TRUE; timer->seqnum) > 0))
} else if (test_timeout == timer_timeout save_best = TRUE;
&& (gst_rtp_buffer_compare_seqnum (test->seqnum, } else if (test_timeout == -1) {
timer->seqnum) > 0)) { /* first immediate timer */
/* same timer, smaller seqnum */ save_best = TRUE;
save_best = TRUE; } else if (test_timeout < timer_timeout) {
} /* earlier timer */
if (save_best) { save_best = TRUE;
GST_DEBUG_OBJECT (jitterbuffer, "new best %d", i); } else if (test_timeout == timer_timeout
timer = test; && (gst_rtp_buffer_compare_seqnum (test->seqnum,
timer_timeout = test_timeout; timer->seqnum) > 0)) {
/* same timer, smaller seqnum */
save_best = TRUE;
}
if (save_best) {
GST_DEBUG_OBJECT (jitterbuffer, "new best %d", i);
timer = test;
timer_timeout = test_timeout;
}
i++;
} }
} }
if (timer && !priv->blocked) { if (timer && !priv->blocked) {
@ -3565,6 +3581,9 @@ wait_next_timeout (GstRtpJitterBuffer * jitterbuffer)
GstClockReturn ret; GstClockReturn ret;
GstClockTimeDiff clock_jitter; GstClockTimeDiff clock_jitter;
/* We have normally removed all lost timers in the loop above */
g_assert (timer->type != TIMER_TYPE_LOST);
if (timer_timeout == -1 || timer_timeout <= now) { if (timer_timeout == -1 || timer_timeout <= now) {
do_timeout (jitterbuffer, timer, now); do_timeout (jitterbuffer, timer, now);
/* check here, do_timeout could have released the lock */ /* check here, do_timeout could have released the lock */