From 2e59a02dd49c122a0e848baaebb7a38faf20dec4 Mon Sep 17 00:00:00 2001 From: deadbeef Date: Fri, 2 Dec 2016 11:29:33 -0800 Subject: [PATCH] Revert of Use different restrictions of acked bitrate lag depending on operating point. (patchset #3 id:40001 of https://codereview.webrtc.org/2542083003/ ) Reason for revert: Appears to cause a regression to RampUpTest.SendSideAudioOnlyUpDownUpRtx: https://build.chromium.org/p/client.webrtc.perf/builders/Android32%20Tests%20%28L%20Nexus6%29/builds/626 Original issue's description: > Use different restrictions of acked bitrate lag depending on operating point. > > Before this the BWE was allowed to operate freely up to 100 kbps. This isn't a good idea for audio BWE. > > BUG=webrtc:5079 > > Committed: https://crrev.com/5932149c9aeaa7679ad0bc3183047766832ca907 > Cr-Commit-Position: refs/heads/master@{#15389} TBR=terelius@webrtc.org,stefan@webrtc.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=webrtc:5079 Review-Url: https://codereview.webrtc.org/2547113002 Cr-Commit-Position: refs/heads/master@{#15394} --- .../aimd_rate_control.cc | 13 +++---- .../aimd_rate_control_unittest.cc | 38 ------------------- 2 files changed, 5 insertions(+), 46 deletions(-) diff --git a/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc b/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc index e0c9ccd3ba..685be422bf 100644 --- a/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc +++ b/webrtc/modules/remote_bitrate_estimator/aimd_rate_control.cc @@ -228,14 +228,11 @@ uint32_t AimdRateControl::ChangeBitrate(uint32_t current_bitrate_bps, default: assert(false); } - // 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(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); + 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_; time_last_bitrate_change_ = now_ms; } return current_bitrate_bps; diff --git a/webrtc/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc b/webrtc/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc index 878a03c4f5..b36b16ddbf 100644 --- a/webrtc/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc +++ b/webrtc/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc @@ -89,42 +89,4 @@ 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(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(1.5 * kAckedBitrate + 10000), - 2000); - EXPECT_EQ(new_estimate, prev_estimate); -} } // namespace webrtc