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
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 25f0f2e..08fc605 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 @@
~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 0d71c26..ed356b4 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() {}
-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 63cedf0..a320a85 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 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 @@
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 a19c4b0..cb57466 100644
--- a/webrtc/video_engine/vie_receiver.cc
+++ b/webrtc/video_engine/vie_receiver.cc
@@ -338,7 +338,21 @@
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 1d23afd..c52fd98 100644
--- a/webrtc/voice_engine/channel.cc
+++ b/webrtc/voice_engine/channel.cc
@@ -1898,8 +1898,20 @@
{
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;
}
