[Bug 772646] rtpjitterbuffer: fix lost-event using dts instead of pts

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=772646

--- Comment #3 from Håvard Graff (hgr) <havard.graff at gmail.com> ---
(In reply to Sebastian Dröge (slomo) from comment #2)
> Review of attachment 337275 [details] [review]:
> 
> Just some comments for now
> 
> ::: gst/rtpmanager/gstrtpjitterbuffer.c
> @@ +2342,2 @@
>      /* calculate expected arrival time of the next seqnum */
> +    expected = pts + priv->packet_spacing;
> 
> Isn't the arrival time dependant on the dts (aka arrival time at this
> point)? While the packet spacing would depend on the pts I guess

So this is a very good question. I was sure using dts was right for a very long
time, but have now changed my mind and believe pts is the right answer. Let me
try to explain why:

Expected (rtx) timers are all about sending a rtx-request for a packet that has
not arrived (yet). If you imagine a "network" (upstream) and a "decoder"
(downstream) side of the jitterbuffer (JB), the idea is that the network is
chaotic, and the job of the JB is to create order for the decoder. Now,
complete chaos in when we are receiving packets should not really affect when
the decoder needs those packets. Extreme jitter and reordering of packets on
the network side should not change the timing of when those buffers needs to be
presented to the decoder. Hence, the expected time of those packets should be
driven by their need for being presented in time, and not by when the happen to
arrive from the wire.


> 
> @@ +2832,3 @@
>  
> +  /* calculate a pts based on rtptime and arrival time (dts) */
> +  pts = rtp_jitter_buffer_calculate_pts (priv->jbuf, dts, rtptime,
> 
> Calculating the PTS changes jbuf's state... is this going to cause problems
> if we later drop this specific packet? Previously we would only change the
> state once the packet is definitely inserted.

Very good question. I was worried about this too, but have not found any cases
(practical or theoretical) where it is a problem. All existing tests are
passing, but I am very happy to help write up a new test-case to prove me wrong
though.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.