Reset the delay-based estimate before forcing a new loss-based send rate.
Without this fix the new estimate would be capped by the old delay-based
estimate. Also revert a part of https://codereview.webrtc.org/2949203002
that only pushes updates from the delay-based estimate if the estimate
change. This is reverted as a safety precaution to prevent situations
where the two estimators get out of sync.
Bug: webrtc:8495
Change-Id: I153f2af4a822e67d47c52bffc97a73ab931a15dd
Reviewed-on: https://webrtc-review.googlesource.com/20981
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Commit-Queue: Björn Terelius <terelius@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20614}diff --git a/modules/bitrate_controller/bitrate_controller_impl.cc b/modules/bitrate_controller/bitrate_controller_impl.cc
index 1a51dec..c7fe322 100644
--- a/modules/bitrate_controller/bitrate_controller_impl.cc
+++ b/modules/bitrate_controller/bitrate_controller_impl.cc
@@ -147,11 +147,13 @@
return;
{
rtc::CritScope cs(&critsect_);
- bandwidth_estimation_.UpdateDelayBasedEstimate(clock_->TimeInMilliseconds(),
- result.target_bitrate_bps);
if (result.probe) {
bandwidth_estimation_.SetSendBitrate(result.target_bitrate_bps);
}
+ // Since SetSendBitrate now resets the delay-based estimate, we have to call
+ // UpdateDelayBasedEstimate after SetSendBitrate.
+ bandwidth_estimation_.UpdateDelayBasedEstimate(clock_->TimeInMilliseconds(),
+ result.target_bitrate_bps);
}
MaybeTriggerOnNetworkChanged();
}
diff --git a/modules/bitrate_controller/send_side_bandwidth_estimation.cc b/modules/bitrate_controller/send_side_bandwidth_estimation.cc
index 02d7595..9e9118b 100644
--- a/modules/bitrate_controller/send_side_bandwidth_estimation.cc
+++ b/modules/bitrate_controller/send_side_bandwidth_estimation.cc
@@ -158,6 +158,7 @@
void SendSideBandwidthEstimation::SetSendBitrate(int bitrate) {
RTC_DCHECK_GT(bitrate, 0);
+ delay_based_bitrate_bps_ = 0; // Reset to avoid being capped by the estimate.
CapBitrateToThresholds(Clock::GetRealTimeClock()->TimeInMilliseconds(),
bitrate);
// Clear last sent bitrate history so the new value can be used directly
diff --git a/modules/bitrate_controller/send_side_bandwidth_estimation_unittest.cc b/modules/bitrate_controller/send_side_bandwidth_estimation_unittest.cc
index f2791ce..567d81f 100644
--- a/modules/bitrate_controller/send_side_bandwidth_estimation_unittest.cc
+++ b/modules/bitrate_controller/send_side_bandwidth_estimation_unittest.cc
@@ -136,4 +136,33 @@
EXPECT_EQ(kRttMs, rtt_ms);
}
+TEST(SendSideBweTest, SettingSendBitrateOverridesDelayBasedEstimate) {
+ ::testing::NiceMock<MockRtcEventLog> event_log;
+ SendSideBandwidthEstimation bwe(&event_log);
+ static const int kMinBitrateBps = 10000;
+ static const int kMaxBitrateBps = 10000000;
+ static const int kInitialBitrateBps = 300000;
+ static const int kDelayBasedBitrateBps = 350000;
+ static const int kForcedHighBitrate = 2500000;
+
+ int64_t now_ms = 0;
+ int bitrate_bps;
+ uint8_t fraction_loss;
+ int64_t rtt_ms;
+
+ bwe.SetMinMaxBitrate(kMinBitrateBps, kMaxBitrateBps);
+ bwe.SetSendBitrate(kInitialBitrateBps);
+
+ bwe.UpdateDelayBasedEstimate(now_ms, kDelayBasedBitrateBps);
+ bwe.UpdateEstimate(now_ms);
+ bwe.CurrentEstimate(&bitrate_bps, &fraction_loss, &rtt_ms);
+ EXPECT_GE(bitrate_bps, kInitialBitrateBps);
+ EXPECT_LE(bitrate_bps, kDelayBasedBitrateBps);
+
+ bwe.SetSendBitrate(kForcedHighBitrate);
+ bwe.CurrentEstimate(&bitrate_bps, &fraction_loss, &rtt_ms);
+ EXPECT_EQ(bitrate_bps, kForcedHighBitrate);
+}
+
+
} // namespace webrtc
diff --git a/modules/congestion_controller/delay_based_bwe.cc b/modules/congestion_controller/delay_based_bwe.cc
index af6af76..35c15ef 100644
--- a/modules/congestion_controller/delay_based_bwe.cc
+++ b/modules/congestion_controller/delay_based_bwe.cc
@@ -296,10 +296,8 @@
const RateControlInput input(
overusing ? BandwidthUsage::kBwOverusing : detector_.State(),
acked_bitrate_bps, 0);
- uint32_t prev_target_bitrate_bps = rate_control_.LatestEstimate();
*target_bitrate_bps = rate_control_.Update(&input, now_ms);
- return rate_control_.ValidEstimate() &&
- prev_target_bitrate_bps != *target_bitrate_bps;
+ return rate_control_.ValidEstimate();
}
void DelayBasedBwe::OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) {