diff --git a/talk/media/webrtc/fakewebrtcvideoengine.h b/talk/media/webrtc/fakewebrtcvideoengine.h index 8f1cfb033a..24a1cbb3a0 100644 --- a/talk/media/webrtc/fakewebrtcvideoengine.h +++ b/talk/media/webrtc/fakewebrtcvideoengine.h @@ -912,6 +912,7 @@ class FakeWebRtcVideoEngine WEBRTC_STUB(DeregisterObserver, (const int)); // webrtc::ViENetwork + WEBRTC_VOID_STUB(SetBitrateConfig, (int, int, int, int)); WEBRTC_VOID_FUNC(SetNetworkTransmissionState, (const int channel, const bool is_transmitting)) { WEBRTC_ASSERT_CHANNEL(channel); diff --git a/talk/media/webrtc/webrtcvideoengine2.cc b/talk/media/webrtc/webrtcvideoengine2.cc index ae14c64c32..af73702c14 100644 --- a/talk/media/webrtc/webrtcvideoengine2.cc +++ b/talk/media/webrtc/webrtcvideoengine2.cc @@ -169,6 +169,10 @@ static const int kDefaultRtcpReceiverReportSsrc = 1; const char kH264CodecName[] = "H264"; +const int kMinBandwidthBps = 30000; +const int kStartBandwidthBps = 300000; +const int kMaxBandwidthBps = 2000000; + static bool FindFirstMatchingCodec(const std::vector& codecs, const VideoCodec& requested_codec, VideoCodec* matching_codec) { @@ -630,7 +634,9 @@ WebRtcVideoChannel2::WebRtcVideoChannel2( if (voice_engine != NULL) { config.voice_engine = voice_engine->voe()->engine(); } - + config.bitrate_config.min_bitrate_bps = kMinBandwidthBps; + config.bitrate_config.start_bitrate_bps = kStartBandwidthBps; + config.bitrate_config.max_bitrate_bps = kMaxBandwidthBps; call_.reset(call_factory->CreateCall(config)); rtcp_receiver_report_ssrc_ = kDefaultRtcpReceiverReportSsrc; @@ -763,6 +769,8 @@ bool WebRtcVideoChannel2::SetSendCodecs(const std::vector& codecs) { it->second->SetCodec(supported_codecs.front()); } + // TODO(holmer): Changing the codec parameters shouldn't necessarily mean that + // we change the min/max of bandwidth estimation. Reevaluate this. VideoCodec codec = supported_codecs.front().codec; int bitrate_kbps; if (codec.GetParam(kCodecParamMinBitrate, &bitrate_kbps) && diff --git a/talk/media/webrtc/webrtcvideoengine2_unittest.cc b/talk/media/webrtc/webrtcvideoengine2_unittest.cc index f07f28104f..e7560f18fa 100644 --- a/talk/media/webrtc/webrtcvideoengine2_unittest.cc +++ b/talk/media/webrtc/webrtcvideoengine2_unittest.cc @@ -321,7 +321,7 @@ webrtc::Call::Stats FakeCall::GetStats() const { void FakeCall::SetBitrateConfig( const webrtc::Call::Config::BitrateConfig& bitrate_config) { - config_.stream_bitrates = bitrate_config; + config_.bitrate_config = bitrate_config; } void FakeCall::SignalNetworkState(webrtc::Call::NetworkState state) { @@ -1012,11 +1012,11 @@ class WebRtcVideoChannel2Test : public WebRtcVideoEngine2Test, EXPECT_TRUE(channel_->SetSendCodecs(codecs)); EXPECT_EQ(expected_min_bitrate_bps, - fake_call_->GetConfig().stream_bitrates.min_bitrate_bps); + fake_call_->GetConfig().bitrate_config.min_bitrate_bps); EXPECT_EQ(expected_start_bitrate_bps, - fake_call_->GetConfig().stream_bitrates.start_bitrate_bps); + fake_call_->GetConfig().bitrate_config.start_bitrate_bps); EXPECT_EQ(expected_max_bitrate_bps, - fake_call_->GetConfig().stream_bitrates.max_bitrate_bps); + fake_call_->GetConfig().bitrate_config.max_bitrate_bps); } void TestSetSendRtpHeaderExtensions(const std::string& cricket_ext, @@ -1858,19 +1858,19 @@ TEST_F(WebRtcVideoChannel2Test, SetSendCodecsShouldWorkForBitrates("100", 100000, "150", 150000, "200", 200000); channel_->SetMaxSendBandwidth(300000); - EXPECT_EQ(100000, fake_call_->GetConfig().stream_bitrates.min_bitrate_bps) + EXPECT_EQ(100000, fake_call_->GetConfig().bitrate_config.min_bitrate_bps) << "Setting max bitrate should keep previous min bitrate."; - EXPECT_EQ(-1, fake_call_->GetConfig().stream_bitrates.start_bitrate_bps) + EXPECT_EQ(-1, fake_call_->GetConfig().bitrate_config.start_bitrate_bps) << "Setting max bitrate should not reset start bitrate."; - EXPECT_EQ(300000, fake_call_->GetConfig().stream_bitrates.max_bitrate_bps); + EXPECT_EQ(300000, fake_call_->GetConfig().bitrate_config.max_bitrate_bps); } TEST_F(WebRtcVideoChannel2Test, SetMaxSendBandwidthShouldBeRemovable) { channel_->SetMaxSendBandwidth(300000); - EXPECT_EQ(300000, fake_call_->GetConfig().stream_bitrates.max_bitrate_bps); + EXPECT_EQ(300000, fake_call_->GetConfig().bitrate_config.max_bitrate_bps); // <= 0 means disable (infinite) max bitrate. channel_->SetMaxSendBandwidth(0); - EXPECT_EQ(-1, fake_call_->GetConfig().stream_bitrates.max_bitrate_bps) + EXPECT_EQ(-1, fake_call_->GetConfig().bitrate_config.max_bitrate_bps) << "Setting zero max bitrate did not reset start bitrate."; } diff --git a/webrtc/call.h b/webrtc/call.h index 99f8b27a52..f5b7fed545 100644 --- a/webrtc/call.h +++ b/webrtc/call.h @@ -82,9 +82,6 @@ class Call { // Bitrate config used until valid bitrate estimates are calculated. Also // used to cap total bitrate used. - // Note: This is currently set only for video and is per-stream rather of - // for the entire link. - // TODO(pbos): Set start bitrate for entire Call. struct BitrateConfig { BitrateConfig() : min_bitrate_bps(0), @@ -93,7 +90,7 @@ class Call { int min_bitrate_bps; int start_bitrate_bps; int max_bitrate_bps; - } stream_bitrates; + } bitrate_config; }; struct Stats { diff --git a/webrtc/modules/bitrate_controller/bitrate_allocator.cc b/webrtc/modules/bitrate_controller/bitrate_allocator.cc index cd904207da..fc83e060a8 100644 --- a/webrtc/modules/bitrate_controller/bitrate_allocator.cc +++ b/webrtc/modules/bitrate_controller/bitrate_allocator.cc @@ -18,61 +18,77 @@ namespace webrtc { +// Allow packets to be transmitted in up to 2 times max video bitrate if the +// bandwidth estimate allows it. +const int kTransmissionMaxBitrateMultiplier = 2; +const int kDefaultBitrateBps = 300000; + BitrateAllocator::BitrateAllocator() : crit_sect_(CriticalSectionWrapper::CreateCriticalSection()), bitrate_observers_(), - enforce_min_bitrate_(true) { + enforce_min_bitrate_(true), + last_bitrate_bps_(kDefaultBitrateBps), + last_fraction_loss_(0), + last_rtt_(0) { } -BitrateAllocator::~BitrateAllocator() { - for (auto& kv : bitrate_observers_) - delete kv.second; -} void BitrateAllocator::OnNetworkChanged(uint32_t bitrate, uint8_t fraction_loss, int64_t rtt) { CriticalSectionScoped lock(crit_sect_.get()); - // Sanity check. + last_bitrate_bps_ = bitrate; + last_fraction_loss_ = fraction_loss; + last_rtt_ = rtt; + ObserverBitrateMap allocation = AllocateBitrates(); + for (const auto& kv : allocation) + kv.first->OnNetworkChanged(kv.second, last_fraction_loss_, last_rtt_); +} + +BitrateAllocator::ObserverBitrateMap BitrateAllocator::AllocateBitrates() { if (bitrate_observers_.empty()) - return; + return ObserverBitrateMap(); uint32_t sum_min_bitrates = 0; - BitrateObserverConfList::iterator it; - for (auto& kv : bitrate_observers_) - sum_min_bitrates += kv.second->min_bitrate_; - if (bitrate <= sum_min_bitrates) - return LowRateAllocation(bitrate, fraction_loss, rtt, sum_min_bitrates); + for (const auto& observer : bitrate_observers_) + sum_min_bitrates += observer.second.min_bitrate_; + if (last_bitrate_bps_ <= sum_min_bitrates) + return LowRateAllocation(last_bitrate_bps_); else - return NormalRateAllocation(bitrate, fraction_loss, rtt, sum_min_bitrates); + return NormalRateAllocation(last_bitrate_bps_, sum_min_bitrates); } int BitrateAllocator::AddBitrateObserver(BitrateObserver* observer, - uint32_t start_bitrate, - uint32_t min_bitrate, - uint32_t max_bitrate) { + uint32_t start_bitrate_bps, + uint32_t min_bitrate_bps, + uint32_t max_bitrate_bps, + int* new_observer_bitrate_bps) { CriticalSectionScoped lock(crit_sect_.get()); BitrateObserverConfList::iterator it = FindObserverConfigurationPair(observer); - int new_bwe_candidate_bps = -1; + // Allow the max bitrate to be exceeded for FEC and retransmissions. + // TODO(holmer): We have to get rid of this hack as it makes it difficult to + // properly allocate bitrate. The allocator should instead distribute any + // extra bitrate after all streams have maxed out. + max_bitrate_bps *= kTransmissionMaxBitrateMultiplier; + int new_bwe_candidate_bps = 0; if (it != bitrate_observers_.end()) { // Update current configuration. - it->second->start_bitrate_ = start_bitrate; - it->second->min_bitrate_ = min_bitrate; - it->second->max_bitrate_ = max_bitrate; + it->second.start_bitrate_ = start_bitrate_bps; + it->second.min_bitrate_ = min_bitrate_bps; + it->second.max_bitrate_ = max_bitrate_bps; // Set the send-side bandwidth to the max of the sum of start bitrates and // the current estimate, so that if the user wants to immediately use more // bandwidth, that can be enforced. - new_bwe_candidate_bps = 0; - for (auto& kv : bitrate_observers_) - new_bwe_candidate_bps += kv.second->start_bitrate_; + for (const auto& observer : bitrate_observers_) + new_bwe_candidate_bps += observer.second.start_bitrate_; } else { // Add new settings. bitrate_observers_.push_back(BitrateObserverConfiguration( - observer, - new BitrateConfiguration(start_bitrate, min_bitrate, max_bitrate))); + observer, BitrateConfiguration(start_bitrate_bps, min_bitrate_bps, + max_bitrate_bps))); bitrate_observers_modified_ = true; // TODO(andresp): This is a ugly way to set start bitrate. @@ -81,9 +97,19 @@ int BitrateAllocator::AddBitrateObserver(BitrateObserver* observer, // you can only have one start bitrate, once we have our first estimate we // will adapt from there. if (bitrate_observers_.size() == 1) - new_bwe_candidate_bps = start_bitrate; + new_bwe_candidate_bps = start_bitrate_bps; } - return new_bwe_candidate_bps; + + last_bitrate_bps_ = std::max(new_bwe_candidate_bps, last_bitrate_bps_); + + ObserverBitrateMap allocation = AllocateBitrates(); + *new_observer_bitrate_bps = 0; + for (auto& kv : allocation) { + kv.first->OnNetworkChanged(kv.second, last_fraction_loss_, last_rtt_); + if (kv.first == observer) + *new_observer_bitrate_bps = kv.second; + } + return last_bitrate_bps_; } void BitrateAllocator::RemoveBitrateObserver(BitrateObserver* observer) { @@ -91,7 +117,6 @@ void BitrateAllocator::RemoveBitrateObserver(BitrateObserver* observer) { BitrateObserverConfList::iterator it = FindObserverConfigurationPair(observer); if (it != bitrate_observers_.end()) { - delete it->second; bitrate_observers_.erase(it); bitrate_observers_modified_ = true; } @@ -103,32 +128,19 @@ void BitrateAllocator::GetMinMaxBitrateSumBps(int* min_bitrate_sum_bps, *max_bitrate_sum_bps = 0; CriticalSectionScoped lock(crit_sect_.get()); - BitrateObserverConfList::const_iterator it; - for (it = bitrate_observers_.begin(); it != bitrate_observers_.end(); ++it) { - *min_bitrate_sum_bps += it->second->min_bitrate_; - *max_bitrate_sum_bps += it->second->max_bitrate_; - } - if (*max_bitrate_sum_bps == 0) { - // No max configured use 1Gbit/s. - *max_bitrate_sum_bps = 1000000000; - } - // TODO(holmer): Enforcing a min bitrate should be per stream, allowing some - // streams to auto-mute while others keep sending. - if (!enforce_min_bitrate_) { - // If not enforcing min bitrate, allow the bandwidth estimation to - // go as low as 10 kbps. - *min_bitrate_sum_bps = std::min(*min_bitrate_sum_bps, 10000); + for (const auto& observer : bitrate_observers_) { + *min_bitrate_sum_bps += observer.second.min_bitrate_; + *max_bitrate_sum_bps += observer.second.max_bitrate_; } } BitrateAllocator::BitrateObserverConfList::iterator BitrateAllocator::FindObserverConfigurationPair( const BitrateObserver* observer) { - BitrateObserverConfList::iterator it = bitrate_observers_.begin(); - for (; it != bitrate_observers_.end(); ++it) { - if (it->first == observer) { + for (auto it = bitrate_observers_.begin(); it != bitrate_observers_.end(); + ++it) { + if (it->first == observer) return it; - } } return bitrate_observers_.end(); } @@ -138,26 +150,25 @@ void BitrateAllocator::EnforceMinBitrate(bool enforce_min_bitrate) { enforce_min_bitrate_ = enforce_min_bitrate; } -void BitrateAllocator::NormalRateAllocation(uint32_t bitrate, - uint8_t fraction_loss, - int64_t rtt, - uint32_t sum_min_bitrates) { +BitrateAllocator::ObserverBitrateMap BitrateAllocator::NormalRateAllocation( + uint32_t bitrate, + uint32_t sum_min_bitrates) { uint32_t number_of_observers = bitrate_observers_.size(); uint32_t bitrate_per_observer = (bitrate - sum_min_bitrates) / number_of_observers; // Use map to sort list based on max bitrate. ObserverSortingMap list_max_bitrates; - BitrateObserverConfList::iterator it; - for (it = bitrate_observers_.begin(); it != bitrate_observers_.end(); ++it) { - list_max_bitrates.insert(std::pair( - it->second->max_bitrate_, - new ObserverConfiguration(it->first, it->second->min_bitrate_))); + for (const auto& observer : bitrate_observers_) { + list_max_bitrates.insert(std::pair( + observer.second.max_bitrate_, + ObserverConfiguration(observer.first, observer.second.min_bitrate_))); } + ObserverBitrateMap allocation; ObserverSortingMap::iterator max_it = list_max_bitrates.begin(); while (max_it != list_max_bitrates.end()) { number_of_observers--; uint32_t observer_allowance = - max_it->second->min_bitrate_ + bitrate_per_observer; + max_it->second.min_bitrate_ + bitrate_per_observer; if (max_it->first < observer_allowance) { // We have more than enough for this observer. // Carry the remainder forward. @@ -165,41 +176,35 @@ void BitrateAllocator::NormalRateAllocation(uint32_t bitrate, if (number_of_observers != 0) { bitrate_per_observer += remainder / number_of_observers; } - max_it->second->observer_->OnNetworkChanged(max_it->first, fraction_loss, - rtt); + allocation[max_it->second.observer_] = max_it->first; } else { - max_it->second->observer_->OnNetworkChanged(observer_allowance, - fraction_loss, rtt); + allocation[max_it->second.observer_] = observer_allowance; } - delete max_it->second; list_max_bitrates.erase(max_it); // Prepare next iteration. max_it = list_max_bitrates.begin(); } + return allocation; } -void BitrateAllocator::LowRateAllocation(uint32_t bitrate, - uint8_t fraction_loss, - int64_t rtt, - uint32_t sum_min_bitrates) { +BitrateAllocator::ObserverBitrateMap BitrateAllocator::LowRateAllocation( + uint32_t bitrate) { + ObserverBitrateMap allocation; if (enforce_min_bitrate_) { // Min bitrate to all observers. - BitrateObserverConfList::iterator it; - for (it = bitrate_observers_.begin(); it != bitrate_observers_.end(); - ++it) { - it->first->OnNetworkChanged(it->second->min_bitrate_, fraction_loss, rtt); - } + for (const auto& observer : bitrate_observers_) + allocation[observer.first] = observer.second.min_bitrate_; } else { // Allocate up to |min_bitrate_| to one observer at a time, until // |bitrate| is depleted. uint32_t remainder = bitrate; - BitrateObserverConfList::iterator it; - for (it = bitrate_observers_.begin(); it != bitrate_observers_.end(); - ++it) { - uint32_t allocation = std::min(remainder, it->second->min_bitrate_); - it->first->OnNetworkChanged(allocation, fraction_loss, rtt); - remainder -= allocation; + for (const auto& observer : bitrate_observers_) { + uint32_t allocated_bitrate = + std::min(remainder, observer.second.min_bitrate_); + allocation[observer.first] = allocated_bitrate; + remainder -= allocated_bitrate; } } + return allocation; } } // namespace webrtc diff --git a/webrtc/modules/bitrate_controller/bitrate_allocator_unittest.cc b/webrtc/modules/bitrate_controller/bitrate_allocator_unittest.cc index f89a29dc7e..b69247e861 100644 --- a/webrtc/modules/bitrate_controller/bitrate_allocator_unittest.cc +++ b/webrtc/modules/bitrate_controller/bitrate_allocator_unittest.cc @@ -36,7 +36,9 @@ class TestBitrateObserver : public BitrateObserver { class BitrateAllocatorTest : public ::testing::Test { protected: - BitrateAllocatorTest() : allocator_(new BitrateAllocator()) {} + BitrateAllocatorTest() : allocator_(new BitrateAllocator()) { + allocator_->OnNetworkChanged(300000u, 0, 0); + } ~BitrateAllocatorTest() {} rtc::scoped_ptr allocator_; @@ -44,15 +46,22 @@ class BitrateAllocatorTest : public ::testing::Test { TEST_F(BitrateAllocatorTest, UpdatingBitrateObserver) { TestBitrateObserver bitrate_observer; - allocator_->AddBitrateObserver(&bitrate_observer, 200000, 100000, 1500000); + int start_bitrate; + allocator_->AddBitrateObserver(&bitrate_observer, 200000, 100000, 1500000, + &start_bitrate); + EXPECT_EQ(300000, start_bitrate); allocator_->OnNetworkChanged(200000, 0, 0); EXPECT_EQ(200000u, bitrate_observer.last_bitrate_); - allocator_->AddBitrateObserver(&bitrate_observer, 1500000, 100000, 1500000); + allocator_->AddBitrateObserver(&bitrate_observer, 1500000, 100000, 1500000, + &start_bitrate); + EXPECT_EQ(1500000, start_bitrate); allocator_->OnNetworkChanged(1500000, 0, 0); EXPECT_EQ(1500000u, bitrate_observer.last_bitrate_); - allocator_->AddBitrateObserver(&bitrate_observer, 500000, 100000, 1500000); + allocator_->AddBitrateObserver(&bitrate_observer, 500000, 100000, 1500000, + &start_bitrate); + EXPECT_EQ(1500000, start_bitrate); allocator_->OnNetworkChanged(1500000, 0, 0); EXPECT_EQ(1500000u, bitrate_observer.last_bitrate_); } @@ -60,8 +69,13 @@ TEST_F(BitrateAllocatorTest, UpdatingBitrateObserver) { TEST_F(BitrateAllocatorTest, TwoBitrateObserversOneRtcpObserver) { TestBitrateObserver bitrate_observer_1; TestBitrateObserver bitrate_observer_2; - allocator_->AddBitrateObserver(&bitrate_observer_1, 200000, 100000, 300000); - allocator_->AddBitrateObserver(&bitrate_observer_2, 200000, 200000, 300000); + int start_bitrate; + allocator_->AddBitrateObserver(&bitrate_observer_1, 200000, 100000, 300000, + &start_bitrate); + EXPECT_EQ(300000, start_bitrate); + allocator_->AddBitrateObserver(&bitrate_observer_2, 200000, 200000, 300000, + &start_bitrate); + EXPECT_EQ(200000, start_bitrate); // Test too low start bitrate, hence lower than sum of min. Min bitrates will // be allocated to all observers. @@ -79,16 +93,17 @@ TEST_F(BitrateAllocatorTest, TwoBitrateObserversOneRtcpObserver) { EXPECT_EQ(100000u + kBitrateToShare / 2, bitrate_observer_1.last_bitrate_); EXPECT_EQ(200000u + kBitrateToShare / 2, bitrate_observer_2.last_bitrate_); - // Limited by max bitrates. - allocator_->OnNetworkChanged(800000, 0, 50); - EXPECT_EQ(300000u, bitrate_observer_1.last_bitrate_); - EXPECT_EQ(300000u, bitrate_observer_2.last_bitrate_); + // Limited by 2x max bitrates since we leave room for FEC and retransmissions. + allocator_->OnNetworkChanged(1500000, 0, 50); + EXPECT_EQ(600000u, bitrate_observer_1.last_bitrate_); + EXPECT_EQ(600000u, bitrate_observer_2.last_bitrate_); } class BitrateAllocatorTestNoEnforceMin : public ::testing::Test { protected: BitrateAllocatorTestNoEnforceMin() : allocator_(new BitrateAllocator()) { allocator_->EnforceMinBitrate(false); + allocator_->OnNetworkChanged(300000u, 0, 0); } ~BitrateAllocatorTestNoEnforceMin() {} @@ -99,7 +114,10 @@ class BitrateAllocatorTestNoEnforceMin : public ::testing::Test { // as intended. TEST_F(BitrateAllocatorTestNoEnforceMin, OneBitrateObserver) { TestBitrateObserver bitrate_observer_1; - allocator_->AddBitrateObserver(&bitrate_observer_1, 200000, 100000, 400000); + int start_bitrate; + allocator_->AddBitrateObserver(&bitrate_observer_1, 200000, 100000, 400000, + &start_bitrate); + EXPECT_EQ(300000, start_bitrate); // High REMB. allocator_->OnNetworkChanged(150000, 0, 0); @@ -117,21 +135,31 @@ TEST_F(BitrateAllocatorTestNoEnforceMin, ThreeBitrateObservers) { TestBitrateObserver bitrate_observer_2; TestBitrateObserver bitrate_observer_3; // Set up the observers with min bitrates at 100000, 200000, and 300000. - // Note: The start bitrate of bitrate_observer_1 (700000) is used as the - // overall start bitrate. - allocator_->AddBitrateObserver(&bitrate_observer_1, 700000, 100000, 400000); - allocator_->AddBitrateObserver(&bitrate_observer_2, 200000, 200000, 400000); - allocator_->AddBitrateObserver(&bitrate_observer_3, 200000, 300000, 400000); + int start_bitrate; + allocator_->AddBitrateObserver(&bitrate_observer_1, 200000, 100000, 400000, + &start_bitrate); + EXPECT_EQ(300000, start_bitrate); + + allocator_->AddBitrateObserver(&bitrate_observer_2, 200000, 200000, 400000, + &start_bitrate); + EXPECT_EQ(200000, start_bitrate); + EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_); + + allocator_->AddBitrateObserver(&bitrate_observer_3, 200000, 300000, 400000, + &start_bitrate); + EXPECT_EQ(0, start_bitrate); + EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_); + EXPECT_EQ(200000u, bitrate_observer_2.last_bitrate_); // High REMB. Make sure the controllers get a fair share of the surplus // (i.e., what is left after each controller gets its min rate). allocator_->OnNetworkChanged(690000, 0, 0); // Verify that each observer gets its min rate (sum of min rates is 600000), // and that the remaining 90000 is divided equally among the three. - const uint32_t kBitrateToShare = 690000u - 100000u - 200000u - 300000u; - EXPECT_EQ(100000u + kBitrateToShare / 3, bitrate_observer_1.last_bitrate_); - EXPECT_EQ(200000u + kBitrateToShare / 3, bitrate_observer_2.last_bitrate_); - EXPECT_EQ(300000u + kBitrateToShare / 3, bitrate_observer_3.last_bitrate_); + uint32_t bitrate_to_share = 690000u - 100000u - 200000u - 300000u; + EXPECT_EQ(100000u + bitrate_to_share / 3, bitrate_observer_1.last_bitrate_); + EXPECT_EQ(200000u + bitrate_to_share / 3, bitrate_observer_2.last_bitrate_); + EXPECT_EQ(300000u + bitrate_to_share / 3, bitrate_observer_3.last_bitrate_); // High REMB, but below the sum of min bitrates. allocator_->OnNetworkChanged(500000, 0, 0); @@ -153,23 +181,25 @@ TEST_F(BitrateAllocatorTestNoEnforceMin, ThreeBitrateObservers) { allocator_->RemoveBitrateObserver(&bitrate_observer_3); } -TEST_F(BitrateAllocatorTestNoEnforceMin, Honors10KbpsLimit) { - TestBitrateObserver bitrate_observer; - allocator_->AddBitrateObserver(&bitrate_observer, 700000, 30000, 400000); - - int min_bitrate = 0; - int max_bitrate = 0; - allocator_->GetMinMaxBitrateSumBps(&min_bitrate, &max_bitrate); - EXPECT_EQ(10000, min_bitrate); -} - TEST_F(BitrateAllocatorTest, ThreeBitrateObserversLowRembEnforceMin) { TestBitrateObserver bitrate_observer_1; TestBitrateObserver bitrate_observer_2; TestBitrateObserver bitrate_observer_3; - allocator_->AddBitrateObserver(&bitrate_observer_1, 200000, 100000, 300000); - allocator_->AddBitrateObserver(&bitrate_observer_2, 200000, 200000, 300000); - allocator_->AddBitrateObserver(&bitrate_observer_3, 200000, 300000, 300000); + int start_bitrate; + allocator_->AddBitrateObserver(&bitrate_observer_1, 200000, 100000, 400000, + &start_bitrate); + EXPECT_EQ(300000, start_bitrate); + + allocator_->AddBitrateObserver(&bitrate_observer_2, 200000, 200000, 400000, + &start_bitrate); + EXPECT_EQ(200000, start_bitrate); + EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_); + + allocator_->AddBitrateObserver(&bitrate_observer_3, 200000, 300000, 400000, + &start_bitrate); + EXPECT_EQ(300000, start_bitrate); + EXPECT_EQ(100000, static_cast(bitrate_observer_1.last_bitrate_)); + EXPECT_EQ(200000, static_cast(bitrate_observer_2.last_bitrate_)); // Low REMB. Verify that all observers still get their respective min bitrate. allocator_->OnNetworkChanged(1000, 0, 0); diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc b/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc index d6f64fbd2d..d54da99bef 100644 --- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc +++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc @@ -94,10 +94,11 @@ BitrateControllerImpl::BitrateControllerImpl(Clock* clock, last_fraction_loss_(0), last_rtt_ms_(0), last_reserved_bitrate_bps_(0) { -} - -BitrateControllerImpl::~BitrateControllerImpl() { - delete critsect_; + // This calls the observer_, which means that the observer provided by the + // user must be ready to accept a bitrate update when it constructs the + // controller. We do this to avoid having to keep synchronized initial values + // in both the controller and the allocator. + MaybeTriggerOnNetworkChanged(); } RtcpBandwidthObserver* BitrateControllerImpl::CreateRtcpBandwidthObserver() { @@ -105,19 +106,25 @@ RtcpBandwidthObserver* BitrateControllerImpl::CreateRtcpBandwidthObserver() { } void BitrateControllerImpl::SetStartBitrate(int start_bitrate_bps) { - CriticalSectionScoped cs(critsect_); - bandwidth_estimation_.SetSendBitrate(start_bitrate_bps); + { + CriticalSectionScoped cs(critsect_.get()); + bandwidth_estimation_.SetSendBitrate(start_bitrate_bps); + } + MaybeTriggerOnNetworkChanged(); } void BitrateControllerImpl::SetMinMaxBitrate(int min_bitrate_bps, int max_bitrate_bps) { - CriticalSectionScoped cs(critsect_); - bandwidth_estimation_.SetMinMaxBitrate(min_bitrate_bps, max_bitrate_bps); + { + CriticalSectionScoped cs(critsect_.get()); + bandwidth_estimation_.SetMinMaxBitrate(min_bitrate_bps, max_bitrate_bps); + } + MaybeTriggerOnNetworkChanged(); } void BitrateControllerImpl::SetReservedBitrate(uint32_t reserved_bitrate_bps) { { - CriticalSectionScoped cs(critsect_); + CriticalSectionScoped cs(critsect_.get()); reserved_bitrate_bps_ = reserved_bitrate_bps; } MaybeTriggerOnNetworkChanged(); @@ -125,7 +132,7 @@ void BitrateControllerImpl::SetReservedBitrate(uint32_t reserved_bitrate_bps) { void BitrateControllerImpl::OnReceivedEstimatedBitrate(uint32_t bitrate) { { - CriticalSectionScoped cs(critsect_); + CriticalSectionScoped cs(critsect_.get()); bandwidth_estimation_.UpdateReceiverEstimate(bitrate); } MaybeTriggerOnNetworkChanged(); @@ -133,7 +140,7 @@ void BitrateControllerImpl::OnReceivedEstimatedBitrate(uint32_t bitrate) { int64_t BitrateControllerImpl::TimeUntilNextProcess() { const int64_t kBitrateControllerUpdateIntervalMs = 25; - CriticalSectionScoped cs(critsect_); + CriticalSectionScoped cs(critsect_.get()); int64_t time_since_update_ms = clock_->TimeInMilliseconds() - last_bitrate_update_ms_; return std::max( @@ -144,7 +151,7 @@ int32_t BitrateControllerImpl::Process() { if (TimeUntilNextProcess() > 0) return 0; { - CriticalSectionScoped cs(critsect_); + CriticalSectionScoped cs(critsect_.get()); bandwidth_estimation_.UpdateEstimate(clock_->TimeInMilliseconds()); } MaybeTriggerOnNetworkChanged(); @@ -158,7 +165,7 @@ void BitrateControllerImpl::OnReceivedRtcpReceiverReport( int number_of_packets, int64_t now_ms) { { - CriticalSectionScoped cs(critsect_); + CriticalSectionScoped cs(critsect_.get()); bandwidth_estimation_.UpdateReceiverBlock(fraction_loss, rtt, number_of_packets, now_ms); } @@ -169,36 +176,44 @@ void BitrateControllerImpl::MaybeTriggerOnNetworkChanged() { uint32_t bitrate; uint8_t fraction_loss; int64_t rtt; - bool new_bitrate = false; - { - CriticalSectionScoped cs(critsect_); - bandwidth_estimation_.CurrentEstimate(&bitrate, &fraction_loss, &rtt); - bitrate -= std::min(bitrate, reserved_bitrate_bps_); - bitrate = std::max(bitrate, bandwidth_estimation_.GetMinBitrate()); - - if (bitrate != last_bitrate_bps_ || fraction_loss != last_fraction_loss_ || - rtt != last_rtt_ms_ || - last_reserved_bitrate_bps_ != reserved_bitrate_bps_) { - last_bitrate_bps_ = bitrate; - last_fraction_loss_ = fraction_loss; - last_rtt_ms_ = rtt; - last_reserved_bitrate_bps_ = reserved_bitrate_bps_; - new_bitrate = true; - } - } - if (new_bitrate) + if (GetNetworkParameters(&bitrate, &fraction_loss, &rtt)) observer_->OnNetworkChanged(bitrate, fraction_loss, rtt); } +bool BitrateControllerImpl::GetNetworkParameters(uint32_t* bitrate, + uint8_t* fraction_loss, + int64_t* rtt) { + CriticalSectionScoped cs(critsect_.get()); + int current_bitrate; + bandwidth_estimation_.CurrentEstimate(¤t_bitrate, fraction_loss, rtt); + *bitrate = current_bitrate; + *bitrate -= std::min(*bitrate, reserved_bitrate_bps_); + *bitrate = + std::max(*bitrate, bandwidth_estimation_.GetMinBitrate()); + + bool new_bitrate = false; + if (*bitrate != last_bitrate_bps_ || *fraction_loss != last_fraction_loss_ || + *rtt != last_rtt_ms_ || + last_reserved_bitrate_bps_ != reserved_bitrate_bps_) { + last_bitrate_bps_ = *bitrate; + last_fraction_loss_ = *fraction_loss; + last_rtt_ms_ = *rtt; + last_reserved_bitrate_bps_ = reserved_bitrate_bps_; + new_bitrate = true; + } + return new_bitrate; +} + bool BitrateControllerImpl::AvailableBandwidth(uint32_t* bandwidth) const { - CriticalSectionScoped cs(critsect_); - uint32_t bitrate; + CriticalSectionScoped cs(critsect_.get()); + int bitrate; uint8_t fraction_loss; int64_t rtt; bandwidth_estimation_.CurrentEstimate(&bitrate, &fraction_loss, &rtt); - if (bitrate) { - *bandwidth = bitrate - std::min(bitrate, reserved_bitrate_bps_); - *bandwidth = std::max(*bandwidth, bandwidth_estimation_.GetMinBitrate()); + if (bitrate > 0) { + bitrate = bitrate - std::min(bitrate, reserved_bitrate_bps_); + bitrate = std::max(bitrate, bandwidth_estimation_.GetMinBitrate()); + *bandwidth = bitrate; return true; } return false; diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_impl.h b/webrtc/modules/bitrate_controller/bitrate_controller_impl.h index dec4afad86..3d38a54f53 100644 --- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.h +++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.h @@ -29,7 +29,7 @@ namespace webrtc { class BitrateControllerImpl : public BitrateController { public: BitrateControllerImpl(Clock* clock, BitrateObserver* observer); - virtual ~BitrateControllerImpl(); + virtual ~BitrateControllerImpl() {} bool AvailableBandwidth(uint32_t* bandwidth) const override; @@ -57,6 +57,11 @@ class BitrateControllerImpl : public BitrateController { void MaybeTriggerOnNetworkChanged(); + // Returns true if the parameters have changed since the last call. + bool GetNetworkParameters(uint32_t* bitrate, + uint8_t* fraction_loss, + int64_t* rtt); + void OnNetworkChanged(uint32_t bitrate, uint8_t fraction_loss, // 0 - 255. int64_t rtt) @@ -67,7 +72,7 @@ class BitrateControllerImpl : public BitrateController { BitrateObserver* observer_; int64_t last_bitrate_update_ms_; - CriticalSectionWrapper* critsect_; + const rtc::scoped_ptr critsect_; SendSideBandwidthEstimation bandwidth_estimation_ GUARDED_BY(*critsect_); uint32_t reserved_bitrate_bps_ GUARDED_BY(*critsect_); diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc b/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc index e8d49c6924..72831c78d6 100644 --- a/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc +++ b/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc @@ -46,11 +46,11 @@ class TestBitrateObserver: public BitrateObserver { virtual void OnNetworkChanged(uint32_t bitrate, uint8_t fraction_loss, int64_t rtt) { - last_bitrate_ = bitrate; + last_bitrate_ = static_cast(bitrate); last_fraction_loss_ = fraction_loss; last_rtt_ = rtt; } - uint32_t last_bitrate_; + int last_bitrate_; uint8_t last_fraction_loss_; int64_t last_rtt_; }; @@ -63,8 +63,10 @@ class BitrateControllerTest : public ::testing::Test { virtual void SetUp() { controller_ = BitrateController::CreateBitrateController(&clock_, &bitrate_observer_); - controller_->SetStartBitrate(200000); - controller_->SetMinMaxBitrate(100000, 300000); + controller_->SetStartBitrate(kStartBitrateBps); + EXPECT_EQ(kStartBitrateBps, bitrate_observer_.last_bitrate_); + controller_->SetMinMaxBitrate(kMinBitrateBps, kMaxBitrateBps); + EXPECT_EQ(kStartBitrateBps, bitrate_observer_.last_bitrate_); bandwidth_observer_ = controller_->CreateRtcpBandwidthObserver(); } @@ -73,19 +75,38 @@ class BitrateControllerTest : public ::testing::Test { delete controller_; } + const int kMinBitrateBps = 100000; + const int kStartBitrateBps = 200000; + const int kMaxBitrateBps = 300000; + + const int kDefaultMinBitrateBps = 10000; + const int kDefaultMaxBitrateBps = 1000000000; + webrtc::SimulatedClock clock_; TestBitrateObserver bitrate_observer_; BitrateController* controller_; RtcpBandwidthObserver* bandwidth_observer_; }; +TEST_F(BitrateControllerTest, DefaultMinMaxBitrate) { + // Receive successively lower REMBs, verify the reserved bitrate is deducted. + controller_->SetMinMaxBitrate(0, 0); + EXPECT_EQ(kStartBitrateBps, bitrate_observer_.last_bitrate_); + bandwidth_observer_->OnReceivedEstimatedBitrate(kDefaultMinBitrateBps / 2); + EXPECT_EQ(kDefaultMinBitrateBps, bitrate_observer_.last_bitrate_); + bandwidth_observer_->OnReceivedEstimatedBitrate(2 * kDefaultMaxBitrateBps); + clock_.AdvanceTimeMilliseconds(1000); + controller_->Process(); + EXPECT_EQ(kDefaultMaxBitrateBps, bitrate_observer_.last_bitrate_); +} + TEST_F(BitrateControllerTest, OneBitrateObserverOneRtcpObserver) { // First REMB applies immediately. int64_t time_ms = 1001; webrtc::ReportBlockList report_blocks; report_blocks.push_back(CreateReportBlock(1, 2, 0, 1)); bandwidth_observer_->OnReceivedEstimatedBitrate(200000); - EXPECT_EQ(200000u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(200000, bitrate_observer_.last_bitrate_); EXPECT_EQ(0, bitrate_observer_.last_fraction_loss_); EXPECT_EQ(0, bitrate_observer_.last_rtt_); bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, time_ms); @@ -98,7 +119,7 @@ TEST_F(BitrateControllerTest, OneBitrateObserverOneRtcpObserver) { // Test bitrate increase 8% per second. report_blocks.push_back(CreateReportBlock(1, 2, 0, 21)); bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, time_ms); - EXPECT_EQ(217000u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(217000, bitrate_observer_.last_bitrate_); EXPECT_EQ(0, bitrate_observer_.last_fraction_loss_); EXPECT_EQ(50, bitrate_observer_.last_rtt_); time_ms += 1000; @@ -106,7 +127,7 @@ TEST_F(BitrateControllerTest, OneBitrateObserverOneRtcpObserver) { report_blocks.clear(); report_blocks.push_back(CreateReportBlock(1, 2, 0, 41)); bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, time_ms); - EXPECT_EQ(235360u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(235360, bitrate_observer_.last_bitrate_); EXPECT_EQ(0, bitrate_observer_.last_fraction_loss_); EXPECT_EQ(50, bitrate_observer_.last_rtt_); time_ms += 1000; @@ -114,41 +135,41 @@ TEST_F(BitrateControllerTest, OneBitrateObserverOneRtcpObserver) { report_blocks.clear(); report_blocks.push_back(CreateReportBlock(1, 2, 0, 61)); bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, time_ms); - EXPECT_EQ(255189u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(255189, bitrate_observer_.last_bitrate_); time_ms += 1000; report_blocks.clear(); report_blocks.push_back(CreateReportBlock(1, 2, 0, 81)); bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, time_ms); - EXPECT_EQ(276604u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(276604, bitrate_observer_.last_bitrate_); time_ms += 1000; report_blocks.clear(); report_blocks.push_back(CreateReportBlock(1, 2, 0, 801)); bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, time_ms); - EXPECT_EQ(299732u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(299732, bitrate_observer_.last_bitrate_); time_ms += 1000; // Reach max cap. report_blocks.clear(); report_blocks.push_back(CreateReportBlock(1, 2, 0, 101)); bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, time_ms); - EXPECT_EQ(300000u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(300000, bitrate_observer_.last_bitrate_); time_ms += 1000; report_blocks.clear(); report_blocks.push_back(CreateReportBlock(1, 2, 0, 141)); bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, time_ms); - EXPECT_EQ(300000u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(300000, bitrate_observer_.last_bitrate_); // Test that a low REMB trigger immediately. bandwidth_observer_->OnReceivedEstimatedBitrate(250000); - EXPECT_EQ(250000u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(250000, bitrate_observer_.last_bitrate_); EXPECT_EQ(0, bitrate_observer_.last_fraction_loss_); EXPECT_EQ(50, bitrate_observer_.last_rtt_); bandwidth_observer_->OnReceivedEstimatedBitrate(1000); - EXPECT_EQ(100000u, bitrate_observer_.last_bitrate_); // Min cap. + EXPECT_EQ(100000, bitrate_observer_.last_bitrate_); // Min cap. } TEST_F(BitrateControllerTest, OneBitrateObserverTwoRtcpObservers) { @@ -167,7 +188,7 @@ TEST_F(BitrateControllerTest, OneBitrateObserverTwoRtcpObservers) { report_blocks.push_back(CreateReportBlock(1, 2, 0, 21)); second_bandwidth_observer->OnReceivedRtcpReceiverReport( report_blocks, 100, 1); - EXPECT_EQ(217000u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(217000, bitrate_observer_.last_bitrate_); EXPECT_EQ(0, bitrate_observer_.last_fraction_loss_); EXPECT_EQ(100, bitrate_observer_.last_rtt_); time_ms += 500; @@ -179,7 +200,7 @@ TEST_F(BitrateControllerTest, OneBitrateObserverTwoRtcpObservers) { time_ms += 500; second_bandwidth_observer->OnReceivedRtcpReceiverReport( report_blocks, 100, time_ms); - EXPECT_EQ(235360u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(235360, bitrate_observer_.last_bitrate_); EXPECT_EQ(0, bitrate_observer_.last_fraction_loss_); EXPECT_EQ(100, bitrate_observer_.last_rtt_); time_ms += 500; @@ -189,20 +210,20 @@ TEST_F(BitrateControllerTest, OneBitrateObserverTwoRtcpObservers) { report_blocks.push_back(CreateReportBlock(1, 2, 0, 31)); second_bandwidth_observer->OnReceivedRtcpReceiverReport( report_blocks, 100, time_ms); - EXPECT_EQ(235360u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(235360, bitrate_observer_.last_bitrate_); time_ms += 500; report_blocks.clear(); report_blocks.push_back(CreateReportBlock(1, 2, 0, 41)); bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, time_ms); - EXPECT_EQ(255189u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(255189, bitrate_observer_.last_bitrate_); // Second report should not change estimate. report_blocks.clear(); report_blocks.push_back(CreateReportBlock(1, 2, 0, 41)); second_bandwidth_observer->OnReceivedRtcpReceiverReport( report_blocks, 100, time_ms); - EXPECT_EQ(255189u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(255189, bitrate_observer_.last_bitrate_); time_ms += 1000; // Reports from only one bandwidth observer is ok. @@ -210,14 +231,14 @@ TEST_F(BitrateControllerTest, OneBitrateObserverTwoRtcpObservers) { report_blocks.push_back(CreateReportBlock(1, 2, 0, 61)); second_bandwidth_observer->OnReceivedRtcpReceiverReport( report_blocks, 50, time_ms); - EXPECT_EQ(276604u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(276604, bitrate_observer_.last_bitrate_); time_ms += 1000; report_blocks.clear(); report_blocks.push_back(CreateReportBlock(1, 2, 0, 81)); second_bandwidth_observer->OnReceivedRtcpReceiverReport( report_blocks, 50, time_ms); - EXPECT_EQ(299732u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(299732, bitrate_observer_.last_bitrate_); time_ms += 1000; // Reach max cap. @@ -225,33 +246,33 @@ TEST_F(BitrateControllerTest, OneBitrateObserverTwoRtcpObservers) { report_blocks.push_back(CreateReportBlock(1, 2, 0, 121)); second_bandwidth_observer->OnReceivedRtcpReceiverReport( report_blocks, 50, time_ms); - EXPECT_EQ(300000u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(300000, bitrate_observer_.last_bitrate_); time_ms += 1000; report_blocks.clear(); report_blocks.push_back(CreateReportBlock(1, 2, 0, 141)); second_bandwidth_observer->OnReceivedRtcpReceiverReport( report_blocks, 50, time_ms); - EXPECT_EQ(300000u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(300000, bitrate_observer_.last_bitrate_); // Test that a low REMB trigger immediately. // We don't care which bandwidth observer that delivers the REMB. second_bandwidth_observer->OnReceivedEstimatedBitrate(250000); - EXPECT_EQ(250000u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(250000, bitrate_observer_.last_bitrate_); EXPECT_EQ(0, bitrate_observer_.last_fraction_loss_); EXPECT_EQ(50, bitrate_observer_.last_rtt_); // Min cap. bandwidth_observer_->OnReceivedEstimatedBitrate(1000); - EXPECT_EQ(100000u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(100000, bitrate_observer_.last_bitrate_); delete second_bandwidth_observer; } TEST_F(BitrateControllerTest, OneBitrateObserverMultipleReportBlocks) { uint32_t sequence_number[2] = {0, 0xFF00}; - const uint32_t kStartBitrate = 200000; - const uint32_t kMinBitrate = 100000; - const uint32_t kMaxBitrate = 300000; + const int kStartBitrate = 200000; + const int kMinBitrate = 100000; + const int kMaxBitrate = 300000; controller_->SetStartBitrate(kStartBitrate); controller_->SetMinMaxBitrate(kMinBitrate, kMaxBitrate); @@ -267,7 +288,7 @@ TEST_F(BitrateControllerTest, OneBitrateObserverMultipleReportBlocks) { // Receive a high REMB, test bitrate increase. bandwidth_observer_->OnReceivedEstimatedBitrate(400000); - uint32_t last_bitrate = 0; + int last_bitrate = 0; // Ramp up to max bitrate. for (int i = 0; i < 6; ++i) { report_blocks.push_back(CreateReportBlock(1, 2, 0, sequence_number[0])); @@ -329,48 +350,48 @@ TEST_F(BitrateControllerTest, SetReservedBitrate) { // Receive successively lower REMBs, verify the reserved bitrate is deducted. controller_->SetReservedBitrate(0); bandwidth_observer_->OnReceivedEstimatedBitrate(400000); - EXPECT_EQ(200000u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(200000, bitrate_observer_.last_bitrate_); controller_->SetReservedBitrate(50000); bandwidth_observer_->OnReceivedEstimatedBitrate(400000); - EXPECT_EQ(150000u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(150000, bitrate_observer_.last_bitrate_); controller_->SetReservedBitrate(0); bandwidth_observer_->OnReceivedEstimatedBitrate(250000); - EXPECT_EQ(200000u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(200000, bitrate_observer_.last_bitrate_); controller_->SetReservedBitrate(50000); bandwidth_observer_->OnReceivedEstimatedBitrate(250000); - EXPECT_EQ(150000u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(150000, bitrate_observer_.last_bitrate_); controller_->SetReservedBitrate(0); bandwidth_observer_->OnReceivedEstimatedBitrate(200000); - EXPECT_EQ(200000u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(200000, bitrate_observer_.last_bitrate_); controller_->SetReservedBitrate(30000); bandwidth_observer_->OnReceivedEstimatedBitrate(200000); - EXPECT_EQ(170000u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(170000, bitrate_observer_.last_bitrate_); controller_->SetReservedBitrate(0); bandwidth_observer_->OnReceivedEstimatedBitrate(160000); - EXPECT_EQ(160000u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(160000, bitrate_observer_.last_bitrate_); controller_->SetReservedBitrate(30000); bandwidth_observer_->OnReceivedEstimatedBitrate(160000); - EXPECT_EQ(130000u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(130000, bitrate_observer_.last_bitrate_); controller_->SetReservedBitrate(0); bandwidth_observer_->OnReceivedEstimatedBitrate(120000); - EXPECT_EQ(120000u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(120000, bitrate_observer_.last_bitrate_); controller_->SetReservedBitrate(10000); bandwidth_observer_->OnReceivedEstimatedBitrate(120000); - EXPECT_EQ(110000u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(110000, bitrate_observer_.last_bitrate_); controller_->SetReservedBitrate(0); bandwidth_observer_->OnReceivedEstimatedBitrate(120000); - EXPECT_EQ(120000u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(120000, bitrate_observer_.last_bitrate_); controller_->SetReservedBitrate(50000); bandwidth_observer_->OnReceivedEstimatedBitrate(120000); // Limited by min bitrate. - EXPECT_EQ(100000u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(100000, bitrate_observer_.last_bitrate_); controller_->SetReservedBitrate(10000); bandwidth_observer_->OnReceivedEstimatedBitrate(1); - EXPECT_EQ(100000u, bitrate_observer_.last_bitrate_); + EXPECT_EQ(100000, bitrate_observer_.last_bitrate_); } diff --git a/webrtc/modules/bitrate_controller/include/bitrate_allocator.h b/webrtc/modules/bitrate_controller/include/bitrate_allocator.h index d2810584e8..9cc4b74711 100644 --- a/webrtc/modules/bitrate_controller/include/bitrate_allocator.h +++ b/webrtc/modules/bitrate_controller/include/bitrate_allocator.h @@ -30,7 +30,6 @@ class BitrateObserver; class BitrateAllocator { public: BitrateAllocator(); - virtual ~BitrateAllocator(); void OnNetworkChanged(uint32_t target_bitrate, uint8_t fraction_loss, @@ -39,17 +38,16 @@ class BitrateAllocator { // Set the start and max send bitrate used by the bandwidth management. // // observer, updates bitrates if already in use. - // min_bitrate_kbit = 0 equals no min bitrate. - // max_bitrate_kit = 0 equals no max bitrate. - // - // Returns a new suggested start bitrate computed from the start and min - // bitrates of the observers, or -1 if the start bitrate shouldn't be changed. - virtual int AddBitrateObserver(BitrateObserver* observer, - uint32_t start_bitrate, - uint32_t min_bitrate, - uint32_t max_bitrate); + // min_bitrate_bps = 0 equals no min bitrate. + // max_bitrate_bps = 0 equals no max bitrate. + // TODO(holmer): Remove start_bitrate_bps when old API is gone. + int AddBitrateObserver(BitrateObserver* observer, + uint32_t start_bitrate_bps, + uint32_t min_bitrate_bps, + uint32_t max_bitrate_bps, + int* new_observer_bitrate_bps); - virtual void RemoveBitrateObserver(BitrateObserver* observer); + void RemoveBitrateObserver(BitrateObserver* observer); void GetMinMaxBitrateSumBps(int* min_bitrate_sum_bps, int* max_bitrate_sum_bps) const; @@ -79,30 +77,30 @@ class BitrateAllocator { BitrateObserver* observer_; uint32_t min_bitrate_; }; - typedef std::pair + typedef std::pair BitrateObserverConfiguration; typedef std::list BitrateObserverConfList; - typedef std::multimap ObserverSortingMap; - - void NormalRateAllocation(uint32_t bitrate, - uint8_t fraction_loss, - int64_t rtt, - uint32_t sum_min_bitrates) - EXCLUSIVE_LOCKS_REQUIRED(crit_sect_); - - void LowRateAllocation(uint32_t bitrate, - uint8_t fraction_loss, - int64_t rtt, - uint32_t sum_min_bitrates) - EXCLUSIVE_LOCKS_REQUIRED(crit_sect_); + typedef std::multimap ObserverSortingMap; + typedef std::map ObserverBitrateMap; BitrateObserverConfList::iterator FindObserverConfigurationPair( const BitrateObserver* observer) EXCLUSIVE_LOCKS_REQUIRED(crit_sect_); + ObserverBitrateMap AllocateBitrates() EXCLUSIVE_LOCKS_REQUIRED(crit_sect_); + ObserverBitrateMap NormalRateAllocation(uint32_t bitrate, + uint32_t sum_min_bitrates) + EXCLUSIVE_LOCKS_REQUIRED(crit_sect_); + + ObserverBitrateMap LowRateAllocation(uint32_t bitrate) + EXCLUSIVE_LOCKS_REQUIRED(crit_sect_); rtc::scoped_ptr crit_sect_; + // Stored in a list to keep track of the insertion order. BitrateObserverConfList bitrate_observers_ GUARDED_BY(crit_sect_); bool bitrate_observers_modified_ GUARDED_BY(crit_sect_); bool enforce_min_bitrate_ GUARDED_BY(crit_sect_); + uint32_t last_bitrate_bps_ GUARDED_BY(crit_sect_); + uint8_t last_fraction_loss_ GUARDED_BY(crit_sect_); + int64_t last_rtt_ GUARDED_BY(crit_sect_); }; } // namespace webrtc #endif // WEBRTC_MODULES_BITRATE_CONTROLLER_INCLUDE_BITRATE_ALLOCATOR_H_ diff --git a/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.cc b/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.cc index 49b900c5eb..247361df47 100644 --- a/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.cc +++ b/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.cc @@ -12,18 +12,21 @@ #include +#include "webrtc/base/checks.h" #include "webrtc/system_wrappers/interface/field_trial.h" #include "webrtc/system_wrappers/interface/logging.h" #include "webrtc/system_wrappers/interface/metrics.h" namespace webrtc { namespace { -enum { kBweIncreaseIntervalMs = 1000 }; -enum { kBweDecreaseIntervalMs = 300 }; -enum { kLimitNumPackets = 20 }; -enum { kAvgPacketSizeBytes = 1000 }; -enum { kStartPhaseMs = 2000 }; -enum { kBweConverganceTimeMs = 20000 }; +const int64_t kBweIncreaseIntervalMs = 1000; +const int64_t kBweDecreaseIntervalMs = 300; +const int64_t kStartPhaseMs = 2000; +const int64_t kBweConverganceTimeMs = 20000; +const int kLimitNumPackets = 20; +const int kAvgPacketSizeBytes = 1000; +const int kDefaultMinBitrateBps = 10000; +const int kDefaultMaxBitrateBps = 1000000000; struct UmaRampUpMetric { const char* metric_name; @@ -66,8 +69,8 @@ SendSideBandwidthEstimation::SendSideBandwidthEstimation() : accumulate_lost_packets_Q8_(0), accumulate_expected_packets_(0), bitrate_(0), - min_bitrate_configured_(0), - max_bitrate_configured_(0), + min_bitrate_configured_(kDefaultMinBitrateBps), + max_bitrate_configured_(kDefaultMaxBitrateBps), time_last_receiver_block_ms_(0), last_fraction_loss_(0), last_round_trip_time_ms_(0), @@ -82,7 +85,8 @@ SendSideBandwidthEstimation::SendSideBandwidthEstimation() SendSideBandwidthEstimation::~SendSideBandwidthEstimation() {} -void SendSideBandwidthEstimation::SetSendBitrate(uint32_t bitrate) { +void SendSideBandwidthEstimation::SetSendBitrate(int bitrate) { + DCHECK_GT(bitrate, 0); bitrate_ = bitrate; // Clear last sent bitrate history so the new value can be used directly @@ -90,17 +94,23 @@ void SendSideBandwidthEstimation::SetSendBitrate(uint32_t bitrate) { min_bitrate_history_.clear(); } -void SendSideBandwidthEstimation::SetMinMaxBitrate(uint32_t min_bitrate, - uint32_t max_bitrate) { - min_bitrate_configured_ = min_bitrate; - max_bitrate_configured_ = max_bitrate; +void SendSideBandwidthEstimation::SetMinMaxBitrate(int min_bitrate, + int max_bitrate) { + DCHECK_GE(min_bitrate, 0); + min_bitrate_configured_ = std::max(min_bitrate, kDefaultMinBitrateBps); + if (max_bitrate > 0) { + max_bitrate_configured_ = + std::max(min_bitrate_configured_, max_bitrate); + } else { + max_bitrate_configured_ = kDefaultMaxBitrateBps; + } } -uint32_t SendSideBandwidthEstimation::GetMinBitrate() const { +int SendSideBandwidthEstimation::GetMinBitrate() const { return min_bitrate_configured_; } -void SendSideBandwidthEstimation::CurrentEstimate(uint32_t* bitrate, +void SendSideBandwidthEstimation::CurrentEstimate(int* bitrate, uint8_t* loss, int64_t* rtt) const { *bitrate = bitrate_; diff --git a/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.h b/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.h index 7b0a3da273..fb8962ad50 100644 --- a/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.h +++ b/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.h @@ -24,7 +24,7 @@ class SendSideBandwidthEstimation { SendSideBandwidthEstimation(); virtual ~SendSideBandwidthEstimation(); - void CurrentEstimate(uint32_t* bitrate, uint8_t* loss, int64_t* rtt) const; + void CurrentEstimate(int* bitrate, uint8_t* loss, int64_t* rtt) const; // Call periodically to update estimate. void UpdateEstimate(int64_t now_ms); @@ -38,9 +38,9 @@ class SendSideBandwidthEstimation { int number_of_packets, int64_t now_ms); - void SetSendBitrate(uint32_t bitrate); - void SetMinMaxBitrate(uint32_t min_bitrate, uint32_t max_bitrate); - uint32_t GetMinBitrate() const; + void SetSendBitrate(int bitrate); + void SetMinMaxBitrate(int min_bitrate, int max_bitrate); + int GetMinBitrate() const; private: enum UmaState { kNoUpdate, kFirstDone, kDone }; diff --git a/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation_unittest.cc b/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation_unittest.cc index 5171049f09..ab052b5244 100644 --- a/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation_unittest.cc +++ b/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation_unittest.cc @@ -21,27 +21,27 @@ TEST(SendSideBweTest, InitialRembWithProbing) { bwe.SetMinMaxBitrate(100000, 1500000); bwe.SetSendBitrate(200000); - const uint32_t kRemb = 1000000u; - const uint32_t kSecondRemb = kRemb + 500000u; + const int kRembBps = 1000000; + const int kSecondRembBps = kRembBps + 500000; int64_t now_ms = 0; bwe.UpdateReceiverBlock(0, 50, 1, now_ms); // Initial REMB applies immediately. - bwe.UpdateReceiverEstimate(kRemb); + bwe.UpdateReceiverEstimate(kRembBps); bwe.UpdateEstimate(now_ms); - uint32_t bitrate; + int bitrate; uint8_t fraction_loss; int64_t rtt; bwe.CurrentEstimate(&bitrate, &fraction_loss, &rtt); - EXPECT_EQ(kRemb, bitrate); + EXPECT_EQ(kRembBps, bitrate); // Second REMB doesn't apply immediately. now_ms += 2001; - bwe.UpdateReceiverEstimate(kSecondRemb); + bwe.UpdateReceiverEstimate(kSecondRembBps); bwe.UpdateEstimate(now_ms); bitrate = 0; bwe.CurrentEstimate(&bitrate, &fraction_loss, &rtt); - EXPECT_EQ(kRemb, bitrate); + EXPECT_EQ(kRembBps, bitrate); } } // namespace webrtc diff --git a/webrtc/modules/modules.gyp b/webrtc/modules/modules.gyp index 1283809539..4442d1c30f 100644 --- a/webrtc/modules/modules.gyp +++ b/webrtc/modules/modules.gyp @@ -206,6 +206,7 @@ 'module_common_types_unittest.cc', 'pacing/bitrate_prober_unittest.cc', 'pacing/paced_sender_unittest.cc', + 'pacing/packet_router_unittest.cc', 'remote_bitrate_estimator/bwe_simulations.cc', 'remote_bitrate_estimator/include/mock/mock_remote_bitrate_observer.h', 'remote_bitrate_estimator/inter_arrival_unittest.cc', diff --git a/webrtc/modules/pacing/BUILD.gn b/webrtc/modules/pacing/BUILD.gn index aa44b0dac8..ffced4d882 100644 --- a/webrtc/modules/pacing/BUILD.gn +++ b/webrtc/modules/pacing/BUILD.gn @@ -9,9 +9,11 @@ source_set("pacing") { sources = [ "include/paced_sender.h", + "include/packet_router.h", "bitrate_prober.cc", "bitrate_prober.h", "paced_sender.cc", + "packet_router.cc", ] configs += [ "../..:common_config" ] diff --git a/webrtc/modules/pacing/include/packet_router.h b/webrtc/modules/pacing/include/packet_router.h new file mode 100644 index 0000000000..c1b332a6bf --- /dev/null +++ b/webrtc/modules/pacing/include/packet_router.h @@ -0,0 +1,59 @@ +/* + * Copyright (c) 2015 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#ifndef WEBRTC_MODULES_PACING_INCLUDE_PACKET_ROUTER_H_ +#define WEBRTC_MODULES_PACING_INCLUDE_PACKET_ROUTER_H_ + +#include + +#include "webrtc/base/constructormagic.h" +#include "webrtc/base/scoped_ptr.h" +#include "webrtc/base/thread_annotations.h" +#include "webrtc/common_types.h" +#include "webrtc/modules/pacing/include/paced_sender.h" + +namespace webrtc { + +class CriticalSectionWrapper; +class RTPFragmentationHeader; +class RtpRtcp; +struct RTPVideoHeader; + +// PacketRouter routes outgoing data to the correct sending RTP module, based +// on the simulcast layer in RTPVideoHeader. +class PacketRouter : public PacedSender::Callback { + public: + PacketRouter(); + virtual ~PacketRouter(); + + void AddRtpModule(RtpRtcp* rtp_module); + void RemoveRtpModule(RtpRtcp* rtp_module); + + // Implements PacedSender::Callback. + bool TimeToSendPacket(uint32_t ssrc, + uint16_t sequence_number, + int64_t capture_timestamp, + bool retransmission) override; + + size_t TimeToSendPadding(size_t bytes) override; + + private: + // TODO(holmer): When the new video API has launched, remove crit_ and + // assume rtp_modules_ will never change during a call. We should then also + // switch rtp_modules_ to a map from ssrc to rtp module. + rtc::scoped_ptr crit_; + + // Map from ssrc to sending rtp module. + std::list rtp_modules_ GUARDED_BY(crit_.get()); + + DISALLOW_COPY_AND_ASSIGN(PacketRouter); +}; +} // namespace webrtc +#endif // WEBRTC_MODULES_PACING_INCLUDE_PACKET_ROUTER_H_ diff --git a/webrtc/modules/pacing/pacing.gypi b/webrtc/modules/pacing/pacing.gypi index 4aaa0aa32c..09be38f414 100644 --- a/webrtc/modules/pacing/pacing.gypi +++ b/webrtc/modules/pacing/pacing.gypi @@ -16,9 +16,11 @@ ], 'sources': [ 'include/paced_sender.h', + 'include/packet_router.h', 'bitrate_prober.cc', 'bitrate_prober.h', 'paced_sender.cc', + 'packet_router.cc', ], }, ], # targets diff --git a/webrtc/modules/pacing/packet_router.cc b/webrtc/modules/pacing/packet_router.cc new file mode 100644 index 0000000000..9e15a71317 --- /dev/null +++ b/webrtc/modules/pacing/packet_router.cc @@ -0,0 +1,61 @@ +/* + * Copyright (c) 2015 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include "webrtc/modules/pacing/include/packet_router.h" + +#include "webrtc/base/checks.h" +#include "webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h" +#include "webrtc/modules/rtp_rtcp/interface/rtp_rtcp_defines.h" +#include "webrtc/system_wrappers/interface/critical_section_wrapper.h" + +namespace webrtc { + +PacketRouter::PacketRouter() + : crit_(CriticalSectionWrapper::CreateCriticalSection()) { +} + +PacketRouter::~PacketRouter() { +} + +void PacketRouter::AddRtpModule(RtpRtcp* rtp_module) { + CriticalSectionScoped cs(crit_.get()); + DCHECK(std::find(rtp_modules_.begin(), rtp_modules_.end(), rtp_module) == + rtp_modules_.end()); + rtp_modules_.push_back(rtp_module); +} + +void PacketRouter::RemoveRtpModule(RtpRtcp* rtp_module) { + CriticalSectionScoped cs(crit_.get()); + rtp_modules_.remove(rtp_module); +} + +bool PacketRouter::TimeToSendPacket(uint32_t ssrc, + uint16_t sequence_number, + int64_t capture_timestamp, + bool retransmission) { + CriticalSectionScoped cs(crit_.get()); + for (auto* rtp_module : rtp_modules_) { + if (rtp_module->SendingMedia() && ssrc == rtp_module->SSRC()) { + return rtp_module->TimeToSendPacket(ssrc, sequence_number, + capture_timestamp, retransmission); + } + } + return true; +} + +size_t PacketRouter::TimeToSendPadding(size_t bytes) { + CriticalSectionScoped cs(crit_.get()); + for (auto* rtp_module : rtp_modules_) { + if (rtp_module->SendingMedia()) + return rtp_module->TimeToSendPadding(bytes); + } + return 0; +} +} // namespace webrtc diff --git a/webrtc/modules/pacing/packet_router_unittest.cc b/webrtc/modules/pacing/packet_router_unittest.cc new file mode 100644 index 0000000000..f7fdf7bbca --- /dev/null +++ b/webrtc/modules/pacing/packet_router_unittest.cc @@ -0,0 +1,151 @@ +/* + * Copyright (c) 2015 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include + +#include "webrtc/base/checks.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "webrtc/modules/pacing/include/packet_router.h" +#include "webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h" +#include "webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h" +#include "webrtc/base/scoped_ptr.h" + +using ::testing::_; +using ::testing::AnyNumber; +using ::testing::NiceMock; +using ::testing::Return; + +namespace webrtc { + +class PacketRouterTest : public ::testing::Test { + public: + PacketRouterTest() : packet_router_(new PacketRouter()) {} + protected: + const rtc::scoped_ptr packet_router_; +}; + +TEST_F(PacketRouterTest, TimeToSendPacket) { + MockRtpRtcp rtp_1; + MockRtpRtcp rtp_2; + packet_router_->AddRtpModule(&rtp_1); + packet_router_->AddRtpModule(&rtp_2); + + const uint16_t kSsrc1 = 1234; + uint16_t sequence_number = 17; + uint64_t timestamp = 7890; + bool retransmission = false; + + // Send on the first module by letting rtp_1 be sending with correct ssrc. + EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(true)); + EXPECT_CALL(rtp_1, SSRC()).Times(1).WillOnce(Return(kSsrc1)); + EXPECT_CALL(rtp_1, TimeToSendPacket(kSsrc1, sequence_number, timestamp, + retransmission)) + .Times(1) + .WillOnce(Return(true)); + EXPECT_CALL(rtp_2, TimeToSendPacket(_, _, _, _)).Times(0); + EXPECT_TRUE(packet_router_->TimeToSendPacket(kSsrc1, sequence_number, + timestamp, retransmission)); + + // Send on the second module by letting rtp_2 be sending, but not rtp_1. + ++sequence_number; + timestamp += 30; + retransmission = true; + const uint16_t kSsrc2 = 4567; + EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(false)); + EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(true)); + EXPECT_CALL(rtp_2, SSRC()).Times(1).WillOnce(Return(kSsrc2)); + EXPECT_CALL(rtp_1, TimeToSendPacket(_, _, _, _)).Times(0); + EXPECT_CALL(rtp_2, TimeToSendPacket(kSsrc2, sequence_number, timestamp, + retransmission)) + .Times(1) + .WillOnce(Return(true)); + EXPECT_TRUE(packet_router_->TimeToSendPacket(kSsrc2, sequence_number, + timestamp, retransmission)); + + // No module is sending, hence no packet should be sent. + EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(false)); + EXPECT_CALL(rtp_1, TimeToSendPacket(_, _, _, _)).Times(0); + EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(false)); + EXPECT_CALL(rtp_2, TimeToSendPacket(_, _, _, _)).Times(0); + EXPECT_TRUE(packet_router_->TimeToSendPacket(kSsrc1, sequence_number, + timestamp, retransmission)); + + // Add a packet with incorrect ssrc and test it's dropped in the router. + EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(true)); + EXPECT_CALL(rtp_1, SSRC()).Times(1).WillOnce(Return(kSsrc1)); + EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(true)); + EXPECT_CALL(rtp_2, SSRC()).Times(1).WillOnce(Return(kSsrc2)); + EXPECT_CALL(rtp_1, TimeToSendPacket(_, _, _, _)).Times(0); + EXPECT_CALL(rtp_2, TimeToSendPacket(_, _, _, _)).Times(0); + EXPECT_TRUE(packet_router_->TimeToSendPacket(kSsrc1 + kSsrc2, sequence_number, + timestamp, retransmission)); + + packet_router_->RemoveRtpModule(&rtp_1); + + // rtp_1 has been removed, try sending a packet on that ssrc and make sure + // it is dropped as expected by not expecting any calls to rtp_1. + EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(true)); + EXPECT_CALL(rtp_2, SSRC()).Times(1).WillOnce(Return(kSsrc2)); + EXPECT_CALL(rtp_2, TimeToSendPacket(_, _, _, _)).Times(0); + EXPECT_TRUE(packet_router_->TimeToSendPacket(kSsrc1, sequence_number, + timestamp, retransmission)); + + packet_router_->RemoveRtpModule(&rtp_2); +} + +TEST_F(PacketRouterTest, TimeToSendPadding) { + MockRtpRtcp rtp_1; + MockRtpRtcp rtp_2; + packet_router_->AddRtpModule(&rtp_1); + packet_router_->AddRtpModule(&rtp_2); + + // Default configuration, sending padding on the first sending module. + const size_t requested_padding_bytes = 1000; + const size_t sent_padding_bytes = 890; + EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(true)); + EXPECT_CALL(rtp_1, TimeToSendPadding(requested_padding_bytes)) + .Times(1) + .WillOnce(Return(sent_padding_bytes)); + EXPECT_CALL(rtp_2, TimeToSendPadding(_)).Times(0); + EXPECT_EQ(sent_padding_bytes, + packet_router_->TimeToSendPadding(requested_padding_bytes)); + + // Let only the second module be sending and verify the padding request is + // routed there. + EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(false)); + EXPECT_CALL(rtp_1, TimeToSendPadding(requested_padding_bytes)).Times(0); + EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(true)); + EXPECT_CALL(rtp_2, TimeToSendPadding(_)) + .Times(1) + .WillOnce(Return(sent_padding_bytes)); + EXPECT_EQ(sent_padding_bytes, + packet_router_->TimeToSendPadding(requested_padding_bytes)); + + // No sending module at all. + EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(false)); + EXPECT_CALL(rtp_1, TimeToSendPadding(requested_padding_bytes)).Times(0); + EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(false)); + EXPECT_CALL(rtp_2, TimeToSendPadding(_)).Times(0); + EXPECT_EQ(static_cast(0), + packet_router_->TimeToSendPadding(requested_padding_bytes)); + + packet_router_->RemoveRtpModule(&rtp_1); + + // rtp_1 has been removed, try sending padding and make sure rtp_1 isn't asked + // to send by not expecting any calls. Instead verify rtp_2 is called. + EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(true)); + EXPECT_CALL(rtp_2, TimeToSendPadding(requested_padding_bytes)).Times(1); + EXPECT_EQ(static_cast(0), + packet_router_->TimeToSendPadding(requested_padding_bytes)); + + packet_router_->RemoveRtpModule(&rtp_2); +} +} // namespace webrtc diff --git a/webrtc/modules/video_coding/main/source/codec_database.cc b/webrtc/modules/video_coding/main/source/codec_database.cc index 7500d3e838..3d887c3aeb 100644 --- a/webrtc/modules/video_coding/main/source/codec_database.cc +++ b/webrtc/modules/video_coding/main/source/codec_database.cc @@ -270,6 +270,9 @@ bool VCMCodecDataBase::SetSendCodec( } } + if (new_send_codec.startBitrate > new_send_codec.maxBitrate) + new_send_codec.startBitrate = new_send_codec.maxBitrate; + if (!reset_required) { reset_required = RequiresEncoderReset(new_send_codec); } @@ -277,7 +280,7 @@ bool VCMCodecDataBase::SetSendCodec( memcpy(&send_codec_, &new_send_codec, sizeof(send_codec_)); if (!reset_required) { - encoded_frame_callback->SetPayloadType(send_codec->plType); + encoded_frame_callback->SetPayloadType(send_codec_.plType); if (ptr_encoder_->RegisterEncodeCallback(encoded_frame_callback) < 0) { LOG(LS_ERROR) << "Failed to register encoded-frame callback."; return false; @@ -287,20 +290,19 @@ bool VCMCodecDataBase::SetSendCodec( // If encoder exists, will destroy it and create new one. DeleteEncoder(); - if (send_codec->plType == external_payload_type_) { + if (send_codec_.plType == external_payload_type_) { // External encoder. ptr_encoder_ = new VCMGenericEncoder( external_encoder_, encoder_rate_observer_, internal_source_); current_enc_is_external_ = true; } else { - ptr_encoder_ = CreateEncoder(send_codec->codecType); + ptr_encoder_ = CreateEncoder(send_codec_.codecType); current_enc_is_external_ = false; if (!ptr_encoder_) return false; } - encoded_frame_callback->SetPayloadType(send_codec->plType); - if (ptr_encoder_->InitEncode(send_codec, - number_of_cores_, + encoded_frame_callback->SetPayloadType(send_codec_.plType); + if (ptr_encoder_->InitEncode(&send_codec_, number_of_cores_, max_payload_size_) < 0) { LOG(LS_ERROR) << "Failed to initialize video encoder."; DeleteEncoder(); diff --git a/webrtc/video/call.cc b/webrtc/video/call.cc index 8d579d3262..c455ba8ee6 100644 --- a/webrtc/video/call.cc +++ b/webrtc/video/call.cc @@ -155,6 +155,7 @@ class Call : public webrtc::Call, public PacketReceiver { ViECodec* codec_; ViERender* render_; ViEBase* base_; + ViENetwork* network_; int base_channel_id_; rtc::scoped_ptr external_render_; @@ -187,12 +188,12 @@ Call::Call(webrtc::VideoEngine* video_engine, const Call::Config& config) DCHECK(video_engine != nullptr); DCHECK(config.send_transport != nullptr); - DCHECK_GE(config.stream_bitrates.min_bitrate_bps, 0); - DCHECK_GE(config.stream_bitrates.start_bitrate_bps, - config.stream_bitrates.min_bitrate_bps); - if (config.stream_bitrates.max_bitrate_bps != -1) { - DCHECK_GE(config.stream_bitrates.max_bitrate_bps, - config.stream_bitrates.start_bitrate_bps); + DCHECK_GE(config.bitrate_config.min_bitrate_bps, 0); + DCHECK_GE(config.bitrate_config.start_bitrate_bps, + config.bitrate_config.min_bitrate_bps); + if (config.bitrate_config.max_bitrate_bps != -1) { + DCHECK_GE(config.bitrate_config.max_bitrate_bps, + config.bitrate_config.start_bitrate_bps); } if (config.overuse_callback) { @@ -211,6 +212,8 @@ Call::Call(webrtc::VideoEngine* video_engine, const Call::Config& config) codec_ = ViECodec::GetInterface(video_engine_); DCHECK(codec_ != nullptr); + network_ = ViENetwork::GetInterface(video_engine_); + // As a workaround for non-existing calls in the old API, create a base // channel used as default channel when creating send and receive streams. base_ = ViEBase::GetInterface(video_engine_); @@ -218,6 +221,11 @@ Call::Call(webrtc::VideoEngine* video_engine, const Call::Config& config) base_->CreateChannel(base_channel_id_); DCHECK(base_channel_id_ != -1); + + network_->SetBitrateConfig(base_channel_id_, + config_.bitrate_config.min_bitrate_bps, + config_.bitrate_config.start_bitrate_bps, + config_.bitrate_config.max_bitrate_bps); } Call::~Call() { @@ -226,6 +234,7 @@ Call::~Call() { render_->DeRegisterVideoRenderModule(*external_render_.get()); base_->Release(); + network_->Release(); codec_->Release(); render_->Release(); rtp_rtcp_->Release(); @@ -245,8 +254,7 @@ VideoSendStream* Call::CreateVideoSendStream( // the call has already started. VideoSendStream* send_stream = new VideoSendStream( config_.send_transport, overuse_observer_proxy_.get(), video_engine_, - config, encoder_config, suspended_send_ssrcs_, base_channel_id_, - config_.stream_bitrates); + config, encoder_config, suspended_send_ssrcs_, base_channel_id_); // This needs to be taken before send_crit_ as both locks need to be held // while changing network state. @@ -380,23 +388,20 @@ void Call::SetBitrateConfig( DCHECK_GE(bitrate_config.min_bitrate_bps, 0); if (bitrate_config.max_bitrate_bps != -1) DCHECK_GT(bitrate_config.max_bitrate_bps, 0); - if (config_.stream_bitrates.min_bitrate_bps == + if (config_.bitrate_config.min_bitrate_bps == bitrate_config.min_bitrate_bps && (bitrate_config.start_bitrate_bps <= 0 || - config_.stream_bitrates.start_bitrate_bps == + config_.bitrate_config.start_bitrate_bps == bitrate_config.start_bitrate_bps) && - config_.stream_bitrates.max_bitrate_bps == + config_.bitrate_config.max_bitrate_bps == bitrate_config.max_bitrate_bps) { // Nothing new to set, early abort to avoid encoder reconfigurations. return; } - config_.stream_bitrates = bitrate_config; - ReadLockScoped read_lock(*send_crit_); - for (std::map::const_iterator it = - send_ssrcs_.begin(); - it != send_ssrcs_.end(); ++it) { - it->second->SetBitrateConfig(bitrate_config); - } + config_.bitrate_config = bitrate_config; + network_->SetBitrateConfig(base_channel_id_, bitrate_config.min_bitrate_bps, + bitrate_config.start_bitrate_bps, + bitrate_config.max_bitrate_bps); } void Call::SignalNetworkState(NetworkState state) { diff --git a/webrtc/video/call_perf_tests.cc b/webrtc/video/call_perf_tests.cc index d182fd11e9..182a83edce 100644 --- a/webrtc/video/call_perf_tests.cc +++ b/webrtc/video/call_perf_tests.cc @@ -639,7 +639,7 @@ TEST_F(CallPerfTest, KeepsHighBitrateWhenReconfiguringSender) { Call::Config GetSenderCallConfig() override { Call::Config config = EndToEndTest::GetSenderCallConfig(); - config.stream_bitrates.start_bitrate_bps = kInitialBitrateKbps * 1000; + config.bitrate_config.start_bitrate_bps = kInitialBitrateKbps * 1000; return config; } diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc index 3f1362cae9..b7f0603062 100644 --- a/webrtc/video/end_to_end_tests.cc +++ b/webrtc/video/end_to_end_tests.cc @@ -625,6 +625,15 @@ void EndToEndTest::TestReceivedFecPacketsNotNacked( return SEND_PACKET; } + // TODO(holmer): Investigate why we don't send FEC packets when the bitrate + // is 10 kbps. + Call::Config GetSenderCallConfig() override { + Call::Config config(SendTransport()); + const int kMinBitrateBps = 30000; + config.bitrate_config.min_bitrate_bps = kMinBitrateBps; + return config; + } + void ModifyConfigs(VideoSendStream::Config* send_config, std::vector* receive_configs, VideoEncoderConfig* encoder_config) override { @@ -1767,7 +1776,7 @@ TEST_F(EndToEndTest, GetStats) { Call::Config GetSenderCallConfig() override { Call::Config config = EndToEndTest::GetSenderCallConfig(); - config.stream_bitrates.start_bitrate_bps = kStartBitrateBps; + config.bitrate_config.start_bitrate_bps = kStartBitrateBps; return config; } diff --git a/webrtc/video/loopback.cc b/webrtc/video/loopback.cc index 99167efbaa..9ef0ec4224 100644 --- a/webrtc/video/loopback.cc +++ b/webrtc/video/loopback.cc @@ -69,11 +69,11 @@ void Loopback::Run() { test::DirectTransport transport(pipe_config); Call::Config call_config(&transport); - call_config.stream_bitrates.min_bitrate_bps = + call_config.bitrate_config.min_bitrate_bps = static_cast(config_.min_bitrate_kbps) * 1000; - call_config.stream_bitrates.start_bitrate_bps = + call_config.bitrate_config.start_bitrate_bps = static_cast(config_.start_bitrate_kbps) * 1000; - call_config.stream_bitrates.max_bitrate_bps = + call_config.bitrate_config.max_bitrate_bps = static_cast(config_.max_bitrate_kbps) * 1000; rtc::scoped_ptr call(Call::Create(call_config)); diff --git a/webrtc/video/rampup_tests.cc b/webrtc/video/rampup_tests.cc index 1e7eeb6309..35d3297f39 100644 --- a/webrtc/video/rampup_tests.cc +++ b/webrtc/video/rampup_tests.cc @@ -404,7 +404,7 @@ void RampUpTest::RunRampUpTest(bool rtx, Call::Config call_config(&stream_observer); if (start_bitrate_bps != 0) { - call_config.stream_bitrates.start_bitrate_bps = start_bitrate_bps; + call_config.bitrate_config.start_bitrate_bps = start_bitrate_bps; stream_observer.set_start_bitrate_bps(start_bitrate_bps); } diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index fcc304f034..4e1e129e87 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -109,25 +109,15 @@ VideoSendStream::VideoSendStream( const VideoSendStream::Config& config, const VideoEncoderConfig& encoder_config, const std::map& suspended_ssrcs, - int base_channel, - Call::Config::BitrateConfig bitrate_config) + int base_channel) : transport_adapter_(transport), encoded_frame_proxy_(config.post_encode_callback), config_(config), - bitrate_config_(bitrate_config), suspended_ssrcs_(suspended_ssrcs), external_codec_(nullptr), channel_(-1), use_config_bitrate_(true), stats_proxy_(Clock::GetRealTimeClock(), config) { - // Duplicate checking of bitrate config. These should be checked in - // Call but are added here for verbosity. - DCHECK_GE(bitrate_config.min_bitrate_bps, 0); - if (bitrate_config.start_bitrate_bps > 0) - DCHECK_GE(bitrate_config.start_bitrate_bps, bitrate_config.min_bitrate_bps); - if (bitrate_config.max_bitrate_bps != -1) - DCHECK_GE(bitrate_config.max_bitrate_bps, bitrate_config.start_bitrate_bps); - video_engine_base_ = ViEBase::GetInterface(video_engine); video_engine_base_->CreateChannelWithoutDefaultEncoder(channel_, base_channel); @@ -405,31 +395,15 @@ bool VideoSendStream::ReconfigureVideoEncoder( video_codec.qpMax = std::max(video_codec.qpMax, static_cast(streams[i].max_qp)); } - // Clamp bitrates to the bitrate config. - if (video_codec.minBitrate < - static_cast(bitrate_config_.min_bitrate_bps / 1000)) { - video_codec.minBitrate = bitrate_config_.min_bitrate_bps / 1000; - } - if (bitrate_config_.max_bitrate_bps != -1 && - video_codec.maxBitrate > - static_cast(bitrate_config_.max_bitrate_bps / 1000)) { - video_codec.maxBitrate = bitrate_config_.max_bitrate_bps / 1000; - } - uint32_t start_bitrate_bps = codec_->GetLastObservedBitrateBps(channel_); - if (start_bitrate_bps == 0 || use_config_bitrate_) { - start_bitrate_bps = bitrate_config_.start_bitrate_bps; - } - video_codec.startBitrate = - static_cast(start_bitrate_bps) / 1000; + + // Set to zero to not update the bitrate controller from ViEEncoder, as + // the bitrate controller is already set from Call. + video_codec.startBitrate = 0; if (video_codec.minBitrate < kViEMinCodecBitrate) video_codec.minBitrate = kViEMinCodecBitrate; if (video_codec.maxBitrate < kViEMinCodecBitrate) video_codec.maxBitrate = kViEMinCodecBitrate; - if (video_codec.startBitrate < video_codec.minBitrate) - video_codec.startBitrate = video_codec.minBitrate; - if (video_codec.startBitrate > video_codec.maxBitrate) - video_codec.startBitrate = video_codec.maxBitrate; DCHECK_GT(streams[0].max_framerate, 0); video_codec.maxFramerate = streams[0].max_framerate; @@ -501,19 +475,6 @@ std::map VideoSendStream::GetRtpStates() const { return rtp_states; } -void VideoSendStream::SetBitrateConfig( - const Call::Config::BitrateConfig& bitrate_config) { - int last_start_bitrate_bps = bitrate_config_.start_bitrate_bps; - bitrate_config_ = bitrate_config; - if (bitrate_config_.start_bitrate_bps <= 0) { - bitrate_config_.start_bitrate_bps = last_start_bitrate_bps; - } else { - // Override start bitrate with bitrate from config. - use_config_bitrate_ = true; - } - ReconfigureVideoEncoder(encoder_config_); -} - void VideoSendStream::SignalNetworkState(Call::NetworkState state) { // When network goes up, enable RTCP status before setting transmission state. // When it goes down, disable RTCP afterwards. This ensures that any packets diff --git a/webrtc/video/video_send_stream.h b/webrtc/video/video_send_stream.h index 898b810283..a5cd1ce77c 100644 --- a/webrtc/video/video_send_stream.h +++ b/webrtc/video/video_send_stream.h @@ -48,8 +48,7 @@ class VideoSendStream : public webrtc::VideoSendStream, const VideoSendStream::Config& config, const VideoEncoderConfig& encoder_config, const std::map& suspended_ssrcs, - int base_channel, - Call::Config::BitrateConfig bitrate_config); + int base_channel); virtual ~VideoSendStream(); @@ -71,7 +70,6 @@ class VideoSendStream : public webrtc::VideoSendStream, typedef std::map RtpStateMap; RtpStateMap GetRtpStates() const; - void SetBitrateConfig(const Call::Config::BitrateConfig& bitrate_config); void SignalNetworkState(Call::NetworkState state); int64_t GetPacerQueuingDelayMs() const; @@ -84,7 +82,6 @@ class VideoSendStream : public webrtc::VideoSendStream, EncodedFrameCallbackAdapter encoded_frame_proxy_; const VideoSendStream::Config config_; VideoEncoderConfig encoder_config_; - Call::Config::BitrateConfig bitrate_config_; std::map suspended_ssrcs_; ViEBase* video_engine_base_; diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc index 9ba3b6206a..3339694414 100644 --- a/webrtc/video/video_send_stream_tests.cc +++ b/webrtc/video/video_send_stream_tests.cc @@ -568,6 +568,13 @@ void VideoSendStreamTest::TestPacketFragmentationSize(VideoFormat format, encoder_.SetFrameSize(static_cast(current_size_frame_.Value())); } + Call::Config GetSenderCallConfig() override { + Call::Config config(SendTransport()); + const int kMinBitrateBps = 30000; + config.bitrate_config.min_bitrate_bps = kMinBitrateBps; + return config; + } + void ModifyConfigs(VideoSendStream::Config* send_config, std::vector* receive_configs, VideoEncoderConfig* encoder_config) override { @@ -1511,7 +1518,7 @@ TEST_F(VideoSendStreamTest, TranslatesTwoLayerScreencastToTargetBitrate) { RunBaseTest(&test); } -TEST_F(VideoSendStreamTest, UsesCallStreamBitratesAndCanReconfigureBitrates) { +TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { // These are chosen to be "kind of odd" to not be accidentally checked against // default values. static const int kMinBitrateKbps = 137; @@ -1560,9 +1567,9 @@ TEST_F(VideoSendStreamTest, UsesCallStreamBitratesAndCanReconfigureBitrates) { Call::Config GetSenderCallConfig() override { Call::Config config(SendTransport()); - config.stream_bitrates.min_bitrate_bps = kMinBitrateKbps * 1000; - config.stream_bitrates.start_bitrate_bps = kStartBitrateKbps * 1000; - config.stream_bitrates.max_bitrate_bps = kMaxBitrateKbps * 1000; + config.bitrate_config.min_bitrate_bps = kMinBitrateKbps * 1000; + config.bitrate_config.start_bitrate_bps = kStartBitrateKbps * 1000; + config.bitrate_config.max_bitrate_bps = kMaxBitrateKbps * 1000; return config; } @@ -1572,35 +1579,46 @@ TEST_F(VideoSendStreamTest, UsesCallStreamBitratesAndCanReconfigureBitrates) { send_config->encoder_settings.encoder = this; // Set bitrates lower/higher than min/max to make sure they are properly // capped. - encoder_config->streams.front().min_bitrate_bps = - (kMinBitrateKbps - 10) * 1000; - encoder_config->streams.front().max_bitrate_bps = - (kIncreasedMaxBitrateKbps + 10) * 1000; + encoder_config->streams.front().min_bitrate_bps = kMinBitrateKbps * 1000; + encoder_config->streams.front().max_bitrate_bps = kMaxBitrateKbps * 1000; + encoder_config_ = *encoder_config; } void OnCallsCreated(Call* sender_call, Call* receiver_call) override { call_ = sender_call; } + void OnStreamsCreated( + VideoSendStream* send_stream, + const std::vector& receive_streams) override { + send_stream_ = send_stream; + } + void PerformTest() override { - EXPECT_EQ(kEventSignaled, Wait()) - << "Timed out while waiting encoder to be configured."; Call::Config::BitrateConfig bitrate_config; - bitrate_config.min_bitrate_bps = 0; - bitrate_config.start_bitrate_bps = -1; - bitrate_config.max_bitrate_bps = kLowerMaxBitrateKbps * 1000; - call_->SetBitrateConfig(bitrate_config); - EXPECT_EQ(2, num_initializations_) - << "Encoder should have been reconfigured with the new value."; bitrate_config.start_bitrate_bps = kIncreasedStartBitrateKbps * 1000; bitrate_config.max_bitrate_bps = kIncreasedMaxBitrateKbps * 1000; call_->SetBitrateConfig(bitrate_config); + EXPECT_EQ(kEventSignaled, Wait()) + << "Timed out while waiting encoder to be configured."; + encoder_config_.streams[0].min_bitrate_bps = 0; + encoder_config_.streams[0].max_bitrate_bps = kLowerMaxBitrateKbps * 1000; + send_stream_->ReconfigureVideoEncoder(encoder_config_); + EXPECT_EQ(2, num_initializations_) + << "Encoder should have been reconfigured with the new value."; + encoder_config_.streams[0].target_bitrate_bps = + encoder_config_.streams[0].min_bitrate_bps; + encoder_config_.streams[0].max_bitrate_bps = + kIncreasedMaxBitrateKbps * 1000; + send_stream_->ReconfigureVideoEncoder(encoder_config_); EXPECT_EQ(3, num_initializations_) << "Encoder should have been reconfigured with the new value."; } int num_initializations_; webrtc::Call* call_; + webrtc::VideoSendStream* send_stream_; + webrtc::VideoEncoderConfig encoder_config_; } test; RunBaseTest(&test); diff --git a/webrtc/video_engine/encoder_state_feedback_unittest.cc b/webrtc/video_engine/encoder_state_feedback_unittest.cc index 59a1b29024..eab33d407a 100644 --- a/webrtc/video_engine/encoder_state_feedback_unittest.cc +++ b/webrtc/video_engine/encoder_state_feedback_unittest.cc @@ -17,8 +17,11 @@ #include "webrtc/base/scoped_ptr.h" #include "webrtc/common.h" +#include "webrtc/modules/pacing/include/paced_sender.h" +#include "webrtc/modules/pacing/include/packet_router.h" #include "webrtc/modules/rtp_rtcp/interface/rtp_rtcp_defines.h" #include "webrtc/modules/utility/interface/mock/mock_process_thread.h" +#include "webrtc/video_engine/payload_router.h" #include "webrtc/video_engine/vie_encoder.h" using ::testing::NiceMock; @@ -27,8 +30,8 @@ namespace webrtc { class MockVieEncoder : public ViEEncoder { public: - explicit MockVieEncoder(ProcessThread* process_thread) - : ViEEncoder(1, 1, config_, *process_thread, NULL, NULL, false) {} + explicit MockVieEncoder(ProcessThread* process_thread, PacedSender* pacer) + : ViEEncoder(1, 1, config_, *process_thread, pacer, NULL, NULL, false) {} ~MockVieEncoder() {} MOCK_METHOD1(OnReceivedIntraFrameRequest, @@ -45,17 +48,26 @@ class MockVieEncoder : public ViEEncoder { class VieKeyRequestTest : public ::testing::Test { protected: + VieKeyRequestTest() + : pacer_(Clock::GetRealTimeClock(), + &router_, + BitrateController::kDefaultStartBitrateKbps, + PacedSender::kDefaultPaceMultiplier * + BitrateController::kDefaultStartBitrateKbps, + 0) {} virtual void SetUp() { process_thread_.reset(new NiceMock); encoder_state_feedback_.reset(new EncoderStateFeedback()); } rtc::scoped_ptr process_thread_; rtc::scoped_ptr encoder_state_feedback_; + PacketRouter router_; + PacedSender pacer_; }; TEST_F(VieKeyRequestTest, CreateAndTriggerRequests) { const int ssrc = 1234; - MockVieEncoder encoder(process_thread_.get()); + MockVieEncoder encoder(process_thread_.get(), &pacer_); EXPECT_TRUE(encoder_state_feedback_->AddEncoder(ssrc, &encoder)); EXPECT_CALL(encoder, OnReceivedIntraFrameRequest(ssrc)) @@ -83,8 +95,8 @@ TEST_F(VieKeyRequestTest, CreateAndTriggerRequests) { TEST_F(VieKeyRequestTest, MultipleEncoders) { const int ssrc_1 = 1234; const int ssrc_2 = 5678; - MockVieEncoder encoder_1(process_thread_.get()); - MockVieEncoder encoder_2(process_thread_.get()); + MockVieEncoder encoder_1(process_thread_.get(), &pacer_); + MockVieEncoder encoder_2(process_thread_.get(), &pacer_); EXPECT_TRUE(encoder_state_feedback_->AddEncoder(ssrc_1, &encoder_1)); EXPECT_TRUE(encoder_state_feedback_->AddEncoder(ssrc_2, &encoder_2)); @@ -129,7 +141,7 @@ TEST_F(VieKeyRequestTest, MultipleEncoders) { TEST_F(VieKeyRequestTest, AddTwiceError) { const int ssrc = 1234; - MockVieEncoder encoder(process_thread_.get()); + MockVieEncoder encoder(process_thread_.get(), &pacer_); EXPECT_TRUE(encoder_state_feedback_->AddEncoder(ssrc, &encoder)); EXPECT_FALSE(encoder_state_feedback_->AddEncoder(ssrc, &encoder)); encoder_state_feedback_->RemoveEncoder(&encoder); diff --git a/webrtc/video_engine/include/vie_network.h b/webrtc/video_engine/include/vie_network.h index fe7a41f850..e962e729aa 100644 --- a/webrtc/video_engine/include/vie_network.h +++ b/webrtc/video_engine/include/vie_network.h @@ -47,6 +47,11 @@ class WEBRTC_DLLEXPORT ViENetwork { // for all sub-API:s before the VideoEngine object can be safely deleted. virtual int Release() = 0; + virtual void SetBitrateConfig(int video_channel, + int min_bitrate_bps, + int start_bitrate_bps, + int max_bitrate_bps) = 0; + // Inform the engine about if the network adapter is currently transmitting // packets or not. virtual void SetNetworkTransmissionState(const int video_channel, diff --git a/webrtc/video_engine/payload_router.cc b/webrtc/video_engine/payload_router.cc index 7d25f10c90..958cdd869c 100644 --- a/webrtc/video_engine/payload_router.cc +++ b/webrtc/video_engine/payload_router.cc @@ -74,20 +74,6 @@ bool PayloadRouter::RoutePayload(FrameType frame_type, payload_length, fragmentation, rtp_video_hdr) == 0 ? true : false; } -bool PayloadRouter::TimeToSendPacket(uint32_t ssrc, - uint16_t sequence_number, - int64_t capture_timestamp, - bool retransmission) { - CriticalSectionScoped cs(crit_.get()); - for (auto* rtp_module : rtp_modules_) { - if (rtp_module->SendingMedia() && ssrc == rtp_module->SSRC()) { - return rtp_module->TimeToSendPacket(ssrc, sequence_number, - capture_timestamp, retransmission); - } - } - return true; -} - void PayloadRouter::SetTargetSendBitrates( const std::vector& stream_bitrates) { CriticalSectionScoped cs(crit_.get()); @@ -101,15 +87,6 @@ void PayloadRouter::SetTargetSendBitrates( } } -size_t PayloadRouter::TimeToSendPadding(size_t bytes) { - CriticalSectionScoped cs(crit_.get()); - for(auto* rtp_module : rtp_modules_) { - if (rtp_module->SendingMedia()) - return rtp_module->TimeToSendPadding(bytes); - } - return 0; -} - size_t PayloadRouter::MaxPayloadLength() const { size_t min_payload_length = DefaultMaxPayloadLength(); CriticalSectionScoped cs(crit_.get()); diff --git a/webrtc/video_engine/payload_router.h b/webrtc/video_engine/payload_router.h index 37b31bf1f8..b96defd5e3 100644 --- a/webrtc/video_engine/payload_router.h +++ b/webrtc/video_engine/payload_router.h @@ -55,16 +55,6 @@ class PayloadRouter { const RTPFragmentationHeader* fragmentation, const RTPVideoHeader* rtp_video_hdr); - // Called when it's time to send a stored packet. - bool TimeToSendPacket(uint32_t ssrc, - uint16_t sequence_number, - int64_t capture_timestamp, - bool retransmission); - - // Called when it's time to send padding, returns the number of bytes actually - // sent. - size_t TimeToSendPadding(size_t bytes); - // Configures current target bitrate per module. 'stream_bitrates' is assumed // to be in the same order as 'SetSendingRtpModules'. void SetTargetSendBitrates(const std::vector& stream_bitrates); diff --git a/webrtc/video_engine/payload_router_unittest.cc b/webrtc/video_engine/payload_router_unittest.cc index 5760e68809..de391576d8 100644 --- a/webrtc/video_engine/payload_router_unittest.cc +++ b/webrtc/video_engine/payload_router_unittest.cc @@ -172,147 +172,6 @@ TEST_F(PayloadRouterTest, MaxPayloadLength) { EXPECT_EQ(kTestMinPayloadLength, payload_router_->MaxPayloadLength()); } -TEST_F(PayloadRouterTest, TimeToSendPacket) { - MockRtpRtcp rtp_1; - MockRtpRtcp rtp_2; - std::list modules; - modules.push_back(&rtp_1); - modules.push_back(&rtp_2); - payload_router_->SetSendingRtpModules(modules); - - const uint16_t kSsrc1 = 1234; - uint16_t sequence_number = 17; - uint64_t timestamp = 7890; - bool retransmission = false; - - // Send on the first module by letting rtp_1 be sending with correct ssrc. - EXPECT_CALL(rtp_1, SendingMedia()) - .Times(1) - .WillOnce(Return(true)); - EXPECT_CALL(rtp_1, SSRC()) - .Times(1) - .WillOnce(Return(kSsrc1)); - EXPECT_CALL(rtp_1, TimeToSendPacket(kSsrc1, sequence_number, timestamp, - retransmission)) - .Times(1) - .WillOnce(Return(true)); - EXPECT_CALL(rtp_2, TimeToSendPacket(_, _, _, _)) - .Times(0); - EXPECT_TRUE(payload_router_->TimeToSendPacket( - kSsrc1, sequence_number, timestamp, retransmission)); - - - // Send on the second module by letting rtp_2 be sending, but not rtp_1. - ++sequence_number; - timestamp += 30; - retransmission = true; - const uint16_t kSsrc2 = 4567; - EXPECT_CALL(rtp_1, SendingMedia()) - .Times(1) - .WillOnce(Return(false)); - EXPECT_CALL(rtp_2, SendingMedia()) - .Times(1) - .WillOnce(Return(true)); - EXPECT_CALL(rtp_2, SSRC()) - .Times(1) - .WillOnce(Return(kSsrc2)); - EXPECT_CALL(rtp_1, TimeToSendPacket(_, _, _, _)) - .Times(0); - EXPECT_CALL(rtp_2, TimeToSendPacket(kSsrc2, sequence_number, timestamp, - retransmission)) - .Times(1) - .WillOnce(Return(true)); - EXPECT_TRUE(payload_router_->TimeToSendPacket( - kSsrc2, sequence_number, timestamp, retransmission)); - - // No module is sending, hence no packet should be sent. - EXPECT_CALL(rtp_1, SendingMedia()) - .Times(1) - .WillOnce(Return(false)); - EXPECT_CALL(rtp_1, TimeToSendPacket(_, _, _,_)) - .Times(0); - EXPECT_CALL(rtp_2, SendingMedia()) - .Times(1) - .WillOnce(Return(false)); - EXPECT_CALL(rtp_2, TimeToSendPacket(_, _, _, _)) - .Times(0); - EXPECT_TRUE(payload_router_->TimeToSendPacket( - kSsrc1, sequence_number, timestamp, retransmission)); - - // Add a packet with incorrect ssrc and test it's dropped in the router. - EXPECT_CALL(rtp_1, SendingMedia()) - .Times(1) - .WillOnce(Return(true)); - EXPECT_CALL(rtp_1, SSRC()) - .Times(1) - .WillOnce(Return(kSsrc1)); - EXPECT_CALL(rtp_2, SendingMedia()) - .Times(1) - .WillOnce(Return(true)); - EXPECT_CALL(rtp_2, SSRC()) - .Times(1) - .WillOnce(Return(kSsrc2)); - EXPECT_CALL(rtp_1, TimeToSendPacket(_, _, _,_)) - .Times(0); - EXPECT_CALL(rtp_2, TimeToSendPacket(_, _, _, _)) - .Times(0); - EXPECT_TRUE(payload_router_->TimeToSendPacket( - kSsrc1 + kSsrc2, sequence_number, timestamp, retransmission)); -} - -TEST_F(PayloadRouterTest, TimeToSendPadding) { - MockRtpRtcp rtp_1; - MockRtpRtcp rtp_2; - std::list modules; - modules.push_back(&rtp_1); - modules.push_back(&rtp_2); - payload_router_->SetSendingRtpModules(modules); - - // Default configuration, sending padding on the first sending module. - const size_t requested_padding_bytes = 1000; - const size_t sent_padding_bytes = 890; - EXPECT_CALL(rtp_1, SendingMedia()) - .Times(1) - .WillOnce(Return(true)); - EXPECT_CALL(rtp_1, TimeToSendPadding(requested_padding_bytes)) - .Times(1) - .WillOnce(Return(sent_padding_bytes)); - EXPECT_CALL(rtp_2, TimeToSendPadding(_)) - .Times(0); - EXPECT_EQ(sent_padding_bytes, - payload_router_->TimeToSendPadding(requested_padding_bytes)); - - // Let only the second module be sending and verify the padding request is - // routed there. - EXPECT_CALL(rtp_1, SendingMedia()) - .Times(1) - .WillOnce(Return(false)); - EXPECT_CALL(rtp_1, TimeToSendPadding(requested_padding_bytes)) - .Times(0); - EXPECT_CALL(rtp_2, SendingMedia()) - .Times(1) - .WillOnce(Return(true)); - EXPECT_CALL(rtp_2, TimeToSendPadding(_)) - .Times(1) - .WillOnce(Return(sent_padding_bytes)); - EXPECT_EQ(sent_padding_bytes, - payload_router_->TimeToSendPadding(requested_padding_bytes)); - - // No sending module at all. - EXPECT_CALL(rtp_1, SendingMedia()) - .Times(1) - .WillOnce(Return(false)); - EXPECT_CALL(rtp_1, TimeToSendPadding(requested_padding_bytes)) - .Times(0); - EXPECT_CALL(rtp_2, SendingMedia()) - .Times(1) - .WillOnce(Return(false)); - EXPECT_CALL(rtp_2, TimeToSendPadding(_)) - .Times(0); - EXPECT_EQ(static_cast(0), - payload_router_->TimeToSendPadding(requested_padding_bytes)); -} - TEST_F(PayloadRouterTest, SetTargetSendBitrates) { MockRtpRtcp rtp_1; MockRtpRtcp rtp_2; @@ -346,5 +205,5 @@ TEST_F(PayloadRouterTest, SetTargetSendBitrates) { EXPECT_CALL(rtp_2, SetTargetSendBitrate(bitrate_2)) .Times(1); payload_router_->SetTargetSendBitrates(bitrates); - } +} } // namespace webrtc diff --git a/webrtc/video_engine/test/auto_test/source/vie_autotest_codec.cc b/webrtc/video_engine/test/auto_test/source/vie_autotest_codec.cc index 5345dc84d1..4538206310 100644 --- a/webrtc/video_engine/test/auto_test/source/vie_autotest_codec.cc +++ b/webrtc/video_engine/test/auto_test/source/vie_autotest_codec.cc @@ -514,7 +514,9 @@ void ViEAutoTest::ViECodecAPITest() { video_codec.startBitrate = 50; EXPECT_EQ(0, codec->SetSendCodec(video_channel, video_codec)); EXPECT_EQ(0, codec->GetSendCodec(video_channel, video_codec)); - EXPECT_EQ(kMinBitrate, video_codec.startBitrate); + // We don't allow allocated start bitrate to be decreased via SetSendCodec, + // and the default bitrate available in the allocator is 300. + EXPECT_EQ(300, video_codec.startBitrate); memset(&video_codec, 0, sizeof(video_codec)); EXPECT_EQ(0, codec->GetSendCodec(video_channel, video_codec)); diff --git a/webrtc/video_engine/vie_channel.cc b/webrtc/video_engine/vie_channel.cc index a6d075fa40..82555d79b6 100644 --- a/webrtc/video_engine/vie_channel.cc +++ b/webrtc/video_engine/vie_channel.cc @@ -19,6 +19,7 @@ #include "webrtc/experiments.h" #include "webrtc/frame_callback.h" #include "webrtc/modules/pacing/include/paced_sender.h" +#include "webrtc/modules/pacing/include/packet_router.h" #include "webrtc/modules/rtp_rtcp/interface/rtp_receiver.h" #include "webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h" #include "webrtc/modules/utility/interface/process_thread.h" @@ -91,6 +92,7 @@ ViEChannel::ViEChannel(int32_t channel_id, RemoteBitrateEstimator* remote_bitrate_estimator, RtcpRttStats* rtt_stats, PacedSender* paced_sender, + PacketRouter* packet_router, bool sender, bool disable_default_encoder) : ViEFrameProviderBase(channel_id, engine_id), @@ -115,6 +117,7 @@ ViEChannel::ViEChannel(int32_t channel_id, intra_frame_observer_(intra_frame_observer), rtt_stats_(rtt_stats), paced_sender_(paced_sender), + packet_router_(packet_router), bandwidth_observer_(bandwidth_observer), send_timestamp_extension_id_(kInvalidRtpExtensionId), absolute_send_time_extension_id_(kInvalidRtpExtensionId), @@ -153,9 +156,10 @@ int32_t ViEChannel::Init() { rtp_rtcp_->SetStorePacketsStatus(true, nack_history_size_sender_); } if (sender_) { - std::list send_rtp_modules(1, rtp_rtcp_.get()); - send_payload_router_->SetSendingRtpModules(send_rtp_modules); - DCHECK(!send_payload_router_->active()); + packet_router_->AddRtpModule(rtp_rtcp_.get()); + std::list send_rtp_modules(1, rtp_rtcp_.get()); + send_payload_router_->SetSendingRtpModules(send_rtp_modules); + DCHECK(!send_payload_router_->active()); } if (vcm_->InitializeReceiver() != 0) { return -1; @@ -203,9 +207,11 @@ ViEChannel::~ViEChannel() { module_process_thread_.DeRegisterModule(vcm_); module_process_thread_.DeRegisterModule(&vie_sync_); send_payload_router_->SetSendingRtpModules(std::list()); + packet_router_->RemoveRtpModule(rtp_rtcp_.get()); while (simulcast_rtp_rtcp_.size() > 0) { std::list::iterator it = simulcast_rtp_rtcp_.begin(); RtpRtcp* rtp_rtcp = *it; + packet_router_->RemoveRtpModule(rtp_rtcp); module_process_thread_.DeRegisterModule(rtp_rtcp); delete rtp_rtcp; simulcast_rtp_rtcp_.erase(it); @@ -355,6 +361,9 @@ int32_t ViEChannel::SetSendCodec(const VideoCodec& video_codec, bool router_was_active = send_payload_router_->active(); send_payload_router_->set_active(false); send_payload_router_->SetSendingRtpModules(std::list()); + packet_router_->RemoveRtpModule(rtp_rtcp_.get()); + for (RtpRtcp* module : simulcast_rtp_rtcp_) + packet_router_->RemoveRtpModule(module); if (rtp_rtcp_->Sending() && new_stream) { restart_rtp = true; rtp_rtcp_->SetSendingStatus(false); @@ -526,7 +535,11 @@ int32_t ViEChannel::SetSendCodec(const VideoCodec& video_codec, (*it)->SetSendingMediaStatus(true); } } - // Update the packet router with the sending RTP RTCP modules. + // Update the packet and payload routers with the sending RTP RTCP modules. + packet_router_->AddRtpModule(rtp_rtcp_.get()); + for (RtpRtcp* module : simulcast_rtp_rtcp_) + packet_router_->AddRtpModule(module); + std::list active_send_modules; active_send_modules.push_back(rtp_rtcp_.get()); for (std::list::const_iterator cit = simulcast_rtp_rtcp_.begin(); diff --git a/webrtc/video_engine/vie_channel.h b/webrtc/video_engine/vie_channel.h index c2c3847153..47724d1fb6 100644 --- a/webrtc/video_engine/vie_channel.h +++ b/webrtc/video_engine/vie_channel.h @@ -39,6 +39,7 @@ class CriticalSectionWrapper; class EncodedImageCallback; class I420FrameCallback; class PacedSender; +class PacketRouter; class PayloadRouter; class ProcessThread; class ReceiveStatisticsProxy; @@ -76,6 +77,7 @@ class ViEChannel RemoteBitrateEstimator* remote_bitrate_estimator, RtcpRttStats* rtt_stats, PacedSender* paced_sender, + PacketRouter* packet_router, bool sender, bool disable_default_encoder); ~ViEChannel(); @@ -524,6 +526,7 @@ class ViEChannel RtcpIntraFrameObserver* intra_frame_observer_; RtcpRttStats* rtt_stats_; PacedSender* paced_sender_; + PacketRouter* packet_router_; rtc::scoped_ptr bandwidth_observer_; int send_timestamp_extension_id_; diff --git a/webrtc/video_engine/vie_channel_group.cc b/webrtc/video_engine/vie_channel_group.cc index afa2d8b543..3508d6a7f9 100644 --- a/webrtc/video_engine/vie_channel_group.cc +++ b/webrtc/video_engine/vie_channel_group.cc @@ -14,6 +14,8 @@ #include "webrtc/base/thread_annotations.h" #include "webrtc/common.h" #include "webrtc/experiments.h" +#include "webrtc/modules/pacing/include/paced_sender.h" +#include "webrtc/modules/pacing/include/packet_router.h" #include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" #include "webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h" #include "webrtc/modules/utility/interface/process_thread.h" @@ -142,14 +144,25 @@ class WrappingBitrateEstimator : public RemoteBitrateEstimator { ChannelGroup::ChannelGroup(ProcessThread* process_thread, const Config* config) : remb_(new VieRemb()), bitrate_allocator_(new BitrateAllocator()), - bitrate_controller_( - BitrateController::CreateBitrateController(Clock::GetRealTimeClock(), - this)), call_stats_(new CallStats()), encoder_state_feedback_(new EncoderStateFeedback()), + packet_router_(new PacketRouter()), + pacer_(new PacedSender(Clock::GetRealTimeClock(), + packet_router_.get(), + BitrateController::kDefaultStartBitrateKbps, + PacedSender::kDefaultPaceMultiplier * + BitrateController::kDefaultStartBitrateKbps, + 0)), + encoder_map_cs_(CriticalSectionWrapper::CreateCriticalSection()), config_(config), own_config_(), - process_thread_(process_thread) { + process_thread_(process_thread), + pacer_thread_(ProcessThread::Create()), + // Constructed last as this object calls the provided callback on + // construction. + bitrate_controller_( + BitrateController::CreateBitrateController(Clock::GetRealTimeClock(), + this)) { if (!config) { own_config_.reset(new Config); config_ = own_config_.get(); @@ -163,12 +176,17 @@ ChannelGroup::ChannelGroup(ProcessThread* process_thread, const Config* config) call_stats_->RegisterStatsObserver(remote_bitrate_estimator_.get()); + pacer_thread_->RegisterModule(pacer_.get()); + pacer_thread_->Start(); + process_thread->RegisterModule(remote_bitrate_estimator_.get()); process_thread->RegisterModule(call_stats_.get()); process_thread->RegisterModule(bitrate_controller_.get()); } ChannelGroup::~ChannelGroup() { + pacer_thread_->Stop(); + pacer_thread_->DeRegisterModule(pacer_.get()); process_thread_->DeRegisterModule(bitrate_controller_.get()); process_thread_->DeRegisterModule(call_stats_.get()); process_thread_->DeRegisterModule(remote_bitrate_estimator_.get()); @@ -184,7 +202,7 @@ bool ChannelGroup::CreateSendChannel(int channel_id, int number_of_cores, bool disable_default_encoder) { rtc::scoped_ptr vie_encoder(new ViEEncoder( - channel_id, number_of_cores, *config_, *process_thread_, + channel_id, number_of_cores, *config_, *process_thread_, pacer_.get(), bitrate_allocator_.get(), bitrate_controller_.get(), false)); if (!vie_encoder->Init()) { return false; @@ -233,7 +251,7 @@ bool ChannelGroup::CreateChannel(int channel_id, encoder_state_feedback_->GetRtcpIntraFrameObserver(), bitrate_controller_->CreateRtcpBandwidthObserver(), remote_bitrate_estimator_.get(), call_stats_->rtcp_rtt_stats(), - vie_encoder->GetPacedSender(), sender, disable_default_encoder)); + pacer_.get(), packet_router_.get(), sender, disable_default_encoder)); if (channel->Init() != 0) { return false; } @@ -252,7 +270,10 @@ bool ChannelGroup::CreateChannel(int channel_id, // Store the channel, add it to the channel group and save the vie_encoder. channel_map_[channel_id] = channel.release(); - vie_encoder_map_[channel_id] = vie_encoder; + { + CriticalSectionScoped lock(encoder_map_cs_.get()); + vie_encoder_map_[channel_id] = vie_encoder; + } return true; } @@ -328,9 +349,9 @@ ViEChannel* ChannelGroup::GetChannel(int channel_id) const { } ViEEncoder* ChannelGroup::GetEncoder(int channel_id) const { + CriticalSectionScoped lock(encoder_map_cs_.get()); EncoderMap::const_iterator it = vie_encoder_map_.find(channel_id); if (it == vie_encoder_map_.end()) { - printf("No encoder found: %d\n", channel_id); return NULL; } return it->second; @@ -346,6 +367,7 @@ ViEChannel* ChannelGroup::PopChannel(int channel_id) { } ViEEncoder* ChannelGroup::PopEncoder(int channel_id) { + CriticalSectionScoped lock(encoder_map_cs_.get()); EncoderMap::iterator e_it = vie_encoder_map_.find(channel_id); DCHECK(e_it != vie_encoder_map_.end()); ViEEncoder* encoder = e_it->second; @@ -362,6 +384,7 @@ std::vector ChannelGroup::GetChannelIds() const { } bool ChannelGroup::OtherChannelsUsingEncoder(int channel_id) const { + CriticalSectionScoped lock(encoder_map_cs_.get()); EncoderMap::const_iterator orig_it = vie_encoder_map_.find(channel_id); if (orig_it == vie_encoder_map_.end()) { // No ViEEncoder for this channel. @@ -390,6 +413,7 @@ void ChannelGroup::SetSyncInterface(VoEVideoSync* sync_interface) { void ChannelGroup::GetChannelsUsingEncoder(int channel_id, ChannelList* channels) const { + CriticalSectionScoped lock(encoder_map_cs_.get()); EncoderMap::const_iterator orig_it = vie_encoder_map_.find(channel_id); for (ChannelMap::const_iterator c_it = channel_map_.begin(); @@ -418,6 +442,10 @@ EncoderStateFeedback* ChannelGroup::GetEncoderStateFeedback() const { return encoder_state_feedback_.get(); } +int64_t ChannelGroup::GetPacerQueuingDelayMs() const { + return pacer_->QueueInMs(); +} + void ChannelGroup::SetChannelRembStatus(int channel_id, bool sender, bool receiver, @@ -442,5 +470,17 @@ void ChannelGroup::OnNetworkChanged(uint32_t target_bitrate_bps, uint8_t fraction_loss, int64_t rtt) { bitrate_allocator_->OnNetworkChanged(target_bitrate_bps, fraction_loss, rtt); + int pad_up_to_bitrate_bps = 0; + { + CriticalSectionScoped lock(encoder_map_cs_.get()); + for (const auto& encoder : vie_encoder_map_) { + pad_up_to_bitrate_bps += + encoder.second->GetPaddingNeededBps(target_bitrate_bps); + } + } + pacer_->UpdateBitrate( + target_bitrate_bps / 1000, + PacedSender::kDefaultPaceMultiplier * target_bitrate_bps / 1000, + pad_up_to_bitrate_bps / 1000); } } // namespace webrtc diff --git a/webrtc/video_engine/vie_channel_group.h b/webrtc/video_engine/vie_channel_group.h index a5fad4129a..546da690b3 100644 --- a/webrtc/video_engine/vie_channel_group.h +++ b/webrtc/video_engine/vie_channel_group.h @@ -25,6 +25,8 @@ class BitrateAllocator; class CallStats; class Config; class EncoderStateFeedback; +class PacedSender; +class PacketRouter; class ProcessThread; class RemoteBitrateEstimator; class ViEChannel; @@ -71,6 +73,7 @@ class ChannelGroup : public BitrateObserver { CallStats* GetCallStats() const; RemoteBitrateEstimator* GetRemoteBitrateEstimator() const; EncoderStateFeedback* GetEncoderStateFeedback() const; + int64_t GetPacerQueuingDelayMs() const; // Implements BitrateObserver. void OnNetworkChanged(uint32_t target_bitrate_bps, @@ -93,20 +96,26 @@ class ChannelGroup : public BitrateObserver { rtc::scoped_ptr remb_; rtc::scoped_ptr bitrate_allocator_; - rtc::scoped_ptr bitrate_controller_; rtc::scoped_ptr call_stats_; rtc::scoped_ptr remote_bitrate_estimator_; rtc::scoped_ptr encoder_state_feedback_; + rtc::scoped_ptr packet_router_; + rtc::scoped_ptr pacer_; ChannelSet channels_; ChannelMap channel_map_; // Maps Channel id -> ViEEncoder. EncoderMap vie_encoder_map_; + rtc::scoped_ptr encoder_map_cs_; + const Config* config_; // Placeholder for the case where this owns the config. rtc::scoped_ptr own_config_; // Registered at construct time and assumed to outlive this class. ProcessThread* process_thread_; + rtc::scoped_ptr pacer_thread_; + + rtc::scoped_ptr bitrate_controller_; }; } // namespace webrtc diff --git a/webrtc/video_engine/vie_channel_manager.cc b/webrtc/video_engine/vie_channel_manager.cc index 4c4c66fedb..c06b7e3f46 100644 --- a/webrtc/video_engine/vie_channel_manager.cc +++ b/webrtc/video_engine/vie_channel_manager.cc @@ -288,6 +288,31 @@ bool ViEChannelManager::GetEstimatedReceiveBandwidth( return true; } +bool ViEChannelManager::GetPacerQueuingDelayMs(int channel_id, + int64_t* delay_ms) const { + CriticalSectionScoped cs(channel_id_critsect_); + ChannelGroup* group = FindGroup(channel_id); + if (!group) + return false; + *delay_ms = group->GetPacerQueuingDelayMs(); + return true; +} + +bool ViEChannelManager::SetBitrateConfig(int channel_id, + int min_bitrate_bps, + int start_bitrate_bps, + int max_bitrate_bps) { + CriticalSectionScoped cs(channel_id_critsect_); + ChannelGroup* group = FindGroup(channel_id); + if (!group) + return false; + BitrateController* bitrate_controller = group->GetBitrateController(); + if (start_bitrate_bps > 0) + bitrate_controller->SetStartBitrate(start_bitrate_bps); + bitrate_controller->SetMinMaxBitrate(min_bitrate_bps, max_bitrate_bps); + return true; +} + ViEChannel* ViEChannelManager::ViEChannelPtr(int channel_id) const { CriticalSectionScoped cs(channel_id_critsect_); ChannelGroup* group = FindGroup(channel_id); diff --git a/webrtc/video_engine/vie_channel_manager.h b/webrtc/video_engine/vie_channel_manager.h index c4a19b5b4c..1732d79238 100644 --- a/webrtc/video_engine/vie_channel_manager.h +++ b/webrtc/video_engine/vie_channel_manager.h @@ -86,6 +86,15 @@ class ViEChannelManager: private ViEManagerBase { bool GetEstimatedReceiveBandwidth(int channel_id, uint32_t* estimated_bandwidth) const; + bool GetPacerQueuingDelayMs(int channel_id, int64_t* delay_ms) const; + + bool SetBitrateConfig(int channel_id, + int min_bitrate_bps, + int start_bitrate_bps, + int max_bitrate_bps); + + bool ReAllocateBitrates(int channel_id); + private: // Used by ViEChannelScoped, forcing a manager user to use scoped. // Returns a pointer to the channel with id 'channel_id'. diff --git a/webrtc/video_engine/vie_codec_impl.cc b/webrtc/video_engine/vie_codec_impl.cc index 74d4f68382..3d696812b7 100644 --- a/webrtc/video_engine/vie_codec_impl.cc +++ b/webrtc/video_engine/vie_codec_impl.cc @@ -181,11 +181,13 @@ int ViECodecImpl::SetSendCodec(const int video_channel, LOG(LS_INFO) << "New max bitrate set " << video_codec_internal.maxBitrate; } - if (video_codec_internal.startBitrate < video_codec_internal.minBitrate) { - video_codec_internal.startBitrate = video_codec_internal.minBitrate; - } - if (video_codec_internal.startBitrate > video_codec_internal.maxBitrate) { - video_codec_internal.startBitrate = video_codec_internal.maxBitrate; + if (video_codec_internal.startBitrate > 0) { + if (video_codec_internal.startBitrate < video_codec_internal.minBitrate) { + video_codec_internal.startBitrate = video_codec_internal.minBitrate; + } + if (video_codec_internal.startBitrate > video_codec_internal.maxBitrate) { + video_codec_internal.startBitrate = video_codec_internal.maxBitrate; + } } // Make sure to generate a new SSRC if the codec type and/or resolution has @@ -669,7 +671,8 @@ bool ViECodecImpl::CodecValid(const VideoCodec& video_codec) { return false; } - if (video_codec.startBitrate < kViEMinCodecBitrate) { + if (video_codec.startBitrate > 0 && + video_codec.startBitrate < kViEMinCodecBitrate) { LOG(LS_ERROR) << "Invalid start bitrate."; return false; } diff --git a/webrtc/video_engine/vie_encoder.cc b/webrtc/video_engine/vie_encoder.cc index c388e39d0d..a03d38e5ce 100644 --- a/webrtc/video_engine/vie_encoder.cc +++ b/webrtc/video_engine/vie_encoder.cc @@ -45,12 +45,6 @@ static const float kEncoderPausePacerMargin = 2.0f; // Don't stop the encoder unless the delay is above this configured value. static const int kMinPacingDelayMs = 200; -// Allow packets to be transmitted in up to 2 times max video bitrate if the -// bandwidth estimate allows it. -// TODO(holmer): Expose transmission start, min and max bitrates in the -// VideoEngine API and remove the kTransmissionMaxBitrateMultiplier. -static const int kTransmissionMaxBitrateMultiplier = 2; - static const float kStopPaddingThresholdMs = 2000; std::vector AllocateStreamBitrates( @@ -106,31 +100,11 @@ class ViEBitrateObserver : public BitrateObserver { ViEEncoder* owner_; }; -// TODO(mflodman): Move this observer to PayloadRouter class. -class ViEPacedSenderCallback : public PacedSender::Callback { - public: - explicit ViEPacedSenderCallback(ViEEncoder* owner) - : owner_(owner) { - } - virtual ~ViEPacedSenderCallback() {} - virtual bool TimeToSendPacket(uint32_t ssrc, - uint16_t sequence_number, - int64_t capture_time_ms, - bool retransmission) { - return owner_->TimeToSendPacket(ssrc, sequence_number, capture_time_ms, - retransmission); - } - virtual size_t TimeToSendPadding(size_t bytes) { - return owner_->TimeToSendPadding(bytes); - } - private: - ViEEncoder* owner_; -}; - ViEEncoder::ViEEncoder(int32_t channel_id, uint32_t number_of_cores, const Config& config, ProcessThread& module_process_thread, + PacedSender* pacer, BitrateAllocator* bitrate_allocator, BitrateController* bitrate_controller, bool disable_default_encoder) @@ -143,6 +117,7 @@ ViEEncoder::ViEEncoder(int32_t channel_id, vcm_protection_callback_(NULL), callback_cs_(CriticalSectionWrapper::CreateCriticalSection()), data_cs_(CriticalSectionWrapper::CreateCriticalSection()), + pacer_(pacer), bitrate_allocator_(bitrate_allocator), bitrate_controller_(bitrate_controller), time_of_last_incoming_frame_ms_(0), @@ -158,7 +133,6 @@ ViEEncoder::ViEEncoder(int32_t channel_id, codec_observer_(NULL), effect_filter_(NULL), module_process_thread_(module_process_thread), - pacer_thread_(ProcessThread::Create()), has_received_sli_(false), picture_id_sli_(0), has_received_rpsi_(false), @@ -169,13 +143,6 @@ ViEEncoder::ViEEncoder(int32_t channel_id, start_ms_(Clock::GetRealTimeClock()->TimeInMilliseconds()), send_statistics_proxy_(NULL) { bitrate_observer_.reset(new ViEBitrateObserver(this)); - pacing_callback_.reset(new ViEPacedSenderCallback(this)); - paced_sender_.reset(new PacedSender( - Clock::GetRealTimeClock(), - pacing_callback_.get(), - kDefaultStartBitrateKbps, - PacedSender::kDefaultPaceMultiplier * kDefaultStartBitrateKbps, - 0)); } bool ViEEncoder::Init() { @@ -233,15 +200,11 @@ void ViEEncoder::StartThreadsAndSetSharedMembers( vcm_protection_callback_ = vcm_protection_callback; module_process_thread_.RegisterModule(&vcm_); - pacer_thread_->RegisterModule(paced_sender_.get()); - pacer_thread_->Start(); } void ViEEncoder::StopThreadsAndRemoveSharedMembers() { vcm_.RegisterProtectionCallback(NULL); vcm_protection_callback_ = NULL; - pacer_thread_->Stop(); - pacer_thread_->DeRegisterModule(paced_sender_.get()); module_process_thread_.DeRegisterModule(&vcm_); module_process_thread_.DeRegisterModule(&vpm_); } @@ -283,9 +246,9 @@ void ViEEncoder::SetNetworkTransmissionState(bool is_transmitting) { network_is_transmitting_ = is_transmitting; } if (is_transmitting) { - paced_sender_->Resume(); + pacer_->Resume(); } else { - paced_sender_->Pause(); + pacer_->Pause(); } } @@ -373,49 +336,46 @@ int32_t ViEEncoder::SetEncoder(const webrtc::VideoCodec& video_codec) { return -1; } - // Convert from kbps to bps. - std::vector stream_bitrates = AllocateStreamBitrates( - video_codec.startBitrate * 1000, - video_codec.simulcastStream, - video_codec.numberOfSimulcastStreams); - send_payload_router_->SetTargetSendBitrates(stream_bitrates); - { CriticalSectionScoped cs(data_cs_.get()); send_padding_ = video_codec.numberOfSimulcastStreams > 1; } + + // Add a bitrate observer to the allocator and update the start, max and + // min bitrates of the bitrate controller as needed. + int allocated_bitrate_bps; + int new_bwe_candidate_bps = bitrate_allocator_->AddBitrateObserver( + bitrate_observer_.get(), video_codec.startBitrate * 1000, + video_codec.minBitrate * 1000, video_codec.maxBitrate * 1000, + &allocated_bitrate_bps); + + // Only set the start/min/max bitrate of the bitrate controller if the start + // bitrate is greater than zero. The new API sets these via the channel group + // and passes a zero start bitrate to SetSendCodec. + // TODO(holmer): Remove this when the new API has been launched. + if (video_codec.startBitrate > 0) { + if (new_bwe_candidate_bps > 0) { + uint32_t current_bwe_bps = 0; + bitrate_controller_->AvailableBandwidth(¤t_bwe_bps); + bitrate_controller_->SetStartBitrate(std::max( + static_cast(new_bwe_candidate_bps), current_bwe_bps)); + } + + int new_bwe_min_bps = 0; + int new_bwe_max_bps = 0; + bitrate_allocator_->GetMinMaxBitrateSumBps(&new_bwe_min_bps, + &new_bwe_max_bps); + bitrate_controller_->SetMinMaxBitrate(new_bwe_min_bps, new_bwe_max_bps); + } + + webrtc::VideoCodec modified_video_codec = video_codec; + modified_video_codec.startBitrate = allocated_bitrate_bps / 1000; + size_t max_data_payload_length = send_payload_router_->MaxPayloadLength(); - if (vcm_.RegisterSendCodec(&video_codec, number_of_cores_, + if (vcm_.RegisterSendCodec(&modified_video_codec, number_of_cores_, max_data_payload_length) != VCM_OK) { return -1; } - - // Add the a bitrate observer to the allocator and update the start, max and - // min bitrates of the bitrate controller as needed. - int new_bwe_candidate_bps = bitrate_allocator_->AddBitrateObserver( - bitrate_observer_.get(), video_codec.startBitrate * 1000, - video_codec.minBitrate * 1000, - kTransmissionMaxBitrateMultiplier * video_codec.maxBitrate * 1000); - if (new_bwe_candidate_bps > 0) { - uint32_t current_bwe_bps = 0; - bitrate_controller_->AvailableBandwidth(¤t_bwe_bps); - bitrate_controller_->SetStartBitrate(std::max( - static_cast(new_bwe_candidate_bps), current_bwe_bps)); - } - - int new_bwe_min_bps = 0; - int new_bwe_max_bps = 0; - bitrate_allocator_->GetMinMaxBitrateSumBps(&new_bwe_min_bps, - &new_bwe_max_bps); - bitrate_controller_->SetMinMaxBitrate(new_bwe_min_bps, new_bwe_max_bps); - - int pad_up_to_bitrate_bps = - GetPaddingNeededBps(1000 * video_codec.startBitrate); - paced_sender_->UpdateBitrate( - video_codec.startBitrate, - PacedSender::kDefaultPaceMultiplier * video_codec.startBitrate, - pad_up_to_bitrate_bps / 1000); - return 0; } @@ -450,18 +410,6 @@ int32_t ViEEncoder::ScaleInputImage(bool enable) { return 0; } -bool ViEEncoder::TimeToSendPacket(uint32_t ssrc, - uint16_t sequence_number, - int64_t capture_time_ms, - bool retransmission) { - return send_payload_router_->TimeToSendPacket( - ssrc, sequence_number, capture_time_ms, retransmission); -} - -size_t ViEEncoder::TimeToSendPadding(size_t bytes) { - return send_payload_router_->TimeToSendPadding(bytes); -} - int ViEEncoder::GetPaddingNeededBps(int bitrate_bps) const { int64_t time_of_last_incoming_frame_ms; int min_transmit_bitrate_bps; @@ -532,12 +480,12 @@ bool ViEEncoder::EncoderPaused() const { // Buffered mode. // TODO(pwestin): Workaround until nack is configured as a time and not // number of packets. - return paced_sender_->QueueInMs() >= - std::max(static_cast(target_delay_ms_ * kEncoderPausePacerMargin), - kMinPacingDelayMs); + return pacer_->QueueInMs() >= + std::max( + static_cast(target_delay_ms_ * kEncoderPausePacerMargin), + kMinPacingDelayMs); } - if (paced_sender_->ExpectedQueueTimeMs() > - PacedSender::kDefaultMaxQueueLengthMs) { + if (pacer_->ExpectedQueueTimeMs() > PacedSender::kDefaultMaxQueueLengthMs) { // Too much data in pacer queue, drop frame. return true; } @@ -687,10 +635,6 @@ int32_t ViEEncoder::SendCodecStatistics( return 0; } -int64_t ViEEncoder::PacerQueuingDelayMs() const { - return paced_sender_->QueueInMs(); -} - uint32_t ViEEncoder::LastObservedBitrateBps() const { CriticalSectionScoped cs(data_cs_.get()); return last_observed_bitrate_bps_; @@ -922,12 +866,6 @@ void ViEEncoder::OnNetworkChanged(uint32_t bitrate_bps, bitrate_bps, stream_configs, send_codec.numberOfSimulcastStreams); send_payload_router_->SetTargetSendBitrates(stream_bitrates); - int pad_up_to_bitrate_bps = GetPaddingNeededBps(bitrate_bps); - paced_sender_->UpdateBitrate( - bitrate_bps / 1000, - PacedSender::kDefaultPaceMultiplier * bitrate_bps / 1000, - pad_up_to_bitrate_bps / 1000); - { CriticalSectionScoped cs(data_cs_.get()); last_observed_bitrate_bps_ = bitrate_bps; @@ -944,10 +882,6 @@ void ViEEncoder::OnNetworkChanged(uint32_t bitrate_bps, } } -PacedSender* ViEEncoder::GetPacedSender() { - return paced_sender_.get(); -} - int32_t ViEEncoder::RegisterEffectFilter(ViEEffectFilter* effect_filter) { CriticalSectionScoped cs(callback_cs_.get()); if (effect_filter != NULL && effect_filter_ != NULL) { @@ -969,12 +903,6 @@ int ViEEncoder::StopDebugRecording() { void ViEEncoder::SuspendBelowMinBitrate() { vcm_.SuspendBelowMinBitrate(); bitrate_allocator_->EnforceMinBitrate(false); - int min_bitrate_sum_bps; - int max_bitrate_sum_bps; - bitrate_allocator_->GetMinMaxBitrateSumBps(&min_bitrate_sum_bps, - &max_bitrate_sum_bps); - bitrate_controller_->SetMinMaxBitrate(min_bitrate_sum_bps, - max_bitrate_sum_bps); } void ViEEncoder::RegisterPreEncodeCallback( diff --git a/webrtc/video_engine/vie_encoder.h b/webrtc/video_engine/vie_encoder.h index 784a2423cf..8683b07803 100644 --- a/webrtc/video_engine/vie_encoder.h +++ b/webrtc/video_engine/vie_encoder.h @@ -43,7 +43,6 @@ class ViEBitrateObserver; class ViEEffectFilter; class ViEEncoderObserver; class VideoCodingModule; -class ViEPacedSenderCallback; class ViEEncoder : public RtcpIntraFrameObserver, @@ -53,12 +52,12 @@ class ViEEncoder public ViEFrameCallback { public: friend class ViEBitrateObserver; - friend class ViEPacedSenderCallback; ViEEncoder(int32_t channel_id, uint32_t number_of_cores, const Config& config, ProcessThread& module_process_thread, + PacedSender* pacer, BitrateAllocator* bitrate_allocator, BitrateController* bitrate_controller, bool disable_default_encoder); @@ -101,8 +100,6 @@ class ViEEncoder unsigned char config_parameters[kConfigParameterSize], unsigned char& config_parameters_size); - PacedSender* GetPacedSender(); - // Scale or crop/pad image. int32_t ScaleInputImage(bool enable); @@ -121,8 +118,6 @@ class ViEEncoder int32_t SendCodecStatistics(uint32_t* num_key_frames, uint32_t* num_delta_frames); - int64_t PacerQueuingDelayMs() const; - uint32_t LastObservedBitrateBps() const; int CodecTargetBitrate(uint32_t* bitrate) const; // Loss protection. @@ -183,18 +178,15 @@ class ViEEncoder int channel_id() const { return channel_id_; } + int GetPaddingNeededBps(int bitrate_bps) const; + protected: // Called by BitrateObserver. void OnNetworkChanged(uint32_t bitrate_bps, uint8_t fraction_lost, int64_t round_trip_time_ms); - // Called by PacedSender. - bool TimeToSendPacket(uint32_t ssrc, uint16_t sequence_number, - int64_t capture_time_ms, bool retransmission); - size_t TimeToSendPadding(size_t bytes); private: - int GetPaddingNeededBps(int bitrate_bps) const; bool EncoderPaused() const EXCLUSIVE_LOCKS_REQUIRED(data_cs_); void TraceFrameDropStart() EXCLUSIVE_LOCKS_REQUIRED(data_cs_); void TraceFrameDropEnd() EXCLUSIVE_LOCKS_REQUIRED(data_cs_); @@ -213,9 +205,8 @@ class ViEEncoder rtc::scoped_ptr callback_cs_; rtc::scoped_ptr data_cs_; rtc::scoped_ptr bitrate_observer_; - rtc::scoped_ptr paced_sender_; - rtc::scoped_ptr pacing_callback_; + PacedSender* const pacer_; BitrateAllocator* const bitrate_allocator_; BitrateController* const bitrate_controller_; @@ -236,7 +227,6 @@ class ViEEncoder ViEEncoderObserver* codec_observer_ GUARDED_BY(callback_cs_); ViEEffectFilter* effect_filter_ GUARDED_BY(callback_cs_); ProcessThread& module_process_thread_; - rtc::scoped_ptr pacer_thread_; bool has_received_sli_ GUARDED_BY(data_cs_); uint8_t picture_id_sli_ GUARDED_BY(data_cs_); diff --git a/webrtc/video_engine/vie_network_impl.cc b/webrtc/video_engine/vie_network_impl.cc index e9089f0997..5d45187a61 100644 --- a/webrtc/video_engine/vie_network_impl.cc +++ b/webrtc/video_engine/vie_network_impl.cc @@ -15,6 +15,7 @@ #include #endif +#include "webrtc/base/checks.h" #include "webrtc/engine_configurations.h" #include "webrtc/system_wrappers/interface/logging.h" #include "webrtc/video_engine/include/vie_errors.h" @@ -51,6 +52,26 @@ int ViENetworkImpl::Release() { return ref_count; } +ViENetworkImpl::ViENetworkImpl(ViESharedData* shared_data) + : shared_data_(shared_data) { +} + +ViENetworkImpl::~ViENetworkImpl() { +} + +void ViENetworkImpl::SetBitrateConfig(int video_channel, + int min_bitrate_bps, + int start_bitrate_bps, + int max_bitrate_bps) { + LOG_F(LS_INFO) << "channel: " << video_channel + << " new bitrate config: min=" << min_bitrate_bps + << ", start=" << start_bitrate_bps + << ", max=" << max_bitrate_bps; + bool success = shared_data_->channel_manager()->SetBitrateConfig( + video_channel, min_bitrate_bps, start_bitrate_bps, max_bitrate_bps); + DCHECK(success); +} + void ViENetworkImpl::SetNetworkTransmissionState(const int video_channel, const bool is_transmitting) { LOG_F(LS_INFO) << "channel: " << video_channel @@ -64,11 +85,6 @@ void ViENetworkImpl::SetNetworkTransmissionState(const int video_channel, vie_encoder->SetNetworkTransmissionState(is_transmitting); } -ViENetworkImpl::ViENetworkImpl(ViESharedData* shared_data) - : shared_data_(shared_data) {} - -ViENetworkImpl::~ViENetworkImpl() {} - int ViENetworkImpl::RegisterSendTransport(const int video_channel, Transport& transport) { LOG_F(LS_INFO) << "channel: " << video_channel; diff --git a/webrtc/video_engine/vie_network_impl.h b/webrtc/video_engine/vie_network_impl.h index 7a09a2c7a3..1354f8cf4a 100644 --- a/webrtc/video_engine/vie_network_impl.h +++ b/webrtc/video_engine/vie_network_impl.h @@ -25,6 +25,10 @@ class ViENetworkImpl public: // Implements ViENetwork. int Release() override; + void SetBitrateConfig(int video_channel, + int min_bitrate_bps, + int start_bitrate_bps, + int max_bitrate_bps) override; void SetNetworkTransmissionState(const int video_channel, const bool is_transmitting) override; int RegisterSendTransport(const int video_channel, diff --git a/webrtc/video_engine/vie_rtp_rtcp_impl.cc b/webrtc/video_engine/vie_rtp_rtcp_impl.cc index 38e7e97dae..5aab8d6281 100644 --- a/webrtc/video_engine/vie_rtp_rtcp_impl.cc +++ b/webrtc/video_engine/vie_rtp_rtcp_impl.cc @@ -828,13 +828,10 @@ int ViERTP_RTCPImpl::GetEstimatedReceiveBandwidth( int ViERTP_RTCPImpl::GetPacerQueuingDelayMs( const int video_channel, int64_t* delay_ms) const { - ViEChannelManagerScoped cs(*(shared_data_->channel_manager())); - ViEEncoder* vie_encoder = cs.Encoder(video_channel); - if (!vie_encoder) { - shared_data_->SetLastError(kViERtpRtcpInvalidChannelId); + if (!shared_data_->channel_manager()->GetPacerQueuingDelayMs(video_channel, + delay_ms)) { return -1; } - *delay_ms = vie_encoder->PacerQueuingDelayMs(); return 0; } diff --git a/webrtc/voice_engine/test/auto_test/standard/rtp_rtcp_extensions.cc b/webrtc/voice_engine/test/auto_test/standard/rtp_rtcp_extensions.cc index bde16220e9..18f064eb77 100644 --- a/webrtc/voice_engine/test/auto_test/standard/rtp_rtcp_extensions.cc +++ b/webrtc/voice_engine/test/auto_test/standard/rtp_rtcp_extensions.cc @@ -159,6 +159,7 @@ class MockViENetwork : public webrtc::ViENetwork { virtual ~MockViENetwork() {} MOCK_METHOD0(Release, int()); + MOCK_METHOD4(SetBitrateConfig, void(int, int, int, int)); MOCK_METHOD2(SetNetworkTransmissionState, void(const int, const bool)); MOCK_METHOD2(RegisterSendTransport, int(const int, webrtc::Transport&)); MOCK_METHOD1(DeregisterSendTransport, int(const int));