Prevent BWE rampdowns without new loss reports.

Before this change, UpdateEstimate would repeatedly decrease bitrate
even though there's no fresh corresponding RTCP loss report, triggering
multiple reactions to a single indication of high packet loss.

BUG=webrtc:5101
R=stefan@webrtc.org

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

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

webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.cc

@@ -43,13 +43,14 @@ const size_t kNumUmaRampupMetrics =
 }
 
 SendSideBandwidthEstimation::SendSideBandwidthEstimation()
-    : accumulate_lost_packets_Q8_(0),
+    : lost_packets_since_last_loss_update_Q8_(0),
-      accumulate_expected_packets_(0),
+      expected_packets_since_last_loss_update_(0),
       bitrate_(0),
       min_bitrate_configured_(kDefaultMinBitrateBps),
       max_bitrate_configured_(kDefaultMaxBitrateBps),
       last_low_bitrate_log_ms_(-1),
-      time_last_receiver_block_ms_(0),
+      has_decreased_since_last_fraction_loss_(false),
+      time_last_receiver_block_ms_(-1),
       last_fraction_loss_(0),
       last_round_trip_time_ms_(0),
       bwe_incoming_(0),
@@ -58,8 +59,7 @@ SendSideBandwidthEstimation::SendSideBandwidthEstimation()
       initially_lost_packets_(0),
       bitrate_at_2_seconds_kbps_(0),
       uma_update_state_(kNoUpdate),
-      rampup_uma_stats_updated_(kNumUmaRampupMetrics, false) {
+      rampup_uma_stats_updated_(kNumUmaRampupMetrics, false) {}
-}
 
 SendSideBandwidthEstimation::~SendSideBandwidthEstimation() {}
 
@@ -117,21 +117,20 @@ void SendSideBandwidthEstimation::UpdateReceiverBlock(uint8_t fraction_loss,
     // Calculate number of lost packets.
     const int num_lost_packets_Q8 = fraction_loss * number_of_packets;
     // Accumulate reports.
-    accumulate_lost_packets_Q8_ += num_lost_packets_Q8;
+    lost_packets_since_last_loss_update_Q8_ += num_lost_packets_Q8;
-    accumulate_expected_packets_ += number_of_packets;
+    expected_packets_since_last_loss_update_ += number_of_packets;
 
-    // Report loss if the total report is based on sufficiently many packets.
+    // Don't generate a loss rate until it can be based on enough packets.
-    if (accumulate_expected_packets_ >= kLimitNumPackets) {
+    if (expected_packets_since_last_loss_update_ < kLimitNumPackets)
-      last_fraction_loss_ =
+      return;
-          accumulate_lost_packets_Q8_ / accumulate_expected_packets_;
+
+    has_decreased_since_last_fraction_loss_ = false;
+    last_fraction_loss_ = lost_packets_since_last_loss_update_Q8_ /
+                          expected_packets_since_last_loss_update_;
 
     // Reset accumulators.
-      accumulate_lost_packets_Q8_ = 0;
+    lost_packets_since_last_loss_update_Q8_ = 0;
-      accumulate_expected_packets_ = 0;
+    expected_packets_since_last_loss_update_ = 0;
-    } else {
-      // Early return without updating estimate.
-      return;
-    }
   }
   time_last_receiver_block_ms_ = now_ms;
   UpdateEstimate(now_ms);
@@ -186,7 +185,9 @@ void SendSideBandwidthEstimation::UpdateEstimate(int64_t now_ms) {
   }
   UpdateMinHistory(now_ms);
   // Only start updating bitrate when receiving receiver blocks.
