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}
This commit is contained in:
committed by
Commit Bot
parent
72c4250cab
commit
29cb0e7f70
@ -147,11 +147,13 @@ void BitrateControllerImpl::OnDelayBasedBweResult(
|
|||||||
return;
|
return;
|
||||||
{
|
{
|
||||||
rtc::CritScope cs(&critsect_);
|
rtc::CritScope cs(&critsect_);
|
||||||
bandwidth_estimation_.UpdateDelayBasedEstimate(clock_->TimeInMilliseconds(),
|
|
||||||
result.target_bitrate_bps);
|
|
||||||
if (result.probe) {
|
if (result.probe) {
|
||||||
bandwidth_estimation_.SetSendBitrate(result.target_bitrate_bps);
|
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();
|
MaybeTriggerOnNetworkChanged();
|
||||||
}
|
}
|
||||||
|
|||||||
@ -158,6 +158,7 @@ void SendSideBandwidthEstimation::SetBitrates(int send_bitrate,
|
|||||||
|
|
||||||
void SendSideBandwidthEstimation::SetSendBitrate(int bitrate) {
|
void SendSideBandwidthEstimation::SetSendBitrate(int bitrate) {
|
||||||
RTC_DCHECK_GT(bitrate, 0);
|
RTC_DCHECK_GT(bitrate, 0);
|
||||||
|
delay_based_bitrate_bps_ = 0; // Reset to avoid being capped by the estimate.
|
||||||
CapBitrateToThresholds(Clock::GetRealTimeClock()->TimeInMilliseconds(),
|
CapBitrateToThresholds(Clock::GetRealTimeClock()->TimeInMilliseconds(),
|
||||||
bitrate);
|
bitrate);
|
||||||
// Clear last sent bitrate history so the new value can be used directly
|
// Clear last sent bitrate history so the new value can be used directly
|
||||||
|
|||||||
@ -136,4 +136,33 @@ TEST(SendSideBweTest, DoesntReapplyBitrateDecreaseWithoutFollowingRemb) {
|
|||||||
EXPECT_EQ(kRttMs, rtt_ms);
|
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
|
} // namespace webrtc
|
||||||
|
|||||||
@ -296,10 +296,8 @@ bool DelayBasedBwe::UpdateEstimate(int64_t now_ms,
|
|||||||
const RateControlInput input(
|
const RateControlInput input(
|
||||||
overusing ? BandwidthUsage::kBwOverusing : detector_.State(),
|
overusing ? BandwidthUsage::kBwOverusing : detector_.State(),
|
||||||
acked_bitrate_bps, 0);
|
acked_bitrate_bps, 0);
|
||||||
uint32_t prev_target_bitrate_bps = rate_control_.LatestEstimate();
|
|
||||||
*target_bitrate_bps = rate_control_.Update(&input, now_ms);
|
*target_bitrate_bps = rate_control_.Update(&input, now_ms);
|
||||||
return rate_control_.ValidEstimate() &&
|
return rate_control_.ValidEstimate();
|
||||||
prev_target_bitrate_bps != *target_bitrate_bps;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
void DelayBasedBwe::OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) {
|
void DelayBasedBwe::OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) {
|
||||||
|
|||||||
Reference in New Issue
Block a user