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/webrtc@7407 4adac7df-926f-26a2-2b94-8c16560cd09d

5 files changed, 49 insertions, 52 deletions

diff --git a/modules/rtp_rtcp/interface/remote_ntp_time_estimator.h b/modules/rtp_rtcp/interface/remote_ntp_time_estimator.h
index 25f0f2ec..08fc605a 100644
--- a/modules/rtp_rtcp/interface/remote_ntp_time_estimator.h
+++ b/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/modules/rtp_rtcp/source/remote_ntp_time_estimator.cc b/modules/rtp_rtcp/source/remote_ntp_time_estimator.cc
index 0d71c26b..ed356b4c 100644
--- a/modules/rtp_rtcp/source/remote_ntp_time_estimator.cc
+++ b/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/modules/rtp_rtcp/source/remote_ntp_time_estimator_unittest.cc b/modules/rtp_rtcp/source/remote_ntp_time_estimator_unittest.cc
index 63cedf03..a320a857 100644
--- a/modules/rtp_rtcp/source/remote_ntp_time_estimator_unittest.cc
+++ b/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/video_engine/vie_receiver.cc b/video_engine/vie_receiver.cc
index a19c4b05..cb574665 100644
--- a/video_engine/vie_receiver.cc
+++ b/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/voice_engine/channel.cc b/voice_engine/channel.cc
index 1d23afd4..c52fd984 100644
--- a/voice_engine/channel.cc
+++ b/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;
 }