From dc62ae432d8eebf02887c6379d4bfcefa5414015 Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Thu, 7 Mar 2019 10:57:41 +0100 Subject: [PATCH] Cleanup of constraints configuration in GoogCcNetworkController. Bug: webrtc:9887 Change-Id: Ic12cc477ae96dac0890337d3f7aa8ff031ff6687 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/126003 Commit-Queue: Sebastian Jansson Reviewed-by: Jonas Olsson Cr-Commit-Position: refs/heads/master@{#27014} --- .../goog_cc/goog_cc_network_control.cc | 115 +++++++----------- .../goog_cc/goog_cc_network_control.h | 10 +- 2 files changed, 54 insertions(+), 71 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 778d638951..7652be668b 100644 --- a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc +++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc @@ -42,21 +42,6 @@ constexpr TimeDelta kLossUpdateInterval = TimeDelta::Millis<1000>(); // overshoots from the encoder. const float kDefaultPaceMultiplier = 2.5f; -// Makes sure that the bitrate and the min, max values are in valid range. -static void ClampBitrates(int64_t* bitrate_bps, - int64_t* min_bitrate_bps, - int64_t* max_bitrate_bps) { - // TODO(holmer): We should make sure the default bitrates are set to 10 kbps, - // and that we don't try to set the min bitrate to 0 from any applications. - // The congestion controller should allow a min bitrate of 0. - if (*min_bitrate_bps < congestion_controller::GetMinBitrateBps()) - *min_bitrate_bps = congestion_controller::GetMinBitrateBps(); - if (*max_bitrate_bps > 0) - *max_bitrate_bps = std::max(*min_bitrate_bps, *max_bitrate_bps); - if (*bitrate_bps > 0) - *bitrate_bps = std::max(*min_bitrate_bps, *bitrate_bps); -} - std::vector ReceivedPacketsFeedbackAsRtp( const TransportPacketsFeedback report) { std::vector packet_feedback_vector; @@ -147,54 +132,38 @@ NetworkControlUpdate GoogCcNetworkController::OnNetworkAvailability( NetworkControlUpdate GoogCcNetworkController::OnNetworkRouteChange( NetworkRouteChange msg) { - int64_t min_bitrate_bps = GetBpsOrDefault(msg.constraints.min_data_rate, 0); - int64_t max_bitrate_bps = GetBpsOrDefault(msg.constraints.max_data_rate, -1); - int64_t start_bitrate_bps = - GetBpsOrDefault(msg.constraints.starting_rate, -1); - - ClampBitrates(&start_bitrate_bps, &min_bitrate_bps, &max_bitrate_bps); - if (safe_reset_on_route_change_) { - absl::optional estimated_bitrate_bps; + absl::optional estimated_bitrate; if (safe_reset_acknowledged_rate_) { - estimated_bitrate_bps = acknowledged_bitrate_estimator_->bitrate_bps(); - if (!estimated_bitrate_bps) - estimated_bitrate_bps = acknowledged_bitrate_estimator_->PeekBps(); + estimated_bitrate = acknowledged_bitrate_estimator_->bitrate(); + if (!estimated_bitrate) + estimated_bitrate = acknowledged_bitrate_estimator_->PeekRate(); } else { int32_t target_bitrate_bps; uint8_t fraction_loss; int64_t rtt_ms; bandwidth_estimation_->CurrentEstimate(&target_bitrate_bps, &fraction_loss, &rtt_ms); - estimated_bitrate_bps = target_bitrate_bps; + estimated_bitrate = DataRate::bps(target_bitrate_bps); } - if (estimated_bitrate_bps && (!msg.constraints.starting_rate || - estimated_bitrate_bps < start_bitrate_bps)) { - start_bitrate_bps = *estimated_bitrate_bps; - msg.constraints.starting_rate = DataRate::bps(start_bitrate_bps); + if (estimated_bitrate) { + if (msg.constraints.starting_rate) { + msg.constraints.starting_rate = + std::min(*msg.constraints.starting_rate, *estimated_bitrate); + } else { + msg.constraints.starting_rate = estimated_bitrate; + } } } acknowledged_bitrate_estimator_.reset( new AcknowledgedBitrateEstimator(key_value_config_)); probe_bitrate_estimator_.reset(new ProbeBitrateEstimator(event_log_)); - - delay_based_bwe_.reset(new DelayBasedBwe(key_value_config_, event_log_)); - if (msg.constraints.starting_rate) - delay_based_bwe_->SetStartBitrate(*msg.constraints.starting_rate); - // TODO(srte): Use original values instead of converted. - delay_based_bwe_->SetMinBitrate(DataRate::bps(min_bitrate_bps)); - + delay_based_bwe_.reset(new DelayBasedBwe(key_value_config_, event_log_)); bandwidth_estimation_->OnRouteChange(); - bandwidth_estimation_->SetBitrates( - msg.constraints.starting_rate, DataRate::bps(min_bitrate_bps), - msg.constraints.max_data_rate.value_or(DataRate::Infinity()), - msg.at_time); - probe_controller_->Reset(msg.at_time.ms()); NetworkControlUpdate update; - update.probe_cluster_configs = probe_controller_->SetBitrates( - min_bitrate_bps, start_bitrate_bps, max_bitrate_bps, msg.at_time.ms()); + update.probe_cluster_configs = ResetConstraints(msg.constraints); MaybeTriggerOnNetworkChanged(&update, msg.at_time); return update; } @@ -204,8 +173,7 @@ NetworkControlUpdate GoogCcNetworkController::OnProcessInterval( NetworkControlUpdate update; if (initial_config_) { update.probe_cluster_configs = - UpdateBitrateConstraints(initial_config_->constraints, - initial_config_->constraints.starting_rate); + ResetConstraints(initial_config_->constraints); update.pacer_config = GetPacingRates(msg.at_time); if (initial_config_->stream_based_config.requests_alr_probing) { @@ -327,34 +295,45 @@ NetworkControlUpdate GoogCcNetworkController::OnStreamsConfig( NetworkControlUpdate GoogCcNetworkController::OnTargetRateConstraints( TargetRateConstraints constraints) { NetworkControlUpdate update; - update.probe_cluster_configs = - UpdateBitrateConstraints(constraints, constraints.starting_rate); + update.probe_cluster_configs = ResetConstraints(constraints); MaybeTriggerOnNetworkChanged(&update, constraints.at_time); return update; } -std::vector -GoogCcNetworkController::UpdateBitrateConstraints( - TargetRateConstraints constraints, - absl::optional starting_rate) { - int64_t min_bitrate_bps = GetBpsOrDefault(constraints.min_data_rate, 0); - int64_t max_bitrate_bps = GetBpsOrDefault(constraints.max_data_rate, -1); - int64_t start_bitrate_bps = GetBpsOrDefault(starting_rate, -1); +void GoogCcNetworkController::ClampConstraints() { + // TODO(holmer): We should make sure the default bitrates are set to 10 kbps, + // and that we don't try to set the min bitrate to 0 from any applications. + // The congestion controller should allow a min bitrate of 0. + min_data_rate_ = + std::max(min_data_rate_, congestion_controller::GetMinBitrate()); + if (max_data_rate_ < min_data_rate_) { + RTC_LOG(LS_WARNING) << "max bitrate smaller than min bitrate"; + max_data_rate_ = min_data_rate_; + } + if (starting_rate_ && starting_rate_ < min_data_rate_) { + RTC_LOG(LS_WARNING) << "start bitrate smaller than min bitrate"; + starting_rate_ = min_data_rate_; + } +} - ClampBitrates(&start_bitrate_bps, &min_bitrate_bps, &max_bitrate_bps); +std::vector GoogCcNetworkController::ResetConstraints( + TargetRateConstraints new_constraints) { + min_data_rate_ = new_constraints.min_data_rate.value_or(DataRate::Zero()); + max_data_rate_ = + new_constraints.max_data_rate.value_or(DataRate::PlusInfinity()); + starting_rate_ = new_constraints.starting_rate; + ClampConstraints(); - std::vector probes(probe_controller_->SetBitrates( - min_bitrate_bps, start_bitrate_bps, max_bitrate_bps, - constraints.at_time.ms())); + bandwidth_estimation_->SetBitrates(starting_rate_, min_data_rate_, + max_data_rate_, new_constraints.at_time); - bandwidth_estimation_->SetBitrates( - starting_rate, DataRate::bps(min_bitrate_bps), - constraints.max_data_rate.value_or(DataRate::Infinity()), - constraints.at_time); - if (starting_rate) - delay_based_bwe_->SetStartBitrate(*starting_rate); - delay_based_bwe_->SetMinBitrate(DataRate::bps(min_bitrate_bps)); - return probes; + if (starting_rate_) + delay_based_bwe_->SetStartBitrate(*starting_rate_); + delay_based_bwe_->SetMinBitrate(min_data_rate_); + + return probe_controller_->SetBitrates( + min_data_rate_.bps(), GetBpsOrDefault(starting_rate_, -1), + max_data_rate_.bps_or(-1), new_constraints.at_time.ms()); } NetworkControlUpdate GoogCcNetworkController::OnTransportLossReport( diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control.h b/modules/congestion_controller/goog_cc/goog_cc_network_control.h index c86774a35d..a32dd77eb0 100644 --- a/modules/congestion_controller/goog_cc/goog_cc_network_control.h +++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.h @@ -63,9 +63,9 @@ class GoogCcNetworkController : public NetworkControllerInterface { private: friend class GoogCcStatePrinter; - std::vector UpdateBitrateConstraints( - TargetRateConstraints constraints, - absl::optional starting_rate); + std::vector ResetConstraints( + TargetRateConstraints new_constraints); + void ClampConstraints(); void MaybeTriggerOnNetworkChanged(NetworkControlUpdate* update, Timestamp at_time); PacerConfig GetPacingRates(Timestamp at_time) const; @@ -92,6 +92,10 @@ class GoogCcNetworkController : public NetworkControllerInterface { absl::optional initial_config_; + DataRate min_data_rate_ = DataRate::Zero(); + DataRate max_data_rate_ = DataRate::PlusInfinity(); + absl::optional starting_rate_; + bool first_packet_sent_ = false; Timestamp next_loss_update_ = Timestamp::MinusInfinity();