From 7172ea13c029b56e68b0b6075034489ebe00f6c0 Mon Sep 17 00:00:00 2001 From: Ilya Nikolaevskiy Date: Mon, 30 Oct 2017 11:17:34 +0100 Subject: [PATCH] Don't use old RTCP SR reports for remote clock estimation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit At the beginning of the call, when rtt is not yet estimated, SR packets are not used for estimation. Yet, it may happen that on some non-SR RTCP packet RTT would become available. At that time an old SR will be used for remote clock estimation. This will lead to remote clock offset to the past too much. Bug: webrtc:8468 Change-Id: I1bdbd56a7bab1c28e73987e5fb307f8e7382b045 Reviewed-on: https://webrtc-review.googlesource.com/16840 Reviewed-by: Danil Chapovalov Reviewed-by: Erik Språng Commit-Queue: Ilya Nikolaevskiy Cr-Commit-Position: refs/heads/master@{#20528} --- video/rtp_video_stream_receiver.cc | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc index aecdfe3386..5778682a08 100644 --- a/video/rtp_video_stream_receiver.cc +++ b/video/rtp_video_stream_receiver.cc @@ -533,12 +533,20 @@ bool RtpVideoStreamReceiver::DeliverRtcp(const uint8_t* rtcp_packet, uint32_t ntp_secs = 0; uint32_t ntp_frac = 0; uint32_t rtp_timestamp = 0; - if (rtp_rtcp_->RemoteNTP(&ntp_secs, &ntp_frac, nullptr, nullptr, - &rtp_timestamp) != 0) { + uint32_t recieved_ntp_secs = 0; + uint32_t recieved_ntp_frac = 0; + if (rtp_rtcp_->RemoteNTP(&ntp_secs, &ntp_frac, &recieved_ntp_secs, + &recieved_ntp_frac, &rtp_timestamp) != 0) { // Waiting for RTCP. return true; } - ntp_estimator_.UpdateRtcpTimestamp(rtt, ntp_secs, ntp_frac, rtp_timestamp); + NtpTime recieved_ntp(recieved_ntp_secs, recieved_ntp_frac); + int64_t time_since_recieved = + clock_->CurrentNtpInMilliseconds() - recieved_ntp.ToMs(); + // Don't use old SRs to estimate time. + if (time_since_recieved <= 1) { + ntp_estimator_.UpdateRtcpTimestamp(rtt, ntp_secs, ntp_frac, rtp_timestamp); + } return true; }