Improve the behavior when the BWE times out and when we have too little data to determine the incoming bitrate.

This is done by changing the RateStatistics so that it resets its window when the accumulator is empty. It also keeps a dynamic window, so that the rates computed before a full window worth of data has been received will be computed over a smaller window. This means that the rate will be closer to the true rate, but with a higher variance.

BUG=webrtc:5773
R=perkj@webrtc.org, sprang@webrtc.org

Review URL: https://codereview.webrtc.org/1908893003 .

Cr-Commit-Position: refs/heads/master@{#12470}

webrtc/base/OWNERS

@@ -15,4 +15,6 @@ per-file *.gyp=*
 per-file *.gypi=*
 
 per-file BUILD.gn=kjellander@webrtc.org
+per-file rate_statistics*=sprang@webrtc.org
+per-file rate_statistics*=stefan@webrtc.org
 

webrtc/base/rate_statistics.cc

@@ -10,7 +10,9 @@
 
 #include "webrtc/base/rate_statistics.h"
 
-#include <assert.h>
+#include <algorithm>
+
+#include "webrtc/base/checks.h"
 
 namespace webrtc {
 
@@ -20,7 +22,7 @@ RateStatistics::RateStatistics(uint32_t window_size_ms, float scale)
       accumulated_count_(0),
       oldest_time_(0),
       oldest_index_(0),
-      scale_(scale / (num_buckets_ - 1)) {}
+      scale_(scale) {}
 
 RateStatistics::~RateStatistics() {}
 
@@ -42,7 +44,7 @@ void RateStatistics::Update(size_t count, int64_t now_ms) {
   EraseOld(now_ms);
 
   int now_offset = static_cast<int>(now_ms - oldest_time_);
-  assert(now_offset < num_buckets_);
+  RTC_DCHECK_LT(now_offset, num_buckets_);
   int index = oldest_index_ + now_offset;
   if (index >= num_buckets_) {
     index -= num_buckets_;
@@ -53,18 +55,20 @@ void RateStatistics::Update(size_t count, int64_t now_ms) {
 
 uint32_t RateStatistics::Rate(int64_t now_ms) {
   EraseOld(now_ms);
-  return static_cast<uint32_t>(accumulated_count_ * scale_ + 0.5f);
+  float scale =  scale_ / (now_ms - oldest_time_ + 1);
+  return static_cast<uint32_t>(accumulated_count_ * scale + 0.5f);
 }
 
 void RateStatistics::EraseOld(int64_t now_ms) {
   int64_t new_oldest_time = now_ms - num_buckets_ + 1;
   if (new_oldest_time <= oldest_time_) {
+    if (accumulated_count_ == 0)
+      oldest_time_ = now_ms;
     return;
   }
-
   while (oldest_time_ < new_oldest_time) {
     size_t count_in_oldest_bucket = buckets_[oldest_index_];
-    assert(accumulated_count_ >= count_in_oldest_bucket);
+    RTC_DCHECK_GE(accumulated_count_, count_in_oldest_bucket);
     accumulated_count_ -= count_in_oldest_bucket;
     buckets_[oldest_index_] = 0;
     if (++oldest_index_ >= num_buckets_) {
@@ -74,6 +78,7 @@ void RateStatistics::EraseOld(int64_t now_ms) {
     if (accumulated_count_ == 0) {
       // This guarantees we go through all the buckets at most once, even if
       // |new_oldest_time| is far greater than |oldest_time_|.
+      new_oldest_time = now_ms;
       break;
     }
   }

webrtc/base/rate_statistics_unittest.cc

@@ -8,6 +8,8 @@
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
+#include <algorithm>
+
 #include "testing/gtest/include/gtest/gtest.h"
 #include "webrtc/base/rate_statistics.h"
 
@@ -15,9 +17,11 @@ namespace {
 
 using webrtc::RateStatistics;
 
+const int64_t kWindowMs = 500;
+
 class RateStatisticsTest : public ::testing::Test {
  protected:
-  RateStatisticsTest() : stats_(500, 8000) {}
+  RateStatisticsTest() : stats_(kWindowMs, 8000) {}
   RateStatistics stats_;
 };
 
@@ -26,8 +30,9 @@ TEST_F(RateStatisticsTest, TestStrictMode) {
   // Should be initialized to 0.
   EXPECT_EQ(0u, stats_.Rate(now_ms));
   stats_.Update(1500, now_ms);
-  // Expecting 24 kbps given a 500 ms window with one 1500 bytes packet.
+  // Expecting 1200 kbps since the window is initially kept small and grows as
-  EXPECT_EQ(24000u, stats_.Rate(now_ms));
+  // we have more data.
+  EXPECT_EQ(12000000u, stats_.Rate(now_ms));
   stats_.Reset();
   // Expecting 0 after init.
   EXPECT_EQ(0u, stats_.Rate(now_ms));
@@ -37,12 +42,12 @@ TEST_F(RateStatisticsTest, TestStrictMode) {
     }
     // Approximately 1200 kbps expected. Not exact since when packets
     // are removed we will jump 10 ms to the next packet.
-    if (now_ms > 0 && now_ms % 500 == 0) {
+    if (now_ms > 0 && now_ms % kWindowMs == 0) {
-      EXPECT_NEAR(1200000u, stats_.Rate(now_ms), 24000u);
+      EXPECT_NEAR(1200000u, stats_.Rate(now_ms), 22000u);
     }
     now_ms += 1;
   }
-  now_ms += 500;
+  now_ms += kWindowMs;
   // The window is 2 seconds. If nothing has been received for that time
   // the estimate should be 0.
   EXPECT_EQ(0u, stats_.Rate(now_ms));
@@ -54,25 +59,26 @@ TEST_F(RateStatisticsTest, IncreasingThenDecreasingBitrate) {
   // Expecting 0 after init.
   uint32_t bitrate = stats_.Rate(now_ms);
   EXPECT_EQ(0u, bitrate);
+  const uint32_t kExpectedBitrate = 8000000;
   // 1000 bytes per millisecond until plateau is reached.
+  int prev_error = kExpectedBitrate;
   while (++now_ms < 10000) {
     stats_.Update(1000, now_ms);
-    uint32_t new_bitrate = stats_.Rate(now_ms);
+    bitrate = stats_.Rate(now_ms);
-    if (new_bitrate != bitrate) {
+    int error = kExpectedBitrate - bitrate;
-      // New bitrate must be higher than previous one.
+    error = std::abs(error);
-      EXPECT_GT(new_bitrate, bitrate);
+    // Expect the estimation error to decrease as the window is extended.
-    } else {
+    EXPECT_LE(error, prev_error + 1);
-      // Plateau reached, 8000 kbps expected.
+    prev_error = error;
-      EXPECT_NEAR(8000000u, bitrate, 80000u);
-      break;
-    }
-    bitrate = new_bitrate;
   }
+  // Window filled, expect to be close to 8000000.
+  EXPECT_EQ(kExpectedBitrate, bitrate);
+
   // 1000 bytes per millisecond until 10-second mark, 8000 kbps expected.
   while (++now_ms < 10000) {
     stats_.Update(1000, now_ms);
     bitrate = stats_.Rate(now_ms);
-    EXPECT_NEAR(8000000u, bitrate, 80000u);
+    EXPECT_EQ(kExpectedBitrate, bitrate);
   }
   // Zero bytes per millisecond until 0 is reached.
   while (++now_ms < 20000) {
@@ -94,4 +100,33 @@ TEST_F(RateStatisticsTest, IncreasingThenDecreasingBitrate) {
     EXPECT_EQ(0u, stats_.Rate(now_ms));
   }
 }
+
+TEST_F(RateStatisticsTest, ResetAfterSilence) {
+  int64_t now_ms = 0;
+  stats_.Reset();
+  // Expecting 0 after init.
+  uint32_t bitrate = stats_.Rate(now_ms);
+  EXPECT_EQ(0u, bitrate);
+  const uint32_t kExpectedBitrate = 8000000;
+  // 1000 bytes per millisecond until the window has been filled.
+  int prev_error = kExpectedBitrate;
+  while (++now_ms < 10000) {
+    stats_.Update(1000, now_ms);
+    bitrate = stats_.Rate(now_ms);
+    int error = kExpectedBitrate - bitrate;
+    error = std::abs(error);
+    // Expect the estimation error to decrease as the window is extended.
+    EXPECT_LE(error, prev_error + 1);
+    prev_error = error;
+  }
+  // Window filled, expect to be close to 8000000.
+  EXPECT_EQ(kExpectedBitrate, bitrate);
+
+  now_ms += kWindowMs + 1;
+  EXPECT_EQ(0u, stats_.Rate(now_ms));
+  stats_.Update(1000, now_ms);
+  // We expect one sample of 1000 bytes, and that the bitrate is measured over
+  // 1 ms, i.e., 8 * 1000 / 0.001 = 8000000.
+  EXPECT_EQ(kExpectedBitrate, stats_.Rate(now_ms));
+}
 }  // namespace

webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc

@@ -176,8 +176,7 @@ RemoteBitrateEstimatorAbsSendTime::ProcessClusters(int64_t now_ms) {
         std::min(best_it->GetSendBitrateBps(), best_it->GetRecvBitrateBps());
     // Make sure that a probe sent on a lower bitrate than our estimate can't
     // reduce the estimate.
-    if (IsBitrateImproving(probe_bitrate_bps) &&
+    if (IsBitrateImproving(probe_bitrate_bps)) {
-        probe_bitrate_bps > static_cast<int>(incoming_bitrate_.Rate(now_ms))) {
       LOG(LS_INFO) << "Probe successful, sent at "
                    << best_it->GetSendBitrateBps() << " bps, received at "
                    << best_it->GetRecvBitrateBps()

webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time_unittest.cc

@@ -35,15 +35,15 @@ TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, RateIncreaseReordering) {
 }
 
 TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, RateIncreaseRtpTimestamps) {
-  RateIncreaseRtpTimestampsTestHelper(1232);
+  RateIncreaseRtpTimestampsTestHelper(1229);
 }
 
 TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, CapacityDropOneStream) {
-  CapacityDropTestHelper(1, false, 633);
+  CapacityDropTestHelper(1, false, 667);
 }
 
 TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, CapacityDropOneStreamWrap) {
-  CapacityDropTestHelper(1, true, 633);
+  CapacityDropTestHelper(1, true, 667);
 }
 
 TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, CapacityDropTwoStreamsWrap) {

webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream_unittest.cc

@@ -47,7 +47,7 @@ TEST_F(RemoteBitrateEstimatorSingleTest, CapacityDropOneStreamWrap) {
 }
 
 TEST_F(RemoteBitrateEstimatorSingleTest, CapacityDropTwoStreamsWrap) {
-  CapacityDropTestHelper(2, true, 767);
+  CapacityDropTestHelper(2, true, 600);
 }
 
 TEST_F(RemoteBitrateEstimatorSingleTest, CapacityDropThreeStreamsWrap) {

webrtc/modules/video_coding/bitrate_adjuster_unittest.cc

@@ -86,7 +86,7 @@ TEST_F(BitrateAdjusterTest, VaryingBitrates) {
   SimulateBitrateBps(actual_bitrate_bps);
   VerifyAdjustment();
   adjusted_bitrate_bps = adjuster_.GetAdjustedBitrateBps();
-  EXPECT_LT(adjusted_bitrate_bps, last_adjusted_bitrate_bps);
+  EXPECT_LE(adjusted_bitrate_bps, last_adjusted_bitrate_bps);
   last_adjusted_bitrate_bps = adjusted_bitrate_bps;
   // After two cycles we should've stabilized and hit the lower bound.
   EXPECT_EQ(GetTargetBitrateBpsPct(kMinAdjustedBitratePct),