admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Do not allow estimate to increase above the estimate when HOLD started.

Browse Source 
To ensure padding, we increase 1 bit instead of 1kbps to avoid that 1kbps adds up over time.
Not have unit test for this, but did manual/hamrit tests many times.

Bug: webrtc:12707
Change-Id: I9b3160ab1808cb3a21ff0609446359a4ec3a4949
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/325520
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41056}
This commit is contained in:
Diep Bui 2023-11-01 00:09:35 +00:00 committed by WebRTC LUCI CQ
parent 93214073f1
commit 7d1693f1c5
3 changed files with 58 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc
View File

@ -132,7 +132,7 @@ LossBasedBweV2::LossBasedBweV2(const FieldTrialsView* key_value_config)
instant_upper_bound_temporal_weights_.resize(
config_->observation_window_size);
CalculateTemporalWeights();
hold_duration_ = kInitHoldDuration;
last_hold_info_.duration = kInitHoldDuration;
}
bool LossBasedBweV2::IsEnabled() const {
@ -305,7 +305,7 @@ void LossBasedBweV2::UpdateBandwidthEstimate(
current_best_estimate_.loss_limited_bandwidth) {
best_candidate.loss_limited_bandwidth =
current_best_estimate_.loss_limited_bandwidth +
DataRate::KilobitsPerSec(1);
DataRate::BitsPerSec(1);
}
}
}
@ -340,12 +340,12 @@ void LossBasedBweV2::UpdateResult() {
}
if (loss_based_result_.state == LossBasedState::kDecreasing &&
last_hold_timestamp_ > last_send_time_most_recent_observation_ &&
last_hold_info_.timestamp > last_send_time_most_recent_observation_ &&
bounded_bandwidth_estimate < delay_based_estimate_) {
// BWE is not allowed to increase during the HOLD duration. The purpose of
// BWE is not allowed to increase above the HOLD rate. The purpose of
// HOLD is to not immediately ramp up BWE to a rate that may cause loss.
loss_based_result_.bandwidth_estimate = std::min(
loss_based_result_.bandwidth_estimate, bounded_bandwidth_estimate);
loss_based_result_.bandwidth_estimate =
std::min(last_hold_info_.rate, bounded_bandwidth_estimate);
return;
}
@ -372,18 +372,23 @@ void LossBasedBweV2::UpdateResult() {
RTC_LOG(LS_INFO) << this << " "
<< "Switch to HOLD. Bounded BWE: "
<< bounded_bandwidth_estimate.kbps()
<< ", duration: " << hold_duration_.seconds();
last_hold_timestamp_ =
last_send_time_most_recent_observation_ + hold_duration_;
hold_duration_ = std::min(kMaxHoldDuration,
hold_duration_ * config_->hold_duration_factor);
<< ", duration: " << last_hold_info_.duration.ms();
last_hold_info_ = {
.timestamp = last_send_time_most_recent_observation_ +
last_hold_info_.duration,
.duration =
std::min(kMaxHoldDuration, last_hold_info_.duration *
config_->hold_duration_factor),
.rate = bounded_bandwidth_estimate};
}
last_padding_info_ = PaddingInfo();
loss_based_result_.state = LossBasedState::kDecreasing;
} else {
// Reset the HOLD duration if delay based estimate works to avoid getting
// Reset the HOLD info if delay based estimate works to avoid getting
// stuck in low bitrate.
hold_duration_ = kInitHoldDuration;
last_hold_info_ = {.timestamp = Timestamp::MinusInfinity(),
.duration = kInitHoldDuration,
.rate = DataRate::PlusInfinity()};
last_padding_info_ = PaddingInfo();
loss_based_result_.state = LossBasedState::kDelayBasedEstimate;
}

9
modules/congestion_controller/goog_cc/loss_based_bwe_v2.h
View File

@ -155,6 +155,12 @@ class LossBasedBweV2 {
Timestamp padding_timestamp = Timestamp::MinusInfinity();
};
struct HoldInfo {
Timestamp timestamp = Timestamp::MinusInfinity();
TimeDelta duration = TimeDelta::Zero();
DataRate rate = DataRate::PlusInfinity();
};
static absl::optional<Config> CreateConfig(
const FieldTrialsView* key_value_config);
bool IsConfigValid() const;
@ -207,8 +213,7 @@ class LossBasedBweV2 {
DataRate max_bitrate_ = DataRate::PlusInfinity();
DataRate delay_based_estimate_ = DataRate::PlusInfinity();
LossBasedBweV2::Result loss_based_result_ = LossBasedBweV2::Result();
Timestamp last_hold_timestamp_ = Timestamp::MinusInfinity();
TimeDelta hold_duration_ = TimeDelta::Zero();
HoldInfo last_hold_info_ = HoldInfo();
PaddingInfo last_padding_info_ = PaddingInfo();
};

