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 0958935fce..517ef1d69e 100644 --- a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc +++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc @@ -109,7 +109,7 @@ GoogCcNetworkController::GoogCcNetworkController( NetworkControllerConfig config) : event_log_(event_log), observer_(observer), - probe_controller_(new ProbeController(observer_)), + probe_controller_(new ProbeController()), bandwidth_estimation_( rtc::MakeUnique(event_log_)), alr_detector_(rtc::MakeUnique()), @@ -167,6 +167,11 @@ void GoogCcNetworkController::OnProcessInterval(ProcessInterval msg) { alr_detector_->GetApplicationLimitedRegionStartTime(); probe_controller_->SetAlrStartTimeMs(start_time_ms); probe_controller_->Process(msg.at_time.ms()); + + for (const ProbeClusterConfig& probe : + probe_controller_->GetAndResetPendingProbes()) { + observer_->OnProbeClusterConfig(probe); + } MaybeTriggerOnNetworkChanged(msg.at_time); } diff --git a/modules/congestion_controller/goog_cc/probe_controller.cc b/modules/congestion_controller/goog_cc/probe_controller.cc index 2f1b5b8061..44836162f8 100644 --- a/modules/congestion_controller/goog_cc/probe_controller.cc +++ b/modules/congestion_controller/goog_cc/probe_controller.cc @@ -76,8 +76,7 @@ constexpr char kBweRapidRecoveryExperiment[] = } // namespace -ProbeController::ProbeController(NetworkControllerObserver* observer) - : observer_(observer), enable_periodic_alr_probing_(false) { +ProbeController::ProbeController() : enable_periodic_alr_probing_(false) { Reset(0); in_rapid_recovery_experiment_ = webrtc::field_trial::FindFullName( kBweRapidRecoveryExperiment) == "Enabled"; @@ -287,6 +286,14 @@ void ProbeController::Process(int64_t at_time_ms) { } } +std::vector ProbeController::GetAndResetPendingProbes() { + if (pending_probes_.empty()) + return std::vector(); + std::vector pending_probes; + pending_probes_.swap(pending_probes); + return pending_probes; +} + void ProbeController::InitiateProbing( int64_t now_ms, std::initializer_list bitrates_to_probe, @@ -305,7 +312,7 @@ void ProbeController::InitiateProbing( config.target_data_rate = DataRate::bps(rtc::dchecked_cast(bitrate)); config.target_duration = TimeDelta::ms(kMinProbeDurationMs); config.target_probe_count = kMinProbePacketsSent; - observer_->OnProbeClusterConfig(config); + pending_probes_.push_back(config); } time_last_probing_initiated_ms_ = now_ms; if (probe_further) { diff --git a/modules/congestion_controller/goog_cc/probe_controller.h b/modules/congestion_controller/goog_cc/probe_controller.h index 5649a99b11..6434b0ec8c 100644 --- a/modules/congestion_controller/goog_cc/probe_controller.h +++ b/modules/congestion_controller/goog_cc/probe_controller.h @@ -14,6 +14,7 @@ #include #include +#include #include "api/optional.h" #include "modules/congestion_controller/network_control/include/network_control.h" @@ -29,7 +30,7 @@ namespace webrtc_cc { // bitrate is adjusted by an application. class ProbeController { public: - explicit ProbeController(NetworkControllerObserver* observer); + ProbeController(); ~ProbeController(); void SetBitrates(int64_t min_bitrate_bps, @@ -59,6 +60,8 @@ class ProbeController { void Process(int64_t at_time_ms); + std::vector GetAndResetPendingProbes(); + private: enum class State { // Initial state where no probing has been triggered yet. @@ -74,8 +77,6 @@ class ProbeController { std::initializer_list bitrates_to_probe, bool probe_further); - NetworkControllerObserver* const observer_; - bool network_available_; State state_; int64_t min_bitrate_to_probe_further_bps_; @@ -96,7 +97,9 @@ class ProbeController { int64_t mid_call_probing_bitrate_bps_; int64_t mid_call_probing_succcess_threshold_; - RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(ProbeController); + std::vector pending_probes_; + + RTC_DISALLOW_COPY_AND_ASSIGN(ProbeController); }; } // namespace webrtc_cc diff --git a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc b/modules/congestion_controller/goog_cc/probe_controller_unittest.cc index 89e329bc41..52903acf49 100644 --- a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc +++ b/modules/congestion_controller/goog_cc/probe_controller_unittest.cc @@ -39,23 +39,12 @@ constexpr int kExponentialProbingTimeoutMs = 5000; constexpr int kAlrProbeInterval = 5000; constexpr int kAlrEndedTimeoutMs = 3000; constexpr int kBitrateDropTimeoutMs = 5000; - -inline Matcher DataRateEqBps(int bps) { - return Field(&ProbeClusterConfig::target_data_rate, DataRate::bps(bps)); -} -class MockNetworkControllerObserver : public NetworkControllerObserver { - public: - MOCK_METHOD1(OnCongestionWindow, void(CongestionWindow)); - MOCK_METHOD1(OnPacerConfig, void(PacerConfig)); - MOCK_METHOD1(OnProbeClusterConfig, void(ProbeClusterConfig)); - MOCK_METHOD1(OnTargetTransferRate, void(TargetTransferRate)); -}; } // namespace class ProbeControllerTest : public ::testing::Test { protected: ProbeControllerTest() : clock_(100000000L) { - probe_controller_.reset(new ProbeController(&cluster_handler_)); + probe_controller_.reset(new ProbeController()); } ~ProbeControllerTest() override {} @@ -69,107 +58,110 @@ class ProbeControllerTest : public ::testing::Test { int64_t NowMs() { return clock_.TimeInMilliseconds(); } SimulatedClock clock_; - NiceMock cluster_handler_; std::unique_ptr probe_controller_; }; TEST_F(ProbeControllerTest, InitiatesProbingAtStart) { - EXPECT_CALL(cluster_handler_, OnProbeClusterConfig(_)).Times(AtLeast(2)); probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, kMaxBitrateBps, NowMs()); + EXPECT_GE(probe_controller_->GetAndResetPendingProbes().size(), 2u); } TEST_F(ProbeControllerTest, ProbeOnlyWhenNetworkIsUp) { SetNetworkAvailable(false); - EXPECT_CALL(cluster_handler_, OnProbeClusterConfig(_)).Times(0); probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, kMaxBitrateBps, NowMs()); - - testing::Mock::VerifyAndClearExpectations(&cluster_handler_); - EXPECT_CALL(cluster_handler_, OnProbeClusterConfig(_)).Times(AtLeast(2)); + EXPECT_EQ(probe_controller_->GetAndResetPendingProbes().size(), 0u); SetNetworkAvailable(true); + EXPECT_GE(probe_controller_->GetAndResetPendingProbes().size(), 2u); } TEST_F(ProbeControllerTest, InitiatesProbingOnMaxBitrateIncrease) { - EXPECT_CALL(cluster_handler_, OnProbeClusterConfig(_)).Times(AtLeast(2)); probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, kMaxBitrateBps, NowMs()); // Long enough to time out exponential probing. clock_.AdvanceTimeMilliseconds(kExponentialProbingTimeoutMs); probe_controller_->SetEstimatedBitrate(kStartBitrateBps, NowMs()); probe_controller_->Process(NowMs()); + EXPECT_GE(probe_controller_->GetAndResetPendingProbes().size(), 2u); - EXPECT_CALL(cluster_handler_, - OnProbeClusterConfig(DataRateEqBps(kMaxBitrateBps + 100))); probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, kMaxBitrateBps + 100, NowMs()); + + EXPECT_EQ( + probe_controller_->GetAndResetPendingProbes()[0].target_data_rate.bps(), + kMaxBitrateBps + 100); } TEST_F(ProbeControllerTest, InitiatesProbingOnMaxBitrateIncreaseAtMaxBitrate) { - EXPECT_CALL(cluster_handler_, OnProbeClusterConfig(_)).Times(AtLeast(2)); probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, kMaxBitrateBps, NowMs()); // Long enough to time out exponential probing. clock_.AdvanceTimeMilliseconds(kExponentialProbingTimeoutMs); probe_controller_->SetEstimatedBitrate(kStartBitrateBps, NowMs()); probe_controller_->Process(NowMs()); + EXPECT_GE(probe_controller_->GetAndResetPendingProbes().size(), 2u); probe_controller_->SetEstimatedBitrate(kMaxBitrateBps, NowMs()); - EXPECT_CALL(cluster_handler_, - OnProbeClusterConfig(DataRateEqBps(kMaxBitrateBps + 100))); probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, kMaxBitrateBps + 100, NowMs()); + EXPECT_EQ( + probe_controller_->GetAndResetPendingProbes()[0].target_data_rate.bps(), + kMaxBitrateBps + 100); } TEST_F(ProbeControllerTest, TestExponentialProbing) { probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, kMaxBitrateBps, NowMs()); + probe_controller_->GetAndResetPendingProbes(); // Repeated probe should only be sent when estimated bitrate climbs above // 0.7 * 6 * kStartBitrateBps = 1260. - EXPECT_CALL(cluster_handler_, OnProbeClusterConfig(_)).Times(0); probe_controller_->SetEstimatedBitrate(1000, NowMs()); - testing::Mock::VerifyAndClearExpectations(&cluster_handler_); + EXPECT_EQ(probe_controller_->GetAndResetPendingProbes().size(), 0u); - EXPECT_CALL(cluster_handler_, OnProbeClusterConfig(DataRateEqBps(2 * 1800))); probe_controller_->SetEstimatedBitrate(1800, NowMs()); + EXPECT_EQ( + probe_controller_->GetAndResetPendingProbes()[0].target_data_rate.bps(), + 2 * 1800); } TEST_F(ProbeControllerTest, TestExponentialProbingTimeout) { probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, kMaxBitrateBps, NowMs()); - + probe_controller_->GetAndResetPendingProbes(); // Advance far enough to cause a time out in waiting for probing result. clock_.AdvanceTimeMilliseconds(kExponentialProbingTimeoutMs); probe_controller_->Process(NowMs()); - EXPECT_CALL(cluster_handler_, OnProbeClusterConfig(_)).Times(0); probe_controller_->SetEstimatedBitrate(1800, NowMs()); + EXPECT_EQ(probe_controller_->GetAndResetPendingProbes().size(), 0u); } TEST_F(ProbeControllerTest, RequestProbeInAlr) { - EXPECT_CALL(cluster_handler_, OnProbeClusterConfig(_)).Times(2); probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, kMaxBitrateBps, NowMs()); probe_controller_->SetEstimatedBitrate(500, NowMs()); - testing::Mock::VerifyAndClearExpectations(&cluster_handler_); - EXPECT_CALL(cluster_handler_, OnProbeClusterConfig(DataRateEqBps(0.85 * 500))) - .Times(1); + EXPECT_GE(probe_controller_->GetAndResetPendingProbes().size(), 2u); + probe_controller_->SetAlrStartTimeMs(clock_.TimeInMilliseconds()); clock_.AdvanceTimeMilliseconds(kAlrProbeInterval + 1); probe_controller_->Process(NowMs()); probe_controller_->SetEstimatedBitrate(250, NowMs()); probe_controller_->RequestProbe(NowMs()); + + std::vector probes = + probe_controller_->GetAndResetPendingProbes(); + EXPECT_EQ(probes.size(), 1u); + EXPECT_EQ(probes[0].target_data_rate.bps(), 0.85 * 500); } TEST_F(ProbeControllerTest, RequestProbeWhenAlrEndedRecently) { - EXPECT_CALL(cluster_handler_, OnProbeClusterConfig(_)).Times(2); probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, kMaxBitrateBps, NowMs()); probe_controller_->SetEstimatedBitrate(500, NowMs()); - testing::Mock::VerifyAndClearExpectations(&cluster_handler_); - EXPECT_CALL(cluster_handler_, OnProbeClusterConfig(DataRateEqBps(0.85 * 500))) - .Times(1); + EXPECT_EQ(probe_controller_->GetAndResetPendingProbes().size(), 2u); + probe_controller_->SetAlrStartTimeMs(rtc::nullopt); clock_.AdvanceTimeMilliseconds(kAlrProbeInterval + 1); probe_controller_->Process(NowMs()); @@ -177,15 +169,19 @@ TEST_F(ProbeControllerTest, RequestProbeWhenAlrEndedRecently) { probe_controller_->SetAlrEndedTimeMs(clock_.TimeInMilliseconds()); clock_.AdvanceTimeMilliseconds(kAlrEndedTimeoutMs - 1); probe_controller_->RequestProbe(NowMs()); + + std::vector probes = + probe_controller_->GetAndResetPendingProbes(); + EXPECT_EQ(probes.size(), 1u); + EXPECT_EQ(probes[0].target_data_rate.bps(), 0.85 * 500); } TEST_F(ProbeControllerTest, RequestProbeWhenAlrNotEndedRecently) { - EXPECT_CALL(cluster_handler_, OnProbeClusterConfig(_)).Times(2); probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, kMaxBitrateBps, NowMs()); probe_controller_->SetEstimatedBitrate(500, NowMs()); - testing::Mock::VerifyAndClearExpectations(&cluster_handler_); - EXPECT_CALL(cluster_handler_, OnProbeClusterConfig(_)).Times(0); + EXPECT_EQ(probe_controller_->GetAndResetPendingProbes().size(), 2u); + probe_controller_->SetAlrStartTimeMs(rtc::nullopt); clock_.AdvanceTimeMilliseconds(kAlrProbeInterval + 1); probe_controller_->Process(NowMs()); @@ -193,65 +189,63 @@ TEST_F(ProbeControllerTest, RequestProbeWhenAlrNotEndedRecently) { probe_controller_->SetAlrEndedTimeMs(clock_.TimeInMilliseconds()); clock_.AdvanceTimeMilliseconds(kAlrEndedTimeoutMs + 1); probe_controller_->RequestProbe(NowMs()); + EXPECT_EQ(probe_controller_->GetAndResetPendingProbes().size(), 0u); } TEST_F(ProbeControllerTest, RequestProbeWhenBweDropNotRecent) { - EXPECT_CALL(cluster_handler_, OnProbeClusterConfig(_)).Times(2); probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, kMaxBitrateBps, NowMs()); probe_controller_->SetEstimatedBitrate(500, NowMs()); - testing::Mock::VerifyAndClearExpectations(&cluster_handler_); - EXPECT_CALL(cluster_handler_, OnProbeClusterConfig(_)).Times(0); + EXPECT_EQ(probe_controller_->GetAndResetPendingProbes().size(), 2u); + probe_controller_->SetAlrStartTimeMs(clock_.TimeInMilliseconds()); clock_.AdvanceTimeMilliseconds(kAlrProbeInterval + 1); probe_controller_->Process(NowMs()); probe_controller_->SetEstimatedBitrate(250, NowMs()); clock_.AdvanceTimeMilliseconds(kBitrateDropTimeoutMs + 1); probe_controller_->RequestProbe(NowMs()); + EXPECT_EQ(probe_controller_->GetAndResetPendingProbes().size(), 0u); } TEST_F(ProbeControllerTest, PeriodicProbing) { - EXPECT_CALL(cluster_handler_, OnProbeClusterConfig(_)).Times(2); probe_controller_->EnablePeriodicAlrProbing(true); probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, kMaxBitrateBps, NowMs()); probe_controller_->SetEstimatedBitrate(500, NowMs()); - testing::Mock::VerifyAndClearExpectations(&cluster_handler_); + EXPECT_EQ(probe_controller_->GetAndResetPendingProbes().size(), 2u); int64_t start_time = clock_.TimeInMilliseconds(); // Expect the controller to send a new probe after 5s has passed. - EXPECT_CALL(cluster_handler_, OnProbeClusterConfig(DataRateEqBps(1000))) - .Times(1); probe_controller_->SetAlrStartTimeMs(start_time); clock_.AdvanceTimeMilliseconds(5000); probe_controller_->Process(NowMs()); probe_controller_->SetEstimatedBitrate(500, NowMs()); - testing::Mock::VerifyAndClearExpectations(&cluster_handler_); + + std::vector probes = + probe_controller_->GetAndResetPendingProbes(); + EXPECT_EQ(probes.size(), 1u); + EXPECT_EQ(probes[0].target_data_rate.bps(), 1000); // The following probe should be sent at 10s into ALR. - EXPECT_CALL(cluster_handler_, OnProbeClusterConfig(_)).Times(0); probe_controller_->SetAlrStartTimeMs(start_time); clock_.AdvanceTimeMilliseconds(4000); probe_controller_->Process(NowMs()); probe_controller_->SetEstimatedBitrate(500, NowMs()); - testing::Mock::VerifyAndClearExpectations(&cluster_handler_); + EXPECT_EQ(probe_controller_->GetAndResetPendingProbes().size(), 0u); - EXPECT_CALL(cluster_handler_, OnProbeClusterConfig(_)).Times(1); probe_controller_->SetAlrStartTimeMs(start_time); clock_.AdvanceTimeMilliseconds(1000); probe_controller_->Process(NowMs()); probe_controller_->SetEstimatedBitrate(500, NowMs()); - testing::Mock::VerifyAndClearExpectations(&cluster_handler_); + EXPECT_EQ(probe_controller_->GetAndResetPendingProbes().size(), 1u); } TEST_F(ProbeControllerTest, PeriodicProbingAfterReset) { - NiceMock local_handler; - probe_controller_.reset(new ProbeController(&local_handler)); + probe_controller_.reset(new ProbeController()); int64_t alr_start_time = clock_.TimeInMilliseconds(); probe_controller_->SetAlrStartTimeMs(alr_start_time); - EXPECT_CALL(local_handler, OnProbeClusterConfig(_)).Times(2); probe_controller_->EnablePeriodicAlrProbing(true); probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, kMaxBitrateBps, NowMs()); @@ -259,17 +253,19 @@ TEST_F(ProbeControllerTest, PeriodicProbingAfterReset) { clock_.AdvanceTimeMilliseconds(10000); probe_controller_->Process(NowMs()); + EXPECT_EQ(probe_controller_->GetAndResetPendingProbes().size(), 2u); - EXPECT_CALL(local_handler, OnProbeClusterConfig(_)).Times(2); probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, kMaxBitrateBps, NowMs()); + EXPECT_EQ(probe_controller_->GetAndResetPendingProbes().size(), 2u); // Make sure we use |kStartBitrateBps| as the estimated bitrate // until SetEstimatedBitrate is called with an updated estimate. clock_.AdvanceTimeMilliseconds(10000); - EXPECT_CALL(local_handler, - OnProbeClusterConfig(DataRateEqBps(kStartBitrateBps * 2))); probe_controller_->Process(NowMs()); + EXPECT_EQ( + probe_controller_->GetAndResetPendingProbes()[0].target_data_rate.bps(), + kStartBitrateBps * 2); } TEST_F(ProbeControllerTest, TestExponentialProbingOverflow) { @@ -277,15 +273,16 @@ TEST_F(ProbeControllerTest, TestExponentialProbingOverflow) { probe_controller_->SetBitrates(kMinBitrateBps, 10 * kMbpsMultiplier, 100 * kMbpsMultiplier, NowMs()); - // Verify that probe bitrate is capped at the specified max bitrate. - EXPECT_CALL(cluster_handler_, - OnProbeClusterConfig(DataRateEqBps(100 * kMbpsMultiplier))); probe_controller_->SetEstimatedBitrate(60 * kMbpsMultiplier, NowMs()); - testing::Mock::VerifyAndClearExpectations(&cluster_handler_); + + // Verify that probe bitrate is capped at the specified max bitrate. + EXPECT_EQ( + probe_controller_->GetAndResetPendingProbes()[2].target_data_rate.bps(), + 100 * kMbpsMultiplier); // Verify that repeated probes aren't sent. - EXPECT_CALL(cluster_handler_, OnProbeClusterConfig(_)).Times(0); probe_controller_->SetEstimatedBitrate(100 * kMbpsMultiplier, NowMs()); + EXPECT_EQ(probe_controller_->GetAndResetPendingProbes().size(), 0u); } } // namespace test diff --git a/modules/congestion_controller/rtp/send_side_congestion_controller.cc b/modules/congestion_controller/rtp/send_side_congestion_controller.cc index 0075919d32..e1defdc140 100644 --- a/modules/congestion_controller/rtp/send_side_congestion_controller.cc +++ b/modules/congestion_controller/rtp/send_side_congestion_controller.cc @@ -348,6 +348,7 @@ void SendSideCongestionController::MaybeCreateControllers() { controller_ = controller_factory_->Create(control_handler_.get(), initial_config_); UpdateStreamsConfig(); + UpdateControllerWithTimeInterval(); StartProcessPeriodicTasks(); } diff --git a/modules/congestion_controller/rtp/send_side_congestion_controller_unittest.cc b/modules/congestion_controller/rtp/send_side_congestion_controller_unittest.cc index b002b6d780..56f010f594 100644 --- a/modules/congestion_controller/rtp/send_side_congestion_controller_unittest.cc +++ b/modules/congestion_controller/rtp/send_side_congestion_controller_unittest.cc @@ -361,6 +361,7 @@ TEST_F(SendSideCongestionControllerTest, ProbeOnRouteChange) { route.local_network_id = 1; controller_->OnNetworkRouteChanged(route, 2 * kInitialBitrateBps, 0, 20 * kInitialBitrateBps); + controller_->Process(); } // Estimated bitrate reduced when the feedbacks arrive with such a long delay,