Revert of Avoid precision loss in MedianSlopeEstimator from int64_t -> double conversion (patchset #3 id:40001 of https://codereview.webrtc.org/2578543002/ )

Reason for revert:
Multiple definitions of TestEstimator

Original issue's description:
> Pass arrival time as an int64_t rather than a double to the MedianSlopeEstimator to avoid precision loss.
>
> Also clean up the unit test.
>
> BUG=webrtc:6892
>
> Committed: https://crrev.com/ebcbcc3b2451f5c4fb07f7b37815bd54f364d057
> Cr-Commit-Position: refs/heads/master@{#15634}

TBR=brandtr@webrtc.org,stefan@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:6892

Review-Url: https://codereview.webrtc.org/2572353003
Cr-Commit-Position: refs/heads/master@{#15635}

webrtc/modules/congestion_controller/median_slope_estimator.cc

@@ -35,15 +35,15 @@ MedianSlopeEstimator::~MedianSlopeEstimator() {}
 
 void MedianSlopeEstimator::Update(double recv_delta_ms,
                                   double send_delta_ms,
-                                  int64_t arrival_time_ms) {
+                                  double now_ms) {
   const double delta_ms = recv_delta_ms - send_delta_ms;
   ++num_of_deltas_;
-  if (num_of_deltas_ > kDeltaCounterMax)
+  if (num_of_deltas_ > kDeltaCounterMax) {
     num_of_deltas_ = kDeltaCounterMax;
+  }
 
   accumulated_delay_ += delta_ms;
-  BWE_TEST_LOGGING_PLOT(1, "accumulated_delay_ms", arrival_time_ms,
-                        accumulated_delay_);
+  BWE_TEST_LOGGING_PLOT(1, "accumulated_delay_ms", now_ms, accumulated_delay_);
 
   // If the window is full, remove the |window_size_| - 1 slopes that belong to
   // the oldest point.
@@ -56,7 +56,7 @@ void MedianSlopeEstimator::Update(double recv_delta_ms,
   }
   // Add |window_size_| - 1 new slopes.
   for (auto& old_delay : delay_hist_) {
-    if (arrival_time_ms - old_delay.time != 0) {
+    if (now_ms - old_delay.time != 0) {
       // The C99 standard explicitly states that casts and assignments must
       // perform the associated conversions. This means that |slope| will be
       // a 64-bit double even if the division is computed using, e.g., 80-bit
@@ -64,21 +64,20 @@ void MedianSlopeEstimator::Update(double recv_delta_ms,
       // C++11 standard isn't as explicit. Furthermore, there are good reasons
       // to believe that compilers couldn't perform optimizations that break
       // this assumption even if they wanted to.
-      double slope = (accumulated_delay_ - old_delay.delay) /
-                     static_cast<double>(arrival_time_ms - old_delay.time);
+      double slope =
+          (accumulated_delay_ - old_delay.delay) / (now_ms - old_delay.time);
       median_filter_.Insert(slope);
       // We want to avoid issues with different rounding mode / precision
       // which we might get if we recomputed the slope when we remove it.
       old_delay.slopes.push_back(slope);
     }
   }
-  delay_hist_.emplace_back(arrival_time_ms, accumulated_delay_,
-                           window_size_ - 1);
+  delay_hist_.emplace_back(now_ms, accumulated_delay_, window_size_ - 1);
   // Recompute the median slope.
   if (delay_hist_.size() == window_size_)
     trendline_ = median_filter_.GetPercentileValue();
 
-  BWE_TEST_LOGGING_PLOT(1, "trendline_slope", arrival_time_ms, trendline_);
+  BWE_TEST_LOGGING_PLOT(1, "trendline_slope", now_ms, trendline_);
 }
 
 }  // namespace webrtc

webrtc/modules/congestion_controller/median_slope_estimator.h

@@ -10,14 +10,13 @@
 #ifndef WEBRTC_MODULES_CONGESTION_CONTROLLER_MEDIAN_SLOPE_ESTIMATOR_H_
 #define WEBRTC_MODULES_CONGESTION_CONTROLLER_MEDIAN_SLOPE_ESTIMATOR_H_
 
-#include <stddef.h>
-#include <stdint.h>
-
 #include <list>
+#include <utility>
 #include <vector>
 
 #include "webrtc/base/analytics/percentile_filter.h"
 #include "webrtc/base/constructormagic.h"
+#include "webrtc/common_types.h"
 
 namespace webrtc {
 
@@ -33,9 +32,7 @@ class MedianSlopeEstimator {
 
   // Update the estimator with a new sample. The deltas should represent deltas
   // between timestamp groups as defined by the InterArrival class.
-  void Update(double recv_delta_ms,
-              double send_delta_ms,
-              int64_t arrival_time_ms);
+  void Update(double recv_delta_ms, double send_delta_ms, double now_ms);
 
   // Returns the estimated trend k multiplied by some gain.
   // 0 < k < 1   ->  the delay increases, queues are filling up
@@ -48,11 +45,11 @@ class MedianSlopeEstimator {
 
  private:
   struct DelayInfo {
-    DelayInfo(int64_t time, double delay, size_t slope_count)
+    DelayInfo(double time, double delay, size_t slope_count)
         : time(time), delay(delay) {
       slopes.reserve(slope_count);
     }
-    int64_t time;
+    double time;
     double delay;
     std::vector<double> slopes;
   };

webrtc/modules/congestion_controller/median_slope_estimator_unittest.cc

@@ -18,55 +18,96 @@ namespace {
 constexpr size_t kWindowSize = 20;
 constexpr double kGain = 1;
 constexpr int64_t kAvgTimeBetweenPackets = 10;
-constexpr size_t kPacketCount = 2 * kWindowSize + 1;
 }  // namespace
 
-void TestEstimator(double slope, double jitter_stddev, double tolerance) {
+TEST(MedianSlopeEstimator, PerfectLineSlopeOneHalf) {
   MedianSlopeEstimator estimator(kWindowSize, kGain);
-  Random random(0x1234567);
-  int64_t send_times[kPacketCount];
-  int64_t recv_times[kPacketCount];
-  int64_t send_start_time = random.Rand(1000000);
-  int64_t recv_start_time = random.Rand(1000000);
-  for (size_t i = 0; i < kPacketCount; ++i) {
-    send_times[i] = send_start_time + i * kAvgTimeBetweenPackets;
-    double latency = i * kAvgTimeBetweenPackets / (1 - slope);
-    double jitter = random.Gaussian(0, jitter_stddev);
-    recv_times[i] = recv_start_time + latency + jitter;
-  }
-  for (size_t i = 1; i < kPacketCount; ++i) {
-    double recv_delta = recv_times[i] - recv_times[i - 1];
-    double send_delta = send_times[i] - send_times[i - 1];
-    estimator.Update(recv_delta, send_delta, recv_times[i]);
+  Random rand(0x1234567);
+  double now_ms = rand.Rand<double>() * 10000;
+  for (size_t i = 1; i < 2 * kWindowSize; i++) {
+    double send_delta = rand.Rand<double>() * 2 * kAvgTimeBetweenPackets;
+    double recv_delta = 2 * send_delta;
+    now_ms += recv_delta;
+    estimator.Update(recv_delta, send_delta, now_ms);
     if (i < kWindowSize)
       EXPECT_NEAR(estimator.trendline_slope(), 0, 0.001);
     else
-      EXPECT_NEAR(estimator.trendline_slope(), slope, tolerance);
+      EXPECT_NEAR(estimator.trendline_slope(), 0.5, 0.001);
   }
 }
 
-TEST(MedianSlopeEstimator, PerfectLineSlopeOneHalf) {
-  TestEstimator(0.5, 0, 0.001);
-}
-
 TEST(MedianSlopeEstimator, PerfectLineSlopeMinusOne) {
-  TestEstimator(-1, 0, 0.001);
+  MedianSlopeEstimator estimator(kWindowSize, kGain);
+  Random rand(0x1234567);
+  double now_ms = rand.Rand<double>() * 10000;
+  for (size_t i = 1; i < 2 * kWindowSize; i++) {
+    double send_delta = rand.Rand<double>() * 2 * kAvgTimeBetweenPackets;
+    double recv_delta = 0.5 * send_delta;
+    now_ms += recv_delta;
+    estimator.Update(recv_delta, send_delta, now_ms);
+    if (i < kWindowSize)
+      EXPECT_NEAR(estimator.trendline_slope(), 0, 0.001);
+    else
+      EXPECT_NEAR(estimator.trendline_slope(), -1, 0.001);
+  }
 }
 
 TEST(MedianSlopeEstimator, PerfectLineSlopeZero) {
-  TestEstimator(0, 0, 0.001);
+  MedianSlopeEstimator estimator(kWindowSize, kGain);
+  Random rand(0x1234567);
+  double now_ms = rand.Rand<double>() * 10000;
+  for (size_t i = 1; i < 2 * kWindowSize; i++) {
+    double send_delta = rand.Rand<double>() * 2 * kAvgTimeBetweenPackets;
+    double recv_delta = send_delta;
+    now_ms += recv_delta;
+    estimator.Update(recv_delta, send_delta, now_ms);
+    EXPECT_NEAR(estimator.trendline_slope(), 0, 0.001);
+  }
 }
 
 TEST(MedianSlopeEstimator, JitteryLineSlopeOneHalf) {
-  TestEstimator(0.5, kAvgTimeBetweenPackets / 3.0, 0.01);
+  MedianSlopeEstimator estimator(kWindowSize, kGain);
+  Random rand(0x1234567);
+  double now_ms = rand.Rand<double>() * 10000;
+  for (size_t i = 1; i < 2 * kWindowSize; i++) {
+    double send_delta = rand.Rand<double>() * 2 * kAvgTimeBetweenPackets;
+    double recv_delta = 2 * send_delta + rand.Gaussian(0, send_delta / 3);
+    now_ms += recv_delta;
+    estimator.Update(recv_delta, send_delta, now_ms);
+    if (i < kWindowSize)
+      EXPECT_NEAR(estimator.trendline_slope(), 0, 0.001);
+    else
+      EXPECT_NEAR(estimator.trendline_slope(), 0.5, 0.1);
+  }
 }
 
 TEST(MedianSlopeEstimator, JitteryLineSlopeMinusOne) {
-  TestEstimator(-1, kAvgTimeBetweenPackets / 3.0, 0.05);
+  MedianSlopeEstimator estimator(kWindowSize, kGain);
+  Random rand(0x1234567);
+  double now_ms = rand.Rand<double>() * 10000;
+  for (size_t i = 1; i < 2 * kWindowSize; i++) {
+    double send_delta = rand.Rand<double>() * 2 * kAvgTimeBetweenPackets;
+    double recv_delta = 0.5 * send_delta + rand.Gaussian(0, send_delta / 20);
+    now_ms += recv_delta;
+    estimator.Update(recv_delta, send_delta, now_ms);
+    if (i < kWindowSize)
+      EXPECT_NEAR(estimator.trendline_slope(), 0, 0.001);
+    else
+      EXPECT_NEAR(estimator.trendline_slope(), -1, 0.1);
+  }
 }
 
 TEST(MedianSlopeEstimator, JitteryLineSlopeZero) {
-  TestEstimator(0, kAvgTimeBetweenPackets / 3.0, 0.02);
+  MedianSlopeEstimator estimator(kWindowSize, kGain);
+  Random rand(0x1234567);
+  double now_ms = rand.Rand<double>() * 10000;
+  for (size_t i = 1; i < 2 * kWindowSize; i++) {
+    double send_delta = rand.Rand<double>() * 2 * kAvgTimeBetweenPackets;
+    double recv_delta = send_delta + rand.Gaussian(0, send_delta / 5);
+    now_ms += recv_delta;
+    estimator.Update(recv_delta, send_delta, now_ms);
+    EXPECT_NEAR(estimator.trendline_slope(), 0, 0.1);
+  }
 }
 
 }  // namespace webrtc