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}
This commit is contained in:
Björn Terelius 2023-09-27 14:10:08 +02:00 committed by WebRTC LUCI CQ
parent 332c56f087
commit 98e71f57ea
5 changed files with 26 additions and 12 deletions

View file

@ -79,7 +79,9 @@ AimdRateControl::AimdRateControl(const FieldTrialsView& key_value_config,
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 @@ void AimdRateControl::ChangeBitrate(const RateControlInput& input,
// 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

View file

@ -103,6 +103,8 @@ class AimdRateControl {
// 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",

View file

@ -106,18 +106,22 @@ TEST(AimdRateControlTest, DefaultPeriodUntilFirstOveruse) {
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));
}

View file

@ -53,7 +53,7 @@ TEST_F(RemoteBitrateEstimatorSingleTest, CapacityDropOneStreamWrap) {
}
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, CapacityDropThreeStreamsWrap) {
}
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) {

View file

@ -477,7 +477,8 @@ void RemoteBitrateEstimatorTest::CapacityDropTestHelper(
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.