From 101f250a30d3f5d0e157cf4569af9384a98f6696 Mon Sep 17 00:00:00 2001 From: mflodman Date: Thu, 9 Jun 2016 17:21:19 +0200 Subject: [PATCH] Implementing auto pausing of video streams. This CL implements auto pausing video streams per stream with logic to avoid toggling state too often. Also re-enabling tests disabled for Mac, with the assumption the new logic removes flakiness. BUG=webrtc:5868,webrtc:5407 R=pbos@webrtc.org, stefan@webrtc.org Review URL: https://codereview.webrtc.org/2035383002 . Cr-Commit-Position: refs/heads/master@{#13092} --- webrtc/call/bitrate_allocator.cc | 262 +++++++++++++++------- webrtc/call/bitrate_allocator.h | 47 ++-- webrtc/call/bitrate_allocator_unittest.cc | 235 +++++++++++++------ webrtc/call/call.cc | 5 + webrtc/call/rampup_tests.cc | 7 - webrtc/video/video_send_stream.cc | 4 - webrtc/video/video_send_stream_tests.cc | 2 +- webrtc/video/vie_encoder.cc | 21 +- webrtc/video/vie_encoder.h | 2 + 9 files changed, 394 insertions(+), 191 deletions(-) diff --git a/webrtc/call/bitrate_allocator.cc b/webrtc/call/bitrate_allocator.cc index 3672ef520c..e4b1ad49fa 100644 --- a/webrtc/call/bitrate_allocator.cc +++ b/webrtc/call/bitrate_allocator.cc @@ -24,30 +24,34 @@ namespace webrtc { const int kTransmissionMaxBitrateMultiplier = 2; const int kDefaultBitrateBps = 300000; +// Require a bitrate increase of max(10%, 20kbps) to resume paused streams. +const double kToggleFactor = 0.1; +const uint32_t kMinToggleBitrateBps = 20000; + BitrateAllocator::BitrateAllocator() : bitrate_observer_configs_(), - enforce_min_bitrate_(true), last_bitrate_bps_(kDefaultBitrateBps), last_non_zero_bitrate_bps_(kDefaultBitrateBps), last_fraction_loss_(0), last_rtt_(0) {} -uint32_t BitrateAllocator::OnNetworkChanged(uint32_t bitrate, +uint32_t BitrateAllocator::OnNetworkChanged(uint32_t target_bitrate_bps, uint8_t fraction_loss, int64_t rtt) { rtc::CritScope lock(&crit_sect_); - last_bitrate_bps_ = bitrate; + last_bitrate_bps_ = target_bitrate_bps; last_non_zero_bitrate_bps_ = - bitrate > 0 ? bitrate : 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; uint32_t allocated_bitrate_bps = 0; - ObserverAllocation allocation = AllocateBitrates(bitrate); + ObserverAllocation allocation = AllocateBitrates(target_bitrate_bps); for (const auto& kv : allocation) { kv.first->OnBitrateUpdated(kv.second, last_fraction_loss_, last_rtt_); allocated_bitrate_bps += kv.second; } + last_allocation_ = allocation; return allocated_bitrate_bps; } @@ -56,45 +60,33 @@ int BitrateAllocator::AddObserver(BitrateAllocatorObserver* observer, uint32_t max_bitrate_bps, bool enforce_min_bitrate) { rtc::CritScope lock(&crit_sect_); - // TODO(mflodman): Enforce this per observer. - EnforceMinBitrate(enforce_min_bitrate); - auto it = FindObserverConfig(observer); - // Allow the max bitrate to be exceeded for FEC and retransmissions. - // TODO(holmer): We have to get rid of this hack as it makes it difficult to - // properly allocate bitrate. The allocator should instead distribute any - // extra bitrate after all streams have maxed out. - max_bitrate_bps *= kTransmissionMaxBitrateMultiplier; + // Update settings if the observer already exists, create a new one otherwise. if (it != bitrate_observer_configs_.end()) { - // Update current configuration. it->min_bitrate_bps = min_bitrate_bps; it->max_bitrate_bps = max_bitrate_bps; + it->enforce_min_bitrate = enforce_min_bitrate; } else { - // Add new settings. bitrate_observer_configs_.push_back(ObserverConfig( observer, min_bitrate_bps, max_bitrate_bps, enforce_min_bitrate)); } - int new_observer_bitrate_bps = 0; - if (last_bitrate_bps_ > 0) { // We have a bitrate to allocate. - ObserverAllocation allocation = AllocateBitrates(last_bitrate_bps_); - for (auto& kv : allocation) { - // Update all observers with the new allocation. + ObserverAllocation allocation; + if (last_bitrate_bps_ > 0) { + // Calculate a new allocation and update all observers. + allocation = AllocateBitrates(last_bitrate_bps_); + for (const auto& kv : allocation) kv.first->OnBitrateUpdated(kv.second, last_fraction_loss_, last_rtt_); - if (kv.first == observer) - new_observer_bitrate_bps = kv.second; - } } else { // Currently, an encoder is not allowed to produce frames. // But we still have to return the initial config bitrate + let the // observer know that it can not produce frames. - ObserverAllocation allocation = - AllocateBitrates(last_non_zero_bitrate_bps_); + allocation = AllocateBitrates(last_non_zero_bitrate_bps_); observer->OnBitrateUpdated(0, last_fraction_loss_, last_rtt_); - new_observer_bitrate_bps = allocation[observer]; } - return new_observer_bitrate_bps; + last_allocation_ = allocation; + return allocation[observer]; } void BitrateAllocator::RemoveObserver(BitrateAllocatorObserver* observer) { @@ -105,10 +97,6 @@ void BitrateAllocator::RemoveObserver(BitrateAllocatorObserver* observer) { } } -void BitrateAllocator::EnforceMinBitrate(bool enforce_min_bitrate) { - enforce_min_bitrate_ = enforce_min_bitrate; -} - BitrateAllocator::ObserverConfigList::iterator BitrateAllocator::FindObserverConfig( const BitrateAllocatorObserver* observer) { @@ -129,56 +117,28 @@ BitrateAllocator::ObserverAllocation BitrateAllocator::AllocateBitrates( return ZeroRateAllocation(); uint32_t sum_min_bitrates = 0; - for (const auto& observer_config : bitrate_observer_configs_) + uint32_t sum_max_bitrates = 0; + for (const auto& observer_config : bitrate_observer_configs_) { sum_min_bitrates += observer_config.min_bitrate_bps; - if (bitrate <= sum_min_bitrates) + sum_max_bitrates += observer_config.max_bitrate_bps; + } + + // Not enough for all observers to get an allocation, allocate according to: + // enforced min bitrate -> allocated bitrate previous round -> restart paused + // streams. + if (!EnoughBitrateForAllObservers(bitrate, sum_min_bitrates)) return LowRateAllocation(bitrate); - return NormalRateAllocation(bitrate, sum_min_bitrates); -} + // All observers will get their min bitrate plus an even share of the rest. + if (bitrate <= sum_max_bitrates) + return NormalRateAllocation(bitrate, sum_min_bitrates); -BitrateAllocator::ObserverAllocation BitrateAllocator::NormalRateAllocation( - uint32_t bitrate, - uint32_t sum_min_bitrates) { - uint32_t num_remaining_observers = - static_cast(bitrate_observer_configs_.size()); - RTC_DCHECK_GT(num_remaining_observers, 0u); - - uint32_t bitrate_per_observer = - (bitrate - sum_min_bitrates) / num_remaining_observers; - // Use map to sort list based on max bitrate. - ObserverSortingMap list_max_bitrates; - for (const auto& config : bitrate_observer_configs_) { - list_max_bitrates.insert(std::pair( - config.max_bitrate_bps, &config)); - } - - ObserverAllocation allocation; - ObserverSortingMap::iterator max_it = list_max_bitrates.begin(); - while (max_it != list_max_bitrates.end()) { - num_remaining_observers--; - uint32_t observer_allowance = - max_it->second->min_bitrate_bps + bitrate_per_observer; - if (max_it->first < observer_allowance) { - // We have more than enough for this observer. - // Carry the remainder forward. - uint32_t remainder = observer_allowance - max_it->first; - if (num_remaining_observers != 0) - bitrate_per_observer += remainder / num_remaining_observers; - allocation[max_it->second->observer] = max_it->first; - } else { - allocation[max_it->second->observer] = observer_allowance; - } - list_max_bitrates.erase(max_it); - // Prepare next iteration. - max_it = list_max_bitrates.begin(); - } - return allocation; + // All observers will get up to kTransmissionMaxBitrateMultiplier x max. + return MaxRateAllocation(bitrate, sum_max_bitrates); } BitrateAllocator::ObserverAllocation BitrateAllocator::ZeroRateAllocation() { ObserverAllocation allocation; - // Zero bitrate to all observers. for (const auto& observer_config : bitrate_observer_configs_) allocation[observer_config.observer] = 0; return allocation; @@ -187,21 +147,153 @@ BitrateAllocator::ObserverAllocation BitrateAllocator::ZeroRateAllocation() { BitrateAllocator::ObserverAllocation BitrateAllocator::LowRateAllocation( uint32_t bitrate) { ObserverAllocation allocation; - if (enforce_min_bitrate_) { - // Min bitrate to all observers. - for (const auto& observer_config : bitrate_observer_configs_) - allocation[observer_config.observer] = observer_config.min_bitrate_bps; - } else { - // Allocate up to |min_bitrate_bps| to one observer at a time, until - // |bitrate| is depleted. - uint32_t remainder = bitrate; + + // Start by allocating bitrate to observers enforcing a min bitrate, hence + // remaining_bitrate might turn negative. + int64_t remaining_bitrate = bitrate; + for (const auto& observer_config : bitrate_observer_configs_) { + int32_t allocated_bitrate = 0; + if (observer_config.enforce_min_bitrate) + allocated_bitrate = observer_config.min_bitrate_bps; + + allocation[observer_config.observer] = allocated_bitrate; + remaining_bitrate -= allocated_bitrate; + } + + // Allocate bitrate to all previously active streams. + if (remaining_bitrate > 0) { for (const auto& observer_config : bitrate_observer_configs_) { - uint32_t allocated_bitrate = - std::min(remainder, observer_config.min_bitrate_bps); - allocation[observer_config.observer] = allocated_bitrate; - remainder -= allocated_bitrate; + if (observer_config.enforce_min_bitrate || + LastAllocatedBitrate(observer_config) == 0) + continue; + + if (remaining_bitrate >= observer_config.min_bitrate_bps) { + allocation[observer_config.observer] = observer_config.min_bitrate_bps; + remaining_bitrate -= observer_config.min_bitrate_bps; + } } } + + // Allocate bitrate to previously paused streams. + if (remaining_bitrate > 0) { + for (const auto& observer_config : bitrate_observer_configs_) { + if (LastAllocatedBitrate(observer_config) != 0) + continue; + + // Add a hysteresis to avoid toggling. + uint32_t required_bitrate = MinBitrateWithHysteresis(observer_config); + if (remaining_bitrate >= required_bitrate) { + allocation[observer_config.observer] = required_bitrate; + remaining_bitrate -= required_bitrate; + } + } + } + + // Split a possible remainder evenly on all streams with an allocation. + if (remaining_bitrate > 0) + DistributeBitrateEvenly(remaining_bitrate, false, 1, &allocation); + + RTC_DCHECK_EQ(allocation.size(), bitrate_observer_configs_.size()); return allocation; } + +BitrateAllocator::ObserverAllocation BitrateAllocator::NormalRateAllocation( + uint32_t bitrate, + uint32_t sum_min_bitrates) { + + ObserverAllocation allocation; + for (const auto& observer_config : bitrate_observer_configs_) + allocation[observer_config.observer] = observer_config.min_bitrate_bps; + + bitrate -= sum_min_bitrates; + if (bitrate > 0) + DistributeBitrateEvenly(bitrate, true, 1, &allocation); + + return allocation; +} + +BitrateAllocator::ObserverAllocation BitrateAllocator::MaxRateAllocation( + uint32_t bitrate, uint32_t sum_max_bitrates) { + ObserverAllocation allocation; + + for (const auto& observer_config : bitrate_observer_configs_) { + allocation[observer_config.observer] = observer_config.max_bitrate_bps; + bitrate -= observer_config.max_bitrate_bps; + } + DistributeBitrateEvenly(bitrate, true, kTransmissionMaxBitrateMultiplier, + &allocation); + return allocation; +} + +uint32_t BitrateAllocator::LastAllocatedBitrate( + const ObserverConfig& observer_config) { + + const auto& it = last_allocation_.find(observer_config.observer); + if (it != last_allocation_.end()) + return it->second; + + // Return the configured minimum bitrate for newly added observers, to avoid + // requiring an extra high bitrate for the observer to get an allocated + // bitrate. + return observer_config.min_bitrate_bps; +} + +uint32_t BitrateAllocator::MinBitrateWithHysteresis( + const ObserverConfig& observer_config) { + uint32_t min_bitrate = observer_config.min_bitrate_bps; + if (LastAllocatedBitrate(observer_config) == 0) { + min_bitrate += std::max(static_cast(kToggleFactor * min_bitrate), + kMinToggleBitrateBps); + } + return min_bitrate; +} + +void BitrateAllocator::DistributeBitrateEvenly(uint32_t bitrate, + bool include_zero_allocations, + int max_multiplier, + ObserverAllocation* allocation) { + RTC_DCHECK_EQ(allocation->size(), bitrate_observer_configs_.size()); + + ObserverSortingMap list_max_bitrates; + for (const auto& observer_config : bitrate_observer_configs_) { + if (include_zero_allocations || + allocation->at(observer_config.observer) != 0) { + list_max_bitrates.insert(std::pair( + observer_config.max_bitrate_bps, &observer_config)); + } + } + auto it = list_max_bitrates.begin(); + while (it != list_max_bitrates.end()) { + RTC_DCHECK_GT(bitrate, 0u); + uint32_t extra_allocation = + bitrate / static_cast(list_max_bitrates.size()); + uint32_t total_allocation = + extra_allocation + allocation->at(it->second->observer); + bitrate -= extra_allocation; + if (total_allocation > max_multiplier * it->first) { + // There is more than we can fit for this observer, carry over to the + // remaining observers. + bitrate += total_allocation - max_multiplier * it->first; + total_allocation = max_multiplier * it->first; + } + // Finally, update the allocation for this observer. + allocation->at(it->second->observer) = total_allocation; + it = list_max_bitrates.erase(it); + } +} + +bool BitrateAllocator::EnoughBitrateForAllObservers(uint32_t bitrate, + uint32_t sum_min_bitrates) { + if (bitrate < sum_min_bitrates) + return false; + + uint32_t extra_bitrate_per_observer = (bitrate - sum_min_bitrates) / + static_cast(bitrate_observer_configs_.size()); + for (const auto& observer_config : bitrate_observer_configs_) { + if (observer_config.min_bitrate_bps + extra_bitrate_per_observer < + MinBitrateWithHysteresis(observer_config)) + return false; + } + return true; +} } // namespace webrtc diff --git a/webrtc/call/bitrate_allocator.h b/webrtc/call/bitrate_allocator.h index bf26a6ae9b..fd9fe7172b 100644 --- a/webrtc/call/bitrate_allocator.h +++ b/webrtc/call/bitrate_allocator.h @@ -42,9 +42,9 @@ class BitrateAllocator { BitrateAllocator(); // Allocate target_bitrate across the registered BitrateAllocatorObservers. - // Returns actual bitrate allocated (might be higher than target_bitrate if - // for instance EnforceMinBitrate() is enabled. - uint32_t OnNetworkChanged(uint32_t target_bitrate, + // Returns actual bitrate allocated, which might be higher than target_bitrate + // if for instance EnforceMinBitrate() is enabled. + uint32_t OnNetworkChanged(uint32_t target_bitrate_bps, uint8_t fraction_loss, int64_t rtt); @@ -66,9 +66,12 @@ class BitrateAllocator { uint32_t max_bitrate_bps, bool enforce_min_bitrate); + // Removes a previously added observer, but will not trigger a new bitrate + // allocation. void RemoveObserver(BitrateAllocatorObserver* observer); private: + // Note: All bitrates for member variables and methods are in bps. struct ObserverConfig { ObserverConfig(BitrateAllocatorObserver* observer, uint32_t min_bitrate_bps, @@ -84,14 +87,6 @@ class BitrateAllocator { bool enforce_min_bitrate; }; - // This method controls the behavior when the available bitrate is lower than - // the minimum bitrate, or the sum of minimum bitrates. - // When true, the bitrate will never be set lower than the minimum bitrate(s). - // When false, the bitrate observers will be allocated rates up to their - // respective minimum bitrate, satisfying one observer after the other. - void EnforceMinBitrate(bool enforce_min_bitrate) - EXCLUSIVE_LOCKS_REQUIRED(crit_sect_); - typedef std::list ObserverConfigList; ObserverConfigList::iterator FindObserverConfig( const BitrateAllocatorObserver* observer) @@ -102,22 +97,44 @@ class BitrateAllocator { ObserverAllocation AllocateBitrates(uint32_t bitrate) EXCLUSIVE_LOCKS_REQUIRED(crit_sect_); - ObserverAllocation NormalRateAllocation(uint32_t bitrate, - uint32_t sum_min_bitrates) - EXCLUSIVE_LOCKS_REQUIRED(crit_sect_); ObserverAllocation ZeroRateAllocation() EXCLUSIVE_LOCKS_REQUIRED(crit_sect_); ObserverAllocation LowRateAllocation(uint32_t bitrate) EXCLUSIVE_LOCKS_REQUIRED(crit_sect_); + ObserverAllocation NormalRateAllocation(uint32_t bitrate, + uint32_t sum_min_bitrates) + EXCLUSIVE_LOCKS_REQUIRED(crit_sect_); + ObserverAllocation MaxRateAllocation(uint32_t bitrate, + uint32_t sum_max_bitrates) + EXCLUSIVE_LOCKS_REQUIRED(crit_sect_); + + uint32_t LastAllocatedBitrate(const ObserverConfig& observer_config) + EXCLUSIVE_LOCKS_REQUIRED(crit_sect_); + // The minimum bitrate required by this observer, including enable-hysteresis + // if the observer is in a paused state. + uint32_t MinBitrateWithHysteresis(const ObserverConfig& observer_config) + EXCLUSIVE_LOCKS_REQUIRED(crit_sect_); + // Splits |bitrate| evenly to observers already in |allocation|. + // |include_zero_allocations| decides if zero allocations should be part of + // the distribution or not. The allowed max bitrate is |max_multiplier| x + // observer max bitrate. + void DistributeBitrateEvenly(uint32_t bitrate, + bool include_zero_allocations, + int max_multiplier, + ObserverAllocation* allocation) + EXCLUSIVE_LOCKS_REQUIRED(crit_sect_); + bool EnoughBitrateForAllObservers(uint32_t bitrate, uint32_t sum_min_bitrates) + EXCLUSIVE_LOCKS_REQUIRED(crit_sect_); + rtc::CriticalSection crit_sect_; // Stored in a list to keep track of the insertion order. ObserverConfigList bitrate_observer_configs_ GUARDED_BY(crit_sect_); - bool enforce_min_bitrate_ GUARDED_BY(crit_sect_); uint32_t last_bitrate_bps_ GUARDED_BY(crit_sect_); uint32_t last_non_zero_bitrate_bps_ GUARDED_BY(crit_sect_); uint8_t last_fraction_loss_ GUARDED_BY(crit_sect_); int64_t last_rtt_ GUARDED_BY(crit_sect_); + ObserverAllocation last_allocation_ GUARDED_BY(crit_sect_); }; } // namespace webrtc #endif // WEBRTC_CALL_BITRATE_ALLOCATOR_H_ diff --git a/webrtc/call/bitrate_allocator_unittest.cc b/webrtc/call/bitrate_allocator_unittest.cc index 4017645c1d..9cb5f6cf99 100644 --- a/webrtc/call/bitrate_allocator_unittest.cc +++ b/webrtc/call/bitrate_allocator_unittest.cc @@ -21,18 +21,18 @@ namespace webrtc { class TestBitrateObserver : public BitrateAllocatorObserver { public: TestBitrateObserver() - : last_bitrate_(0), last_fraction_loss_(0), last_rtt_(0) {} + : last_bitrate_bps_(0), last_fraction_loss_(0), last_rtt_ms_(0) {} - virtual void OnBitrateUpdated(uint32_t bitrate, - uint8_t fraction_loss, - int64_t rtt) { - last_bitrate_ = bitrate; + void OnBitrateUpdated(uint32_t bitrate_bps, + uint8_t fraction_loss, + int64_t rtt) override { + last_bitrate_bps_ = bitrate_bps; last_fraction_loss_ = fraction_loss; - last_rtt_ = rtt; + last_rtt_ms_ = rtt; } - uint32_t last_bitrate_; + uint32_t last_bitrate_bps_; uint8_t last_fraction_loss_; - int64_t last_rtt_; + int64_t last_rtt_ms_; }; class BitrateAllocatorTest : public ::testing::Test { @@ -51,12 +51,12 @@ TEST_F(BitrateAllocatorTest, UpdatingBitrateObserver) { allocator_->AddObserver(&bitrate_observer, 100000, 1500000, true); EXPECT_EQ(300000, start_bitrate); allocator_->OnNetworkChanged(200000, 0, 0); - EXPECT_EQ(200000u, bitrate_observer.last_bitrate_); + EXPECT_EQ(200000u, bitrate_observer.last_bitrate_bps_); // 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); - EXPECT_EQ(3000000u, bitrate_observer.last_bitrate_); + EXPECT_EQ(3000000u, bitrate_observer.last_bitrate_bps_); start_bitrate = allocator_->AddObserver(&bitrate_observer, 100000, 4000000, true); EXPECT_EQ(4000000, start_bitrate); @@ -64,9 +64,9 @@ TEST_F(BitrateAllocatorTest, UpdatingBitrateObserver) { start_bitrate = allocator_->AddObserver(&bitrate_observer, 100000, 1500000, true); EXPECT_EQ(3000000, start_bitrate); - EXPECT_EQ(3000000u, bitrate_observer.last_bitrate_); + EXPECT_EQ(3000000u, bitrate_observer.last_bitrate_bps_); allocator_->OnNetworkChanged(1500000, 0, 0); - EXPECT_EQ(1500000u, bitrate_observer.last_bitrate_); + EXPECT_EQ(1500000u, bitrate_observer.last_bitrate_bps_); } TEST_F(BitrateAllocatorTest, TwoBitrateObserversOneRtcpObserver) { @@ -82,29 +82,31 @@ 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); - EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_); + 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_); - EXPECT_EQ(200000u, bitrate_observer_2.last_bitrate_); + EXPECT_EQ(50, bitrate_observer_1.last_rtt_ms_); + EXPECT_EQ(200000u, bitrate_observer_2.last_bitrate_bps_); EXPECT_EQ(0, bitrate_observer_2.last_fraction_loss_); - EXPECT_EQ(50, bitrate_observer_2.last_rtt_); + EXPECT_EQ(50, bitrate_observer_2.last_rtt_ms_); // Test a bitrate which should be distributed equally. allocator_->OnNetworkChanged(500000, 0, 50); const uint32_t kBitrateToShare = 500000 - 200000 - 100000; - EXPECT_EQ(100000u + kBitrateToShare / 2, bitrate_observer_1.last_bitrate_); - EXPECT_EQ(200000u + kBitrateToShare / 2, bitrate_observer_2.last_bitrate_); + EXPECT_EQ(100000u + kBitrateToShare / 2, + bitrate_observer_1.last_bitrate_bps_); + EXPECT_EQ(200000u + kBitrateToShare / 2, + bitrate_observer_2.last_bitrate_bps_); // Limited by 2x max bitrates since we leave room for FEC and retransmissions. allocator_->OnNetworkChanged(1500000, 0, 50); - EXPECT_EQ(600000u, bitrate_observer_1.last_bitrate_); - EXPECT_EQ(600000u, bitrate_observer_2.last_bitrate_); + 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); - EXPECT_EQ(0u, bitrate_observer_1.last_bitrate_); - EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_); + EXPECT_EQ(0u, bitrate_observer_1.last_bitrate_bps_); + EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_bps_); } class BitrateAllocatorTestNoEnforceMin : public ::testing::Test { @@ -117,21 +119,21 @@ class BitrateAllocatorTestNoEnforceMin : public ::testing::Test { std::unique_ptr allocator_; }; -// The following three tests verify that the EnforceMinBitrate() method works -// as intended. +// The following three tests verify enforcing a minimum bitrate works as +// intended. TEST_F(BitrateAllocatorTestNoEnforceMin, OneBitrateObserver) { TestBitrateObserver bitrate_observer_1; int start_bitrate = allocator_->AddObserver(&bitrate_observer_1, 100000, 400000, false); EXPECT_EQ(300000, start_bitrate); - // High REMB. + // High BWE. allocator_->OnNetworkChanged(150000, 0, 0); - EXPECT_EQ(150000u, bitrate_observer_1.last_bitrate_); + EXPECT_EQ(150000u, bitrate_observer_1.last_bitrate_bps_); - // Low REMB. + // Low BWE. allocator_->OnNetworkChanged(10000, 0, 0); - EXPECT_EQ(10000u, bitrate_observer_1.last_bitrate_); + EXPECT_EQ(0u, bitrate_observer_1.last_bitrate_bps_); allocator_->RemoveObserver(&bitrate_observer_1); } @@ -148,52 +150,59 @@ TEST_F(BitrateAllocatorTestNoEnforceMin, ThreeBitrateObservers) { start_bitrate = allocator_->AddObserver(&bitrate_observer_2, 200000, 400000, false); EXPECT_EQ(200000, start_bitrate); - EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_); + EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_bps_); start_bitrate = allocator_->AddObserver(&bitrate_observer_3, 300000, 400000, false); EXPECT_EQ(0, start_bitrate); - EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_); - EXPECT_EQ(200000u, bitrate_observer_2.last_bitrate_); + EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_bps_); + EXPECT_EQ(200000u, bitrate_observer_2.last_bitrate_bps_); - // High REMB. Make sure the controllers get a fair share of the surplus - // (i.e., what is left after each controller gets its min rate). + // 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); // 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; - EXPECT_EQ(100000u + bitrate_to_share / 3, bitrate_observer_1.last_bitrate_); - EXPECT_EQ(200000u + bitrate_to_share / 3, bitrate_observer_2.last_bitrate_); - EXPECT_EQ(300000u + bitrate_to_share / 3, bitrate_observer_3.last_bitrate_); + EXPECT_EQ(100000u + bitrate_to_share / 3, + bitrate_observer_1.last_bitrate_bps_); + EXPECT_EQ(200000u + bitrate_to_share / 3, + bitrate_observer_2.last_bitrate_bps_); + EXPECT_EQ(300000u + bitrate_to_share / 3, + bitrate_observer_3.last_bitrate_bps_); - // High REMB, but below the sum of min bitrates. + // BWE below the sum of observer's min bitrate. + 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); - // Verify that the first and second observers get their min bitrates, and the - // third gets the remainder. - EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_); // Min bitrate. - EXPECT_EQ(200000u, bitrate_observer_2.last_bitrate_); // Min bitrate. - EXPECT_EQ(200000u, bitrate_observer_3.last_bitrate_); // Remainder. + 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. - // Low REMB. + // Below min for all. allocator_->OnNetworkChanged(10000, 0, 0); - // Verify that the first observer gets all the rate, and the rest get zero. - EXPECT_EQ(10000u, bitrate_observer_1.last_bitrate_); - EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_); - EXPECT_EQ(0u, bitrate_observer_3.last_bitrate_); + 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_); - allocator_->OnNetworkChanged(0, 0, 0); // Verify that zero estimated bandwidth, means that that all gets zero, // regardless of set min bitrate. - EXPECT_EQ(0u, bitrate_observer_1.last_bitrate_); - EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_); - EXPECT_EQ(0u, bitrate_observer_3.last_bitrate_); + 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_); allocator_->RemoveObserver(&bitrate_observer_1); allocator_->RemoveObserver(&bitrate_observer_2); allocator_->RemoveObserver(&bitrate_observer_3); } -TEST_F(BitrateAllocatorTest, ThreeBitrateObserversLowRembEnforceMin) { +TEST_F(BitrateAllocatorTest, ThreeBitrateObserversLowBweEnforceMin) { TestBitrateObserver bitrate_observer_1; TestBitrateObserver bitrate_observer_2; TestBitrateObserver bitrate_observer_3; @@ -204,19 +213,19 @@ TEST_F(BitrateAllocatorTest, ThreeBitrateObserversLowRembEnforceMin) { start_bitrate = allocator_->AddObserver(&bitrate_observer_2, 200000, 400000, true); EXPECT_EQ(200000, start_bitrate); - EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_); + EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_bps_); start_bitrate = allocator_->AddObserver(&bitrate_observer_3, 300000, 400000, true); EXPECT_EQ(300000, start_bitrate); - EXPECT_EQ(100000, static_cast(bitrate_observer_1.last_bitrate_)); - EXPECT_EQ(200000, static_cast(bitrate_observer_2.last_bitrate_)); + EXPECT_EQ(100000, static_cast(bitrate_observer_1.last_bitrate_bps_)); + EXPECT_EQ(200000, static_cast(bitrate_observer_2.last_bitrate_bps_)); - // Low REMB. Verify that all observers still get their respective min bitrate. + // Low BWE. Verify that all observers still get their respective min bitrate. allocator_->OnNetworkChanged(1000, 0, 0); - EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_); // Min cap. - EXPECT_EQ(200000u, bitrate_observer_2.last_bitrate_); // Min cap. - EXPECT_EQ(300000u, bitrate_observer_3.last_bitrate_); // Min cap. + 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. allocator_->RemoveObserver(&bitrate_observer_1); allocator_->RemoveObserver(&bitrate_observer_2); @@ -232,7 +241,7 @@ TEST_F(BitrateAllocatorTest, AddObserverWhileNetworkDown) { // Set network down, ie, no available bitrate. allocator_->OnNetworkChanged(0, 0, 0); - EXPECT_EQ(0u, bitrate_observer_1.last_bitrate_); + EXPECT_EQ(0u, bitrate_observer_1.last_bitrate_bps_); TestBitrateObserver bitrate_observer_2; start_bitrate = @@ -241,13 +250,111 @@ TEST_F(BitrateAllocatorTest, AddObserverWhileNetworkDown) { // Expect the start_bitrate to be set as if the network was still up but that // the new observer have been notified that the network is down. EXPECT_EQ(300000 / 2, start_bitrate); - EXPECT_EQ(0u, bitrate_observer_1.last_bitrate_); - EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_); + EXPECT_EQ(0u, bitrate_observer_1.last_bitrate_bps_); + EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_bps_); // Set network back up. allocator_->OnNetworkChanged(1500000, 0, 50); - EXPECT_EQ(750000u, bitrate_observer_1.last_bitrate_); - EXPECT_EQ(750000u, bitrate_observer_2.last_bitrate_); + EXPECT_EQ(750000u, bitrate_observer_1.last_bitrate_bps_); + EXPECT_EQ(750000u, bitrate_observer_2.last_bitrate_bps_); +} + +TEST_F(BitrateAllocatorTest, MixedEnforecedConfigs) { + TestBitrateObserver enforced_observer; + int start_bitrate = + allocator_->AddObserver(&enforced_observer, 6000, 30000, true); + EXPECT_EQ(60000, start_bitrate); + + TestBitrateObserver not_enforced_observer; + start_bitrate = + allocator_->AddObserver(¬_enforced_observer, 30000, 2500000, false); + EXPECT_EQ(270000, start_bitrate); + EXPECT_EQ(30000u, enforced_observer.last_bitrate_bps_); + + 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); + EXPECT_EQ(30000u, enforced_observer.last_bitrate_bps_); + EXPECT_EQ(0u, not_enforced_observer.last_bitrate_bps_); + + 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); + EXPECT_EQ(30000u, enforced_observer.last_bitrate_bps_); + EXPECT_EQ(0u, not_enforced_observer.last_bitrate_bps_); + + 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); + EXPECT_EQ(6000u, enforced_observer.last_bitrate_bps_); + EXPECT_EQ(50000u, not_enforced_observer.last_bitrate_bps_); + + allocator_->OnNetworkChanged(56000, 0, 50); + EXPECT_EQ(16000u, enforced_observer.last_bitrate_bps_); + EXPECT_EQ(40000u, not_enforced_observer.last_bitrate_bps_); + + allocator_->RemoveObserver(&enforced_observer); + allocator_->RemoveObserver(¬_enforced_observer); +} + +TEST_F(BitrateAllocatorTest, AvoidToggleAbsolute) { + TestBitrateObserver observer; + int start_bitrate = + allocator_->AddObserver(&observer, 30000, 300000, false); + EXPECT_EQ(300000, start_bitrate); + + allocator_->OnNetworkChanged(30000, 0, 50); + EXPECT_EQ(30000u, observer.last_bitrate_bps_); + + allocator_->OnNetworkChanged(20000, 0, 50); + EXPECT_EQ(0u, observer.last_bitrate_bps_); + + allocator_->OnNetworkChanged(30000, 0, 50); + EXPECT_EQ(0u, observer.last_bitrate_bps_); + + allocator_->OnNetworkChanged(49000, 0, 50); + EXPECT_EQ(0u, observer.last_bitrate_bps_); + + allocator_->OnNetworkChanged(50000, 0, 50); + EXPECT_EQ(50000u, observer.last_bitrate_bps_); + + allocator_->OnNetworkChanged(30000, 0, 50); + EXPECT_EQ(30000u, observer.last_bitrate_bps_); + + allocator_->RemoveObserver(&observer); +} + +TEST_F(BitrateAllocatorTest, AvoidTogglePercent) { + TestBitrateObserver observer; + int start_bitrate = + allocator_->AddObserver(&observer, 300000, 600000, false); + EXPECT_EQ(300000, start_bitrate); + + allocator_->OnNetworkChanged(300000, 0, 50); + EXPECT_EQ(300000u, observer.last_bitrate_bps_); + + allocator_->OnNetworkChanged(200000, 0, 50); + EXPECT_EQ(0u, observer.last_bitrate_bps_); + + allocator_->OnNetworkChanged(300000, 0, 50); + EXPECT_EQ(0u, observer.last_bitrate_bps_); + + allocator_->OnNetworkChanged(329000, 0, 50); + EXPECT_EQ(0u, observer.last_bitrate_bps_); + + allocator_->OnNetworkChanged(330000, 0, 50); + EXPECT_EQ(330000u, observer.last_bitrate_bps_); + + allocator_->OnNetworkChanged(300000, 0, 50); + EXPECT_EQ(300000u, observer.last_bitrate_bps_); + + allocator_->RemoveObserver(&observer); } } // namespace webrtc diff --git a/webrtc/call/call.cc b/webrtc/call/call.cc index e3c6ec7a0c..0ac5a0f1e1 100644 --- a/webrtc/call/call.cc +++ b/webrtc/call/call.cc @@ -10,6 +10,7 @@ #include +#include #include #include #include @@ -700,6 +701,10 @@ void Call::OnNetworkChanged(uint32_t target_bitrate_bps, uint8_t fraction_loss, pacer_bitrate_sum_kbits_ += pacer_bitrate_bps / 1000; ++num_bitrate_updates_; } + + // Make sure to not ask for more padding than the current BWE allows for. + pad_up_to_bitrate_bps = std::min(static_cast(pad_up_to_bitrate_bps), + target_bitrate_bps); congestion_controller_->SetAllocatedSendBitrate(allocated_bitrate_bps, pad_up_to_bitrate_bps); } diff --git a/webrtc/call/rampup_tests.cc b/webrtc/call/rampup_tests.cc index d6271d3ccb..150fc770ff 100644 --- a/webrtc/call/rampup_tests.cc +++ b/webrtc/call/rampup_tests.cc @@ -340,7 +340,6 @@ bool RampUpDownUpTester::PollStats() { for (auto it : stats.substreams) { transmit_bitrate_bps += it.second.total_bitrate_bps; } - EvolveTestState(transmit_bitrate_bps, stats.suspended); } @@ -464,10 +463,6 @@ TEST_F(RampUpTest, SingleStreamWithHighStartBitrate) { RunBaseTest(&test); } -// Disabled on Mac due to flakiness, see -// https://bugs.chromium.org/p/webrtc/issues/detail?id=5407 -#ifndef WEBRTC_MAC - static const uint32_t kStartBitrateBps = 60000; // Disabled: https://bugs.chromium.org/p/webrtc/issues/detail?id=5576 @@ -525,8 +520,6 @@ TEST_F(RampUpTest, DISABLED_SendSideAudioVideoUpDownUpRtx) { RunBaseTest(&test); } -#endif - TEST_F(RampUpTest, AbsSendTimeSingleStream) { RampUpTester test(1, 0, 0, RtpExtension::kAbsSendTimeUri, false, false); RunBaseTest(&test); diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index d1811d2ced..1c2a6423c1 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -560,10 +560,6 @@ void VideoSendStream::EncoderProcess() { encoder_settings->min_transmit_bitrate_bps, payload_router_.MaxPayloadLength()); - // vie_encoder_.SetEncoder must be called before this. - if (config_.suspend_below_min_bitrate) - video_sender_->SuspendBelowMinBitrate(); - // Clear stats for disabled layers. for (size_t i = encoder_settings->streams.size(); i < config_.rtp.ssrcs.size(); ++i) { diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc index 87abfea08c..265d7495b1 100644 --- a/webrtc/video/video_send_stream_tests.cc +++ b/webrtc/video/video_send_stream_tests.cc @@ -905,7 +905,7 @@ TEST_F(VideoSendStreamTest, SuspendBelowMinBitrate) { send_config->suspend_below_min_bitrate = true; int min_bitrate_bps = encoder_config->streams[0].min_bitrate_bps; set_low_remb_bps(min_bitrate_bps - 10000); - int threshold_window = std::max(min_bitrate_bps / 10, 10000); + int threshold_window = std::max(min_bitrate_bps / 10, 20000); ASSERT_GT(encoder_config->streams[0].max_bitrate_bps, min_bitrate_bps + threshold_window + 5000); set_high_remb_bps(min_bitrate_bps + threshold_window + 5000); diff --git a/webrtc/video/vie_encoder.cc b/webrtc/video/vie_encoder.cc index 73b29f319e..529b38f5b8 100644 --- a/webrtc/video/vie_encoder.cc +++ b/webrtc/video/vie_encoder.cc @@ -132,8 +132,8 @@ void ViEEncoder::SetEncoder(const webrtc::VideoCodec& video_codec, int ViEEncoder::GetPaddingNeededBps() const { int64_t time_of_last_frame_activity_ms; int min_transmit_bitrate_bps; - int bitrate_bps; VideoCodec send_codec; + bool video_is_suspended; { rtc::CritScope lock(&data_cs_); bool send_padding = encoder_config_.numberOfSimulcastStreams > 1 || @@ -142,12 +142,10 @@ int ViEEncoder::GetPaddingNeededBps() const { return 0; time_of_last_frame_activity_ms = time_of_last_frame_activity_ms_; min_transmit_bitrate_bps = min_transmit_bitrate_bps_; - bitrate_bps = last_observed_bitrate_bps_; send_codec = encoder_config_; + video_is_suspended = video_suspended_; } - bool video_is_suspended = video_sender_.VideoSuspended(); - // Find the max amount of padding we can allow ourselves to send at this // point, based on which streams are currently active and what our current // available bandwidth is. @@ -179,10 +177,6 @@ int ViEEncoder::GetPaddingNeededBps() const { if (pad_up_to_bitrate_bps < min_transmit_bitrate_bps) pad_up_to_bitrate_bps = min_transmit_bitrate_bps; - // Padding may never exceed bitrate estimate. - if (pad_up_to_bitrate_bps > bitrate_bps) - pad_up_to_bitrate_bps = bitrate_bps; - return pad_up_to_bitrate_bps; } @@ -320,8 +314,8 @@ void ViEEncoder::OnBitrateUpdated(uint32_t bitrate_bps, << " rtt " << round_trip_time_ms; video_sender_.SetChannelParameters(bitrate_bps, fraction_lost, round_trip_time_ms); - bool video_is_suspended = video_sender_.VideoSuspended(); bool video_suspension_changed; + bool video_is_suspended = bitrate_bps == 0; { rtc::CritScope lock(&data_cs_); last_observed_bitrate_bps_ = bitrate_bps; @@ -329,13 +323,10 @@ void ViEEncoder::OnBitrateUpdated(uint32_t bitrate_bps, video_suspended_ = video_is_suspended; } - if (!video_suspension_changed) - return; - // Video suspend-state changed, inform codec observer. - LOG(LS_INFO) << "Video suspend state changed " << video_is_suspended; - - if (stats_proxy_) + if (stats_proxy_ && video_suspension_changed) { + LOG(LS_INFO) << "Video suspend state changed " << video_is_suspended; stats_proxy_->OnSuspendChange(video_is_suspended); + } } } // namespace webrtc diff --git a/webrtc/video/vie_encoder.h b/webrtc/video/vie_encoder.h index 4372bd11c9..91497df6e9 100644 --- a/webrtc/video/vie_encoder.h +++ b/webrtc/video/vie_encoder.h @@ -106,6 +106,8 @@ class ViEEncoder : public VideoEncoderRateObserver, virtual void OnReceivedSLI(uint8_t picture_id); virtual void OnReceivedRPSI(uint64_t picture_id); + // Note that this method might return a value higher than the current BWE and + // it's up to the caller to possibly limit the value. int GetPaddingNeededBps() const; void OnBitrateUpdated(uint32_t bitrate_bps,