From 43d0b98fe5adc4d566c7d862a576cc16ceab3fb4 Mon Sep 17 00:00:00 2001 From: Bjorn Terelius Date: Thu, 28 Jun 2018 16:38:05 +0200 Subject: [PATCH] Clean up RateControlInput struct, used by bandwidth estimation. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove unused member noise_var from RateControlInput struct. Rename incoming_bitrate to estimated_throughput_bps to reflect that the AimdRateControl may be running on either the send side or the receive side. Bug: webrtc:9411 Change-Id: Ie1ae0c29dc9559ef389993144e69fcd41684f1a4 Reviewed-on: https://webrtc-review.googlesource.com/83728 Reviewed-by: Stefan Holmer Reviewed-by: Anastasia Koloskova Commit-Queue: Björn Terelius Cr-Commit-Position: refs/heads/master@{#23783} --- .../congestion_controller/delay_based_bwe.cc | 4 +- .../goog_cc/delay_based_bwe.cc | 4 +- .../aimd_rate_control.cc | 68 ++++++++++--------- .../aimd_rate_control.h | 27 ++++---- .../aimd_rate_control_unittest.cc | 4 +- .../remote_bitrate_estimator/bwe_defines.cc | 7 +- .../include/bwe_defines.h | 6 +- .../remote_bitrate_estimator_abs_send_time.cc | 3 +- .../remote_bitrate_estimator_single_stream.cc | 7 +- 9 files changed, 60 insertions(+), 70 deletions(-) diff --git a/modules/congestion_controller/delay_based_bwe.cc b/modules/congestion_controller/delay_based_bwe.cc index ceca36d58f..163dc2846e 100644 --- a/modules/congestion_controller/delay_based_bwe.cc +++ b/modules/congestion_controller/delay_based_bwe.cc @@ -282,9 +282,7 @@ DelayBasedBwe::Result DelayBasedBwe::MaybeUpdateEstimate( bool DelayBasedBwe::UpdateEstimate(int64_t now_ms, absl::optional acked_bitrate_bps, uint32_t* target_bitrate_bps) { - // TODO(terelius): RateControlInput::noise_var is deprecated and will be - // removed. In the meantime, we set it to zero. - const RateControlInput input(delay_detector_->State(), acked_bitrate_bps, 0); + const RateControlInput input(delay_detector_->State(), acked_bitrate_bps); *target_bitrate_bps = rate_control_.Update(&input, now_ms); return rate_control_.ValidEstimate(); } diff --git a/modules/congestion_controller/goog_cc/delay_based_bwe.cc b/modules/congestion_controller/goog_cc/delay_based_bwe.cc index 818af94f3f..cb5b15a487 100644 --- a/modules/congestion_controller/goog_cc/delay_based_bwe.cc +++ b/modules/congestion_controller/goog_cc/delay_based_bwe.cc @@ -286,9 +286,7 @@ DelayBasedBwe::Result DelayBasedBwe::MaybeUpdateEstimate( bool DelayBasedBwe::UpdateEstimate(int64_t now_ms, absl::optional acked_bitrate_bps, uint32_t* target_bitrate_bps) { - // TODO(terelius): RateControlInput::noise_var is deprecated and will be - // removed. In the meantime, we set it to zero. - const RateControlInput input(delay_detector_->State(), acked_bitrate_bps, 0); + const RateControlInput input(delay_detector_->State(), acked_bitrate_bps); *target_bitrate_bps = rate_control_.Update(&input, now_ms); return rate_control_.ValidEstimate(); } diff --git a/modules/remote_bitrate_estimator/aimd_rate_control.cc b/modules/remote_bitrate_estimator/aimd_rate_control.cc index 8be5e49834..6fbe450880 100644 --- a/modules/remote_bitrate_estimator/aimd_rate_control.cc +++ b/modules/remote_bitrate_estimator/aimd_rate_control.cc @@ -58,13 +58,13 @@ AimdRateControl::AimdRateControl() : min_configured_bitrate_bps_(congestion_controller::GetMinBitrateBps()), max_configured_bitrate_bps_(30000000), current_bitrate_bps_(max_configured_bitrate_bps_), - latest_incoming_bitrate_bps_(current_bitrate_bps_), + latest_estimated_throughput_bps_(current_bitrate_bps_), avg_max_bitrate_kbps_(-1.0f), var_max_bitrate_kbps_(0.4f), rate_control_state_(kRcHold), rate_control_region_(kRcMaxUnknown), time_last_bitrate_change_(-1), - time_first_incoming_estimate_(-1), + time_first_throughput_estimate_(-1), bitrate_is_initialized_(false), beta_(webrtc::field_trial::IsEnabled(kBweBackOffFactorExperiment) ? ReadBackoffFactor() @@ -80,7 +80,7 @@ AimdRateControl::~AimdRateControl() {} void AimdRateControl::SetStartBitrate(int start_bitrate_bps) { current_bitrate_bps_ = start_bitrate_bps; - latest_incoming_bitrate_bps_ = current_bitrate_bps_; + latest_estimated_throughput_bps_ = current_bitrate_bps_; bitrate_is_initialized_ = true; } @@ -104,8 +104,9 @@ int64_t AimdRateControl::GetFeedbackInterval() const { kMaxFeedbackIntervalMs); } -bool AimdRateControl::TimeToReduceFurther(int64_t time_now, - uint32_t incoming_bitrate_bps) const { +bool AimdRateControl::TimeToReduceFurther( + int64_t time_now, + uint32_t estimated_throughput_bps) const { const int64_t bitrate_reduction_interval = std::max(std::min(rtt_, 200), 10); if (time_now - time_last_bitrate_change_ >= bitrate_reduction_interval) { @@ -115,7 +116,7 @@ bool AimdRateControl::TimeToReduceFurther(int64_t time_now, // TODO(terelius/holmer): Investigate consequences of increasing // the threshold to 0.95 * LatestEstimate(). const uint32_t threshold = static_cast(0.5 * LatestEstimate()); - return incoming_bitrate_bps < threshold; + return estimated_throughput_bps < threshold; } return false; } @@ -138,12 +139,13 @@ uint32_t AimdRateControl::Update(const RateControlInput* input, if (!bitrate_is_initialized_) { const int64_t kInitializationTimeMs = 5000; RTC_DCHECK_LE(kBitrateWindowMs, kInitializationTimeMs); - if (time_first_incoming_estimate_ < 0) { - if (input->incoming_bitrate) - time_first_incoming_estimate_ = now_ms; - } else if (now_ms - time_first_incoming_estimate_ > kInitializationTimeMs && - input->incoming_bitrate) { - current_bitrate_bps_ = *input->incoming_bitrate; + if (time_first_throughput_estimate_ < 0) { + if (input->estimated_throughput_bps) + time_first_throughput_estimate_ = now_ms; + } else if (now_ms - time_first_throughput_estimate_ > + kInitializationTimeMs && + input->estimated_throughput_bps) { + current_bitrate_bps_ = *input->estimated_throughput_bps; bitrate_is_initialized_ = true; } } @@ -189,10 +191,10 @@ int AimdRateControl::GetExpectedBandwidthPeriodMs() const { uint32_t AimdRateControl::ChangeBitrate(uint32_t new_bitrate_bps, const RateControlInput& input, int64_t now_ms) { - uint32_t incoming_bitrate_bps = - input.incoming_bitrate.value_or(latest_incoming_bitrate_bps_); - if (input.incoming_bitrate) - latest_incoming_bitrate_bps_ = *input.incoming_bitrate; + uint32_t estimated_throughput_bps = + input.estimated_throughput_bps.value_or(latest_estimated_throughput_bps_); + if (input.estimated_throughput_bps) + latest_estimated_throughput_bps_ = *input.estimated_throughput_bps; // An over-use should always trigger us to reduce the bitrate, even though // we have not yet established our first estimate. By acting on the over-use, @@ -203,9 +205,9 @@ uint32_t AimdRateControl::ChangeBitrate(uint32_t new_bitrate_bps, ChangeState(input, now_ms); // Calculated here because it's used in multiple places. - const float incoming_bitrate_kbps = incoming_bitrate_bps / 1000.0f; + const float estimated_throughput_kbps = estimated_throughput_bps / 1000.0f; // Calculate the max bit rate std dev given the normalized - // variance and the current incoming bit rate. + // variance and the current throughput bitrate. const float std_max_bit_rate = sqrt(var_max_bitrate_kbps_ * avg_max_bitrate_kbps_); switch (rate_control_state_) { @@ -214,7 +216,7 @@ uint32_t AimdRateControl::ChangeBitrate(uint32_t new_bitrate_bps, case kRcIncrease: if (avg_max_bitrate_kbps_ >= 0 && - incoming_bitrate_kbps > + estimated_throughput_kbps > avg_max_bitrate_kbps_ + 3 * std_max_bit_rate) { ChangeRegion(kRcMaxUnknown); avg_max_bitrate_kbps_ = -1.0; @@ -236,7 +238,7 @@ uint32_t AimdRateControl::ChangeBitrate(uint32_t new_bitrate_bps, // Set bit rate to something slightly lower than max // to get rid of any self-induced delay. new_bitrate_bps = - static_cast(beta_ * incoming_bitrate_bps + 0.5); + static_cast(beta_ * estimated_throughput_bps + 0.5); if (new_bitrate_bps > current_bitrate_bps_) { // Avoid increasing the rate when over-using. if (rate_control_region_ != kRcMaxUnknown) { @@ -248,7 +250,7 @@ uint32_t AimdRateControl::ChangeBitrate(uint32_t new_bitrate_bps, ChangeRegion(kRcNearMax); if (bitrate_is_initialized_ && - incoming_bitrate_bps < current_bitrate_bps_) { + estimated_throughput_bps < current_bitrate_bps_) { constexpr float kDegradationFactor = 0.9f; if (smoothing_experiment_ && new_bitrate_bps < @@ -261,13 +263,13 @@ uint32_t AimdRateControl::ChangeBitrate(uint32_t new_bitrate_bps, last_decrease_ = current_bitrate_bps_ - new_bitrate_bps; } } - if (incoming_bitrate_kbps < + if (estimated_throughput_kbps < avg_max_bitrate_kbps_ - 3 * std_max_bit_rate) { avg_max_bitrate_kbps_ = -1.0f; } bitrate_is_initialized_ = true; - UpdateMaxBitRateEstimate(incoming_bitrate_kbps); + UpdateMaxThroughputEstimate(estimated_throughput_kbps); // Stay on hold until the pipes are cleared. rate_control_state_ = kRcHold; time_last_bitrate_change_ = now_ms; @@ -276,16 +278,17 @@ uint32_t AimdRateControl::ChangeBitrate(uint32_t new_bitrate_bps, default: assert(false); } - return ClampBitrate(new_bitrate_bps, incoming_bitrate_bps); + return ClampBitrate(new_bitrate_bps, estimated_throughput_bps); } -uint32_t AimdRateControl::ClampBitrate(uint32_t new_bitrate_bps, - uint32_t incoming_bitrate_bps) const { +uint32_t AimdRateControl::ClampBitrate( + uint32_t new_bitrate_bps, + uint32_t estimated_throughput_bps) const { // Don't change the bit rate if the send side is too far off. // We allow a bit more lag at very low rates to not too easily get stuck if // the encoder produces uneven outputs. const uint32_t max_bitrate_bps = - static_cast(1.5f * incoming_bitrate_bps) + 10000; + static_cast(1.5f * estimated_throughput_bps) + 10000; if (new_bitrate_bps > current_bitrate_bps_ && new_bitrate_bps > max_bitrate_bps) { new_bitrate_bps = std::max(current_bitrate_bps_, max_bitrate_bps); @@ -315,21 +318,22 @@ uint32_t AimdRateControl::AdditiveRateIncrease(int64_t now_ms, GetNearMaxIncreaseRateBps() / 1000); } -void AimdRateControl::UpdateMaxBitRateEstimate(float incoming_bitrate_kbps) { +void AimdRateControl::UpdateMaxThroughputEstimate( + float estimated_throughput_kbps) { const float alpha = 0.05f; if (avg_max_bitrate_kbps_ == -1.0f) { - avg_max_bitrate_kbps_ = incoming_bitrate_kbps; + avg_max_bitrate_kbps_ = estimated_throughput_kbps; } else { avg_max_bitrate_kbps_ = - (1 - alpha) * avg_max_bitrate_kbps_ + alpha * incoming_bitrate_kbps; + (1 - alpha) * avg_max_bitrate_kbps_ + alpha * estimated_throughput_kbps; } // Estimate the max bit rate variance and normalize the variance // with the average max bit rate. const float norm = std::max(avg_max_bitrate_kbps_, 1.0f); var_max_bitrate_kbps_ = (1 - alpha) * var_max_bitrate_kbps_ + - alpha * (avg_max_bitrate_kbps_ - incoming_bitrate_kbps) * - (avg_max_bitrate_kbps_ - incoming_bitrate_kbps) / norm; + alpha * (avg_max_bitrate_kbps_ - estimated_throughput_kbps) * + (avg_max_bitrate_kbps_ - estimated_throughput_kbps) / norm; // 0.4 ~= 14 kbit/s at 500 kbit/s if (var_max_bitrate_kbps_ < 0.4f) { var_max_bitrate_kbps_ = 0.4f; diff --git a/modules/remote_bitrate_estimator/aimd_rate_control.h b/modules/remote_bitrate_estimator/aimd_rate_control.h index 4de15d04b9..0ff36bbffa 100644 --- a/modules/remote_bitrate_estimator/aimd_rate_control.h +++ b/modules/remote_bitrate_estimator/aimd_rate_control.h @@ -26,18 +26,19 @@ class AimdRateControl { AimdRateControl(); ~AimdRateControl(); - // Returns true if there is a valid estimate of the incoming bitrate, false - // otherwise. + // Returns true if the target bitrate has been initialized. This happens + // either if it has been explicitly set via SetStartBitrate/SetEstimate, or if + // we have measured a throughput. bool ValidEstimate() const; void SetStartBitrate(int start_bitrate_bps); void SetMinBitrate(int min_bitrate_bps); int64_t GetFeedbackInterval() const; // Returns true if the bitrate estimate hasn't been changed for more than - // an RTT, or if the incoming_bitrate is less than half of the current + // an RTT, or if the estimated_throughput is less than half of the current // estimate. Should be used to decide if we should reduce the rate further // when over-using. bool TimeToReduceFurther(int64_t time_now, - uint32_t incoming_bitrate_bps) const; + uint32_t estimated_throughput_bps) const; uint32_t LatestEstimate() const; void SetRtt(int64_t rtt); uint32_t Update(const RateControlInput* input, int64_t now_ms); @@ -49,40 +50,40 @@ class AimdRateControl { int GetExpectedBandwidthPeriodMs() const; private: - // Update the target bitrate according based on, among other things, - // the current rate control state, the current target bitrate and the incoming - // bitrate. When in the "increase" state the bitrate will be increased either + // Update the target bitrate based on, among other things, the current rate + // control state, the current target bitrate and the estimated throughput. + // When in the "increase" state the bitrate will be increased either // additively or multiplicatively depending on the rate control region. When // in the "decrease" state the bitrate will be decreased to slightly below the - // incoming bitrate. When in the "hold" state the bitrate will be kept + // current throughput. When in the "hold" state the bitrate will be kept // constant to allow built up queues to drain. uint32_t ChangeBitrate(uint32_t current_bitrate, const RateControlInput& input, int64_t now_ms); // Clamps new_bitrate_bps to within the configured min bitrate and a linear - // function of the incoming bitrate, so that the new bitrate can't grow too + // function of the throughput, so that the new bitrate can't grow too // large compared to the bitrate actually being received by the other end. uint32_t ClampBitrate(uint32_t new_bitrate_bps, - uint32_t incoming_bitrate_bps) const; + uint32_t estimated_throughput_bps) const; uint32_t MultiplicativeRateIncrease(int64_t now_ms, int64_t last_ms, uint32_t current_bitrate_bps) const; uint32_t AdditiveRateIncrease(int64_t now_ms, int64_t last_ms) const; void UpdateChangePeriod(int64_t now_ms); - void UpdateMaxBitRateEstimate(float incoming_bit_rate_kbps); + void UpdateMaxThroughputEstimate(float estimated_throughput_kbps); void ChangeState(const RateControlInput& input, int64_t now_ms); void ChangeRegion(RateControlRegion region); uint32_t min_configured_bitrate_bps_; uint32_t max_configured_bitrate_bps_; uint32_t current_bitrate_bps_; - uint32_t latest_incoming_bitrate_bps_; + uint32_t latest_estimated_throughput_bps_; float avg_max_bitrate_kbps_; float var_max_bitrate_kbps_; RateControlState rate_control_state_; RateControlRegion rate_control_region_; int64_t time_last_bitrate_change_; - int64_t time_first_incoming_estimate_; + int64_t time_first_throughput_estimate_; bool bitrate_is_initialized_; float beta_; int64_t rtt_; diff --git a/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc b/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc index a561cdf20c..5cde7cedaf 100644 --- a/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc +++ b/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc @@ -43,9 +43,9 @@ AimdRateControlStates CreateAimdRateControlStates() { void UpdateRateControl(const AimdRateControlStates& states, const BandwidthUsage& bandwidth_usage, - absl::optional bitrate, + absl::optional throughput_estimate, int64_t now_ms) { - RateControlInput input(bandwidth_usage, bitrate, now_ms); + RateControlInput input(bandwidth_usage, throughput_estimate); states.aimd_rate_control->Update(&input, now_ms); } diff --git a/modules/remote_bitrate_estimator/bwe_defines.cc b/modules/remote_bitrate_estimator/bwe_defines.cc index af2ec7648e..35f1d19cc3 100644 --- a/modules/remote_bitrate_estimator/bwe_defines.cc +++ b/modules/remote_bitrate_estimator/bwe_defines.cc @@ -30,11 +30,8 @@ int GetMinBitrateBps() { RateControlInput::RateControlInput( BandwidthUsage bw_state, - const absl::optional& incoming_bitrate, - double noise_var) - : bw_state(bw_state), - incoming_bitrate(incoming_bitrate), - noise_var(noise_var) {} + const absl::optional& estimated_throughput_bps) + : bw_state(bw_state), estimated_throughput_bps(estimated_throughput_bps) {} RateControlInput::~RateControlInput() = default; diff --git a/modules/remote_bitrate_estimator/include/bwe_defines.h b/modules/remote_bitrate_estimator/include/bwe_defines.h index de393b6e0a..2905118ea3 100644 --- a/modules/remote_bitrate_estimator/include/bwe_defines.h +++ b/modules/remote_bitrate_estimator/include/bwe_defines.h @@ -48,13 +48,11 @@ enum RateControlRegion { kRcNearMax, kRcAboveMax, kRcMaxUnknown }; struct RateControlInput { RateControlInput(BandwidthUsage bw_state, - const absl::optional& incoming_bitrate, - double noise_var); + const absl::optional& estimated_throughput_bps); ~RateControlInput(); BandwidthUsage bw_state; - absl::optional incoming_bitrate; - double noise_var; + absl::optional estimated_throughput_bps; }; } // namespace webrtc diff --git a/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc b/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc index 045674d003..375028b35c 100644 --- a/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc +++ b/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc @@ -331,8 +331,7 @@ void RemoteBitrateEstimatorAbsSendTime::IncomingPacketInfo( // We also have to update the estimate immediately if we are overusing // and the target bitrate is too high compared to what we are receiving. const RateControlInput input(detector_.State(), - incoming_bitrate_.Rate(arrival_time_ms), - estimator_->var_noise()); + incoming_bitrate_.Rate(arrival_time_ms)); target_bitrate_bps = remote_rate_.Update(&input, now_ms); update_estimate = remote_rate_.ValidEstimate(); ssrcs = Keys(ssrcs_); diff --git a/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc b/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc index 8188215b29..7efda7a872 100644 --- a/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc +++ b/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc @@ -157,7 +157,6 @@ int64_t RemoteBitrateEstimatorSingleStream::TimeUntilNextProcess() { void RemoteBitrateEstimatorSingleStream::UpdateEstimate(int64_t now_ms) { BandwidthUsage bw_state = BandwidthUsage::kBwNormal; - double sum_var_noise = 0.0; SsrcOveruseEstimatorMap::iterator it = overuse_detectors_.begin(); while (it != overuse_detectors_.end()) { const int64_t time_of_last_received_packet = @@ -169,7 +168,6 @@ void RemoteBitrateEstimatorSingleStream::UpdateEstimate(int64_t now_ms) { delete it->second; overuse_detectors_.erase(it++); } else { - sum_var_noise += it->second->estimator.var_noise(); // Make sure that we trigger an over-use if any of the over-use detectors // is detecting over-use. if (it->second->detector.State() > bw_state) { @@ -184,10 +182,7 @@ void RemoteBitrateEstimatorSingleStream::UpdateEstimate(int64_t now_ms) { } AimdRateControl* remote_rate = GetRemoteRate(); - double mean_noise_var = - sum_var_noise / static_cast(overuse_detectors_.size()); - const RateControlInput input(bw_state, incoming_bitrate_.Rate(now_ms), - mean_noise_var); + const RateControlInput input(bw_state, incoming_bitrate_.Rate(now_ms)); uint32_t target_bitrate = remote_rate->Update(&input, now_ms); if (remote_rate->ValidEstimate()) { process_interval_ms_ = remote_rate->GetFeedbackInterval();