From 987a903d893d8e5408262717e14319dd56f81cce Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Mon, 26 May 2008 10:09:29 +0000 Subject: [PATCH] gst/rtpmanager/gstrtpjitterbuffer.c: When checking the seqnum, reset the jitterbuffer if the gap is too big, we need ... Original commit message from CVS: * gst/rtpmanager/gstrtpjitterbuffer.c: (gst_rtp_jitter_buffer_chain), (gst_rtp_jitter_buffer_loop): When checking the seqnum, reset the jitterbuffer if the gap is too big, we need to do this so that we can better handle a restarted source. Fix some comments. * gst/rtpmanager/rtpjitterbuffer.c: (calculate_skew), (rtp_jitter_buffer_insert): Tweak the skew resync diff. Use our working seqnum compare function in -base. Rework the jitterbuffer insert code to make it clearer and more performant by only retrieving the seqnum of the input buffer once and by adding some G_LIKELY compiler hints. Improve debugging for duplicate packets. * gst/rtpmanager/rtpsource.c: (rtp_source_process_rtp): Fix a comment, we don't do skew correction here.. --- ChangeLog | 20 +++++++++ gst/rtpmanager/gstrtpjitterbuffer.c | 34 ++++++++++++---- gst/rtpmanager/rtpjitterbuffer.c | 63 +++++++++++++++-------------- gst/rtpmanager/rtpsource.c | 2 +- 4 files changed, 81 insertions(+), 38 deletions(-) diff --git a/ChangeLog b/ChangeLog index 6c7365cdfa..b98c11cb41 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,23 @@ +2008-05-26 Wim Taymans + + * gst/rtpmanager/gstrtpjitterbuffer.c: + (gst_rtp_jitter_buffer_chain), (gst_rtp_jitter_buffer_loop): + When checking the seqnum, reset the jitterbuffer if the gap is too big, + we need to do this so that we can better handle a restarted source. + Fix some comments. + + * gst/rtpmanager/rtpjitterbuffer.c: (calculate_skew), + (rtp_jitter_buffer_insert): + Tweak the skew resync diff. + Use our working seqnum compare function in -base. + Rework the jitterbuffer insert code to make it clearer and more + performant by only retrieving the seqnum of the input buffer once and by + adding some G_LIKELY compiler hints. + Improve debugging for duplicate packets. + + * gst/rtpmanager/rtpsource.c: (rtp_source_process_rtp): + Fix a comment, we don't do skew correction here.. + 2008-05-26 Wim Taymans Patch by: Håvard Graff diff --git a/gst/rtpmanager/gstrtpjitterbuffer.c b/gst/rtpmanager/gstrtpjitterbuffer.c index 46439dc1b9..17dd4a9099 100644 --- a/gst/rtpmanager/gstrtpjitterbuffer.c +++ b/gst/rtpmanager/gstrtpjitterbuffer.c @@ -885,12 +885,32 @@ gst_rtp_jitter_buffer_chain (GstPad * pad, GstBuffer * buffer) if (priv->eos) goto have_eos; - /* let's check if this buffer is too late, we cannot accept packets with - * bigger seqnum than the one we already pushed. */ + /* let's check if this buffer is too late, we can only accept packets with + * bigger seqnum than the one we last pushed. */ if (priv->last_popped_seqnum != -1) { - /* FIXME. isn't this supposed to be <= ? */ - if (gst_rtp_buffer_compare_seqnum (priv->last_popped_seqnum, seqnum) < 0) - goto too_late; + gint gap; + + gap = gst_rtp_buffer_compare_seqnum (priv->last_popped_seqnum, seqnum); + + if (gap <= 0) { + /* priv->last_popped_seqnum >= seqnum, this packet is too late or the + * sender might have been restarted with different seqnum. */ + if (gap < -100) { + GST_DEBUG_OBJECT (jitterbuffer, "reset: buffer too old %d", gap); + priv->last_popped_seqnum = -1; + priv->next_seqnum = -1; + } else { + goto too_late; + } + } else { + /* priv->last_popped_seqnum < seqnum, this is a new packet */ + if (gap > 3000) { + GST_DEBUG_OBJECT (jitterbuffer, "reset: too many dropped packets %d", + gap); + priv->last_popped_seqnum = -1; + priv->next_seqnum = -1; + } + } } /* let's drop oldest packet if the queue is already full and drop-on-latency @@ -1041,7 +1061,7 @@ again: if (priv->eos) goto do_eos; } - /* wait for packets or flushing now */ + /* underrun, wait for packets or flushing now */ priv->waiting = TRUE; JBUF_WAIT_CHECK (priv, flushing); priv->waiting = FALSE; @@ -1187,7 +1207,7 @@ again: if (gap > 0) { GstEvent *event; - /* we had a gap and thus we lost a packet. Creat an event for this. */ + /* we had a gap and thus we lost a packet. Create an event for this. */ GST_DEBUG_OBJECT (jitterbuffer, "Packet #%d lost", next_seqnum); priv->num_late++; discont = TRUE; diff --git a/gst/rtpmanager/rtpjitterbuffer.c b/gst/rtpmanager/rtpjitterbuffer.c index 8aaefeb00b..caca165df4 100644 --- a/gst/rtpmanager/rtpjitterbuffer.c +++ b/gst/rtpmanager/rtpjitterbuffer.c @@ -217,8 +217,10 @@ again: delta_diff = jbuf->prev_send_diff - send_diff; /* server changed rtp timestamps too quickly, reset skew detection and start - * again. */ - if (delta_diff > GST_SECOND / 4) { + * again. This value is sortof arbitrary and can be a bad measurement up if + * there are many packets missing because then we get a big gap that is + * unrelated to a timestamp switch. */ + if (delta_diff > GST_SECOND) { GST_DEBUG ("delta changed too quickly %" GST_TIME_FORMAT " reset skew", GST_TIME_ARGS (delta_diff)); rtp_jitter_buffer_reset_skew (jbuf); @@ -326,23 +328,6 @@ no_skew: return out_time; } -static gint -compare_seqnum (GstBuffer * a, GstBuffer * b, RTPJitterBuffer * jbuf) -{ - guint16 seq1, seq2; - - seq1 = gst_rtp_buffer_get_seq (a); - seq2 = gst_rtp_buffer_get_seq (b); - - /* check if diff more than half of the 16bit range */ - if (abs (seq2 - seq1) > (1 << 15)) { - /* one of a/b has wrapped */ - return seq1 - seq2; - } else { - return seq2 - seq1; - } -} - /** * rtp_jitter_buffer_insert: * @jbuf: an #RTPJitterBuffer @@ -362,22 +347,32 @@ rtp_jitter_buffer_insert (RTPJitterBuffer * jbuf, GstBuffer * buf, GstClockTime time, guint32 clock_rate, gboolean * tail) { GList *list; - gint func_ret = 1; guint32 rtptime; + guint16 seqnum; g_return_val_if_fail (jbuf != NULL, FALSE); g_return_val_if_fail (buf != NULL, FALSE); - /* loop the list to skip strictly smaller seqnum buffers */ - list = jbuf->packets->head; - while (list - && (func_ret = - compare_seqnum (GST_BUFFER_CAST (list->data), buf, jbuf)) < 0) - list = list->next; + seqnum = gst_rtp_buffer_get_seq (buf); - /* we hit a packet with the same seqnum, return FALSE to notify a duplicate */ - if (func_ret == 0) - return FALSE; + /* loop the list to skip strictly smaller seqnum buffers */ + for (list = jbuf->packets->head; list; list = g_list_next (list)) { + guint16 qseq; + gint gap; + + qseq = gst_rtp_buffer_get_seq (GST_BUFFER_CAST (list->data)); + + /* compare the new seqnum to the one in the buffer */ + gap = gst_rtp_buffer_compare_seqnum (seqnum, qseq); + + /* we hit a packet with the same seqnum, notify a duplicate */ + if (G_UNLIKELY (gap == 0)) + goto duplicate; + + /* seqnum > qseq, we can stop looking */ + if (G_LIKELY (gap < 0)) + break; + } /* do skew calculation by measuring the difference between rtptime and the * receive time, this function will retimestamp @buf with the skew corrected @@ -391,11 +386,19 @@ rtp_jitter_buffer_insert (RTPJitterBuffer * jbuf, GstBuffer * buf, else g_queue_push_tail (jbuf->packets, buf); - /* tail was changed when we did not find a previous packet */ + /* tail was changed when we did not find a previous packet, we set the return + * flag when requested. */ if (tail) *tail = (list == NULL); return TRUE; + + /* ERRORS */ +duplicate: + { + GST_WARNING ("duplicate packet %d found", (gint) seqnum); + return FALSE; + } } /** diff --git a/gst/rtpmanager/rtpsource.c b/gst/rtpmanager/rtpsource.c index 4c351a1a6e..7eab91e087 100644 --- a/gst/rtpmanager/rtpsource.c +++ b/gst/rtpmanager/rtpsource.c @@ -940,7 +940,7 @@ rtp_source_process_rtp (RTPSource * src, GstBuffer * buffer, GST_DEBUG ("seq %d, PC: %" G_GUINT64_FORMAT ", OC: %" G_GUINT64_FORMAT, seqnr, src->stats.packets_received, src->stats.octets_received); - /* calculate jitter and perform skew correction */ + /* calculate jitter for the stats */ calculate_jitter (src, buffer, arrival); /* we're ready to push the RTP packet now */