From 5f133dbc29e837bcd6fa6a2bdeba317eea5a02b3 Mon Sep 17 00:00:00 2001 From: Per Kjellander Date: Tue, 25 Jan 2022 11:21:41 +0100 Subject: [PATCH] Field trial for pacing relative lower link capacity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds a field trial to change the pacing rate to pace at a rate relative to max(bwe ,lower link capacity) Bug: none Change-Id: Ibe9ef3e08eb422e9abff6488780e82188958eeeb Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/248865 Reviewed-by: Björn Terelius Commit-Queue: Per Kjellander Cr-Commit-Position: refs/heads/main@{#35795} --- .../goog_cc/goog_cc_network_control.cc | 18 +++++-- .../goog_cc/goog_cc_network_control.h | 1 + .../goog_cc_network_control_unittest.cc | 52 +++++++++++++++++++ 3 files changed, 68 insertions(+), 3 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 2344f45a65..ba656be234 100644 --- a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc +++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc @@ -22,6 +22,7 @@ #include #include "absl/strings/match.h" +#include "api/units/data_rate.h" #include "api/units/time_delta.h" #include "logging/rtc_event_log/events/rtc_event_remote_estimate.h" #include "modules/congestion_controller/goog_cc/alr_detector.h" @@ -88,6 +89,9 @@ GoogCcNetworkController::GoogCcNetworkController(NetworkControllerConfig config, RateControlSettings::ParseFromKeyValueConfig(key_value_config_)), loss_based_stable_rate_( IsEnabled(key_value_config_, "WebRTC-Bwe-LossBasedStableRate")), + pace_at_max_of_bwe_and_lower_link_capacity_( + IsEnabled(key_value_config_, + "WebRTC-Bwe-PaceAtMaxOfBweAndLowerLinkCapacity")), probe_controller_( new ProbeController(key_value_config_, config.event_log)), congestion_window_pushback_controller_( @@ -694,9 +698,17 @@ void GoogCcNetworkController::MaybeTriggerOnNetworkChanged( PacerConfig GoogCcNetworkController::GetPacingRates(Timestamp at_time) const { // Pacing rate is based on target rate before congestion window pushback, // because we don't want to build queues in the pacer when pushback occurs. - DataRate pacing_rate = - std::max(min_total_allocated_bitrate_, last_loss_based_target_rate_) * - pacing_factor_; + DataRate pacing_rate = DataRate::Zero(); + if (pace_at_max_of_bwe_and_lower_link_capacity_ && estimate_) { + pacing_rate = + std::max({min_total_allocated_bitrate_, estimate_->link_capacity_lower, + last_loss_based_target_rate_}) * + pacing_factor_; + } else { + pacing_rate = + std::max(min_total_allocated_bitrate_, last_loss_based_target_rate_) * + pacing_factor_; + } DataRate padding_rate = std::min(max_padding_rate_, last_pushback_target_rate_); PacerConfig msg; 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 6dd70c8969..946c076939 100644 --- a/modules/congestion_controller/goog_cc/goog_cc_network_control.h +++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.h @@ -94,6 +94,7 @@ class GoogCcNetworkController : public NetworkControllerInterface { const bool limit_probes_lower_than_throughput_estimate_; const RateControlSettings rate_control_settings_; const bool loss_based_stable_rate_; + const bool pace_at_max_of_bwe_and_lower_link_capacity_; const std::unique_ptr probe_controller_; const std::unique_ptr 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 ed80b731bf..0552109a0d 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 @@ -13,6 +13,7 @@ #include "api/test/network_emulation/create_cross_traffic.h" #include "api/test/network_emulation/cross_traffic.h" #include "api/transport/goog_cc_factory.h" +#include "api/transport/network_types.h" #include "api/units/data_rate.h" #include "logging/rtc_event_log/mock/mock_rtc_event_log.h" #include "test/field_trial.h" @@ -359,6 +360,57 @@ TEST(GoogCcNetworkControllerTest, UpdatesDelayBasedEstimate) { EXPECT_LT(*target_bitrate_after_delay, *target_bitrate_before_delay); } +TEST(GoogCcNetworkControllerTest, PaceAtMaxOfLowerLinkCapacityAndBwe) { + ScopedFieldTrials trial( + "WebRTC-Bwe-PaceAtMaxOfBweAndLowerLinkCapacity/Enabled/"); + NetworkControllerTestFixture fixture; + std::unique_ptr controller = + fixture.CreateController(); + Timestamp current_time = Timestamp::Millis(123); + NetworkControlUpdate update = + controller->OnProcessInterval({.at_time = current_time}); + current_time += TimeDelta::Millis(100); + NetworkStateEstimate network_estimate = {.link_capacity_lower = + 10 * kInitialBitrate}; + update = controller->OnNetworkStateEstimate(network_estimate); + // OnNetworkStateEstimate does not trigger processing a new estimate. So add a + // dummy loss report to trigger a BWE update in the next process interval. + TransportLossReport loss_report; + loss_report.start_time = current_time; + loss_report.end_time = current_time; + loss_report.receive_time = current_time; + loss_report.packets_received_delta = 50; + loss_report.packets_lost_delta = 1; + update = controller->OnTransportLossReport(loss_report); + update = controller->OnProcessInterval({.at_time = current_time}); + ASSERT_TRUE(update.pacer_config); + ASSERT_TRUE(update.target_rate); + ASSERT_LT(update.target_rate->target_rate, + network_estimate.link_capacity_lower); + EXPECT_EQ(update.pacer_config->data_rate().kbps(), + network_estimate.link_capacity_lower.kbps() * kDefaultPacingRate); + + current_time += TimeDelta::Millis(100); + // Set a low link capacity estimate and verify that pacing rate is set + // relative to loss based/delay based estimate. + network_estimate = {.link_capacity_lower = 0.5 * kInitialBitrate}; + update = controller->OnNetworkStateEstimate(network_estimate); + // Again, we need to inject a dummy loss report to trigger an update of the + // BWE in the next process interval. + loss_report.start_time = current_time; + loss_report.end_time = current_time; + loss_report.receive_time = current_time; + loss_report.packets_received_delta = 50; + loss_report.packets_lost_delta = 0; + update = controller->OnTransportLossReport(loss_report); + update = controller->OnProcessInterval({.at_time = current_time}); + ASSERT_TRUE(update.target_rate); + ASSERT_GT(update.target_rate->target_rate, + network_estimate.link_capacity_lower); + EXPECT_EQ(update.pacer_config->data_rate().kbps(), + update.target_rate->target_rate.kbps() * kDefaultPacingRate); +} + // Test congestion window pushback on network delay happens. TEST(GoogCcScenario, CongestionWindowPushbackOnNetworkDelay) { auto factory = CreateFeedbackOnlyFactory();