From 2c0cdbce226137a8f755ae0fb51c28a335b2ea5d Mon Sep 17 00:00:00 2001 From: "minyue@webrtc.org" Date: Thu, 9 Oct 2014 10:52:43 +0000 Subject: [PATCH] Estimating NTP time with a given RTT. RemoteNtpTimeEstimator needed user to give a remote SSRC and it intended to call RtpRtcp module to obtain RTT, to be able to calculate Ntp time. When RTT cannot be directly obtained from the RtpRtcp module with the specified SSRC, RemoteNtpTimeEstimator would fail. This change allows RemoteNtpTimeEstimator to calculate NTP with an external RTT estimate. An immediate benefit is that capture_start_ntp_time_ms_ can be obtained in a Google hangout call. BUG= TEST=chromium + hangout call R=stefan@webrtc.org, xians@webrtc.org Review URL: https://webrtc-codereview.appspot.com/24879004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@7407 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../interface/remote_ntp_time_estimator.h | 8 ++--- .../source/remote_ntp_time_estimator.cc | 28 +++------------- .../remote_ntp_time_estimator_unittest.cc | 33 +++++++------------ webrtc/video_engine/vie_receiver.cc | 16 ++++++++- webrtc/voice_engine/channel.cc | 16 +++++++-- 5 files changed, 49 insertions(+), 52 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/interface/remote_ntp_time_estimator.h b/webrtc/modules/rtp_rtcp/interface/remote_ntp_time_estimator.h index 25f0f2ecf9..08fc605a13 100644 --- a/webrtc/modules/rtp_rtcp/interface/remote_ntp_time_estimator.h +++ b/webrtc/modules/rtp_rtcp/interface/remote_ntp_time_estimator.h @@ -17,7 +17,6 @@ namespace webrtc { class Clock; -class RtpRtcp; class TimestampExtrapolator; // RemoteNtpTimeEstimator can be used to estimate a given RTP timestamp's NTP @@ -30,9 +29,10 @@ class RemoteNtpTimeEstimator { ~RemoteNtpTimeEstimator(); - // Updates the estimator with the timestamp from newly received RTCP SR for - // |ssrc|. The RTCP SR is read from |rtp_rtcp|. - bool UpdateRtcpTimestamp(uint32_t ssrc, RtpRtcp* rtp_rtcp); + // Updates the estimator with round trip time |rtt|, NTP seconds |ntp_secs|, + // NTP fraction |ntp_frac| and RTP timestamp |rtcp_timestamp|. + bool UpdateRtcpTimestamp(uint16_t rtt, uint32_t ntp_secs, uint32_t ntp_frac, + uint32_t rtp_timestamp); // Estimates the NTP timestamp in local timebase from |rtp_timestamp|. // Returns the NTP timestamp in ms when success. -1 if failed. diff --git a/webrtc/modules/rtp_rtcp/source/remote_ntp_time_estimator.cc b/webrtc/modules/rtp_rtcp/source/remote_ntp_time_estimator.cc index 0d71c26b63..ed356b4c2f 100644 --- a/webrtc/modules/rtp_rtcp/source/remote_ntp_time_estimator.cc +++ b/webrtc/modules/rtp_rtcp/source/remote_ntp_time_estimator.cc @@ -10,7 +10,6 @@ #include "webrtc/modules/rtp_rtcp/interface/remote_ntp_time_estimator.h" -#include "webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h" #include "webrtc/system_wrappers/interface/clock.h" #include "webrtc/system_wrappers/interface/timestamp_extrapolator.h" @@ -26,30 +25,13 @@ RemoteNtpTimeEstimator::RemoteNtpTimeEstimator(Clock* clock) RemoteNtpTimeEstimator::~RemoteNtpTimeEstimator() {} -bool RemoteNtpTimeEstimator::UpdateRtcpTimestamp(uint32_t ssrc, - RtpRtcp* rtp_rtcp) { - assert(rtp_rtcp); - uint16_t rtt = 0; - rtp_rtcp->RTT(ssrc, &rtt, NULL, NULL, NULL); - if (rtt == 0) { - // Waiting for valid rtt. - return true; - } - // Update RTCP list - uint32_t ntp_secs = 0; - uint32_t ntp_frac = 0; - uint32_t rtp_timestamp = 0; - if (0 != rtp_rtcp->RemoteNTP(&ntp_secs, - &ntp_frac, - NULL, - NULL, - &rtp_timestamp)) { - // Waiting for RTCP. - return true; - } +bool RemoteNtpTimeEstimator::UpdateRtcpTimestamp(uint16_t rtt, + uint32_t ntp_secs, + uint32_t ntp_frac, + uint32_t rtcp_timestamp) { bool new_rtcp_sr = false; if (!UpdateRtcpList( - ntp_secs, ntp_frac, rtp_timestamp, &rtcp_list_, &new_rtcp_sr)) { + ntp_secs, ntp_frac, rtcp_timestamp, &rtcp_list_, &new_rtcp_sr)) { return false; } if (!new_rtcp_sr) { diff --git a/webrtc/modules/rtp_rtcp/source/remote_ntp_time_estimator_unittest.cc b/webrtc/modules/rtp_rtcp/source/remote_ntp_time_estimator_unittest.cc index 63cedf03af..a320a85793 100644 --- a/webrtc/modules/rtp_rtcp/source/remote_ntp_time_estimator_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/remote_ntp_time_estimator_unittest.cc @@ -12,7 +12,7 @@ #include "testing/gtest/include/gtest/gtest.h" #include "webrtc/common_types.h" #include "webrtc/modules/rtp_rtcp/interface/remote_ntp_time_estimator.h" -#include "webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h" +#include "webrtc/system_wrappers/interface/clock.h" using ::testing::_; using ::testing::DoAll; @@ -25,7 +25,6 @@ static const int kTestRtt = 10; static const int64_t kLocalClockInitialTimeMs = 123; static const int64_t kRemoteClockInitialTimeMs = 345; static const uint32_t kTimestampOffset = 567; -static const int kTestSsrc = 789; class RemoteNtpTimeEstimatorTest : public ::testing::Test { protected: @@ -52,41 +51,31 @@ class RemoteNtpTimeEstimatorTest : public ::testing::Test { remote_clock_.CurrentNtp(ntp_seconds, ntp_fractions); AdvanceTimeMilliseconds(kTestRtt / 2); - ReceiveRtcpSr(rtcp_timestamp, ntp_seconds, ntp_fractions); + ReceiveRtcpSr(kTestRtt, rtcp_timestamp, ntp_seconds, ntp_fractions); } - void UpdateRtcpTimestamp(MockRtpRtcp* rtp_rtcp, bool expected_result) { - if (rtp_rtcp) { - EXPECT_CALL(*rtp_rtcp, RTT(_, _, _, _, _)) - .WillOnce(DoAll(SetArgPointee<1>(kTestRtt), - Return(0))); - } + void UpdateRtcpTimestamp(uint16_t rtt, uint32_t ntp_secs, uint32_t ntp_frac, + uint32_t rtp_timestamp, bool expected_result) { EXPECT_EQ(expected_result, - estimator_.UpdateRtcpTimestamp(kTestSsrc, rtp_rtcp)); + estimator_.UpdateRtcpTimestamp(rtt, ntp_secs, ntp_frac, + rtp_timestamp)); } - void ReceiveRtcpSr(uint32_t rtcp_timestamp, + void ReceiveRtcpSr(uint16_t rtt, + uint32_t rtcp_timestamp, uint32_t ntp_seconds, uint32_t ntp_fractions) { - EXPECT_CALL(rtp_rtcp_, RemoteNTP(_, _, _, _, _)) - .WillOnce(DoAll(SetArgPointee<0>(ntp_seconds), - SetArgPointee<1>(ntp_fractions), - SetArgPointee<4>(rtcp_timestamp), - Return(0))); - - UpdateRtcpTimestamp(&rtp_rtcp_, true); + UpdateRtcpTimestamp(rtt, ntp_seconds, ntp_fractions, rtcp_timestamp, true); } SimulatedClock local_clock_; SimulatedClock remote_clock_; - MockRtpRtcp rtp_rtcp_; RemoteNtpTimeEstimator estimator_; }; TEST_F(RemoteNtpTimeEstimatorTest, Estimate) { - // Failed without any RTCP SR, where RemoteNTP returns without valid NTP. - EXPECT_CALL(rtp_rtcp_, RemoteNTP(_, _, _, _, _)).WillOnce(Return(0)); - UpdateRtcpTimestamp(&rtp_rtcp_, false); + // Failed without valid NTP. + UpdateRtcpTimestamp(kTestRtt, 0, 0, 0, false); AdvanceTimeMilliseconds(1000); // Remote peer sends first RTCP SR. diff --git a/webrtc/video_engine/vie_receiver.cc b/webrtc/video_engine/vie_receiver.cc index a19c4b05ea..cb574665ca 100644 --- a/webrtc/video_engine/vie_receiver.cc +++ b/webrtc/video_engine/vie_receiver.cc @@ -338,7 +338,21 @@ int ViEReceiver::InsertRTCPPacket(const uint8_t* rtcp_packet, return ret; } - ntp_estimator_->UpdateRtcpTimestamp(rtp_receiver_->SSRC(), rtp_rtcp_); + uint16_t rtt = 0; + rtp_rtcp_->RTT(rtp_receiver_->SSRC(), &rtt, NULL, NULL, NULL); + if (rtt == 0) { + // Waiting for valid rtt. + return 0; + } + uint32_t ntp_secs = 0; + uint32_t ntp_frac = 0; + uint32_t rtp_timestamp = 0; + if (0 != rtp_rtcp_->RemoteNTP(&ntp_secs, &ntp_frac, NULL, NULL, + &rtp_timestamp)) { + // Waiting for RTCP. + return 0; + } + ntp_estimator_->UpdateRtcpTimestamp(rtt, ntp_secs, ntp_frac, rtp_timestamp); return 0; } diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc index 1d23afd4b9..c52fd98419 100644 --- a/webrtc/voice_engine/channel.cc +++ b/webrtc/voice_engine/channel.cc @@ -1898,8 +1898,20 @@ int32_t Channel::ReceivedRTCPPacket(const int8_t* data, int32_t length) { { CriticalSectionScoped lock(ts_stats_lock_.get()); - ntp_estimator_.UpdateRtcpTimestamp(rtp_receiver_->SSRC(), - _rtpRtcpModule.get()); + uint16_t rtt = GetRTT(); + if (rtt == 0) { + // Waiting for valid RTT. + return 0; + } + uint32_t ntp_secs = 0; + uint32_t ntp_frac = 0; + uint32_t rtp_timestamp = 0; + if (0 != _rtpRtcpModule->RemoteNTP(&ntp_secs, &ntp_frac, NULL, NULL, + &rtp_timestamp)) { + // Waiting for RTCP. + return 0; + } + ntp_estimator_.UpdateRtcpTimestamp(rtt, ntp_secs, ntp_frac, rtp_timestamp); } return 0; }