From 1067d31022ad8fd7683679476a58c640b28be20b Mon Sep 17 00:00:00 2001 From: Florent Castelli Date: Wed, 4 Sep 2019 14:58:32 +0200 Subject: [PATCH] Make the stable target rate always less or equal than the target rate This behavior seems to conform to expectations from the rate allocators, using this signal to chose which layers to enable and then distributing the remaining bandwidth to the activated layers. Bug: webrtc:10126 Change-Id: If0e1b27dc672ec2fbb30a5f5ac734e5ed4b42e45 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/151306 Reviewed-by: Sebastian Jansson Commit-Queue: Florent Castelli Cr-Commit-Position: refs/heads/master@{#29065} --- .../send_side_bandwidth_estimation.cc | 11 +++++------ .../send_side_bandwidth_estimation.h | 2 +- .../goog_cc/goog_cc_network_control.cc | 4 ++-- .../goog_cc_network_control_unittest.cc | 18 ++++++++++-------- 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/modules/bitrate_controller/send_side_bandwidth_estimation.cc b/modules/bitrate_controller/send_side_bandwidth_estimation.cc index 393ce943fd..2905e6c8dd 100644 --- a/modules/bitrate_controller/send_side_bandwidth_estimation.cc +++ b/modules/bitrate_controller/send_side_bandwidth_estimation.cc @@ -112,10 +112,10 @@ LinkCapacityTracker::LinkCapacityTracker() LinkCapacityTracker::~LinkCapacityTracker() {} -void LinkCapacityTracker::OnOveruse(DataRate acknowledged_rate, +void LinkCapacityTracker::OnOveruse(DataRate delay_based_bitrate, Timestamp at_time) { capacity_estimate_bps_ = - std::min(capacity_estimate_bps_, acknowledged_rate.bps()); + std::min(capacity_estimate_bps_, delay_based_bitrate.bps()); last_link_capacity_update_ = at_time; } @@ -327,11 +327,10 @@ void SendSideBandwidthEstimation::UpdateReceiverEstimate(Timestamp at_time, void SendSideBandwidthEstimation::UpdateDelayBasedEstimate(Timestamp at_time, DataRate bitrate) { - if (acknowledged_rate_) { - if (bitrate < delay_based_bitrate_) { - link_capacity_.OnOveruse(*acknowledged_rate_, at_time); - } + if (bitrate < delay_based_bitrate_) { + link_capacity_.OnOveruse(bitrate, at_time); } + delay_based_bitrate_ = bitrate; CapBitrateToThresholds(at_time, current_bitrate_); } diff --git a/modules/bitrate_controller/send_side_bandwidth_estimation.h b/modules/bitrate_controller/send_side_bandwidth_estimation.h index 7e7f1a5712..be35e5db08 100644 --- a/modules/bitrate_controller/send_side_bandwidth_estimation.h +++ b/modules/bitrate_controller/send_side_bandwidth_estimation.h @@ -35,7 +35,7 @@ class LinkCapacityTracker { public: LinkCapacityTracker(); ~LinkCapacityTracker(); - void OnOveruse(DataRate acknowledged_rate, Timestamp at_time); + void OnOveruse(DataRate delay_based_bitrate, Timestamp at_time); void OnStartingRate(DataRate start_rate); void OnRateUpdate(DataRate acknowledged, Timestamp at_time); void OnRttBackoff(DataRate backoff_rate, Timestamp at_time); 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 2d6516813e..6fb340f79f 100644 --- a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc +++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc @@ -632,8 +632,8 @@ void GoogCcNetworkController::MaybeTriggerOnNetworkChanged( TargetTransferRate target_rate_msg; target_rate_msg.at_time = at_time; target_rate_msg.target_rate = target_rate; - target_rate_msg.stable_target_rate = - bandwidth_estimation_->GetEstimatedLinkCapacity(); + target_rate_msg.stable_target_rate = std::min( + bandwidth_estimation_->GetEstimatedLinkCapacity(), target_rate); target_rate_msg.network_estimate.at_time = at_time; target_rate_msg.network_estimate.round_trip_time = TimeDelta::ms(rtt_ms); target_rate_msg.network_estimate.bandwidth = last_raw_target_rate_; diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc b/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc index dd50896ed4..0da341051c 100644 --- a/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc +++ b/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc @@ -508,18 +508,20 @@ TEST_F(GoogCcNetworkControllerTest, StableEstimateDoesNotVaryInSteadyState) { // Measure variation in steady state. for (int i = 0; i < 20; ++i) { - min_stable_target = - std::min(min_stable_target, client->stable_target_rate()); - max_stable_target = - std::max(max_stable_target, client->stable_target_rate()); - min_target = std::min(min_target, client->link_capacity()); - max_target = std::max(max_target, client->link_capacity()); + auto stable_target_rate = client->stable_target_rate(); + auto target_rate = client->link_capacity(); + EXPECT_LE(stable_target_rate, target_rate); + + min_stable_target = std::min(min_stable_target, stable_target_rate); + max_stable_target = std::max(max_stable_target, stable_target_rate); + min_target = std::min(min_target, target_rate); + max_target = std::max(max_target, target_rate); s.RunFor(TimeDelta::seconds(1)); } - // We expect no variation under the trial in steady state. - EXPECT_GT(min_stable_target / max_stable_target, 0.95); // We should expect drops by at least 15% (default backoff.) EXPECT_LT(min_target / max_target, 0.85); + // We should expect the stable target to be more stable than the immediate one + EXPECT_GE(min_stable_target / max_stable_target, min_target / max_target); } TEST_F(GoogCcNetworkControllerTest,