-  if (time_last_receiver_block_ms_ != 0) {
+  // TODO(pbos): Handle the case when no receiver report is received for a very
+  // long time.
+  if (time_last_receiver_block_ms_ != -1) {
     if (last_fraction_loss_ <= 5) {
       // Loss < 2%: Increase rate by 8% of the min bitrate in the last
       // kBweIncreaseIntervalMs.
@@ -207,11 +208,11 @@ void SendSideBandwidthEstimation::UpdateEstimate(int64_t now_ms) {
 
     } else if (last_fraction_loss_ <= 26) {
       // Loss between 2% - 10%: Do nothing.
-
     } else {
       // Loss > 10%: Limit the rate decreases to once a kBweDecreaseIntervalMs +
       // rtt.
-      if ((now_ms - time_last_decrease_ms_) >=
+      if (!has_decreased_since_last_fraction_loss_ &&
+          (now_ms - time_last_decrease_ms_) >=
               (kBweDecreaseIntervalMs + last_round_trip_time_ms_)) {
         time_last_decrease_ms_ = now_ms;
 
@@ -221,6 +222,7 @@ void SendSideBandwidthEstimation::UpdateEstimate(int64_t now_ms) {
         bitrate_ = static_cast<uint32_t>(
             (bitrate_ * static_cast<double>(512 - last_fraction_loss_)) /
             512.0);
+        has_decreased_since_last_fraction_loss_ = true;
       }
     }
   }

webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.h

@@ -61,14 +61,15 @@ class SendSideBandwidthEstimation {
   std::deque<std::pair<int64_t, uint32_t> > min_bitrate_history_;
 
   // incoming filters
-  int accumulate_lost_packets_Q8_;
+  int lost_packets_since_last_loss_update_Q8_;
-  int accumulate_expected_packets_;
+  int expected_packets_since_last_loss_update_;
 
   uint32_t bitrate_;
   uint32_t min_bitrate_configured_;
   uint32_t max_bitrate_configured_;
   int64_t last_low_bitrate_log_ms_;
 
+  bool has_decreased_since_last_fraction_loss_;
   int64_t time_last_receiver_block_ms_;
   uint8_t last_fraction_loss_;
   int64_t last_round_trip_time_ms_;

webrtc/modules/bitrate_controller/send_side_bandwidth_estimation_unittest.cc

@@ -44,4 +44,55 @@ TEST(SendSideBweTest, InitialRembWithProbing) {
   bwe.CurrentEstimate(&bitrate, &fraction_loss, &rtt);
   EXPECT_EQ(kRembBps, bitrate);
 }
+
+TEST(SendSideBweTest, DoesntReapplyBitrateDecreaseWithoutFollowingRemb) {
+  SendSideBandwidthEstimation bwe;
+  static const int kMinBitrateBps = 100000;
+  static const int kInitialBitrateBps = 1000000;
+  bwe.SetMinMaxBitrate(kMinBitrateBps, 1500000);
+  bwe.SetSendBitrate(kInitialBitrateBps);
+
+  static const uint8_t kFractionLoss = 128;
+  static const int64_t kRttMs = 50;
+
+  int64_t now_ms = 0;
+  int bitrate_bps;
+  uint8_t fraction_loss;
+  int64_t rtt_ms;
+  bwe.CurrentEstimate(&bitrate_bps, &fraction_loss, &rtt_ms);
+  EXPECT_EQ(kInitialBitrateBps, bitrate_bps);
+  EXPECT_EQ(0, fraction_loss);
+  EXPECT_EQ(0, rtt_ms);
+
+  // Signal heavy loss to go down in bitrate.
+  bwe.UpdateReceiverBlock(kFractionLoss, kRttMs, 100, now_ms);
+  // Trigger an update 2 seconds later to not be rate limited.
+  now_ms += 2000;
+  bwe.UpdateEstimate(now_ms);
+
+  bwe.CurrentEstimate(&bitrate_bps, &fraction_loss, &rtt_ms);
+  EXPECT_LT(bitrate_bps, kInitialBitrateBps);
+  // Verify that the obtained bitrate isn't hitting the min bitrate, or this
+  // test doesn't make sense. If this ever happens, update the thresholds or
+  // loss rates so that it doesn't hit min bitrate after one bitrate update.
+  EXPECT_GT(bitrate_bps, kMinBitrateBps);
+  EXPECT_EQ(kFractionLoss, fraction_loss);
+  EXPECT_EQ(kRttMs, rtt_ms);
+
+  // Triggering an update shouldn't apply further downgrade nor upgrade since
+  // there's no intermediate receiver block received indicating whether this is
+  // currently good or not.
+  int last_bitrate_bps = bitrate_bps;
+  // Trigger an update 2 seconds later to not be rate limited (but it still
+  // shouldn't update).
+  now_ms += 2000;
+  bwe.UpdateEstimate(now_ms);
+  bwe.CurrentEstimate(&bitrate_bps, &fraction_loss, &rtt_ms);
+
+  EXPECT_EQ(last_bitrate_bps, bitrate_bps);
+  // The old loss rate should still be applied though.
+  EXPECT_EQ(kFractionLoss, fraction_loss);
+  EXPECT_EQ(kRttMs, rtt_ms);
+}
+
 }  // namespace webrtc