Move BWE period calculation from ProbingIntervalEstimator to AimdRateControl.

webrtc/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc

 /*
  *  Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 #include <memory>
 
 #include "webrtc/modules/remote_bitrate_estimator/aimd_rate_control.h"
 #include "webrtc/system_wrappers/include/clock.h"
 #include "webrtc/test/gtest.h"
 
 namespace webrtc {
 namespace {
 
 constexpr int64_t kClockInitialTime = 123456;
 
+constexpr int kMinBwePeriodMs = 2000;
+constexpr int kMaxBwePeriodMs = 50000;
+constexpr int kDefaultPeriodMs = 3000;
+
 struct AimdRateControlStates {
   std::unique_ptr<AimdRateControl> aimd_rate_control;
   std::unique_ptr<SimulatedClock> simulated_clock;
 };
 
 AimdRateControlStates CreateAimdRateControlStates() {
   AimdRateControlStates states;
   states.aimd_rate_control.reset(new AimdRateControl());
   states.simulated_clock.reset(new SimulatedClock(kClockInitialTime));
   return states;
@@ skipping 28 matching lines @@
 
 TEST(AimdRateControlTest, NearMaxIncreaseRateIs5kbpsOn60kbpsAnd100msRtt) {
   auto states = CreateAimdRateControlStates();
   constexpr int kBitrate = 60000;
   states.aimd_rate_control->SetEstimate(
       kBitrate, states.simulated_clock->TimeInMilliseconds());
   states.aimd_rate_control->SetRtt(100);
   EXPECT_EQ(5000, states.aimd_rate_control->GetNearMaxIncreaseRateBps());
 }
 
-TEST(AimdRateControlTest, UnknownBitrateDecreaseBeforeFirstOveruse) {
-  auto states = CreateAimdRateControlStates();
-  EXPECT_EQ(rtc::Optional<int>(),
-            states.aimd_rate_control->GetLastBitrateDecreaseBps());
-}
-
 TEST(AimdRateControlTest, GetLastBitrateDecrease) {

[minyue-webrtc, 2017/04/11 10:50:02]

rename test

[terelius, 2017/04/19 07:36:01]

Done.

   auto states = CreateAimdRateControlStates();
   constexpr int kBitrate = 300000;
   states.aimd_rate_control->SetEstimate(
       kBitrate, states.simulated_clock->TimeInMilliseconds());
-  UpdateRateControl(states, kBwOverusing, kBitrate - 2000,
+  UpdateRateControl(states, kBwOverusing, kBitrate,
                     states.simulated_clock->TimeInMilliseconds());
-  EXPECT_EQ(rtc::Optional<int>(46700),
-            states.aimd_rate_control->GetLastBitrateDecreaseBps());
+  EXPECT_NEAR(14000, states.aimd_rate_control->GetNearMaxIncreaseRateBps(),

[minyue-webrtc, 2017/04/11 10:50:02]

can we be stricter on this?

[terelius, 2017/04/19 07:36:01]

Yes, we could enforce strict equality but that would just make the test fail
whenever the code changes. What we really want to test is that
GetNearMaxIncreaseRateBps gives reasonable results, e.g. about 14 seconds at 300
kbps. Afaik, we don't care if the period is 14 or 14.51 seconds. WDYT?

[minyue-webrtc, 2017/04/19 07:49:05]

Sure, if you think 1s a good margin.

+              1000);
+  ASSERT_TRUE(states.aimd_rate_control->GetExpectedBandwidthPeriodMs());

[minyue-webrtc, 2017/04/11 10:50:02]

what does 82 do?

[terelius, 2017/04/19 07:36:01]

Left-over from previous version. Removed.

+  EXPECT_EQ(3000, states.aimd_rate_control->GetExpectedBandwidthPeriodMs());
 }
 
 TEST(AimdRateControlTest, BweLimitedByAckedBitrate) {
   auto states = CreateAimdRateControlStates();
   constexpr int kAckedBitrate = 10000;
   states.aimd_rate_control->SetEstimate(
       kAckedBitrate, states.simulated_clock->TimeInMilliseconds());
   while (states.simulated_clock->TimeInMilliseconds() - kClockInitialTime <
          20000) {
     UpdateRateControl(states, kBwNormal, kAckedBitrate,
@@ skipping 20 matching lines @@
   // 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);
 }
+
+TEST(AimdRateControlTest, DefaultPeriodUntilFirstOveruse) {
+  auto states = CreateAimdRateControlStates();
+  states.aimd_rate_control->SetStartBitrate(300000);
+  EXPECT_EQ(kDefaultPeriodMs,
+            states.aimd_rate_control->GetExpectedBandwidthPeriodMs());
+  states.simulated_clock->AdvanceTimeMilliseconds(100);
+  UpdateRateControl(states, kBwOverusing, 100000,
+                    states.simulated_clock->TimeInMilliseconds());
+  EXPECT_NE(kDefaultPeriodMs,
+            states.aimd_rate_control->GetExpectedBandwidthPeriodMs());
+}
+
+TEST(AimdRateControlTest, ExpectedPeriodAfter20kbpsDrop5kbpsIncrease) {

[minyue-webrtc, 2017/04/11 10:50:02]

Preferrably not hard code numerics in the name

[terelius, 2017/04/19 07:36:01]

I'm following the convention used in the other test cases, e.g.
NearMaxIncreaseRateIs5kbpsOn60kbpsAnd100msRtt

+  auto states = CreateAimdRateControlStates();
+  constexpr int kInitialBitrate = 110000;
+  states.aimd_rate_control->SetEstimate(
+      kInitialBitrate, states.simulated_clock->TimeInMilliseconds());
+  states.simulated_clock->AdvanceTimeMilliseconds(100);
+  // Make the bitrate drop to 20 kbps fo get to 90 kbps.

[minyue-webrtc, 2017/04/11 10:50:02]

fo -> to

[terelius, 2017/04/19 07:36:01]

Done.

+  // The rate increase at 90 kbps should be 5 kbps, so the period should be 4 s.
+  UpdateRateControl(states, kBwOverusing, (kInitialBitrate - 20000) / 0.85,

[minyue-webrtc, 2017/04/11 10:50:02]

what is 0.85

[terelius, 2017/04/19 07:36:01]

We back off to 85% of the measured bitrate after an overuse. If we want to drop
to 90 kbps then we need to "measure" 90/0.85 kbps.

[minyue-webrtc, 2017/04/19 07:49:05]

can we give a name to it and constexpr?

[terelius, 2017/04/19 15:15:04]

Done.

+                    states.simulated_clock->TimeInMilliseconds());
+  EXPECT_EQ(5000, states.aimd_rate_control->GetNearMaxIncreaseRateBps());
+  EXPECT_EQ(4000, states.aimd_rate_control->GetExpectedBandwidthPeriodMs());
+}
+
+TEST(AimdRateControlTest, BandwidthPeriodNotBelowMin) {

[minyue-webrtc, 2017/04/11 10:50:02]

Not -> CanNot

[terelius, 2017/04/19 07:36:01]

Done. (IsNot)

+  auto states = CreateAimdRateControlStates();
+  constexpr int kInitialBitrate = 10000;
+  states.aimd_rate_control->SetEstimate(
+      kInitialBitrate, states.simulated_clock->TimeInMilliseconds());
+  states.simulated_clock->AdvanceTimeMilliseconds(100);
+  // Make a small (1.5 kbps) bitrate drop to 8.5 kbps.
+  UpdateRateControl(states, kBwOverusing, kInitialBitrate - 1,
+                    states.simulated_clock->TimeInMilliseconds());
+  EXPECT_EQ(kMinBwePeriodMs,
+            states.aimd_rate_control->GetExpectedBandwidthPeriodMs());
+}
+
+TEST(AimdRateControlTest, BandwidthPeriodNotAboveMax) {

[minyue-webrtc, 2017/04/11 10:50:02]

Not -> CanNot

[terelius, 2017/04/19 07:36:01]

Done. (IsNot)

+  auto states = CreateAimdRateControlStates();
+  constexpr int kInitialBitrate = 10010000;
+  states.aimd_rate_control->SetEstimate(
+      kInitialBitrate, states.simulated_clock->TimeInMilliseconds());
+  states.simulated_clock->AdvanceTimeMilliseconds(100);
+  // Make a large (10 Mbps) bitrate drop to 10 kbps.
+  UpdateRateControl(states, kBwOverusing, 10000 / 0.85,
+                    states.simulated_clock->TimeInMilliseconds());
+  EXPECT_EQ(kMaxBwePeriodMs,
+            states.aimd_rate_control->GetExpectedBandwidthPeriodMs());
+}
+
 }  // namespace webrtc