From 88af20356f5abb11c8efe14b692d4ac383c239a4 Mon Sep 17 00:00:00 2001 From: Per Kjellander Date: Mon, 16 May 2022 19:58:40 +0200 Subject: [PATCH] Use ProbeClusterConfig in BitrateProber from GoogCC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of using field trials in BitrateProber for probe duration, use values provided in ProbeClusterConfig from GoogCC. Field trials are instead read in ProbeController. To avoid having to do a thread jump for every ProbeClusterConfig, RtpPacketPacer interface is changed to RtpPacketPacer::CreateProbeClusters(std::vector Deprecates field trial "WebRTC-Bwe-ProbingConfiguration" Change-Id: I3991e4b54770601855a3af2d6a16678f11d41c31 Bug: webrtc:14027 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/261265 Reviewed-by: Erik Språng Commit-Queue: Per Kjellander Cr-Commit-Position: refs/heads/main@{#36911} --- call/rtp_transport_controller_send.cc | 5 +- .../goog_cc/probe_controller.cc | 16 +- .../goog_cc/probe_controller.h | 5 + .../goog_cc/probe_controller_unittest.cc | 422 +++++++++++------- modules/pacing/bitrate_prober.cc | 50 +-- modules/pacing/bitrate_prober.h | 15 +- modules/pacing/bitrate_prober_unittest.cc | 101 +++-- modules/pacing/g3doc/index.md | 7 +- modules/pacing/pacing_controller.cc | 13 +- modules/pacing/pacing_controller.h | 3 + modules/pacing/pacing_controller_unittest.cc | 122 ++--- modules/pacing/rtp_packet_pacer.h | 5 +- modules/pacing/task_queue_paced_sender.cc | 16 +- modules/pacing/task_queue_paced_sender.h | 3 +- .../task_queue_paced_sender_unittest.cc | 44 +- 15 files changed, 478 insertions(+), 349 deletions(-) diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc index 393acc44d0..704b9d51e8 100644 --- a/call/rtp_transport_controller_send.cc +++ b/call/rtp_transport_controller_send.cc @@ -71,7 +71,6 @@ bool IsDisabled(const FieldTrialsView& trials, absl::string_view key) { bool IsRelayed(const rtc::NetworkRoute& route) { return route.local.uses_turn() || route.remote.uses_turn(); } - } // namespace RtpTransportControllerSend::PacerSettings::PacerSettings( @@ -660,8 +659,8 @@ void RtpTransportControllerSend::PostUpdates(NetworkControlUpdate update) { pacer_.SetPacingRates(update.pacer_config->data_rate(), update.pacer_config->pad_rate()); } - for (const auto& probe : update.probe_cluster_configs) { - pacer_.CreateProbeCluster(probe.target_data_rate, probe.id); + if (!update.probe_cluster_configs.empty()) { + pacer_.CreateProbeClusters(std::move(update.probe_cluster_configs)); } if (update.target_rate) { control_handler_->SetTargetRate(*update.target_rate); diff --git a/modules/congestion_controller/goog_cc/probe_controller.cc b/modules/congestion_controller/goog_cc/probe_controller.cc index 9c263ebe52..59021109ba 100644 --- a/modules/congestion_controller/goog_cc/probe_controller.cc +++ b/modules/congestion_controller/goog_cc/probe_controller.cc @@ -28,12 +28,6 @@ namespace webrtc { namespace { -// The minimum number probing packets used. -constexpr int kMinProbePacketsSent = 5; - -// The minimum probing duration in ms. -constexpr int kMinProbeDurationMs = 15; - // Maximum waiting time from the time of initiating probing to getting // the measured results back. constexpr int64_t kMaxWaitingTimeForProbingResultMs = 1000; @@ -101,7 +95,9 @@ ProbeControllerConfig::ProbeControllerConfig( first_allocation_probe_scale("alloc_p1", 1), second_allocation_probe_scale("alloc_p2", 2), allocation_allow_further_probing("alloc_probe_further", false), - allocation_probe_max("alloc_probe_max", DataRate::PlusInfinity()) { + allocation_probe_max("alloc_probe_max", DataRate::PlusInfinity()), + min_probe_packets_sent("min_probe_packets_sent", 5), + min_probe_duration("min_probe_duration", TimeDelta::Millis(15)) { ParseFieldTrial( {&first_exponential_probe_scale, &second_exponential_probe_scale, &further_exponential_probe_scale, &further_probe_threshold, @@ -121,6 +117,8 @@ ProbeControllerConfig::ProbeControllerConfig( {&first_allocation_probe_scale, &second_allocation_probe_scale, &allocation_allow_further_probing, &allocation_probe_max}, key_value_config->Lookup("WebRTC-Bwe-AllocationProbing")); + ParseFieldTrial({&min_probe_packets_sent, &min_probe_duration}, + key_value_config->Lookup("WebRTC-Bwe-ProbingBehavior")); } ProbeControllerConfig::ProbeControllerConfig(const ProbeControllerConfig&) = @@ -429,8 +427,8 @@ std::vector ProbeController::InitiateProbing( config.at_time = Timestamp::Millis(now_ms); config.target_data_rate = DataRate::BitsPerSec(rtc::dchecked_cast(bitrate)); - config.target_duration = TimeDelta::Millis(kMinProbeDurationMs); - config.target_probe_count = kMinProbePacketsSent; + config.target_duration = config_.min_probe_duration; + config.target_probe_count = config_.min_probe_packets_sent; config.id = next_probe_cluster_id_; next_probe_cluster_id_++; MaybeLogProbeClusterCreated(event_log_, config); diff --git a/modules/congestion_controller/goog_cc/probe_controller.h b/modules/congestion_controller/goog_cc/probe_controller.h index 86931ee8b1..3c1b62c30e 100644 --- a/modules/congestion_controller/goog_cc/probe_controller.h +++ b/modules/congestion_controller/goog_cc/probe_controller.h @@ -51,6 +51,11 @@ struct ProbeControllerConfig { FieldTrialOptional second_allocation_probe_scale; FieldTrialFlag allocation_allow_further_probing; FieldTrialParameter allocation_probe_max; + + // The minimum number probing packets used. + FieldTrialParameter min_probe_packets_sent; + // The minimum probing duration. + FieldTrialParameter min_probe_duration; }; // This class controls initiation of probing to estimate initial channel diff --git a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc b/modules/congestion_controller/goog_cc/probe_controller_unittest.cc index 4e9144f54c..23329ee041 100644 --- a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc +++ b/modules/congestion_controller/goog_cc/probe_controller_unittest.cc @@ -18,7 +18,7 @@ #include "logging/rtc_event_log/mock/mock_rtc_event_log.h" #include "rtc_base/logging.h" #include "system_wrappers/include/clock.h" -#include "test/field_trial.h" +#include "test/explicit_key_value_config.h" #include "test/gmock.h" #include "test/gtest.h" @@ -45,326 +45,402 @@ constexpr int kAlrEndedTimeoutMs = 3000; constexpr int kBitrateDropTimeoutMs = 5000; } // namespace -class ProbeControllerTest : public ::testing::Test { - protected: - ProbeControllerTest() : clock_(100000000L) { - probe_controller_.reset( - new ProbeController(&field_trial_config_, &mock_rtc_event_log)); - } - ~ProbeControllerTest() override {} +class ProbeControllerFixture { + public: + explicit ProbeControllerFixture(absl::string_view field_trials = "") + : field_trial_config_(field_trials), clock_(100000000L) {} - std::vector SetNetworkAvailable(bool available) { - NetworkAvailability msg; - msg.at_time = Timestamp::Millis(NowMs()); - msg.network_available = available; - return probe_controller_->OnNetworkAvailability(msg); + std::unique_ptr CreateController() { + return std::make_unique(&field_trial_config_, + &mock_rtc_event_log); } + Timestamp CurrentTime() { return clock_.CurrentTime(); } int64_t NowMs() { return clock_.TimeInMilliseconds(); } + void AdvanceTimeMilliseconds(int64_t delta_ms) { + clock_.AdvanceTimeMilliseconds(delta_ms); + } - FieldTrialBasedConfig field_trial_config_; + ExplicitKeyValueConfig field_trial_config_; SimulatedClock clock_; NiceMock mock_rtc_event_log; - std::unique_ptr probe_controller_; }; -TEST_F(ProbeControllerTest, InitiatesProbingAtStart) { - auto probes = probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps, NowMs()); +TEST(ProbeControllerTest, InitiatesProbingAtStart) { + ProbeControllerFixture fixture; + std::unique_ptr probe_controller = + fixture.CreateController(); + + auto probes = probe_controller->SetBitrates(kMinBitrateBps, kStartBitrateBps, + kMaxBitrateBps, fixture.NowMs()); EXPECT_GE(probes.size(), 2u); } -TEST_F(ProbeControllerTest, ProbeOnlyWhenNetworkIsUp) { - SetNetworkAvailable(false); - auto probes = probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps, NowMs()); +TEST(ProbeControllerTest, SetsDefaultTargetDurationAndTargetProbeCount) { + ProbeControllerFixture fixture; + std::unique_ptr probe_controller = + fixture.CreateController(); + std::vector probes = probe_controller->SetBitrates( + kMinBitrateBps, kStartBitrateBps, kMaxBitrateBps, fixture.NowMs()); + ASSERT_GE(probes.size(), 2u); + + EXPECT_EQ(probes[0].target_duration, TimeDelta::Millis(15)); + EXPECT_EQ(probes[0].target_probe_count, 5); +} + +TEST(ProbeControllerTest, + FieldTrialsOverrideDefaultTargetDurationAndTargetProbeCount) { + ProbeControllerFixture fixture( + "WebRTC-Bwe-ProbingBehavior/" + "min_probe_packets_sent:2,min_probe_duration:123ms/"); + std::unique_ptr probe_controller = + fixture.CreateController(); + std::vector probes = probe_controller->SetBitrates( + kMinBitrateBps, kStartBitrateBps, kMaxBitrateBps, fixture.NowMs()); + ASSERT_GE(probes.size(), 2u); + + EXPECT_EQ(probes[0].target_duration, TimeDelta::Millis(123)); + EXPECT_EQ(probes[0].target_probe_count, 2); +} + +TEST(ProbeControllerTest, ProbeOnlyWhenNetworkIsUp) { + ProbeControllerFixture fixture; + std::unique_ptr probe_controller = + fixture.CreateController(); + auto probes = probe_controller->OnNetworkAvailability( + {.at_time = fixture.CurrentTime(), .network_available = false}); + probes = probe_controller->SetBitrates(kMinBitrateBps, kStartBitrateBps, + kMaxBitrateBps, fixture.NowMs()); EXPECT_EQ(probes.size(), 0u); - probes = SetNetworkAvailable(true); + probes = probe_controller->OnNetworkAvailability( + {.at_time = fixture.CurrentTime(), .network_available = true}); EXPECT_GE(probes.size(), 2u); } -TEST_F(ProbeControllerTest, InitiatesProbingOnMaxBitrateIncrease) { - auto probes = probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps, NowMs()); +TEST(ProbeControllerTest, InitiatesProbingOnMaxBitrateIncrease) { + ProbeControllerFixture fixture; + std::unique_ptr probe_controller = + fixture.CreateController(); + auto probes = probe_controller->SetBitrates(kMinBitrateBps, kStartBitrateBps, + kMaxBitrateBps, fixture.NowMs()); // Long enough to time out exponential probing. - clock_.AdvanceTimeMilliseconds(kExponentialProbingTimeoutMs); - probes = probe_controller_->SetEstimatedBitrate(kStartBitrateBps, NowMs()); - probes = probe_controller_->Process(NowMs()); - probes = probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps + 100, NowMs()); + fixture.AdvanceTimeMilliseconds(kExponentialProbingTimeoutMs); + probes = + probe_controller->SetEstimatedBitrate(kStartBitrateBps, fixture.NowMs()); + probes = probe_controller->Process(fixture.NowMs()); + probes = probe_controller->SetBitrates(kMinBitrateBps, kStartBitrateBps, + kMaxBitrateBps + 100, fixture.NowMs()); EXPECT_EQ(probes.size(), 1u); EXPECT_EQ(probes[0].target_data_rate.bps(), kMaxBitrateBps + 100); } -TEST_F(ProbeControllerTest, ProbesOnMaxBitrateIncreaseOnlyWhenInAlr) { - probe_controller_.reset( - new ProbeController(&field_trial_config_, &mock_rtc_event_log)); - auto probes = probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps, NowMs()); - probes = probe_controller_->SetEstimatedBitrate(kMaxBitrateBps - 1, NowMs()); +TEST(ProbeControllerTest, ProbesOnMaxBitrateIncreaseOnlyWhenInAlr) { + ProbeControllerFixture fixture; + std::unique_ptr probe_controller = + fixture.CreateController(); + auto probes = probe_controller->SetBitrates(kMinBitrateBps, kStartBitrateBps, + kMaxBitrateBps, fixture.NowMs()); + probes = probe_controller->SetEstimatedBitrate(kMaxBitrateBps - 1, + fixture.NowMs()); // Wait long enough to time out exponential probing. - clock_.AdvanceTimeMilliseconds(kExponentialProbingTimeoutMs); - probes = probe_controller_->Process(NowMs()); + fixture.AdvanceTimeMilliseconds(kExponentialProbingTimeoutMs); + probes = probe_controller->Process(fixture.NowMs()); EXPECT_EQ(probes.size(), 0u); // Probe when in alr. - probe_controller_->SetAlrStartTimeMs(clock_.TimeInMilliseconds()); - probes = probe_controller_->OnMaxTotalAllocatedBitrate(kMaxBitrateBps + 1, - NowMs()); + probe_controller->SetAlrStartTimeMs(fixture.NowMs()); + probes = probe_controller->OnMaxTotalAllocatedBitrate(kMaxBitrateBps + 1, + fixture.NowMs()); EXPECT_EQ(probes.size(), 2u); // Do not probe when not in alr. - probe_controller_->SetAlrStartTimeMs(absl::nullopt); - probes = probe_controller_->OnMaxTotalAllocatedBitrate(kMaxBitrateBps + 2, - NowMs()); + probe_controller->SetAlrStartTimeMs(absl::nullopt); + probes = probe_controller->OnMaxTotalAllocatedBitrate(kMaxBitrateBps + 2, + fixture.NowMs()); EXPECT_TRUE(probes.empty()); } -TEST_F(ProbeControllerTest, InitiatesProbingOnMaxBitrateIncreaseAtMaxBitrate) { - auto probes = probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps, NowMs()); +TEST(ProbeControllerTest, InitiatesProbingOnMaxBitrateIncreaseAtMaxBitrate) { + ProbeControllerFixture fixture; + std::unique_ptr probe_controller = + fixture.CreateController(); + auto probes = probe_controller->SetBitrates(kMinBitrateBps, kStartBitrateBps, + kMaxBitrateBps, fixture.NowMs()); // Long enough to time out exponential probing. - clock_.AdvanceTimeMilliseconds(kExponentialProbingTimeoutMs); - probes = probe_controller_->SetEstimatedBitrate(kStartBitrateBps, NowMs()); - probes = probe_controller_->Process(NowMs()); - probes = probe_controller_->SetEstimatedBitrate(kMaxBitrateBps, NowMs()); - probes = probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps + 100, NowMs()); + fixture.AdvanceTimeMilliseconds(kExponentialProbingTimeoutMs); + probes = + probe_controller->SetEstimatedBitrate(kStartBitrateBps, fixture.NowMs()); + probes = probe_controller->Process(fixture.NowMs()); + probes = + probe_controller->SetEstimatedBitrate(kMaxBitrateBps, fixture.NowMs()); + probes = probe_controller->SetBitrates(kMinBitrateBps, kStartBitrateBps, + kMaxBitrateBps + 100, fixture.NowMs()); EXPECT_EQ(probes.size(), 1u); EXPECT_EQ(probes[0].target_data_rate.bps(), kMaxBitrateBps + 100); } -TEST_F(ProbeControllerTest, TestExponentialProbing) { - auto probes = probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps, NowMs()); +TEST(ProbeControllerTest, TestExponentialProbing) { + ProbeControllerFixture fixture; + std::unique_ptr probe_controller = + fixture.CreateController(); + auto probes = probe_controller->SetBitrates(kMinBitrateBps, kStartBitrateBps, + kMaxBitrateBps, fixture.NowMs()); // Repeated probe should only be sent when estimated bitrate climbs above // 0.7 * 6 * kStartBitrateBps = 1260. - probes = probe_controller_->SetEstimatedBitrate(1000, NowMs()); + probes = probe_controller->SetEstimatedBitrate(1000, fixture.NowMs()); EXPECT_EQ(probes.size(), 0u); - probes = probe_controller_->SetEstimatedBitrate(1800, NowMs()); + probes = probe_controller->SetEstimatedBitrate(1800, fixture.NowMs()); EXPECT_EQ(probes.size(), 1u); EXPECT_EQ(probes[0].target_data_rate.bps(), 2 * 1800); } -TEST_F(ProbeControllerTest, TestExponentialProbingTimeout) { - auto probes = probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps, NowMs()); +TEST(ProbeControllerTest, TestExponentialProbingTimeout) { + ProbeControllerFixture fixture; + std::unique_ptr probe_controller = + fixture.CreateController(); + auto probes = probe_controller->SetBitrates(kMinBitrateBps, kStartBitrateBps, + kMaxBitrateBps, fixture.NowMs()); // Advance far enough to cause a time out in waiting for probing result. - clock_.AdvanceTimeMilliseconds(kExponentialProbingTimeoutMs); - probes = probe_controller_->Process(NowMs()); + fixture.AdvanceTimeMilliseconds(kExponentialProbingTimeoutMs); + probes = probe_controller->Process(fixture.NowMs()); - probes = probe_controller_->SetEstimatedBitrate(1800, NowMs()); + probes = probe_controller->SetEstimatedBitrate(1800, fixture.NowMs()); EXPECT_EQ(probes.size(), 0u); } -TEST_F(ProbeControllerTest, RequestProbeInAlr) { - auto probes = probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps, NowMs()); +TEST(ProbeControllerTest, RequestProbeInAlr) { + ProbeControllerFixture fixture; + std::unique_ptr probe_controller = + fixture.CreateController(); + auto probes = probe_controller->SetBitrates(kMinBitrateBps, kStartBitrateBps, + kMaxBitrateBps, fixture.NowMs()); EXPECT_GE(probes.size(), 2u); - probes = probe_controller_->SetEstimatedBitrate(500, NowMs()); + probes = probe_controller->SetEstimatedBitrate(500, fixture.NowMs()); - probe_controller_->SetAlrStartTimeMs(clock_.TimeInMilliseconds()); - clock_.AdvanceTimeMilliseconds(kAlrProbeInterval + 1); - probes = probe_controller_->Process(NowMs()); - probes = probe_controller_->SetEstimatedBitrate(250, NowMs()); - probes = probe_controller_->RequestProbe(NowMs()); + probe_controller->SetAlrStartTimeMs(fixture.NowMs()); + fixture.AdvanceTimeMilliseconds(kAlrProbeInterval + 1); + probes = probe_controller->Process(fixture.NowMs()); + probes = probe_controller->SetEstimatedBitrate(250, fixture.NowMs()); + probes = probe_controller->RequestProbe(fixture.NowMs()); EXPECT_EQ(probes.size(), 1u); EXPECT_EQ(probes[0].target_data_rate.bps(), 0.85 * 500); } -TEST_F(ProbeControllerTest, RequestProbeWhenAlrEndedRecently) { - auto probes = probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps, NowMs()); +TEST(ProbeControllerTest, RequestProbeWhenAlrEndedRecently) { + ProbeControllerFixture fixture; + std::unique_ptr probe_controller = + fixture.CreateController(); + auto probes = probe_controller->SetBitrates(kMinBitrateBps, kStartBitrateBps, + kMaxBitrateBps, fixture.NowMs()); EXPECT_EQ(probes.size(), 2u); - probes = probe_controller_->SetEstimatedBitrate(500, NowMs()); + probes = probe_controller->SetEstimatedBitrate(500, fixture.NowMs()); - probe_controller_->SetAlrStartTimeMs(absl::nullopt); - clock_.AdvanceTimeMilliseconds(kAlrProbeInterval + 1); - probes = probe_controller_->Process(NowMs()); - probes = probe_controller_->SetEstimatedBitrate(250, NowMs()); - probe_controller_->SetAlrEndedTimeMs(clock_.TimeInMilliseconds()); - clock_.AdvanceTimeMilliseconds(kAlrEndedTimeoutMs - 1); - probes = probe_controller_->RequestProbe(NowMs()); + probe_controller->SetAlrStartTimeMs(absl::nullopt); + fixture.AdvanceTimeMilliseconds(kAlrProbeInterval + 1); + probes = probe_controller->Process(fixture.NowMs()); + probes = probe_controller->SetEstimatedBitrate(250, fixture.NowMs()); + probe_controller->SetAlrEndedTimeMs(fixture.NowMs()); + fixture.AdvanceTimeMilliseconds(kAlrEndedTimeoutMs - 1); + probes = probe_controller->RequestProbe(fixture.NowMs()); EXPECT_EQ(probes.size(), 1u); EXPECT_EQ(probes[0].target_data_rate.bps(), 0.85 * 500); } -TEST_F(ProbeControllerTest, RequestProbeWhenAlrNotEndedRecently) { - auto probes = probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps, NowMs()); +TEST(ProbeControllerTest, RequestProbeWhenAlrNotEndedRecently) { + ProbeControllerFixture fixture; + std::unique_ptr probe_controller = + fixture.CreateController(); + auto probes = probe_controller->SetBitrates(kMinBitrateBps, kStartBitrateBps, + kMaxBitrateBps, fixture.NowMs()); EXPECT_EQ(probes.size(), 2u); - probes = probe_controller_->SetEstimatedBitrate(500, NowMs()); + probes = probe_controller->SetEstimatedBitrate(500, fixture.NowMs()); - probe_controller_->SetAlrStartTimeMs(absl::nullopt); - clock_.AdvanceTimeMilliseconds(kAlrProbeInterval + 1); - probes = probe_controller_->Process(NowMs()); - probes = probe_controller_->SetEstimatedBitrate(250, NowMs()); - probe_controller_->SetAlrEndedTimeMs(clock_.TimeInMilliseconds()); - clock_.AdvanceTimeMilliseconds(kAlrEndedTimeoutMs + 1); - probes = probe_controller_->RequestProbe(NowMs()); + probe_controller->SetAlrStartTimeMs(absl::nullopt); + fixture.AdvanceTimeMilliseconds(kAlrProbeInterval + 1); + probes = probe_controller->Process(fixture.NowMs()); + probes = probe_controller->SetEstimatedBitrate(250, fixture.NowMs()); + probe_controller->SetAlrEndedTimeMs(fixture.NowMs()); + fixture.AdvanceTimeMilliseconds(kAlrEndedTimeoutMs + 1); + probes = probe_controller->RequestProbe(fixture.NowMs()); EXPECT_EQ(probes.size(), 0u); } -TEST_F(ProbeControllerTest, RequestProbeWhenBweDropNotRecent) { - auto probes = probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps, NowMs()); +TEST(ProbeControllerTest, RequestProbeWhenBweDropNotRecent) { + ProbeControllerFixture fixture; + std::unique_ptr probe_controller = + fixture.CreateController(); + auto probes = probe_controller->SetBitrates(kMinBitrateBps, kStartBitrateBps, + kMaxBitrateBps, fixture.NowMs()); EXPECT_EQ(probes.size(), 2u); - probes = probe_controller_->SetEstimatedBitrate(500, NowMs()); + probes = probe_controller->SetEstimatedBitrate(500, fixture.NowMs()); - probe_controller_->SetAlrStartTimeMs(clock_.TimeInMilliseconds()); - clock_.AdvanceTimeMilliseconds(kAlrProbeInterval + 1); - probes = probe_controller_->Process(NowMs()); - probes = probe_controller_->SetEstimatedBitrate(250, NowMs()); - clock_.AdvanceTimeMilliseconds(kBitrateDropTimeoutMs + 1); - probes = probe_controller_->RequestProbe(NowMs()); + probe_controller->SetAlrStartTimeMs(fixture.NowMs()); + fixture.AdvanceTimeMilliseconds(kAlrProbeInterval + 1); + probes = probe_controller->Process(fixture.NowMs()); + probes = probe_controller->SetEstimatedBitrate(250, fixture.NowMs()); + fixture.AdvanceTimeMilliseconds(kBitrateDropTimeoutMs + 1); + probes = probe_controller->RequestProbe(fixture.NowMs()); EXPECT_EQ(probes.size(), 0u); } -TEST_F(ProbeControllerTest, PeriodicProbing) { - probe_controller_->EnablePeriodicAlrProbing(true); - auto probes = probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps, NowMs()); +TEST(ProbeControllerTest, PeriodicProbing) { + ProbeControllerFixture fixture; + std::unique_ptr probe_controller = + fixture.CreateController(); + probe_controller->EnablePeriodicAlrProbing(true); + auto probes = probe_controller->SetBitrates(kMinBitrateBps, kStartBitrateBps, + kMaxBitrateBps, fixture.NowMs()); EXPECT_EQ(probes.size(), 2u); - probes = probe_controller_->SetEstimatedBitrate(500, NowMs()); + probes = probe_controller->SetEstimatedBitrate(500, fixture.NowMs()); - int64_t start_time = clock_.TimeInMilliseconds(); + int64_t start_time = fixture.NowMs(); // Expect the controller to send a new probe after 5s has passed. - probe_controller_->SetAlrStartTimeMs(start_time); - clock_.AdvanceTimeMilliseconds(5000); - probes = probe_controller_->Process(NowMs()); + probe_controller->SetAlrStartTimeMs(start_time); + fixture.AdvanceTimeMilliseconds(5000); + probes = probe_controller->Process(fixture.NowMs()); EXPECT_EQ(probes.size(), 1u); EXPECT_EQ(probes[0].target_data_rate.bps(), 1000); - probes = probe_controller_->SetEstimatedBitrate(500, NowMs()); + probes = probe_controller->SetEstimatedBitrate(500, fixture.NowMs()); // The following probe should be sent at 10s into ALR. - probe_controller_->SetAlrStartTimeMs(start_time); - clock_.AdvanceTimeMilliseconds(4000); - probes = probe_controller_->Process(NowMs()); - probes = probe_controller_->SetEstimatedBitrate(500, NowMs()); + probe_controller->SetAlrStartTimeMs(start_time); + fixture.AdvanceTimeMilliseconds(4000); + probes = probe_controller->Process(fixture.NowMs()); + probes = probe_controller->SetEstimatedBitrate(500, fixture.NowMs()); EXPECT_EQ(probes.size(), 0u); - probe_controller_->SetAlrStartTimeMs(start_time); - clock_.AdvanceTimeMilliseconds(1000); - probes = probe_controller_->Process(NowMs()); + probe_controller->SetAlrStartTimeMs(start_time); + fixture.AdvanceTimeMilliseconds(1000); + probes = probe_controller->Process(fixture.NowMs()); EXPECT_EQ(probes.size(), 1u); - probes = probe_controller_->SetEstimatedBitrate(500, NowMs()); + probes = probe_controller->SetEstimatedBitrate(500, fixture.NowMs()); EXPECT_EQ(probes.size(), 0u); } -TEST_F(ProbeControllerTest, PeriodicProbingAfterReset) { - probe_controller_.reset( - new ProbeController(&field_trial_config_, &mock_rtc_event_log)); - int64_t alr_start_time = clock_.TimeInMilliseconds(); +TEST(ProbeControllerTest, PeriodicProbingAfterReset) { + ProbeControllerFixture fixture; + std::unique_ptr probe_controller = + fixture.CreateController(); + int64_t alr_start_time = fixture.NowMs(); - probe_controller_->SetAlrStartTimeMs(alr_start_time); - probe_controller_->EnablePeriodicAlrProbing(true); - auto probes = probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps, NowMs()); - probe_controller_->Reset(NowMs()); + probe_controller->SetAlrStartTimeMs(alr_start_time); + probe_controller->EnablePeriodicAlrProbing(true); + auto probes = probe_controller->SetBitrates(kMinBitrateBps, kStartBitrateBps, + kMaxBitrateBps, fixture.NowMs()); + probe_controller->Reset(fixture.NowMs()); - clock_.AdvanceTimeMilliseconds(10000); - probes = probe_controller_->Process(NowMs()); + fixture.AdvanceTimeMilliseconds(10000); + probes = probe_controller->Process(fixture.NowMs()); // Since bitrates are not yet set, no probe is sent event though we are in ALR // mode. EXPECT_EQ(probes.size(), 0u); - probes = probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps, NowMs()); + probes = probe_controller->SetBitrates(kMinBitrateBps, kStartBitrateBps, + kMaxBitrateBps, fixture.NowMs()); EXPECT_EQ(probes.size(), 2u); // Make sure we use `kStartBitrateBps` as the estimated bitrate // until SetEstimatedBitrate is called with an updated estimate. - clock_.AdvanceTimeMilliseconds(10000); - probes = probe_controller_->Process(NowMs()); + fixture.AdvanceTimeMilliseconds(10000); + probes = probe_controller->Process(fixture.NowMs()); EXPECT_EQ(probes.size(), 1u); EXPECT_EQ(probes[0].target_data_rate.bps(), kStartBitrateBps * 2); } -TEST_F(ProbeControllerTest, TestExponentialProbingOverflow) { +TEST(ProbeControllerTest, TestExponentialProbingOverflow) { + ProbeControllerFixture fixture; + std::unique_ptr probe_controller = + fixture.CreateController(); const int64_t kMbpsMultiplier = 1000000; - auto probes = probe_controller_->SetBitrates( - kMinBitrateBps, 10 * kMbpsMultiplier, 100 * kMbpsMultiplier, NowMs()); + auto probes = + probe_controller->SetBitrates(kMinBitrateBps, 10 * kMbpsMultiplier, + 100 * kMbpsMultiplier, fixture.NowMs()); // Verify that probe bitrate is capped at the specified max bitrate. - probes = - probe_controller_->SetEstimatedBitrate(60 * kMbpsMultiplier, NowMs()); + probes = probe_controller->SetEstimatedBitrate(60 * kMbpsMultiplier, + fixture.NowMs()); EXPECT_EQ(probes.size(), 1u); EXPECT_EQ(probes[0].target_data_rate.bps(), 100 * kMbpsMultiplier); // Verify that repeated probes aren't sent. - probes = - probe_controller_->SetEstimatedBitrate(100 * kMbpsMultiplier, NowMs()); + probes = probe_controller->SetEstimatedBitrate(100 * kMbpsMultiplier, + fixture.NowMs()); EXPECT_EQ(probes.size(), 0u); } -TEST_F(ProbeControllerTest, TestAllocatedBitrateCap) { +TEST(ProbeControllerTest, TestAllocatedBitrateCap) { + ProbeControllerFixture fixture; + std::unique_ptr probe_controller = + fixture.CreateController(); const int64_t kMbpsMultiplier = 1000000; const int64_t kMaxBitrateBps = 100 * kMbpsMultiplier; - auto probes = probe_controller_->SetBitrates( - kMinBitrateBps, 10 * kMbpsMultiplier, kMaxBitrateBps, NowMs()); + auto probes = probe_controller->SetBitrates( + kMinBitrateBps, 10 * kMbpsMultiplier, kMaxBitrateBps, fixture.NowMs()); // Configure ALR for periodic probing. - probe_controller_->EnablePeriodicAlrProbing(true); - int64_t alr_start_time = clock_.TimeInMilliseconds(); - probe_controller_->SetAlrStartTimeMs(alr_start_time); + probe_controller->EnablePeriodicAlrProbing(true); + int64_t alr_start_time = fixture.NowMs(); + probe_controller->SetAlrStartTimeMs(alr_start_time); int64_t estimated_bitrate_bps = kMaxBitrateBps / 10; - probes = - probe_controller_->SetEstimatedBitrate(estimated_bitrate_bps, NowMs()); + probes = probe_controller->SetEstimatedBitrate(estimated_bitrate_bps, + fixture.NowMs()); // Set a max allocated bitrate below the current estimate. int64_t max_allocated_bps = estimated_bitrate_bps - 1 * kMbpsMultiplier; - probes = - probe_controller_->OnMaxTotalAllocatedBitrate(max_allocated_bps, NowMs()); + probes = probe_controller->OnMaxTotalAllocatedBitrate(max_allocated_bps, + fixture.NowMs()); EXPECT_TRUE(probes.empty()); // No probe since lower than current max. // Probes such as ALR capped at 2x the max allocation limit. - clock_.AdvanceTimeMilliseconds(5000); - probes = probe_controller_->Process(NowMs()); + fixture.AdvanceTimeMilliseconds(5000); + probes = probe_controller->Process(fixture.NowMs()); EXPECT_EQ(probes.size(), 1u); EXPECT_EQ(probes[0].target_data_rate.bps(), 2 * max_allocated_bps); // Remove allocation limit. EXPECT_TRUE( - probe_controller_->OnMaxTotalAllocatedBitrate(0, NowMs()).empty()); - clock_.AdvanceTimeMilliseconds(5000); - probes = probe_controller_->Process(NowMs()); + probe_controller->OnMaxTotalAllocatedBitrate(0, fixture.NowMs()).empty()); + fixture.AdvanceTimeMilliseconds(5000); + probes = probe_controller->Process(fixture.NowMs()); EXPECT_EQ(probes.size(), 1u); EXPECT_EQ(probes[0].target_data_rate.bps(), estimated_bitrate_bps * 2); } -TEST_F(ProbeControllerTest, ConfigurableProbingFieldTrial) { - test::ScopedFieldTrials trials( +TEST(ProbeControllerTest, ConfigurableProbingFieldTrial) { + ProbeControllerFixture fixture( "WebRTC-Bwe-ProbingConfiguration/" "p1:2,p2:5,step_size:3,further_probe_threshold:0.8," "alloc_p1:2,alloc_p2/"); - probe_controller_.reset( - new ProbeController(&field_trial_config_, &mock_rtc_event_log)); - auto probes = probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - 5000000, NowMs()); + std::unique_ptr probe_controller = + fixture.CreateController(); + + auto probes = probe_controller->SetBitrates(kMinBitrateBps, kStartBitrateBps, + 5000000, fixture.NowMs()); EXPECT_EQ(probes.size(), 2u); EXPECT_EQ(probes[0].target_data_rate.bps(), 600); EXPECT_EQ(probes[1].target_data_rate.bps(), 1500); // Repeated probe should only be sent when estimated bitrate climbs above // 0.8 * 5 * kStartBitrateBps = 1200. - probes = probe_controller_->SetEstimatedBitrate(1100, NowMs()); + probes = probe_controller->SetEstimatedBitrate(1100, fixture.NowMs()); EXPECT_EQ(probes.size(), 0u); - probes = probe_controller_->SetEstimatedBitrate(1250, NowMs()); + probes = probe_controller->SetEstimatedBitrate(1250, fixture.NowMs()); EXPECT_EQ(probes.size(), 1u); EXPECT_EQ(probes[0].target_data_rate.bps(), 3 * 1250); - clock_.AdvanceTimeMilliseconds(5000); - probes = probe_controller_->Process(NowMs()); + fixture.AdvanceTimeMilliseconds(5000); + probes = probe_controller->Process(fixture.NowMs()); - probe_controller_->SetAlrStartTimeMs(NowMs()); - probes = probe_controller_->OnMaxTotalAllocatedBitrate(200000, NowMs()); + probe_controller->SetAlrStartTimeMs(fixture.NowMs()); + probes = + probe_controller->OnMaxTotalAllocatedBitrate(200000, fixture.NowMs()); EXPECT_EQ(probes.size(), 1u); EXPECT_EQ(probes[0].target_data_rate.bps(), 400000); } diff --git a/modules/pacing/bitrate_prober.cc b/modules/pacing/bitrate_prober.cc index d2b93dabd0..ec9182fca2 100644 --- a/modules/pacing/bitrate_prober.cc +++ b/modules/pacing/bitrate_prober.cc @@ -34,19 +34,10 @@ constexpr TimeDelta kProbeClusterTimeout = TimeDelta::Seconds(5); BitrateProberConfig::BitrateProberConfig( const FieldTrialsView* key_value_config) - : min_probe_packets_sent("min_probe_packets_sent", 5), - min_probe_delta("min_probe_delta", TimeDelta::Millis(1)), - min_probe_duration("min_probe_duration", TimeDelta::Millis(15)), - max_probe_delay("max_probe_delay", TimeDelta::Millis(10)), - abort_delayed_probes("abort_delayed_probes", true) { - ParseFieldTrial( - {&min_probe_packets_sent, &min_probe_delta, &min_probe_duration, - &max_probe_delay, &abort_delayed_probes}, - key_value_config->Lookup("WebRTC-Bwe-ProbingConfiguration")); - ParseFieldTrial( - {&min_probe_packets_sent, &min_probe_delta, &min_probe_duration, - &max_probe_delay, &abort_delayed_probes}, - key_value_config->Lookup("WebRTC-Bwe-ProbingBehavior")); + : min_probe_delta("min_probe_delta", TimeDelta::Millis(1)), + max_probe_delay("max_probe_delay", TimeDelta::Millis(10)) { + ParseFieldTrial({&min_probe_delta, &max_probe_delay}, + key_value_config->Lookup("WebRTC-Bwe-ProbingBehavior")); } BitrateProber::~BitrateProber() { @@ -88,27 +79,28 @@ void BitrateProber::OnIncomingPacket(DataSize packet_size) { } } -void BitrateProber::CreateProbeCluster(DataRate bitrate, - Timestamp now, - int cluster_id) { +void BitrateProber::CreateProbeCluster( + const ProbeClusterConfig& cluster_config) { RTC_DCHECK(probing_state_ != ProbingState::kDisabled); - RTC_DCHECK_GT(bitrate, DataRate::Zero()); total_probe_count_++; while (!clusters_.empty() && - now - clusters_.front().created_at > kProbeClusterTimeout) { + cluster_config.at_time - clusters_.front().requested_at > + kProbeClusterTimeout) { clusters_.pop(); total_failed_probe_count_++; } ProbeCluster cluster; - cluster.created_at = now; - cluster.pace_info.probe_cluster_min_probes = config_.min_probe_packets_sent; + cluster.requested_at = cluster_config.at_time; + cluster.pace_info.probe_cluster_min_probes = + cluster_config.target_probe_count; cluster.pace_info.probe_cluster_min_bytes = - (bitrate * config_.min_probe_duration.Get()).bytes(); + (cluster_config.target_data_rate * cluster_config.target_duration) + .bytes(); RTC_DCHECK_GE(cluster.pace_info.probe_cluster_min_bytes, 0); - cluster.pace_info.send_bitrate_bps = bitrate.bps(); - cluster.pace_info.probe_cluster_id = cluster_id; + cluster.pace_info.send_bitrate_bps = cluster_config.target_data_rate.bps(); + cluster.pace_info.probe_cluster_id = cluster_config.id; clusters_.push(cluster); RTC_LOG(LS_INFO) << "Probe cluster (bitrate:min bytes:min packets): (" @@ -127,16 +119,6 @@ Timestamp BitrateProber::NextProbeTime(Timestamp now) const { return Timestamp::PlusInfinity(); } - // Legacy behavior, just warn about late probe and return as if not probing. - if (!config_.abort_delayed_probes && next_probe_time_.IsFinite() && - now - next_probe_time_ > config_.max_probe_delay.Get()) { - RTC_DLOG(LS_WARNING) << "Probe delay too high" - " (next_ms:" - << next_probe_time_.ms() << ", now_ms: " << now.ms() - << ")"; - return Timestamp::PlusInfinity(); - } - return next_probe_time_; } @@ -145,7 +127,7 @@ absl::optional BitrateProber::CurrentCluster(Timestamp now) { return absl::nullopt; } - if (config_.abort_delayed_probes && next_probe_time_.IsFinite() && + if (next_probe_time_.IsFinite() && now - next_probe_time_ > config_.max_probe_delay.Get()) { RTC_DLOG(LS_WARNING) << "Probe delay too high" " (next_ms:" diff --git a/modules/pacing/bitrate_prober.h b/modules/pacing/bitrate_prober.h index 94016d5250..d38da7f841 100644 --- a/modules/pacing/bitrate_prober.h +++ b/modules/pacing/bitrate_prober.h @@ -29,17 +29,10 @@ struct BitrateProberConfig { BitrateProberConfig& operator=(const BitrateProberConfig&) = default; ~BitrateProberConfig() = default; - // The minimum number probing packets used. - FieldTrialParameter min_probe_packets_sent; // A minimum interval between probes to allow scheduling to be feasible. FieldTrialParameter min_probe_delta; - // The minimum probing duration. - FieldTrialParameter min_probe_duration; // Maximum amount of time each probe can be delayed. FieldTrialParameter max_probe_delay; - // If NextProbeTime() is called with a delay higher than specified by - // `max_probe_delay`, abort it. - FieldTrialParameter abort_delayed_probes; }; // Note that this class isn't thread-safe by itself and therefore relies @@ -61,10 +54,8 @@ class BitrateProber { // with. void OnIncomingPacket(DataSize packet_size); - // Create a cluster used to probe for `bitrate_bps` with `num_probes` number - // of probes. - void CreateProbeCluster(DataRate bitrate, Timestamp now, int cluster_id); - + // Create a cluster used to probe. + void CreateProbeCluster(const ProbeClusterConfig& cluster_config); // Returns the time at which the next probe should be sent to get accurate // probing. If probing is not desired at this time, Timestamp::PlusInfinity() // will be returned. @@ -104,7 +95,7 @@ class BitrateProber { int sent_probes = 0; int sent_bytes = 0; - Timestamp created_at = Timestamp::MinusInfinity(); + Timestamp requested_at = Timestamp::MinusInfinity(); Timestamp started_at = Timestamp::MinusInfinity(); int retries = 0; }; diff --git a/modules/pacing/bitrate_prober_unittest.cc b/modules/pacing/bitrate_prober_unittest.cc index 5627db0519..f8410d73ec 100644 --- a/modules/pacing/bitrate_prober_unittest.cc +++ b/modules/pacing/bitrate_prober_unittest.cc @@ -12,6 +12,7 @@ #include +#include "api/units/data_rate.h" #include "test/explicit_key_value_config.h" #include "test/gtest.h" @@ -32,8 +33,16 @@ TEST(BitrateProberTest, VerifyStatesAndTimeBetweenProbes) { const DataSize kProbeSize = DataSize::Bytes(1000); const TimeDelta kMinProbeDuration = TimeDelta::Millis(15); - prober.CreateProbeCluster(kTestBitrate1, now, 0); - prober.CreateProbeCluster(kTestBitrate2, now, 1); + prober.CreateProbeCluster({.at_time = now, + .target_data_rate = kTestBitrate1, + .target_duration = TimeDelta::Millis(15), + .target_probe_count = 5, + .id = 0}); + prober.CreateProbeCluster({.at_time = now, + .target_data_rate = kTestBitrate2, + .target_duration = TimeDelta::Millis(15), + .target_probe_count = 5, + .id = 1}); EXPECT_FALSE(prober.is_probing()); prober.OnIncomingPacket(kProbeSize); @@ -85,7 +94,11 @@ TEST(BitrateProberTest, DoesntProbeWithoutRecentPackets) { Timestamp now = Timestamp::Zero(); EXPECT_EQ(prober.NextProbeTime(now), Timestamp::PlusInfinity()); - prober.CreateProbeCluster(DataRate::KilobitsPerSec(900), now, 0); + prober.CreateProbeCluster({.at_time = now, + .target_data_rate = DataRate::KilobitsPerSec(900), + .target_duration = TimeDelta::Millis(15), + .target_probe_count = 5, + .id = 0}); EXPECT_FALSE(prober.is_probing()); prober.OnIncomingPacket(kProbeSize); @@ -94,37 +107,7 @@ TEST(BitrateProberTest, DoesntProbeWithoutRecentPackets) { prober.ProbeSent(now, kProbeSize); } -TEST(BitrateProberTest, DoesntDiscardDelayedProbesInLegacyMode) { - const TimeDelta kMaxProbeDelay = TimeDelta::Millis(3); - const test::ExplicitKeyValueConfig trials( - "WebRTC-Bwe-ProbingBehavior/" - "abort_delayed_probes:0," - "max_probe_delay:3ms/"); - BitrateProber prober(trials); - const DataSize kProbeSize = DataSize::Bytes(1000); - - Timestamp now = Timestamp::Zero(); - prober.CreateProbeCluster(DataRate::KilobitsPerSec(900), now, 0); - prober.OnIncomingPacket(kProbeSize); - EXPECT_TRUE(prober.is_probing()); - EXPECT_EQ(prober.CurrentCluster(now)->probe_cluster_id, 0); - // Advance to first probe time and indicate sent probe. - now = std::max(now, prober.NextProbeTime(now)); - prober.ProbeSent(now, kProbeSize); - - // Advance time 1ms past timeout for the next probe. - Timestamp next_probe_time = prober.NextProbeTime(now); - EXPECT_GT(next_probe_time, now); - now += next_probe_time - now + kMaxProbeDelay + TimeDelta::Millis(1); - - EXPECT_EQ(prober.NextProbeTime(now), Timestamp::PlusInfinity()); - // Check that legacy behaviour where prober is reset in TimeUntilNextProbe is - // no longer there. Probes are no longer retried if they are timed out. - prober.OnIncomingPacket(kProbeSize); - EXPECT_EQ(prober.NextProbeTime(now), Timestamp::PlusInfinity()); -} - -TEST(BitrateProberTest, DiscardsDelayedProbesWhenNotInLegacyMode) { +TEST(BitrateProberTest, DiscardsDelayedProbes) { const TimeDelta kMaxProbeDelay = TimeDelta::Millis(3); const test::ExplicitKeyValueConfig trials( "WebRTC-Bwe-ProbingBehavior/" @@ -136,7 +119,11 @@ TEST(BitrateProberTest, DiscardsDelayedProbesWhenNotInLegacyMode) { Timestamp now = Timestamp::Zero(); // Add two probe clusters. - prober.CreateProbeCluster(DataRate::KilobitsPerSec(900), now, /*id=*/0); + prober.CreateProbeCluster({.at_time = now, + .target_data_rate = DataRate::KilobitsPerSec(900), + .target_duration = TimeDelta::Millis(15), + .target_probe_count = 5, + .id = 0}); prober.OnIncomingPacket(kProbeSize); EXPECT_TRUE(prober.is_probing()); @@ -173,8 +160,11 @@ TEST(BitrateProberTest, VerifyProbeSizeOnHighBitrate) { const DataRate kHighBitrate = DataRate::KilobitsPerSec(10000); // 10 Mbps - prober.CreateProbeCluster(kHighBitrate, Timestamp::Millis(0), - /*cluster_id=*/0); + prober.CreateProbeCluster({.at_time = Timestamp::Millis(0), + .target_data_rate = kHighBitrate, + .target_duration = TimeDelta::Millis(15), + .target_probe_count = 5, + .id = 0}); // Probe size should ensure a minimum of 1 ms interval. EXPECT_GT(prober.RecommendedMinProbeSize(), kHighBitrate * TimeDelta::Millis(1)); @@ -189,7 +179,12 @@ TEST(BitrateProberTest, MinumumNumberOfProbingPackets) { const DataSize kPacketSize = DataSize::Bytes(1000); Timestamp now = Timestamp::Millis(0); - prober.CreateProbeCluster(kBitrate, now, 0); + prober.CreateProbeCluster({.at_time = Timestamp::Millis(0), + .target_data_rate = kBitrate, + .target_duration = TimeDelta::Millis(15), + .target_probe_count = 5, + .id = 0}); + prober.OnIncomingPacket(kPacketSize); for (int i = 0; i < 5; ++i) { EXPECT_TRUE(prober.is_probing()); @@ -207,7 +202,11 @@ TEST(BitrateProberTest, ScaleBytesUsedForProbing) { const DataSize kExpectedDataSent = kBitrate * TimeDelta::Millis(15); Timestamp now = Timestamp::Millis(0); - prober.CreateProbeCluster(kBitrate, now, /*cluster_id=*/0); + prober.CreateProbeCluster({.at_time = Timestamp::Millis(0), + .target_data_rate = kBitrate, + .target_duration = TimeDelta::Millis(15), + .target_probe_count = 5, + .id = 0}); prober.OnIncomingPacket(kPacketSize); DataSize data_sent = DataSize::Zero(); while (data_sent < kExpectedDataSent) { @@ -227,7 +226,11 @@ TEST(BitrateProberTest, HighBitrateProbing) { const DataSize kExpectedDataSent = kBitrate * TimeDelta::Millis(15); Timestamp now = Timestamp::Millis(0); - prober.CreateProbeCluster(kBitrate, now, 0); + prober.CreateProbeCluster({.at_time = Timestamp::Millis(0), + .target_data_rate = kBitrate, + .target_duration = TimeDelta::Millis(15), + .target_probe_count = 5, + .id = 0}); prober.OnIncomingPacket(kPacketSize); DataSize data_sent = DataSize::Zero(); while (data_sent < kExpectedDataSent) { @@ -249,15 +252,27 @@ TEST(BitrateProberTest, ProbeClusterTimeout) { const TimeDelta kTimeout = TimeDelta::Millis(5000); Timestamp now = Timestamp::Millis(0); - prober.CreateProbeCluster(kBitrate, now, /*cluster_id=*/0); + prober.CreateProbeCluster({.at_time = now, + .target_data_rate = kBitrate, + .target_duration = TimeDelta::Millis(15), + .target_probe_count = 5, + .id = 0}); prober.OnIncomingPacket(kSmallPacketSize); EXPECT_FALSE(prober.is_probing()); now += kTimeout; - prober.CreateProbeCluster(kBitrate / 10, now, /*cluster_id=*/1); + prober.CreateProbeCluster({.at_time = now, + .target_data_rate = kBitrate / 10, + .target_duration = TimeDelta::Millis(15), + .target_probe_count = 5, + .id = 1}); prober.OnIncomingPacket(kSmallPacketSize); EXPECT_FALSE(prober.is_probing()); now += TimeDelta::Millis(1); - prober.CreateProbeCluster(kBitrate / 10, now, /*cluster_id=*/2); + prober.CreateProbeCluster({.at_time = now, + .target_data_rate = kBitrate / 10, + .target_duration = TimeDelta::Millis(15), + .target_probe_count = 5, + .id = 2}); prober.OnIncomingPacket(kSmallPacketSize); EXPECT_TRUE(prober.is_probing()); DataSize data_sent = DataSize::Zero(); diff --git a/modules/pacing/g3doc/index.md b/modules/pacing/g3doc/index.md index c06fdc8b4f..6f938be45b 100644 --- a/modules/pacing/g3doc/index.md +++ b/modules/pacing/g3doc/index.md @@ -136,10 +136,9 @@ than cause severe delays. If the bandwidth estimator supports bandwidth probing, it may request a cluster of packets to be sent at a specified rate in order to gauge if this causes -increased delay/loss on the network. Use the `void CreateProbeCluster(DataRate -bitrate, int cluster_id)` method - packets sent via this `PacketRouter` will be -marked with the corresponding cluster_id in the attached `PacedPacketInfo` -struct. +increased delay/loss on the network. Use the `void CreateProbeCluster(...)` +method - packets sent via this `PacketRouter` will be marked with the +corresponding cluster_id in the attached `PacedPacketInfo` struct. If congestion window pushback is used, the state can be updated using `SetCongestionWindow()` and `UpdateOutstandingData()`. diff --git a/modules/pacing/pacing_controller.cc b/modules/pacing/pacing_controller.cc index 9aded858b8..83f79ddc5f 100644 --- a/modules/pacing/pacing_controller.cc +++ b/modules/pacing/pacing_controller.cc @@ -118,7 +118,18 @@ PacingController::PacingController(Clock* clock, PacingController::~PacingController() = default; void PacingController::CreateProbeCluster(DataRate bitrate, int cluster_id) { - prober_.CreateProbeCluster(bitrate, CurrentTime(), cluster_id); + prober_.CreateProbeCluster({.at_time = CurrentTime(), + .target_data_rate = bitrate, + .target_duration = TimeDelta::Millis(15), + .target_probe_count = 5, + .id = cluster_id}); +} + +void PacingController::CreateProbeClusters( + rtc::ArrayView probe_cluster_configs) { + for (const ProbeClusterConfig probe_cluster_config : probe_cluster_configs) { + prober_.CreateProbeCluster(probe_cluster_config); + } } void PacingController::Pause() { diff --git a/modules/pacing/pacing_controller.h b/modules/pacing/pacing_controller.h index d11ff5b96f..b1686c277f 100644 --- a/modules/pacing/pacing_controller.h +++ b/modules/pacing/pacing_controller.h @@ -126,7 +126,10 @@ class PacingController { // it's time to send. void EnqueuePacket(std::unique_ptr packet); + // ABSL_DEPRECATED("Use CreateProbeClusters instead") void CreateProbeCluster(DataRate bitrate, int cluster_id); + void CreateProbeClusters( + rtc::ArrayView probe_cluster_configs); void Pause(); // Temporarily pause all sending. void Resume(); // Resume sending packets. diff --git a/modules/pacing/pacing_controller_unittest.cc b/modules/pacing/pacing_controller_unittest.cc index 3eddb8fff7..5ac5935c97 100644 --- a/modules/pacing/pacing_controller_unittest.cc +++ b/modules/pacing/pacing_controller_unittest.cc @@ -17,7 +17,9 @@ #include #include +#include "api/transport/network_types.h" #include "api/units/data_rate.h" +#include "api/units/time_delta.h" #include "modules/pacing/packet_router.h" #include "system_wrappers/include/clock.h" #include "test/explicit_key_value_config.h" @@ -218,8 +220,17 @@ class PacingControllerTest : public ::testing::Test { } void Init() { - pacer_->CreateProbeCluster(kFirstClusterRate, /*cluster_id=*/0); - pacer_->CreateProbeCluster(kSecondClusterRate, /*cluster_id=*/1); + pacer_->CreateProbeClusters(std::vector( + {{.at_time = clock_.CurrentTime(), + .target_data_rate = kFirstClusterRate, + .target_duration = TimeDelta::Millis(15), + .target_probe_count = 5, + .id = 0}, + {.at_time = clock_.CurrentTime(), + .target_data_rate = kSecondClusterRate, + .target_duration = TimeDelta::Millis(15), + .target_probe_count = 5, + .id = 1}})); // Default to bitrate probing disabled for testing purposes. Probing tests // have to enable probing, either by creating a new PacingController // instance or by calling SetProbingEnabled(true). @@ -1130,10 +1141,18 @@ TEST_F(PacingControllerTest, ProbingWithInsertedPackets) { PacingControllerProbing packet_sender; pacer_ = std::make_unique(&clock_, &packet_sender, trials_); - pacer_->CreateProbeCluster(kFirstClusterRate, - /*cluster_id=*/0); - pacer_->CreateProbeCluster(kSecondClusterRate, - /*cluster_id=*/1); + std::vector probe_clusters = { + {.at_time = clock_.CurrentTime(), + .target_data_rate = kFirstClusterRate, + .target_duration = TimeDelta::Millis(15), + .target_probe_count = 5, + .id = 0}, + {.at_time = clock_.CurrentTime(), + .target_data_rate = kSecondClusterRate, + .target_duration = TimeDelta::Millis(15), + .target_probe_count = 5, + .id = 1}}; + pacer_->CreateProbeClusters(probe_clusters); pacer_->SetPacingRates( DataRate::BitsPerSec(kInitialBitrateBps * kPaceMultiplier), DataRate::Zero()); @@ -1176,18 +1195,12 @@ TEST_F(PacingControllerTest, SkipsProbesWhenProcessIntervalTooLarge) { const uint32_t ssrc = 12346; const int kProbeClusterId = 3; - // Test with both legacy and new probe discard modes. - // TODO(bugs.webrtc.org/11780): Clean up when legacy is gone. - for (bool abort_delayed_probes : {false, true}) { uint16_t sequence_number = 1234; PacingControllerProbing packet_sender; const test::ExplicitKeyValueConfig trials( - abort_delayed_probes ? "WebRTC-Bwe-ProbingBehavior/" - "abort_delayed_probes:1,max_probe_delay:2ms/" - : "WebRTC-Bwe-ProbingBehavior/" - "abort_delayed_probes:0,max_probe_delay:2ms/"); + "WebRTC-Bwe-ProbingBehavior/max_probe_delay:2ms/"); pacer_ = std::make_unique(&clock_, &packet_sender, trials); pacer_->SetPacingRates( @@ -1201,8 +1214,14 @@ TEST_F(PacingControllerTest, SkipsProbesWhenProcessIntervalTooLarge) { ProcessUntilEmpty(); // Probe at a very high rate. - pacer_->CreateProbeCluster(DataRate::KilobitsPerSec(10000), // 10 Mbps. - /*cluster_id=*/kProbeClusterId); + std::vector probe_clusters = { + {.at_time = clock_.CurrentTime(), + .target_data_rate = DataRate::KilobitsPerSec(10000), // 10 Mbps, + .target_duration = TimeDelta::Millis(15), + .target_probe_count = 5, + .id = kProbeClusterId}}; + pacer_->CreateProbeClusters(probe_clusters); + // We need one packet to start the probe. Send(RtpPacketMediaType::kVideo, ssrc, sequence_number++, clock_.TimeInMilliseconds(), kPacketSize); @@ -1240,42 +1259,17 @@ TEST_F(PacingControllerTest, SkipsProbesWhenProcessIntervalTooLarge) { clock_.AdvanceTime(TimeDelta::Micros(1)); int packets_sent_before_timeout = packet_sender.total_packets_sent(); - if (abort_delayed_probes) { - // Expected next process time is unchanged, but calling should not - // generate new packets. - EXPECT_EQ(pacer_->NextSendTime(), probe_time); - pacer_->ProcessPackets(); - EXPECT_EQ(packet_sender.total_packets_sent(), - packets_sent_before_timeout); + // Expected next process time is unchanged, but calling should not + // generate new packets. + EXPECT_EQ(pacer_->NextSendTime(), probe_time); + pacer_->ProcessPackets(); + EXPECT_EQ(packet_sender.total_packets_sent(), packets_sent_before_timeout); - // Next packet sent is not part of probe. - AdvanceTimeAndProcess(); - const int expected_probe_id = PacedPacketInfo::kNotAProbe; - EXPECT_EQ(packet_sender.last_pacing_info().probe_cluster_id, - expected_probe_id); - } else { - // Legacy behaviour, probe "aborted" so send time moved back. Next call to - // ProcessPackets() still results in packets being marked as part of probe - // cluster. - EXPECT_GT(pacer_->NextSendTime(), probe_time); - AdvanceTimeAndProcess(); - EXPECT_GT(packet_sender.total_packets_sent(), - packets_sent_before_timeout); - const int expected_probe_id = last_pacing_info.probe_cluster_id; - EXPECT_EQ(packet_sender.last_pacing_info().probe_cluster_id, - expected_probe_id); - - // Time between sent packets keeps being too large, but we still mark the - // packets as being part of the cluster. - Timestamp a = clock_.CurrentTime(); - AdvanceTimeAndProcess(); - EXPECT_GT(packet_sender.total_packets_sent(), - packets_sent_before_timeout); - EXPECT_EQ(packet_sender.last_pacing_info().probe_cluster_id, - expected_probe_id); - EXPECT_GT(clock_.CurrentTime() - a, time_between_probes); - } - } + // Next packet sent is not part of probe. + AdvanceTimeAndProcess(); + const int expected_probe_id = PacedPacketInfo::kNotAProbe; + EXPECT_EQ(packet_sender.last_pacing_info().probe_cluster_id, + expected_probe_id); } TEST_F(PacingControllerTest, ProbingWithPaddingSupport) { @@ -1286,8 +1280,14 @@ TEST_F(PacingControllerTest, ProbingWithPaddingSupport) { PacingControllerProbing packet_sender; pacer_ = std::make_unique(&clock_, &packet_sender, trials_); - pacer_->CreateProbeCluster(kFirstClusterRate, - /*cluster_id=*/0); + std::vector probe_clusters = { + {.at_time = clock_.CurrentTime(), + .target_data_rate = kFirstClusterRate, + .target_duration = TimeDelta::Millis(15), + .target_probe_count = 5, + .id = 0}}; + pacer_->CreateProbeClusters(probe_clusters); + pacer_->SetPacingRates( DataRate::BitsPerSec(kInitialBitrateBps * kPaceMultiplier), DataRate::Zero()); @@ -1442,7 +1442,14 @@ TEST_F(PacingControllerTest, OwnedPacketPrioritizedOnType) { TEST_F(PacingControllerTest, SmallFirstProbePacket) { MockPacketSender callback; pacer_ = std::make_unique(&clock_, &callback, trials_); - pacer_->CreateProbeCluster(kFirstClusterRate, /*cluster_id=*/0); + std::vector probe_clusters = { + {.at_time = clock_.CurrentTime(), + .target_data_rate = kFirstClusterRate, + .target_duration = TimeDelta::Millis(15), + .target_probe_count = 5, + .id = 0}}; + pacer_->CreateProbeClusters(probe_clusters); + pacer_->SetPacingRates(kTargetRate * kPaceMultiplier, DataRate::Zero()); // Add high prio media. @@ -1528,8 +1535,13 @@ TEST_F(PacingControllerTest, NoProbingWhilePaused) { ProcessUntilEmpty(); // Trigger probing. - pacer_->CreateProbeCluster(DataRate::KilobitsPerSec(10000), // 10 Mbps. - /*cluster_id=*/3); + std::vector probe_clusters = { + {.at_time = clock_.CurrentTime(), + .target_data_rate = DataRate::KilobitsPerSec(10000), // 10 Mbps. + .target_duration = TimeDelta::Millis(15), + .target_probe_count = 5, + .id = 3}}; + pacer_->CreateProbeClusters(probe_clusters); // Time to next send time should be small. EXPECT_LT(pacer_->NextSendTime() - clock_.CurrentTime(), diff --git a/modules/pacing/rtp_packet_pacer.h b/modules/pacing/rtp_packet_pacer.h index a201838858..e2cf806385 100644 --- a/modules/pacing/rtp_packet_pacer.h +++ b/modules/pacing/rtp_packet_pacer.h @@ -13,6 +13,8 @@ #include +#include + #include "absl/types/optional.h" #include "api/units/data_rate.h" #include "api/units/data_size.h" @@ -26,7 +28,8 @@ class RtpPacketPacer { public: virtual ~RtpPacketPacer() = default; - virtual void CreateProbeCluster(DataRate bitrate, int cluster_id) = 0; + virtual void CreateProbeClusters( + std::vector probe_cluster_configs) = 0; // Temporarily pause all sending. virtual void Pause() = 0; diff --git a/modules/pacing/task_queue_paced_sender.cc b/modules/pacing/task_queue_paced_sender.cc index fe9c34265d..b95bed28bb 100644 --- a/modules/pacing/task_queue_paced_sender.cc +++ b/modules/pacing/task_queue_paced_sender.cc @@ -14,6 +14,7 @@ #include #include "absl/memory/memory.h" +#include "api/transport/network_types.h" #include "rtc_base/checks.h" #include "rtc_base/experiments/field_trial_parser.h" #include "rtc_base/experiments/field_trial_units.h" @@ -84,13 +85,14 @@ void TaskQueuePacedSender::EnsureStarted() { }); } -void TaskQueuePacedSender::CreateProbeCluster(DataRate bitrate, - int cluster_id) { - task_queue_.PostTask([this, bitrate, cluster_id]() { - RTC_DCHECK_RUN_ON(&task_queue_); - pacing_controller_.CreateProbeCluster(bitrate, cluster_id); - MaybeProcessPackets(Timestamp::MinusInfinity()); - }); +void TaskQueuePacedSender::CreateProbeClusters( + std::vector probe_cluster_configs) { + task_queue_.PostTask( + [this, probe_cluster_configs = std::move(probe_cluster_configs)]() { + RTC_DCHECK_RUN_ON(&task_queue_); + pacing_controller_.CreateProbeClusters(probe_cluster_configs); + MaybeProcessPackets(Timestamp::MinusInfinity()); + }); } void TaskQueuePacedSender::Pause() { diff --git a/modules/pacing/task_queue_paced_sender.h b/modules/pacing/task_queue_paced_sender.h index 32c5c1f847..a390be28b2 100644 --- a/modules/pacing/task_queue_paced_sender.h +++ b/modules/pacing/task_queue_paced_sender.h @@ -65,7 +65,8 @@ class TaskQueuePacedSender : public RtpPacketPacer, public RtpPacketSender { // Methods implementing RtpPacketPacer. - void CreateProbeCluster(DataRate bitrate, int cluster_id) override; + void CreateProbeClusters( + std::vector probe_cluster_configs) override; // Temporarily pause all sending. void Pause() override; diff --git a/modules/pacing/task_queue_paced_sender_unittest.cc b/modules/pacing/task_queue_paced_sender_unittest.cc index 7b05110822..70ee31ea38 100644 --- a/modules/pacing/task_queue_paced_sender_unittest.cc +++ b/modules/pacing/task_queue_paced_sender_unittest.cc @@ -380,7 +380,12 @@ TEST(TaskQueuePacedSenderTest, ProbingOverridesCoalescingWindow) { // Add 10 packets. The first should be sent immediately since the buffers // are clear. This will also trigger the probe to start. EXPECT_CALL(packet_router, SendPacket).Times(AtLeast(1)); - pacer.CreateProbeCluster(kPacingDataRate * 2, 17); + pacer.CreateProbeClusters( + {{.at_time = time_controller.GetClock()->CurrentTime(), + .target_data_rate = kPacingDataRate * 2, + .target_duration = TimeDelta::Millis(15), + .target_probe_count = 5, + .id = 17}}); pacer.EnqueuePackets(GeneratePackets(RtpPacketMediaType::kVideo, 10)); time_controller.AdvanceTime(TimeDelta::Zero()); ::testing::Mock::VerifyAndClearExpectations(&packet_router); @@ -428,7 +433,12 @@ TEST(TaskQueuePacedSenderTest, SchedulesProbeAtSentTime) { // packets the probe needs. const DataRate kProbeRate = 2 * kPacingDataRate; const int kProbeClusterId = 1; - pacer.CreateProbeCluster(kProbeRate, kProbeClusterId); + pacer.CreateProbeClusters( + {{.at_time = time_controller.GetClock()->CurrentTime(), + .target_data_rate = kProbeRate, + .target_duration = TimeDelta::Millis(15), + .target_probe_count = 4, + .id = kProbeClusterId}}); // Expected size for each probe in a cluster is twice the expected bits sent // during min_probe_delta. @@ -485,7 +495,13 @@ TEST(TaskQueuePacedSenderTest, NoMinSleepTimeWhenProbing) { // Set a high probe rate. const int kProbeClusterId = 1; DataRate kProbingRate = kPacingDataRate * 10; - pacer.CreateProbeCluster(kProbingRate, kProbeClusterId); + + pacer.CreateProbeClusters( + {{.at_time = time_controller.GetClock()->CurrentTime(), + .target_data_rate = kProbingRate, + .target_duration = TimeDelta::Millis(15), + .target_probe_count = 5, + .id = kProbeClusterId}}); // Advance time less than PacingController::kMinSleepTime, probing packets // for the first millisecond should be sent immediately. Min delta between @@ -640,7 +656,13 @@ TEST(TaskQueuePacedSenderTest, ProbingStopDuringSendLoop) { // Set probe rate. const int kProbeClusterId = 1; const DataRate kProbingRate = kPacingDataRate; - pacer.CreateProbeCluster(kProbingRate, kProbeClusterId); + + pacer.CreateProbeClusters( + {{.at_time = time_controller.GetClock()->CurrentTime(), + .target_data_rate = kProbingRate, + .target_duration = TimeDelta::Millis(15), + .target_probe_count = 4, + .id = kProbeClusterId}}); const int kPacketsToSend = 100; const TimeDelta kPacketsPacedTime = @@ -762,7 +784,12 @@ TEST(TaskQueuePacedSenderTest, HighPrecisionPacingWhenSlackIsDisabled) { EXPECT_GT(task_queue_factory.delayed_high_precision_count(), 0); // Create probe cluster which is also high precision. - pacer.CreateProbeCluster(kPacingRate, 123); + pacer.CreateProbeClusters( + {{.at_time = time_controller.GetClock()->CurrentTime(), + .target_data_rate = kPacingRate, + .target_duration = TimeDelta::Millis(15), + .target_probe_count = 4, + .id = 123}}); pacer.EnqueuePackets(GeneratePackets(RtpPacketMediaType::kVideo, 1)); time_controller.AdvanceTime(TimeDelta::Seconds(1)); EXPECT_EQ(task_queue_factory.delayed_low_precision_count(), 0); @@ -807,7 +834,12 @@ TEST(TaskQueuePacedSenderTest, LowPrecisionPacingWhenSlackIsEnabled) { // Create probe cluster, which uses high precision despite regular pacing // being low precision. - pacer.CreateProbeCluster(kPacingRate, 123); + pacer.CreateProbeClusters( + {{.at_time = time_controller.GetClock()->CurrentTime(), + .target_data_rate = kPacingRate, + .target_duration = TimeDelta::Millis(15), + .target_probe_count = 4, + .id = 123}}); pacer.EnqueuePackets(GeneratePackets(RtpPacketMediaType::kVideo, 1)); time_controller.AdvanceTime(TimeDelta::Seconds(1)); EXPECT_GT(task_queue_factory.delayed_high_precision_count(), 0);