rtpjitterbuffer: don't use RTX packets in rate-calc and reset-logic
authorHavard Graff <havard@pexip.com>
Thu, 16 Apr 2020 14:47:50 +0000 (16:47 +0200)
committerHavard Graff <havard@pexip.com>
Thu, 16 Apr 2020 15:06:31 +0000 (17:06 +0200)
commit981d0c02de7229edcaf2e31843df4899692779d1
tree2ab5ad27c8da31b7d844c7dde7c72a6dccc1551b
parent8e3184a2137c3483c1aabaef39f78655421acd52
rtpjitterbuffer: don't use RTX packets in rate-calc and reset-logic

The problem was this:

Due to the highly irregular arrival of RTX-packet the max-misorder variable
could be pushed very low. (-10).

If you then at some point get a big in the sequence-numbers (62 in the
test) you end up sending RTX-requests for some of those packets, and then
if the sender answers those requests, you are going to get a bunch of
RTX-packets arriving. (-13 and then 5 more packets in the test)

Now, if max-misorder is pushed very low at this point, these RTX-packets
will trigger the handle_big_gap_buffer() logic, and because they arriving
so neatly in order, (as they would, since they have been requested like
that), the gst_rtp_jitter_buffer_reset() will be called, and two things
will happen:
1. priv->next_seqnum will be set to the first RTX packet
2. the 5 RTX-packet will be pushed into the chain() function

However, at this point, these RTX-packets are no longer valid, the
jitterbuffer has already pushed lost-events for these, so they will now
be dropped on the floor, and never make it to the waiting loop-function.

And, since we now have a priv->next_seqnum that will never arrive
in the loop-function, the jitterbuffer is now stalled forever, and will
not push out another buffer.

The proposed fixes:
1. Don't use RTX in calculation of the packet-rate.
2. Don't use RTX in large-gap logic, as they are likely to be dropped.
gst/rtpmanager/gstrtpjitterbuffer.c
tests/check/elements/rtpjitterbuffer.c