From 6287e82b9bc07fa9bb8474a1140f27d3b5660f8b Mon Sep 17 00:00:00 2001 From: ossu Date: Mon, 28 Nov 2016 08:05:16 -0800 Subject: [PATCH] Revert of Pass time constant to bwe smoothing filter. (patchset #8 id:140001 of https://codereview.webrtc.org/2518923003/ ) Reason for revert: Unfortunately, this change breaks internal projects. Specifically the change to the CongestionController interface means anything implementing it will be forced to change in lock-step. Original issue's description: > Pass time constanct to bwe smoothing filter. > > BUG=webrtc:6443, webrtc:6303 > > Committed: https://crrev.com/9abbf5ae4ec7d688a9b4aa03a405f3faadb74b90 > Cr-Commit-Position: refs/heads/master@{#15266} TBR=minyue@webrtc.org,stefan@webrtc.org,solenberg@webrtc.org,michaelt@webrtc.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=webrtc:6443, webrtc:6303 Review-Url: https://codereview.webrtc.org/2532993002 Cr-Commit-Position: refs/heads/master@{#15272} --- webrtc/audio/audio_send_stream.cc | 5 +- webrtc/audio/audio_send_stream.h | 3 +- webrtc/audio/audio_send_stream_unittest.cc | 24 ---- webrtc/call/bitrate_allocator.cc | 13 +- webrtc/call/bitrate_allocator.h | 8 +- webrtc/call/bitrate_allocator_unittest.cc | 131 ++++++++---------- webrtc/call/call.cc | 22 ++- .../congestion_controller.cc | 4 +- .../congestion_controller_unittest.cc | 44 +++--- .../congestion_controller/delay_based_bwe.cc | 7 +- .../congestion_controller/delay_based_bwe.h | 3 - .../delay_based_bwe_unittest.cc | 9 -- .../include/congestion_controller.h | 3 +- .../include/mock/mock_congestion_controller.h | 5 +- .../probing_interval_estimator.cc | 17 +-- .../probing_interval_estimator.h | 12 +- .../probing_interval_estimator_unittest.cc | 19 +-- .../transport_feedback_adapter.cc | 5 - .../transport_feedback_adapter.h | 2 - .../congestion_controller_feedback_fuzzer.cc | 3 +- webrtc/test/mock_voe_channel_proxy.h | 2 +- webrtc/tools/event_log_visualizer/analyzer.cc | 3 +- webrtc/video/video_send_stream.cc | 6 +- webrtc/voice_engine/channel.cc | 15 +- webrtc/voice_engine/channel.h | 2 +- webrtc/voice_engine/channel_proxy.cc | 4 +- webrtc/voice_engine/channel_proxy.h | 2 +- webrtc/voice_engine/voe_codec_impl.cc | 3 +- 28 files changed, 128 insertions(+), 248 deletions(-) diff --git a/webrtc/audio/audio_send_stream.cc b/webrtc/audio/audio_send_stream.cc index 840ae68d06..b031762b4c 100644 --- a/webrtc/audio/audio_send_stream.cc +++ b/webrtc/audio/audio_send_stream.cc @@ -229,8 +229,7 @@ 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 probing_interval_ms) { + int64_t rtt) { RTC_DCHECK_GE(bitrate_bps, static_cast(config_.min_bitrate_bps)); // The bitrate allocator might allocate an higher than max configured bitrate @@ -239,7 +238,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, probing_interval_ms); + channel_proxy_->SetBitrate(bitrate_bps); // 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 88ecbfc9fb..af0513ccf7 100644 --- a/webrtc/audio/audio_send_stream.h +++ b/webrtc/audio/audio_send_stream.h @@ -54,8 +54,7 @@ class AudioSendStream final : public webrtc::AudioSendStream, // Implements BitrateAllocatorObserver. uint32_t OnBitrateUpdated(uint32_t bitrate_bps, uint8_t fraction_loss, - int64_t rtt, - int64_t probing_interval_ms) override; + int64_t rtt) 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 a296513d9d..dbc49662e6 100644 --- a/webrtc/audio/audio_send_stream_unittest.cc +++ b/webrtc/audio/audio_send_stream_unittest.cc @@ -105,8 +105,6 @@ 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_; } @@ -402,27 +400,5 @@ 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 70763f3e8d..2c5ba6d76c 100644 --- a/webrtc/call/bitrate_allocator.cc +++ b/webrtc/call/bitrate_allocator.cc @@ -65,15 +65,13 @@ BitrateAllocator::~BitrateAllocator() { void BitrateAllocator::OnNetworkChanged(uint32_t target_bitrate_bps, uint8_t fraction_loss, - int64_t rtt, - int64_t probing_interval_ms) { + int64_t rtt) { 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(); @@ -87,8 +85,7 @@ 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_, - last_probing_interval_ms_); + allocated_bitrate, last_fraction_loss_, last_rtt_); if (allocated_bitrate == 0 && config.allocated_bitrate_bps > 0) { if (target_bitrate_bps > 0) @@ -144,8 +141,7 @@ 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_, - last_probing_interval_ms_); + allocated_bitrate, last_fraction_loss_, last_rtt_); config.allocated_bitrate_bps = allocated_bitrate; if (allocated_bitrate > 0) config.media_ratio = MediaRatio(allocated_bitrate, protection_bitrate); @@ -155,8 +151,7 @@ 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_, - last_probing_interval_ms_); + observer->OnBitrateUpdated(0, last_fraction_loss_, last_rtt_); } UpdateAllocationLimits(); } diff --git a/webrtc/call/bitrate_allocator.h b/webrtc/call/bitrate_allocator.h index cd3b2e163b..a5ed26c71b 100644 --- a/webrtc/call/bitrate_allocator.h +++ b/webrtc/call/bitrate_allocator.h @@ -33,9 +33,7 @@ class BitrateAllocatorObserver { // implementation, as bitrate in bps. virtual uint32_t OnBitrateUpdated(uint32_t bitrate_bps, uint8_t fraction_loss, - int64_t rtt, - int64_t probing_interval_ms) = 0; - + int64_t rtt) = 0; protected: virtual ~BitrateAllocatorObserver() {} }; @@ -63,8 +61,7 @@ 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 probing_interval_ms); + int64_t rtt); // Set the start and max send bitrate used by the bandwidth management. // @@ -160,7 +157,6 @@ 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 751ec93657..5d0395d82c 100644 --- a/webrtc/call/bitrate_allocator_unittest.cc +++ b/webrtc/call/bitrate_allocator_unittest.cc @@ -34,7 +34,6 @@ 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) { @@ -43,29 +42,22 @@ class TestBitrateObserver : public BitrateAllocatorObserver { uint32_t OnBitrateUpdated(uint32_t bitrate_bps, uint8_t fraction_loss, - int64_t rtt, - int64_t probing_interval_ms) override { + int64_t rtt) 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, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(300000u, 0, 0); } ~BitrateAllocatorTest() {} @@ -83,12 +75,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, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(200000, 0, 0); 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, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(4000000, 0, 0); EXPECT_EQ(3000000, allocator_->GetStartBitrate(&bitrate_observer)); // Expect |max_padding_bitrate_bps| to change to 0 if the observer is updated. @@ -104,7 +96,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, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(1500000, 0, 0); EXPECT_EQ(1500000u, bitrate_observer.last_bitrate_bps_); } @@ -122,7 +114,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, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(200000, 0, 50); 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_); @@ -131,7 +123,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, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(500000, 0, 50); const uint32_t kBitrateToShare = 500000 - 200000 - 100000; EXPECT_EQ(100000u + kBitrateToShare / 2, bitrate_observer_1.last_bitrate_bps_); @@ -140,14 +132,14 @@ TEST_F(BitrateAllocatorTest, TwoBitrateObserversOneRtcpObserver) { // Limited by 2x max bitrates since we leave room for FEC and // retransmissions. - allocator_->OnNetworkChanged(1500000, 0, 50, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(1500000, 0, 50); 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, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(0, 0, 50); EXPECT_EQ(0u, bitrate_observer_1.last_bitrate_bps_); EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_bps_); } @@ -169,7 +161,7 @@ class BitrateAllocatorTestNoEnforceMin : public ::testing::Test { protected: BitrateAllocatorTestNoEnforceMin() : allocator_(new BitrateAllocator(&limit_observer_)) { - allocator_->OnNetworkChanged(300000u, 0, 0, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(300000u, 0, 0); } ~BitrateAllocatorTestNoEnforceMin() {} @@ -188,11 +180,11 @@ TEST_F(BitrateAllocatorTestNoEnforceMin, OneBitrateObserver) { EXPECT_EQ(300000, allocator_->GetStartBitrate(&bitrate_observer_1)); // High BWE. - allocator_->OnNetworkChanged(150000, 0, 0, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(150000, 0, 0); EXPECT_EQ(150000u, bitrate_observer_1.last_bitrate_bps_); // Low BWE. - allocator_->OnNetworkChanged(10000, 0, 0, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(10000, 0, 0); EXPECT_EQ(0u, bitrate_observer_1.last_bitrate_bps_); EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(0, 0)); @@ -218,7 +210,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, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(690000, 0, 0); // 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; @@ -230,27 +222,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, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(300000, 0, 0); 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, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(500000, 0, 0); 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, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(10000, 0, 0); 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, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(0, 0, 0); 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_); @@ -270,38 +262,34 @@ TEST_F(BitrateAllocatorTestNoEnforceMin, OneBitrateObserverWithPacketLoss) { EXPECT_EQ(300000, allocator_->GetStartBitrate(&bitrate_observer)); // High BWE. - allocator_->OnNetworkChanged(150000, 0, 0, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(150000, 0, 0); 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, - kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(200000, 0, fraction_loss); 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, - kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(139000, 0, fraction_loss); EXPECT_EQ(0u, bitrate_observer.last_bitrate_bps_); // Verify the hysteresis is added for the protection. - allocator_->OnNetworkChanged(150000, 0, fraction_loss, - kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(150000, 0, fraction_loss); EXPECT_EQ(0u, bitrate_observer.last_bitrate_bps_); // Just enough to enable video again. - allocator_->OnNetworkChanged(168000, 0, fraction_loss, - kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(168000, 0, fraction_loss); 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, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(140000, 0, 0); EXPECT_EQ(140000u, bitrate_observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(139000, 0, 0, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(139000, 0, 0); EXPECT_EQ(139000u, bitrate_observer.last_bitrate_bps_); EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(0, 0)); @@ -320,37 +308,37 @@ TEST_F(BitrateAllocatorTestNoEnforceMin, TwoBitrateObserverWithPacketLoss) { // Enough bitrate for both. bitrate_observer_2.SetBitrateProtectionRatio(0.5); - allocator_->OnNetworkChanged(300000, 0, 0, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(300000, 0, 0); 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, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(330000, 0, 0); EXPECT_EQ(330000u, bitrate_observer_1.last_bitrate_bps_); EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_bps_); - allocator_->OnNetworkChanged(100000, 0, 0, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(100000, 0, 0); EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_bps_); EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_bps_); - allocator_->OnNetworkChanged(99999, 0, 0, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(99999, 0, 0); EXPECT_EQ(0u, bitrate_observer_1.last_bitrate_bps_); EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_bps_); - allocator_->OnNetworkChanged(119000, 0, 0, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(119000, 0, 0); EXPECT_EQ(0u, bitrate_observer_1.last_bitrate_bps_); EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_bps_); - allocator_->OnNetworkChanged(120000, 0, 0, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(120000, 0, 0); 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, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(429000, 0, 0); EXPECT_EQ(400000u, bitrate_observer_1.last_bitrate_bps_); EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_bps_); - allocator_->OnNetworkChanged(430000, 0, 0, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(430000, 0, 0); EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_bps_); EXPECT_EQ(330000u, bitrate_observer_2.last_bitrate_bps_); @@ -377,7 +365,7 @@ TEST_F(BitrateAllocatorTest, ThreeBitrateObserversLowBweEnforceMin) { // Low BWE. Verify that all observers still get their respective min // bitrate. - allocator_->OnNetworkChanged(1000, 0, 0, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(1000, 0, 0); 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. @@ -395,7 +383,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, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(0, 0, 0); EXPECT_EQ(0u, bitrate_observer_1.last_bitrate_bps_); @@ -411,7 +399,7 @@ TEST_F(BitrateAllocatorTest, AddObserverWhileNetworkDown) { EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_bps_); // Set network back up. - allocator_->OnNetworkChanged(1500000, 0, 50, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(1500000, 0, 50); EXPECT_EQ(750000u, bitrate_observer_1.last_bitrate_bps_); EXPECT_EQ(750000u, bitrate_observer_2.last_bitrate_bps_); } @@ -426,31 +414,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, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(36000, 0, 50); EXPECT_EQ(6000u, enforced_observer.last_bitrate_bps_); EXPECT_EQ(30000u, not_enforced_observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(35000, 0, 50, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(35000, 0, 50); EXPECT_EQ(30000u, enforced_observer.last_bitrate_bps_); EXPECT_EQ(0u, not_enforced_observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(5000, 0, 50, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(5000, 0, 50); EXPECT_EQ(6000u, enforced_observer.last_bitrate_bps_); EXPECT_EQ(0u, not_enforced_observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(36000, 0, 50, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(36000, 0, 50); EXPECT_EQ(30000u, enforced_observer.last_bitrate_bps_); EXPECT_EQ(0u, not_enforced_observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(55000, 0, 50, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(55000, 0, 50); EXPECT_EQ(30000u, enforced_observer.last_bitrate_bps_); EXPECT_EQ(0u, not_enforced_observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(56000, 0, 50, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(56000, 0, 50); EXPECT_EQ(6000u, enforced_observer.last_bitrate_bps_); EXPECT_EQ(50000u, not_enforced_observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(56000, 0, 50, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(56000, 0, 50); EXPECT_EQ(16000u, enforced_observer.last_bitrate_bps_); EXPECT_EQ(40000u, not_enforced_observer.last_bitrate_bps_); @@ -463,22 +451,22 @@ TEST_F(BitrateAllocatorTest, AvoidToggleAbsolute) { allocator_->AddObserver(&observer, 30000, 300000, 0, false); EXPECT_EQ(300000, allocator_->GetStartBitrate(&observer)); - allocator_->OnNetworkChanged(30000, 0, 50, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(30000, 0, 50); EXPECT_EQ(30000u, observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(20000, 0, 50, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(20000, 0, 50); EXPECT_EQ(0u, observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(30000, 0, 50, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(30000, 0, 50); EXPECT_EQ(0u, observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(49000, 0, 50, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(49000, 0, 50); EXPECT_EQ(0u, observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(50000, 0, 50, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(50000, 0, 50); EXPECT_EQ(50000u, observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(30000, 0, 50, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(30000, 0, 50); EXPECT_EQ(30000u, observer.last_bitrate_bps_); allocator_->RemoveObserver(&observer); @@ -489,36 +477,25 @@ TEST_F(BitrateAllocatorTest, AvoidTogglePercent) { allocator_->AddObserver(&observer, 300000, 600000, 0, false); EXPECT_EQ(300000, allocator_->GetStartBitrate(&observer)); - allocator_->OnNetworkChanged(300000, 0, 50, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(300000, 0, 50); EXPECT_EQ(300000u, observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(200000, 0, 50, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(200000, 0, 50); EXPECT_EQ(0u, observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(300000, 0, 50, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(300000, 0, 50); EXPECT_EQ(0u, observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(329000, 0, 50, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(329000, 0, 50); EXPECT_EQ(0u, observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(330000, 0, 50, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(330000, 0, 50); EXPECT_EQ(330000u, observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(300000, 0, 50, kDefaultProbingIntervalMs); + allocator_->OnNetworkChanged(300000, 0, 50); 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 afcf546566..d2bc153d32 100644 --- a/webrtc/call/call.cc +++ b/webrtc/call/call.cc @@ -120,10 +120,8 @@ 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, - int64_t probing_interval_ms) override; + void OnNetworkChanged(uint32_t bitrate_bps, uint8_t fraction_loss, + int64_t rtt_ms) override; // Implements BitrateAllocator::LimitObserver. void OnAllocationLimitsChanged(uint32_t min_send_bitrate_bps, @@ -879,23 +877,19 @@ 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, - int64_t probing_interval_ms) { +void Call::OnNetworkChanged(uint32_t target_bitrate_bps, uint8_t fraction_loss, + int64_t rtt_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, probing_interval_ms] { - OnNetworkChanged(target_bitrate_bps, fraction_loss, rtt_ms, - probing_interval_ms); - }); + worker_queue_.PostTask([this, target_bitrate_bps, fraction_loss, rtt_ms] { + OnNetworkChanged(target_bitrate_bps, fraction_loss, rtt_ms); + }); return; } RTC_DCHECK_RUN_ON(&worker_queue_); bitrate_allocator_->OnNetworkChanged(target_bitrate_bps, fraction_loss, - rtt_ms, probing_interval_ms); + rtt_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 86ad275d2a..accc1411f9 100644 --- a/webrtc/modules/congestion_controller/congestion_controller.cc +++ b/webrtc/modules/congestion_controller/congestion_controller.cc @@ -342,9 +342,7 @@ void CongestionController::MaybeTriggerOnNetworkChanged() { bitrate_bps = IsNetworkDown() || IsSendQueueFull() ? 0 : bitrate_bps; if (HasNetworkParametersToReportChanged(bitrate_bps, fraction_loss, rtt)) { - observer_->OnNetworkChanged( - bitrate_bps, fraction_loss, rtt, - transport_feedback_adapter_.GetProbingIntervalMs()); + observer_->OnNetworkChanged(bitrate_bps, fraction_loss, rtt); 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 a72d80bbb3..42bb5267b2 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,33 +113,32 @@ 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); @@ -150,7 +149,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. @@ -170,12 +169,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)); @@ -190,16 +189,5 @@ 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 04dadc489c..78bc7f0e6e 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe.cc @@ -180,8 +180,7 @@ DelayBasedBwe::DelayBasedBwe(Clock* clock) trendline_window_size_(kDefaultTrendlineWindowSize), trendline_smoothing_coeff_(kDefaultTrendlineSmoothingCoeff), trendline_threshold_gain_(kDefaultTrendlineThresholdGain), - in_trendline_experiment_(TrendlineFilterExperimentIsEnabled()), - probing_interval_estimator_(&rate_control_) { + in_trendline_experiment_(TrendlineFilterExperimentIsEnabled()) { if (in_trendline_experiment_) { ReadTrendlineFilterExperimentParameters(&trendline_window_size_, &trendline_smoothing_coeff_, @@ -331,8 +330,4 @@ 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 c5a599f334..0f1472e672 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe.h +++ b/webrtc/modules/congestion_controller/delay_based_bwe.h @@ -20,7 +20,6 @@ #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" @@ -52,7 +51,6 @@ 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 @@ -101,7 +99,6 @@ 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 c242bf4f27..3f26e592ae 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc @@ -116,15 +116,6 @@ 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 1eb5b3441b..8663489d54 100644 --- a/webrtc/modules/congestion_controller/include/congestion_controller.h +++ b/webrtc/modules/congestion_controller/include/congestion_controller.h @@ -48,8 +48,7 @@ class CongestionController : public CallStatsObserver, public Module { public: virtual void OnNetworkChanged(uint32_t bitrate_bps, uint8_t fraction_loss, // 0 - 255. - int64_t rtt_ms, - int64_t probing_interval_ms) = 0; + int64_t rtt_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 9fb422634d..128d64ea98 100644 --- a/webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h +++ b/webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h @@ -21,11 +21,10 @@ namespace test { class MockCongestionObserver : public CongestionController::Observer { public: - MOCK_METHOD4(OnNetworkChanged, + MOCK_METHOD3(OnNetworkChanged, void(uint32_t bitrate_bps, uint8_t fraction_loss, - int64_t rtt_ms, - int64_t probing_interval_ms)); + int64_t rtt_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 6e7a674ae8..ed1134737e 100644 --- a/webrtc/modules/congestion_controller/probing_interval_estimator.cc +++ b/webrtc/modules/congestion_controller/probing_interval_estimator.cc @@ -17,7 +17,6 @@ namespace webrtc { namespace { constexpr int kMinIntervalMs = 2000; -constexpr int kDefaultIntervalMs = 3000; constexpr int kMaxIntervalMs = 50000; } @@ -25,31 +24,27 @@ ProbingIntervalEstimator::ProbingIntervalEstimator( const AimdRateControl* aimd_rate_control) : ProbingIntervalEstimator(kMinIntervalMs, kMaxIntervalMs, - kDefaultIntervalMs, aimd_rate_control) {} ProbingIntervalEstimator::ProbingIntervalEstimator( - int64_t min_interval_ms, - int64_t max_interval_ms, - int64_t default_interval_ms, + int min_interval_ms, + int max_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) {} -int64_t ProbingIntervalEstimator::GetIntervalMs() const { +rtc::Optional 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 default_interval_ms_; + return rtc::Optional(); - return std::min( + return rtc::Optional(std::min( max_interval_ms_, - std::max(static_cast(1000 * (*bitrate_drop) / increase_rate), - min_interval_ms_)); + std::max(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 993e0da550..daa6b91a8c 100644 --- a/webrtc/modules/congestion_controller/probing_interval_estimator.h +++ b/webrtc/modules/congestion_controller/probing_interval_estimator.h @@ -21,16 +21,14 @@ class ProbingIntervalEstimator { public: explicit ProbingIntervalEstimator(const AimdRateControl* aimd_rate_control); - ProbingIntervalEstimator(int64_t min_interval_ms, - int64_t max_interval_ms, - int64_t default_interval_ms, + ProbingIntervalEstimator(int min_interval_ms, + int max_interval_ms, const AimdRateControl* aimd_rate_control); - int64_t GetIntervalMs() const; + rtc::Optional GetIntervalMs() const; private: - const int64_t min_interval_ms_; - const int64_t max_interval_ms_; - const int64_t default_interval_ms_; + const int min_interval_ms_; + const int max_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 bf10718860..051f3561d3 100644 --- a/webrtc/modules/congestion_controller/probing_interval_estimator_unittest.cc +++ b/webrtc/modules/congestion_controller/probing_interval_estimator_unittest.cc @@ -24,7 +24,6 @@ namespace { constexpr int kMinIntervalMs = 2000; constexpr int kMaxIntervalMs = 50000; -constexpr int kDefaultIntervalMs = 3000; struct ProbingIntervalEstimatorStates { std::unique_ptr probing_interval_estimator; @@ -35,13 +34,12 @@ ProbingIntervalEstimatorStates CreateProbingIntervalEstimatorStates() { ProbingIntervalEstimatorStates states; states.aimd_rate_control.reset(new MockAimdRateControl()); states.probing_interval_estimator.reset(new ProbingIntervalEstimator( - kMinIntervalMs, kMaxIntervalMs, kDefaultIntervalMs, - states.aimd_rate_control.get())); + kMinIntervalMs, kMaxIntervalMs, states.aimd_rate_control.get())); return states; } } // namespace -TEST(ProbingIntervalEstimatorTest, DefaultIntervalUntillWeHaveDrop) { +TEST(ProbingIntervalEstimatorTest, NoIntervalUntillWeHaveDrop) { auto states = CreateProbingIntervalEstimatorStates(); EXPECT_CALL(*states.aimd_rate_control, GetLastBitrateDecreaseBps()) @@ -51,9 +49,9 @@ TEST(ProbingIntervalEstimatorTest, DefaultIntervalUntillWeHaveDrop) { .WillOnce(Return(4000)) .WillOnce(Return(4000)); - EXPECT_EQ(kDefaultIntervalMs, + EXPECT_EQ(rtc::Optional(), states.probing_interval_estimator->GetIntervalMs()); - EXPECT_NE(kDefaultIntervalMs, + EXPECT_NE(rtc::Optional(), states.probing_interval_estimator->GetIntervalMs()); } @@ -63,7 +61,8 @@ TEST(ProbingIntervalEstimatorTest, CalcInterval) { .WillOnce(Return(rtc::Optional(20000))); EXPECT_CALL(*states.aimd_rate_control, GetNearMaxIncreaseRateBps()) .WillOnce(Return(5000)); - EXPECT_EQ(4000, states.probing_interval_estimator->GetIntervalMs()); + EXPECT_EQ(rtc::Optional(4000), + states.probing_interval_estimator->GetIntervalMs()); } TEST(ProbingIntervalEstimatorTest, IntervalDoesNotExceedMin) { @@ -72,7 +71,8 @@ TEST(ProbingIntervalEstimatorTest, IntervalDoesNotExceedMin) { .WillOnce(Return(rtc::Optional(1000))); EXPECT_CALL(*states.aimd_rate_control, GetNearMaxIncreaseRateBps()) .WillOnce(Return(5000)); - EXPECT_EQ(kMinIntervalMs, states.probing_interval_estimator->GetIntervalMs()); + EXPECT_EQ(rtc::Optional(kMinIntervalMs), + states.probing_interval_estimator->GetIntervalMs()); } TEST(ProbingIntervalEstimatorTest, IntervalDoesNotExceedMax) { @@ -81,6 +81,7 @@ TEST(ProbingIntervalEstimatorTest, IntervalDoesNotExceedMax) { .WillOnce(Return(rtc::Optional(50000))); EXPECT_CALL(*states.aimd_rate_control, GetNearMaxIncreaseRateBps()) .WillOnce(Return(100)); - EXPECT_EQ(kMaxIntervalMs, states.probing_interval_estimator->GetIntervalMs()); + EXPECT_EQ(rtc::Optional(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 7756fee365..46639067c2 100644 --- a/webrtc/modules/congestion_controller/transport_feedback_adapter.cc +++ b/webrtc/modules/congestion_controller/transport_feedback_adapter.cc @@ -73,11 +73,6 @@ 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 09363ec50c..6422736c5c 100644 --- a/webrtc/modules/congestion_controller/transport_feedback_adapter.h +++ b/webrtc/modules/congestion_controller/transport_feedback_adapter.h @@ -50,8 +50,6 @@ 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 6d5deab2cd..496af90c69 100644 --- a/webrtc/test/fuzzers/congestion_controller_feedback_fuzzer.cc +++ b/webrtc/test/fuzzers/congestion_controller_feedback_fuzzer.cc @@ -21,8 +21,7 @@ class NullBitrateObserver : public CongestionController::Observer, ~NullBitrateObserver() override {} void OnNetworkChanged(uint32_t bitrate_bps, uint8_t fraction_loss, - int64_t rtt_ms, - int64_t probing_interval_ms) override {} + int64_t rtt_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 4f5a0d2156..3676e89153 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_METHOD2(SetBitrate, void(int bitrate_bps, int64_t probing_interval_ms)); + MOCK_METHOD1(SetBitrate, void(int bitrate_bps)); // 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 485866c1a6..d9b772bcd6 100644 --- a/webrtc/tools/event_log_visualizer/analyzer.cc +++ b/webrtc/tools/event_log_visualizer/analyzer.cc @@ -451,8 +451,7 @@ class BitrateObserver : public CongestionController::Observer, void OnNetworkChanged(uint32_t bitrate_bps, uint8_t fraction_loss, - int64_t rtt_ms, - int64_t probing_interval_ms) override { + int64_t rtt_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 e6b418fe71..964475fd38 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -328,8 +328,7 @@ class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver, // Implements BitrateAllocatorObserver. uint32_t OnBitrateUpdated(uint32_t bitrate_bps, uint8_t fraction_loss, - int64_t rtt, - int64_t probing_interval_ms) override; + int64_t rtt) override; // Implements webrtc::VCMProtectionCallback. int ProtectionRequest(const FecProtectionParams* delta_params, @@ -1149,8 +1148,7 @@ void VideoSendStreamImpl::SignalNetworkState(NetworkState state) { uint32_t VideoSendStreamImpl::OnBitrateUpdated(uint32_t bitrate_bps, uint8_t fraction_loss, - int64_t rtt, - int64_t probing_interval_ms) { + int64_t rtt) { 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 d946676fd6..a7df1f2176 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, int64_t probing_interval_ms) { +void Channel::SetBitRate(int bitrate_bps) { WEBRTC_TRACE(kTraceInfo, kTraceVoice, VoEId(_instanceId, _channelId), "Channel::SetBitRate(bitrate_bps=%d)", bitrate_bps); audio_coding_->ModifyEncoder([&](std::unique_ptr* encoder) { @@ -1312,15 +1312,10 @@ void Channel::SetBitRate(int bitrate_bps, int64_t probing_interval_ms) { // We give smoothed bitrate allocation to audio network adaptor as // the uplink bandwidth. - // 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); + // TODO(michaelt) : Remove kDefaultBitrateSmoothingTimeConstantMs as soon as + // we pass the probing interval to this function. + constexpr int64_t kDefaultBitrateSmoothingTimeConstantMs = 20000; + bitrate_smoother_.SetTimeConstantMs(kDefaultBitrateSmoothingTimeConstantMs); 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 9b9d5c927f..84f9691ef2 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, int64_t probing_interval_ms); + void SetBitRate(int bitrate_bps); 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 04fa93f6ab..1d67f689a7 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, int64_t probing_interval_ms) { +void ChannelProxy::SetBitrate(int bitrate_bps) { // May be called on different threads and needs to be handled by the channel. - channel()->SetBitRate(bitrate_bps, probing_interval_ms); + channel()->SetBitRate(bitrate_bps); } void ChannelProxy::SetSink(std::unique_ptr sink) { diff --git a/webrtc/voice_engine/channel_proxy.h b/webrtc/voice_engine/channel_proxy.h index 6d5f7b0d28..6e0171b395 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, int64_t probing_interval_ms); + virtual void SetBitrate(int bitrate_bps); 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 b2fd08d599..8ac24da4e7 100644 --- a/webrtc/voice_engine/voe_codec_impl.cc +++ b/webrtc/voice_engine/voe_codec_impl.cc @@ -137,9 +137,8 @@ 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, kDefaultProbingIntervalMs); + bitrate_bps); return 0; }