Add clock skew estimate between sender and receiver in RemoteNtpTimeEstimator.

Bug: webrtc:11342
Change-Id: Ied155984794670ad08a663ac71f98719e96f8037
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/168223
Commit-Queue: Minyue Li <minyue@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Chen Xing <chxg@google.com>
Cr-Commit-Position: refs/heads/master@{#30474}

modules/rtp_rtcp/include/remote_ntp_time_estimator.h

@@ -13,6 +13,7 @@
 
 #include <stdint.h>
 
+#include "absl/types/optional.h"
 #include "rtc_base/constructor_magic.h"
 #include "rtc_base/numerics/moving_median_filter.h"
 #include "system_wrappers/include/rtp_to_ntp_estimator.h"
@@ -32,7 +33,7 @@ class RemoteNtpTimeEstimator {
   ~RemoteNtpTimeEstimator();
 
   // Updates the estimator with round trip time |rtt|, NTP seconds |ntp_secs|,
-  // NTP fraction |ntp_frac| and RTP timestamp |rtcp_timestamp|.
+  // NTP fraction |ntp_frac| and RTP timestamp |rtp_timestamp|.
   bool UpdateRtcpTimestamp(int64_t rtt,
                            uint32_t ntp_secs,
                            uint32_t ntp_frac,
@@ -42,6 +43,10 @@ class RemoteNtpTimeEstimator {
   // Returns the NTP timestamp in ms when success. -1 if failed.
   int64_t Estimate(uint32_t rtp_timestamp);
 
+  // Estimates the offset, in milliseconds, between the remote clock and the
+  // local one. This is equal to local NTP clock - remote NTP clock.
+  absl::optional<int64_t> EstimateRemoteToLocalClockOffsetMs();
+
  private:
   Clock* clock_;
   MovingMedianFilter<int64_t> ntp_clocks_offset_estimator_;

modules/rtp_rtcp/source/remote_ntp_time_estimator.cc

@@ -12,14 +12,17 @@
 
 #include <cstdint>
 
+#include "modules/rtp_rtcp/source/time_util.h"
 #include "rtc_base/logging.h"
 #include "system_wrappers/include/clock.h"
 
 namespace webrtc {
 
 namespace {
-static const int kTimingLogIntervalMs = 10000;
+
-static const int kClocksOffsetSmoothingWindow = 100;
+constexpr int kMinimumNumberOfSamples = 2;
+constexpr int kTimingLogIntervalMs = 10000;
+constexpr int kClocksOffsetSmoothingWindow = 100;
 
 }  // namespace
 
@@ -35,9 +38,9 @@ RemoteNtpTimeEstimator::~RemoteNtpTimeEstimator() {}
 bool RemoteNtpTimeEstimator::UpdateRtcpTimestamp(int64_t rtt,
                                                  uint32_t ntp_secs,
                                                  uint32_t ntp_frac,
-                                                 uint32_t rtcp_timestamp) {
+                                                 uint32_t rtp_timestamp) {
   bool new_rtcp_sr = false;
-  if (!rtp_to_ntp_.UpdateMeasurements(ntp_secs, ntp_frac, rtcp_timestamp,
+  if (!rtp_to_ntp_.UpdateMeasurements(ntp_secs, ntp_frac, rtp_timestamp,
                                       &new_rtcp_sr)) {
     return false;
   }
@@ -47,8 +50,9 @@ bool RemoteNtpTimeEstimator::UpdateRtcpTimestamp(int64_t rtt,
   }
 
   // Update extrapolator with the new arrival time.
-  // The extrapolator assumes the TimeInMilliseconds time.
+  // The extrapolator assumes the ntp time.
-  int64_t receiver_arrival_time_ms = clock_->TimeInMilliseconds();
+  int64_t receiver_arrival_time_ms =
+      clock_->TimeInMilliseconds() + NtpOffsetMs();
   int64_t sender_send_time_ms = Clock::NtpToMs(ntp_secs, ntp_frac);
   int64_t sender_arrival_time_ms = sender_send_time_ms + rtt / 2;
   int64_t remote_to_local_clocks_offset =
@@ -65,21 +69,36 @@ int64_t RemoteNtpTimeEstimator::Estimate(uint32_t rtp_timestamp) {
 
   int64_t remote_to_local_clocks_offset =
       ntp_clocks_offset_estimator_.GetFilteredValue();
-  int64_t receiver_capture_ms =
+  int64_t receiver_capture_ntp_ms =
       sender_capture_ntp_ms + remote_to_local_clocks_offset;
+
+  // TODO(bugs.webrtc.org/11327): Clock::CurrentNtpInMilliseconds() was
+  // previously used to calculate the offset between the local and the remote
+  // clock. However, rtc::TimeMillis() + NtpOffsetMs() is now used as the local
+  // ntp clock value. To preserve the old behavior of this method, the return
+  // value is adjusted with the difference between the two local ntp clocks.
   int64_t now_ms = clock_->TimeInMilliseconds();
-  int64_t ntp_offset = clock_->CurrentNtpInMilliseconds() - now_ms;
+  int64_t offset_between_local_ntp_clocks =
-  int64_t receiver_capture_ntp_ms = receiver_capture_ms + ntp_offset;
+      clock_->CurrentNtpInMilliseconds() - now_ms - NtpOffsetMs();
+  receiver_capture_ntp_ms += offset_between_local_ntp_clocks;
 
   if (now_ms - last_timing_log_ms_ > kTimingLogIntervalMs) {
     RTC_LOG(LS_INFO) << "RTP timestamp: " << rtp_timestamp
                      << " in NTP clock: " << sender_capture_ntp_ms
-                     << " estimated time in receiver clock: "
+                     << " estimated time in receiver NTP clock: "
-                     << receiver_capture_ms
+                     << receiver_capture_ntp_ms;
-                     << " converted to NTP clock: " << receiver_capture_ntp_ms;
     last_timing_log_ms_ = now_ms;
   }
   return receiver_capture_ntp_ms;
 }
 
+absl::optional<int64_t>
+RemoteNtpTimeEstimator::EstimateRemoteToLocalClockOffsetMs() {
+  if (ntp_clocks_offset_estimator_.GetNumberOfSamplesStored() <
+      kMinimumNumberOfSamples) {
+    return absl::nullopt;
+  }
+  return ntp_clocks_offset_estimator_.GetFilteredValue();
+}
+
 }  // namespace webrtc

modules/rtp_rtcp/source/remote_ntp_time_estimator_unittest.cc

@@ -9,17 +9,21 @@
  */
 
 #include "modules/rtp_rtcp/include/remote_ntp_time_estimator.h"
-
+#include "absl/types/optional.h"
+#include "modules/rtp_rtcp/source/time_util.h"
 #include "system_wrappers/include/clock.h"
+#include "system_wrappers/include/ntp_time.h"
 #include "test/gmock.h"
 #include "test/gtest.h"
 
 namespace webrtc {
 
-static const int64_t kTestRtt = 10;
+constexpr int64_t kTestRtt = 10;
-static const int64_t kLocalClockInitialTimeMs = 123;
+constexpr int64_t kLocalClockInitialTimeMs = 123;
-static const int64_t kRemoteClockInitialTimeMs = 345;
+constexpr int64_t kRemoteClockInitialTimeMs = 345;
-static const uint32_t kTimestampOffset = 567;
+constexpr uint32_t kTimestampOffset = 567;
+constexpr int64_t kRemoteToLocalClockOffsetMs =
+    kLocalClockInitialTimeMs - kRemoteClockInitialTimeMs;
 
 class RemoteNtpTimeEstimatorTest : public ::testing::Test {
  protected:
@@ -39,9 +43,13 @@ class RemoteNtpTimeEstimatorTest : public ::testing::Test {
            kTimestampOffset;
   }
 
+  NtpTime GetRemoteNtpTime() {
+    return TimeMicrosToNtp(remote_clock_.TimeInMicroseconds());
+  }
+
   void SendRtcpSr() {
     uint32_t rtcp_timestamp = GetRemoteTimestamp();
-    NtpTime ntp = remote_clock_.CurrentNtpTime();
+    NtpTime ntp = GetRemoteNtpTime();
 
     AdvanceTimeMilliseconds(kTestRtt / 2);
     ReceiveRtcpSr(kTestRtt, rtcp_timestamp, ntp.seconds(), ntp.fractions());
@@ -53,7 +61,7 @@ class RemoteNtpTimeEstimatorTest : public ::testing::Test {
     int64_t ntp_error_fractions =
         ntp_error_ms * static_cast<int64_t>(NtpTime::kFractionsPerSecond) /
         1000;
-    NtpTime ntp(static_cast<uint64_t>(remote_clock_.CurrentNtpTime()) +
+    NtpTime ntp(static_cast<uint64_t>(GetRemoteNtpTime()) +
                 ntp_error_fractions);
     AdvanceTimeMilliseconds(kTestRtt / 2 + networking_delay_ms);
     ReceiveRtcpSr(kTestRtt, rtcp_timestamp, ntp.seconds(), ntp.fractions());
@@ -96,6 +104,7 @@ TEST_F(RemoteNtpTimeEstimatorTest, Estimate) {
   // Local peer needs at least 2 RTCP SR to calculate the capture time.
   const int64_t kNotEnoughRtcpSr = -1;
   EXPECT_EQ(kNotEnoughRtcpSr, estimator_->Estimate(rtp_timestamp));
+  EXPECT_EQ(absl::nullopt, estimator_->EstimateRemoteToLocalClockOffsetMs());
 
   AdvanceTimeMilliseconds(800);
   // Remote sends second RTCP SR.
@@ -103,36 +112,24 @@ TEST_F(RemoteNtpTimeEstimatorTest, Estimate) {
 
   // Local peer gets enough RTCP SR to calculate the capture time.
   EXPECT_EQ(capture_ntp_time_ms, estimator_->Estimate(rtp_timestamp));
+  EXPECT_EQ(kRemoteToLocalClockOffsetMs,
+            estimator_->EstimateRemoteToLocalClockOffsetMs());
 }
 
 TEST_F(RemoteNtpTimeEstimatorTest, AveragesErrorsOut) {
   // Remote peer sends first 10 RTCP SR without errors.
+  for (int i = 0; i < 10; ++i) {
     AdvanceTimeMilliseconds(1000);
     SendRtcpSr();
-  AdvanceTimeMilliseconds(1000);
+  }
-  SendRtcpSr();
-  AdvanceTimeMilliseconds(1000);
-  SendRtcpSr();
-  AdvanceTimeMilliseconds(1000);
-  SendRtcpSr();
-  AdvanceTimeMilliseconds(1000);
-  SendRtcpSr();
-  AdvanceTimeMilliseconds(1000);
-  SendRtcpSr();
-  AdvanceTimeMilliseconds(1000);
-  SendRtcpSr();
-  AdvanceTimeMilliseconds(1000);
-  SendRtcpSr();
-  AdvanceTimeMilliseconds(1000);
-  SendRtcpSr();
-  AdvanceTimeMilliseconds(1000);
-  SendRtcpSr();
 
   AdvanceTimeMilliseconds(150);
   uint32_t rtp_timestamp = GetRemoteTimestamp();
   int64_t capture_ntp_time_ms = local_clock_.CurrentNtpInMilliseconds();
   // Local peer gets enough RTCP SR to calculate the capture time.
   EXPECT_EQ(capture_ntp_time_ms, estimator_->Estimate(rtp_timestamp));
+  EXPECT_EQ(kRemoteToLocalClockOffsetMs,
+            estimator_->EstimateRemoteToLocalClockOffsetMs());
 
   // Remote sends corrupted RTCP SRs
   AdvanceTimeMilliseconds(1000);
@@ -147,6 +144,8 @@ TEST_F(RemoteNtpTimeEstimatorTest, AveragesErrorsOut) {
 
   // Errors should be averaged out.
   EXPECT_EQ(capture_ntp_time_ms, estimator_->Estimate(rtp_timestamp));
+  EXPECT_EQ(kRemoteToLocalClockOffsetMs,
+            estimator_->EstimateRemoteToLocalClockOffsetMs());
 }
 
 }  // namespace webrtc