Revert "Use loss based bwe v2 in the start phase."
This reverts commit b6c7ddd6a137e187fa459255488da3b70b0a6c24.
Reason for revert: broken unit test
Original change's description:
> Use loss based bwe v2 in the start phase.
>
> TESTED=manual before:screen/ANtkMApoYczA2V5; after:screen/9kBoSvYKzKZR4sK
>
> Bug: webrtc:12707
> Change-Id: Ic156e363625c4b7476011059f3cd95641972091c
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/320840
> Commit-Queue: Diep Bui <diepbp@webrtc.org>
> Reviewed-by: Per Kjellander <perkj@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#40789}
Bug: webrtc:12707
Change-Id: Ibde45436934707b8e0084aa496dc249bc1c78ab2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/321180
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#40790}
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 b58f0f7..3ad910b 100644
--- a/modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc
+++ b/modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc
@@ -136,10 +136,6 @@
num_observations_ > 0;
}
-bool LossBasedBweV2::ReadyToUseInStartPhase() const {
- return IsReady() && config_->use_in_start_phase;
-}
-
LossBasedBweV2::Result LossBasedBweV2::GetLossBasedResult() const {
Result result;
result.state = current_state_;
@@ -242,12 +238,9 @@
SetProbeBitrate(probe_bitrate);
if (!IsValid(current_estimate_.loss_limited_bandwidth)) {
- if (!IsValid(delay_based_estimate)) {
- RTC_LOG(LS_WARNING) << "The delay based estimate must be finite: "
- << ToString(delay_based_estimate);
- return;
- }
- current_estimate_.loss_limited_bandwidth = delay_based_estimate;
+ RTC_LOG(LS_VERBOSE)
+ << "The estimator must be initialized before it can be used.";
+ return;
}
ChannelParameters best_candidate = current_estimate_;
@@ -423,7 +416,6 @@
TimeDelta::Seconds(10));
FieldTrialParameter<bool> not_use_acked_rate_in_alr("NotUseAckedRateInAlr",
false);
- FieldTrialParameter<bool> use_in_start_phase("UseInStartPhase", false);
if (key_value_config) {
ParseFieldTrial({&enabled,
&bandwidth_rampup_upper_bound_factor,
@@ -461,8 +453,7 @@
&high_loss_rate_threshold,
&bandwidth_cap_at_high_loss_rate,
&slope_of_bwe_high_loss_func,
- ¬_use_acked_rate_in_alr,
- &use_in_start_phase},
+ ¬_use_acked_rate_in_alr},
key_value_config->Lookup("WebRTC-Bwe-LossBasedBweV2"));
}
@@ -525,7 +516,6 @@
config->probe_integration_enabled = probe_integration_enabled.Get();
config->probe_expiration = probe_expiration.Get();
config->not_use_acked_rate_in_alr = not_use_acked_rate_in_alr.Get();
- config->use_in_start_phase = use_in_start_phase.Get();
return config;
}
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 cd49d05..649e765 100644
--- a/modules/congestion_controller/goog_cc/loss_based_bwe_v2.h
+++ b/modules/congestion_controller/goog_cc/loss_based_bwe_v2.h
@@ -57,13 +57,11 @@
// initialized with a BWE and then has received enough `PacketResult`s.
bool IsReady() const;
- // Returns true if loss based BWE is ready to be used in the start phase.
- bool ReadyToUseInStartPhase() const;
-
// Returns `DataRate::PlusInfinity` if no BWE can be calculated.
Result GetLossBasedResult() const;
void SetAcknowledgedBitrate(DataRate acknowledged_bitrate);
+ void SetBandwidthEstimate(DataRate bandwidth_estimate);
void SetMinMaxBitrate(DataRate min_bitrate, DataRate max_bitrate);
void UpdateBandwidthEstimate(
rtc::ArrayView<const PacketResult> packet_results,
@@ -72,9 +70,6 @@
absl::optional<DataRate> probe_bitrate,
bool in_alr);
- // For unit testing only.
- void SetBandwidthEstimate(DataRate bandwidth_estimate);
-
private:
struct ChannelParameters {
double inherent_loss = 0.0;
@@ -119,7 +114,6 @@
bool probe_integration_enabled = false;
TimeDelta probe_expiration = TimeDelta::Zero();
bool not_use_acked_rate_in_alr = false;
- bool use_in_start_phase = false;
};
struct Derivatives {
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 ca8596f..bdd7498 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
@@ -1530,34 +1530,6 @@
acked_rate);
}
-TEST_P(LossBasedBweV2Test, NotReadyToUseInStartPhase) {
- ExplicitKeyValueConfig key_value_config(
- "WebRTC-Bwe-LossBasedBweV2/"
- "Enabled:true,UseInStartPhase:true/");
- LossBasedBweV2 loss_based_bandwidth_estimator(&key_value_config);
- // Make sure that the estimator is not ready to use in start phase because of
- // lacking TWCC feedback.
- EXPECT_FALSE(loss_based_bandwidth_estimator.ReadyToUseInStartPhase());
-}
-
-TEST_P(LossBasedBweV2Test,
- ReadyToUseInStartPhase) {
- ExplicitKeyValueConfig key_value_config(
- "WebRTC-Bwe-LossBasedBweV2/"
- "Enabled:true,ObservationDurationLowerBound:200ms,UseInStartPhase:true/");
- LossBasedBweV2 loss_based_bandwidth_estimator(&key_value_config);
- std::vector<PacketResult> enough_feedback =
- CreatePacketResultsWithReceivedPackets(
- /*first_packet_timestamp=*/Timestamp::Zero());
-
- loss_based_bandwidth_estimator.UpdateBandwidthEstimate(
- enough_feedback, /*delay_based_estimate=*/DataRate::KilobitsPerSec(600),
- BandwidthUsage::kBwNormal,
- /*probe_estimate=*/absl::nullopt,
- /*upper_link_capacity=*/DataRate::PlusInfinity(), /*in_alr=*/false);
- EXPECT_TRUE(loss_based_bandwidth_estimator.ReadyToUseInStartPhase());
-}
-
INSTANTIATE_TEST_SUITE_P(LossBasedBweV2Tests,
LossBasedBweV2Test,
::testing::Bool());
diff --git a/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.cc b/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.cc
index 3102466..9f97c06 100644
--- a/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.cc
+++ b/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.cc
@@ -492,8 +492,7 @@
// We trust the REMB and/or delay-based estimate during the first 2 seconds if
// we haven't had any packet loss reported, to allow startup bitrate probing.
- if (last_fraction_loss_ == 0 && IsInStartPhase(at_time) &&
- !loss_based_bandwidth_estimator_v2_.ReadyToUseInStartPhase()) {
+ if (last_fraction_loss_ == 0 && IsInStartPhase(at_time)) {
DataRate new_bitrate = current_target_;
// TODO(srte): We should not allow the new_bitrate to be larger than the
// receiver limit here.
@@ -504,6 +503,9 @@
if (LossBasedBandwidthEstimatorV1Enabled()) {
loss_based_bandwidth_estimator_v1_.Initialize(new_bitrate);
}
+ if (LossBasedBandwidthEstimatorV2Enabled()) {
+ loss_based_bandwidth_estimator_v2_.SetBandwidthEstimate(new_bitrate);
+ }
if (new_bitrate != current_target_) {
min_bitrate_history_.clear();
