From 206b77f7ad183d125107113410c9925b5c1a527e Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Mon, 6 Aug 2018 19:15:04 +0200 Subject: [PATCH] Adds start bitrate handling to task queue congestion controller. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds a start bitrate field to TargetRateConstraints since this is required in VideoSendStreamTests. This also reduces the differences between the old SendSideCongestionController and the new TaskQueue based version. Bug: webrtc:9586 Change-Id: I5d3d1414e9d30b51723c911a0bf2e96b876c04e5 Reviewed-on: https://webrtc-review.googlesource.com/92624 Reviewed-by: Björn Terelius Reviewed-by: Karl Wiberg Commit-Queue: Sebastian Jansson Cr-Commit-Position: refs/heads/master@{#24384} --- api/transport/network_types.h | 2 +- .../bbr/bbr_network_controller.cc | 4 ++-- .../bbr/bbr_network_controller_unittest.cc | 2 +- .../goog_cc/goog_cc_network_control.cc | 5 +++-- .../rtp/send_side_congestion_controller.cc | 22 ++++++++++--------- 5 files changed, 19 insertions(+), 16 deletions(-) diff --git a/api/transport/network_types.h b/api/transport/network_types.h index a389716882..8e9526beb2 100644 --- a/api/transport/network_types.h +++ b/api/transport/network_types.h @@ -45,6 +45,7 @@ struct TargetRateConstraints { Timestamp at_time = Timestamp::Infinity(); absl::optional min_data_rate; absl::optional max_data_rate; + absl::optional starting_rate; }; // Send side information @@ -62,7 +63,6 @@ struct NetworkRouteChange { // The TargetRateConstraints are set here so they can be changed synchronously // when network route changes. TargetRateConstraints constraints; - absl::optional starting_rate; }; struct PacedPacketInfo { diff --git a/modules/congestion_controller/bbr/bbr_network_controller.cc b/modules/congestion_controller/bbr/bbr_network_controller.cc index 890b285502..b6bc98347a 100644 --- a/modules/congestion_controller/bbr/bbr_network_controller.cc +++ b/modules/congestion_controller/bbr/bbr_network_controller.cc @@ -311,8 +311,8 @@ NetworkControlUpdate BbrNetworkController::OnNetworkRouteChange( NetworkRouteChange msg) { constraints_ = msg.constraints; Reset(); - if (msg.starting_rate) - default_bandwidth_ = *msg.starting_rate; + if (msg.constraints.starting_rate) + default_bandwidth_ = *msg.constraints.starting_rate; rtt_stats_.OnConnectionMigration(); return CreateRateUpdate(msg.at_time); diff --git a/modules/congestion_controller/bbr/bbr_network_controller_unittest.cc b/modules/congestion_controller/bbr/bbr_network_controller_unittest.cc index 68bc8ceff4..b4fcf95a94 100644 --- a/modules/congestion_controller/bbr/bbr_network_controller_unittest.cc +++ b/modules/congestion_controller/bbr/bbr_network_controller_unittest.cc @@ -72,7 +72,7 @@ NetworkRouteChange CreateRouteChange(Timestamp at_time, route_change.constraints.at_time = at_time; route_change.constraints.min_data_rate = min_rate; route_change.constraints.max_data_rate = max_rate; - route_change.starting_rate = start_rate; + route_change.constraints.starting_rate = start_rate; return route_change; } } // namespace 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 4c33054fea..d8bd1fda8a 100644 --- a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc +++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc @@ -160,7 +160,8 @@ NetworkControlUpdate GoogCcNetworkController::OnNetworkRouteChange( NetworkRouteChange msg) { int64_t min_bitrate_bps = GetBpsOrDefault(msg.constraints.min_data_rate, -1); int64_t max_bitrate_bps = GetBpsOrDefault(msg.constraints.max_data_rate, -1); - int64_t start_bitrate_bps = GetBpsOrDefault(msg.starting_rate, -1); + int64_t start_bitrate_bps = + GetBpsOrDefault(msg.constraints.starting_rate, -1); ClampBitrates(&start_bitrate_bps, &min_bitrate_bps, &max_bitrate_bps); @@ -283,7 +284,7 @@ NetworkControlUpdate GoogCcNetworkController::OnTargetRateConstraints( TargetRateConstraints constraints) { NetworkControlUpdate update; update.probe_cluster_configs = - UpdateBitrateConstraints(constraints, absl::nullopt); + UpdateBitrateConstraints(constraints, constraints.starting_rate); MaybeTriggerOnNetworkChanged(&update, constraints.at_time); return update; } diff --git a/modules/congestion_controller/rtp/send_side_congestion_controller.cc b/modules/congestion_controller/rtp/send_side_congestion_controller.cc index 661abb99f3..19de1aeae3 100644 --- a/modules/congestion_controller/rtp/send_side_congestion_controller.cc +++ b/modules/congestion_controller/rtp/send_side_congestion_controller.cc @@ -107,6 +107,7 @@ std::vector PacketResultsFromRtpFeedbackVector( TargetRateConstraints ConvertConstraints(int min_bitrate_bps, int max_bitrate_bps, + int start_bitrate_bps, const Clock* clock) { TargetRateConstraints msg; msg.at_time = Timestamp::ms(clock->TimeInMilliseconds()); @@ -114,6 +115,8 @@ TargetRateConstraints ConvertConstraints(int min_bitrate_bps, min_bitrate_bps >= 0 ? DataRate::bps(min_bitrate_bps) : DataRate::Zero(); msg.max_data_rate = max_bitrate_bps > 0 ? DataRate::bps(max_bitrate_bps) : DataRate::Infinity(); + if (start_bitrate_bps > 0) + msg.starting_rate = DataRate::bps(start_bitrate_bps); return msg; } @@ -366,8 +369,8 @@ SendSideCongestionController::SendSideCongestionController( pacer_queue_update_task_(nullptr), controller_task_(nullptr), task_queue_(task_queue) { - initial_config_.constraints = - ConvertConstraints(min_bitrate_bps, max_bitrate_bps, clock_); + initial_config_.constraints = ConvertConstraints( + min_bitrate_bps, max_bitrate_bps, start_bitrate_bps, clock_); RTC_DCHECK(start_bitrate_bps > 0); initial_config_.starting_bandwidth = DataRate::bps(start_bitrate_bps); } @@ -442,8 +445,8 @@ void SendSideCongestionController::RegisterNetworkObserver( void SendSideCongestionController::SetBweBitrates(int min_bitrate_bps, int start_bitrate_bps, int max_bitrate_bps) { - TargetRateConstraints constraints = - ConvertConstraints(min_bitrate_bps, max_bitrate_bps, clock_); + TargetRateConstraints constraints = ConvertConstraints( + min_bitrate_bps, max_bitrate_bps, start_bitrate_bps, clock_); task_queue_->PostTask([this, constraints, start_bitrate_bps]() { RTC_DCHECK_RUN_ON(task_queue_); if (controller_) { @@ -482,17 +485,16 @@ void SendSideCongestionController::OnNetworkRouteChanged( NetworkRouteChange msg; msg.at_time = Timestamp::ms(clock_->TimeInMilliseconds()); - msg.constraints = - ConvertConstraints(min_bitrate_bps, max_bitrate_bps, clock_); - if (start_bitrate_bps > 0) - msg.starting_rate = DataRate::bps(start_bitrate_bps); + msg.constraints = ConvertConstraints(min_bitrate_bps, max_bitrate_bps, + start_bitrate_bps, clock_); + task_queue_->PostTask([this, msg]() { RTC_DCHECK_RUN_ON(task_queue_); if (controller_) { control_handler_->PostUpdates(controller_->OnNetworkRouteChange(msg)); } else { - if (msg.starting_rate) - initial_config_.starting_bandwidth = *msg.starting_rate; + if (msg.constraints.starting_rate) + initial_config_.starting_bandwidth = *msg.constraints.starting_rate; initial_config_.constraints = msg.constraints; } pacer_controller_->OnNetworkRouteChange(msg);