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 223a92885e..4c0f5fc5ee 100644 --- a/modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc +++ b/modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc @@ -209,9 +209,8 @@ void LossBasedBweV2::SetMinMaxBitrate(DataRate min_bitrate, void LossBasedBweV2::SetProbeBitrate(absl::optional probe_bitrate) { if (probe_bitrate.has_value() && IsValid(probe_bitrate.value())) { - if (!IsValid(probe_bitrate_) || probe_bitrate_ > probe_bitrate.value()) { - probe_bitrate_ = probe_bitrate.value(); - } + probe_bitrate_ = probe_bitrate.value(); + last_probe_timestamp_ = last_send_time_most_recent_observation_; } } @@ -229,7 +228,7 @@ void LossBasedBweV2::UpdateBandwidthEstimate( << "The estimator must be enabled before it can be used."; return; } - SetProbeBitrate(probe_bitrate); + if (packet_results.empty()) { RTC_LOG(LS_VERBOSE) << "The estimate cannot be updated without any loss statistics."; @@ -240,6 +239,8 @@ void LossBasedBweV2::UpdateBandwidthEstimate( return; } + SetProbeBitrate(probe_bitrate); + if (!IsValid(current_estimate_.loss_limited_bandwidth)) { RTC_LOG(LS_VERBOSE) << "The estimator must be initialized before it can be used."; @@ -300,24 +301,27 @@ void LossBasedBweV2::UpdateBandwidthEstimate( : config_->bandwidth_rampup_upper_bound_factor * (*acknowledged_bitrate_); } - - // Use probe bitrate as the estimate as probe bitrate is trusted to be - // correct. After being used, the probe bitrate is reset. - if (config_->probe_integration_enabled && IsValid(probe_bitrate_)) { - best_candidate.loss_limited_bandwidth = - std::min(probe_bitrate_, best_candidate.loss_limited_bandwidth); - probe_bitrate_ = DataRate::MinusInfinity(); - } } if (IsEstimateIncreasingWhenLossLimited(best_candidate) && - best_candidate.loss_limited_bandwidth < delay_based_estimate) { + best_candidate.loss_limited_bandwidth < delay_based_estimate_) { current_state_ = LossBasedState::kIncreasing; } else if (best_candidate.loss_limited_bandwidth < delay_based_estimate_) { current_state_ = LossBasedState::kDecreasing; } else if (best_candidate.loss_limited_bandwidth >= delay_based_estimate_) { current_state_ = LossBasedState::kDelayBasedEstimate; } + + // Use probe bitrate as the estimate limit when probes are requested. + if (config_->probe_integration_enabled && IsValid(probe_bitrate_) && + IsRequestingProbe()) { + if (last_probe_timestamp_ + config_->probe_expiration >= + last_send_time_most_recent_observation_) { + best_candidate.loss_limited_bandwidth = + std::min(probe_bitrate_, best_candidate.loss_limited_bandwidth); + } + } + current_estimate_ = best_candidate; if (IsBandwidthLimitedDueToLoss() && @@ -412,6 +416,8 @@ absl::optional LossBasedBweV2::CreateConfig( "SlopeOfBweHighLossFunc", 1000); FieldTrialParameter probe_integration_enabled("ProbeIntegrationEnabled", false); + FieldTrialParameter probe_expiration("ProbeExpiration", + TimeDelta::Seconds(10)); FieldTrialParameter bound_by_upper_link_capacity_when_loss_limited( "BoundByUpperLinkCapacityWhenLossLimited", true); FieldTrialParameter not_use_acked_rate_in_alr("NotUseAckedRateInAlr", @@ -449,6 +455,7 @@ absl::optional LossBasedBweV2::CreateConfig( &use_acked_bitrate_only_when_overusing, ¬_increase_if_inherent_loss_less_than_average_loss, &probe_integration_enabled, + &probe_expiration, &high_loss_rate_threshold, &bandwidth_cap_at_high_loss_rate, &slope_of_bwe_high_loss_func, @@ -514,6 +521,7 @@ absl::optional LossBasedBweV2::CreateConfig( bandwidth_cap_at_high_loss_rate.Get(); config->slope_of_bwe_high_loss_func = slope_of_bwe_high_loss_func.Get(); config->probe_integration_enabled = probe_integration_enabled.Get(); + config->probe_expiration = probe_expiration.Get(); config->bound_by_upper_link_capacity_when_loss_limited = bound_by_upper_link_capacity_when_loss_limited.Get(); config->not_use_acked_rate_in_alr = not_use_acked_rate_in_alr.Get(); @@ -1084,4 +1092,8 @@ bool LossBasedBweV2::IsBandwidthLimitedDueToLoss() const { return current_state_ != LossBasedState::kDelayBasedEstimate; } +bool LossBasedBweV2::IsRequestingProbe() const { + return current_state_ == LossBasedState::kIncreasing; +} + } // 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 84f2378df3..f5a6396de2 100644 --- a/modules/congestion_controller/goog_cc/loss_based_bwe_v2.h +++ b/modules/congestion_controller/goog_cc/loss_based_bwe_v2.h @@ -40,6 +40,8 @@ class LossBasedBweV2 { struct Result { ~Result() = default; DataRate bandwidth_estimate = DataRate::Zero(); + // State is used by goog_cc, which later sends probe requests to probe + // controller if state is kIncreasing. LossBasedState state = LossBasedState::kDelayBasedEstimate; }; // Creates a disabled `LossBasedBweV2` if the @@ -112,6 +114,7 @@ class LossBasedBweV2 { DataRate bandwidth_cap_at_high_loss_rate = DataRate::MinusInfinity(); double slope_of_bwe_high_loss_func = 1000.0; bool probe_integration_enabled = false; + TimeDelta probe_expiration = TimeDelta::Zero(); bool bound_by_upper_link_capacity_when_loss_limited = false; bool not_use_acked_rate_in_alr = false; }; @@ -177,6 +180,7 @@ class LossBasedBweV2 { const ChannelParameters& best_candidate); bool IsBandwidthLimitedDueToLoss() const; void SetProbeBitrate(absl::optional probe_bitrate); + bool IsRequestingProbe() const; absl::optional acknowledged_bitrate_; absl::optional config_; @@ -198,6 +202,7 @@ class LossBasedBweV2 { DataRate probe_bitrate_ = DataRate::PlusInfinity(); DataRate delay_based_estimate_ = DataRate::PlusInfinity(); DataRate upper_link_capacity_ = DataRate::PlusInfinity(); + Timestamp last_probe_timestamp_ = Timestamp::MinusInfinity(); }; } // 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 67714202f0..d745f37a5c 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 @@ -1148,13 +1148,13 @@ TEST_P(LossBasedBweV2Test, DataRate::KilobitsPerSec(600)); } -TEST_P(LossBasedBweV2Test, UseProbeResultWhenRecoveringFromLoss) { +TEST_P(LossBasedBweV2Test, LimitByProbeResultWhenRecoveringFromLoss) { ExplicitKeyValueConfig key_value_config( "WebRTC-Bwe-LossBasedBweV2/" "Enabled:true,CandidateFactors:1.2|1|0.5,AckedRateCandidate:true," "ObservationWindowSize:2,ObservationDurationLowerBound:200ms," - "InstantUpperBoundBwBalance:10000kbps," - "DelayBasedCandidate:true,MaxIncreaseFactor:1000," + "InstantUpperBoundBwBalance:10000kbps,DelayedIncreaseWindow:100s," + "DelayBasedCandidate:true,MaxIncreaseFactor:1.3," "BwRampupUpperBoundFactor:2.0,ProbeIntegrationEnabled:true/"); LossBasedBweV2 loss_based_bandwidth_estimator(&key_value_config); DataRate delay_based_estimate = DataRate::KilobitsPerSec(5000); @@ -1172,7 +1172,7 @@ TEST_P(LossBasedBweV2Test, UseProbeResultWhenRecoveringFromLoss) { /*probe_estimate=*/absl::nullopt, /*upper_link_capacity=*/DataRate::PlusInfinity(), /*in_alr=*/false); - // Network recovers after loss. + // Network recovers after loss DataRate probe_estimate = DataRate::KilobitsPerSec(300); std::vector enough_feedback_2 = CreatePacketResultsWithReceivedPackets( @@ -1183,9 +1183,82 @@ TEST_P(LossBasedBweV2Test, UseProbeResultWhenRecoveringFromLoss) { probe_estimate, /*upper_link_capacity=*/DataRate::PlusInfinity(), /*in_alr=*/false); + for (int i = 2; i < 5; ++i) { + enough_feedback_2 = CreatePacketResultsWithReceivedPackets( + /*first_packet_timestamp=*/Timestamp::Zero() + + kObservationDurationLowerBound * i); + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + enough_feedback_2, delay_based_estimate, BandwidthUsage::kBwNormal, + /*probe_estimate=*/absl::nullopt, + /*upper_link_capacity=*/DataRate::PlusInfinity(), + /*in_alr=*/false); + LossBasedBweV2::Result result_after_recovery = + loss_based_bandwidth_estimator.GetLossBasedResult(); + EXPECT_LE(result_after_recovery.bandwidth_estimate, probe_estimate); + } +} + +TEST_P(LossBasedBweV2Test, NotLimitByProbeResultWhenProbeResultIsExpired) { + ExplicitKeyValueConfig key_value_config( + "WebRTC-Bwe-LossBasedBweV2/" + "Enabled:true,CandidateFactors:1.2|1|0.5,AckedRateCandidate:true," + "ObservationWindowSize:2,ObservationDurationLowerBound:200ms," + "InstantUpperBoundBwBalance:10000kbps,DelayedIncreaseWindow:100s," + "DelayBasedCandidate:true,MaxIncreaseFactor:1.3," + "BwRampupUpperBoundFactor:2.0,ProbeIntegrationEnabled:true," + "ProbeExpiration:10s/"); + LossBasedBweV2 loss_based_bandwidth_estimator(&key_value_config); + DataRate delay_based_estimate = DataRate::KilobitsPerSec(5000); + DataRate acked_rate = DataRate::KilobitsPerSec(300); + loss_based_bandwidth_estimator.SetBandwidthEstimate( + DataRate::KilobitsPerSec(600)); + loss_based_bandwidth_estimator.SetAcknowledgedBitrate(acked_rate); + + // Create some loss to create the loss limited scenario. + std::vector enough_feedback_1 = + CreatePacketResultsWith100pLossRate( + /*first_packet_timestamp=*/Timestamp::Zero()); + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + enough_feedback_1, delay_based_estimate, BandwidthUsage::kBwNormal, + /*probe_estimate=*/absl::nullopt, + /*upper_link_capacity=*/DataRate::PlusInfinity(), /*in_alr=*/false); + + // Network recovers after loss + DataRate probe_estimate = DataRate::KilobitsPerSec(300); + std::vector enough_feedback_2 = + CreatePacketResultsWithReceivedPackets( + /*first_packet_timestamp=*/Timestamp::Zero() + + kObservationDurationLowerBound); + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + enough_feedback_2, delay_based_estimate, BandwidthUsage::kBwNormal, + probe_estimate, /*upper_link_capacity=*/DataRate::PlusInfinity(), + /*in_alr=*/false); + + for (int i = 2; i < 5; ++i) { + enough_feedback_2 = CreatePacketResultsWithReceivedPackets( + /*first_packet_timestamp=*/Timestamp::Zero() + + kObservationDurationLowerBound * i); + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + enough_feedback_2, delay_based_estimate, BandwidthUsage::kBwNormal, + /*probe_estimate=*/absl::nullopt, + /*upper_link_capacity=*/DataRate::PlusInfinity(), + /*in_alr=*/false); + } + + std::vector enough_feedback_3 = + CreatePacketResultsWithReceivedPackets( + /*first_packet_timestamp=*/Timestamp::Zero() + + kObservationDurationLowerBound + TimeDelta::Seconds(11)); + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + enough_feedback_3, delay_based_estimate, BandwidthUsage::kBwNormal, + /*probe_estimate=*/absl::nullopt, + /*upper_link_capacity=*/DataRate::PlusInfinity(), + /*in_alr=*/false); + + // Probe result is expired after 10s. LossBasedBweV2::Result result_after_recovery = loss_based_bandwidth_estimator.GetLossBasedResult(); - EXPECT_EQ(result_after_recovery.bandwidth_estimate, probe_estimate); + EXPECT_GT(result_after_recovery.bandwidth_estimate, probe_estimate); } // If BoundByUpperLinkCapacityWhenLossLimited is enabled, the estimate is