From 57c21f9b44621b6415d81d353923eb3975276703 Mon Sep 17 00:00:00 2001 From: perkj Date: Fri, 17 Jun 2016 07:27:16 -0700 Subject: [PATCH] Remove ViEEncoder::Pause / Start This cl change so that VideoSendStream::Start adds the stream as a BitrateObserver and VideoSendStream::Stop removes the stream as observer. That also means that start will trigger a VideoEncoder::SetRate call with the most recent bitrate estimate. VideoSendStream::Stop will trigger a VideoEncoder::SetRate with bitrate = 0. BUG=webrtc:5687 b/28636240 Review-Url: https://codereview.webrtc.org/2070343002 Cr-Commit-Position: refs/heads/master@{#13192} --- webrtc/call/bitrate_allocator.cc | 23 +- webrtc/call/bitrate_allocator.h | 15 +- webrtc/call/bitrate_allocator_unittest.cc | 278 +++++++++--------- .../modules/video_coding/generic_encoder.cc | 6 - .../protection_bitrate_calculator.cc | 2 +- .../protection_bitrate_calculator.h | 1 - .../protection_bitrate_calculator_unittest.cc | 11 + .../modules/video_coding/video_coding_impl.h | 2 +- webrtc/modules/video_coding/video_sender.cc | 20 +- webrtc/video/end_to_end_tests.cc | 1 + webrtc/video/video_send_stream.cc | 81 +++-- webrtc/video/video_send_stream.h | 15 +- webrtc/video/video_send_stream_tests.cc | 73 +++++ webrtc/video/vie_encoder.cc | 34 +-- webrtc/video/vie_encoder.h | 8 +- 15 files changed, 342 insertions(+), 228 deletions(-) diff --git a/webrtc/call/bitrate_allocator.cc b/webrtc/call/bitrate_allocator.cc index e20c34e05d..e82123a8b0 100644 --- a/webrtc/call/bitrate_allocator.cc +++ b/webrtc/call/bitrate_allocator.cc @@ -53,11 +53,11 @@ void BitrateAllocator::OnNetworkChanged(uint32_t target_bitrate_bps, last_allocation_ = allocation; } -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) { +void 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); @@ -89,7 +89,6 @@ int BitrateAllocator::AddObserver(BitrateAllocatorObserver* observer, UpdateAllocationLimits(); last_allocation_ = allocation; - return allocation[observer]; } void BitrateAllocator::UpdateAllocationLimits() { @@ -121,6 +120,18 @@ void BitrateAllocator::RemoveObserver(BitrateAllocatorObserver* observer) { UpdateAllocationLimits(); } +int BitrateAllocator::GetStartBitrate(BitrateAllocatorObserver* observer) { + rtc::CritScope lock(&crit_sect_); + const auto& it = last_allocation_.find(observer); + if (it != last_allocation_.end()) + return it->second; + + // This is a new observer that has not yet been started. Assume that if it is + // added, all observers would split the available bitrate evenly. + return last_non_zero_bitrate_bps_ / + static_cast((bitrate_observer_configs_.size() + 1)); +} + BitrateAllocator::ObserverConfigList::iterator BitrateAllocator::FindObserverConfig( const BitrateAllocatorObserver* observer) { diff --git a/webrtc/call/bitrate_allocator.h b/webrtc/call/bitrate_allocator.h index 06e8b305d9..58d1c72fb9 100644 --- a/webrtc/call/bitrate_allocator.h +++ b/webrtc/call/bitrate_allocator.h @@ -68,21 +68,24 @@ class BitrateAllocator { // |enforce_min_bitrate| = 'true' will allocate at least |min_bitrate_bps| for // this observer, even if the BWE is too low, 'false' will allocate 0 to // the observer if BWE doesn't allow |min_bitrate_bps|. - // Returns initial bitrate allocated for |observer|. // Note that |observer|->OnBitrateUpdated() will be called within the scope of // this method with the current rtt, fraction_loss and available bitrate and // that the bitrate in OnBitrateUpdated will be zero if the |observer| is // currently not allowed to send data. - int AddObserver(BitrateAllocatorObserver* observer, - uint32_t min_bitrate_bps, - uint32_t max_bitrate_bps, - uint32_t pad_up_bitrate_bps, - bool enforce_min_bitrate); + void 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 // allocation. void RemoveObserver(BitrateAllocatorObserver* observer); + // Returns initial bitrate allocated for |observer|. If |observer| is not in + // the list of added observers, a best guess is returned. + int GetStartBitrate(BitrateAllocatorObserver* observer); + private: // Note: All bitrates for member variables and methods are in bps. struct ObserverConfig { diff --git a/webrtc/call/bitrate_allocator_unittest.cc b/webrtc/call/bitrate_allocator_unittest.cc index 74c635d0c5..5b1410f61d 100644 --- a/webrtc/call/bitrate_allocator_unittest.cc +++ b/webrtc/call/bitrate_allocator_unittest.cc @@ -63,29 +63,29 @@ TEST_F(BitrateAllocatorTest, UpdatingBitrateObserver) { EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(kMinSendBitrateBps, kPadUpToBitrateBps)); - int start_bitrate = allocator_->AddObserver( - &bitrate_observer, kMinSendBitrateBps, 1500000, kPadUpToBitrateBps, true); - EXPECT_EQ(300000, start_bitrate); + allocator_->AddObserver(&bitrate_observer, kMinSendBitrateBps, 1500000, + kPadUpToBitrateBps, true); + EXPECT_EQ(300000, allocator_->GetStartBitrate(&bitrate_observer)); allocator_->OnNetworkChanged(200000, 0, 0); - EXPECT_EQ(200000u, bitrate_observer.last_bitrate_bps_); + 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); - EXPECT_EQ(3000000u, bitrate_observer.last_bitrate_bps_); + EXPECT_EQ(3000000, allocator_->GetStartBitrate(&bitrate_observer)); // 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); + allocator_->AddObserver(&bitrate_observer, kMinSendBitrateBps, 4000000, 0, + true); + EXPECT_EQ(4000000, allocator_->GetStartBitrate(&bitrate_observer)); EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(kMinSendBitrateBps, 0)); - start_bitrate = allocator_->AddObserver(&bitrate_observer, kMinSendBitrateBps, - 1500000, 0, true); - EXPECT_EQ(3000000, start_bitrate); + allocator_->AddObserver(&bitrate_observer, kMinSendBitrateBps, 1500000, 0, + true); + EXPECT_EQ(3000000, allocator_->GetStartBitrate(&bitrate_observer)); EXPECT_EQ(3000000u, bitrate_observer.last_bitrate_bps_); allocator_->OnNetworkChanged(1500000, 0, 0); EXPECT_EQ(1500000u, bitrate_observer.last_bitrate_bps_); @@ -95,42 +95,44 @@ 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, 0, true); - EXPECT_EQ(300000, start_bitrate); - EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(100000 + 200000, 0)); - start_bitrate = + EXPECT_EQ(300000, allocator_->GetStartBitrate(&bitrate_observer_1)); + EXPECT_CALL(limit_observer_, + OnAllocationLimitsChanged(100000 + 200000, 0)); allocator_->AddObserver(&bitrate_observer_2, 200000, 300000, 0, true); - EXPECT_EQ(200000, start_bitrate); + EXPECT_EQ(200000, allocator_->GetStartBitrate(&bitrate_observer_2)); - // 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_bps_); - EXPECT_EQ(0, bitrate_observer_1.last_fraction_loss_); - 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_ms_); + // 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_bps_); + EXPECT_EQ(0, bitrate_observer_1.last_fraction_loss_); + 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_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_bps_); - EXPECT_EQ(200000u + kBitrateToShare / 2, - bitrate_observer_2.last_bitrate_bps_); + // 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_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_bps_); - EXPECT_EQ(600000u, 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_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_bps_); - EXPECT_EQ(0u, 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_bps_); + EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_bps_); } TEST_F(BitrateAllocatorTest, RemoveObserverTriggersLimitObserver) { @@ -165,20 +167,19 @@ TEST_F(BitrateAllocatorTestNoEnforceMin, OneBitrateObserver) { // 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, 0, false); - EXPECT_EQ(300000, start_bitrate); + EXPECT_EQ(300000, allocator_->GetStartBitrate(&bitrate_observer_1)); - // High BWE. - allocator_->OnNetworkChanged(150000, 0, 0); - EXPECT_EQ(150000u, bitrate_observer_1.last_bitrate_bps_); + // High BWE. + allocator_->OnNetworkChanged(150000, 0, 0); + EXPECT_EQ(150000u, bitrate_observer_1.last_bitrate_bps_); - // Low BWE. - allocator_->OnNetworkChanged(10000, 0, 0); - EXPECT_EQ(0u, bitrate_observer_1.last_bitrate_bps_); + // Low BWE. + 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); + EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(0, 0)); + allocator_->RemoveObserver(&bitrate_observer_1); } TEST_F(BitrateAllocatorTestNoEnforceMin, ThreeBitrateObservers) { @@ -186,18 +187,15 @@ TEST_F(BitrateAllocatorTestNoEnforceMin, ThreeBitrateObservers) { TestBitrateObserver bitrate_observer_2; 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, 0, false); - EXPECT_EQ(300000, start_bitrate); + allocator_->AddObserver(&bitrate_observer_1, 100000, 400000, 0, false); + EXPECT_EQ(300000, allocator_->GetStartBitrate(&bitrate_observer_1)); - start_bitrate = - allocator_->AddObserver(&bitrate_observer_2, 200000, 400000, 0, false); - EXPECT_EQ(200000, start_bitrate); + allocator_->AddObserver(&bitrate_observer_2, 200000, 400000, 0, false); + EXPECT_EQ(200000, allocator_->GetStartBitrate(&bitrate_observer_2)); EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_bps_); - start_bitrate = - allocator_->AddObserver(&bitrate_observer_3, 300000, 400000, 0, false); - EXPECT_EQ(0, start_bitrate); + allocator_->AddObserver(&bitrate_observer_3, 300000, 400000, 0, false); + EXPECT_EQ(0, allocator_->GetStartBitrate(&bitrate_observer_3)); EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_bps_); EXPECT_EQ(200000u, bitrate_observer_2.last_bitrate_bps_); @@ -249,38 +247,37 @@ TEST_F(BitrateAllocatorTest, ThreeBitrateObserversLowBweEnforceMin) { TestBitrateObserver bitrate_observer_1; TestBitrateObserver bitrate_observer_2; TestBitrateObserver bitrate_observer_3; - int start_bitrate = - allocator_->AddObserver(&bitrate_observer_1, 100000, 400000, 0, true); - EXPECT_EQ(300000, start_bitrate); - start_bitrate = - allocator_->AddObserver(&bitrate_observer_2, 200000, 400000, 0, true); - EXPECT_EQ(200000, start_bitrate); + allocator_->AddObserver(&bitrate_observer_1, 100000, 400000, 0, true); + EXPECT_EQ(300000, allocator_->GetStartBitrate(&bitrate_observer_1)); + + allocator_->AddObserver(&bitrate_observer_2, 200000, 400000, 0, true); + EXPECT_EQ(200000, allocator_->GetStartBitrate(&bitrate_observer_2)); EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_bps_); - start_bitrate = 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_)); + EXPECT_EQ(300000, allocator_->GetStartBitrate(&bitrate_observer_3)); + EXPECT_EQ(100000, static_cast(bitrate_observer_1.last_bitrate_bps_)); + EXPECT_EQ(200000, static_cast(bitrate_observer_2.last_bitrate_bps_)); - // 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_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. + // 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_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); - allocator_->RemoveObserver(&bitrate_observer_3); + allocator_->RemoveObserver(&bitrate_observer_1); + allocator_->RemoveObserver(&bitrate_observer_2); + allocator_->RemoveObserver(&bitrate_observer_3); } 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, 0, true); - EXPECT_EQ(300000, start_bitrate); + + allocator_->AddObserver(&bitrate_observer_1, 50000, 400000, 0, true); + EXPECT_EQ(300000, allocator_->GetStartBitrate(&bitrate_observer_1)); // Set network down, ie, no available bitrate. allocator_->OnNetworkChanged(0, 0, 0); @@ -290,12 +287,11 @@ TEST_F(BitrateAllocatorTest, AddObserverWhileNetworkDown) { 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, 0, 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. - EXPECT_EQ(300000 / 2, start_bitrate); + EXPECT_EQ(300000 / 2, allocator_->GetStartBitrate(&bitrate_observer_2)); EXPECT_EQ(0u, bitrate_observer_1.last_bitrate_bps_); EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_bps_); @@ -307,100 +303,96 @@ TEST_F(BitrateAllocatorTest, AddObserverWhileNetworkDown) { TEST_F(BitrateAllocatorTest, MixedEnforecedConfigs) { TestBitrateObserver enforced_observer; - int start_bitrate = allocator_->AddObserver(&enforced_observer, 6000, 30000, 0, true); - EXPECT_EQ(60000, start_bitrate); + EXPECT_EQ(60000, allocator_->GetStartBitrate(&enforced_observer)); - TestBitrateObserver not_enforced_observer; - start_bitrate = + TestBitrateObserver not_enforced_observer; allocator_->AddObserver(¬_enforced_observer, 30000, 2500000, 0, false); - EXPECT_EQ(270000, start_bitrate); - EXPECT_EQ(30000u, enforced_observer.last_bitrate_bps_); + EXPECT_EQ(270000, allocator_->GetStartBitrate(¬_enforced_observer)); + 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(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(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(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(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(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(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_->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); + allocator_->RemoveObserver(&enforced_observer); + allocator_->RemoveObserver(¬_enforced_observer); } TEST_F(BitrateAllocatorTest, AvoidToggleAbsolute) { TestBitrateObserver observer; - int start_bitrate = allocator_->AddObserver(&observer, 30000, 300000, 0, false); - EXPECT_EQ(300000, start_bitrate); + EXPECT_EQ(300000, allocator_->GetStartBitrate(&observer)); - allocator_->OnNetworkChanged(30000, 0, 50); - EXPECT_EQ(30000u, observer.last_bitrate_bps_); + 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(20000, 0, 50); + EXPECT_EQ(0u, observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(30000, 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(49000, 0, 50); + EXPECT_EQ(0u, observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(50000, 0, 50); - EXPECT_EQ(50000u, 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_->OnNetworkChanged(30000, 0, 50); + EXPECT_EQ(30000u, observer.last_bitrate_bps_); - allocator_->RemoveObserver(&observer); + allocator_->RemoveObserver(&observer); } TEST_F(BitrateAllocatorTest, AvoidTogglePercent) { TestBitrateObserver observer; - int start_bitrate = allocator_->AddObserver(&observer, 300000, 600000, 0, false); - EXPECT_EQ(300000, start_bitrate); + EXPECT_EQ(300000, allocator_->GetStartBitrate(&observer)); - allocator_->OnNetworkChanged(300000, 0, 50); - EXPECT_EQ(300000u, observer.last_bitrate_bps_); + 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(200000, 0, 50); + EXPECT_EQ(0u, observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(300000, 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(329000, 0, 50); + EXPECT_EQ(0u, observer.last_bitrate_bps_); - allocator_->OnNetworkChanged(330000, 0, 50); - EXPECT_EQ(330000u, 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_->OnNetworkChanged(300000, 0, 50); + EXPECT_EQ(300000u, observer.last_bitrate_bps_); - allocator_->RemoveObserver(&observer); + allocator_->RemoveObserver(&observer); } } // namespace webrtc diff --git a/webrtc/modules/video_coding/generic_encoder.cc b/webrtc/modules/video_coding/generic_encoder.cc index abc6369a00..176a14f078 100644 --- a/webrtc/modules/video_coding/generic_encoder.cc +++ b/webrtc/modules/video_coding/generic_encoder.cc @@ -44,12 +44,6 @@ int32_t VCMGenericEncoder::InitEncode(const VideoCodec* settings, int32_t number_of_cores, size_t max_payload_size) { TRACE_EVENT0("webrtc", "VCMGenericEncoder::InitEncode"); - { - rtc::CritScope lock(¶ms_lock_); - encoder_params_.target_bitrate = settings->startBitrate * 1000; - encoder_params_.input_frame_rate = settings->maxFramerate; - } - is_screenshare_ = settings->mode == VideoCodecMode::kScreensharing; if (encoder_->InitEncode(settings, number_of_cores, max_payload_size) != 0) { LOG(LS_ERROR) << "Failed to initialize the encoder associated with " diff --git a/webrtc/modules/video_coding/protection_bitrate_calculator.cc b/webrtc/modules/video_coding/protection_bitrate_calculator.cc index c92c3c445c..ba93442f39 100644 --- a/webrtc/modules/video_coding/protection_bitrate_calculator.cc +++ b/webrtc/modules/video_coding/protection_bitrate_calculator.cc @@ -8,7 +8,7 @@ * be found in the AUTHORS file in the root of the source tree. */ -#include +#include "webrtc/modules/video_coding/protection_bitrate_calculator.h" namespace webrtc { diff --git a/webrtc/modules/video_coding/protection_bitrate_calculator.h b/webrtc/modules/video_coding/protection_bitrate_calculator.h index cdbab102d4..01edc6c607 100644 --- a/webrtc/modules/video_coding/protection_bitrate_calculator.h +++ b/webrtc/modules/video_coding/protection_bitrate_calculator.h @@ -59,7 +59,6 @@ class ProtectionBitrateCalculator { int actual_framerate, uint8_t fraction_lost, int64_t round_trip_time_ms); - // Informs of encoded output. void UpdateWithEncodedData(const EncodedImage& encoded_image); diff --git a/webrtc/modules/video_coding/protection_bitrate_calculator_unittest.cc b/webrtc/modules/video_coding/protection_bitrate_calculator_unittest.cc index 667dd6a359..7855bbd942 100644 --- a/webrtc/modules/video_coding/protection_bitrate_calculator_unittest.cc +++ b/webrtc/modules/video_coding/protection_bitrate_calculator_unittest.cc @@ -86,4 +86,15 @@ TEST_F(ProtectionBitrateCalculatorTest, ProtectsUsingNackBitrate) { EXPECT_EQ(kMaxBitrateBps / 2, target_bitrate); } +TEST_F(ProtectionBitrateCalculatorTest, NoProtection) { + static const uint32_t kMaxBitrateBps = 130000; + + media_opt_.SetProtectionMethod(false /*enable_fec*/, false /* enable_nack */); + media_opt_.SetEncodingData(kCodecBitrateBps, 640, 480, 30, 1, 1000); + + uint32_t target_bitrate = + media_opt_.SetTargetRates(kMaxBitrateBps, 30, 128, 100); + EXPECT_EQ(kMaxBitrateBps, target_bitrate); +} + } // namespace webrtc diff --git a/webrtc/modules/video_coding/video_coding_impl.h b/webrtc/modules/video_coding/video_coding_impl.h index 2aabf05859..6f48a23dbb 100644 --- a/webrtc/modules/video_coding/video_coding_impl.h +++ b/webrtc/modules/video_coding/video_coding_impl.h @@ -96,7 +96,7 @@ class VideoSender : public Module { void Process() override; private: - void SetEncoderParameters(EncoderParameters params) + void SetEncoderParameters(EncoderParameters params, bool has_internal_source) EXCLUSIVE_LOCKS_REQUIRED(encoder_crit_); Clock* const clock_; diff --git a/webrtc/modules/video_coding/video_sender.cc b/webrtc/modules/video_coding/video_sender.cc index e2c0d1f8a9..d7d8110921 100644 --- a/webrtc/modules/video_coding/video_sender.cc +++ b/webrtc/modules/video_coding/video_sender.cc @@ -219,19 +219,23 @@ int32_t VideoSender::SetChannelParameters(uint32_t target_bitrate, if (encoder_has_internal_source) { rtc::CritScope cs(&encoder_crit_); if (_encoder) { - SetEncoderParameters(encoder_params); + SetEncoderParameters(encoder_params, encoder_has_internal_source); } } return VCM_OK; } -void VideoSender::SetEncoderParameters(EncoderParameters params) { +void VideoSender::SetEncoderParameters(EncoderParameters params, + bool has_internal_source) { // |target_bitrate == 0 | means that the network is down or the send pacer is - // full. - // TODO(perkj): Consider setting |target_bitrate| == 0 to the encoders. - // Especially if |encoder_has_internal_source_ | == true. - if (params.target_bitrate == 0) + // full. We currently only report this if the encoder has an internal source. + // If the encoder does not have an internal source, higher levels are expected + // to not call AddVideoFrame. We do this since its unclear how current + // encoder implementations behave when given a zero target bitrate. + // TODO(perkj): Make sure all known encoder implementations handle zero + // target bitrate and remove this check. + if (!has_internal_source && params.target_bitrate == 0) return; if (params.input_frame_rate == 0) { @@ -258,15 +262,17 @@ int32_t VideoSender::AddVideoFrame(const VideoFrame& videoFrame, const CodecSpecificInfo* codecSpecificInfo) { EncoderParameters encoder_params; std::vector next_frame_types; + bool encoder_has_internal_source = false; { rtc::CritScope lock(¶ms_crit_); encoder_params = encoder_params_; next_frame_types = next_frame_types_; + encoder_has_internal_source = encoder_has_internal_source_; } rtc::CritScope lock(&encoder_crit_); if (_encoder == nullptr) return VCM_UNINITIALIZED; - SetEncoderParameters(encoder_params); + SetEncoderParameters(encoder_params, encoder_has_internal_source); if (_mediaOpt.DropFrame()) { LOG(LS_VERBOSE) << "Drop Frame " << "target bitrate " << encoder_params.target_bitrate diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc index c8c122a97b..fb7cc491d9 100644 --- a/webrtc/video/end_to_end_tests.cc +++ b/webrtc/video/end_to_end_tests.cc @@ -2447,6 +2447,7 @@ TEST_F(EndToEndTest, ReportsSetEncoderRates) { std::vector* receive_configs, VideoEncoderConfig* encoder_config) override { send_config->encoder_settings.encoder = this; + RTC_DCHECK_EQ(1u, encoder_config->streams.size()); } int32_t SetRates(uint32_t new_target_bitrate, uint32_t framerate) override { diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index 22f735466b..01c2fc836c 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -396,7 +396,7 @@ VideoSendStream::VideoSendStream( encoder_thread_(EncoderThreadFunction, this, "EncoderThread"), encoder_wakeup_event_(false, false), stop_encoder_thread_(0), - send_stream_registered_as_observer_(false), + state_(State::kStopped), overuse_detector_( Clock::GetRealTimeClock(), GetCpuOveruseOptions(config.encoder_settings.full_overuse_time), @@ -440,7 +440,6 @@ VideoSendStream::VideoSendStream( RTC_DCHECK(congestion_controller_); RTC_DCHECK(remb_); - // RTP/RTCP initialization. for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) { module_process_thread_->RegisterModule(rtp_rtcp); @@ -489,6 +488,7 @@ VideoSendStream::VideoSendStream( module_process_thread_->RegisterModule(&overuse_detector_); + encoder_thread_checker_.DetachFromThread(); encoder_thread_.Start(); encoder_thread_.SetPriority(rtc::kHighPriority); } @@ -530,17 +530,23 @@ void VideoSendStream::Start() { return; TRACE_EVENT_INSTANT0("webrtc", "VideoSendStream::Start"); payload_router_.set_active(true); - // Was not already started, trigger a keyframe. - vie_encoder_.SendKeyFrame(); - vie_encoder_.Start(); + { + rtc::CritScope lock(&encoder_settings_crit_); + pending_state_change_ = rtc::Optional(State::kStarted); + } + encoder_wakeup_event_.Set(); } void VideoSendStream::Stop() { if (!payload_router_.active()) return; TRACE_EVENT_INSTANT0("webrtc", "VideoSendStream::Stop"); - vie_encoder_.Pause(); payload_router_.set_active(false); + { + rtc::CritScope lock(&encoder_settings_crit_); + pending_state_change_ = rtc::Optional(State::kStopped); + } + encoder_wakeup_event_.Set(); } VideoCaptureInput* VideoSendStream::Input() { @@ -558,7 +564,7 @@ void VideoSendStream::EncoderProcess() { config_.encoder_settings.encoder, config_.encoder_settings.payload_type, config_.encoder_settings.internal_source)); - + RTC_DCHECK_RUN_ON(&encoder_thread_checker_); while (true) { // Wake up every kEncodeCheckForActivityPeriodMs to check if the encoder is // active. If not, deregister as BitrateAllocatorObserver. @@ -567,25 +573,21 @@ void VideoSendStream::EncoderProcess() { if (rtc::AtomicOps::AcquireLoad(&stop_encoder_thread_)) break; bool change_settings = false; + rtc::Optional pending_state_change; { rtc::CritScope lock(&encoder_settings_crit_); if (pending_encoder_settings_) { std::swap(current_encoder_settings_, pending_encoder_settings_); pending_encoder_settings_.reset(); change_settings = true; + } else if (pending_state_change_) { + swap(pending_state_change, pending_state_change_); } } if (change_settings) { - current_encoder_settings_->video_codec.startBitrate = - 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) / - 1000; - send_stream_registered_as_observer_ = true; - + current_encoder_settings_->video_codec.startBitrate = std::max( + bitrate_allocator_->GetStartBitrate(this) / 1000, + static_cast(current_encoder_settings_->video_codec.minBitrate)); payload_router_.SetSendStreams(current_encoder_settings_->config.streams); vie_encoder_.SetEncoder(current_encoder_settings_->video_codec, payload_router_.MaxPayloadLength()); @@ -614,27 +616,39 @@ void VideoSendStream::EncoderProcess() { continue; } - VideoFrame frame; - if (input_.GetVideoFrame(&frame)) { - // TODO(perkj): |pre_encode_callback| is only used by tests. Tests should - // register as a sink to the VideoSource instead. - if (config_.pre_encode_callback) { - config_.pre_encode_callback->OnFrame(frame); + if (pending_state_change) { + if (*pending_state_change == State::kStarted && + state_ == State::kStopped) { + 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); + vie_encoder_.SendKeyFrame(); + state_ = State::kStarted; + LOG_F(LS_INFO) << "Encoder started."; + } else if (*pending_state_change == State::kStopped) { + bitrate_allocator_->RemoveObserver(this); + vie_encoder_.OnBitrateUpdated(0, 0, 0); + state_ = State::kStopped; + LOG_F(LS_INFO) << "Encoder stopped."; } - vie_encoder_.EncodeVideoFrame(frame); + encoder_wakeup_event_.Set(); + continue; } // Check if the encoder has produced anything the last kEncoderTimeOutMs. // If not, deregister as BitrateAllocatorObserver. - if (send_stream_registered_as_observer_ && + if (state_ == State::kStarted && 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; + state_ = State::kEncoderTimedOut; } - if (!send_stream_registered_as_observer_ && + if (state_ == State::kEncoderTimedOut && vie_encoder_.time_of_last_frame_activity_ms() > rtc::TimeMillis() - kEncoderTimeOutMs) { LOG_F(LS_INFO) << "Encoder is active."; @@ -644,7 +658,17 @@ void VideoSendStream::EncoderProcess() { CalulcateMaxPadBitrateBps(current_encoder_settings_->config, config_.suspend_below_min_bitrate), !config_.suspend_below_min_bitrate); - send_stream_registered_as_observer_ = true; + state_ = State::kStarted; + } + + VideoFrame frame; + if (input_.GetVideoFrame(&frame)) { + // TODO(perkj): |pre_encode_callback| is only used by tests. Tests should + // register as a sink to the VideoSource instead. + if (config_.pre_encode_callback) { + config_.pre_encode_callback->OnFrame(frame); + } + vie_encoder_.EncodeVideoFrame(frame); } } vie_encoder_.DeRegisterExternalEncoder(config_.encoder_settings.payload_type); @@ -850,7 +874,6 @@ void VideoSendStream::OnBitrateUpdated(uint32_t bitrate_bps, // protection overhead. uint32_t encoder_target_rate = protection_bitrate_calculator_.SetTargetRates( bitrate_bps, stats_proxy_.GetSendFrameRate(), fraction_loss, rtt); - vie_encoder_.OnBitrateUpdated(encoder_target_rate, fraction_loss, rtt); } diff --git a/webrtc/video/video_send_stream.h b/webrtc/video/video_send_stream.h index 79f99a4bff..1c66c1fe33 100644 --- a/webrtc/video/video_send_stream.h +++ b/webrtc/video/video_send_stream.h @@ -138,9 +138,20 @@ class VideoSendStream : public webrtc::VideoSendStream, rtc::CriticalSection encoder_settings_crit_; std::unique_ptr pending_encoder_settings_ GUARDED_BY(encoder_settings_crit_); + + enum class State { + kStopped, // VideoSendStream::Start has not yet been called. + kStarted, // VideoSendStream::Start has been called. + // VideoSendStream::Start has been called but the encoder have timed out. + kEncoderTimedOut, + }; + rtc::Optional pending_state_change_ GUARDED_BY(encoder_settings_crit_); + // Only used on the encoder thread. - bool send_stream_registered_as_observer_; - std::unique_ptr current_encoder_settings_; + rtc::ThreadChecker encoder_thread_checker_; + State state_ ACCESS_ON(&encoder_thread_checker_); + std::unique_ptr current_encoder_settings_ + ACCESS_ON(&encoder_thread_checker_); 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 6a77530f71..8adf24717c 100644 --- a/webrtc/video/video_send_stream_tests.cc +++ b/webrtc/video/video_send_stream_tests.cc @@ -1193,6 +1193,79 @@ TEST_F(VideoSendStreamTest, CanReconfigureToUseStartBitrateAbovePreviousMax) { DestroyStreams(); } +// This test that if the encoder use an internal source, VideoEncoder::SetRates +// will be called with zero bitrate during initialization and that +// VideoSendStream::Stop also triggers VideoEncoder::SetRates Start to be called +// with zero bitrate. +TEST_F(VideoSendStreamTest, VideoSendStreamStopSetEncoderRateToZero) { + class StartStopBitrateObserver : public test::FakeEncoder { + public: + StartStopBitrateObserver() + : FakeEncoder(Clock::GetRealTimeClock()), + encoder_init_(false, false), + bitrate_changed_(false, false), + bitrate_kbps_(0) {} + int32_t InitEncode(const VideoCodec* config, + int32_t number_of_cores, + size_t max_payload_size) override { + rtc::CritScope lock(&crit_); + bitrate_kbps_ = config->startBitrate; + encoder_init_.Set(); + return FakeEncoder::InitEncode(config, number_of_cores, max_payload_size); + } + + int32_t SetRates(uint32_t new_target_bitrate, uint32_t framerate) override { + rtc::CritScope lock(&crit_); + bitrate_kbps_ = new_target_bitrate; + bitrate_changed_.Set(); + return FakeEncoder::SetRates(new_target_bitrate, framerate); + } + + int GetBitrateKbps() const { + rtc::CritScope lock(&crit_); + return bitrate_kbps_; + } + + bool WaitForEncoderInit() { + return encoder_init_.Wait(VideoSendStreamTest::kDefaultTimeoutMs); + } + bool WaitBitrateChanged() { + return bitrate_changed_.Wait(VideoSendStreamTest::kDefaultTimeoutMs); + } + + private: + rtc::CriticalSection crit_; + rtc::Event encoder_init_; + rtc::Event bitrate_changed_; + int bitrate_kbps_ GUARDED_BY(crit_); + }; + + CreateSenderCall(Call::Config()); + + test::NullTransport transport; + CreateSendConfig(1, 0, &transport); + + StartStopBitrateObserver encoder; + video_send_config_.encoder_settings.encoder = &encoder; + video_send_config_.encoder_settings.internal_source = true; + + CreateVideoStreams(); + + EXPECT_TRUE(encoder.WaitForEncoderInit()); + EXPECT_GT(encoder.GetBitrateKbps(), 0); + video_send_stream_->Start(); + EXPECT_TRUE(encoder.WaitBitrateChanged()); + EXPECT_GT(encoder.GetBitrateKbps(), 0); + video_send_stream_->Stop(); + EXPECT_TRUE(encoder.WaitBitrateChanged()); + EXPECT_EQ(0, encoder.GetBitrateKbps()); + video_send_stream_->Start(); + EXPECT_TRUE(encoder.WaitBitrateChanged()); + EXPECT_GT(encoder.GetBitrateKbps(), 0); + + DestroyStreams(); +} + TEST_F(VideoSendStreamTest, CapturesTextureAndVideoFrames) { class FrameObserver : public rtc::VideoSinkInterface { public: diff --git a/webrtc/video/vie_encoder.cc b/webrtc/video/vie_encoder.cc index 8704947c2e..94cf2858ae 100644 --- a/webrtc/video/vie_encoder.cc +++ b/webrtc/video/vie_encoder.cc @@ -10,9 +10,8 @@ #include "webrtc/video/vie_encoder.h" -#include - #include +#include #include "webrtc/base/checks.h" #include "webrtc/base/logging.h" @@ -39,10 +38,9 @@ ViEEncoder::ViEEncoder(uint32_t number_of_cores, video_sender_(Clock::GetRealTimeClock(), this, this, this), stats_proxy_(stats_proxy), overuse_detector_(overuse_detector), - time_of_last_frame_activity_ms_(0), + time_of_last_frame_activity_ms_(std::numeric_limits::max()), encoder_config_(), last_observed_bitrate_bps_(0), - encoder_paused_(true), encoder_paused_and_dropped_frame_(false), module_process_thread_(module_process_thread), has_received_sli_(false), @@ -62,16 +60,6 @@ ViEEncoder::~ViEEncoder() { module_process_thread_->DeRegisterModule(&video_sender_); } -void ViEEncoder::Pause() { - rtc::CritScope lock(&data_cs_); - encoder_paused_ = true; -} - -void ViEEncoder::Start() { - rtc::CritScope lock(&data_cs_); - encoder_paused_ = false; -} - int32_t ViEEncoder::RegisterExternalEncoder(webrtc::VideoEncoder* encoder, uint8_t pl_type, bool internal_source) { @@ -90,9 +78,6 @@ void ViEEncoder::SetEncoder(const webrtc::VideoCodec& video_codec, RTC_CHECK_EQ(VPM_OK, vp_->SetTargetResolution(video_codec.width, video_codec.height, video_codec.maxFramerate)); - - // Cache codec before calling AddBitrateObserver (which calls OnBitrateUpdated - // that makes use of the number of simulcast streams configured). { rtc::CritScope lock(&data_cs_); encoder_config_ = video_codec; @@ -101,6 +86,7 @@ void ViEEncoder::SetEncoder(const webrtc::VideoCodec& video_codec, bool success = video_sender_.RegisterSendCodec( &video_codec, number_of_cores_, static_cast(max_data_payload_length)) == VCM_OK; + if (!success) { LOG(LS_ERROR) << "Failed to configure encoder."; RTC_DCHECK(success); @@ -127,9 +113,9 @@ void ViEEncoder::SetEncoder(const webrtc::VideoCodec& video_codec, 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. - // If the pacer queue has grown to large or the network is down, + // If the pacer queue has grown too large or the network is down, // last_observed_bitrate_bps_ will be 0. - return encoder_paused_ || video_suspended_ || last_observed_bitrate_bps_ == 0; + return video_suspended_ || last_observed_bitrate_bps_ == 0; } void ViEEncoder::TraceFrameDropStart() { @@ -270,6 +256,16 @@ void ViEEncoder::OnBitrateUpdated(uint32_t bitrate_bps, last_observed_bitrate_bps_ = bitrate_bps; video_suspension_changed = video_suspended_ != video_is_suspended; video_suspended_ = video_is_suspended; + // Set |time_of_last_frame_activity_ms_| to now if this is the first time + // the encoder is supposed to produce encoded frames. + // TODO(perkj): Remove this hack. It is here to avoid a race that the + // encoder report that it has timed out before it has processed the first + // frame. + if (last_observed_bitrate_bps_ != 0 && + time_of_last_frame_activity_ms_ == + std::numeric_limits::max()) { + time_of_last_frame_activity_ms_ = rtc::TimeMillis(); + } } if (stats_proxy_ && video_suspension_changed) { diff --git a/webrtc/video/vie_encoder.h b/webrtc/video/vie_encoder.h index 621cf0b020..098d67f359 100644 --- a/webrtc/video/vie_encoder.h +++ b/webrtc/video/vie_encoder.h @@ -47,8 +47,7 @@ class VideoEncoder; // 3. Call RegisterExternalEncoder if available. // 4. Call SetEncoder with the codec settings and the object that shall receive // the encoded bit stream. -// 5. Call Start. -// 6. For each available raw video frame call EncodeVideoFrame. +// 5. For each available raw video frame call EncodeVideoFrame. class ViEEncoder : public VideoEncoderRateObserver, public EncodedImageCallback, public VCMSendStatisticsCallback { @@ -67,10 +66,6 @@ class ViEEncoder : public VideoEncoderRateObserver, // Returns the id of the owning channel. int Owner() const; - void Start(); - // Drops incoming packets before they get to the encoder. - void Pause(); - // Codec settings. int32_t RegisterExternalEncoder(VideoEncoder* encoder, uint8_t pl_type, @@ -134,7 +129,6 @@ class ViEEncoder : public VideoEncoderRateObserver, int64_t time_of_last_frame_activity_ms_ GUARDED_BY(data_cs_); VideoCodec encoder_config_ 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_); ProcessThread* module_process_thread_;