Remove WebRTC-ClockEstimation experiment and make new clock estimation always enabled

Bug: webrtc:8468
Change-Id: Id9feb8e2c015f0a895a093d20caedae4a8b1337e
Reviewed-on: https://webrtc-review.googlesource.com/29161
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21075}

modules/rtp_rtcp/include/remote_ntp_time_estimator.h

@@ -20,7 +20,6 @@
 namespace webrtc {
 
 class Clock;
-class TimestampExtrapolator;
 
 // RemoteNtpTimeEstimator can be used to estimate a given RTP timestamp's NTP
 // time in local timebase.
@@ -43,11 +42,9 @@ class RemoteNtpTimeEstimator {
 
  private:
   Clock* clock_;
-  std::unique_ptr<TimestampExtrapolator> ts_extrapolator_;
   MovingMedianFilter<int64_t> ntp_clocks_offset_estimator_;
   RtpToNtpEstimator rtp_to_ntp_;
   int64_t last_timing_log_ms_;
-  const bool is_experiment_enabled_;
   RTC_DISALLOW_COPY_AND_ASSIGN(RemoteNtpTimeEstimator);
 };
 

modules/rtp_rtcp/source/remote_ntp_time_estimator.cc

@@ -12,8 +12,6 @@
 
 #include "rtc_base/logging.h"
 #include "system_wrappers/include/clock.h"
-#include "system_wrappers/include/field_trial.h"
-#include "system_wrappers/include/timestamp_extrapolator.h"
 
 namespace webrtc {
 
@@ -21,19 +19,14 @@ namespace {
 static const int kTimingLogIntervalMs = 10000;
 static const int kClocksOffsetSmoothingWindow = 100;
 
-bool IsClockEstimationExperimentEnabled() {
-  return webrtc::field_trial::IsEnabled("WebRTC-ClockEstimation");
-}
 }  // namespace
 
 // TODO(wu): Refactor this class so that it can be shared with
 // vie_sync_module.cc.
 RemoteNtpTimeEstimator::RemoteNtpTimeEstimator(Clock* clock)
     : clock_(clock),
-      ts_extrapolator_(new TimestampExtrapolator(clock_->TimeInMilliseconds())),
       ntp_clocks_offset_estimator_(kClocksOffsetSmoothingWindow),
-      last_timing_log_ms_(-1),
-      is_experiment_enabled_(IsClockEstimationExperimentEnabled()) {}
+      last_timing_log_ms_(-1) {}
 
 RemoteNtpTimeEstimator::~RemoteNtpTimeEstimator() {}
 
@@ -55,9 +48,6 @@ bool RemoteNtpTimeEstimator::UpdateRtcpTimestamp(int64_t rtt,
   // The extrapolator assumes the TimeInMilliseconds time.
   int64_t receiver_arrival_time_ms = clock_->TimeInMilliseconds();
   int64_t sender_send_time_ms = Clock::NtpToMs(ntp_secs, ntp_frac);
-  int64_t sender_arrival_time_90k = (sender_send_time_ms + rtt / 2) * 90;
-  ts_extrapolator_->Update(receiver_arrival_time_ms, sender_arrival_time_90k);
-
   int64_t sender_arrival_time_ms = sender_send_time_ms + rtt / 2;
   int64_t remote_to_local_clocks_offset =
       receiver_arrival_time_ms - sender_arrival_time_ms;
@@ -71,16 +61,10 @@ int64_t RemoteNtpTimeEstimator::Estimate(uint32_t rtp_timestamp) {
     return -1;
   }
 
-  int64_t receiver_capture_ms;
-
-  if (is_experiment_enabled_) {
-    int64_t remote_to_local_clocks_offset =
-        ntp_clocks_offset_estimator_.GetFilteredValue();
-    receiver_capture_ms = sender_capture_ntp_ms + remote_to_local_clocks_offset;
-  } else {
-    uint32_t timestamp = sender_capture_ntp_ms * 90;
-    receiver_capture_ms = ts_extrapolator_->ExtrapolateLocalTime(timestamp);
-  }
+  int64_t remote_to_local_clocks_offset =
+      ntp_clocks_offset_estimator_.GetFilteredValue();
+  int64_t receiver_capture_ms =
+      sender_capture_ntp_ms + remote_to_local_clocks_offset;
   int64_t now_ms = clock_->TimeInMilliseconds();
   int64_t ntp_offset = clock_->CurrentNtpInMilliseconds() - now_ms;
   int64_t receiver_capture_ntp_ms = receiver_capture_ms + ntp_offset;

modules/rtp_rtcp/source/remote_ntp_time_estimator_unittest.cc

@@ -11,7 +11,6 @@
 #include "modules/rtp_rtcp/include/remote_ntp_time_estimator.h"
 #include "common_types.h"  // NOLINT(build/include)
 #include "system_wrappers/include/clock.h"
-#include "test/field_trial.h"
 #include "test/gmock.h"
 #include "test/gtest.h"
 
@@ -108,11 +107,6 @@ TEST_F(RemoteNtpTimeEstimatorTest, Estimate) {
 }
 
 TEST_F(RemoteNtpTimeEstimatorTest, AveragesErrorsOut) {
-  test::ScopedFieldTrials override_field_trials(
-      "WebRTC-ClockEstimation/Enabled/");
-  // Reset estimator_ because it checks experiment status during construction.
-  estimator_.reset(new RemoteNtpTimeEstimator(&local_clock_));
-
   // Remote peer sends first 5 RTCP SR without errors.
   AdvanceTimeMilliseconds(1000);
   SendRtcpSr();

system_wrappers/include/rtp_to_ntp_estimator.h

@@ -81,11 +81,9 @@ class RtpToNtpEstimator {
 
   int consecutive_invalid_samples_;
   std::list<RtcpMeasurement> measurements_;
-  Parameters params_;
   MovingMedianFilter<Parameters> smoothing_filter_;
   bool params_calculated_;
   mutable TimestampUnwrapper unwrapper_;
-  const bool is_experiment_enabled_;
 };
 }  // namespace webrtc
 

system_wrappers/source/rtp_to_ntp_estimator.cc

@@ -13,7 +13,6 @@
 #include "rtc_base/checks.h"
 #include "rtc_base/logging.h"
 #include "system_wrappers/include/clock.h"
-#include "system_wrappers/include/field_trial.h"
 
 namespace webrtc {
 namespace {
@@ -22,9 +21,6 @@ const size_t kNumRtcpReportsToUse = 2;
 // Number of parameters samples used to smooth.
 const size_t kNumSamplesToSmooth = 20;
 
-bool IsClockEstimationExperimentEnabled() {
-  return webrtc::field_trial::IsEnabled("WebRTC-ClockEstimation");
-}
 
 // Calculates the RTP timestamp frequency from two pairs of NTP/RTP timestamps.
 bool CalculateFrequency(int64_t ntp_ms1,
@@ -90,8 +86,7 @@ bool RtpToNtpEstimator::RtcpMeasurement::IsEqual(
 RtpToNtpEstimator::RtpToNtpEstimator()
     : consecutive_invalid_samples_(0),
       smoothing_filter_(kNumSamplesToSmooth),
-      params_calculated_(false),
-      is_experiment_enabled_(IsClockEstimationExperimentEnabled()) {}
+      params_calculated_(false) {}
 
 RtpToNtpEstimator::~RtpToNtpEstimator() {}
 
@@ -112,11 +107,7 @@ void RtpToNtpEstimator::UpdateParameters() {
   }
   params.offset_ms = timestamp_new - params.frequency_khz * ntp_ms_new;
   params_calculated_ = true;
-  if (is_experiment_enabled_) {
-    smoothing_filter_.Insert(params);
-  } else {
-    params_ = params;
-  }
+  smoothing_filter_.Insert(params);
 }
 
 bool RtpToNtpEstimator::UpdateMeasurements(uint32_t ntp_secs,
@@ -186,8 +177,7 @@ bool RtpToNtpEstimator::Estimate(int64_t rtp_timestamp,
 
   int64_t rtp_timestamp_unwrapped = unwrapper_.Unwrap(rtp_timestamp);
 
-  Parameters params =
-      is_experiment_enabled_ ? smoothing_filter_.GetFilteredValue() : params_;
+  Parameters params = smoothing_filter_.GetFilteredValue();
 
   // params_calculated_ should not be true unless ms params.frequency_khz has
   // been calculated to something non zero.
@@ -208,8 +198,7 @@ const rtc::Optional<RtpToNtpEstimator::Parameters> RtpToNtpEstimator::params()
     const {
   rtc::Optional<Parameters> res;
   if (params_calculated_) {
-    res.emplace(is_experiment_enabled_ ? smoothing_filter_.GetFilteredValue()
-                                       : params_);
+    res.emplace(smoothing_filter_.GetFilteredValue());
   }
   return res;
 }