ProbingEstimator: Erase history based on time threshold

webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc

Index: webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc
diff --git a/webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc b/webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc
index e3bbd5a22793e2d8a73c0a277ae02f9fc036acb9..1fe6f2223ac958a39ea2ff295716ebb96aebb4be 100644
--- a/webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc
+++ b/webrtc/modules/congestion_controller/probe_bitrate_estimator_unittest.cc
@@ -15,6 +15,7 @@
 
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "webrtc/base/logging.h"

[danilchap, 2016/08/19 11:46:30]

is it needed?

[Irfan, 2016/08/22 18:16:30]

Done.

 #include "webrtc/modules/remote_bitrate_estimator/aimd_rate_control.h"
 
 namespace webrtc {
@@ -29,19 +30,12 @@ class TestProbeBitrateEstimator : public ::testing::Test {
                          int64_t arrival_time_ms) {
     PacketInfo info(arrival_time_ms, send_time_ms, 0, size_bytes,
                     probe_cluster_id);
-    ProbingResult res = probe_bitrate_estimator_.PacketFeedback(info);
-    if (res.valid())
-      results_.emplace_back(res.bps, res.timestamp);
-  }
-
-  void CheckResult(size_t index, int bps, int max_diff, int64_t timestamp) {
-    ASSERT_GT(results_.size(), index);
-    EXPECT_NEAR(results_[index].first, bps, max_diff);
-    EXPECT_EQ(results_[index].second, timestamp);
+    int bps = probe_bitrate_estimator_.HandleProbeAndEstimateBitrate(info, 1);
+    measured_bps_ = bps > 0 ? bps : 0;

[danilchap, 2016/08/19 11:46:30]

is it needed to check if bps > 0?
Shouldn't last AddPacketFeedback in all tests collect valid bitrate?
May be cleaner to remove measured_bps_ and make AddPacketFeedback return bps
instead.

[Irfan, 2016/08/22 18:16:30]

Not necessarily. The HandleProbeAndEstimateBitrate() returns a valid bitrate if
the given probe completes a valid cluster and -1 otherwise.

   }
 
  protected:
-  std::vector<std::pair<int, int64_t>> results_;
+  int measured_bps_ = 0;
   ProbeBitrateEstimator probe_bitrate_estimator_;
 };
 
@@ -51,7 +45,7 @@ TEST_F(TestProbeBitrateEstimator, OneCluster) {
   AddPacketFeedback(0, 1000, 20, 30);
   AddPacketFeedback(0, 1000, 30, 40);
 
-  CheckResult(0, 800000, 10, 40);
+  EXPECT_NEAR(measured_bps_, 800000, 10);
 }
 
 TEST_F(TestProbeBitrateEstimator, FastReceive) {
@@ -60,7 +54,7 @@ TEST_F(TestProbeBitrateEstimator, FastReceive) {
   AddPacketFeedback(0, 1000, 20, 35);
   AddPacketFeedback(0, 1000, 30, 40);
 
-  CheckResult(0, 800000, 10, 40);
+  EXPECT_NEAR(measured_bps_, 800000, 10);
 }
 
 TEST_F(TestProbeBitrateEstimator, TooFastReceive) {
@@ -69,7 +63,7 @@ TEST_F(TestProbeBitrateEstimator, TooFastReceive) {
   AddPacketFeedback(0, 1000, 20, 40);
   AddPacketFeedback(0, 1000, 40, 50);
 
-  EXPECT_TRUE(results_.empty());
+  EXPECT_EQ(measured_bps_, 0);
 }
 
 TEST_F(TestProbeBitrateEstimator, SlowReceive) {
@@ -78,7 +72,7 @@ TEST_F(TestProbeBitrateEstimator, SlowReceive) {
   AddPacketFeedback(0, 1000, 20, 70);
   AddPacketFeedback(0, 1000, 30, 85);
 
-  CheckResult(0, 320000, 10, 85);
+  EXPECT_NEAR(measured_bps_, 320000, 10);
 }
 
 TEST_F(TestProbeBitrateEstimator, BurstReceive) {
@@ -87,7 +81,7 @@ TEST_F(TestProbeBitrateEstimator, BurstReceive) {
   AddPacketFeedback(0, 1000, 20, 50);
   AddPacketFeedback(0, 1000, 40, 50);
 
-  EXPECT_TRUE(results_.empty());
+  EXPECT_EQ(measured_bps_, 0);
 }
 
 TEST_F(TestProbeBitrateEstimator, MultipleClusters) {
@@ -95,20 +89,22 @@ TEST_F(TestProbeBitrateEstimator, MultipleClusters) {
   AddPacketFeedback(0, 1000, 10, 20);
   AddPacketFeedback(0, 1000, 20, 30);
   AddPacketFeedback(0, 1000, 40, 60);
+
+  EXPECT_NEAR(measured_bps_, 480000, 10);
+
   AddPacketFeedback(0, 1000, 50, 60);
 
-  CheckResult(0, 480000, 10, 60);
-  CheckResult(1, 640000, 10, 60);
+  EXPECT_NEAR(measured_bps_, 640000, 10);
 
   AddPacketFeedback(1, 1000, 60, 70);
   AddPacketFeedback(1, 1000, 65, 77);
   AddPacketFeedback(1, 1000, 70, 84);
   AddPacketFeedback(1, 1000, 75, 90);
 
-  CheckResult(2, 1200000, 10, 90);
+  EXPECT_NEAR(measured_bps_, 1200000, 10);
 }
 
-TEST_F(TestProbeBitrateEstimator, OldProbe) {
+TEST_F(TestProbeBitrateEstimator, IgnoreOldClusters) {
   AddPacketFeedback(0, 1000, 0, 10);
   AddPacketFeedback(0, 1000, 10, 20);
   AddPacketFeedback(0, 1000, 20, 30);
@@ -118,11 +114,12 @@ TEST_F(TestProbeBitrateEstimator, OldProbe) {
   AddPacketFeedback(1, 1000, 70, 84);
   AddPacketFeedback(1, 1000, 75, 90);
 
-  CheckResult(0, 1200000, 10, 90);
+  EXPECT_NEAR(measured_bps_, 1200000, 10);
 
-  AddPacketFeedback(0, 1000, 40, 60);
+  // Coming in 6s later
+  AddPacketFeedback(0, 1000, 40 + 6000, 60 + 6000);
 
-  EXPECT_EQ(1ul, results_.size());
+  EXPECT_EQ(measured_bps_, 0);
 }
 
 }  // namespace webrtc