Revert "Revert of Use different restrictions of acked bitrate lag depending on operating point. (patchset #3 id:40001 of https://codereview.webrtc.org/2542083003/ )"

This reverts commit 2e59a02dd49c122a0e848baaebb7a38faf20dec4.

BUG=webrtc:5079

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

webrtc/call/rampup_tests.cc

@@ -20,6 +20,11 @@ namespace webrtc {
 namespace {
 
 static const int64_t kPollIntervalMs = 20;
+static const int kHighBandwidthLimitBps = 80000;
+static const int kExpectedHighVideoBitrateBps = 60000;
+static const int kExpectedHighAudioBitrateBps = 30000;
+static const int kLowBandwidthLimitBps = 20000;
+static const int kExpectedLowBitrateBps = 20000;
 
 std::vector<uint32_t> GenerateSsrcs(size_t num_streams, uint32_t ssrc_offset) {
   std::vector<uint32_t> ssrcs;
@@ -414,12 +419,18 @@ std::string RampUpDownUpTester::GetModifierString() const {
   return str;
 }
 
+int RampUpDownUpTester::GetExpectedHighBitrate() const {
+  if (num_audio_streams_ > 0 && num_video_streams_ == 0)
+    return kExpectedHighAudioBitrateBps;
+  return kExpectedHighVideoBitrateBps;
+}
+
 void RampUpDownUpTester::EvolveTestState(int bitrate_bps, bool suspended) {
   int64_t now = clock_->TimeInMilliseconds();
   switch (test_state_) {
     case kFirstRampup: {
       EXPECT_FALSE(suspended);
-      if (bitrate_bps >= kExpectedHighBitrateBps) {
+      if (bitrate_bps >= GetExpectedHighBitrate()) {
         // The first ramp-up has reached the target bitrate. Change the
         // channel limit, and move to the next test state.
         forward_transport_config_.link_capacity_kbps =
@@ -456,7 +467,7 @@ void RampUpDownUpTester::EvolveTestState(int bitrate_bps, bool suspended) {
       break;
     }
     case kSecondRampup: {
-      if (bitrate_bps >= kExpectedHighBitrateBps && !suspended) {
+      if (bitrate_bps >= GetExpectedHighBitrate() && !suspended) {
         webrtc::test::PrintResult("ramp_up_down_up", GetModifierString(),
                                   "second_rampup", now - state_start_ms_, "ms",
                                   false);

webrtc/call/rampup_tests.h

@@ -118,15 +118,12 @@ class RampUpDownUpTester : public RampUpTester {
   bool PollStats() override;
 
  private:
-  static const int kHighBandwidthLimitBps = 80000;
-  static const int kExpectedHighBitrateBps = 60000;
-  static const int kLowBandwidthLimitBps = 20000;
-  static const int kExpectedLowBitrateBps = 20000;
   enum TestStates { kFirstRampup, kLowRate, kSecondRampup };
 
   Call::Config GetReceiverCallConfig() override;
 
   std::string GetModifierString() const;
+  int GetExpectedHighBitrate() const;
   void EvolveTestState(int bitrate_bps, bool suspended);
 
   TestStates test_state_;

webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc

@@ -228,11 +228,14 @@ uint32_t AimdRateControl::ChangeBitrate(uint32_t current_bitrate_bps,
     default:
       assert(false);
   }
-  if ((incoming_bitrate_bps > 100000 || current_bitrate_bps > 150000) &&
-      current_bitrate_bps > 1.5 * incoming_bitrate_bps) {
-    // Allow changing the bit rate if we are operating at very low rates
-    // Don't change the bit rate if the send side is too far off
-    current_bitrate_bps = current_bitrate_bps_;
+  // Don't change the bit rate if the send side is too far off.
+  // We allow a bit more lag at very low rates to not too easily get stuck if
+  // the encoder produces uneven outputs.
+  const uint32_t max_bitrate_bps =
+      static_cast<uint32_t>(1.5f * incoming_bitrate_bps) + 10000;
+  if (current_bitrate_bps > current_bitrate_bps_ &&
+      current_bitrate_bps > max_bitrate_bps) {
+    current_bitrate_bps = std::max(current_bitrate_bps_, max_bitrate_bps);
     time_last_bitrate_change_ = now_ms;
   }
   return current_bitrate_bps;

webrtc/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc

@@ -89,4 +89,42 @@ TEST(AimdRateControlTest, GetLastBitrateDecrease) {
             states.aimd_rate_control->GetLastBitrateDecreaseBps());
 }
 
+TEST(AimdRateControlTest, BweLimitedByAckedBitrate) {
+  auto states = CreateAimdRateControlStates();
+  constexpr int kAckedBitrate = 10000;
+  InitBitrate(states, kAckedBitrate,
+              states.simulated_clock->TimeInMilliseconds());
+  while (states.simulated_clock->TimeInMilliseconds() - kClockInitialTime <
+         20000) {
+    UpdateRateControl(states, kBwNormal, kAckedBitrate,
+                      states.simulated_clock->TimeInMilliseconds());
+    states.simulated_clock->AdvanceTimeMilliseconds(100);
+  }
+  ASSERT_TRUE(states.aimd_rate_control->ValidEstimate());
+  EXPECT_EQ(static_cast<uint32_t>(1.5 * kAckedBitrate + 10000),
+            states.aimd_rate_control->LatestEstimate());
+}
+
+TEST(AimdRateControlTest, BweNotLimitedByDecreasingAckedBitrate) {
+  auto states = CreateAimdRateControlStates();
+  constexpr int kAckedBitrate = 100000;
+  InitBitrate(states, kAckedBitrate,
+              states.simulated_clock->TimeInMilliseconds());
+  while (states.simulated_clock->TimeInMilliseconds() - kClockInitialTime <
+         20000) {
+    UpdateRateControl(states, kBwNormal, kAckedBitrate,
+                      states.simulated_clock->TimeInMilliseconds());
+    states.simulated_clock->AdvanceTimeMilliseconds(100);
+  }
+  ASSERT_TRUE(states.aimd_rate_control->ValidEstimate());
+  // If the acked bitrate decreases the BWE shouldn't be reduced to 1.5x
+  // what's being acked, but also shouldn't get to increase more.
+  uint32_t prev_estimate = states.aimd_rate_control->LatestEstimate();
+  UpdateRateControl(states, kBwNormal, kAckedBitrate / 2,
+                    states.simulated_clock->TimeInMilliseconds());
+  uint32_t new_estimate = states.aimd_rate_control->LatestEstimate();
+  EXPECT_NEAR(new_estimate, static_cast<uint32_t>(1.5 * kAckedBitrate + 10000),
+              2000);
+  EXPECT_EQ(new_estimate, prev_estimate);
+}
 }  // namespace webrtc