Update RateStatistics to handle too-little-data case.

To avoid the case where a single data point or too short window is used,
causing bad behavior due to bad stats, update RateStatistics to return
an Optional rather than a plain rate.

There was also a strange off by one bug where the rate was slightly
overestimated (N + 1 buckets, N ms time window).

These changes requires updates to a number of places, and may very well
cause seeming perf regressions (but the stats were probablty more wrong
previously).

BUG=
R=mflodman@webrtc.org, stefan@webrtc.org

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

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

webrtc/base/rate_statistics.cc

@@ -16,23 +16,26 @@
 
 namespace webrtc {
 
-RateStatistics::RateStatistics(uint32_t window_size_ms, float scale)
-    : num_buckets_(window_size_ms + 1),  // N ms in (N+1) buckets.
-      buckets_(new size_t[num_buckets_]()),
+RateStatistics::RateStatistics(int64_t window_size_ms, float scale)
+    : buckets_(new Bucket[window_size_ms]()),
       accumulated_count_(0),
-      oldest_time_(0),
+      num_samples_(0),
+      oldest_time_(-window_size_ms),
       oldest_index_(0),
-      scale_(scale) {}
+      scale_(scale),
+      max_window_size_ms_(window_size_ms),
+      current_window_size_ms_(max_window_size_ms_) {}
 
 RateStatistics::~RateStatistics() {}
 
 void RateStatistics::Reset() {
   accumulated_count_ = 0;
-  oldest_time_ = 0;
+  num_samples_ = 0;
+  oldest_time_ = -max_window_size_ms_;
   oldest_index_ = 0;
-  for (int i = 0; i < num_buckets_; i++) {
-    buckets_[i] = 0;
-  }
+  current_window_size_ms_ = max_window_size_ms_;
+  for (int64_t i = 0; i < max_window_size_ms_; i++)
+    buckets_[i] = Bucket();
 }
 
 void RateStatistics::Update(size_t count, int64_t now_ms) {
@@ -43,46 +46,74 @@ void RateStatistics::Update(size_t count, int64_t now_ms) {
 
   EraseOld(now_ms);
 
-  int now_offset = static_cast<int>(now_ms - oldest_time_);
-  RTC_DCHECK_LT(now_offset, num_buckets_);
-  int index = oldest_index_ + now_offset;
-  if (index >= num_buckets_) {
-    index -= num_buckets_;
-  }
-  buckets_[index] += count;
+  // First ever sample, reset window to start now.
+  if (!IsInitialized())
+    oldest_time_ = now_ms;
+
+  uint32_t now_offset = static_cast<uint32_t>(now_ms - oldest_time_);
+  RTC_DCHECK_LT(now_offset, max_window_size_ms_);
+  uint32_t index = oldest_index_ + now_offset;
+  if (index >= max_window_size_ms_)
+    index -= max_window_size_ms_;
+  buckets_[index].sum += count;
+  ++buckets_[index].samples;
   accumulated_count_ += count;
+  ++num_samples_;
 }
 
-uint32_t RateStatistics::Rate(int64_t now_ms) {
+rtc::Optional<uint32_t> RateStatistics::Rate(int64_t now_ms) {
   EraseOld(now_ms);
-  float scale =  scale_ / (now_ms - oldest_time_ + 1);
-  return static_cast<uint32_t>(accumulated_count_ * scale + 0.5f);
+
+  // If window is a single bucket or there is only one sample in a data set that
+  // has not grown to the full window size, treat this as rate unavailable.
+  int64_t active_window_size = now_ms - oldest_time_ + 1;
+  if (num_samples_ == 0 || active_window_size <= 1 ||
+      (num_samples_ <= 1 && active_window_size < current_window_size_ms_)) {
+    return rtc::Optional<uint32_t>();
+  }
+
+  float scale = scale_ / active_window_size;
+  return rtc::Optional<uint32_t>(
+      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;
+  if (!IsInitialized())
     return;
-  }
-  while (oldest_time_ < new_oldest_time) {
-    size_t count_in_oldest_bucket = buckets_[oldest_index_];
-    RTC_DCHECK_GE(accumulated_count_, count_in_oldest_bucket);
-    accumulated_count_ -= count_in_oldest_bucket;
-    buckets_[oldest_index_] = 0;
-    if (++oldest_index_ >= num_buckets_) {
+
+  // New oldest time that is included in data set.
+  int64_t new_oldest_time = now_ms - current_window_size_ms_ + 1;
+
+  // New oldest time is older than the current one, no need to cull data.
+  if (new_oldest_time <= oldest_time_)
+    return;
+
+  // Loop over buckets and remove too old data points.
+  while (num_samples_ > 0 && oldest_time_ < new_oldest_time) {
+    const Bucket& oldest_bucket = buckets_[oldest_index_];
+    RTC_DCHECK_GE(accumulated_count_, oldest_bucket.sum);
+    RTC_DCHECK_GE(num_samples_, oldest_bucket.samples);
+    accumulated_count_ -= oldest_bucket.sum;
+    num_samples_ -= oldest_bucket.samples;
+    buckets_[oldest_index_] = Bucket();
+    if (++oldest_index_ >= max_window_size_ms_)
       oldest_index_ = 0;
-    }
     ++oldest_time_;
-    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;
-    }
   }
   oldest_time_ = new_oldest_time;
 }
 
+bool RateStatistics::SetWindowSize(int64_t window_size_ms, int64_t now_ms) {
+  if (window_size_ms <= 0 || window_size_ms > max_window_size_ms_)
+    return false;
+
+  current_window_size_ms_ = window_size_ms;
+  EraseOld(now_ms);
+  return true;
+}
+
+bool RateStatistics::IsInitialized() {
+  return oldest_time_ != -max_window_size_ms_;
+}
+
 }  // namespace webrtc

webrtc/base/rate_statistics.h

@@ -13,41 +13,56 @@
 
 #include <memory>
 
+#include "webrtc/base/optional.h"
 #include "webrtc/typedefs.h"
 
 namespace webrtc {
 
 class RateStatistics {
  public:
-  // window_size = window size in ms for the rate estimation
+  // max_window_size_ms = Maximum window size in ms for the rate estimation.
+  //                      Initial window size is set to this, but may be changed
+  //                      to something lower by calling SetWindowSize().
   // scale = coefficient to convert counts/ms to desired units,
   //         ex: if counts represents bytes, use 8*1000 to go to bits/s
-  RateStatistics(uint32_t window_size_ms, float scale);
+  RateStatistics(int64_t max_window_size_ms, float scale);
   ~RateStatistics();
 
   void Reset();
   void Update(size_t count, int64_t now_ms);
-  uint32_t Rate(int64_t now_ms);
+  rtc::Optional<uint32_t> Rate(int64_t now_ms);
+  bool SetWindowSize(int64_t window_size_ms, int64_t now_ms);
 
  private:
   void EraseOld(int64_t now_ms);
+  bool IsInitialized();
 
   // Counters are kept in buckets (circular buffer), with one bucket
   // per millisecond.
-  const int num_buckets_;
-  std::unique_ptr<size_t[]> buckets_;
+  struct Bucket {
+    size_t sum;      // Sum of all samples in this bucket.
+    size_t samples;  // Number of samples in this bucket.
+  };
+  std::unique_ptr<Bucket[]> buckets_;
 
   // Total count recorded in buckets.
   size_t accumulated_count_;
 
+  // The total number of samples in the buckets.
+  size_t num_samples_;
+
   // Oldest time recorded in buckets.
   int64_t oldest_time_;
 
   // Bucket index of oldest counter recorded in buckets.
-  int oldest_index_;
+  uint32_t oldest_index_;
 
   // To convert counts/ms to desired units
   const float scale_;
+
+  // The window sizes, in ms, over which the rate is calculated.
+  const int64_t max_window_size_ms_;
+  int64_t current_window_size_ms_;
 };
 }  // namespace webrtc
 

webrtc/base/rate_statistics_unittest.cc

@@ -27,77 +27,97 @@ class RateStatisticsTest : public ::testing::Test {
 
 TEST_F(RateStatisticsTest, TestStrictMode) {
   int64_t now_ms = 0;
-  // Should be initialized to 0.
-  EXPECT_EQ(0u, stats_.Rate(now_ms));
-  stats_.Update(1500, now_ms);
+  EXPECT_FALSE(static_cast<bool>(stats_.Rate(now_ms)));
+
+  const uint32_t kPacketSize = 1500u;
+  const uint32_t kExpectedRateBps = kPacketSize * 1000 * 8;
+
+  // Single data point is not enough for valid estimate.
+  stats_.Update(kPacketSize, now_ms++);
+  EXPECT_FALSE(static_cast<bool>(stats_.Rate(now_ms)));
+
   // Expecting 1200 kbps since the window is initially kept small and grows as
   // we have more data.
-  EXPECT_EQ(12000000u, stats_.Rate(now_ms));
+  stats_.Update(kPacketSize, now_ms);
+  EXPECT_EQ(kExpectedRateBps, *stats_.Rate(now_ms));
+
   stats_.Reset();
   // Expecting 0 after init.
-  EXPECT_EQ(0u, stats_.Rate(now_ms));
+  EXPECT_FALSE(static_cast<bool>(stats_.Rate(now_ms)));
+
+  const int kInterval = 10;
   for (int i = 0; i < 100000; ++i) {
-    if (now_ms % 10 == 0) {
-      stats_.Update(1500, now_ms);
-    }
+    if (i % kInterval == 0)
+      stats_.Update(kPacketSize, now_ms);
+
     // 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 % kWindowMs == 0) {
-      EXPECT_NEAR(1200000u, stats_.Rate(now_ms), 22000u);
+    if (i > kInterval) {
+      rtc::Optional<uint32_t> rate = stats_.Rate(now_ms);
+      EXPECT_TRUE(static_cast<bool>(rate));
+      uint32_t samples = i / kInterval + 1;
+      uint64_t total_bits = samples * kPacketSize * 8;
+      uint32_t rate_bps = static_cast<uint32_t>((1000 * total_bits) / (i + 1));
+      EXPECT_NEAR(rate_bps, *rate, 22000u);
     }
     now_ms += 1;
   }
   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));
+  EXPECT_FALSE(static_cast<bool>(stats_.Rate(now_ms)));
 }
 
 TEST_F(RateStatisticsTest, IncreasingThenDecreasingBitrate) {
   int64_t now_ms = 0;
   stats_.Reset();
   // Expecting 0 after init.
-  uint32_t bitrate = stats_.Rate(now_ms);
-  EXPECT_EQ(0u, bitrate);
+  EXPECT_FALSE(static_cast<bool>(stats_.Rate(now_ms)));
+
+  stats_.Update(1000, ++now_ms);
   const uint32_t kExpectedBitrate = 8000000;
   // 1000 bytes per millisecond until plateau is reached.
   int prev_error = kExpectedBitrate;
+  rtc::Optional<uint32_t> bitrate;
   while (++now_ms < 10000) {
     stats_.Update(1000, now_ms);
     bitrate = stats_.Rate(now_ms);
-    int error = kExpectedBitrate - bitrate;
+    EXPECT_TRUE(static_cast<bool>(bitrate));
+    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);
+  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_EQ(kExpectedBitrate, bitrate);
+    EXPECT_EQ(kExpectedBitrate, *bitrate);
   }
+
   // Zero bytes per millisecond until 0 is reached.
   while (++now_ms < 20000) {
     stats_.Update(0, now_ms);
-    uint32_t new_bitrate = stats_.Rate(now_ms);
-    if (new_bitrate != bitrate) {
+    rtc::Optional<uint32_t> new_bitrate = stats_.Rate(now_ms);
+    if (static_cast<bool>(new_bitrate) && *new_bitrate != *bitrate) {
       // New bitrate must be lower than previous one.
-      EXPECT_LT(new_bitrate, bitrate);
+      EXPECT_LT(*new_bitrate, *bitrate);
     } else {
       // 0 kbps expected.
-      EXPECT_EQ(0u, bitrate);
+      EXPECT_EQ(0u, *new_bitrate);
       break;
     }
     bitrate = new_bitrate;
   }
+
   // Zero bytes per millisecond until 20-second mark, 0 kbps expected.
   while (++now_ms < 20000) {
     stats_.Update(0, now_ms);
-    EXPECT_EQ(0u, stats_.Rate(now_ms));
+    EXPECT_EQ(0u, *stats_.Rate(now_ms));
   }
 }
 
@@ -105,28 +125,156 @@ 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);
+  EXPECT_FALSE(static_cast<bool>(stats_.Rate(now_ms)));
+
   const uint32_t kExpectedBitrate = 8000000;
   // 1000 bytes per millisecond until the window has been filled.
   int prev_error = kExpectedBitrate;
