Skip to content

Commit

Permalink
gstrtpjitterbuffer: fix gap-time calculation and remove "late"
Browse files Browse the repository at this point in the history
The amount of time that is completely expired and not worth waiting for,
is the duration of the packets in the gap (gap * duration) - the
latency (size) of the jitterbuffer (priv->latency_ns). This is the duration
that we make a "multi-lost" packet for.

The "late" concept made some sense in 0.10 as it reflected that a buffer
coming in had not been waited for at all, but had a timestamp that was
outside the jitterbuffer to wait for. With the rewrite of the waiting
(timeout) mechanism in 1.0, this no longer makes any sense, and the
variable no longer reflects anything meaningful (num > 0 is useless,
the duration is what matters)

Fixed up the tests that had been slightly modified in 1.0 to allow faulty
behavior to sneak in, and port some of them to use GstHarness.
  • Loading branch information
havardgraff authored and Stian Selnes committed Jul 15, 2014
1 parent 6049aea commit ca1e66a
Show file tree
Hide file tree
Showing 2 changed files with 279 additions and 255 deletions.
34 changes: 19 additions & 15 deletions gst/rtpmanager/gstrtpjitterbuffer.c
Expand Up @@ -1961,29 +1961,35 @@ calculate_expected (GstRtpJitterBuffer * jitterbuffer, guint32 expected,
GstClockTime gap_time;
guint lost_packets;

gap_time = total_duration - priv->latency_ns;

if (duration > 0) {
GstClockTime gap_dur = gap * duration;
if (gap_dur > priv->latency_ns)
gap_time = gap_dur - priv->latency_ns;
else
gap_time = 0;
lost_packets = gap_time / duration;
gap_time = lost_packets * duration;
} else {
gap_time = total_duration - priv->latency_ns;
lost_packets = gap;
}

/* too many lost packets, some of the missing packets are already
* too late and we can generate lost packet events for them. */
GST_DEBUG_OBJECT (jitterbuffer, "too many lost packets %" GST_TIME_FORMAT
" > %" GST_TIME_FORMAT ", consider %u lost",
GST_TIME_ARGS (total_duration), GST_TIME_ARGS (priv->latency_ns),
lost_packets);
GST_DEBUG_OBJECT (jitterbuffer,
"lost packets (%d, #%d->#%d) duration too large %" GST_TIME_FORMAT
" > %" GST_TIME_FORMAT ", consider %u lost (%" GST_TIME_FORMAT ")",
gap, expected, seqnum, GST_TIME_ARGS (total_duration),
GST_TIME_ARGS (priv->latency_ns), lost_packets,
GST_TIME_ARGS(gap_time));

/* this timer will fire immediately and the lost event will be pushed from
* the timer thread */
add_timer (jitterbuffer, TIMER_TYPE_LOST, expected, lost_packets,
priv->last_in_dts + duration, 0, gap_time);

expected += lost_packets;
priv->last_in_dts += gap_time;
if (lost_packets > 0) {
add_timer (jitterbuffer, TIMER_TYPE_LOST, expected, lost_packets,
priv->last_in_dts + duration, 0, gap_time);
expected += lost_packets;
priv->last_in_dts += gap_time;
}
}

expected_dts = priv->last_in_dts + duration;
Expand Down Expand Up @@ -2759,7 +2765,7 @@ do_lost_timeout (GstRtpJitterBuffer * jitterbuffer, TimerData * timer,
GstRtpJitterBufferPrivate *priv = jitterbuffer->priv;
GstClockTime duration, timestamp;
guint seqnum, lost_packets, num_rtx_retry, next_in_seqnum;
gboolean late, head;
gboolean head;
GstEvent *event;
RTPJitterBufferItem *item;

Expand All @@ -2769,7 +2775,6 @@ do_lost_timeout (GstRtpJitterBuffer * jitterbuffer, TimerData * timer,
if (duration == GST_CLOCK_TIME_NONE && priv->packet_spacing > 0)
duration = priv->packet_spacing;
lost_packets = MAX (timer->num, 1);
late = timer->num > 0;
num_rtx_retry = timer->num_rtx_retry;

/* we had a gap and thus we lost some packets. Create an event for this. */
Expand All @@ -2794,7 +2799,6 @@ do_lost_timeout (GstRtpJitterBuffer * jitterbuffer, TimerData * timer,
"seqnum", G_TYPE_UINT, (guint) seqnum,
"timestamp", G_TYPE_UINT64, timestamp,
"duration", G_TYPE_UINT64, duration,
"late", G_TYPE_BOOLEAN, late,
"retry", G_TYPE_UINT, num_rtx_retry, NULL));

item = alloc_item (event, ITEM_TYPE_LOST, -1, -1, seqnum, lost_packets, -1);
Expand Down

0 comments on commit ca1e66a

Please sign in to comment.