[Bug 785733] rtpbin: ntp-sync does no sanity checking for clock sync

[GStreamer (GNOME Bugzilla)]

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

--- Comment #3 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Note, when I say "rendered" below this ignores the latency configured on the
pipeline. It only considers the running time of the packets at the receiver, in
relation to its clock time.


Correct me if I'm wrong, but basically the ts-offset here is the offset we have
to apply between the correct synchronization we now know via RTCP (this RTP
time was supposed to be rendered at this NTP time) and what we did before the
first RTCP packet. Before the first RTCP packet we would render it as soon as
possible, so basically at the time it arrived.

The ts-offset is always related to the very first packet that was rendered, as
that defines what our running-time/clock-time/base-time relationship is, and
the RTCP packets would tell us how to correct for the error we introduced
there.


For negative offsets that means that we rendered the first packet later than we
should have, which seems like how it is expected to be. Simply said, the first
packet is usually rendered $network_latency later than it was sent from the
sender, so we have to subtract at least the network latency (which might be
higher than 3s, especially if there are further processing steps or if the
RTP/NTP timestamp relationship in the RTCP packets is based on the capture time
instead of the sending time). Of course the receiver would have to have enough
latency configured in the pipeline to be able to handle this.


For positive offsets that would mean that we rendered the first packet earlier
than it should've been. And I think you're right here, this would mean that we
rendered a packet before it was sent (if RTCP timestamps are based on sending
time) or even captured :)


So ignoring positive offsets seems reasonable, ignoring big negative offsets
not so much. What I think should happen here is that a) positive offsets are
ignored for ntp-sync as you suggest, and b) negative offsets are ignored if
they're higher than the value set via a new property (which defaults to
"infinity" for now I guess).

What do you think?

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