+  rtc::Optional<uint32_t> bitrate;
   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;
+    if (bitrate) {
+      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);
+  EXPECT_EQ(kExpectedBitrate, *bitrate);
 
   now_ms += kWindowMs + 1;
-  EXPECT_EQ(0u, stats_.Rate(now_ms));
+  EXPECT_FALSE(static_cast<bool>(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));
+  ++now_ms;
+  stats_.Update(1000, now_ms);
+  // We expect two samples of 1000 bytes, and that the bitrate is measured over
+  // 500 ms, i.e. 2 * 8 * 1000 / 0.500 = 32000.
+  EXPECT_EQ(32000u, *stats_.Rate(now_ms));
+
+  // Reset, add the same samples again.
+  stats_.Reset();
+  EXPECT_FALSE(static_cast<bool>(stats_.Rate(now_ms)));
+  stats_.Update(1000, now_ms);
+  ++now_ms;
+  stats_.Update(1000, now_ms);
+  // We expect two samples of 1000 bytes, and that the bitrate is measured over
+  // 2 ms (window size has been reset) i.e. 2 * 8 * 1000 / 0.002 = 8000000.
+  EXPECT_EQ(kExpectedBitrate, *stats_.Rate(now_ms));
+}
+
+TEST_F(RateStatisticsTest, HandlesChangingWindowSize) {
+  int64_t now_ms = 0;
+  stats_.Reset();
+
+  // Sanity test window size.
+  EXPECT_TRUE(stats_.SetWindowSize(kWindowMs, now_ms));
+  EXPECT_FALSE(stats_.SetWindowSize(kWindowMs + 1, now_ms));
+  EXPECT_FALSE(stats_.SetWindowSize(0, now_ms));
+  EXPECT_TRUE(stats_.SetWindowSize(1, now_ms));
+  EXPECT_TRUE(stats_.SetWindowSize(kWindowMs, now_ms));
+
+  // Fill the buffer at a rate of 1 byte / millisecond (8 kbps).
+  const int kBatchSize = 10;
+  for (int i = 0; i <= kWindowMs; i += kBatchSize)
+    stats_.Update(kBatchSize, now_ms += kBatchSize);
+  EXPECT_EQ(static_cast<uint32_t>(8000), *stats_.Rate(now_ms));
+
+  // Halve the window size, rate should stay the same.
+  EXPECT_TRUE(stats_.SetWindowSize(kWindowMs / 2, now_ms));
+  EXPECT_EQ(static_cast<uint32_t>(8000), *stats_.Rate(now_ms));
+
+  // Double the window size again, rate should stay the same. (As the window
+  // won't actually expand until new bit and bobs fall into it.
+  EXPECT_TRUE(stats_.SetWindowSize(kWindowMs, now_ms));
+  EXPECT_EQ(static_cast<uint32_t>(8000), *stats_.Rate(now_ms));
+
+  // Fill the now empty half with bits it twice the rate.
+  for (int i = 0; i < kWindowMs / 2; i += kBatchSize)
+    stats_.Update(kBatchSize * 2, now_ms += kBatchSize);
+
+  // Rate should have increase be 50%.
+  EXPECT_EQ(static_cast<uint32_t>((8000 * 3) / 2), *stats_.Rate(now_ms));
+}
+
+TEST_F(RateStatisticsTest, RespectsWindowSizeEdges) {
+  int64_t now_ms = 0;
+  stats_.Reset();
+  // Expecting 0 after init.
+  EXPECT_FALSE(static_cast<bool>(stats_.Rate(now_ms)));
+
+  // One byte per ms, using one big sample.
+  stats_.Update(kWindowMs, now_ms);
+  now_ms += kWindowMs - 2;
+  // Shouldn't work! (Only one sample, not full window size.)
+  EXPECT_FALSE(static_cast<bool>(stats_.Rate(now_ms)));
+
+  // Window size should be full, and the single data point should be accepted.
+  ++now_ms;
+  rtc::Optional<uint32_t> bitrate = stats_.Rate(now_ms);
+  EXPECT_TRUE(static_cast<bool>(bitrate));
+  EXPECT_EQ(1000 * 8u, *bitrate);
+
+  // Add another, now we have twice the bitrate.
+  stats_.Update(kWindowMs, now_ms);
+  bitrate = stats_.Rate(now_ms);
+  EXPECT_TRUE(static_cast<bool>(bitrate));
+  EXPECT_EQ(2 * 1000 * 8u, *bitrate);
+
+  // Now that first sample should drop out...
+  now_ms += 1;
+  bitrate = stats_.Rate(now_ms);
+  EXPECT_TRUE(static_cast<bool>(bitrate));
+  EXPECT_EQ(1000 * 8u, *bitrate);
+}
+
+TEST_F(RateStatisticsTest, HandlesZeroCounts) {
+  int64_t now_ms = 0;
+  stats_.Reset();
+  // Expecting 0 after init.
+  EXPECT_FALSE(static_cast<bool>(stats_.Rate(now_ms)));
+
+  stats_.Update(kWindowMs, now_ms);
+  now_ms += kWindowMs - 1;
+  stats_.Update(0, now_ms);
+  rtc::Optional<uint32_t> bitrate = stats_.Rate(now_ms);
+  EXPECT_TRUE(static_cast<bool>(bitrate));
+  EXPECT_EQ(1000 * 8u, *bitrate);
+
+  // Move window along so first data point falls out.
+  ++now_ms;
+  bitrate = stats_.Rate(now_ms);
+  EXPECT_TRUE(static_cast<bool>(bitrate));
+  EXPECT_EQ(0u, *bitrate);
+
+  // Move window so last data point falls out.
+  now_ms += kWindowMs;
+  EXPECT_FALSE(static_cast<bool>(stats_.Rate(now_ms)));
+}
+
+TEST_F(RateStatisticsTest, HandlesQuietPeriods) {
+  int64_t now_ms = 0;
+  stats_.Reset();
+  // Expecting 0 after init.
+  EXPECT_FALSE(static_cast<bool>(stats_.Rate(now_ms)));
+
+  stats_.Update(0, now_ms);
+  now_ms += kWindowMs - 1;
+  rtc::Optional<uint32_t> bitrate = stats_.Rate(now_ms);
+  EXPECT_TRUE(static_cast<bool>(bitrate));
+  EXPECT_EQ(0u, *bitrate);
+
+  // Move window along so first data point falls out.
+  ++now_ms;
+  EXPECT_FALSE(static_cast<bool>(stats_.Rate(now_ms)));
+
+  // Move window a long way out.
+  now_ms += 2 * kWindowMs;
+  stats_.Update(0, now_ms);
+  bitrate = stats_.Rate(now_ms);
+  EXPECT_TRUE(static_cast<bool>(bitrate));
+  EXPECT_EQ(0u, *bitrate);
 }
 }  // namespace

