From c4ca83c2bdf2353c93ff34cf0b5105c736f7b35c Mon Sep 17 00:00:00 2001 From: Diep Bui Date: Wed, 1 Jun 2022 22:11:00 +0000 Subject: [PATCH] Send side bwe receives the delay based state even if the delay based bwe does not change its estimate. Bug: webrtc:12707 Change-Id: If67dcc6d1cb70dc763ab65bdb8426de100bcc626 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/261312 Reviewed-by: Per Kjellander Commit-Queue: Per Kjellander Cr-Commit-Position: refs/heads/main@{#37084} --- .../goog_cc/goog_cc_network_control.cc | 8 ++++---- .../goog_cc/send_side_bandwidth_estimation.cc | 18 +++++++----------- .../goog_cc/send_side_bandwidth_estimation.h | 9 +++------ .../send_side_bandwidth_estimation_unittest.cc | 10 +++------- 4 files changed, 17 insertions(+), 28 deletions(-) diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc index 5e85c9fe7a..1a2932794a 100644 --- a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc +++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc @@ -554,13 +554,13 @@ NetworkControlUpdate GoogCcNetworkController::OnTransportPacketsFeedback( } // Since SetSendBitrate now resets the delay-based estimate, we have to // call UpdateDelayBasedEstimate after SetSendBitrate. - bandwidth_estimation_->UpdateDelayBasedEstimate( - report.feedback_time, result.target_bitrate, - result.delay_detector_state); + bandwidth_estimation_->UpdateDelayBasedEstimate(report.feedback_time, + result.target_bitrate); // Update the estimate in the ProbeController, in case we want to probe. MaybeTriggerOnNetworkChanged(&update, report.feedback_time); } - bandwidth_estimation_->UpdateLossBasedEstimatorFromFeedbackVector(report); + bandwidth_estimation_->UpdateLossBasedEstimator(report, + result.delay_detector_state); recovered_from_overuse = result.recovered_from_overuse; backoff_in_alr = result.backoff_in_alr; 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 d0d7b83ae6..852485b189 100644 --- a/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.cc +++ b/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.cc @@ -230,8 +230,7 @@ SendSideBandwidthEstimation::SendSideBandwidthEstimation( bitrate_threshold_(kDefaultBitrateThreshold), loss_based_bandwidth_estimator_v1_(key_value_config), loss_based_bandwidth_estimator_v2_(key_value_config), - disable_receiver_limit_caps_only_("Disabled"), - delay_detector_state_(BandwidthUsage::kBwNormal) { + disable_receiver_limit_caps_only_("Disabled") { RTC_DCHECK(event_log); if (BweLossExperimentIsEnabled()) { uint32_t bitrate_threshold_kbps; @@ -273,7 +272,6 @@ void SendSideBandwidthEstimation::OnRouteChange() { uma_update_state_ = kNoUpdate; uma_rtt_state_ = kNoUpdate; last_rtc_event_log_ = Timestamp::MinusInfinity(); - delay_detector_state_ = BandwidthUsage::kBwNormal; } void SendSideBandwidthEstimation::SetBitrates( @@ -333,12 +331,9 @@ void SendSideBandwidthEstimation::UpdateReceiverEstimate(Timestamp at_time, ApplyTargetLimits(at_time); } -void SendSideBandwidthEstimation::UpdateDelayBasedEstimate( - Timestamp at_time, - DataRate bitrate, - BandwidthUsage delay_detector_state) { +void SendSideBandwidthEstimation::UpdateDelayBasedEstimate(Timestamp at_time, + DataRate bitrate) { link_capacity_.UpdateDelayBasedEstimate(at_time, bitrate); - delay_detector_state_ = delay_detector_state; // TODO(srte): Ensure caller passes PlusInfinity, not zero, to represent no // limitation. delay_based_limit_ = bitrate.IsZero() ? DataRate::PlusInfinity() : bitrate; @@ -362,15 +357,16 @@ void SendSideBandwidthEstimation::SetAcknowledgedRate( } } -void SendSideBandwidthEstimation::UpdateLossBasedEstimatorFromFeedbackVector( - const TransportPacketsFeedback& report) { +void SendSideBandwidthEstimation::UpdateLossBasedEstimator( + const TransportPacketsFeedback& report, + BandwidthUsage delay_detector_state) { if (LossBasedBandwidthEstimatorV1Enabled()) { loss_based_bandwidth_estimator_v1_.UpdateLossStatistics( report.packet_feedbacks, report.feedback_time); } if (LossBasedBandwidthEstimatorV2Enabled()) { loss_based_bandwidth_estimator_v2_.UpdateBandwidthEstimate( - report.packet_feedbacks, delay_based_limit_, delay_detector_state_); + report.packet_feedbacks, delay_based_limit_, delay_detector_state); } } diff --git a/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.h b/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.h index cbcad86df4..d79d285c19 100644 --- a/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.h +++ b/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.h @@ -98,9 +98,7 @@ class SendSideBandwidthEstimation { void UpdateReceiverEstimate(Timestamp at_time, DataRate bandwidth); // Call when a new delay-based estimate is available. - void UpdateDelayBasedEstimate(Timestamp at_time, - DataRate bitrate, - BandwidthUsage delay_detector_state); + void UpdateDelayBasedEstimate(Timestamp at_time, DataRate bitrate); // Call when we receive a RTCP message with a ReceiveBlock. void UpdatePacketsLost(int64_t packets_lost, @@ -119,8 +117,8 @@ class SendSideBandwidthEstimation { int GetMinBitrate() const; void SetAcknowledgedRate(absl::optional acknowledged_rate, Timestamp at_time); - void UpdateLossBasedEstimatorFromFeedbackVector( - const TransportPacketsFeedback& report); + void UpdateLossBasedEstimator(const TransportPacketsFeedback& report, + BandwidthUsage delay_detector_state); private: friend class GoogCcStatePrinter; @@ -203,7 +201,6 @@ class SendSideBandwidthEstimation { LossBasedBandwidthEstimation loss_based_bandwidth_estimator_v1_; LossBasedBweV2 loss_based_bandwidth_estimator_v2_; FieldTrialFlag disable_receiver_limit_caps_only_; - BandwidthUsage delay_detector_state_; }; } // namespace webrtc #endif // MODULES_CONGESTION_CONTROLLER_GOOG_CC_SEND_SIDE_BANDWIDTH_ESTIMATION_H_ diff --git a/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation_unittest.cc b/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation_unittest.cc index e3db866cf7..85ce401098 100644 --- a/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation_unittest.cc +++ b/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation_unittest.cc @@ -10,7 +10,6 @@ #include "modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.h" -#include "api/network_state_predictor.h" #include "api/rtc_event_log/rtc_event.h" #include "logging/rtc_event_log/events/rtc_event_bwe_update_loss_based.h" #include "logging/rtc_event_log/mock/mock_rtc_event_log.h" @@ -55,8 +54,7 @@ void TestProbing(bool use_delay_based) { // Initial REMB applies immediately. if (use_delay_based) { bwe.UpdateDelayBasedEstimate(Timestamp::Millis(now_ms), - DataRate::BitsPerSec(kRembBps), - BandwidthUsage::kBwNormal); + DataRate::BitsPerSec(kRembBps)); } else { bwe.UpdateReceiverEstimate(Timestamp::Millis(now_ms), DataRate::BitsPerSec(kRembBps)); @@ -68,8 +66,7 @@ void TestProbing(bool use_delay_based) { now_ms += 2001; if (use_delay_based) { bwe.UpdateDelayBasedEstimate(Timestamp::Millis(now_ms), - DataRate::BitsPerSec(kSecondRembBps), - BandwidthUsage::kBwNormal); + DataRate::BitsPerSec(kSecondRembBps)); } else { bwe.UpdateReceiverEstimate(Timestamp::Millis(now_ms), DataRate::BitsPerSec(kSecondRembBps)); @@ -160,8 +157,7 @@ TEST(SendSideBweTest, SettingSendBitrateOverridesDelayBasedEstimate) { Timestamp::Millis(now_ms)); bwe.UpdateDelayBasedEstimate(Timestamp::Millis(now_ms), - DataRate::BitsPerSec(kDelayBasedBitrateBps), - BandwidthUsage::kBwNormal); + DataRate::BitsPerSec(kDelayBasedBitrateBps)); bwe.UpdateEstimate(Timestamp::Millis(now_ms)); EXPECT_GE(bwe.target_rate().bps(), kInitialBitrateBps); EXPECT_LE(bwe.target_rate().bps(), kDelayBasedBitrateBps);