From 6704c91061bb4a5a8609985aa83b5d0d31bc31ea Mon Sep 17 00:00:00 2001 From: Ying Wang Date: Thu, 3 Jan 2019 13:58:36 +0100 Subject: [PATCH] Bugfix: Activate pushback on every sent packet. Fix a bug introduced in (https://webrtc-review.googlesource.com/c/src/+/105102) that causes cwnd pushback only active when there is network condition changes. Bug: None Change-Id: I8164d5663304ce2e445db09205f706011ff7d784 Reviewed-on: https://webrtc-review.googlesource.com/c/115945 Commit-Queue: Ying Wang Reviewed-by: Sebastian Jansson Cr-Commit-Position: refs/heads/master@{#26122} --- .../goog_cc/goog_cc_network_control.cc | 44 +++++++++---------- .../goog_cc/goog_cc_network_control.h | 4 +- 2 files changed, 24 insertions(+), 24 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 b69d83837a..bc7777f956 100644 --- a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc +++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc @@ -157,8 +157,8 @@ GoogCcNetworkController::GoogCcNetworkController(RtcEventLog* event_log, acknowledged_bitrate_estimator_( absl::make_unique()), initial_config_(config), - last_target_rate_(*config.constraints.starting_rate), - pushback_target_rate_(last_target_rate_), + last_raw_target_rate_(*config.constraints.starting_rate), + last_pushback_target_rate_(last_raw_target_rate_), pacing_factor_(config.stream_based_config.pacing_factor.value_or( kDefaultPaceMultiplier)), min_pacing_rate_(config.stream_based_config.min_pacing_rate.value_or( @@ -582,7 +582,7 @@ NetworkControlUpdate GoogCcNetworkController::OnTransportPacketsFeedback( const DataSize kMinCwnd = DataSize::bytes(2 * 1500); TimeDelta time_window = TimeDelta::ms(min_feedback_max_rtt_ms + accepted_queue_ms_); - DataSize data_window = last_target_rate_ * time_window; + DataSize data_window = last_raw_target_rate_ * time_window; if (current_data_window_) { data_window = std::max(kMinCwnd, (data_window + current_data_window_.value()) / 2); @@ -642,38 +642,37 @@ void GoogCcNetworkController::MaybeTriggerOnNetworkChanged( BWE_TEST_LOGGING_PLOT(1, "Target_bitrate_kbps", at_time.ms(), estimated_bitrate_bps / 1000); + DataRate target_rate = DataRate::bps(estimated_bitrate_bps); + if (congestion_window_pushback_controller_) { + int64_t pushback_rate = + congestion_window_pushback_controller_->UpdateTargetBitrate( + target_rate.bps()); + pushback_rate = std::max(bandwidth_estimation_->GetMinBitrate(), + pushback_rate); + target_rate = DataRate::bps(pushback_rate); + } + if ((estimated_bitrate_bps != last_estimated_bitrate_bps_) || (fraction_loss != last_estimated_fraction_loss_) || - (rtt_ms != last_estimated_rtt_ms_)) { + (rtt_ms != last_estimated_rtt_ms_) || + (target_rate != last_pushback_target_rate_)) { + last_pushback_target_rate_ = target_rate; last_estimated_bitrate_bps_ = estimated_bitrate_bps; last_estimated_fraction_loss_ = fraction_loss; last_estimated_rtt_ms_ = rtt_ms; alr_detector_->SetEstimatedBitrate(estimated_bitrate_bps); - last_target_rate_ = DataRate::bps(estimated_bitrate_bps); + last_raw_target_rate_ = DataRate::bps(estimated_bitrate_bps); DataRate bandwidth = use_stable_bandwidth_estimate_ ? bandwidth_estimation_->GetEstimatedLinkCapacity() - : last_target_rate_; + : last_raw_target_rate_; TimeDelta bwe_period = delay_based_bwe_ ? delay_based_bwe_->GetExpectedBwePeriod() : delay_based_controller_->GetExpectedBandwidthPeriod(); - // Set the target rate to the full estimated bandwidth since the estimation - // for legacy reasons includes target rate constraints. - DataRate target_rate = last_target_rate_; - if (congestion_window_pushback_controller_) { - int64_t pushback_rate = - congestion_window_pushback_controller_->UpdateTargetBitrate( - target_rate.bps()); - pushback_rate = std::max(bandwidth_estimation_->GetMinBitrate(), - pushback_rate); - target_rate = DataRate::bps(pushback_rate); - } - pushback_target_rate_ = target_rate; - TargetTransferRate target_rate_msg; target_rate_msg.at_time = at_time; target_rate_msg.target_rate = target_rate; @@ -686,7 +685,7 @@ void GoogCcNetworkController::MaybeTriggerOnNetworkChanged( update->target_rate = target_rate_msg; auto probes = probe_controller_->SetEstimatedBitrate( - last_target_rate_.bps(), at_time.ms()); + last_raw_target_rate_.bps(), at_time.ms()); update->probe_cluster_configs.insert(update->probe_cluster_configs.end(), probes.begin(), probes.end()); update->pacer_config = GetPacingRates(at_time); @@ -695,8 +694,9 @@ void GoogCcNetworkController::MaybeTriggerOnNetworkChanged( PacerConfig GoogCcNetworkController::GetPacingRates(Timestamp at_time) const { DataRate pacing_rate = - std::max(min_pacing_rate_, last_target_rate_) * pacing_factor_; - DataRate padding_rate = std::min(max_padding_rate_, pushback_target_rate_); + std::max(min_pacing_rate_, last_raw_target_rate_) * pacing_factor_; + DataRate padding_rate = + std::min(max_padding_rate_, last_pushback_target_rate_); PacerConfig msg; msg.at_time = at_time; msg.time_window = TimeDelta::seconds(1); 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 370ce829c6..6ba12ae0d4 100644 --- a/modules/congestion_controller/goog_cc/goog_cc_network_control.h +++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.h @@ -97,8 +97,8 @@ class GoogCcNetworkController : public NetworkControllerInterface { std::deque feedback_max_rtts_; - DataRate last_target_rate_; - DataRate pushback_target_rate_; + DataRate last_raw_target_rate_; + DataRate last_pushback_target_rate_; int32_t last_estimated_bitrate_bps_ = 0; uint8_t last_estimated_fraction_loss_ = 0;