webrtc/common_video/bitrate_adjuster.cc

@@ -70,7 +70,7 @@ uint32_t BitrateAdjuster::GetAdjustedBitrateBps() const {
   return adjusted_bitrate_bps_;
 }
 
-uint32_t BitrateAdjuster::GetEstimatedBitrateBps() {
+rtc::Optional<uint32_t> BitrateAdjuster::GetEstimatedBitrateBps() {
   rtc::CritScope cs(&crit_);
   return bitrate_tracker_.Rate(clock_->TimeInMilliseconds());
 }
@@ -121,8 +121,9 @@ void BitrateAdjuster::UpdateBitrate(uint32_t current_time_ms) {
       frames_since_last_update_ < kBitrateUpdateFrameInterval) {
     return;
   }
-  float estimated_bitrate_bps = bitrate_tracker_.Rate(current_time_ms);
   float target_bitrate_bps = target_bitrate_bps_;
+  float estimated_bitrate_bps =
+      bitrate_tracker_.Rate(current_time_ms).value_or(target_bitrate_bps);
   float error = target_bitrate_bps - estimated_bitrate_bps;
 
   // Adjust if we've overshot by any amount or if we've undershot too much.

webrtc/common_video/bitrate_adjuster_unittest.cc

@@ -48,7 +48,8 @@ class BitrateAdjusterTest : public ::testing::Test {
     // target bitrate within clamp.
     uint32_t target_bitrate_bps = adjuster_.GetTargetBitrateBps();
     uint32_t adjusted_bitrate_bps = adjuster_.GetAdjustedBitrateBps();
-    uint32_t estimated_bitrate_bps = adjuster_.GetEstimatedBitrateBps();
+    uint32_t estimated_bitrate_bps =
+        adjuster_.GetEstimatedBitrateBps().value_or(target_bitrate_bps);
     uint32_t adjusted_lower_bound_bps =
         GetTargetBitrateBpsPct(kMinAdjustedBitratePct);
     uint32_t adjusted_upper_bound_bps =

webrtc/common_video/include/bitrate_adjuster.h

@@ -47,7 +47,7 @@ class BitrateAdjuster {
   uint32_t GetAdjustedBitrateBps() const;
 
   // Returns what we think the current bitrate is.
-  uint32_t GetEstimatedBitrateBps();
+  rtc::Optional<uint32_t> GetEstimatedBitrateBps();
 
   // This should be called after each frame is encoded. The timestamp at which
   // it is called is used to estimate the output bitrate of the encoder.

webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc

@@ -37,7 +37,7 @@ AimdRateControl::AimdRateControl()
       rate_control_state_(kRcHold),
       rate_control_region_(kRcMaxUnknown),
       time_last_bitrate_change_(-1),
-      current_input_(kBwNormal, 0, 1.0),
+      current_input_(kBwNormal, rtc::Optional<uint32_t>(), 1.0),
       updated_(false),
       time_first_incoming_estimate_(-1),
       bitrate_is_initialized_(false),
@@ -87,8 +87,9 @@ uint32_t AimdRateControl::LatestEstimate() const {
 }
 
 uint32_t AimdRateControl::UpdateBandwidthEstimate(int64_t now_ms) {
-  current_bitrate_bps_ = ChangeBitrate(current_bitrate_bps_,
-                                       current_input_.incoming_bitrate, now_ms);
+  current_bitrate_bps_ = ChangeBitrate(
+      current_bitrate_bps_,
+      current_input_.incoming_bitrate.value_or(current_bitrate_bps_), now_ms);
   if (now_ms - time_of_last_log_ > kLogIntervalMs) {
     time_of_last_log_ = now_ms;
   }
@@ -100,7 +101,7 @@ void AimdRateControl::SetRtt(int64_t rtt) {
 }
 
 void AimdRateControl::Update(const RateControlInput* input, int64_t now_ms) {
-  assert(input);
+  RTC_CHECK(input);
 
   // Set the initial bit rate value to what we're receiving the first half
   // second.
@@ -108,12 +109,11 @@ void AimdRateControl::Update(const RateControlInput* input, int64_t now_ms) {
     const int64_t kInitializationTimeMs = 5000;
     RTC_DCHECK_LE(kBitrateWindowMs, kInitializationTimeMs);
     if (time_first_incoming_estimate_ < 0) {
-      if (input->incoming_bitrate > 0) {
+      if (input->incoming_bitrate)
         time_first_incoming_estimate_ = now_ms;
-      }
     } else if (now_ms - time_first_incoming_estimate_ > kInitializationTimeMs &&
-               input->incoming_bitrate > 0) {
-      current_bitrate_bps_ = input->incoming_bitrate;
+               input->incoming_bitrate) {
+      current_bitrate_bps_ = *input->incoming_bitrate;
       bitrate_is_initialized_ = true;
     }
   }

webrtc/modules/remote_bitrate_estimator/aimd_rate_control.h

@@ -67,7 +67,6 @@ class AimdRateControl {
   uint32_t min_configured_bitrate_bps_;
   uint32_t max_configured_bitrate_bps_;
   uint32_t current_bitrate_bps_;
-  uint32_t max_hold_rate_bps_;
   float avg_max_bitrate_kbps_;
   float var_max_bitrate_kbps_;
   RateControlState rate_control_state_;

webrtc/modules/remote_bitrate_estimator/include/bwe_defines.h

@@ -11,6 +11,7 @@
 #ifndef WEBRTC_MODULES_REMOTE_BITRATE_ESTIMATOR_INCLUDE_BWE_DEFINES_H_
 #define WEBRTC_MODULES_REMOTE_BITRATE_ESTIMATOR_INCLUDE_BWE_DEFINES_H_
 
+#include "webrtc/base/optional.h"
 #include "webrtc/typedefs.h"
 
 #define BWE_MAX(a, b) ((a) > (b) ? (a) : (b))
@@ -32,14 +33,14 @@ enum RateControlRegion { kRcNearMax, kRcAboveMax, kRcMaxUnknown };
 
 struct RateControlInput {
   RateControlInput(BandwidthUsage bw_state,
-                   uint32_t incoming_bitrate,
+                   const rtc::Optional<uint32_t>& incoming_bitrate,
                    double noise_var)
       : bw_state(bw_state),
         incoming_bitrate(incoming_bitrate),
         noise_var(noise_var) {}
 
   BandwidthUsage bw_state;
-  uint32_t incoming_bitrate;
+  rtc::Optional<uint32_t> incoming_bitrate;
   double noise_var;
 };
 }  // namespace webrtc

webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc

@@ -85,6 +85,7 @@ bool RemoteBitrateEstimatorAbsSendTime::IsWithinClusterBounds(
         estimator_(),
         detector_(OverUseDetectorOptions()),
         incoming_bitrate_(kBitrateWindowMs, 8000),
+        incoming_bitrate_initialized_(false),
         total_probes_received_(0),
         first_packet_time_ms_(-1),
         last_update_ms_(-1),
@@ -243,6 +244,18 @@ void RemoteBitrateEstimatorAbsSendTime::IncomingPacketInfo(
   int64_t now_ms = arrival_time_ms;
   // TODO(holmer): SSRCs are only needed for REMB, should be broken out from
   // here.
+
+  // Check if incoming bitrate estimate is valid, and if it needs to be reset.
+  rtc::Optional<uint32_t> incoming_bitrate = incoming_bitrate_.Rate(now_ms);
+  if (incoming_bitrate) {
+    incoming_bitrate_initialized_ = true;
+  } else if (incoming_bitrate_initialized_) {
+    // Incoming bitrate had a previous valid value, but now not enough data
+    // point are left within the current window. Reset incoming bitrate
+    // estimator so that the window size will only contain new data points.
+    incoming_bitrate_.Reset();
+    incoming_bitrate_initialized_ = false;
+  }
   incoming_bitrate_.Update(payload_size, now_ms);
 
   if (first_packet_time_ms_ == -1)
@@ -303,10 +316,12 @@ void RemoteBitrateEstimatorAbsSendTime::IncomingPacketInfo(
       if (last_update_ms_ == -1 ||
           now_ms - last_update_ms_ > remote_rate_.GetFeedbackInterval()) {
         update_estimate = true;
-      } else if (detector_.State() == kBwOverusing &&
-                 remote_rate_.TimeToReduceFurther(
-                     now_ms, incoming_bitrate_.Rate(now_ms))) {
-        update_estimate = true;
+      } else if (detector_.State() == kBwOverusing) {
+        rtc::Optional<uint32_t> incoming_rate = incoming_bitrate_.Rate(now_ms);
+        if (incoming_rate &&
+            remote_rate_.TimeToReduceFurther(now_ms, *incoming_rate)) {
+          update_estimate = true;
+        }
       }
     }
 

webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h

@@ -124,6 +124,7 @@ class RemoteBitrateEstimatorAbsSendTime : public RemoteBitrateEstimator {
   std::unique_ptr<OveruseEstimator> estimator_;
   OveruseDetector detector_;
   RateStatistics incoming_bitrate_;
+  bool incoming_bitrate_initialized_;
   std::vector<int> recent_propagation_delta_ms_;
   std::vector<int64_t> recent_update_time_ms_;
   std::list<Probe> probes_;

webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time_unittest.cc

@@ -35,7 +35,7 @@ TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, RateIncreaseReordering) {
 }
 
 TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, RateIncreaseRtpTimestamps) {
-  RateIncreaseRtpTimestampsTestHelper(1229);
+  RateIncreaseRtpTimestampsTestHelper(1237);
 }
 
 TEST_F(RemoteBitrateEstimatorAbsSendTimeTest, CapacityDropOneStream) {

webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc

@@ -44,16 +44,17 @@ struct RemoteBitrateEstimatorSingleStream::Detector {
   OveruseDetector detector;
 };
 
-  RemoteBitrateEstimatorSingleStream::RemoteBitrateEstimatorSingleStream(
-      RemoteBitrateObserver* observer,
-      Clock* clock)
-      : clock_(clock),
-        incoming_bitrate_(kBitrateWindowMs, 8000),
-        remote_rate_(new AimdRateControl()),
-        observer_(observer),
-        crit_sect_(CriticalSectionWrapper::CreateCriticalSection()),
-        last_process_time_(-1),
-        process_interval_ms_(kProcessIntervalMs) {
+RemoteBitrateEstimatorSingleStream::RemoteBitrateEstimatorSingleStream(
+    RemoteBitrateObserver* observer,
+    Clock* clock)
+    : clock_(clock),
+      incoming_bitrate_(kBitrateWindowMs, 8000),
+      last_valid_incoming_bitrate_(0),
+      remote_rate_(new AimdRateControl()),
+      observer_(observer),
+      crit_sect_(CriticalSectionWrapper::CreateCriticalSection()),
+      last_process_time_(-1),
+      process_interval_ms_(kProcessIntervalMs) {
   assert(observer_);
   LOG(LS_INFO) << "RemoteBitrateEstimatorSingleStream: Instantiating.";
 }
@@ -90,7 +91,20 @@ void RemoteBitrateEstimatorSingleStream::IncomingPacket(int64_t arrival_time_ms,
   }
   Detector* estimator = it->second;
   estimator->last_packet_time_ms = now_ms;
+
+  // Check if incoming bitrate estimate is valid, and if it needs to be reset.
+  rtc::Optional<uint32_t> incoming_bitrate = incoming_bitrate_.Rate(now_ms);
+  if (incoming_bitrate) {
+    last_valid_incoming_bitrate_ = *incoming_bitrate;
+  } else if (last_valid_incoming_bitrate_ > 0) {
+    // Incoming bitrate had a previous valid value, but now not enough data
+    // point are left within the current window. Reset incoming bitrate
+    // estimator so that the window size will only contain new data points.
+    incoming_bitrate_.Reset();
+    last_valid_incoming_bitrate_ = 0;
+  }
   incoming_bitrate_.Update(payload_size, now_ms);
+
   const BandwidthUsage prior_state = estimator->detector.State();
   uint32_t timestamp_delta = 0;
   int64_t time_delta = 0;
@@ -106,9 +120,11 @@ void RemoteBitrateEstimatorSingleStream::IncomingPacket(int64_t arrival_time_ms,
                                estimator->estimator.num_of_deltas(), now_ms);
   }
   if (estimator->detector.State() == kBwOverusing) {
-    uint32_t incoming_bitrate_bps = incoming_bitrate_.Rate(now_ms);
-    if (prior_state != kBwOverusing ||
-        remote_rate_->TimeToReduceFurther(now_ms, incoming_bitrate_bps)) {
+    rtc::Optional<uint32_t> incoming_bitrate_bps =
+        incoming_bitrate_.Rate(now_ms);
+    if (incoming_bitrate_bps &&
+        (prior_state != kBwOverusing ||
+         remote_rate_->TimeToReduceFurther(now_ms, *incoming_bitrate_bps))) {
       // The first overuse should immediately trigger a new estimate.
       // We also have to update the estimate immediately if we are overusing
       // and the target bitrate is too high compared to what we are receiving.
@@ -167,6 +183,7 @@ void RemoteBitrateEstimatorSingleStream::UpdateEstimate(int64_t now_ms) {
     remote_rate_.reset(new AimdRateControl());
     return;
   }
+
   double mean_noise_var = sum_var_noise /
       static_cast<double>(overuse_detectors_.size());
   const RateControlInput input(bw_state,

webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h

@@ -56,6 +56,7 @@ class RemoteBitrateEstimatorSingleStream : public RemoteBitrateEstimator {
   Clock* clock_;
   SsrcOveruseEstimatorMap overuse_detectors_ GUARDED_BY(crit_sect_.get());
   RateStatistics incoming_bitrate_ GUARDED_BY(crit_sect_.get());
+  uint32_t last_valid_incoming_bitrate_ GUARDED_BY(crit_sect_.get());
   std::unique_ptr<AimdRateControl> remote_rate_ GUARDED_BY(crit_sect_.get());
   RemoteBitrateObserver* observer_ GUARDED_BY(crit_sect_.get());
   std::unique_ptr<CriticalSectionWrapper> crit_sect_;

webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream_unittest.cc

@@ -35,7 +35,7 @@ TEST_F(RemoteBitrateEstimatorSingleTest, RateIncreaseReordering) {
 }
 
 TEST_F(RemoteBitrateEstimatorSingleTest, RateIncreaseRtpTimestamps) {
-  RateIncreaseRtpTimestampsTestHelper(1240);
+  RateIncreaseRtpTimestampsTestHelper(1267);
 }
 
 TEST_F(RemoteBitrateEstimatorSingleTest, CapacityDropOneStream) {
@@ -47,15 +47,15 @@ TEST_F(RemoteBitrateEstimatorSingleTest, CapacityDropOneStreamWrap) {
 }
 
 TEST_F(RemoteBitrateEstimatorSingleTest, CapacityDropTwoStreamsWrap) {
-  CapacityDropTestHelper(2, true, 600);
+  CapacityDropTestHelper(2, true, 767);
 }
 
 TEST_F(RemoteBitrateEstimatorSingleTest, CapacityDropThreeStreamsWrap) {
-  CapacityDropTestHelper(3, true, 767);
+  CapacityDropTestHelper(3, true, 567);
 }
 
 TEST_F(RemoteBitrateEstimatorSingleTest, CapacityDropThirteenStreamsWrap) {
-  CapacityDropTestHelper(13, true, 733);
+  CapacityDropTestHelper(13, true, 567);
 }
 
 TEST_F(RemoteBitrateEstimatorSingleTest, CapacityDropNineteenStreamsWrap) {

webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc

@@ -18,6 +18,9 @@ namespace webrtc {
 const size_t kMtu = 1200;
 const uint32_t kAcceptedBitrateErrorBps = 50000;
 
+// Number of packets needed before we have a valid estimate.
+const int kNumInitialPackets = 2;
+
 namespace testing {
 
 void TestBitrateObserver::OnReceiveBitrateChanged(
@@ -317,16 +320,16 @@ void RemoteBitrateEstimatorTest::InitialBehaviorTestHelper(
   EXPECT_FALSE(bitrate_observer_->updated());
   bitrate_observer_->Reset();
   clock_.AdvanceTimeMilliseconds(1000);
-  // Inserting a packet. Still no valid estimate. We need to wait 5 seconds.
-  IncomingPacket(kDefaultSsrc, kMtu, clock_.TimeInMilliseconds(), timestamp,
-                 absolute_send_time, true);
-  bitrate_estimator_->Process();
-  EXPECT_FALSE(bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_bps));
-  EXPECT_EQ(0u, ssrcs.size());
-  EXPECT_FALSE(bitrate_observer_->updated());
-  bitrate_observer_->Reset();
   // Inserting packets for 5 seconds to get a valid estimate.
-  for (int i = 0; i < 5 * kFramerate + 1; ++i) {
+  for (int i = 0; i < 5 * kFramerate + 1 + kNumInitialPackets; ++i) {
+    if (i == kNumInitialPackets) {
+      bitrate_estimator_->Process();
+      EXPECT_FALSE(bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_bps));
+      EXPECT_EQ(0u, ssrcs.size());
+      EXPECT_FALSE(bitrate_observer_->updated());
+      bitrate_observer_->Reset();
+    }
+
     IncomingPacket(kDefaultSsrc, kMtu, clock_.TimeInMilliseconds(), timestamp,
                    absolute_send_time, true);
     clock_.AdvanceTimeMilliseconds(1000 / kFramerate);
@@ -355,12 +358,16 @@ void RemoteBitrateEstimatorTest::RateIncreaseReorderingTestHelper(
   const uint32_t kFrameIntervalAbsSendTime = AbsSendTime(1, kFramerate);
   uint32_t timestamp = 0;
   uint32_t absolute_send_time = 0;
-  IncomingPacket(kDefaultSsrc, 1000, clock_.TimeInMilliseconds(), timestamp,
-                 absolute_send_time, true);
-  bitrate_estimator_->Process();
-  EXPECT_FALSE(bitrate_observer_->updated());  // No valid estimate.
-  // Inserting packets for one second to get a valid estimate.
-  for (int i = 0; i < 5 * kFramerate + 1; ++i) {
+  // Inserting packets for five seconds to get a valid estimate.
+  for (int i = 0; i < 5 * kFramerate + 1 + kNumInitialPackets; ++i) {
+    // TODO(sprang): Remove this hack once the single stream estimator is gone,
+    // as it doesn't do anything in Process().
+    if (i == kNumInitialPackets) {
+      // Process after we have enough frames to get a valid input rate estimate.
+      bitrate_estimator_->Process();
+      EXPECT_FALSE(bitrate_observer_->updated());  // No valid estimate.
+    }
+
     IncomingPacket(kDefaultSsrc, kMtu, clock_.TimeInMilliseconds(), timestamp,
                    absolute_send_time, true);
     clock_.AdvanceTimeMilliseconds(kFrameIntervalMs);

webrtc/video/receive_statistics_proxy.cc

@@ -247,7 +247,7 @@ void ReceiveStatisticsProxy::OnDecodedFrame() {
 
   rtc::CritScope lock(&crit_);
   decode_fps_estimator_.Update(1, now);
-  stats_.decode_frame_rate = decode_fps_estimator_.Rate(now);
+  stats_.decode_frame_rate = decode_fps_estimator_.Rate(now).value_or(0);
 }
 
 void ReceiveStatisticsProxy::OnRenderedFrame(int width, int height) {
@@ -257,7 +257,7 @@ void ReceiveStatisticsProxy::OnRenderedFrame(int width, int height) {
 
   rtc::CritScope lock(&crit_);
   renders_fps_estimator_.Update(1, now);
-  stats_.render_frame_rate = renders_fps_estimator_.Rate(now);
+  stats_.render_frame_rate = renders_fps_estimator_.Rate(now).value_or(0);
   render_width_counter_.Add(width);
   render_height_counter_.Add(height);
   render_fps_tracker_.AddSamples(1);