GNOME Bugzilla – Bug 725159
rtpjitterbuffer: RTP sequence number rollover problems
Last modified: 2014-02-26 20:12:44 UTC
Created attachment 270285 [details] [review] changes made to rtpjitterbuffer Test Environment : - gstreamer 1.2.3 - Windows - Pipeline : rtspsrc ! fakesink - Pulling an Uncompressed MJPEG stream from an Axis Camera at 30 fps which results to approximately 2700 RTP packets per second. Symptom : The rtpjitterbuffer will occasionally enter a state where it is unable to push new buffers to downstream elements. The rtpjitterbuffer::handle_next_buffer() function will repeatedly post the debug message "Sequence number GAP detected : expected .... instead of ...." This eventually results in "Packet too late" errors in the rtpjitterbuffer:gst_rtp_jitter_buffer_chain() function. The problem appears to occur when a packet is received ahead of what is expected and at the RTP sequence number rollover point. (eg. expected seqnum = 65533, received seqnum = 2). In this case, the rtpjitterbuffer:calculate_expected() function is called and should create timers for the expected arrival of packets 65533, 65534, 65535, 0, and 1. However, in the rtpjitterbuffer:calculate_expected() function, the while loop that adds these timers uses the condition : while(expected < seqnum) which does not account for the rollover of the RTP sequence number. Suggested Changes: In gst-plugins-good/gst/rtpmanager/gstrtpjitterbuffer.c Change all raw comparisons of RTP sequence numbers to use the gst_rtp_buffer_compare_seqnum() function. I found 3 lines where this was happening. See attached patch.
Review of attachment 270285 [details] [review]: Can you attach this as a "git format-patch" style patch? ::: gstrtpjitterbuffer.c.orig @@ +2868,3 @@ /* earlier timer */ save_best = TRUE; + } else if (test_timeout == timer_timeout && (gst_rtp_buffer_compare_seqnum (test->seqnum < timer->seqnum) > 0)) { Here is a typo, the < should be a comma.
Created attachment 270379 [details] [review] Updated patch Fixed the typo in the original patch, and used "git format-patch" this time.
Thanks for the patch :) commit 6dd9142592475e6bbe56a5c07e81c14412ae18f1 Author: Jake Foytik <jake.foytik@ipconfigure.com> Date: Wed Feb 26 07:32:32 2014 -0500 rtpjitterbuffer: Remove raw comparisons of RTP sequence numbers Several conditional statements perform comparison on RTP sequence numbers without taking the sequence number rollover into account. Instead, use the gst_rtp_buffer_compare_seqnum function to perform the comparison. https://bugzilla.gnome.org/show_bug.cgi?id=725159