diff --git a/webrtc/call/bitrate_allocator.cc b/webrtc/call/bitrate_allocator.cc index e4b1ad49fa..e20c34e05d 100644 --- a/webrtc/call/bitrate_allocator.cc +++ b/webrtc/call/bitrate_allocator.cc @@ -28,16 +28,17 @@ const int kDefaultBitrateBps = 300000; const double kToggleFactor = 0.1; const uint32_t kMinToggleBitrateBps = 20000; -BitrateAllocator::BitrateAllocator() - : bitrate_observer_configs_(), +BitrateAllocator::BitrateAllocator(LimitObserver* limit_observer) + : limit_observer_(limit_observer), + bitrate_observer_configs_(), last_bitrate_bps_(kDefaultBitrateBps), last_non_zero_bitrate_bps_(kDefaultBitrateBps), last_fraction_loss_(0), last_rtt_(0) {} -uint32_t BitrateAllocator::OnNetworkChanged(uint32_t target_bitrate_bps, - uint8_t fraction_loss, - int64_t rtt) { +void BitrateAllocator::OnNetworkChanged(uint32_t target_bitrate_bps, + uint8_t fraction_loss, + int64_t rtt) { rtc::CritScope lock(&crit_sect_); last_bitrate_bps_ = target_bitrate_bps; last_non_zero_bitrate_bps_ = @@ -45,19 +46,17 @@ uint32_t BitrateAllocator::OnNetworkChanged(uint32_t target_bitrate_bps, last_fraction_loss_ = fraction_loss; last_rtt_ = rtt; - uint32_t allocated_bitrate_bps = 0; 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; } int BitrateAllocator::AddObserver(BitrateAllocatorObserver* observer, uint32_t min_bitrate_bps, uint32_t max_bitrate_bps, + uint32_t pad_up_bitrate_bps, bool enforce_min_bitrate) { rtc::CritScope lock(&crit_sect_); auto it = FindObserverConfig(observer); @@ -66,10 +65,12 @@ int BitrateAllocator::AddObserver(BitrateAllocatorObserver* observer, if (it != bitrate_observer_configs_.end()) { it->min_bitrate_bps = min_bitrate_bps; it->max_bitrate_bps = max_bitrate_bps; + it->pad_up_bitrate_bps = pad_up_bitrate_bps; it->enforce_min_bitrate = enforce_min_bitrate; } else { - bitrate_observer_configs_.push_back(ObserverConfig( - observer, min_bitrate_bps, max_bitrate_bps, enforce_min_bitrate)); + bitrate_observer_configs_.push_back( + ObserverConfig(observer, min_bitrate_bps, max_bitrate_bps, + pad_up_bitrate_bps, enforce_min_bitrate)); } ObserverAllocation allocation; @@ -85,16 +86,39 @@ int BitrateAllocator::AddObserver(BitrateAllocatorObserver* observer, allocation = AllocateBitrates(last_non_zero_bitrate_bps_); observer->OnBitrateUpdated(0, last_fraction_loss_, last_rtt_); } + UpdateAllocationLimits(); + last_allocation_ = allocation; return allocation[observer]; } -void BitrateAllocator::RemoveObserver(BitrateAllocatorObserver* observer) { - rtc::CritScope lock(&crit_sect_); - auto it = FindObserverConfig(observer); - if (it != bitrate_observer_configs_.end()) { - bitrate_observer_configs_.erase(it); +void BitrateAllocator::UpdateAllocationLimits() { + uint32_t total_requested_padding_bitrate = 0; + uint32_t total_requested_min_bitrate = 0; + + { + rtc::CritScope lock(&crit_sect_); + for (const auto& config : bitrate_observer_configs_) { + if (config.enforce_min_bitrate) { + total_requested_min_bitrate += config.min_bitrate_bps; + } + total_requested_padding_bitrate += config.pad_up_bitrate_bps; + } } + + limit_observer_->OnAllocationLimitsChanged(total_requested_min_bitrate, + total_requested_padding_bitrate); +} + +void BitrateAllocator::RemoveObserver(BitrateAllocatorObserver* observer) { + { + rtc::CritScope lock(&crit_sect_); + auto it = FindObserverConfig(observer); + if (it != bitrate_observer_configs_.end()) { + bitrate_observer_configs_.erase(it); + } + } + UpdateAllocationLimits(); } BitrateAllocator::ObserverConfigList::iterator diff --git a/webrtc/call/bitrate_allocator.h b/webrtc/call/bitrate_allocator.h index fd9fe7172b..06e8b305d9 100644 --- a/webrtc/call/bitrate_allocator.h +++ b/webrtc/call/bitrate_allocator.h @@ -31,6 +31,8 @@ class BitrateAllocatorObserver { virtual void OnBitrateUpdated(uint32_t bitrate_bps, uint8_t fraction_loss, int64_t rtt) = 0; + + protected: virtual ~BitrateAllocatorObserver() {} }; @@ -39,14 +41,24 @@ class BitrateAllocatorObserver { // and push the result to the encoders via BitrateAllocatorObserver(s). class BitrateAllocator { public: - BitrateAllocator(); + // Used to get notified when send stream limits such as the minimum send + // bitrate and max padding bitrate is changed. + class LimitObserver { + public: + virtual void OnAllocationLimitsChanged( + uint32_t min_send_bitrate_bps, + uint32_t max_padding_bitrate_bps) = 0; + + protected: + virtual ~LimitObserver() {} + }; + + explicit BitrateAllocator(LimitObserver* limit_observer); // Allocate target_bitrate across the registered BitrateAllocatorObservers. - // 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); + void OnNetworkChanged(uint32_t target_bitrate_bps, + uint8_t fraction_loss, + int64_t rtt); // Set the start and max send bitrate used by the bandwidth management. // @@ -64,6 +76,7 @@ class BitrateAllocator { int AddObserver(BitrateAllocatorObserver* observer, uint32_t min_bitrate_bps, uint32_t max_bitrate_bps, + uint32_t pad_up_bitrate_bps, bool enforce_min_bitrate); // Removes a previously added observer, but will not trigger a new bitrate @@ -76,17 +89,24 @@ class BitrateAllocator { ObserverConfig(BitrateAllocatorObserver* observer, uint32_t min_bitrate_bps, uint32_t max_bitrate_bps, + uint32_t pad_up_bitrate_bps, bool enforce_min_bitrate) : observer(observer), min_bitrate_bps(min_bitrate_bps), max_bitrate_bps(max_bitrate_bps), + pad_up_bitrate_bps(pad_up_bitrate_bps), enforce_min_bitrate(enforce_min_bitrate) {} BitrateAllocatorObserver* const observer; uint32_t min_bitrate_bps; uint32_t max_bitrate_bps; + uint32_t pad_up_bitrate_bps; bool enforce_min_bitrate; }; + // Calculates the minimum requested send bitrate and max padding bitrate and + // calls LimitObserver::OnAllocationLimitsChanged. + void UpdateAllocationLimits(); + typedef std::list ObserverConfigList; ObserverConfigList::iterator FindObserverConfig( const BitrateAllocatorObserver* observer) @@ -126,6 +146,7 @@ class BitrateAllocator { bool EnoughBitrateForAllObservers(uint32_t bitrate, uint32_t sum_min_bitrates) EXCLUSIVE_LOCKS_REQUIRED(crit_sect_); + LimitObserver* const limit_observer_; rtc::CriticalSection crit_sect_; // Stored in a list to keep track of the insertion order. diff --git a/webrtc/call/bitrate_allocator_unittest.cc b/webrtc/call/bitrate_allocator_unittest.cc index 9cb5f6cf99..74c635d0c5 100644 --- a/webrtc/call/bitrate_allocator_unittest.cc +++ b/webrtc/call/bitrate_allocator_unittest.cc @@ -12,12 +12,22 @@ #include #include +#include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" #include "webrtc/call/bitrate_allocator.h" #include "webrtc/modules/bitrate_controller/include/bitrate_controller.h" +using testing::NiceMock; + namespace webrtc { +class MockLimitObserver : public BitrateAllocator::LimitObserver { + public: + MOCK_METHOD2(OnAllocationLimitsChanged, + void(uint32_t min_send_bitrate_bps, + uint32_t max_padding_bitrate_bps)); +}; + class TestBitrateObserver : public BitrateAllocatorObserver { public: TestBitrateObserver() @@ -37,18 +47,24 @@ class TestBitrateObserver : public BitrateAllocatorObserver { class BitrateAllocatorTest : public ::testing::Test { protected: - BitrateAllocatorTest() : allocator_(new BitrateAllocator()) { + BitrateAllocatorTest() : allocator_(new BitrateAllocator(&limit_observer_)) { allocator_->OnNetworkChanged(300000u, 0, 0); } ~BitrateAllocatorTest() {} + NiceMock limit_observer_; std::unique_ptr allocator_; }; TEST_F(BitrateAllocatorTest, UpdatingBitrateObserver) { TestBitrateObserver bitrate_observer; - int start_bitrate = - allocator_->AddObserver(&bitrate_observer, 100000, 1500000, true); + const uint32_t kMinSendBitrateBps = 100000; + const uint32_t kPadUpToBitrateBps = 50000; + + EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(kMinSendBitrateBps, + kPadUpToBitrateBps)); + int start_bitrate = allocator_->AddObserver( + &bitrate_observer, kMinSendBitrateBps, 1500000, kPadUpToBitrateBps, true); EXPECT_EQ(300000, start_bitrate); allocator_->OnNetworkChanged(200000, 0, 0); EXPECT_EQ(200000u, bitrate_observer.last_bitrate_bps_); @@ -57,12 +73,18 @@ TEST_F(BitrateAllocatorTest, UpdatingBitrateObserver) { // bitrate for FEC/retransmissions (see todo in BitrateAllocator). allocator_->OnNetworkChanged(4000000, 0, 0); EXPECT_EQ(3000000u, bitrate_observer.last_bitrate_bps_); - start_bitrate = - allocator_->AddObserver(&bitrate_observer, 100000, 4000000, true); + + // Expect |max_padding_bitrate_bps| to change to 0 if the observer is updated. + EXPECT_CALL(limit_observer_, + OnAllocationLimitsChanged(kMinSendBitrateBps, 0)); + start_bitrate = allocator_->AddObserver(&bitrate_observer, kMinSendBitrateBps, + 4000000, 0, true); EXPECT_EQ(4000000, start_bitrate); - start_bitrate = - allocator_->AddObserver(&bitrate_observer, 100000, 1500000, true); + EXPECT_CALL(limit_observer_, + OnAllocationLimitsChanged(kMinSendBitrateBps, 0)); + start_bitrate = allocator_->AddObserver(&bitrate_observer, kMinSendBitrateBps, + 1500000, 0, true); EXPECT_EQ(3000000, start_bitrate); EXPECT_EQ(3000000u, bitrate_observer.last_bitrate_bps_); allocator_->OnNetworkChanged(1500000, 0, 0); @@ -72,11 +94,13 @@ TEST_F(BitrateAllocatorTest, UpdatingBitrateObserver) { TEST_F(BitrateAllocatorTest, TwoBitrateObserversOneRtcpObserver) { TestBitrateObserver bitrate_observer_1; TestBitrateObserver bitrate_observer_2; + EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(100000, 0)); int start_bitrate = - allocator_->AddObserver(&bitrate_observer_1, 100000, 300000, true); + allocator_->AddObserver(&bitrate_observer_1, 100000, 300000, 0, true); EXPECT_EQ(300000, start_bitrate); + EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(100000 + 200000, 0)); start_bitrate = - allocator_->AddObserver(&bitrate_observer_2, 200000, 300000, true); + allocator_->AddObserver(&bitrate_observer_2, 200000, 300000, 0, true); EXPECT_EQ(200000, start_bitrate); // Test too low start bitrate, hence lower than sum of min. Min bitrates will @@ -109,13 +133,28 @@ TEST_F(BitrateAllocatorTest, TwoBitrateObserversOneRtcpObserver) { EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_bps_); } +TEST_F(BitrateAllocatorTest, RemoveObserverTriggersLimitObserver) { + TestBitrateObserver bitrate_observer; + const uint32_t kMinSendBitrateBps = 100000; + const uint32_t kPadUpToBitrateBps = 50000; + + EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(kMinSendBitrateBps, + kPadUpToBitrateBps)); + allocator_->AddObserver(&bitrate_observer, kMinSendBitrateBps, 1500000, + kPadUpToBitrateBps, true); + EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(0, 0)); + allocator_->RemoveObserver(&bitrate_observer); +} + class BitrateAllocatorTestNoEnforceMin : public ::testing::Test { protected: - BitrateAllocatorTestNoEnforceMin() : allocator_(new BitrateAllocator()) { + BitrateAllocatorTestNoEnforceMin() + : allocator_(new BitrateAllocator(&limit_observer_)) { allocator_->OnNetworkChanged(300000u, 0, 0); } ~BitrateAllocatorTestNoEnforceMin() {} + NiceMock limit_observer_; std::unique_ptr allocator_; }; @@ -123,8 +162,11 @@ class BitrateAllocatorTestNoEnforceMin : public ::testing::Test { // intended. TEST_F(BitrateAllocatorTestNoEnforceMin, OneBitrateObserver) { TestBitrateObserver bitrate_observer_1; + // Expect OnAllocationLimitsChanged with |min_send_bitrate_bps| = 0 since + // AddObserver is called with |enforce_min_bitrate| = false. + EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(0, 0)); int start_bitrate = - allocator_->AddObserver(&bitrate_observer_1, 100000, 400000, false); + allocator_->AddObserver(&bitrate_observer_1, 100000, 400000, 0, false); EXPECT_EQ(300000, start_bitrate); // High BWE. @@ -135,6 +177,7 @@ TEST_F(BitrateAllocatorTestNoEnforceMin, OneBitrateObserver) { allocator_->OnNetworkChanged(10000, 0, 0); EXPECT_EQ(0u, bitrate_observer_1.last_bitrate_bps_); + EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(0, 0)); allocator_->RemoveObserver(&bitrate_observer_1); } @@ -144,16 +187,16 @@ TEST_F(BitrateAllocatorTestNoEnforceMin, ThreeBitrateObservers) { TestBitrateObserver bitrate_observer_3; // Set up the observers with min bitrates at 100000, 200000, and 300000. int start_bitrate = - allocator_->AddObserver(&bitrate_observer_1, 100000, 400000, false); + allocator_->AddObserver(&bitrate_observer_1, 100000, 400000, 0, false); EXPECT_EQ(300000, start_bitrate); start_bitrate = - allocator_->AddObserver(&bitrate_observer_2, 200000, 400000, false); + allocator_->AddObserver(&bitrate_observer_2, 200000, 400000, 0, false); EXPECT_EQ(200000, start_bitrate); EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_bps_); start_bitrate = - allocator_->AddObserver(&bitrate_observer_3, 300000, 400000, false); + allocator_->AddObserver(&bitrate_observer_3, 300000, 400000, 0, false); EXPECT_EQ(0, start_bitrate); EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_bps_); EXPECT_EQ(200000u, bitrate_observer_2.last_bitrate_bps_); @@ -207,16 +250,16 @@ TEST_F(BitrateAllocatorTest, ThreeBitrateObserversLowBweEnforceMin) { TestBitrateObserver bitrate_observer_2; TestBitrateObserver bitrate_observer_3; int start_bitrate = - allocator_->AddObserver(&bitrate_observer_1, 100000, 400000, true); + allocator_->AddObserver(&bitrate_observer_1, 100000, 400000, 0, true); EXPECT_EQ(300000, start_bitrate); start_bitrate = - allocator_->AddObserver(&bitrate_observer_2, 200000, 400000, true); + allocator_->AddObserver(&bitrate_observer_2, 200000, 400000, 0, true); EXPECT_EQ(200000, start_bitrate); EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_bps_); start_bitrate = - allocator_->AddObserver(&bitrate_observer_3, 300000, 400000, true); + allocator_->AddObserver(&bitrate_observer_3, 300000, 400000, 0, true); EXPECT_EQ(300000, start_bitrate); EXPECT_EQ(100000, static_cast(bitrate_observer_1.last_bitrate_bps_)); EXPECT_EQ(200000, static_cast(bitrate_observer_2.last_bitrate_bps_)); @@ -234,8 +277,9 @@ TEST_F(BitrateAllocatorTest, ThreeBitrateObserversLowBweEnforceMin) { TEST_F(BitrateAllocatorTest, AddObserverWhileNetworkDown) { TestBitrateObserver bitrate_observer_1; + EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(50000, 0)); int start_bitrate = - allocator_->AddObserver(&bitrate_observer_1, 50000, 400000, true); + allocator_->AddObserver(&bitrate_observer_1, 50000, 400000, 0, true); EXPECT_EQ(300000, start_bitrate); // Set network down, ie, no available bitrate. @@ -244,8 +288,10 @@ TEST_F(BitrateAllocatorTest, AddObserverWhileNetworkDown) { EXPECT_EQ(0u, bitrate_observer_1.last_bitrate_bps_); TestBitrateObserver bitrate_observer_2; + // Adding an observer while the network is down should not affect the limits. + EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(50000 + 50000, 0)); start_bitrate = - allocator_->AddObserver(&bitrate_observer_2, 50000, 400000, true); + allocator_->AddObserver(&bitrate_observer_2, 50000, 400000, 0, true); // 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. @@ -262,12 +308,12 @@ TEST_F(BitrateAllocatorTest, AddObserverWhileNetworkDown) { TEST_F(BitrateAllocatorTest, MixedEnforecedConfigs) { TestBitrateObserver enforced_observer; int start_bitrate = - allocator_->AddObserver(&enforced_observer, 6000, 30000, true); + allocator_->AddObserver(&enforced_observer, 6000, 30000, 0, true); EXPECT_EQ(60000, start_bitrate); TestBitrateObserver not_enforced_observer; start_bitrate = - allocator_->AddObserver(¬_enforced_observer, 30000, 2500000, false); + allocator_->AddObserver(¬_enforced_observer, 30000, 2500000, 0, false); EXPECT_EQ(270000, start_bitrate); EXPECT_EQ(30000u, enforced_observer.last_bitrate_bps_); @@ -306,7 +352,7 @@ TEST_F(BitrateAllocatorTest, MixedEnforecedConfigs) { TEST_F(BitrateAllocatorTest, AvoidToggleAbsolute) { TestBitrateObserver observer; int start_bitrate = - allocator_->AddObserver(&observer, 30000, 300000, false); + allocator_->AddObserver(&observer, 30000, 300000, 0, false); EXPECT_EQ(300000, start_bitrate); allocator_->OnNetworkChanged(30000, 0, 50); @@ -333,7 +379,7 @@ TEST_F(BitrateAllocatorTest, AvoidToggleAbsolute) { TEST_F(BitrateAllocatorTest, AvoidTogglePercent) { TestBitrateObserver observer; int start_bitrate = - allocator_->AddObserver(&observer, 300000, 600000, false); + allocator_->AddObserver(&observer, 300000, 600000, 0, false); EXPECT_EQ(300000, start_bitrate); allocator_->OnNetworkChanged(300000, 0, 50); diff --git a/webrtc/call/call.cc b/webrtc/call/call.cc index ea01d91330..93b638a448 100644 --- a/webrtc/call/call.cc +++ b/webrtc/call/call.cc @@ -55,7 +55,8 @@ namespace internal { class Call : public webrtc::Call, public PacketReceiver, - public CongestionController::Observer { + public CongestionController::Observer, + public BitrateAllocator::LimitObserver { public: explicit Call(const Call::Config& config); virtual ~Call(); @@ -102,6 +103,10 @@ class Call : public webrtc::Call, 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, + uint32_t max_padding_bitrate_bps) override; + private: DeliveryStatus DeliverRtcp(MediaType media_type, const uint8_t* packet, size_t length); @@ -174,6 +179,7 @@ class Call : public webrtc::Call, rtc::CriticalSection bitrate_crit_; int64_t estimated_send_bitrate_sum_kbits_ GUARDED_BY(&bitrate_crit_); int64_t pacer_bitrate_sum_kbits_ GUARDED_BY(&bitrate_crit_); + uint32_t min_allocated_send_bitrate_bps_ GUARDED_BY(&bitrate_crit_); int64_t num_bitrate_updates_ GUARDED_BY(&bitrate_crit_); std::map network_routes_; @@ -198,7 +204,7 @@ Call::Call(const Call::Config& config) module_process_thread_(ProcessThread::Create("ModuleProcessThread")), pacer_thread_(ProcessThread::Create("PacerThread")), call_stats_(new CallStats(clock_)), - bitrate_allocator_(new BitrateAllocator()), + bitrate_allocator_(new BitrateAllocator(this)), config_(config), audio_network_state_(kNetworkUp), video_network_state_(kNetworkUp), @@ -212,6 +218,7 @@ Call::Call(const Call::Config& config) first_packet_sent_ms_(-1), estimated_send_bitrate_sum_kbits_(0), pacer_bitrate_sum_kbits_(0), + min_allocated_send_bitrate_bps_(0), num_bitrate_updates_(0), remb_(clock_), congestion_controller_(new CongestionController(clock_, this, &remb_)), @@ -677,38 +684,29 @@ void Call::OnSentPacket(const rtc::SentPacket& sent_packet) { void Call::OnNetworkChanged(uint32_t target_bitrate_bps, uint8_t fraction_loss, int64_t rtt_ms) { - uint32_t allocated_bitrate_bps = bitrate_allocator_->OnNetworkChanged( - target_bitrate_bps, fraction_loss, rtt_ms); + bitrate_allocator_->OnNetworkChanged(target_bitrate_bps, fraction_loss, + rtt_ms); - int pad_up_to_bitrate_bps = 0; - { - ReadLockScoped read_lock(*send_crit_); - // No need to update as long as we're not sending. - if (video_send_streams_.empty()) - return; - - for (VideoSendStream* stream : video_send_streams_) - pad_up_to_bitrate_bps += stream->GetPaddingNeededBps(); - } - // Allocated bitrate might be higher than bitrate estimate if enforcing min - // bitrate, or lower if estimate is higher than the sum of max bitrates, so - // set the pacer bitrate to the maximum of the two. - uint32_t pacer_bitrate_bps = - std::max(target_bitrate_bps, allocated_bitrate_bps); { rtc::CritScope lock(&bitrate_crit_); // We only update these stats if we have send streams, and assume that // OnNetworkChanged is called roughly with a fixed frequency. estimated_send_bitrate_sum_kbits_ += target_bitrate_bps / 1000; + // Pacer bitrate might be higher than bitrate estimate if enforcing min + // bitrate. + uint32_t pacer_bitrate_bps = + std::max(target_bitrate_bps, min_allocated_send_bitrate_bps_); 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); +void Call::OnAllocationLimitsChanged(uint32_t min_send_bitrate_bps, + uint32_t max_padding_bitrate_bps) { + congestion_controller_->SetAllocatedSendBitrateLimits( + min_send_bitrate_bps, max_padding_bitrate_bps); + rtc::CritScope lock(&bitrate_crit_); + min_allocated_send_bitrate_bps_ = min_send_bitrate_bps; } void Call::ConfigureSync(const std::string& sync_group) { diff --git a/webrtc/modules/congestion_controller/congestion_controller.cc b/webrtc/modules/congestion_controller/congestion_controller.cc index 5767448a5f..38e488a09d 100644 --- a/webrtc/modules/congestion_controller/congestion_controller.cc +++ b/webrtc/modules/congestion_controller/congestion_controller.cc @@ -257,9 +257,10 @@ CongestionController::GetTransportFeedbackObserver() { return &transport_feedback_adapter_; } -void CongestionController::SetAllocatedSendBitrate(int allocated_bitrate_bps, - int padding_bitrate_bps) { - pacer_->SetAllocatedSendBitrate(allocated_bitrate_bps, padding_bitrate_bps); +void CongestionController::SetAllocatedSendBitrateLimits( + int min_send_bitrate_bps, + int max_padding_bitrate_bps) { + pacer_->SetSendBitrateLimits(min_send_bitrate_bps, max_padding_bitrate_bps); } int64_t CongestionController::GetPacerQueuingDelayMs() const { diff --git a/webrtc/modules/congestion_controller/include/congestion_controller.h b/webrtc/modules/congestion_controller/include/congestion_controller.h index da8719d33a..1c04c119df 100644 --- a/webrtc/modules/congestion_controller/include/congestion_controller.h +++ b/webrtc/modules/congestion_controller/include/congestion_controller.h @@ -79,8 +79,18 @@ class CongestionController : public CallStatsObserver, public Module { virtual PacketRouter* packet_router() { return packet_router_.get(); } virtual TransportFeedbackObserver* GetTransportFeedbackObserver(); - void SetAllocatedSendBitrate(int allocated_bitrate_bps, - int padding_bitrate_bps); + // SetAllocatedSendBitrateLimits sets bitrates limits imposed by send codec + // settings. + // |min_send_bitrate_bps| is the total minimum send bitrate required by all + // sending streams. This is the minimum bitrate the PacedSender will use. + // Note that CongestionController::OnNetworkChanged can still be called with + // a lower bitrate estimate. + // |max_padding_bitrate_bps| is the max bitrate the send streams request for + // padding. This can be higher than the current network estimate and tells + // the PacedSender how much it should max pad unless there is real packets to + // send. + void SetAllocatedSendBitrateLimits(int min_send_bitrate_bps, + int max_padding_bitrate_bps); virtual void OnSentPacket(const rtc::SentPacket& sent_packet); diff --git a/webrtc/modules/pacing/paced_sender.cc b/webrtc/modules/pacing/paced_sender.cc index efbf969ac1..c1741d6902 100644 --- a/webrtc/modules/pacing/paced_sender.cc +++ b/webrtc/modules/pacing/paced_sender.cc @@ -245,8 +245,7 @@ class IntervalBudget { const int64_t PacedSender::kMaxQueueLengthMs = 2000; const float PacedSender::kDefaultPaceMultiplier = 2.5f; -PacedSender::PacedSender(Clock* clock, - PacketSender* packet_sender) +PacedSender::PacedSender(Clock* clock, PacketSender* packet_sender) : clock_(clock), packet_sender_(packet_sender), critsect_(CriticalSectionWrapper::CreateCriticalSection()), @@ -257,6 +256,7 @@ PacedSender::PacedSender(Clock* clock, prober_(new BitrateProber()), estimated_bitrate_bps_(0), min_send_bitrate_kbps_(0u), + max_padding_bitrate_kbps_(0u), pacing_bitrate_kbps_(0), time_last_update_us_(clock->TimeInMicroseconds()), packets_(new paced_sender::PacketQueue(clock)), @@ -284,19 +284,23 @@ void PacedSender::SetProbingEnabled(bool enabled) { void PacedSender::SetEstimatedBitrate(uint32_t bitrate_bps) { CriticalSectionScoped cs(critsect_.get()); estimated_bitrate_bps_ = bitrate_bps; + padding_budget_->set_target_rate_kbps( + std::min(estimated_bitrate_bps_ / 1000, max_padding_bitrate_kbps_)); pacing_bitrate_kbps_ = std::max(min_send_bitrate_kbps_, estimated_bitrate_bps_ / 1000) * kDefaultPaceMultiplier; } -void PacedSender::SetAllocatedSendBitrate(int allocated_bitrate, - int padding_bitrate) { +void PacedSender::SetSendBitrateLimits(int min_send_bitrate_bps, + int padding_bitrate) { CriticalSectionScoped cs(critsect_.get()); - min_send_bitrate_kbps_ = allocated_bitrate / 1000; + min_send_bitrate_kbps_ = min_send_bitrate_bps / 1000; pacing_bitrate_kbps_ = std::max(min_send_bitrate_kbps_, estimated_bitrate_bps_ / 1000) * kDefaultPaceMultiplier; - padding_budget_->set_target_rate_kbps(padding_bitrate / 1000); + max_padding_bitrate_kbps_ = padding_bitrate / 1000; + padding_budget_->set_target_rate_kbps( + std::min(estimated_bitrate_bps_ / 1000, max_padding_bitrate_kbps_)); } void PacedSender::InsertPacket(RtpPacketSender::Priority priority, @@ -418,15 +422,15 @@ void PacedSender::Process() { if (paused_ || !packets_->Empty()) return; - size_t padding_needed; - if (is_probing) { - padding_needed = prober_->RecommendedPacketSize(); - } else { - padding_needed = padding_budget_->bytes_remaining(); - } + // We can not send padding unless a normal packet has first been sent. If we + // do, timestamps get messed up. + if (packet_counter_ > 0) { + size_t padding_needed = is_probing ? prober_->RecommendedPacketSize() + : padding_budget_->bytes_remaining(); - if (padding_needed > 0) - SendPadding(padding_needed, probe_cluster_id); + if (padding_needed > 0) + SendPadding(padding_needed, probe_cluster_id); + } } bool PacedSender::SendPacket(const paced_sender::Packet& packet, diff --git a/webrtc/modules/pacing/paced_sender.h b/webrtc/modules/pacing/paced_sender.h index 8879f12d03..1d9f2de261 100644 --- a/webrtc/modules/pacing/paced_sender.h +++ b/webrtc/modules/pacing/paced_sender.h @@ -89,13 +89,15 @@ class PacedSender : public Module, public RtpPacketSender { // |bitrate_bps| * kDefaultPaceMultiplier. virtual void SetEstimatedBitrate(uint32_t bitrate_bps); - // Sets the bitrate that has been allocated for encoders. - // |allocated_bitrate| might be higher that the estimated available network - // bitrate and if so, the pacer will send with |allocated_bitrate|. - // Padding packets will be utilized to reach |padding_bitrate| unless enough - // media packets are available. - void SetAllocatedSendBitrate(int allocated_bitrate_bps, - int padding_bitrate_bps); + // Sets the minimum send bitrate and maximum padding bitrate requested by send + // streams. + // |min_send_bitrate_bps| might be higher that the estimated available network + // bitrate and if so, the pacer will send with |min_send_bitrate_bps|. + // |max_padding_bitrate_bps| might be higher than the estimate available + // network bitrate and if so, the pacer will send padding packets to reach + // the min of the estimated available bitrate and |max_padding_bitrate_bps|. + void SetSendBitrateLimits(int min_send_bitrate_bps, + int max_padding_bitrate_bps); // Returns true if we send the packet now, else it will add the packet // information to the queue and call TimeToSendPacket when it's time to send. @@ -157,6 +159,7 @@ class PacedSender : public Module, public RtpPacketSender { // order to meet pace time constraint). uint32_t estimated_bitrate_bps_ GUARDED_BY(critsect_); uint32_t min_send_bitrate_kbps_ GUARDED_BY(critsect_); + uint32_t max_padding_bitrate_kbps_ GUARDED_BY(critsect_); uint32_t pacing_bitrate_kbps_ GUARDED_BY(critsect_); int64_t time_last_update_us_ GUARDED_BY(critsect_); diff --git a/webrtc/modules/pacing/paced_sender_unittest.cc b/webrtc/modules/pacing/paced_sender_unittest.cc index 063ec1e194..51394c72d1 100644 --- a/webrtc/modules/pacing/paced_sender_unittest.cc +++ b/webrtc/modules/pacing/paced_sender_unittest.cc @@ -322,7 +322,7 @@ TEST_F(PacedSenderTest, Padding) { uint16_t sequence_number = 1234; send_bucket_->SetEstimatedBitrate(kTargetBitrateBps); - send_bucket_->SetAllocatedSendBitrate(kTargetBitrateBps, kTargetBitrateBps); + send_bucket_->SetSendBitrateLimits(kTargetBitrateBps, kTargetBitrateBps); // Due to the multiplicative factor we can send 5 packets during a send // interval. (network capacity * multiplier / (8 bits per byte * @@ -357,6 +357,29 @@ TEST_F(PacedSenderTest, Padding) { send_bucket_->Process(); } +TEST_F(PacedSenderTest, NoPaddingBeforeNormalPacket) { + send_bucket_->SetEstimatedBitrate(kTargetBitrateBps); + send_bucket_->SetSendBitrateLimits(kTargetBitrateBps, kTargetBitrateBps); + + EXPECT_CALL(callback_, TimeToSendPadding(_, _)).Times(0); + send_bucket_->Process(); + clock_.AdvanceTimeMilliseconds(send_bucket_->TimeUntilNextProcess()); + + send_bucket_->Process(); + clock_.AdvanceTimeMilliseconds(send_bucket_->TimeUntilNextProcess()); + + uint32_t ssrc = 12345; + uint16_t sequence_number = 1234; + int64_t capture_time_ms = 56789; + + SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++, + capture_time_ms, 250, false); + EXPECT_CALL(callback_, TimeToSendPadding(250, _)) + .Times(1) + .WillOnce(Return(250)); + send_bucket_->Process(); +} + TEST_F(PacedSenderTest, VerifyPaddingUpToBitrate) { uint32_t ssrc = 12345; uint16_t sequence_number = 1234; @@ -364,7 +387,7 @@ TEST_F(PacedSenderTest, VerifyPaddingUpToBitrate) { const int kTimeStep = 5; const int64_t kBitrateWindow = 100; send_bucket_->SetEstimatedBitrate(kTargetBitrateBps); - send_bucket_->SetAllocatedSendBitrate(kTargetBitrateBps, kTargetBitrateBps); + send_bucket_->SetSendBitrateLimits(kTargetBitrateBps, kTargetBitrateBps); int64_t start_time = clock_.TimeInMilliseconds(); while (clock_.TimeInMilliseconds() - start_time < kBitrateWindow) { @@ -392,7 +415,10 @@ TEST_F(PacedSenderTest, VerifyAverageBitrateVaryingMediaPayload) { send_bucket_.reset(new PacedSender(&clock_, &callback)); send_bucket_->SetProbingEnabled(false); send_bucket_->SetEstimatedBitrate(kTargetBitrateBps); - send_bucket_->SetAllocatedSendBitrate(kTargetBitrateBps, kTargetBitrateBps); + + send_bucket_->SetSendBitrateLimits( + 0 /*allocated_bitrate_bps*/, + kTargetBitrateBps * 2 /* max_padding_bitrate_bps */); int64_t start_time = clock_.TimeInMilliseconds(); size_t media_bytes = 0; @@ -806,7 +832,7 @@ TEST_F(PacedSenderTest, PaddingOveruse) { send_bucket_->Process(); send_bucket_->SetEstimatedBitrate(60000); - send_bucket_->SetAllocatedSendBitrate(60000, 0); + send_bucket_->SetSendBitrateLimits(60000, 0); SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++, clock_.TimeInMilliseconds(), kPacketSize, false); @@ -815,7 +841,7 @@ TEST_F(PacedSenderTest, PaddingOveruse) { // Add 30kbit padding. When increasing budget, media budget will increase from // negative (overuse) while padding budget will increase from 0. clock_.AdvanceTimeMilliseconds(5); - send_bucket_->SetAllocatedSendBitrate(60000, 30000); + send_bucket_->SetSendBitrateLimits(60000, 30000); SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++, clock_.TimeInMilliseconds(), kPacketSize, false); @@ -875,7 +901,7 @@ TEST_F(PacedSenderTest, ProbeClusterId) { uint16_t sequence_number = 1234; const size_t kPacketSize = 1200; - send_bucket_->SetAllocatedSendBitrate(kTargetBitrateBps, kTargetBitrateBps); + send_bucket_->SetSendBitrateLimits(kTargetBitrateBps, kTargetBitrateBps); send_bucket_->SetProbingEnabled(true); for (int i = 0; i < 11; ++i) { send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc, diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc index 186e14e1f3..c8c122a97b 100644 --- a/webrtc/video/end_to_end_tests.cc +++ b/webrtc/video/end_to_end_tests.cc @@ -2952,6 +2952,7 @@ void EndToEndTest::TestRtpStatePreservation(bool use_rtx) { static const int32_t kMaxTimestampGap = kDefaultTimeoutMs * 90; auto timestamp_it = last_observed_timestamp_.find(ssrc); if (timestamp_it == last_observed_timestamp_.end()) { + EXPECT_FALSE(only_padding); last_observed_timestamp_[ssrc] = timestamp; } else { // Verify timestamps are reasonably close. diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index b9f306d2dd..df4b295546 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -13,6 +13,7 @@ #include #include #include +#include #include #include "webrtc/base/checks.h" @@ -36,6 +37,7 @@ class RtcpIntraFrameObserver; class TransportFeedbackObserver; static const int kMinSendSidePacketHistorySize = 600; +static const int kEncoderTimeOutMs = 2000; namespace { @@ -344,6 +346,27 @@ VideoCodec VideoEncoderConfigToVideoCodec(const VideoEncoderConfig& config, return video_codec; } +int CalulcateMaxPadBitrateBps(const VideoEncoderConfig& config, + bool pad_to_min_bitrate) { + int pad_up_to_bitrate_bps = 0; + // Calculate max padding bitrate for a multi layer codec. + if (config.streams.size() > 1) { + // Pad to min bitrate of the highest layer. + pad_up_to_bitrate_bps = + config.streams[config.streams.size() - 1].min_bitrate_bps; + // Add target_bitrate_bps of the lower layers. + for (size_t i = 0; i < config.streams.size() - 1; ++i) + pad_up_to_bitrate_bps += config.streams[i].target_bitrate_bps; + } else if (pad_to_min_bitrate) { + pad_up_to_bitrate_bps = config.streams[0].min_bitrate_bps; + } + + pad_up_to_bitrate_bps = + std::max(pad_up_to_bitrate_bps, config.min_transmit_bitrate_bps); + + return pad_up_to_bitrate_bps; +} + } // namespace namespace internal { @@ -372,6 +395,7 @@ VideoSendStream::VideoSendStream( encoder_thread_(EncoderThreadFunction, this, "EncoderThread"), encoder_wakeup_event_(false, false), stop_encoder_thread_(0), + send_stream_registered_as_observer_(false), overuse_detector_( Clock::GetRealTimeClock(), GetCpuOveruseOptions(config.encoder_settings.full_overuse_time), @@ -535,46 +559,52 @@ void VideoSendStream::EncoderProcess() { config_.encoder_settings.internal_source)); while (true) { - encoder_wakeup_event_.Wait(rtc::Event::kForever); + // Wake up every kEncodeCheckForActivityPeriodMs to check if the encoder is + // active. If not, deregister as BitrateAllocatorObserver. + const int kEncodeCheckForActivityPeriodMs = 1000; + encoder_wakeup_event_.Wait(kEncodeCheckForActivityPeriodMs); if (rtc::AtomicOps::AcquireLoad(&stop_encoder_thread_)) break; - rtc::Optional encoder_settings; + bool change_settings = false; { rtc::CritScope lock(&encoder_settings_crit_); if (pending_encoder_settings_) { - encoder_settings = pending_encoder_settings_; - pending_encoder_settings_ = rtc::Optional(); + std::swap(current_encoder_settings_, pending_encoder_settings_); + pending_encoder_settings_.reset(); + change_settings = true; } } - if (encoder_settings) { - encoder_settings->video_codec.startBitrate = + if (change_settings) { + current_encoder_settings_->video_codec.startBitrate = bitrate_allocator_->AddObserver( - this, encoder_settings->video_codec.minBitrate * 1000, - encoder_settings->video_codec.maxBitrate * 1000, + this, current_encoder_settings_->video_codec.minBitrate * 1000, + current_encoder_settings_->video_codec.maxBitrate * 1000, + CalulcateMaxPadBitrateBps(current_encoder_settings_->config, + config_.suspend_below_min_bitrate), !config_.suspend_below_min_bitrate) / 1000; + send_stream_registered_as_observer_ = true; - payload_router_.SetSendStreams(encoder_settings->streams); - vie_encoder_.SetEncoder(encoder_settings->video_codec, - encoder_settings->min_transmit_bitrate_bps, + payload_router_.SetSendStreams(current_encoder_settings_->config.streams); + vie_encoder_.SetEncoder(current_encoder_settings_->video_codec, payload_router_.MaxPayloadLength()); // Clear stats for disabled layers. - for (size_t i = encoder_settings->streams.size(); + for (size_t i = current_encoder_settings_->config.streams.size(); i < config_.rtp.ssrcs.size(); ++i) { stats_proxy_.OnInactiveSsrc(config_.rtp.ssrcs[i]); } size_t number_of_temporal_layers = - encoder_settings->streams.back() + current_encoder_settings_->config.streams.back() .temporal_layer_thresholds_bps.size() + 1; protection_bitrate_calculator_.SetEncodingData( - encoder_settings->video_codec.startBitrate * 1000, - encoder_settings->video_codec.width, - encoder_settings->video_codec.height, - encoder_settings->video_codec.maxFramerate, number_of_temporal_layers, - payload_router_.MaxPayloadLength()); + current_encoder_settings_->video_codec.startBitrate * 1000, + current_encoder_settings_->video_codec.width, + current_encoder_settings_->video_codec.height, + current_encoder_settings_->video_codec.maxFramerate, + number_of_temporal_layers, payload_router_.MaxPayloadLength()); // We might've gotten new settings while configuring the encoder settings, // restart from the top to see if that's the case before trying to encode @@ -592,6 +622,29 @@ void VideoSendStream::EncoderProcess() { } vie_encoder_.EncodeVideoFrame(frame); } + + // Check if the encoder has produced anything the last kEncoderTimeOutMs. + // If not, deregister as BitrateAllocatorObserver. + if (send_stream_registered_as_observer_ && + vie_encoder_.time_of_last_frame_activity_ms() < + rtc::TimeMillis() - kEncoderTimeOutMs) { + // The encoder has timed out. + LOG_F(LS_INFO) << "Encoder timed out."; + bitrate_allocator_->RemoveObserver(this); + send_stream_registered_as_observer_ = false; + } + if (!send_stream_registered_as_observer_ && + vie_encoder_.time_of_last_frame_activity_ms() > + rtc::TimeMillis() - kEncoderTimeOutMs) { + LOG_F(LS_INFO) << "Encoder is active."; + bitrate_allocator_->AddObserver( + this, current_encoder_settings_->video_codec.minBitrate * 1000, + current_encoder_settings_->video_codec.maxBitrate * 1000, + CalulcateMaxPadBitrateBps(current_encoder_settings_->config, + config_.suspend_below_min_bitrate), + !config_.suspend_below_min_bitrate); + send_stream_registered_as_observer_ = true; + } } vie_encoder_.DeRegisterExternalEncoder(config_.encoder_settings.payload_type); } @@ -606,8 +659,7 @@ void VideoSendStream::ReconfigureVideoEncoder( config_.encoder_settings.payload_type); { rtc::CritScope lock(&encoder_settings_crit_); - pending_encoder_settings_ = rtc::Optional( - {video_codec, config.min_transmit_bitrate_bps, config.streams}); + pending_encoder_settings_.reset(new EncoderSettings({video_codec, config})); } encoder_wakeup_event_.Set(); } @@ -789,10 +841,6 @@ void VideoSendStream::SignalNetworkState(NetworkState state) { } } -int VideoSendStream::GetPaddingNeededBps() const { - return vie_encoder_.GetPaddingNeededBps(); -} - void VideoSendStream::OnBitrateUpdated(uint32_t bitrate_bps, uint8_t fraction_loss, int64_t rtt) { diff --git a/webrtc/video/video_send_stream.h b/webrtc/video/video_send_stream.h index cb8363aeab..79f99a4bff 100644 --- a/webrtc/video/video_send_stream.h +++ b/webrtc/video/video_send_stream.h @@ -51,7 +51,7 @@ class VideoSendStream : public webrtc::VideoSendStream, public webrtc::CpuOveruseObserver, public webrtc::BitrateAllocatorObserver, public webrtc::VCMProtectionCallback, - protected webrtc::EncodedImageCallback { + public EncodedImageCallback { public: VideoSendStream(int num_cpu_cores, ProcessThread* module_process_thread, @@ -102,8 +102,7 @@ class VideoSendStream : public webrtc::VideoSendStream, private: struct EncoderSettings { VideoCodec video_codec; - int min_transmit_bitrate_bps; - std::vector streams; + VideoEncoderConfig config; }; // Implements EncodedImageCallback. The implementation routes encoded frames @@ -137,8 +136,11 @@ class VideoSendStream : public webrtc::VideoSendStream, rtc::Event encoder_wakeup_event_; volatile int stop_encoder_thread_; rtc::CriticalSection encoder_settings_crit_; - rtc::Optional pending_encoder_settings_ + std::unique_ptr pending_encoder_settings_ GUARDED_BY(encoder_settings_crit_); + // Only used on the encoder thread. + bool send_stream_registered_as_observer_; + std::unique_ptr current_encoder_settings_; OveruseFrameDetector overuse_detector_; ViEEncoder vie_encoder_; diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc index d104f966cd..6a77530f71 100644 --- a/webrtc/video/video_send_stream_tests.cc +++ b/webrtc/video/video_send_stream_tests.cc @@ -955,6 +955,8 @@ TEST_F(VideoSendStreamTest, SuspendBelowMinBitrate) { RunBaseTest(&test); } +// This test that padding stops being send after a while if the Camera stops +// producing video frames and that padding resumes if the camera restarts. TEST_F(VideoSendStreamTest, NoPaddingWhenVideoIsMuted) { class NoPaddingWhenVideoIsMuted : public test::SendTest { public: @@ -969,40 +971,37 @@ TEST_F(VideoSendStreamTest, NoPaddingWhenVideoIsMuted) { Action OnSendRtp(const uint8_t* packet, size_t length) override { rtc::CritScope lock(&crit_); last_packet_time_ms_ = clock_->TimeInMilliseconds(); - capturer_->Stop(); + + RTPHeader header; + parser_->Parse(packet, length, &header); + const bool only_padding = + header.headerLength + header.paddingLength == length; + + if (test_state_ == kBeforeStopCapture) { + capturer_->Stop(); + test_state_ = kWaitingForPadding; + } else if (test_state_ == kWaitingForPadding && only_padding) { + test_state_ = kWaitingForNoPackets; + } else if (test_state_ == kWaitingForPaddingAfterCameraRestart && + only_padding) { + observation_complete_.Set(); + } return SEND_PACKET; } Action OnSendRtcp(const uint8_t* packet, size_t length) override { rtc::CritScope lock(&crit_); - const int kVideoMutedThresholdMs = 10000; - if (last_packet_time_ms_ > 0 && - clock_->TimeInMilliseconds() - last_packet_time_ms_ > - kVideoMutedThresholdMs) - observation_complete_.Set(); - // Receive statistics reporting having lost 50% of the packets. - FakeReceiveStatistics receive_stats(kVideoSendSsrcs[0], 1, 1, 0); - RTCPSender rtcp_sender(false, Clock::GetRealTimeClock(), &receive_stats, - nullptr, nullptr, transport_adapter_.get()); - - rtcp_sender.SetRTCPStatus(RtcpMode::kReducedSize); - rtcp_sender.SetRemoteSSRC(kVideoSendSsrcs[0]); - - RTCPSender::FeedbackState feedback_state; - - EXPECT_EQ(0, rtcp_sender.SendRTCP(feedback_state, kRtcpRr)); + const int kNoPacketsThresholdMs = 2000; + if (test_state_ == kWaitingForNoPackets && + (last_packet_time_ms_ > 0 && + clock_->TimeInMilliseconds() - last_packet_time_ms_ > + kNoPacketsThresholdMs)) { + capturer_->Start(); + test_state_ = kWaitingForPaddingAfterCameraRestart; + } return SEND_PACKET; } - test::PacketTransport* CreateReceiveTransport() override { - test::PacketTransport* transport = new test::PacketTransport( - nullptr, this, test::PacketTransport::kReceiver, - FakeNetworkPipe::Config()); - transport_adapter_.reset(new internal::TransportAdapter(transport)); - transport_adapter_->Enable(); - return transport; - } - size_t GetNumVideoStreams() const override { return 3; } void OnFrameGeneratorCapturerCreated( @@ -1016,6 +1015,14 @@ TEST_F(VideoSendStreamTest, NoPaddingWhenVideoIsMuted) { << "Timed out while waiting for RTP packets to stop being sent."; } + enum TestState { + kBeforeStopCapture, + kWaitingForPadding, + kWaitingForNoPackets, + kWaitingForPaddingAfterCameraRestart + }; + + TestState test_state_ = kBeforeStopCapture; Clock* const clock_; std::unique_ptr transport_adapter_; rtc::CriticalSection crit_; diff --git a/webrtc/video/vie_encoder.cc b/webrtc/video/vie_encoder.cc index 529b38f5b8..8704947c2e 100644 --- a/webrtc/video/vie_encoder.cc +++ b/webrtc/video/vie_encoder.cc @@ -28,8 +28,6 @@ namespace webrtc { -static const float kStopPaddingThresholdMs = 2000; - ViEEncoder::ViEEncoder(uint32_t number_of_cores, ProcessThread* module_process_thread, SendStatisticsProxy* stats_proxy, @@ -43,7 +41,6 @@ ViEEncoder::ViEEncoder(uint32_t number_of_cores, overuse_detector_(overuse_detector), time_of_last_frame_activity_ms_(0), encoder_config_(), - min_transmit_bitrate_bps_(0), last_observed_bitrate_bps_(0), encoder_paused_(true), encoder_paused_and_dropped_frame_(false), @@ -88,7 +85,6 @@ int32_t ViEEncoder::DeRegisterExternalEncoder(uint8_t pl_type) { } void ViEEncoder::SetEncoder(const webrtc::VideoCodec& video_codec, - int min_transmit_bitrate_bps, size_t max_data_payload_length) { // Setting target width and height for VPM. RTC_CHECK_EQ(VPM_OK, @@ -100,7 +96,6 @@ void ViEEncoder::SetEncoder(const webrtc::VideoCodec& video_codec, { rtc::CritScope lock(&data_cs_); encoder_config_ = video_codec; - min_transmit_bitrate_bps_ = min_transmit_bitrate_bps; } bool success = video_sender_.RegisterSendCodec( @@ -129,57 +124,6 @@ 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; - VideoCodec send_codec; - bool video_is_suspended; - { - rtc::CritScope lock(&data_cs_); - bool send_padding = encoder_config_.numberOfSimulcastStreams > 1 || - video_suspended_ || min_transmit_bitrate_bps_ > 0; - if (!send_padding) - return 0; - time_of_last_frame_activity_ms = time_of_last_frame_activity_ms_; - min_transmit_bitrate_bps = min_transmit_bitrate_bps_; - send_codec = encoder_config_; - video_is_suspended = video_suspended_; - } - - // 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. - int pad_up_to_bitrate_bps = 0; - if (send_codec.numberOfSimulcastStreams == 0) { - pad_up_to_bitrate_bps = send_codec.minBitrate * 1000; - } else { - SimulcastStream* stream_configs = send_codec.simulcastStream; - pad_up_to_bitrate_bps = - stream_configs[send_codec.numberOfSimulcastStreams - 1].minBitrate * - 1000; - for (int i = 0; i < send_codec.numberOfSimulcastStreams - 1; ++i) { - pad_up_to_bitrate_bps += stream_configs[i].targetBitrate * 1000; - } - } - - // Disable padding if only sending one stream and video isn't suspended and - // min-transmit bitrate isn't used (applied later). - if (!video_is_suspended && send_codec.numberOfSimulcastStreams <= 1) - pad_up_to_bitrate_bps = 0; - - // The amount of padding should decay to zero if no frames are being - // captured/encoded unless a min-transmit bitrate is used. - int64_t now_ms = rtc::TimeMillis(); - if (now_ms - time_of_last_frame_activity_ms > kStopPaddingThresholdMs) - pad_up_to_bitrate_bps = 0; - - // Pad up to min bitrate. - if (pad_up_to_bitrate_bps < min_transmit_bitrate_bps) - pad_up_to_bitrate_bps = min_transmit_bitrate_bps; - - return pad_up_to_bitrate_bps; -} - bool ViEEncoder::EncoderPaused() const { // Pause video if paused by caller or as long as the network is down or the // pacer queue has grown too large in buffered mode. @@ -258,6 +202,11 @@ void ViEEncoder::SendKeyFrame() { video_sender_.IntraFrameRequest(0); } +int64_t ViEEncoder::time_of_last_frame_activity_ms() { + rtc::CritScope lock(&data_cs_); + return time_of_last_frame_activity_ms_; +} + void ViEEncoder::OnSetRates(uint32_t bitrate_bps, int framerate) { if (stats_proxy_) stats_proxy_->OnSetRates(bitrate_bps, framerate); diff --git a/webrtc/video/vie_encoder.h b/webrtc/video/vie_encoder.h index 91497df6e9..621cf0b020 100644 --- a/webrtc/video/vie_encoder.h +++ b/webrtc/video/vie_encoder.h @@ -60,7 +60,6 @@ class ViEEncoder : public VideoEncoderRateObserver, SendStatisticsProxy* stats_proxy, OveruseFrameDetector* overuse_detector, EncodedImageCallback* sink); - ~ViEEncoder(); vcm::VideoSender* video_sender(); @@ -78,12 +77,15 @@ class ViEEncoder : public VideoEncoderRateObserver, bool internal_source); int32_t DeRegisterExternalEncoder(uint8_t pl_type); void SetEncoder(const VideoCodec& video_codec, - int min_transmit_bitrate_bps, size_t max_data_payload_length); void EncodeVideoFrame(const VideoFrame& video_frame); void SendKeyFrame(); + // Returns the time when the encoder last received an input frame or produced + // an encoded frame. + int64_t time_of_last_frame_activity_ms(); + // Implements VideoEncoderRateObserver. // TODO(perkj): Refactor VideoEncoderRateObserver. This is only used for // stats. The stats should be set in VideoSendStream instead. @@ -106,10 +108,6 @@ 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, uint8_t fraction_lost, int64_t round_trip_time_ms); @@ -135,7 +133,6 @@ class ViEEncoder : public VideoEncoderRateObserver, // padding. int64_t time_of_last_frame_activity_ms_ GUARDED_BY(data_cs_); VideoCodec encoder_config_ GUARDED_BY(data_cs_); - int min_transmit_bitrate_bps_ GUARDED_BY(data_cs_); uint32_t last_observed_bitrate_bps_ GUARDED_BY(data_cs_); bool encoder_paused_ GUARDED_BY(data_cs_); bool encoder_paused_and_dropped_frame_ GUARDED_BY(data_cs_);