Add 1s as padding duration limit in loss based BWE.
If we have been sending padding for 1s and estimate still is unchanged, then stop padding by transitioning to decrease state.
Bug: webrtc:12707
Change-Id: I0dca2e5cd98263fc7fae9882c23c21634413c7a0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/324740
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41018}
diff --git a/modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc b/modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc
index 2ebbe33..1a90aa0 100644
--- a/modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc
+++ b/modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc
@@ -338,9 +338,17 @@
if (IsEstimateIncreasingWhenLossLimited(
/*old_estimate=*/loss_based_result_.bandwidth_estimate,
/*new_estimate=*/bounded_bandwidth_estimate) &&
+ CanKeepIncreasingState(bounded_bandwidth_estimate) &&
bounded_bandwidth_estimate < delay_based_estimate_ &&
bounded_bandwidth_estimate < max_bitrate_) {
- loss_based_result_.state = config_->use_padding_for_increase
+ if (config_->padding_duration > TimeDelta::Zero() &&
+ bounded_bandwidth_estimate > last_padding_info_.padding_rate) {
+ // Start a new padding duration.
+ last_padding_info_.padding_rate = bounded_bandwidth_estimate;
+ last_padding_info_.padding_timestamp =
+ last_send_time_most_recent_observation_;
+ }
+ loss_based_result_.state = config_->padding_duration > TimeDelta::Zero()
? LossBasedState::kIncreaseUsingPadding
: LossBasedState::kIncreasing;
} else if (bounded_bandwidth_estimate < delay_based_estimate_ &&
@@ -355,11 +363,13 @@
last_send_time_most_recent_observation_ + hold_duration_;
hold_duration_ = hold_duration_ * config_->hold_duration_factor;
}
+ last_padding_info_ = PaddingInfo();
loss_based_result_.state = LossBasedState::kDecreasing;
} else {
// Reset the HOLD duration if delay based estimate works to avoid getting
// stuck in low bitrate.
hold_duration_ = kInitHoldDuration;
+ last_padding_info_ = PaddingInfo();
loss_based_result_.state = LossBasedState::kDelayBasedEstimate;
}
loss_based_result_.bandwidth_estimate = bounded_bandwidth_estimate;
@@ -445,11 +455,10 @@
FieldTrialParameter<int> min_num_observations("MinNumObservations", 3);
FieldTrialParameter<double> lower_bound_by_acked_rate_factor(
"LowerBoundByAckedRateFactor", 0.0);
- FieldTrialParameter<bool> use_padding_for_increase("UsePadding", false);
-
FieldTrialParameter<double> hold_duration_factor("HoldDurationFactor", 0.0);
FieldTrialParameter<bool> use_byte_loss_rate("UseByteLossRate", false);
-
+ FieldTrialParameter<TimeDelta> padding_duration("PaddingDuration",
+ TimeDelta::Zero());
if (key_value_config) {
ParseFieldTrial({&enabled,
&bandwidth_rampup_upper_bound_factor,
@@ -487,9 +496,9 @@
&use_in_start_phase,
&min_num_observations,
&lower_bound_by_acked_rate_factor,
- &use_padding_for_increase,
&hold_duration_factor,
- &use_byte_loss_rate},
+ &use_byte_loss_rate,
+ &padding_duration},
key_value_config->Lookup("WebRTC-Bwe-LossBasedBweV2"));
}
@@ -551,9 +560,9 @@
config->min_num_observations = min_num_observations.Get();
config->lower_bound_by_acked_rate_factor =
lower_bound_by_acked_rate_factor.Get();
- config->use_padding_for_increase = use_padding_for_increase.Get();
config->hold_duration_factor = hold_duration_factor.Get();
config->use_byte_loss_rate = use_byte_loss_rate.Get();
+ config->padding_duration = padding_duration.Get();
return config;
}
@@ -1135,4 +1144,17 @@
return loss_based_result_.state != LossBasedState::kDelayBasedEstimate;
}
+bool LossBasedBweV2::CanKeepIncreasingState(DataRate estimate) const {
+ if (config_->padding_duration == TimeDelta::Zero() ||
+ loss_based_result_.state != LossBasedState::kIncreaseUsingPadding)
+ return true;
+
+ // Keep using the kIncreaseUsingPadding if either the state has been
+ // kIncreaseUsingPadding for less than kPaddingDuration or the estimate
+ // increases.
+ return last_padding_info_.padding_timestamp + config_->padding_duration >=
+ last_send_time_most_recent_observation_ ||
+ last_padding_info_.padding_rate < estimate;
+}
+
} // namespace webrtc
diff --git a/modules/congestion_controller/goog_cc/loss_based_bwe_v2.h b/modules/congestion_controller/goog_cc/loss_based_bwe_v2.h
index b39657c..425ca2a 100644
--- a/modules/congestion_controller/goog_cc/loss_based_bwe_v2.h
+++ b/modules/congestion_controller/goog_cc/loss_based_bwe_v2.h
@@ -118,9 +118,9 @@
bool use_in_start_phase = false;
int min_num_observations = 0;
double lower_bound_by_acked_rate_factor = 0.0;
- bool use_padding_for_increase = false;
double hold_duration_factor = 0.0;
bool use_byte_loss_rate = false;
+ TimeDelta padding_duration = TimeDelta::Zero();
};
struct Derivatives {
@@ -147,6 +147,11 @@
DataSize lost_size = DataSize::Zero();
};
+ struct PaddingInfo {
+ DataRate padding_rate = DataRate::MinusInfinity();
+ Timestamp padding_timestamp = Timestamp::MinusInfinity();
+ };
+
static absl::optional<Config> CreateConfig(
const FieldTrialsView* key_value_config);
bool IsConfigValid() const;
@@ -179,6 +184,7 @@
bool IsEstimateIncreasingWhenLossLimited(DataRate old_estimate,
DataRate new_estimate);
bool IsInLossLimitedState() const;
+ bool CanKeepIncreasingState(DataRate estimate) const;
absl::optional<DataRate> acknowledged_bitrate_;
absl::optional<Config> config_;
@@ -200,6 +206,7 @@
LossBasedBweV2::Result loss_based_result_ = LossBasedBweV2::Result();
Timestamp last_hold_timestamp_ = Timestamp::MinusInfinity();
TimeDelta hold_duration_ = TimeDelta::Zero();
+ PaddingInfo last_padding_info_ = PaddingInfo();
};
} // namespace webrtc
diff --git a/modules/congestion_controller/goog_cc/loss_based_bwe_v2_test.cc b/modules/congestion_controller/goog_cc/loss_based_bwe_v2_test.cc
index fc1f410..347e2a8 100644
--- a/modules/congestion_controller/goog_cc/loss_based_bwe_v2_test.cc
+++ b/modules/congestion_controller/goog_cc/loss_based_bwe_v2_test.cc
@@ -1472,7 +1472,7 @@
TEST_F(LossBasedBweV2Test, IncreaseUsingPaddingStateIfFieldTrial) {
ExplicitKeyValueConfig key_value_config(
- ShortObservationConfig("UsePadding:true"));
+ ShortObservationConfig("PaddingDuration:1000ms"));
LossBasedBweV2 loss_based_bandwidth_estimator(&key_value_config);
loss_based_bandwidth_estimator.SetBandwidthEstimate(
DataRate::KilobitsPerSec(2500));
@@ -1494,6 +1494,61 @@
LossBasedState::kIncreaseUsingPadding);
}
+TEST_F(LossBasedBweV2Test, DecreaseAfterPadding) {
+ ExplicitKeyValueConfig key_value_config(ShortObservationConfig(
+ "PaddingDuration:1000ms,BwRampupUpperBoundFactor:2.0"));
+ LossBasedBweV2 loss_based_bandwidth_estimator(&key_value_config);
+ loss_based_bandwidth_estimator.SetBandwidthEstimate(
+ DataRate::KilobitsPerSec(2500));
+ DataRate acknowledged_bitrate = DataRate::KilobitsPerSec(51);
+ loss_based_bandwidth_estimator.SetAcknowledgedBitrate(acknowledged_bitrate);
+ loss_based_bandwidth_estimator.UpdateBandwidthEstimate(
+ CreatePacketResultsWith50pPacketLossRate(
+ /*first_packet_timestamp=*/Timestamp::Zero()),
+ /*delay_based_estimate=*/DataRate::PlusInfinity(),
+ /*in_alr=*/false);
+ ASSERT_EQ(loss_based_bandwidth_estimator.GetLossBasedResult().state,
+ LossBasedState::kDecreasing);
+ ASSERT_EQ(
+ loss_based_bandwidth_estimator.GetLossBasedResult().bandwidth_estimate,
+ acknowledged_bitrate);
+
+ acknowledged_bitrate = DataRate::KilobitsPerSec(26);
+ loss_based_bandwidth_estimator.SetAcknowledgedBitrate(acknowledged_bitrate);
+ int feedback_id = 1;
+ while (loss_based_bandwidth_estimator.GetLossBasedResult().state !=
+ LossBasedState::kIncreaseUsingPadding) {
+ loss_based_bandwidth_estimator.UpdateBandwidthEstimate(
+ CreatePacketResultsWithReceivedPackets(
+ /*first_packet_timestamp=*/Timestamp::Zero() +
+ kObservationDurationLowerBound * feedback_id),
+ /*delay_based_estimate=*/DataRate::PlusInfinity(),
+ /*in_alr=*/false);
+ feedback_id++;
+ }
+
+ const Timestamp estimate_increased =
+ Timestamp::Zero() + kObservationDurationLowerBound * feedback_id;
+ // The state is kIncreaseUsingPadding for a while without changing the
+ // estimate, which is limited by 2 * acked rate.
+ while (loss_based_bandwidth_estimator.GetLossBasedResult().state ==
+ LossBasedState::kIncreaseUsingPadding) {
+ loss_based_bandwidth_estimator.UpdateBandwidthEstimate(
+ CreatePacketResultsWithReceivedPackets(
+ /*first_packet_timestamp=*/Timestamp::Zero() +
+ kObservationDurationLowerBound * feedback_id),
+ /*delay_based_estimate=*/DataRate::PlusInfinity(),
+ /*in_alr=*/false);
+ feedback_id++;
+ }
+
+ EXPECT_EQ(loss_based_bandwidth_estimator.GetLossBasedResult().state,
+ LossBasedState::kDecreasing);
+ const Timestamp start_decreasing =
+ Timestamp::Zero() + kObservationDurationLowerBound * (feedback_id - 1);
+ EXPECT_EQ(start_decreasing - estimate_increased, TimeDelta::Seconds(1));
+}
+
TEST_F(LossBasedBweV2Test, IncreaseEstimateIfNotHold) {
ExplicitKeyValueConfig key_value_config(
ShortObservationConfig("HoldDurationFactor:0"));
