rtpjitterbuffer: remove the concept of "already-lost"

This is a concept that only applies when a buffer arrives in the chain
function, and it has already been scheduled as part of a "multi"-lost
timer.

However, "multi"-lost timers are now a thing of the past, making this
whole concept superflous, and this buffer is now simply counted as "late",
having already been pushed out (albeit as a lost-event).
This commit is contained in:
Havard Graff 2020-03-19 23:37:26 +01:00 committed by GStreamer Merge Bot
parent 5dacf366c0
commit f1ff80ced0
2 changed files with 0 additions and 126 deletions

View file

@ -399,13 +399,11 @@ struct _GstRtpJitterBufferPrivate
GstClockTime last_drop_msg_timestamp;
/* accumulators; reset every time a drop message is posted */
guint num_too_late;
guint num_already_lost;
guint num_drop_on_latency;
};
typedef enum
{
REASON_TOO_LATE,
REASON_ALREADY_LOST,
REASON_DROP_ON_LATENCY
} DropMessageReason;
@ -597,8 +595,6 @@ gst_rtp_jitter_buffer_class_init (GstRtpJitterBufferClass * klass)
* * #gstring `reason`: Reason for dropping the packet.
* * #guint `num-too-late`: Number of packets arriving too late since
* last drop message.
* * #guint `num-already-lost`: Number of packets already considered lost
* since drop message.
* * #guint `num-drop-on-latency`: Number of packets dropped due to the
* drop-on-latency property since last drop message.
*
@ -1022,7 +1018,6 @@ gst_rtp_jitter_buffer_init (GstRtpJitterBuffer * jitterbuffer)
priv->avg_jitter = 0;
priv->last_drop_msg_timestamp = GST_CLOCK_TIME_NONE;
priv->num_too_late = 0;
priv->num_already_lost = 0;
priv->num_drop_on_latency = 0;
priv->segment_seqnum = GST_SEQNUM_INVALID;
priv->timers = rtp_timer_queue_new ();
@ -1619,7 +1614,6 @@ gst_rtp_jitter_buffer_flush_stop (GstRtpJitterBuffer * jitterbuffer)
priv->segment_seqnum = GST_SEQNUM_INVALID;
priv->last_drop_msg_timestamp = GST_CLOCK_TIME_NONE;
priv->num_too_late = 0;
priv->num_already_lost = 0;
priv->num_drop_on_latency = 0;
GST_DEBUG_OBJECT (jitterbuffer, "flush and reset jitterbuffer");
rtp_jitter_buffer_flush (priv->jbuf, NULL, NULL);
@ -2047,9 +2041,6 @@ new_drop_message (GstRtpJitterBuffer * jitterbuffer, guint seqnum,
if (reason == REASON_TOO_LATE) {
priv->num_too_late++;
reason_str = "too-late";
} else if (reason == REASON_ALREADY_LOST) {
priv->num_already_lost++;
reason_str = "already-lost";
} else if (reason == REASON_DROP_ON_LATENCY) {
priv->num_drop_on_latency++;
reason_str = "drop-on-latency";
@ -2068,12 +2059,10 @@ new_drop_message (GstRtpJitterBuffer * jitterbuffer, guint seqnum,
"timestamp", GST_TYPE_CLOCK_TIME, timestamp,
"reason", G_TYPE_STRING, reason_str,
"num-too-late", G_TYPE_UINT, priv->num_too_late,
"num-already-lost", G_TYPE_UINT, priv->num_already_lost,
"num-drop-on-latency", G_TYPE_UINT, priv->num_drop_on_latency, NULL);
priv->last_drop_msg_timestamp = current_time;
priv->num_too_late = 0;
priv->num_already_lost = 0;
priv->num_drop_on_latency = 0;
drop_msg = gst_message_new_element (GST_OBJECT (jitterbuffer), s);
}
@ -2239,33 +2228,6 @@ get_rtx_delay (GstRtpJitterBufferPrivate * priv)
return delay;
}
/* Check if packet with seqnum is already considered definitely lost by being
* part of a "lost timer" for multiple packets */
static gboolean
already_lost (GstRtpJitterBuffer * jitterbuffer, GstClockTime pts,
guint16 seqnum)
{
GstRtpJitterBufferPrivate *priv = jitterbuffer->priv;
RtpTimer *test;
test = rtp_timer_queue_peek_earliest (priv->timers);
while (test && get_pts_timeout (test) <= pts) {
gint gap = gst_rtp_buffer_compare_seqnum (test->seqnum, seqnum);
if (test->num > 1 && test->type == RTP_TIMER_LOST && gap >= 0 &&
gap < test->num) {
GST_DEBUG_OBJECT (jitterbuffer,
"seqnum #%d already considered definitely lost (#%d->#%d)",
seqnum, test->seqnum, (test->seqnum + test->num - 1) & 0xffff);
return TRUE;
}
test = rtp_timer_get_next (test);
}
return FALSE;
}
/* we just received a packet with seqnum and dts.
*
* First check for old seqnum that we are still expecting. If the gap with the
@ -3188,9 +3150,6 @@ gst_rtp_jitter_buffer_chain (GstPad * pad, GstObject * parent,
}
}
if (already_lost (jitterbuffer, pts, seqnum))
goto already_lost;
/* let's drop oldest packet if the queue is already full and drop-on-latency
* is set. We can only do this when there actually is a latency. When no
* latency is set, we just pump it in the queue and let the other end push it
@ -3320,18 +3279,6 @@ too_late:
gst_buffer_unref (buffer);
goto finished;
}
already_lost:
{
GST_DEBUG_OBJECT (jitterbuffer, "Packet #%d too late as it was already "
"considered lost", seqnum);
priv->num_late++;
if (priv->post_drop_messages) {
drop_msg =
new_drop_message (jitterbuffer, seqnum, pts, REASON_ALREADY_LOST);
}
gst_buffer_unref (buffer);
goto finished;
}
duplicate:
{
GST_DEBUG_OBJECT (jitterbuffer, "Duplicate packet #%d detected, dropping",

View file

@ -2769,19 +2769,15 @@ check_drop_message (GstMessage * drop_msg, const char *reason_check,
GstClockTime timestamp;
guint seqnum;
guint num_too_late;
guint num_already_lost;
guint num_drop_on_latency;
guint num_too_late_check = 0;
guint num_already_lost_check = 0;
guint num_drop_on_latency_check = 0;
/* Check that fields exist */
fail_unless (gst_structure_get_uint (s, "seqnum", &seqnum));
fail_unless (gst_structure_get_uint64 (s, "timestamp", &timestamp));
fail_unless (gst_structure_get_uint (s, "num-too-late", &num_too_late));
fail_unless (gst_structure_get_uint (s, "num-already-lost",
&num_already_lost));
fail_unless (gst_structure_get_uint (s, "num-drop-on-latency",
&num_drop_on_latency));
fail_unless (reason_str = gst_structure_get_string (s, "reason"));
@ -2789,8 +2785,6 @@ check_drop_message (GstMessage * drop_msg, const char *reason_check,
/* Assing what to compare message fields to based on message reason */
if (g_strcmp0 (reason_check, "too-late") == 0) {
num_too_late_check += num_msg;
} else if (g_strcmp0 (reason_check, "already-lost") == 0) {
num_already_lost_check += num_msg;
} else if (g_strcmp0 (reason_check, "drop-on-latency") == 0) {
num_drop_on_latency_check += num_msg;
} else {
@ -2801,7 +2795,6 @@ check_drop_message (GstMessage * drop_msg, const char *reason_check,
fail_unless (seqnum == seqnum_check);
fail_unless (g_strcmp0 (reason_str, reason_check) == 0);
fail_unless (num_too_late == num_too_late_check);
fail_unless (num_already_lost == num_already_lost_check);
fail_unless (num_drop_on_latency == num_drop_on_latency_check);
return TRUE;
@ -2855,71 +2848,6 @@ GST_START_TEST (test_drop_messages_too_late)
GST_END_TEST;
GST_START_TEST (test_drop_messages_already_lost)
{
GstHarness *h = gst_harness_new ("rtpjitterbuffer");
GstTestClock *testclock;
GstClockID id;
gint latency_ms = 20;
guint seqnum_late;
guint seqnum_final;
GstBus *bus;
GstMessage *drop_msg;
gboolean have_message = FALSE;
testclock = gst_harness_get_testclock (h);
g_object_set (h->element, "post-drop-messages", TRUE, NULL);
/* Create a bus to get the drop message on */
bus = gst_bus_new ();
gst_element_set_bus (h->element, bus);
/* Get seqnum from initial state */
seqnum_late = construct_deterministic_initial_state (h, latency_ms);
/* Hop over 3 buffers and push buffer (gap of 3) */
seqnum_final = seqnum_late + 4;
fail_unless_equals_int (GST_FLOW_OK,
gst_harness_push (h,
generate_test_buffer_full (seqnum_final * TEST_BUF_DURATION,
seqnum_final, seqnum_final * TEST_RTP_TS_DURATION)));
/* The jitterbuffer should be waiting for the timeout of a "large gap timer"
* for buffer seqnum_late and seqnum_late+1 */
gst_test_clock_wait_for_next_pending_id (testclock, &id);
fail_unless_equals_uint64 (seqnum_late * TEST_BUF_DURATION +
latency_ms * GST_MSECOND, gst_clock_id_get_time (id));
/* Now seqnum_late sneaks in before the lost event for buffer seqnum_late and seqnum_late+1 is
* processed. It will be dropped due to already having been considered lost */
fail_unless_equals_int (GST_FLOW_OK,
gst_harness_push (h,
generate_test_buffer_full (seqnum_late * TEST_BUF_DURATION,
seqnum_late, seqnum_late * TEST_RTP_TS_DURATION)));
/* Pop the resulting drop message and check its correctness */
while (!have_message &&
(drop_msg = gst_bus_pop_filtered (bus, GST_MESSAGE_ELEMENT)) != NULL) {
if (gst_message_has_name (drop_msg, "drop-msg")) {
fail_unless (check_drop_message (drop_msg, "already-lost", seqnum_late,
1));
have_message = TRUE;
}
gst_message_unref (drop_msg);
}
fail_unless (have_message);
/* Cleanup */
gst_clock_id_unref (id);
gst_element_set_bus (h->element, NULL);
gst_buffer_unref (gst_harness_take_all_data_as_buffer (h));
gst_object_unref (bus);
gst_object_unref (testclock);
gst_harness_teardown (h);
}
GST_END_TEST;
GST_START_TEST (test_drop_messages_drop_on_latency)
{
GstHarness *h = gst_harness_new ("rtpjitterbuffer");
@ -3226,7 +3154,6 @@ rtpjitterbuffer_suite (void)
tcase_add_test (tc_chain, test_performance);
tcase_add_test (tc_chain, test_drop_messages_too_late);
tcase_skip_broken_test (tc_chain, test_drop_messages_already_lost);
tcase_add_test (tc_chain, test_drop_messages_drop_on_latency);
tcase_add_test (tc_chain, test_drop_messages_interval);