diff --git a/webrtc/audio/audio_send_stream.cc b/webrtc/audio/audio_send_stream.cc index b031762b4c..840ae68d06 100644 --- a/webrtc/audio/audio_send_stream.cc +++ b/webrtc/audio/audio_send_stream.cc @@ -229,7 +229,8 @@ bool AudioSendStream::DeliverRtcp(const uint8_t* packet, size_t length) { uint32_t AudioSendStream::OnBitrateUpdated(uint32_t bitrate_bps, uint8_t fraction_loss, - int64_t rtt) { + int64_t rtt, + int64_t probing_interval_ms) { RTC_DCHECK_GE(bitrate_bps, static_cast(config_.min_bitrate_bps)); // The bitrate allocator might allocate an higher than max configured bitrate @@ -238,7 +239,7 @@ uint32_t AudioSendStream::OnBitrateUpdated(uint32_t bitrate_bps, if (bitrate_bps > max_bitrate_bps) bitrate_bps = max_bitrate_bps; - channel_proxy_->SetBitrate(bitrate_bps); + channel_proxy_->SetBitrate(bitrate_bps, probing_interval_ms); // The amount of audio protection is not exposed by the encoder, hence // always returning 0. diff --git a/webrtc/audio/audio_send_stream.h b/webrtc/audio/audio_send_stream.h index af0513ccf7..88ecbfc9fb 100644 --- a/webrtc/audio/audio_send_stream.h +++ b/webrtc/audio/audio_send_stream.h @@ -54,7 +54,8 @@ class AudioSendStream final : public webrtc::AudioSendStream, // Implements BitrateAllocatorObserver. uint32_t OnBitrateUpdated(uint32_t bitrate_bps, uint8_t fraction_loss, - int64_t rtt) override; + int64_t rtt, + int64_t probing_interval_ms) override; const webrtc::AudioSendStream::Config& config() const; void SetTransportOverhead(int transport_overhead_per_packet); diff --git a/webrtc/audio/audio_send_stream_unittest.cc b/webrtc/audio/audio_send_stream_unittest.cc index dbc49662e6..a296513d9d 100644 --- a/webrtc/audio/audio_send_stream_unittest.cc +++ b/webrtc/audio/audio_send_stream_unittest.cc @@ -105,6 +105,8 @@ struct ConfigHelper { // Use ISAC as default codec so as to prevent unnecessary |voice_engine_| // calls from the default ctor behavior. stream_config_.send_codec_spec.codec_inst = kIsacCodec; + stream_config_.min_bitrate_bps = 10000; + stream_config_.max_bitrate_bps = 65000; } AudioSendStream::Config& config() { return stream_config_; } @@ -400,5 +402,27 @@ TEST(AudioSendStreamTest, SendCodecCanApplyVad) { helper.event_log()); } +TEST(AudioSendStreamTest, DoesNotPassHigherBitrateThanMaxBitrate) { + ConfigHelper helper; + internal::AudioSendStream send_stream( + helper.config(), helper.audio_state(), helper.worker_queue(), + helper.congestion_controller(), helper.bitrate_allocator(), + helper.event_log()); + EXPECT_CALL(*helper.channel_proxy(), + SetBitrate(helper.config().max_bitrate_bps, _)); + send_stream.OnBitrateUpdated(helper.config().max_bitrate_bps + 5000, 0.0, 50, + 6000); +} + +TEST(AudioSendStreamTest, ProbingIntervalOnBitrateUpdated) { + ConfigHelper helper; + internal::AudioSendStream send_stream( + helper.config(), helper.audio_state(), helper.worker_queue(), + helper.congestion_controller(), helper.bitrate_allocator(), + helper.event_log()); + EXPECT_CALL(*helper.channel_proxy(), SetBitrate(_, 5000)); + send_stream.OnBitrateUpdated(50000, 0.0, 50, 5000); +} + } // namespace test } // namespace webrtc diff --git a/webrtc/call/bitrate_allocator.cc b/webrtc/call/bitrate_allocator.cc index 2c5ba6d76c..70763f3e8d 100644 --- a/webrtc/call/bitrate_allocator.cc +++ b/webrtc/call/bitrate_allocator.cc @@ -65,13 +65,15 @@ BitrateAllocator::~BitrateAllocator() { void BitrateAllocator::OnNetworkChanged(uint32_t target_bitrate_bps, uint8_t fraction_loss, - int64_t rtt) { + int64_t rtt, + int64_t probing_interval_ms) { RTC_DCHECK_CALLED_SEQUENTIALLY(&sequenced_checker_); last_bitrate_bps_ = target_bitrate_bps; last_non_zero_bitrate_bps_ = target_bitrate_bps > 0 ? target_bitrate_bps : last_non_zero_bitrate_bps_; last_fraction_loss_ = fraction_loss; last_rtt_ = rtt; + last_probing_interval_ms_ = probing_interval_ms; // Periodically log the incoming BWE. int64_t now = clock_->TimeInMilliseconds(); @@ -85,7 +87,8 @@ void BitrateAllocator::OnNetworkChanged(uint32_t target_bitrate_bps, for (auto& config : bitrate_observer_configs_) { uint32_t allocated_bitrate = allocation[config.observer]; uint32_t protection_bitrate = config.observer->OnBitrateUpdated( - allocated_bitrate, last_fraction_loss_, last_rtt_); + allocated_bitrate, last_fraction_loss_, last_rtt_, + last_probing_interval_ms_); if (allocated_bitrate == 0 && config.allocated_bitrate_bps > 0) { if (target_bitrate_bps > 0) @@ -141,7 +144,8 @@ void BitrateAllocator::AddObserver(BitrateAllocatorObserver* observer, for (auto& config : bitrate_observer_configs_) { uint32_t allocated_bitrate = allocation[config.observer]; uint32_t protection_bitrate = config.observer->OnBitrateUpdated( - allocated_bitrate, last_fraction_loss_, last_rtt_); + allocated_bitrate, last_fraction_loss_, last_rtt_, + last_probing_interval_ms_); config.allocated_bitrate_bps = allocated_bitrate; if (allocated_bitrate > 0) config.media_ratio = MediaRatio(allocated_bitrate, protection_bitrate); @@ -151,7 +155,8 @@ void BitrateAllocator::AddObserver(BitrateAllocatorObserver* observer, // But we still have to return the initial config bitrate + let the // observer know that it can not produce frames. allocation = AllocateBitrates(last_non_zero_bitrate_bps_); - observer->OnBitrateUpdated(0, last_fraction_loss_, last_rtt_); + observer->OnBitrateUpdated(0, last_fraction_loss_, last_rtt_, + last_probing_interval_ms_); } UpdateAllocationLimits(); } diff --git a/webrtc/call/bitrate_allocator.h b/webrtc/call/bitrate_allocator.h index a5ed26c71b..cd3b2e163b 100644 --- a/webrtc/call/bitrate_allocator.h +++ b/webrtc/call/bitrate_allocator.h @@ -33,7 +33,9 @@ class BitrateAllocatorObserver { // implementation, as bitrate in bps. virtual uint32_t OnBitrateUpdated(uint32_t bitrate_bps, uint8_t fraction_loss, - int64_t rtt) = 0; + int64_t rtt, + int64_t probing_interval_ms) = 0; + protected: virtual ~BitrateAllocatorObserver() {} }; @@ -61,7 +63,8 @@ class BitrateAllocator { // Allocate target_bitrate across the registered BitrateAllocatorObservers. void OnNetworkChanged(uint32_t target_bitrate_bps, uint8_t fraction_loss, - int64_t rtt); + int64_t rtt, + int64_t probing_interval_ms); // Set the start and max send bitrate used by the bandwidth management. // @@ -157,6 +160,7 @@ class BitrateAllocator { uint32_t last_non_zero_bitrate_bps_ GUARDED_BY(&sequenced_checker_); uint8_t last_fraction_loss_ GUARDED_BY(&sequenced_checker_); int64_t last_rtt_ GUARDED_BY(&sequenced_checker_); + int64_t last_probing_interval_ms_ GUARDED_BY(&sequenced_checker_); // Number of mute events based on too low BWE, not network up/down. int num_pause_events_ GUARDED_BY(&sequenced_checker_); Clock* const clock_ GUARDED_BY(&sequenced_checker_); diff --git a/webrtc/call/bitrate_allocator_unittest.cc b/webrtc/call/bitrate_allocator_unittest.cc index 5d0395d82c..751ec93657 100644 --- a/webrtc/call/bitrate_allocator_unittest.cc +++ b/webrtc/call/bitrate_allocator_unittest.cc @@ -34,6 +34,7 @@ class TestBitrateObserver : public BitrateAllocatorObserver { : last_bitrate_bps_(0), last_fraction_loss_(0), last_rtt_ms_(0), + last_probing_interval_ms_(0), protection_ratio_(0.0) {} void SetBitrateProtectionRatio(double protection_ratio) { @@ -42,22 +43,29 @@ class TestBitrateObserver : public BitrateAllocatorObserver { uint32_t OnBitrateUpdated(uint32_t bitrate_bps, uint8_t fraction_loss, - int64_t rtt) override { + int64_t rtt, + int64_t probing_interval_ms) override { last_bitrate_bps_ = bitrate_bps; last_fraction_loss_ = fraction_loss; last_rtt_ms_ = rtt; + last_probing_interval_ms_ = probing_interval_ms; return bitrate_bps * protection_ratio_; } uint32_t last_bitrate_bps_; uint8_t last_fraction_loss_; int64_t last_rtt_ms_; + int last_probing_interval_ms_; double protection_ratio_; }; +namespace { +constexpr int64_t kDefaultProbingIntervalMs = 3000; +} + class BitrateAllocatorTest : public ::testing::Test { protected: BitrateAllocatorTest() : allocator_(new BitrateAllocator(&limit_observer_)) { - allocator_->OnNetworkChanged(300000u, 0, 0); + allocator_->OnNetworkChanged(300000u, 0, 0, kDefaultProbingIntervalMs); } ~BitrateAllocatorTest() {} @@ -75,12 +83,12 @@ TEST_F(BitrateAllocatorTest, UpdatingBitrateObserver) { allocator_->AddObserver(&bitrate_observer, kMinSendBitrateBps, 1500000, kPadUpToBitrateBps, true); EXPECT_EQ(300000, allocator_->GetStartBitrate(&bitrate_observer)); - allocator_->OnNetworkChanged(200000, 0, 0); + allocator_->OnNetworkChanged(200000, 0, 0, kDefaultProbingIntervalMs); EXPECT_EQ(200000, allocator_->GetStartBitrate(&bitrate_observer)); // TODO(pbos): Expect capping to 1.5M instead of 3M when not boosting the max // bitrate for FEC/retransmissions (see todo in BitrateAllocator). - allocator_->OnNetworkChanged(4000000, 0, 0); + allocator_->OnNetworkChanged(4000000, 0, 0, kDefaultProbingIntervalMs); EXPECT_EQ(3000000, allocator_->GetStartBitrate(&bitrate_observer)); // Expect |max_padding_bitrate_bps| to change to 0 if the observer is updated. @@ -96,7 +104,7 @@ TEST_F(BitrateAllocatorTest, UpdatingBitrateObserver) { true); EXPECT_EQ(3000000, allocator_->GetStartBitrate(&bitrate_observer)); EXPECT_EQ(3000000u, bitrate_observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(1500000, 0, 0); + allocator_->OnNetworkChanged(1500000, 0, 0, kDefaultProbingIntervalMs); EXPECT_EQ(1500000u, bitrate_observer.last_bitrate_bps_); } @@ -114,7 +122,7 @@ TEST_F(BitrateAllocatorTest, TwoBitrateObserversOneRtcpObserver) { // Test too low start bitrate, hence lower than sum of min. Min bitrates // will // be allocated to all observers. - allocator_->OnNetworkChanged(200000, 0, 50); + allocator_->OnNetworkChanged(200000, 0, 50, kDefaultProbingIntervalMs); EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_bps_); EXPECT_EQ(0, bitrate_observer_1.last_fraction_loss_); EXPECT_EQ(50, bitrate_observer_1.last_rtt_ms_); @@ -123,7 +131,7 @@ TEST_F(BitrateAllocatorTest, TwoBitrateObserversOneRtcpObserver) { EXPECT_EQ(50, bitrate_observer_2.last_rtt_ms_); // Test a bitrate which should be distributed equally. - allocator_->OnNetworkChanged(500000, 0, 50); + allocator_->OnNetworkChanged(500000, 0, 50, kDefaultProbingIntervalMs); const uint32_t kBitrateToShare = 500000 - 200000 - 100000; EXPECT_EQ(100000u + kBitrateToShare / 2, bitrate_observer_1.last_bitrate_bps_); @@ -132,14 +140,14 @@ TEST_F(BitrateAllocatorTest, TwoBitrateObserversOneRtcpObserver) { // Limited by 2x max bitrates since we leave room for FEC and // retransmissions. - allocator_->OnNetworkChanged(1500000, 0, 50); + allocator_->OnNetworkChanged(1500000, 0, 50, kDefaultProbingIntervalMs); EXPECT_EQ(600000u, bitrate_observer_1.last_bitrate_bps_); EXPECT_EQ(600000u, bitrate_observer_2.last_bitrate_bps_); // Verify that if the bandwidth estimate is set to zero, the allocated // rate is // zero. - allocator_->OnNetworkChanged(0, 0, 50); + allocator_->OnNetworkChanged(0, 0, 50, kDefaultProbingIntervalMs); EXPECT_EQ(0u, bitrate_observer_1.last_bitrate_bps_); EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_bps_); } @@ -161,7 +169,7 @@ class BitrateAllocatorTestNoEnforceMin : public ::testing::Test { protected: BitrateAllocatorTestNoEnforceMin() : allocator_(new BitrateAllocator(&limit_observer_)) { - allocator_->OnNetworkChanged(300000u, 0, 0); + allocator_->OnNetworkChanged(300000u, 0, 0, kDefaultProbingIntervalMs); } ~BitrateAllocatorTestNoEnforceMin() {} @@ -180,11 +188,11 @@ TEST_F(BitrateAllocatorTestNoEnforceMin, OneBitrateObserver) { EXPECT_EQ(300000, allocator_->GetStartBitrate(&bitrate_observer_1)); // High BWE. - allocator_->OnNetworkChanged(150000, 0, 0); + allocator_->OnNetworkChanged(150000, 0, 0, kDefaultProbingIntervalMs); EXPECT_EQ(150000u, bitrate_observer_1.last_bitrate_bps_); // Low BWE. - allocator_->OnNetworkChanged(10000, 0, 0); + allocator_->OnNetworkChanged(10000, 0, 0, kDefaultProbingIntervalMs); EXPECT_EQ(0u, bitrate_observer_1.last_bitrate_bps_); EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(0, 0)); @@ -210,7 +218,7 @@ TEST_F(BitrateAllocatorTestNoEnforceMin, ThreeBitrateObservers) { // High BWE. Make sure the controllers get a fair share of the surplus (i.e., // what is left after each controller gets its min rate). - allocator_->OnNetworkChanged(690000, 0, 0); + allocator_->OnNetworkChanged(690000, 0, 0, kDefaultProbingIntervalMs); // Verify that each observer gets its min rate (sum of min rates is 600000), // and that the remaining 90000 is divided equally among the three. uint32_t bitrate_to_share = 690000u - 100000u - 200000u - 300000u; @@ -222,27 +230,27 @@ TEST_F(BitrateAllocatorTestNoEnforceMin, ThreeBitrateObservers) { bitrate_observer_3.last_bitrate_bps_); // BWE below the sum of observer's min bitrate. - allocator_->OnNetworkChanged(300000, 0, 0); + allocator_->OnNetworkChanged(300000, 0, 0, kDefaultProbingIntervalMs); EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_bps_); // Min bitrate. EXPECT_EQ(200000u, bitrate_observer_2.last_bitrate_bps_); // Min bitrate. EXPECT_EQ(0u, bitrate_observer_3.last_bitrate_bps_); // Nothing. // Increased BWE, but still below the sum of configured min bitrates for all // observers and too little for observer 3. 1 and 2 will share the rest. - allocator_->OnNetworkChanged(500000, 0, 0); + allocator_->OnNetworkChanged(500000, 0, 0, kDefaultProbingIntervalMs); EXPECT_EQ(200000u, bitrate_observer_1.last_bitrate_bps_); // Min + split. EXPECT_EQ(300000u, bitrate_observer_2.last_bitrate_bps_); // Min + split. EXPECT_EQ(0u, bitrate_observer_3.last_bitrate_bps_); // Nothing. // Below min for all. - allocator_->OnNetworkChanged(10000, 0, 0); + allocator_->OnNetworkChanged(10000, 0, 0, kDefaultProbingIntervalMs); EXPECT_EQ(0u, bitrate_observer_1.last_bitrate_bps_); EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_bps_); EXPECT_EQ(0u, bitrate_observer_3.last_bitrate_bps_); // Verify that zero estimated bandwidth, means that that all gets zero, // regardless of set min bitrate. - allocator_->OnNetworkChanged(0, 0, 0); + allocator_->OnNetworkChanged(0, 0, 0, kDefaultProbingIntervalMs); EXPECT_EQ(0u, bitrate_observer_1.last_bitrate_bps_); EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_bps_); EXPECT_EQ(0u, bitrate_observer_3.last_bitrate_bps_); @@ -262,34 +270,38 @@ TEST_F(BitrateAllocatorTestNoEnforceMin, OneBitrateObserverWithPacketLoss) { EXPECT_EQ(300000, allocator_->GetStartBitrate(&bitrate_observer)); // High BWE. - allocator_->OnNetworkChanged(150000, 0, 0); + allocator_->OnNetworkChanged(150000, 0, 0, kDefaultProbingIntervalMs); EXPECT_EQ(150000u, bitrate_observer.last_bitrate_bps_); // Add loss and use a part of the bitrate for protection. double protection_ratio = 0.4; uint8_t fraction_loss = protection_ratio * 256; bitrate_observer.SetBitrateProtectionRatio(protection_ratio); - allocator_->OnNetworkChanged(200000, 0, fraction_loss); + allocator_->OnNetworkChanged(200000, 0, fraction_loss, + kDefaultProbingIntervalMs); EXPECT_EQ(200000u, bitrate_observer.last_bitrate_bps_); // Above the min threshold, but not enough given the protection used. - allocator_->OnNetworkChanged(139000, 0, fraction_loss); + allocator_->OnNetworkChanged(139000, 0, fraction_loss, + kDefaultProbingIntervalMs); EXPECT_EQ(0u, bitrate_observer.last_bitrate_bps_); // Verify the hysteresis is added for the protection. - allocator_->OnNetworkChanged(150000, 0, fraction_loss); + allocator_->OnNetworkChanged(150000, 0, fraction_loss, + kDefaultProbingIntervalMs); EXPECT_EQ(0u, bitrate_observer.last_bitrate_bps_); // Just enough to enable video again. - allocator_->OnNetworkChanged(168000, 0, fraction_loss); + allocator_->OnNetworkChanged(168000, 0, fraction_loss, + kDefaultProbingIntervalMs); EXPECT_EQ(168000u, bitrate_observer.last_bitrate_bps_); // Remove all protection and make sure video is not paused as earlier. bitrate_observer.SetBitrateProtectionRatio(0.0); - allocator_->OnNetworkChanged(140000, 0, 0); + allocator_->OnNetworkChanged(140000, 0, 0, kDefaultProbingIntervalMs); EXPECT_EQ(140000u, bitrate_observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(139000, 0, 0); + allocator_->OnNetworkChanged(139000, 0, 0, kDefaultProbingIntervalMs); EXPECT_EQ(139000u, bitrate_observer.last_bitrate_bps_); EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(0, 0)); @@ -308,37 +320,37 @@ TEST_F(BitrateAllocatorTestNoEnforceMin, TwoBitrateObserverWithPacketLoss) { // Enough bitrate for both. bitrate_observer_2.SetBitrateProtectionRatio(0.5); - allocator_->OnNetworkChanged(300000, 0, 0); + allocator_->OnNetworkChanged(300000, 0, 0, kDefaultProbingIntervalMs); EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_bps_); EXPECT_EQ(200000u, bitrate_observer_2.last_bitrate_bps_); // Above min for observer 2, but too little given the protection used. - allocator_->OnNetworkChanged(330000, 0, 0); + allocator_->OnNetworkChanged(330000, 0, 0, kDefaultProbingIntervalMs); EXPECT_EQ(330000u, bitrate_observer_1.last_bitrate_bps_); EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_bps_); - allocator_->OnNetworkChanged(100000, 0, 0); + allocator_->OnNetworkChanged(100000, 0, 0, kDefaultProbingIntervalMs); EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_bps_); EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_bps_); - allocator_->OnNetworkChanged(99999, 0, 0); + allocator_->OnNetworkChanged(99999, 0, 0, kDefaultProbingIntervalMs); EXPECT_EQ(0u, bitrate_observer_1.last_bitrate_bps_); EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_bps_); - allocator_->OnNetworkChanged(119000, 0, 0); + allocator_->OnNetworkChanged(119000, 0, 0, kDefaultProbingIntervalMs); EXPECT_EQ(0u, bitrate_observer_1.last_bitrate_bps_); EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_bps_); - allocator_->OnNetworkChanged(120000, 0, 0); + allocator_->OnNetworkChanged(120000, 0, 0, kDefaultProbingIntervalMs); EXPECT_EQ(120000u, bitrate_observer_1.last_bitrate_bps_); EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_bps_); // Verify the protection is accounted for before resuming observer 2. - allocator_->OnNetworkChanged(429000, 0, 0); + allocator_->OnNetworkChanged(429000, 0, 0, kDefaultProbingIntervalMs); EXPECT_EQ(400000u, bitrate_observer_1.last_bitrate_bps_); EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_bps_); - allocator_->OnNetworkChanged(430000, 0, 0); + allocator_->OnNetworkChanged(430000, 0, 0, kDefaultProbingIntervalMs); EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_bps_); EXPECT_EQ(330000u, bitrate_observer_2.last_bitrate_bps_); @@ -365,7 +377,7 @@ TEST_F(BitrateAllocatorTest, ThreeBitrateObserversLowBweEnforceMin) { // Low BWE. Verify that all observers still get their respective min // bitrate. - allocator_->OnNetworkChanged(1000, 0, 0); + allocator_->OnNetworkChanged(1000, 0, 0, kDefaultProbingIntervalMs); EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_bps_); // Min cap. EXPECT_EQ(200000u, bitrate_observer_2.last_bitrate_bps_); // Min cap. EXPECT_EQ(300000u, bitrate_observer_3.last_bitrate_bps_); // Min cap. @@ -383,7 +395,7 @@ TEST_F(BitrateAllocatorTest, AddObserverWhileNetworkDown) { EXPECT_EQ(300000, allocator_->GetStartBitrate(&bitrate_observer_1)); // Set network down, ie, no available bitrate. - allocator_->OnNetworkChanged(0, 0, 0); + allocator_->OnNetworkChanged(0, 0, 0, kDefaultProbingIntervalMs); EXPECT_EQ(0u, bitrate_observer_1.last_bitrate_bps_); @@ -399,7 +411,7 @@ TEST_F(BitrateAllocatorTest, AddObserverWhileNetworkDown) { EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_bps_); // Set network back up. - allocator_->OnNetworkChanged(1500000, 0, 50); + allocator_->OnNetworkChanged(1500000, 0, 50, kDefaultProbingIntervalMs); EXPECT_EQ(750000u, bitrate_observer_1.last_bitrate_bps_); EXPECT_EQ(750000u, bitrate_observer_2.last_bitrate_bps_); } @@ -414,31 +426,31 @@ TEST_F(BitrateAllocatorTest, MixedEnforecedConfigs) { EXPECT_EQ(270000, allocator_->GetStartBitrate(¬_enforced_observer)); EXPECT_EQ(30000u, enforced_observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(36000, 0, 50); + allocator_->OnNetworkChanged(36000, 0, 50, kDefaultProbingIntervalMs); EXPECT_EQ(6000u, enforced_observer.last_bitrate_bps_); EXPECT_EQ(30000u, not_enforced_observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(35000, 0, 50); + allocator_->OnNetworkChanged(35000, 0, 50, kDefaultProbingIntervalMs); EXPECT_EQ(30000u, enforced_observer.last_bitrate_bps_); EXPECT_EQ(0u, not_enforced_observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(5000, 0, 50); + allocator_->OnNetworkChanged(5000, 0, 50, kDefaultProbingIntervalMs); EXPECT_EQ(6000u, enforced_observer.last_bitrate_bps_); EXPECT_EQ(0u, not_enforced_observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(36000, 0, 50); + allocator_->OnNetworkChanged(36000, 0, 50, kDefaultProbingIntervalMs); EXPECT_EQ(30000u, enforced_observer.last_bitrate_bps_); EXPECT_EQ(0u, not_enforced_observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(55000, 0, 50); + allocator_->OnNetworkChanged(55000, 0, 50, kDefaultProbingIntervalMs); EXPECT_EQ(30000u, enforced_observer.last_bitrate_bps_); EXPECT_EQ(0u, not_enforced_observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(56000, 0, 50); + allocator_->OnNetworkChanged(56000, 0, 50, kDefaultProbingIntervalMs); EXPECT_EQ(6000u, enforced_observer.last_bitrate_bps_); EXPECT_EQ(50000u, not_enforced_observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(56000, 0, 50); + allocator_->OnNetworkChanged(56000, 0, 50, kDefaultProbingIntervalMs); EXPECT_EQ(16000u, enforced_observer.last_bitrate_bps_); EXPECT_EQ(40000u, not_enforced_observer.last_bitrate_bps_); @@ -451,22 +463,22 @@ TEST_F(BitrateAllocatorTest, AvoidToggleAbsolute) { allocator_->AddObserver(&observer, 30000, 300000, 0, false); EXPECT_EQ(300000, allocator_->GetStartBitrate(&observer)); - allocator_->OnNetworkChanged(30000, 0, 50); + allocator_->OnNetworkChanged(30000, 0, 50, kDefaultProbingIntervalMs); EXPECT_EQ(30000u, observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(20000, 0, 50); + allocator_->OnNetworkChanged(20000, 0, 50, kDefaultProbingIntervalMs); EXPECT_EQ(0u, observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(30000, 0, 50); + allocator_->OnNetworkChanged(30000, 0, 50, kDefaultProbingIntervalMs); EXPECT_EQ(0u, observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(49000, 0, 50); + allocator_->OnNetworkChanged(49000, 0, 50, kDefaultProbingIntervalMs); EXPECT_EQ(0u, observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(50000, 0, 50); + allocator_->OnNetworkChanged(50000, 0, 50, kDefaultProbingIntervalMs); EXPECT_EQ(50000u, observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(30000, 0, 50); + allocator_->OnNetworkChanged(30000, 0, 50, kDefaultProbingIntervalMs); EXPECT_EQ(30000u, observer.last_bitrate_bps_); allocator_->RemoveObserver(&observer); @@ -477,25 +489,36 @@ TEST_F(BitrateAllocatorTest, AvoidTogglePercent) { allocator_->AddObserver(&observer, 300000, 600000, 0, false); EXPECT_EQ(300000, allocator_->GetStartBitrate(&observer)); - allocator_->OnNetworkChanged(300000, 0, 50); + allocator_->OnNetworkChanged(300000, 0, 50, kDefaultProbingIntervalMs); EXPECT_EQ(300000u, observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(200000, 0, 50); + allocator_->OnNetworkChanged(200000, 0, 50, kDefaultProbingIntervalMs); EXPECT_EQ(0u, observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(300000, 0, 50); + allocator_->OnNetworkChanged(300000, 0, 50, kDefaultProbingIntervalMs); EXPECT_EQ(0u, observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(329000, 0, 50); + allocator_->OnNetworkChanged(329000, 0, 50, kDefaultProbingIntervalMs); EXPECT_EQ(0u, observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(330000, 0, 50); + allocator_->OnNetworkChanged(330000, 0, 50, kDefaultProbingIntervalMs); EXPECT_EQ(330000u, observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(300000, 0, 50); + allocator_->OnNetworkChanged(300000, 0, 50, kDefaultProbingIntervalMs); EXPECT_EQ(300000u, observer.last_bitrate_bps_); allocator_->RemoveObserver(&observer); } +TEST_F(BitrateAllocatorTest, PassProbingInterval) { + TestBitrateObserver observer; + allocator_->AddObserver(&observer, 300000, 600000, 0, false); + EXPECT_EQ(300000, allocator_->GetStartBitrate(&observer)); + + allocator_->OnNetworkChanged(300000, 0, 50, 5000); + EXPECT_EQ(5000, observer.last_probing_interval_ms_); + + allocator_->RemoveObserver(&observer); +} + } // namespace webrtc diff --git a/webrtc/call/call.cc b/webrtc/call/call.cc index d2bc153d32..afcf546566 100644 --- a/webrtc/call/call.cc +++ b/webrtc/call/call.cc @@ -120,8 +120,10 @@ class Call : public webrtc::Call, void OnSentPacket(const rtc::SentPacket& sent_packet) override; // Implements BitrateObserver. - void OnNetworkChanged(uint32_t bitrate_bps, uint8_t fraction_loss, - int64_t rtt_ms) override; + void OnNetworkChanged(uint32_t bitrate_bps, + uint8_t fraction_loss, + int64_t rtt_ms, + int64_t probing_interval_ms) override; // Implements BitrateAllocator::LimitObserver. void OnAllocationLimitsChanged(uint32_t min_send_bitrate_bps, @@ -877,19 +879,23 @@ void Call::OnSentPacket(const rtc::SentPacket& sent_packet) { congestion_controller_->OnSentPacket(sent_packet); } -void Call::OnNetworkChanged(uint32_t target_bitrate_bps, uint8_t fraction_loss, - int64_t rtt_ms) { +void Call::OnNetworkChanged(uint32_t target_bitrate_bps, + uint8_t fraction_loss, + int64_t rtt_ms, + int64_t probing_interval_ms) { // TODO(perkj): Consider making sure CongestionController operates on // |worker_queue_|. if (!worker_queue_.IsCurrent()) { - worker_queue_.PostTask([this, target_bitrate_bps, fraction_loss, rtt_ms] { - OnNetworkChanged(target_bitrate_bps, fraction_loss, rtt_ms); - }); + worker_queue_.PostTask( + [this, target_bitrate_bps, fraction_loss, rtt_ms, probing_interval_ms] { + OnNetworkChanged(target_bitrate_bps, fraction_loss, rtt_ms, + probing_interval_ms); + }); return; } RTC_DCHECK_RUN_ON(&worker_queue_); bitrate_allocator_->OnNetworkChanged(target_bitrate_bps, fraction_loss, - rtt_ms); + rtt_ms, probing_interval_ms); // Ignore updates if bitrate is zero (the aggregate network state is down). if (target_bitrate_bps == 0) { diff --git a/webrtc/modules/congestion_controller/congestion_controller.cc b/webrtc/modules/congestion_controller/congestion_controller.cc index accc1411f9..86ad275d2a 100644 --- a/webrtc/modules/congestion_controller/congestion_controller.cc +++ b/webrtc/modules/congestion_controller/congestion_controller.cc @@ -342,7 +342,9 @@ void CongestionController::MaybeTriggerOnNetworkChanged() { bitrate_bps = IsNetworkDown() || IsSendQueueFull() ? 0 : bitrate_bps; if (HasNetworkParametersToReportChanged(bitrate_bps, fraction_loss, rtt)) { - observer_->OnNetworkChanged(bitrate_bps, fraction_loss, rtt); + observer_->OnNetworkChanged( + bitrate_bps, fraction_loss, rtt, + transport_feedback_adapter_.GetProbingIntervalMs()); remote_estimator_proxy_.OnBitrateChanged(bitrate_bps); } } diff --git a/webrtc/modules/congestion_controller/congestion_controller_unittest.cc b/webrtc/modules/congestion_controller/congestion_controller_unittest.cc index 42bb5267b2..a72d80bbb3 100644 --- a/webrtc/modules/congestion_controller/congestion_controller_unittest.cc +++ b/webrtc/modules/congestion_controller/congestion_controller_unittest.cc @@ -46,7 +46,7 @@ class CongestionControllerTest : public ::testing::Test { // Set the initial bitrate estimate and expect the |observer| and |pacer_| // to be updated. - EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps, _, _)); + EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps, _, _, _)); EXPECT_CALL(*pacer_, SetEstimatedBitrate(kInitialBitrateBps)); controller_->SetBweBitrates(0, kInitialBitrateBps, 5 * kInitialBitrateBps); } @@ -66,13 +66,13 @@ TEST_F(CongestionControllerTest, OnNetworkChanged) { clock_.AdvanceTimeMilliseconds(25); controller_->Process(); - EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps * 2, _, _)); + EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps * 2, _, _, _)); EXPECT_CALL(*pacer_, SetEstimatedBitrate(kInitialBitrateBps * 2)); bandwidth_observer_->OnReceivedEstimatedBitrate(kInitialBitrateBps * 2); clock_.AdvanceTimeMilliseconds(25); controller_->Process(); - EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps, _, _)); + EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps, _, _, _)); EXPECT_CALL(*pacer_, SetEstimatedBitrate(kInitialBitrateBps)); bandwidth_observer_->OnReceivedEstimatedBitrate(kInitialBitrateBps); clock_.AdvanceTimeMilliseconds(25); @@ -83,21 +83,21 @@ TEST_F(CongestionControllerTest, OnSendQueueFull) { EXPECT_CALL(*pacer_, ExpectedQueueTimeMs()) .WillOnce(Return(PacedSender::kMaxQueueLengthMs + 1)); - EXPECT_CALL(observer_, OnNetworkChanged(0, _, _)); + EXPECT_CALL(observer_, OnNetworkChanged(0, _, _, _)); controller_->Process(); // Let the pacer not be full next time the controller checks. EXPECT_CALL(*pacer_, ExpectedQueueTimeMs()) .WillOnce(Return(PacedSender::kMaxQueueLengthMs - 1)); - EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps, _, _)); + EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps, _, _, _)); controller_->Process(); } TEST_F(CongestionControllerTest, OnSendQueueFullAndEstimateChange) { EXPECT_CALL(*pacer_, ExpectedQueueTimeMs()) .WillOnce(Return(PacedSender::kMaxQueueLengthMs + 1)); - EXPECT_CALL(observer_, OnNetworkChanged(0, _, _)); + EXPECT_CALL(observer_, OnNetworkChanged(0, _, _, _)); controller_->Process(); // Receive new estimate but let the queue still be full. @@ -113,32 +113,33 @@ TEST_F(CongestionControllerTest, OnSendQueueFullAndEstimateChange) { // |OnNetworkChanged| should be called with the new estimate. EXPECT_CALL(*pacer_, ExpectedQueueTimeMs()) .WillOnce(Return(PacedSender::kMaxQueueLengthMs - 1)); - EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps * 2, _, _)); + EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps * 2, _, _, _)); clock_.AdvanceTimeMilliseconds(25); controller_->Process(); } TEST_F(CongestionControllerTest, SignalNetworkState) { - EXPECT_CALL(observer_, OnNetworkChanged(0, _, _)); + EXPECT_CALL(observer_, OnNetworkChanged(0, _, _, _)); controller_->SignalNetworkState(kNetworkDown); - EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps, _, _)); + EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps, _, _, _)); controller_->SignalNetworkState(kNetworkUp); - EXPECT_CALL(observer_, OnNetworkChanged(0, _, _)); + EXPECT_CALL(observer_, OnNetworkChanged(0, _, _, _)); controller_->SignalNetworkState(kNetworkDown); } TEST_F(CongestionControllerTest, ResetBweAndBitrates) { int new_bitrate = 200000; - EXPECT_CALL(observer_, OnNetworkChanged(new_bitrate, _, _)); + EXPECT_CALL(observer_, OnNetworkChanged(new_bitrate, _, _, _)); EXPECT_CALL(*pacer_, SetEstimatedBitrate(new_bitrate)); controller_->ResetBweAndBitrates(new_bitrate, -1, -1); // If the bitrate is reset to -1, the new starting bitrate will be // the minimum default bitrate kMinBitrateBps. - EXPECT_CALL(observer_, OnNetworkChanged( - congestion_controller::GetMinBitrateBps(), _, _)); + EXPECT_CALL( + observer_, + OnNetworkChanged(congestion_controller::GetMinBitrateBps(), _, _, _)); EXPECT_CALL(*pacer_, SetEstimatedBitrate(congestion_controller::GetMinBitrateBps())); controller_->ResetBweAndBitrates(-1, -1, -1); @@ -149,7 +150,7 @@ TEST_F(CongestionControllerTest, // Send queue is full EXPECT_CALL(*pacer_, ExpectedQueueTimeMs()) .WillRepeatedly(Return(PacedSender::kMaxQueueLengthMs + 1)); - EXPECT_CALL(observer_, OnNetworkChanged(0, _, _)); + EXPECT_CALL(observer_, OnNetworkChanged(0, _, _, _)); controller_->Process(); // Queue is full and network is down. Expect no bitrate change. @@ -169,12 +170,12 @@ TEST_F(CongestionControllerTest, // Let the pacer not be full next time the controller checks. EXPECT_CALL(*pacer_, ExpectedQueueTimeMs()) .WillOnce(Return(PacedSender::kMaxQueueLengthMs - 1)); - EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps * 2, _, _)); + EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps * 2, _, _, _)); controller_->Process(); } TEST_F(CongestionControllerTest, GetPacerQueuingDelayMs) { - EXPECT_CALL(observer_, OnNetworkChanged(_, _, _)).Times(AtLeast(1)); + EXPECT_CALL(observer_, OnNetworkChanged(_, _, _, _)).Times(AtLeast(1)); const int64_t kQueueTimeMs = 123; EXPECT_CALL(*pacer_, QueueInMs()).WillRepeatedly(Return(kQueueTimeMs)); @@ -189,5 +190,16 @@ TEST_F(CongestionControllerTest, GetPacerQueuingDelayMs) { EXPECT_EQ(kQueueTimeMs, controller_->GetPacerQueuingDelayMs()); } +TEST_F(CongestionControllerTest, GetProbingInterval) { + clock_.AdvanceTimeMilliseconds(25); + controller_->Process(); + + EXPECT_CALL(observer_, OnNetworkChanged(_, _, _, testing::Ne(0))); + EXPECT_CALL(*pacer_, SetEstimatedBitrate(_)); + bandwidth_observer_->OnReceivedEstimatedBitrate(kInitialBitrateBps * 2); + clock_.AdvanceTimeMilliseconds(25); + controller_->Process(); +} + } // namespace test } // namespace webrtc diff --git a/webrtc/modules/congestion_controller/delay_based_bwe.cc b/webrtc/modules/congestion_controller/delay_based_bwe.cc index 78bc7f0e6e..04dadc489c 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe.cc @@ -180,7 +180,8 @@ DelayBasedBwe::DelayBasedBwe(Clock* clock) trendline_window_size_(kDefaultTrendlineWindowSize), trendline_smoothing_coeff_(kDefaultTrendlineSmoothingCoeff), trendline_threshold_gain_(kDefaultTrendlineThresholdGain), - in_trendline_experiment_(TrendlineFilterExperimentIsEnabled()) { + in_trendline_experiment_(TrendlineFilterExperimentIsEnabled()), + probing_interval_estimator_(&rate_control_) { if (in_trendline_experiment_) { ReadTrendlineFilterExperimentParameters(&trendline_window_size_, &trendline_smoothing_coeff_, @@ -330,4 +331,8 @@ void DelayBasedBwe::SetMinBitrate(int min_bitrate_bps) { // be called from the network thread in the future. rate_control_.SetMinBitrate(min_bitrate_bps); } + +int64_t DelayBasedBwe::GetProbingIntervalMs() const { + return probing_interval_estimator_.GetIntervalMs(); +} } // namespace webrtc diff --git a/webrtc/modules/congestion_controller/delay_based_bwe.h b/webrtc/modules/congestion_controller/delay_based_bwe.h index 0f1472e672..c5a599f334 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe.h +++ b/webrtc/modules/congestion_controller/delay_based_bwe.h @@ -20,6 +20,7 @@ #include "webrtc/base/rate_statistics.h" #include "webrtc/base/thread_checker.h" #include "webrtc/modules/congestion_controller/probe_bitrate_estimator.h" +#include "webrtc/modules/congestion_controller/probing_interval_estimator.h" #include "webrtc/modules/congestion_controller/trendline_estimator.h" #include "webrtc/modules/remote_bitrate_estimator/aimd_rate_control.h" #include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" @@ -51,6 +52,7 @@ class DelayBasedBwe { bool LatestEstimate(std::vector* ssrcs, uint32_t* bitrate_bps) const; void SetMinBitrate(int min_bitrate_bps); + int64_t GetProbingIntervalMs() const; private: // Computes a bayesian estimate of the throughput given acks containing @@ -99,6 +101,7 @@ class DelayBasedBwe { double trendline_smoothing_coeff_; double trendline_threshold_gain_; const bool in_trendline_experiment_; + ProbingIntervalEstimator probing_interval_estimator_; RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(DelayBasedBwe); }; diff --git a/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc b/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc index 3f26e592ae..c242bf4f27 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc @@ -116,6 +116,15 @@ TEST_F(DelayBasedBweTest, ProbeDetectionSlowerArrivalHighBitrate) { EXPECT_NEAR(bitrate_observer_.latest_bitrate(), 4000000u, 10000u); } +TEST_F(DelayBasedBweTest, GetProbingInterval) { + int64_t default_interval_ms = bitrate_estimator_->GetProbingIntervalMs(); + EXPECT_GT(default_interval_ms, 0); + CapacityDropTestHelper(1, true, 567, 0); + int64_t interval_ms = bitrate_estimator_->GetProbingIntervalMs(); + EXPECT_GT(interval_ms, 0); + EXPECT_NE(interval_ms, default_interval_ms); +} + TEST_F(DelayBasedBweTest, InitialBehavior) { InitialBehaviorTestHelper(674840); } diff --git a/webrtc/modules/congestion_controller/include/congestion_controller.h b/webrtc/modules/congestion_controller/include/congestion_controller.h index 8663489d54..1eb5b3441b 100644 --- a/webrtc/modules/congestion_controller/include/congestion_controller.h +++ b/webrtc/modules/congestion_controller/include/congestion_controller.h @@ -48,7 +48,8 @@ class CongestionController : public CallStatsObserver, public Module { public: virtual void OnNetworkChanged(uint32_t bitrate_bps, uint8_t fraction_loss, // 0 - 255. - int64_t rtt_ms) = 0; + int64_t rtt_ms, + int64_t probing_interval_ms) = 0; protected: virtual ~Observer() {} diff --git a/webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h b/webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h index 128d64ea98..9fb422634d 100644 --- a/webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h +++ b/webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h @@ -21,10 +21,11 @@ namespace test { class MockCongestionObserver : public CongestionController::Observer { public: - MOCK_METHOD3(OnNetworkChanged, + MOCK_METHOD4(OnNetworkChanged, void(uint32_t bitrate_bps, uint8_t fraction_loss, - int64_t rtt_ms)); + int64_t rtt_ms, + int64_t probing_interval_ms)); }; class MockCongestionController : public CongestionController { diff --git a/webrtc/modules/congestion_controller/probing_interval_estimator.cc b/webrtc/modules/congestion_controller/probing_interval_estimator.cc index ed1134737e..6e7a674ae8 100644 --- a/webrtc/modules/congestion_controller/probing_interval_estimator.cc +++ b/webrtc/modules/congestion_controller/probing_interval_estimator.cc @@ -17,6 +17,7 @@ namespace webrtc { namespace { constexpr int kMinIntervalMs = 2000; +constexpr int kDefaultIntervalMs = 3000; constexpr int kMaxIntervalMs = 50000; } @@ -24,27 +25,31 @@ ProbingIntervalEstimator::ProbingIntervalEstimator( const AimdRateControl* aimd_rate_control) : ProbingIntervalEstimator(kMinIntervalMs, kMaxIntervalMs, + kDefaultIntervalMs, aimd_rate_control) {} ProbingIntervalEstimator::ProbingIntervalEstimator( - int min_interval_ms, - int max_interval_ms, + int64_t min_interval_ms, + int64_t max_interval_ms, + int64_t default_interval_ms, const AimdRateControl* aimd_rate_control) : min_interval_ms_(min_interval_ms), max_interval_ms_(max_interval_ms), + default_interval_ms_(default_interval_ms), aimd_rate_control_(aimd_rate_control) {} -rtc::Optional ProbingIntervalEstimator::GetIntervalMs() const { +int64_t ProbingIntervalEstimator::GetIntervalMs() const { rtc::Optional bitrate_drop = aimd_rate_control_->GetLastBitrateDecreaseBps(); int increase_rate = aimd_rate_control_->GetNearMaxIncreaseRateBps(); if (!bitrate_drop || increase_rate <= 0) - return rtc::Optional(); + return default_interval_ms_; - return rtc::Optional(std::min( + return std::min( max_interval_ms_, - std::max(1000 * (*bitrate_drop) / increase_rate, min_interval_ms_))); + std::max(static_cast(1000 * (*bitrate_drop) / increase_rate), + min_interval_ms_)); } } // namespace webrtc diff --git a/webrtc/modules/congestion_controller/probing_interval_estimator.h b/webrtc/modules/congestion_controller/probing_interval_estimator.h index daa6b91a8c..993e0da550 100644 --- a/webrtc/modules/congestion_controller/probing_interval_estimator.h +++ b/webrtc/modules/congestion_controller/probing_interval_estimator.h @@ -21,14 +21,16 @@ class ProbingIntervalEstimator { public: explicit ProbingIntervalEstimator(const AimdRateControl* aimd_rate_control); - ProbingIntervalEstimator(int min_interval_ms, - int max_interval_ms, + ProbingIntervalEstimator(int64_t min_interval_ms, + int64_t max_interval_ms, + int64_t default_interval_ms, const AimdRateControl* aimd_rate_control); - rtc::Optional GetIntervalMs() const; + int64_t GetIntervalMs() const; private: - const int min_interval_ms_; - const int max_interval_ms_; + const int64_t min_interval_ms_; + const int64_t max_interval_ms_; + const int64_t default_interval_ms_; const AimdRateControl* const aimd_rate_control_; RTC_DISALLOW_COPY_AND_ASSIGN(ProbingIntervalEstimator); }; diff --git a/webrtc/modules/congestion_controller/probing_interval_estimator_unittest.cc b/webrtc/modules/congestion_controller/probing_interval_estimator_unittest.cc index 051f3561d3..bf10718860 100644 --- a/webrtc/modules/congestion_controller/probing_interval_estimator_unittest.cc +++ b/webrtc/modules/congestion_controller/probing_interval_estimator_unittest.cc @@ -24,6 +24,7 @@ namespace { constexpr int kMinIntervalMs = 2000; constexpr int kMaxIntervalMs = 50000; +constexpr int kDefaultIntervalMs = 3000; struct ProbingIntervalEstimatorStates { std::unique_ptr probing_interval_estimator; @@ -34,12 +35,13 @@ ProbingIntervalEstimatorStates CreateProbingIntervalEstimatorStates() { ProbingIntervalEstimatorStates states; states.aimd_rate_control.reset(new MockAimdRateControl()); states.probing_interval_estimator.reset(new ProbingIntervalEstimator( - kMinIntervalMs, kMaxIntervalMs, states.aimd_rate_control.get())); + kMinIntervalMs, kMaxIntervalMs, kDefaultIntervalMs, + states.aimd_rate_control.get())); return states; } } // namespace -TEST(ProbingIntervalEstimatorTest, NoIntervalUntillWeHaveDrop) { +TEST(ProbingIntervalEstimatorTest, DefaultIntervalUntillWeHaveDrop) { auto states = CreateProbingIntervalEstimatorStates(); EXPECT_CALL(*states.aimd_rate_control, GetLastBitrateDecreaseBps()) @@ -49,9 +51,9 @@ TEST(ProbingIntervalEstimatorTest, NoIntervalUntillWeHaveDrop) { .WillOnce(Return(4000)) .WillOnce(Return(4000)); - EXPECT_EQ(rtc::Optional(), + EXPECT_EQ(kDefaultIntervalMs, states.probing_interval_estimator->GetIntervalMs()); - EXPECT_NE(rtc::Optional(), + EXPECT_NE(kDefaultIntervalMs, states.probing_interval_estimator->GetIntervalMs()); } @@ -61,8 +63,7 @@ TEST(ProbingIntervalEstimatorTest, CalcInterval) { .WillOnce(Return(rtc::Optional(20000))); EXPECT_CALL(*states.aimd_rate_control, GetNearMaxIncreaseRateBps()) .WillOnce(Return(5000)); - EXPECT_EQ(rtc::Optional(4000), - states.probing_interval_estimator->GetIntervalMs()); + EXPECT_EQ(4000, states.probing_interval_estimator->GetIntervalMs()); } TEST(ProbingIntervalEstimatorTest, IntervalDoesNotExceedMin) { @@ -71,8 +72,7 @@ TEST(ProbingIntervalEstimatorTest, IntervalDoesNotExceedMin) { .WillOnce(Return(rtc::Optional(1000))); EXPECT_CALL(*states.aimd_rate_control, GetNearMaxIncreaseRateBps()) .WillOnce(Return(5000)); - EXPECT_EQ(rtc::Optional(kMinIntervalMs), - states.probing_interval_estimator->GetIntervalMs()); + EXPECT_EQ(kMinIntervalMs, states.probing_interval_estimator->GetIntervalMs()); } TEST(ProbingIntervalEstimatorTest, IntervalDoesNotExceedMax) { @@ -81,7 +81,6 @@ TEST(ProbingIntervalEstimatorTest, IntervalDoesNotExceedMax) { .WillOnce(Return(rtc::Optional(50000))); EXPECT_CALL(*states.aimd_rate_control, GetNearMaxIncreaseRateBps()) .WillOnce(Return(100)); - EXPECT_EQ(rtc::Optional(kMaxIntervalMs), - states.probing_interval_estimator->GetIntervalMs()); + EXPECT_EQ(kMaxIntervalMs, states.probing_interval_estimator->GetIntervalMs()); } } // namespace webrtc diff --git a/webrtc/modules/congestion_controller/transport_feedback_adapter.cc b/webrtc/modules/congestion_controller/transport_feedback_adapter.cc index 46639067c2..7756fee365 100644 --- a/webrtc/modules/congestion_controller/transport_feedback_adapter.cc +++ b/webrtc/modules/congestion_controller/transport_feedback_adapter.cc @@ -73,6 +73,11 @@ void TransportFeedbackAdapter::SetMinBitrate(int min_bitrate_bps) { delay_based_bwe_->SetMinBitrate(min_bitrate_bps); } +int64_t TransportFeedbackAdapter::GetProbingIntervalMs() const { + rtc::CritScope cs(&bwe_lock_); + return delay_based_bwe_->GetProbingIntervalMs(); +} + std::vector TransportFeedbackAdapter::GetPacketFeedbackVector( const rtcp::TransportFeedback& feedback) { int64_t timestamp_us = feedback.GetBaseTimeUs(); diff --git a/webrtc/modules/congestion_controller/transport_feedback_adapter.h b/webrtc/modules/congestion_controller/transport_feedback_adapter.h index 6422736c5c..09363ec50c 100644 --- a/webrtc/modules/congestion_controller/transport_feedback_adapter.h +++ b/webrtc/modules/congestion_controller/transport_feedback_adapter.h @@ -50,6 +50,8 @@ class TransportFeedbackAdapter : public TransportFeedbackObserver, void SetMinBitrate(int min_bitrate_bps); + int64_t GetProbingIntervalMs() const; + private: std::vector GetPacketFeedbackVector( const rtcp::TransportFeedback& feedback); diff --git a/webrtc/test/fuzzers/congestion_controller_feedback_fuzzer.cc b/webrtc/test/fuzzers/congestion_controller_feedback_fuzzer.cc index 496af90c69..6d5deab2cd 100644 --- a/webrtc/test/fuzzers/congestion_controller_feedback_fuzzer.cc +++ b/webrtc/test/fuzzers/congestion_controller_feedback_fuzzer.cc @@ -21,7 +21,8 @@ class NullBitrateObserver : public CongestionController::Observer, ~NullBitrateObserver() override {} void OnNetworkChanged(uint32_t bitrate_bps, uint8_t fraction_loss, - int64_t rtt_ms) override {} + int64_t rtt_ms, + int64_t probing_interval_ms) override {} void OnReceiveBitrateChanged(const std::vector& ssrcs, uint32_t bitrate) override {} }; diff --git a/webrtc/test/mock_voe_channel_proxy.h b/webrtc/test/mock_voe_channel_proxy.h index 3676e89153..4f5a0d2156 100644 --- a/webrtc/test/mock_voe_channel_proxy.h +++ b/webrtc/test/mock_voe_channel_proxy.h @@ -45,7 +45,7 @@ class MockVoEChannelProxy : public voe::ChannelProxy { MOCK_METHOD2(SetSendTelephoneEventPayloadType, bool(int payload_type, int payload_frequency)); MOCK_METHOD2(SendTelephoneEventOutband, bool(int event, int duration_ms)); - MOCK_METHOD1(SetBitrate, void(int bitrate_bps)); + MOCK_METHOD2(SetBitrate, void(int bitrate_bps, int64_t probing_interval_ms)); // TODO(solenberg): Talk the compiler into accepting this mock method: // MOCK_METHOD1(SetSink, void(std::unique_ptr sink)); MOCK_METHOD1(SetInputMute, void(bool muted)); diff --git a/webrtc/tools/event_log_visualizer/analyzer.cc b/webrtc/tools/event_log_visualizer/analyzer.cc index d9b772bcd6..485866c1a6 100644 --- a/webrtc/tools/event_log_visualizer/analyzer.cc +++ b/webrtc/tools/event_log_visualizer/analyzer.cc @@ -451,7 +451,8 @@ class BitrateObserver : public CongestionController::Observer, void OnNetworkChanged(uint32_t bitrate_bps, uint8_t fraction_loss, - int64_t rtt_ms) override { + int64_t rtt_ms, + int64_t probing_interval_ms) override { last_bitrate_bps_ = bitrate_bps; bitrate_updated_ = true; } diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index 964475fd38..e6b418fe71 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -328,7 +328,8 @@ class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver, // Implements BitrateAllocatorObserver. uint32_t OnBitrateUpdated(uint32_t bitrate_bps, uint8_t fraction_loss, - int64_t rtt) override; + int64_t rtt, + int64_t probing_interval_ms) override; // Implements webrtc::VCMProtectionCallback. int ProtectionRequest(const FecProtectionParams* delta_params, @@ -1148,7 +1149,8 @@ void VideoSendStreamImpl::SignalNetworkState(NetworkState state) { uint32_t VideoSendStreamImpl::OnBitrateUpdated(uint32_t bitrate_bps, uint8_t fraction_loss, - int64_t rtt) { + int64_t rtt, + int64_t probing_interval_ms) { RTC_DCHECK_RUN_ON(worker_queue_); RTC_DCHECK(payload_router_.active()) << "VideoSendStream::Start has not been called."; diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc index a7df1f2176..d946676fd6 100644 --- a/webrtc/voice_engine/channel.cc +++ b/webrtc/voice_engine/channel.cc @@ -1301,7 +1301,7 @@ int32_t Channel::SetSendCodec(const CodecInst& codec) { return 0; } -void Channel::SetBitRate(int bitrate_bps) { +void Channel::SetBitRate(int bitrate_bps, int64_t probing_interval_ms) { WEBRTC_TRACE(kTraceInfo, kTraceVoice, VoEId(_instanceId, _channelId), "Channel::SetBitRate(bitrate_bps=%d)", bitrate_bps); audio_coding_->ModifyEncoder([&](std::unique_ptr* encoder) { @@ -1312,10 +1312,15 @@ void Channel::SetBitRate(int bitrate_bps) { // We give smoothed bitrate allocation to audio network adaptor as // the uplink bandwidth. - // TODO(michaelt) : Remove kDefaultBitrateSmoothingTimeConstantMs as soon as - // we pass the probing interval to this function. - constexpr int64_t kDefaultBitrateSmoothingTimeConstantMs = 20000; - bitrate_smoother_.SetTimeConstantMs(kDefaultBitrateSmoothingTimeConstantMs); + // The probing spikes should not affect the bitrate smoother more than 25%. + // To simplify the calculations we use a step response as input signal. + // The step response of an exponential filter is + // u(t) = 1 - e^(-t / time_constant). + // In order to limit the affect of a BWE spike within 25% of its value before + // the next probing, we would choose a time constant that fulfills + // 1 - e^(-probing_interval_ms / time_constant) < 0.25 + // Then 4 * probing_interval_ms is a good choice. + bitrate_smoother_.SetTimeConstantMs(probing_interval_ms * 4); bitrate_smoother_.AddSample(bitrate_bps); audio_coding_->ModifyEncoder([&](std::unique_ptr* encoder) { if (*encoder) { diff --git a/webrtc/voice_engine/channel.h b/webrtc/voice_engine/channel.h index 84f9691ef2..9b9d5c927f 100644 --- a/webrtc/voice_engine/channel.h +++ b/webrtc/voice_engine/channel.h @@ -192,7 +192,7 @@ class Channel int32_t GetSendCodec(CodecInst& codec); int32_t GetRecCodec(CodecInst& codec); int32_t SetSendCodec(const CodecInst& codec); - void SetBitRate(int bitrate_bps); + void SetBitRate(int bitrate_bps, int64_t probing_interval_ms); int32_t SetVADStatus(bool enableVAD, ACMVADMode mode, bool disableDTX); int32_t GetVADStatus(bool& enabledVAD, ACMVADMode& mode, bool& disabledDTX); int32_t SetRecPayloadType(const CodecInst& codec); diff --git a/webrtc/voice_engine/channel_proxy.cc b/webrtc/voice_engine/channel_proxy.cc index 1d67f689a7..04fa93f6ab 100644 --- a/webrtc/voice_engine/channel_proxy.cc +++ b/webrtc/voice_engine/channel_proxy.cc @@ -148,9 +148,9 @@ bool ChannelProxy::SendTelephoneEventOutband(int event, int duration_ms) { return channel()->SendTelephoneEventOutband(event, duration_ms) == 0; } -void ChannelProxy::SetBitrate(int bitrate_bps) { +void ChannelProxy::SetBitrate(int bitrate_bps, int64_t probing_interval_ms) { // May be called on different threads and needs to be handled by the channel. - channel()->SetBitRate(bitrate_bps); + channel()->SetBitRate(bitrate_bps, probing_interval_ms); } void ChannelProxy::SetSink(std::unique_ptr sink) { diff --git a/webrtc/voice_engine/channel_proxy.h b/webrtc/voice_engine/channel_proxy.h index 6e0171b395..6d5f7b0d28 100644 --- a/webrtc/voice_engine/channel_proxy.h +++ b/webrtc/voice_engine/channel_proxy.h @@ -72,7 +72,7 @@ class ChannelProxy { virtual bool SetSendTelephoneEventPayloadType(int payload_type, int payload_frequency); virtual bool SendTelephoneEventOutband(int event, int duration_ms); - virtual void SetBitrate(int bitrate_bps); + virtual void SetBitrate(int bitrate_bps, int64_t probing_interval_ms); virtual void SetSink(std::unique_ptr sink); virtual void SetInputMute(bool muted); virtual void RegisterExternalTransport(Transport* transport); diff --git a/webrtc/voice_engine/voe_codec_impl.cc b/webrtc/voice_engine/voe_codec_impl.cc index 8ac24da4e7..b2fd08d599 100644 --- a/webrtc/voice_engine/voe_codec_impl.cc +++ b/webrtc/voice_engine/voe_codec_impl.cc @@ -137,8 +137,9 @@ int VoECodecImpl::SetBitRate(int channel, int bitrate_bps) { _shared->SetLastError(VE_NOT_INITED, kTraceError); return -1; } + constexpr int64_t kDefaultProbingIntervalMs = 3000; _shared->channel_manager().GetChannel(channel).channel()->SetBitRate( - bitrate_bps); + bitrate_bps, kDefaultProbingIntervalMs); return 0; }