Subtract an additional 5kbps of the bitrate when backing off.
Traditionally, we'd back off to 85% of the measured throughput in response to an overuse. However, this backoff doesn't appear to be sufficient to drain the queues in some low-bitrate scenarios, and the problem has gotten a bit worse with the RobustThroughputEstimator. (The new estimate looks more stable. The old estimator had more variation, the lowest points were lower, causing backoffs to lower rates.)
With this change, we back off to 0.85*thoughput-5kbps. The difference is negligible except at low bitrates.
Bug: webrtc:13402,b/298636540
Change-Id: I53328953c056b8ad77f6c7561d6017f171b2dfbc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/321701
Commit-Queue: Björn Terelius <terelius@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40827}
diff --git a/modules/remote_bitrate_estimator/aimd_rate_control.cc b/modules/remote_bitrate_estimator/aimd_rate_control.cc
index 5ac4ce8..fde6665 100644
--- a/modules/remote_bitrate_estimator/aimd_rate_control.cc
+++ b/modules/remote_bitrate_estimator/aimd_rate_control.cc
@@ -79,7 +79,9 @@
rtt_(kDefaultRtt),
send_side_(send_side),
no_bitrate_increase_in_alr_(
- key_value_config.IsEnabled("WebRTC-DontIncreaseDelayBasedBweInAlr")) {
+ key_value_config.IsEnabled("WebRTC-DontIncreaseDelayBasedBweInAlr")),
+ subtract_additional_backoff_term_(!key_value_config.IsDisabled(
+ "WebRTC-Bwe-SubtractAdditionalBackoffTerm")) {
ParseFieldTrial(
{&disable_estimate_bounded_increase_,
&use_current_estimate_as_min_upper_bound_},
@@ -287,6 +289,11 @@
// Set bit rate to something slightly lower than the measured throughput
// to get rid of any self-induced delay.
decreased_bitrate = estimated_throughput * beta_;
+ if (decreased_bitrate > DataRate::KilobitsPerSec(5) &&
+ subtract_additional_backoff_term_) {
+ decreased_bitrate -= DataRate::KilobitsPerSec(5);
+ }
+
if (decreased_bitrate > current_bitrate_) {
// TODO(terelius): The link_capacity estimate may be based on old
// throughput measurements. Relying on them may lead to unnecessary
diff --git a/modules/remote_bitrate_estimator/aimd_rate_control.h b/modules/remote_bitrate_estimator/aimd_rate_control.h
index 4efde54..97fa490 100644
--- a/modules/remote_bitrate_estimator/aimd_rate_control.h
+++ b/modules/remote_bitrate_estimator/aimd_rate_control.h
@@ -103,6 +103,8 @@
// Allow the delay based estimate to only increase as long as application
// limited region (alr) is not detected.
const bool no_bitrate_increase_in_alr_;
+ // If true, subtract an additional 5kbps when backing off.
+ const bool subtract_additional_backoff_term_;
// If "Disabled", estimated link capacity is not used as upper bound.
FieldTrialFlag disable_estimate_bounded_increase_{"Disabled"};
FieldTrialParameter<bool> use_current_estimate_as_min_upper_bound_{"c_upper",
diff --git a/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc b/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc
index 5b8b0ca..f26afe9 100644
--- a/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc
+++ b/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc
@@ -106,18 +106,22 @@
EXPECT_NE(aimd_rate_control.GetExpectedBandwidthPeriod(), kDefaultPeriod);
}
-TEST(AimdRateControlTest, ExpectedPeriodAfter20kbpsDropAnd5kbpsIncrease) {
+TEST(AimdRateControlTest, ExpectedPeriodAfterTypicalDrop) {
AimdRateControl aimd_rate_control(ExplicitKeyValueConfig(""));
- constexpr DataRate kInitialBitrate = DataRate::BitsPerSec(110'000);
+ // The rate increase at 216 kbps should be 12 kbps. If we drop from
+ // 216 + 4*12 = 264 kbps, it should take 4 seconds to recover. Since we
+ // back off to 0.85*acked_rate-5kbps, the acked bitrate needs to be 260
+ // kbps to end up at 216 kbps.
+ constexpr DataRate kInitialBitrate = DataRate::BitsPerSec(264'000);
+ constexpr DataRate kUpdatedBitrate = DataRate::BitsPerSec(216'000);
+ const DataRate kAckedBitrate =
+ (kUpdatedBitrate + DataRate::BitsPerSec(5'000)) / kFractionAfterOveruse;
Timestamp now = kInitialTime;
aimd_rate_control.SetEstimate(kInitialBitrate, now);
now += TimeDelta::Millis(100);
- // Make the bitrate drop by 20 kbps to get to 90 kbps.
- // The rate increase at 90 kbps should be 5 kbps, so the period should be 4 s.
- const DataRate kAckedBitrate =
- (kInitialBitrate - DataRate::BitsPerSec(20'000)) / kFractionAfterOveruse;
aimd_rate_control.Update({BandwidthUsage::kBwOverusing, kAckedBitrate}, now);
- EXPECT_EQ(aimd_rate_control.GetNearMaxIncreaseRateBpsPerSecond(), 5'000);
+ EXPECT_EQ(aimd_rate_control.LatestEstimate(), kUpdatedBitrate);
+ EXPECT_EQ(aimd_rate_control.GetNearMaxIncreaseRateBpsPerSecond(), 12'000);
EXPECT_EQ(aimd_rate_control.GetExpectedBandwidthPeriod(),
TimeDelta::Seconds(4));
}
diff --git a/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream_unittest.cc b/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream_unittest.cc
index 64ef39d..3d72807 100644
--- a/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream_unittest.cc
+++ b/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream_unittest.cc
@@ -53,7 +53,7 @@
}
TEST_F(RemoteBitrateEstimatorSingleTest, CapacityDropTwoStreamsWrap) {
- CapacityDropTestHelper(2, true, 767, 0);
+ CapacityDropTestHelper(2, true, 567, 0);
}
TEST_F(RemoteBitrateEstimatorSingleTest, CapacityDropThreeStreamsWrap) {
@@ -61,11 +61,11 @@
}
TEST_F(RemoteBitrateEstimatorSingleTest, CapacityDropThirteenStreamsWrap) {
- CapacityDropTestHelper(13, true, 567, 0);
+ CapacityDropTestHelper(13, true, 767, 0);
}
TEST_F(RemoteBitrateEstimatorSingleTest, CapacityDropNineteenStreamsWrap) {
- CapacityDropTestHelper(19, true, 700, 0);
+ CapacityDropTestHelper(19, true, 767, 0);
}
TEST_F(RemoteBitrateEstimatorSingleTest, CapacityDropThirtyStreamsWrap) {
diff --git a/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc b/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc
index ee96445..f8652b4 100644
--- a/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc
+++ b/modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc
@@ -477,7 +477,8 @@
uint32_t bitrate_bps = SteadyStateRun(
kDefaultSsrc, steady_state_time * kFramerate, kStartBitrate,
kMinExpectedBitrate, kMaxExpectedBitrate, kInitialCapacityBps);
- EXPECT_NEAR(kInitialCapacityBps, bitrate_bps, 130000u);
+ EXPECT_GE(bitrate_bps, 0.85 * kInitialCapacityBps);
+ EXPECT_LE(bitrate_bps, 1.05 * kInitialCapacityBps);
bitrate_observer_->Reset();
// Add an offset to make sure the BWE can handle it.
