mirrors/gstreamer
mirrors/gstreamer
1
1
Fork 0
mirror of https://gitlab.freedesktop.org/gstreamer/gstreamer.git synced 2025-01-24 08:08:22 +00:00
Code Issues 1 Projects Releases Wiki Activity

rtpjitterbuffer: don't use RTX packets in rate-calc and reset-logic

Browse source 
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.
This commit is contained in:
Havard Graff 2020-04-16 16:47:50 +02:00
parent 8e3184a213
commit 981d0c02de
2 changed files with 33 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
gst/rtpmanager/gstrtpjitterbuffer.c
View file

@ -2946,16 +2946,19 @@ gst_rtp_jitter_buffer_chain (GstPad * pad, GstObject * parent,
expected = priv->next_in_seqnum;
packet_rate =
gst_rtp_packet_rate_ctx_update (&priv->packet_rate_ctx, seqnum, rtptime);
/* don't update packet-rate based on RTX, as those arrive highly unregularly */
if (!is_rtx) {
packet_rate = gst_rtp_packet_rate_ctx_update (&priv->packet_rate_ctx,
seqnum, rtptime);
GST_TRACE_OBJECT (jitterbuffer, "updated packet_rate: %d", packet_rate);
}
max_dropout =
gst_rtp_packet_rate_ctx_get_max_dropout (&priv->packet_rate_ctx,
priv->max_dropout_time);
max_misorder =
gst_rtp_packet_rate_ctx_get_max_misorder (&priv->packet_rate_ctx,
priv->max_misorder_time);
GST_TRACE_OBJECT (jitterbuffer,
"packet_rate: %d, max_dropout: %d, max_misorder: %d", packet_rate,
GST_TRACE_OBJECT (jitterbuffer, "max_dropout: %d, max_misorder: %d",
max_dropout, max_misorder);
timer = rtp_timer_queue_find (priv->timers, seqnum);
@ -3021,7 +3024,7 @@ gst_rtp_jitter_buffer_chain (GstPad * pad, GstObject * parent,
}
/* Special handling of large gaps */
if ((gap != -1 && gap < -max_misorder) || (gap >= max_dropout)) {
if (!is_rtx && ((gap != -1 && gap < -max_misorder) || (gap >= max_dropout))) {
gboolean reset = handle_big_gap_buffer (jitterbuffer, buffer, pt, seqnum,
gap, max_dropout, max_misorder);
if (reset) {

25
tests/check/elements/rtpjitterbuffer.c
View file

@ -3126,6 +3126,29 @@ GST_START_TEST (test_multiple_lost_do_not_stall)
GST_END_TEST;
GST_START_TEST (test_reset_using_rtx_packets_does_not_stall)
{
GstHarness *h = gst_harness_new ("rtpjitterbuffer");
BufferArrayCtx bufs[] = {
/* *INDENT-OFF* */
{ 1, 1 * TEST_RTP_TS_DURATION, FALSE, 2000000},
{ 62, 62 * TEST_RTP_TS_DURATION, FALSE, 0},
{ -13, -13 * TEST_RTP_TS_DURATION, TRUE, 10000},
{ 1, 1 * TEST_RTP_TS_DURATION, TRUE, 0},
{ 1, 1 * TEST_RTP_TS_DURATION, TRUE, 0},
{ 1, 1 * TEST_RTP_TS_DURATION, TRUE, 0},
{ 1, 1 * TEST_RTP_TS_DURATION, TRUE, 0},
{ 1, 1 * TEST_RTP_TS_DURATION, TRUE, 0},
/* *INDENT-ON* */
};
g_object_set (h->element, "latency", 400,
"do-retransmission", TRUE, "do-lost", TRUE, "max-misorder-time", 1, NULL);
fail_unless (check_for_stall (h, bufs, G_N_ELEMENTS (bufs)));
gst_harness_teardown (h);
}
GST_END_TEST;
static Suite *
rtpjitterbuffer_suite (void)
@ -3196,6 +3219,8 @@ rtpjitterbuffer_suite (void)
tcase_add_test (tc_chain, test_reset_timers_does_not_stall);
tcase_add_test (tc_chain, test_multiple_lost_do_not_stall);
tcase_add_test (tc_chain, test_reset_using_rtx_packets_does_not_stall);
return s;
}