53
modules/congestion_controller/goog_cc/loss_based_bwe_v2_test.cc
View File

@ -853,8 +853,7 @@ TEST_F(LossBasedBweV2Test, EnsureIncreaseEvenIfAckedBitrateBound) {
ASSERT_EQ(result.state, LossBasedState::kIncreasing);
// The estimate increases by 1kbps.
EXPECT_EQ(result.bandwidth_estimate,
estimate_1 + DataRate::KilobitsPerSec(1));
EXPECT_EQ(result.bandwidth_estimate, estimate_1 + DataRate::BitsPerSec(1));
}
// After loss based bwe backs off, the estimate is bounded during the delayed
@ -1678,7 +1677,7 @@ TEST_F(LossBasedBweV2Test, IncreaseEstimateIfNotHold) {
TEST_F(LossBasedBweV2Test, IncreaseEstimateAfterHoldDuration) {
ExplicitKeyValueConfig key_value_config(
ShortObservationConfig("HoldDurationFactor:3"));
ShortObservationConfig("HoldDurationFactor:10"));
LossBasedBweV2 loss_based_bandwidth_estimator(&key_value_config);
loss_based_bandwidth_estimator.SetBandwidthEstimate(
DataRate::KilobitsPerSec(2500));
@ -1727,36 +1726,50 @@ TEST_F(LossBasedBweV2Test, IncreaseEstimateAfterHoldDuration) {
/*in_alr=*/false);
EXPECT_EQ(loss_based_bandwidth_estimator.GetLossBasedResult().state,
LossBasedState::kDecreasing);
estimate =
DataRate estimate_at_hold =
loss_based_bandwidth_estimator.GetLossBasedResult().bandwidth_estimate;
// During the hold duration, e.g. next 900ms, the estimate cannot increase.
// In the hold duration, e.g. next 3s, the estimate cannot increase above the
// hold rate. Get some lost packets to get lower estimate than the HOLD rate.
for (int i = 4; i <= 6; ++i) {
loss_based_bandwidth_estimator.UpdateBandwidthEstimate(
CreatePacketResultsWithReceivedPackets(
CreatePacketResultsWith100pLossRate(
/*first_packet_timestamp=*/Timestamp::Zero() +
kObservationDurationLowerBound * i),
/*delay_based_estimate=*/DataRate::PlusInfinity(),
/*in_alr=*/false);
EXPECT_EQ(loss_based_bandwidth_estimator.GetLossBasedResult().state,
LossBasedState::kDecreasing);
EXPECT_EQ(
EXPECT_LT(
loss_based_bandwidth_estimator.GetLossBasedResult().bandwidth_estimate,
estimate);
estimate_at_hold);
}
// After the hold duration, the estimate can increase again.
loss_based_bandwidth_estimator.UpdateBandwidthEstimate(
CreatePacketResultsWithReceivedPackets(
/*first_packet_timestamp=*/Timestamp::Zero() +
kObservationDurationLowerBound * 7),
/*delay_based_estimate=*/DataRate::PlusInfinity(),
/*in_alr=*/false);
EXPECT_EQ(loss_based_bandwidth_estimator.GetLossBasedResult().state,
LossBasedState::kIncreasing);
EXPECT_GE(
loss_based_bandwidth_estimator.GetLossBasedResult().bandwidth_estimate,
estimate);
int feedback_id = 7;
while (loss_based_bandwidth_estimator.GetLossBasedResult().state !=
LossBasedState::kIncreasing) {
loss_based_bandwidth_estimator.UpdateBandwidthEstimate(
CreatePacketResultsWithReceivedPackets(
/*first_packet_timestamp=*/Timestamp::Zero() +
kObservationDurationLowerBound * feedback_id),
/*delay_based_estimate=*/DataRate::PlusInfinity(),
/*in_alr=*/false);
if (loss_based_bandwidth_estimator.GetLossBasedResult().state ==
LossBasedState::kDecreasing) {
// In the hold duration, the estimate can not go higher than estimate at
// hold.
EXPECT_LE(loss_based_bandwidth_estimator.GetLossBasedResult()
.bandwidth_estimate,
estimate_at_hold);
} else if (loss_based_bandwidth_estimator.GetLossBasedResult().state ==
LossBasedState::kIncreasing) {
// After the hold duration, the estimate can increase again.
EXPECT_GT(loss_based_bandwidth_estimator.GetLossBasedResult()
.bandwidth_estimate,
estimate_at_hold);
}
feedback_id++;
}
}
TEST_F(LossBasedBweV2Test, EndHoldDurationIfDelayBasedEstimateWorks) {