diff --git a/webrtc/modules/bitrate_controller/bitrate_allocator.cc b/webrtc/modules/bitrate_controller/bitrate_allocator.cc index fc83e060a8..0aec528cde 100644 --- a/webrtc/modules/bitrate_controller/bitrate_allocator.cc +++ b/webrtc/modules/bitrate_controller/bitrate_allocator.cc @@ -51,7 +51,7 @@ BitrateAllocator::ObserverBitrateMap BitrateAllocator::AllocateBitrates() { uint32_t sum_min_bitrates = 0; for (const auto& observer : bitrate_observers_) - sum_min_bitrates += observer.second.min_bitrate_; + sum_min_bitrates += observer.second.min_bitrate; if (last_bitrate_bps_ <= sum_min_bitrates) return LowRateAllocation(last_bitrate_bps_); else @@ -59,10 +59,8 @@ BitrateAllocator::ObserverBitrateMap BitrateAllocator::AllocateBitrates() { } int BitrateAllocator::AddBitrateObserver(BitrateObserver* observer, - uint32_t start_bitrate_bps, uint32_t min_bitrate_bps, - uint32_t max_bitrate_bps, - int* new_observer_bitrate_bps) { + uint32_t max_bitrate_bps) { CriticalSectionScoped lock(crit_sect_.get()); BitrateObserverConfList::iterator it = @@ -73,43 +71,25 @@ int BitrateAllocator::AddBitrateObserver(BitrateObserver* observer, // 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_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. - for (const auto& observer : bitrate_observers_) - new_bwe_candidate_bps += observer.second.start_bitrate_; + it->second.min_bitrate = min_bitrate_bps; + it->second.max_bitrate = max_bitrate_bps; } else { // Add new settings. bitrate_observers_.push_back(BitrateObserverConfiguration( - observer, BitrateConfiguration(start_bitrate_bps, min_bitrate_bps, - max_bitrate_bps))); + observer, BitrateConfiguration(min_bitrate_bps, max_bitrate_bps))); bitrate_observers_modified_ = true; - - // TODO(andresp): This is a ugly way to set start bitrate. - // - // Only change start bitrate if we have exactly one observer. By definition - // 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_bps; } - last_bitrate_bps_ = std::max(new_bwe_candidate_bps, last_bitrate_bps_); - ObserverBitrateMap allocation = AllocateBitrates(); - *new_observer_bitrate_bps = 0; + int 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; + new_observer_bitrate_bps = kv.second; } - return last_bitrate_bps_; + return new_observer_bitrate_bps; } void BitrateAllocator::RemoveBitrateObserver(BitrateObserver* observer) { @@ -129,8 +109,8 @@ void BitrateAllocator::GetMinMaxBitrateSumBps(int* min_bitrate_sum_bps, CriticalSectionScoped lock(crit_sect_.get()); for (const auto& observer : bitrate_observers_) { - *min_bitrate_sum_bps += observer.second.min_bitrate_; - *max_bitrate_sum_bps += observer.second.max_bitrate_; + *min_bitrate_sum_bps += observer.second.min_bitrate; + *max_bitrate_sum_bps += observer.second.max_bitrate; } } @@ -160,15 +140,15 @@ BitrateAllocator::ObserverBitrateMap BitrateAllocator::NormalRateAllocation( ObserverSortingMap list_max_bitrates; for (const auto& observer : bitrate_observers_) { list_max_bitrates.insert(std::pair( - observer.second.max_bitrate_, - ObserverConfiguration(observer.first, observer.second.min_bitrate_))); + 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. @@ -176,9 +156,9 @@ BitrateAllocator::ObserverBitrateMap BitrateAllocator::NormalRateAllocation( if (number_of_observers != 0) { bitrate_per_observer += remainder / number_of_observers; } - allocation[max_it->second.observer_] = max_it->first; + allocation[max_it->second.observer] = max_it->first; } else { - allocation[max_it->second.observer_] = observer_allowance; + allocation[max_it->second.observer] = observer_allowance; } list_max_bitrates.erase(max_it); // Prepare next iteration. @@ -193,14 +173,14 @@ BitrateAllocator::ObserverBitrateMap BitrateAllocator::LowRateAllocation( if (enforce_min_bitrate_) { // Min bitrate to all observers. for (const auto& observer : bitrate_observers_) - allocation[observer.first] = observer.second.min_bitrate_; + allocation[observer.first] = observer.second.min_bitrate; } else { - // Allocate up to |min_bitrate_| to one observer at a time, until + // Allocate up to |min_bitrate| to one observer at a time, until // |bitrate| is depleted. uint32_t remainder = bitrate; for (const auto& observer : bitrate_observers_) { uint32_t allocated_bitrate = - std::min(remainder, observer.second.min_bitrate_); + std::min(remainder, observer.second.min_bitrate); allocation[observer.first] = allocated_bitrate; remainder -= allocated_bitrate; } diff --git a/webrtc/modules/bitrate_controller/bitrate_allocator_unittest.cc b/webrtc/modules/bitrate_controller/bitrate_allocator_unittest.cc index b69247e861..4fc7e83b5b 100644 --- a/webrtc/modules/bitrate_controller/bitrate_allocator_unittest.cc +++ b/webrtc/modules/bitrate_controller/bitrate_allocator_unittest.cc @@ -46,22 +46,24 @@ class BitrateAllocatorTest : public ::testing::Test { TEST_F(BitrateAllocatorTest, UpdatingBitrateObserver) { TestBitrateObserver bitrate_observer; - int start_bitrate; - allocator_->AddBitrateObserver(&bitrate_observer, 200000, 100000, 1500000, - &start_bitrate); + int start_bitrate = + allocator_->AddBitrateObserver(&bitrate_observer, 100000, 1500000); EXPECT_EQ(300000, start_bitrate); allocator_->OnNetworkChanged(200000, 0, 0); EXPECT_EQ(200000u, bitrate_observer.last_bitrate_); - 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_); + // TODO(pbos): Expect capping to 1.5M instead of 3M when not boosting the max + // bitrate for FEC/retransmissions (see TODO in BitrateAllocator). + allocator_->OnNetworkChanged(4000000, 0, 0); + EXPECT_EQ(3000000u, bitrate_observer.last_bitrate_); + start_bitrate = + allocator_->AddBitrateObserver(&bitrate_observer, 100000, 4000000); + EXPECT_EQ(4000000, start_bitrate); - allocator_->AddBitrateObserver(&bitrate_observer, 500000, 100000, 1500000, - &start_bitrate); - EXPECT_EQ(1500000, start_bitrate); + start_bitrate = + allocator_->AddBitrateObserver(&bitrate_observer, 100000, 1500000); + EXPECT_EQ(3000000, start_bitrate); + EXPECT_EQ(3000000u, bitrate_observer.last_bitrate_); allocator_->OnNetworkChanged(1500000, 0, 0); EXPECT_EQ(1500000u, bitrate_observer.last_bitrate_); } @@ -69,12 +71,11 @@ TEST_F(BitrateAllocatorTest, UpdatingBitrateObserver) { TEST_F(BitrateAllocatorTest, TwoBitrateObserversOneRtcpObserver) { TestBitrateObserver bitrate_observer_1; TestBitrateObserver bitrate_observer_2; - int start_bitrate; - allocator_->AddBitrateObserver(&bitrate_observer_1, 200000, 100000, 300000, - &start_bitrate); + int start_bitrate = + allocator_->AddBitrateObserver(&bitrate_observer_1, 100000, 300000); EXPECT_EQ(300000, start_bitrate); - allocator_->AddBitrateObserver(&bitrate_observer_2, 200000, 200000, 300000, - &start_bitrate); + start_bitrate = + allocator_->AddBitrateObserver(&bitrate_observer_2, 200000, 300000); EXPECT_EQ(200000, start_bitrate); // Test too low start bitrate, hence lower than sum of min. Min bitrates will @@ -114,9 +115,8 @@ class BitrateAllocatorTestNoEnforceMin : public ::testing::Test { // as intended. TEST_F(BitrateAllocatorTestNoEnforceMin, OneBitrateObserver) { TestBitrateObserver bitrate_observer_1; - int start_bitrate; - allocator_->AddBitrateObserver(&bitrate_observer_1, 200000, 100000, 400000, - &start_bitrate); + int start_bitrate = + allocator_->AddBitrateObserver(&bitrate_observer_1, 100000, 400000); EXPECT_EQ(300000, start_bitrate); // High REMB. @@ -135,18 +135,17 @@ TEST_F(BitrateAllocatorTestNoEnforceMin, ThreeBitrateObservers) { TestBitrateObserver bitrate_observer_2; TestBitrateObserver bitrate_observer_3; // Set up the observers with min bitrates at 100000, 200000, and 300000. - int start_bitrate; - allocator_->AddBitrateObserver(&bitrate_observer_1, 200000, 100000, 400000, - &start_bitrate); + int start_bitrate = + allocator_->AddBitrateObserver(&bitrate_observer_1, 100000, 400000); EXPECT_EQ(300000, start_bitrate); - allocator_->AddBitrateObserver(&bitrate_observer_2, 200000, 200000, 400000, - &start_bitrate); + start_bitrate = + allocator_->AddBitrateObserver(&bitrate_observer_2, 200000, 400000); EXPECT_EQ(200000, start_bitrate); EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_); - allocator_->AddBitrateObserver(&bitrate_observer_3, 200000, 300000, 400000, - &start_bitrate); + start_bitrate = + allocator_->AddBitrateObserver(&bitrate_observer_3, 300000, 400000); EXPECT_EQ(0, start_bitrate); EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_); EXPECT_EQ(200000u, bitrate_observer_2.last_bitrate_); @@ -185,18 +184,17 @@ TEST_F(BitrateAllocatorTest, ThreeBitrateObserversLowRembEnforceMin) { TestBitrateObserver bitrate_observer_1; TestBitrateObserver bitrate_observer_2; TestBitrateObserver bitrate_observer_3; - int start_bitrate; - allocator_->AddBitrateObserver(&bitrate_observer_1, 200000, 100000, 400000, - &start_bitrate); + int start_bitrate = + allocator_->AddBitrateObserver(&bitrate_observer_1, 100000, 400000); EXPECT_EQ(300000, start_bitrate); - allocator_->AddBitrateObserver(&bitrate_observer_2, 200000, 200000, 400000, - &start_bitrate); + start_bitrate = + allocator_->AddBitrateObserver(&bitrate_observer_2, 200000, 400000); EXPECT_EQ(200000, start_bitrate); EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_); - allocator_->AddBitrateObserver(&bitrate_observer_3, 200000, 300000, 400000, - &start_bitrate); + start_bitrate = + allocator_->AddBitrateObserver(&bitrate_observer_3, 300000, 400000); 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_)); diff --git a/webrtc/modules/bitrate_controller/include/bitrate_allocator.h b/webrtc/modules/bitrate_controller/include/bitrate_allocator.h index 9cc4b74711..5c58f569d2 100644 --- a/webrtc/modules/bitrate_controller/include/bitrate_allocator.h +++ b/webrtc/modules/bitrate_controller/include/bitrate_allocator.h @@ -37,15 +37,13 @@ class BitrateAllocator { // Set the start and max send bitrate used by the bandwidth management. // - // observer, updates bitrates if already in use. - // 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. + // |observer| updates bitrates if already in use. + // |min_bitrate_bps| = 0 equals no min bitrate. + // |max_bitrate_bps| = 0 equals no max bitrate. + // Returns bitrate allocated for the bitrate observer. int AddBitrateObserver(BitrateObserver* observer, - uint32_t start_bitrate_bps, uint32_t min_bitrate_bps, - uint32_t max_bitrate_bps, - int* new_observer_bitrate_bps); + uint32_t max_bitrate_bps); void RemoveBitrateObserver(BitrateObserver* observer); @@ -61,21 +59,16 @@ class BitrateAllocator { private: struct BitrateConfiguration { - BitrateConfiguration(uint32_t start_bitrate, - uint32_t min_bitrate, - uint32_t max_bitrate) - : start_bitrate_(start_bitrate), - min_bitrate_(min_bitrate), - max_bitrate_(max_bitrate) {} - uint32_t start_bitrate_; - uint32_t min_bitrate_; - uint32_t max_bitrate_; + BitrateConfiguration(uint32_t min_bitrate, uint32_t max_bitrate) + : min_bitrate(min_bitrate), max_bitrate(max_bitrate) {} + uint32_t min_bitrate; + uint32_t max_bitrate; }; struct ObserverConfiguration { ObserverConfiguration(BitrateObserver* observer, uint32_t bitrate) - : observer_(observer), min_bitrate_(bitrate) {} - BitrateObserver* observer_; - uint32_t min_bitrate_; + : observer(observer), min_bitrate(bitrate) {} + BitrateObserver* const observer; + uint32_t min_bitrate; }; typedef std::pair BitrateObserverConfiguration; diff --git a/webrtc/video_engine/encoder_state_feedback_unittest.cc b/webrtc/video_engine/encoder_state_feedback_unittest.cc index 9dc6c2ff9a..dc93266589 100644 --- a/webrtc/video_engine/encoder_state_feedback_unittest.cc +++ b/webrtc/video_engine/encoder_state_feedback_unittest.cc @@ -17,6 +17,7 @@ #include "webrtc/base/scoped_ptr.h" #include "webrtc/common.h" +#include "webrtc/modules/bitrate_controller/include/bitrate_controller.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" @@ -31,7 +32,7 @@ namespace webrtc { class MockVieEncoder : public ViEEncoder { public: explicit MockVieEncoder(ProcessThread* process_thread, PacedSender* pacer) - : ViEEncoder(1, 1, *process_thread, pacer, NULL, NULL) {} + : ViEEncoder(1, 1, *process_thread, pacer, NULL) {} ~MockVieEncoder() {} MOCK_METHOD1(OnReceivedIntraFrameRequest, diff --git a/webrtc/video_engine/vie_channel_group.cc b/webrtc/video_engine/vie_channel_group.cc index 2eff0caf94..fc7a2d1a52 100644 --- a/webrtc/video_engine/vie_channel_group.cc +++ b/webrtc/video_engine/vie_channel_group.cc @@ -191,9 +191,9 @@ bool ChannelGroup::CreateSendChannel(int channel_id, int number_of_cores, const std::vector& ssrcs) { DCHECK(!ssrcs.empty()); - rtc::scoped_ptr vie_encoder(new ViEEncoder( - channel_id, number_of_cores, *process_thread_, pacer_.get(), - bitrate_allocator_.get(), bitrate_controller_.get())); + rtc::scoped_ptr vie_encoder( + new ViEEncoder(channel_id, number_of_cores, *process_thread_, + pacer_.get(), bitrate_allocator_.get())); if (!vie_encoder->Init()) { return false; } diff --git a/webrtc/video_engine/vie_encoder.cc b/webrtc/video_engine/vie_encoder.cc index df3a4d465a..4dbb0f09ca 100644 --- a/webrtc/video_engine/vie_encoder.cc +++ b/webrtc/video_engine/vie_encoder.cc @@ -18,6 +18,7 @@ #include "webrtc/common_video/interface/video_image.h" #include "webrtc/common_video/libyuv/include/webrtc_libyuv.h" #include "webrtc/frame_callback.h" +#include "webrtc/modules/bitrate_controller/include/bitrate_controller.h" #include "webrtc/modules/pacing/include/paced_sender.h" #include "webrtc/modules/utility/interface/process_thread.h" #include "webrtc/modules/video_coding/codecs/interface/video_codec_interface.h" @@ -105,8 +106,7 @@ ViEEncoder::ViEEncoder(int32_t channel_id, uint32_t number_of_cores, ProcessThread& module_process_thread, PacedSender* pacer, - BitrateAllocator* bitrate_allocator, - BitrateController* bitrate_controller) + BitrateAllocator* bitrate_allocator) : channel_id_(channel_id), number_of_cores_(number_of_cores), vpm_(VideoProcessingModule::Create(ViEModuleId(-1, channel_id))), @@ -119,7 +119,6 @@ ViEEncoder::ViEEncoder(int32_t channel_id, data_cs_(CriticalSectionWrapper::CreateCriticalSection()), pacer_(pacer), bitrate_allocator_(bitrate_allocator), - bitrate_controller_(bitrate_controller), time_of_last_frame_activity_ms_(0), simulcast_enabled_(false), min_transmit_bitrate_kbps_(0), @@ -269,30 +268,9 @@ int32_t ViEEncoder::SetEncoder(const webrtc::VideoCodec& video_codec) { // 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); - } + int allocated_bitrate_bps = bitrate_allocator_->AddBitrateObserver( + bitrate_observer_.get(), video_codec.minBitrate * 1000, + video_codec.maxBitrate * 1000); webrtc::VideoCodec modified_video_codec = video_codec; modified_video_codec.startBitrate = allocated_bitrate_bps / 1000; diff --git a/webrtc/video_engine/vie_encoder.h b/webrtc/video_engine/vie_encoder.h index 9edd5bf3d3..424a2a2b51 100644 --- a/webrtc/video_engine/vie_encoder.h +++ b/webrtc/video_engine/vie_encoder.h @@ -20,7 +20,6 @@ #include "webrtc/common_types.h" #include "webrtc/frame_callback.h" #include "webrtc/modules/bitrate_controller/include/bitrate_allocator.h" -#include "webrtc/modules/bitrate_controller/include/bitrate_controller.h" #include "webrtc/modules/rtp_rtcp/interface/rtp_rtcp_defines.h" #include "webrtc/modules/video_coding/main/interface/video_coding_defines.h" #include "webrtc/modules/video_processing/main/interface/video_processing.h" @@ -75,8 +74,7 @@ class ViEEncoder : public RtcpIntraFrameObserver, uint32_t number_of_cores, ProcessThread& module_process_thread, PacedSender* pacer, - BitrateAllocator* bitrate_allocator, - BitrateController* bitrate_controller); + BitrateAllocator* bitrate_allocator); ~ViEEncoder(); bool Init(); @@ -204,7 +202,6 @@ class ViEEncoder : public RtcpIntraFrameObserver, PacedSender* const pacer_; BitrateAllocator* const bitrate_allocator_; - BitrateController* const bitrate_controller_; // The time we last received an input frame or encoded frame. This is used to // track when video is stopped long enough that we also want to stop sending