diff --git a/webrtc/audio/audio_receive_stream_unittest.cc b/webrtc/audio/audio_receive_stream_unittest.cc index a5318d5150..300ab20054 100644 --- a/webrtc/audio/audio_receive_stream_unittest.cc +++ b/webrtc/audio/audio_receive_stream_unittest.cc @@ -157,7 +157,7 @@ struct ConfigHelper { private: SimulatedClock simulated_clock_; PacketRouter packet_router_; - testing::NiceMock bitrate_observer_; + testing::NiceMock bitrate_observer_; testing::NiceMock remote_bitrate_observer_; MockCongestionController congestion_controller_; MockRemoteBitrateEstimator remote_bitrate_estimator_; diff --git a/webrtc/audio/audio_send_stream_unittest.cc b/webrtc/audio/audio_send_stream_unittest.cc index a94034c649..24efaada10 100644 --- a/webrtc/audio/audio_send_stream_unittest.cc +++ b/webrtc/audio/audio_send_stream_unittest.cc @@ -16,7 +16,7 @@ #include "webrtc/audio/audio_send_stream.h" #include "webrtc/audio/audio_state.h" #include "webrtc/audio/conversion.h" -#include "webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h" +#include "webrtc/modules/bitrate_controller/include/mock/mock_bitrate_controller.h" #include "webrtc/modules/congestion_controller/include/congestion_controller.h" #include "webrtc/modules/pacing/paced_sender.h" #include "webrtc/modules/remote_bitrate_estimator/include/mock/mock_remote_bitrate_estimator.h" @@ -161,7 +161,7 @@ struct ConfigHelper { rtc::scoped_refptr audio_state_; AudioSendStream::Config stream_config_; testing::StrictMock* channel_proxy_ = nullptr; - testing::NiceMock bitrate_observer_; + testing::NiceMock bitrate_observer_; testing::NiceMock remote_bitrate_observer_; CongestionController congestion_controller_; }; diff --git a/webrtc/call/bitrate_allocator.cc b/webrtc/call/bitrate_allocator.cc index 4c8d2a0c44..097378f02a 100644 --- a/webrtc/call/bitrate_allocator.cc +++ b/webrtc/call/bitrate_allocator.cc @@ -54,9 +54,7 @@ BitrateAllocator::ObserverBitrateMap BitrateAllocator::AllocateBitrates() { uint32_t sum_min_bitrates = 0; for (const auto& observer : bitrate_observers_) sum_min_bitrates += observer.second.min_bitrate; - if (last_bitrate_bps_ == 0) - return ZeroRateAllocation(); - else if (last_bitrate_bps_ <= sum_min_bitrates) + if (last_bitrate_bps_ <= sum_min_bitrates) return LowRateAllocation(last_bitrate_bps_); else return NormalRateAllocation(last_bitrate_bps_, sum_min_bitrates); @@ -106,6 +104,18 @@ void BitrateAllocator::RemoveObserver(BitrateAllocatorObserver* observer) { } } +void BitrateAllocator::GetMinMaxBitrateSumBps(int* min_bitrate_sum_bps, + int* max_bitrate_sum_bps) const { + *min_bitrate_sum_bps = 0; + *max_bitrate_sum_bps = 0; + + rtc::CritScope lock(&crit_sect_); + 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 BitrateAllocatorObserver* observer) { @@ -160,14 +170,6 @@ BitrateAllocator::ObserverBitrateMap BitrateAllocator::NormalRateAllocation( return allocation; } -BitrateAllocator::ObserverBitrateMap BitrateAllocator::ZeroRateAllocation() { - ObserverBitrateMap allocation; - // Zero bitrate to all observers. - for (const auto& observer : bitrate_observers_) - allocation[observer.first] = 0; - return allocation; -} - BitrateAllocator::ObserverBitrateMap BitrateAllocator::LowRateAllocation( uint32_t bitrate) { ObserverBitrateMap allocation; diff --git a/webrtc/call/bitrate_allocator.h b/webrtc/call/bitrate_allocator.h index fc88b783b5..404a312dad 100644 --- a/webrtc/call/bitrate_allocator.h +++ b/webrtc/call/bitrate_allocator.h @@ -60,6 +60,9 @@ class BitrateAllocator { void RemoveObserver(BitrateAllocatorObserver* observer); + void GetMinMaxBitrateSumBps(int* min_bitrate_sum_bps, + int* max_bitrate_sum_bps) const; + // This method controls the behavior when the available bitrate is lower than // the minimum bitrate, or the sum of minimum bitrates. // When true, the bitrate will never be set lower than the minimum bitrate(s). @@ -94,7 +97,6 @@ class BitrateAllocator { uint32_t sum_min_bitrates) EXCLUSIVE_LOCKS_REQUIRED(crit_sect_); - ObserverBitrateMap ZeroRateAllocation() EXCLUSIVE_LOCKS_REQUIRED(crit_sect_); ObserverBitrateMap LowRateAllocation(uint32_t bitrate) EXCLUSIVE_LOCKS_REQUIRED(crit_sect_); diff --git a/webrtc/call/bitrate_allocator_unittest.cc b/webrtc/call/bitrate_allocator_unittest.cc index 63149acbe3..6e0cdd4d78 100644 --- a/webrtc/call/bitrate_allocator_unittest.cc +++ b/webrtc/call/bitrate_allocator_unittest.cc @@ -96,12 +96,6 @@ TEST_F(BitrateAllocatorTest, TwoBitrateObserversOneRtcpObserver) { allocator_->OnNetworkChanged(1500000, 0, 50); EXPECT_EQ(600000u, bitrate_observer_1.last_bitrate_); EXPECT_EQ(600000u, bitrate_observer_2.last_bitrate_); - - // Verify that if the bandwidth estimate is set to zero, the allocated rate is - // zero. - allocator_->OnNetworkChanged(0, 0, 50); - EXPECT_EQ(0u, bitrate_observer_1.last_bitrate_); - EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_); } class BitrateAllocatorTestNoEnforceMin : public ::testing::Test { @@ -177,13 +171,6 @@ TEST_F(BitrateAllocatorTestNoEnforceMin, ThreeBitrateObservers) { EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_); EXPECT_EQ(0u, bitrate_observer_3.last_bitrate_); - allocator_->OnNetworkChanged(0, 0, 0); - // Verify that zero estimated bandwidth, means that that all gets zero, - // regardless of set min bitrate. - EXPECT_EQ(0u, bitrate_observer_1.last_bitrate_); - EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_); - EXPECT_EQ(0u, bitrate_observer_3.last_bitrate_); - allocator_->RemoveObserver(&bitrate_observer_1); allocator_->RemoveObserver(&bitrate_observer_2); allocator_->RemoveObserver(&bitrate_observer_3); diff --git a/webrtc/call/call.cc b/webrtc/call/call.cc index f7c66db4b2..82ca630ad2 100644 --- a/webrtc/call/call.cc +++ b/webrtc/call/call.cc @@ -52,9 +52,8 @@ const int Call::Config::kDefaultStartBitrateBps = 300000; namespace internal { -class Call : public webrtc::Call, - public PacketReceiver, - public CongestionController::Observer { +class Call : public webrtc::Call, public PacketReceiver, + public BitrateObserver { public: explicit Call(const Call::Config& config); virtual ~Call(); @@ -700,8 +699,10 @@ void Call::OnNetworkChanged(uint32_t target_bitrate_bps, uint8_t fraction_loss, pacer_bitrate_sum_kbits_ += pacer_bitrate_bps / 1000; ++num_bitrate_updates_; } - congestion_controller_->SetAllocatedSendBitrate(allocated_bitrate_bps, - pad_up_to_bitrate_bps); + congestion_controller_->UpdatePacerBitrate( + target_bitrate_bps / 1000, + PacedSender::kDefaultPaceMultiplier * pacer_bitrate_bps / 1000, + pad_up_to_bitrate_bps / 1000); } void Call::ConfigureSync(const std::string& sync_group) { diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc b/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc index 09652d8419..3c0d37c3b0 100644 --- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc +++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc @@ -83,10 +83,6 @@ BitrateController* BitrateController::CreateBitrateController( return new BitrateControllerImpl(clock, observer); } -BitrateController* BitrateController::CreateBitrateController(Clock* clock) { - return new BitrateControllerImpl(clock, nullptr); -} - BitrateControllerImpl::BitrateControllerImpl(Clock* clock, BitrateObserver* observer) : clock_(clock), @@ -98,8 +94,8 @@ BitrateControllerImpl::BitrateControllerImpl(Clock* clock, last_fraction_loss_(0), last_rtt_ms_(0), last_reserved_bitrate_bps_(0) { - // This calls the observer_ if set, which means that the observer provided by - // the user must be ready to accept a bitrate update when it constructs the + // 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(); @@ -203,15 +199,11 @@ void BitrateControllerImpl::OnReceivedRtcpReceiverReport( } void BitrateControllerImpl::MaybeTriggerOnNetworkChanged() { - if (!observer_) - return; - - uint32_t bitrate_bps; + uint32_t bitrate; uint8_t fraction_loss; int64_t rtt; - - if (GetNetworkParameters(&bitrate_bps, &fraction_loss, &rtt)) - observer_->OnNetworkChanged(bitrate_bps, fraction_loss, rtt); + if (GetNetworkParameters(&bitrate, &fraction_loss, &rtt)) + observer_->OnNetworkChanged(bitrate, fraction_loss, rtt); } bool BitrateControllerImpl::GetNetworkParameters(uint32_t* bitrate, diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_impl.h b/webrtc/modules/bitrate_controller/bitrate_controller_impl.h index 5a61379ce0..a9661212d1 100644 --- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.h +++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.h @@ -28,8 +28,6 @@ namespace webrtc { class BitrateControllerImpl : public BitrateController { public: - // TODO(perkj): BitrateObserver has been deprecated and is not used in WebRTC. - // |observer| is left for project that is not yet updated. BitrateControllerImpl(Clock* clock, BitrateObserver* observer); virtual ~BitrateControllerImpl() {} @@ -52,11 +50,6 @@ class BitrateControllerImpl : public BitrateController { void SetEventLog(RtcEventLog* event_log) override; - // Returns true if the parameters have changed since the last call. - bool GetNetworkParameters(uint32_t* bitrate, - uint8_t* fraction_loss, - int64_t* rtt) override; - int64_t TimeUntilNextProcess() override; void Process() override; @@ -71,16 +64,20 @@ class BitrateControllerImpl : public BitrateController { int number_of_packets, int64_t now_ms); - // Deprecated 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) EXCLUSIVE_LOCKS_REQUIRED(critsect_); // Used by process thread. - Clock* const clock_; - BitrateObserver* const observer_; + Clock* clock_; + BitrateObserver* observer_; int64_t last_bitrate_update_ms_; rtc::CriticalSection critsect_; diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc b/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc index 4f92a3884b..3f467ef8a4 100644 --- a/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc +++ b/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc @@ -14,16 +14,11 @@ #include "testing/gtest/include/gtest/gtest.h" #include "webrtc/modules/bitrate_controller/include/bitrate_controller.h" -#include "webrtc/modules/pacing/mock/mock_paced_sender.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" -using ::testing::Exactly; -using ::testing::Return; - -using webrtc::BitrateController; -using webrtc::BitrateObserver; -using webrtc::PacedSender; using webrtc::RtcpBandwidthObserver; +using webrtc::BitrateObserver; +using webrtc::BitrateController; uint8_t WeightedLoss(int num_packets1, uint8_t fraction_loss1, int num_packets2, uint8_t fraction_loss2) { diff --git a/webrtc/modules/bitrate_controller/include/bitrate_controller.h b/webrtc/modules/bitrate_controller/include/bitrate_controller.h index a61cf6a7a7..d6cbc02d18 100644 --- a/webrtc/modules/bitrate_controller/include/bitrate_controller.h +++ b/webrtc/modules/bitrate_controller/include/bitrate_controller.h @@ -18,7 +18,6 @@ #include #include "webrtc/modules/include/module.h" -#include "webrtc/modules/pacing/paced_sender.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" namespace webrtc { @@ -27,8 +26,6 @@ class CriticalSectionWrapper; class RtcEventLog; struct PacketInfo; -// Deprecated -// TODO(perkj): Remove BitrateObserver when no implementations use it. class BitrateObserver { // Observer class for bitrate changes announced due to change in bandwidth // estimate or due to bitrate allocation changes. Fraction loss and rtt is @@ -49,15 +46,10 @@ class BitrateController : public Module { // estimation and divide the available bitrate between all its registered // BitrateObservers. public: - static const int kDefaultStartBitratebps = 300000; + static const int kDefaultStartBitrateKbps = 300; - // Deprecated: - // TODO(perkj): BitrateObserver has been deprecated and is not used in WebRTC. - // Remove this method once other other projects does not use it. static BitrateController* CreateBitrateController(Clock* clock, BitrateObserver* observer); - static BitrateController* CreateBitrateController(Clock* clock); - virtual ~BitrateController() {} virtual RtcpBandwidthObserver* CreateRtcpBandwidthObserver() = 0; @@ -79,10 +71,6 @@ class BitrateController : public Module { virtual bool AvailableBandwidth(uint32_t* bandwidth) const = 0; virtual void SetReservedBitrate(uint32_t reserved_bitrate_bps) = 0; - - virtual bool GetNetworkParameters(uint32_t* bitrate, - uint8_t* fraction_loss, - int64_t* rtt) = 0; }; } // namespace webrtc #endif // WEBRTC_MODULES_BITRATE_CONTROLLER_INCLUDE_BITRATE_CONTROLLER_H_ diff --git a/webrtc/modules/bitrate_controller/include/mock/mock_bitrate_controller.h b/webrtc/modules/bitrate_controller/include/mock/mock_bitrate_controller.h index da6169e748..5290b01106 100644 --- a/webrtc/modules/bitrate_controller/include/mock/mock_bitrate_controller.h +++ b/webrtc/modules/bitrate_controller/include/mock/mock_bitrate_controller.h @@ -39,8 +39,6 @@ class MockBitrateController : public BitrateController { MOCK_METHOD1(SetEventLog, void(RtcEventLog* event_log)); MOCK_CONST_METHOD1(AvailableBandwidth, bool(uint32_t* bandwidth)); MOCK_METHOD1(SetReservedBitrate, void(uint32_t reserved_bitrate_bps)); - MOCK_METHOD3(GetNetworkParameters, - bool(uint32_t* bitrate, uint8_t* fraction_loss, int64_t* rtt)); MOCK_METHOD0(Process, void()); MOCK_METHOD0(TimeUntilNextProcess, int64_t()); diff --git a/webrtc/modules/congestion_controller/congestion_controller.cc b/webrtc/modules/congestion_controller/congestion_controller.cc index d557843be2..14a73fe1cd 100644 --- a/webrtc/modules/congestion_controller/congestion_controller.cc +++ b/webrtc/modules/congestion_controller/congestion_controller.cc @@ -20,6 +20,7 @@ #include "webrtc/base/socket.h" #include "webrtc/base/thread_annotations.h" #include "webrtc/modules/bitrate_controller/include/bitrate_controller.h" +#include "webrtc/modules/pacing/paced_sender.h" #include "webrtc/modules/remote_bitrate_estimator/include/send_time_history.h" #include "webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h" #include "webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h" @@ -140,76 +141,28 @@ CongestionController::CongestionController( BitrateObserver* bitrate_observer, RemoteBitrateObserver* remote_bitrate_observer) : clock_(clock), - observer_(nullptr), - packet_router_(new PacketRouter()), pacer_(new PacedSender(clock_, - packet_router_.get(), - BitrateController::kDefaultStartBitratebps)), - remote_bitrate_estimator_( - new WrappingBitrateEstimator(remote_bitrate_observer, clock_)), - bitrate_controller_( - BitrateController::CreateBitrateController(clock_, bitrate_observer)), - remote_estimator_proxy_(clock_, packet_router_.get()), - transport_feedback_adapter_(bitrate_controller_.get(), clock_), - min_bitrate_bps_(RemoteBitrateEstimator::kDefaultMinBitrateBps), - send_queue_is_full_(false) { - Init(); -} - -CongestionController::CongestionController( - Clock* clock, - Observer* observer, - RemoteBitrateObserver* remote_bitrate_observer) - : clock_(clock), - observer_(observer), - packet_router_(new PacketRouter()), - pacer_(new PacedSender(clock_, - packet_router_.get(), - BitrateController::kDefaultStartBitratebps)), - remote_bitrate_estimator_( - new WrappingBitrateEstimator(remote_bitrate_observer, clock_)), - bitrate_controller_(BitrateController::CreateBitrateController(clock_)), - remote_estimator_proxy_(clock_, packet_router_.get()), - transport_feedback_adapter_(bitrate_controller_.get(), clock_), - min_bitrate_bps_(RemoteBitrateEstimator::kDefaultMinBitrateBps), - send_queue_is_full_(false) { - Init(); -} - -CongestionController::CongestionController( - Clock* clock, - Observer* observer, - RemoteBitrateObserver* remote_bitrate_observer, - std::unique_ptr packet_router, - std::unique_ptr pacer) - : clock_(clock), - observer_(observer), - packet_router_(std::move(packet_router)), - pacer_(std::move(pacer)), + &packet_router_, + BitrateController::kDefaultStartBitrateKbps, + PacedSender::kDefaultPaceMultiplier * + BitrateController::kDefaultStartBitrateKbps, + 0)), remote_bitrate_estimator_( new WrappingBitrateEstimator(remote_bitrate_observer, clock_)), // Constructed last as this object calls the provided callback on // construction. - bitrate_controller_(BitrateController::CreateBitrateController(clock_)), - remote_estimator_proxy_(clock_, packet_router_.get()), + bitrate_controller_( + BitrateController::CreateBitrateController(clock_, bitrate_observer)), + remote_estimator_proxy_(clock_, &packet_router_), transport_feedback_adapter_(bitrate_controller_.get(), clock_), - min_bitrate_bps_(RemoteBitrateEstimator::kDefaultMinBitrateBps), - send_queue_is_full_(false) { - Init(); -} - -CongestionController::~CongestionController() {} - -void CongestionController::Init() { + min_bitrate_bps_(RemoteBitrateEstimator::kDefaultMinBitrateBps) { transport_feedback_adapter_.SetBitrateEstimator( new RemoteBitrateEstimatorAbsSendTime(&transport_feedback_adapter_)); transport_feedback_adapter_.GetBitrateEstimator()->SetMinBitrate( min_bitrate_bps_); - // 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(); +} + +CongestionController::~CongestionController() { } @@ -236,7 +189,6 @@ void CongestionController::SetBweBitrates(int min_bitrate_bps, min_bitrate_bps_ = min_bitrate_bps; transport_feedback_adapter_.GetBitrateEstimator()->SetMinBitrate( min_bitrate_bps_); - MaybeTriggerOnNetworkChanged(); } BitrateController* CongestionController::GetBitrateController() const { @@ -257,9 +209,10 @@ CongestionController::GetTransportFeedbackObserver() { return &transport_feedback_adapter_; } -void CongestionController::SetAllocatedSendBitrate(int allocated_bitrate_bps, - int padding_bitrate_bps) { - pacer_->SetAllocatedSendBitrate(allocated_bitrate_bps, padding_bitrate_bps); +void CongestionController::UpdatePacerBitrate(int bitrate_kbps, + int max_bitrate_kbps, + int min_bitrate_kbps) { + pacer_->UpdateBitrate(bitrate_kbps, max_bitrate_kbps, min_bitrate_kbps); } int64_t CongestionController::GetPacerQueuingDelayMs() const { @@ -292,36 +245,6 @@ int64_t CongestionController::TimeUntilNextProcess() { void CongestionController::Process() { bitrate_controller_->Process(); remote_bitrate_estimator_->Process(); - MaybeTriggerOnNetworkChanged(); -} - -void CongestionController::MaybeTriggerOnNetworkChanged() { - // TODO(perkj): |observer_| can be nullptr if the ctor that accepts a - // BitrateObserver is used. Remove this check once the ctor is removed. - if (!observer_) - return; - - uint32_t bitrate_bps; - uint8_t fraction_loss; - int64_t rtt; - bool network_changed = bitrate_controller_->GetNetworkParameters( - &bitrate_bps, &fraction_loss, &rtt); - if (network_changed) - pacer_->SetEstimatedBitrate(bitrate_bps); - bool send_queue_is_full = - pacer_->ExpectedQueueTimeMs() > PacedSender::kMaxQueueLengthMs; - bitrate_bps = send_queue_is_full ? 0 : bitrate_bps; - if ((network_changed && !send_queue_is_full) || - UpdateSendQueueStatus(send_queue_is_full)) { - observer_->OnNetworkChanged(bitrate_bps, fraction_loss, rtt); - } -} - -bool CongestionController::UpdateSendQueueStatus(bool send_queue_is_full) { - rtc::CritScope cs(&critsect_); - bool result = send_queue_is_full_ != send_queue_is_full; - send_queue_is_full_ = send_queue_is_full; - return result; } } // namespace webrtc diff --git a/webrtc/modules/congestion_controller/congestion_controller_unittest.cc b/webrtc/modules/congestion_controller/congestion_controller_unittest.cc deleted file mode 100644 index a86e43329b..0000000000 --- a/webrtc/modules/congestion_controller/congestion_controller_unittest.cc +++ /dev/null @@ -1,111 +0,0 @@ -/* - * Copyright (c) 2016 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 "testing/gmock/include/gmock/gmock.h" -#include "testing/gtest/include/gtest/gtest.h" -#include "webrtc/modules/pacing/mock/mock_paced_sender.h" -#include "webrtc/modules/congestion_controller/include/congestion_controller.h" -#include "webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h" -#include "webrtc/modules/remote_bitrate_estimator/include/mock/mock_remote_bitrate_observer.h" -#include "webrtc/system_wrappers/include/clock.h" - -using testing::_; -using testing::NiceMock; -using testing::Return; -using testing::SaveArg; -using testing::StrictMock; - -namespace webrtc { -namespace test { - -class CongestionControllerTest : public ::testing::Test { - protected: - CongestionControllerTest() : clock_(123456) {} - ~CongestionControllerTest() override {} - - void SetUp() override { - EXPECT_CALL(observer_, OnNetworkChanged(_, _, _)) - .WillOnce(SaveArg<0>(&initial_bitrate_bps_)); - - pacer_ = new NiceMock(); - std::unique_ptr pacer(pacer_); // Passes ownership. - std::unique_ptr packet_router(new PacketRouter()); - controller_.reset( - new CongestionController(&clock_, &observer_, &remote_bitrate_observer_, - std::move(packet_router), std::move(pacer))); - EXPECT_GT(initial_bitrate_bps_, 0u); - bandwidth_observer_.reset( - controller_->GetBitrateController()->CreateRtcpBandwidthObserver()); - } - - SimulatedClock clock_; - StrictMock observer_; - NiceMock* pacer_; - NiceMock remote_bitrate_observer_; - std::unique_ptr bandwidth_observer_; - std::unique_ptr controller_; - uint32_t initial_bitrate_bps_ = 0; -}; - -TEST_F(CongestionControllerTest, OnNetworkChanged) { - // Test no change. - clock_.AdvanceTimeMilliseconds(25); - controller_->Process(); - - EXPECT_CALL(observer_, OnNetworkChanged(initial_bitrate_bps_ * 2, _, _)); - bandwidth_observer_->OnReceivedEstimatedBitrate(initial_bitrate_bps_ * 2); - clock_.AdvanceTimeMilliseconds(25); - controller_->Process(); - - EXPECT_CALL(observer_, OnNetworkChanged(initial_bitrate_bps_, _, _)); - bandwidth_observer_->OnReceivedEstimatedBitrate(initial_bitrate_bps_); - clock_.AdvanceTimeMilliseconds(25); - controller_->Process(); -} - -TEST_F(CongestionControllerTest, OnSendQueueFull) { - EXPECT_CALL(*pacer_, ExpectedQueueTimeMs()) - .WillOnce(Return(PacedSender::kMaxQueueLengthMs + 1)); - - EXPECT_CALL(observer_, OnNetworkChanged(0, _, _)); - controller_->Process(); - - // Let the pacer not be full next time the controller checks. - EXPECT_CALL(*pacer_, ExpectedQueueTimeMs()) - .WillOnce(Return(PacedSender::kMaxQueueLengthMs - 1)); - - EXPECT_CALL(observer_, OnNetworkChanged(initial_bitrate_bps_, _, _)); - controller_->Process(); -} - -TEST_F(CongestionControllerTest, OnSendQueueFullAndEstimateChange) { - EXPECT_CALL(*pacer_, ExpectedQueueTimeMs()) - .WillOnce(Return(PacedSender::kMaxQueueLengthMs + 1)); - EXPECT_CALL(observer_, OnNetworkChanged(0, _, _)); - controller_->Process(); - - // Receive new estimate but let the queue still be full. - bandwidth_observer_->OnReceivedEstimatedBitrate(initial_bitrate_bps_ * 2); - EXPECT_CALL(*pacer_, ExpectedQueueTimeMs()) - .WillOnce(Return(PacedSender::kMaxQueueLengthMs + 1)); - clock_.AdvanceTimeMilliseconds(25); - controller_->Process(); - - // Let the pacer not be full next time the controller checks. - // |OnNetworkChanged| should be called with the new estimate. - EXPECT_CALL(*pacer_, ExpectedQueueTimeMs()) - .WillOnce(Return(PacedSender::kMaxQueueLengthMs - 1)); - EXPECT_CALL(observer_, OnNetworkChanged(initial_bitrate_bps_ * 2, _, _)); - clock_.AdvanceTimeMilliseconds(25); - controller_->Process(); -} - -} // namespace test -} // namespace webrtc diff --git a/webrtc/modules/congestion_controller/include/congestion_controller.h b/webrtc/modules/congestion_controller/include/congestion_controller.h index a3b672e3bc..284070cc21 100644 --- a/webrtc/modules/congestion_controller/include/congestion_controller.h +++ b/webrtc/modules/congestion_controller/include/congestion_controller.h @@ -19,7 +19,6 @@ #include "webrtc/modules/include/module.h" #include "webrtc/modules/include/module_common_types.h" #include "webrtc/modules/pacing/packet_router.h" -#include "webrtc/modules/pacing/paced_sender.h" #include "webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.h" #include "webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.h" @@ -32,6 +31,7 @@ namespace webrtc { class BitrateController; class BitrateObserver; class Clock; +class PacedSender; class ProcessThread; class RemoteBitrateEstimator; class RemoteBitrateObserver; @@ -39,33 +39,9 @@ class TransportFeedbackObserver; class CongestionController : public CallStatsObserver, public Module { public: - // Observer class for bitrate changes announced due to change in bandwidth - // estimate or due to that the send pacer is full. Fraction loss and rtt is - // also part of this callback to allow the observer to optimize its settings - // for different types of network environments. The bitrate does not include - // packet headers and is measured in bits per second. - class Observer { - public: - virtual void OnNetworkChanged(uint32_t bitrate_bps, - uint8_t fraction_loss, // 0 - 255. - int64_t rtt_ms) = 0; - - protected: - virtual ~Observer() {} - }; - // Deprecated - // TODO(perkj): Remove once no other clients use this ctor. CongestionController(Clock* clock, BitrateObserver* bitrate_observer, RemoteBitrateObserver* remote_bitrate_observer); - CongestionController(Clock* clock, - Observer* observer, - RemoteBitrateObserver* remote_bitrate_observer); - CongestionController(Clock* clock, - Observer* observer, - RemoteBitrateObserver* remote_bitrate_observer, - std::unique_ptr packet_router, - std::unique_ptr pacer); virtual ~CongestionController(); virtual void SetBweBitrates(int min_bitrate_bps, @@ -77,11 +53,12 @@ class CongestionController : public CallStatsObserver, public Module { bool send_side_bwe); virtual int64_t GetPacerQueuingDelayMs() const; virtual PacedSender* pacer() { return pacer_.get(); } - virtual PacketRouter* packet_router() { return packet_router_.get(); } + virtual PacketRouter* packet_router() { return &packet_router_; } virtual TransportFeedbackObserver* GetTransportFeedbackObserver(); - void SetAllocatedSendBitrate(int allocated_bitrate_bps, - int padding_bitrate_bps); + virtual void UpdatePacerBitrate(int bitrate_kbps, + int max_bitrate_kbps, + int min_bitrate_kbps); virtual void OnSentPacket(const rtc::SentPacket& sent_packet); @@ -93,23 +70,14 @@ class CongestionController : public CallStatsObserver, public Module { void Process() override; private: - void Init(); - void MaybeTriggerOnNetworkChanged(); - // Updates |send_queue_is_full_|. Returns true if |send_queue_is_full_| - // has changed. - bool UpdateSendQueueStatus(bool send_queue_is_full); - Clock* const clock_; - Observer* const observer_; - const std::unique_ptr packet_router_; const std::unique_ptr pacer_; const std::unique_ptr remote_bitrate_estimator_; const std::unique_ptr bitrate_controller_; + PacketRouter packet_router_; RemoteEstimatorProxy remote_estimator_proxy_; TransportFeedbackAdapter transport_feedback_adapter_; int min_bitrate_bps_; - rtc::CriticalSection critsect_; - bool send_queue_is_full_ GUARDED_BY(critsect_); RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(CongestionController); }; diff --git a/webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h b/webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h index 20955ea81a..c5b2412845 100644 --- a/webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h +++ b/webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h @@ -19,20 +19,14 @@ namespace webrtc { namespace test { -class MockCongestionObserver : public CongestionController::Observer { - public: - MOCK_METHOD3(OnNetworkChanged, - void(uint32_t bitrate_bps, - uint8_t fraction_loss, - int64_t rtt_ms)); -}; - class MockCongestionController : public CongestionController { public: MockCongestionController(Clock* clock, - Observer* observer, + BitrateObserver* bitrate_observer, RemoteBitrateObserver* remote_bitrate_observer) - : CongestionController(clock, observer, remote_bitrate_observer) {} + : CongestionController(clock, + bitrate_observer, + remote_bitrate_observer) {} MOCK_METHOD3(SetBweBitrates, void(int min_bitrate_bps, int start_bitrate_bps, diff --git a/webrtc/modules/modules.gyp b/webrtc/modules/modules.gyp index 4365c653f0..921697e528 100644 --- a/webrtc/modules/modules.gyp +++ b/webrtc/modules/modules.gyp @@ -270,7 +270,6 @@ 'audio_processing/vad/voice_activity_detector_unittest.cc', 'bitrate_controller/bitrate_controller_unittest.cc', 'bitrate_controller/send_side_bandwidth_estimation_unittest.cc', - 'congestion_controller/congestion_controller_unittest.cc', 'media_file/media_file_unittest.cc', 'module_common_types_unittest.cc', 'pacing/bitrate_prober_unittest.cc', diff --git a/webrtc/modules/pacing/mock/mock_paced_sender.h b/webrtc/modules/pacing/mock/mock_paced_sender.h index c0dd4887c8..c710dbcbea 100644 --- a/webrtc/modules/pacing/mock/mock_paced_sender.h +++ b/webrtc/modules/pacing/mock/mock_paced_sender.h @@ -22,7 +22,7 @@ namespace webrtc { class MockPacedSender : public PacedSender { public: - MockPacedSender() : PacedSender(Clock::GetRealTimeClock(), nullptr, 0) {} + MockPacedSender() : PacedSender(Clock::GetRealTimeClock(), NULL, 0, 0, 0) {} MOCK_METHOD6(SendPacket, bool(Priority priority, uint32_t ssrc, uint16_t sequence_number, @@ -31,7 +31,6 @@ class MockPacedSender : public PacedSender { bool retransmission)); MOCK_CONST_METHOD0(QueueInMs, int64_t()); MOCK_CONST_METHOD0(QueueInPackets, int()); - MOCK_CONST_METHOD0(ExpectedQueueTimeMs, int64_t()); }; } // namespace webrtc diff --git a/webrtc/modules/pacing/paced_sender.cc b/webrtc/modules/pacing/paced_sender.cc index 418c115c0c..b56d28510f 100644 --- a/webrtc/modules/pacing/paced_sender.cc +++ b/webrtc/modules/pacing/paced_sender.cc @@ -246,21 +246,20 @@ const int64_t PacedSender::kMaxQueueLengthMs = 2000; const float PacedSender::kDefaultPaceMultiplier = 2.5f; PacedSender::PacedSender(Clock* clock, - PacketSender* packet_sender, - int estimated_bitrate_bps) + Callback* callback, + int bitrate_kbps, + int max_bitrate_kbps, + int min_bitrate_kbps) : clock_(clock), - packet_sender_(packet_sender), + callback_(callback), critsect_(CriticalSectionWrapper::CreateCriticalSection()), paused_(false), probing_enabled_(true), - media_budget_(new paced_sender::IntervalBudget( - estimated_bitrate_bps / 1000 * kDefaultPaceMultiplier)), - padding_budget_(new paced_sender::IntervalBudget(0)), + media_budget_(new paced_sender::IntervalBudget(max_bitrate_kbps)), + padding_budget_(new paced_sender::IntervalBudget(min_bitrate_kbps)), prober_(new BitrateProber()), - estimated_bitrate_bps_(estimated_bitrate_bps), - min_send_bitrate_kbps_(0u), - pacing_bitrate_kbps_(estimated_bitrate_bps / 1000 * - kDefaultPaceMultiplier), + bitrate_bps_(1000 * bitrate_kbps), + max_bitrate_kbps_(max_bitrate_kbps), time_last_update_us_(clock->TimeInMicroseconds()), packets_(new paced_sender::PacketQueue(clock)), packet_counter_(0) { @@ -284,24 +283,16 @@ void PacedSender::SetProbingEnabled(bool enabled) { probing_enabled_ = enabled; } -void PacedSender::SetEstimatedBitrate(uint32_t bitrate_bps) { - LOG(LS_INFO) << "SetNetWorkEstimateTargetBitrate, bitrate " << bitrate_bps; - +void PacedSender::UpdateBitrate(int bitrate_kbps, + int max_bitrate_kbps, + int min_bitrate_kbps) { CriticalSectionScoped cs(critsect_.get()); - estimated_bitrate_bps_ = bitrate_bps; - pacing_bitrate_kbps_ = - std::max(min_send_bitrate_kbps_, estimated_bitrate_bps_ / 1000) * - kDefaultPaceMultiplier; -} - -void PacedSender::SetAllocatedSendBitrate(int allocated_bitrate, - int padding_bitrate) { - CriticalSectionScoped cs(critsect_.get()); - min_send_bitrate_kbps_ = allocated_bitrate / 1000; - pacing_bitrate_kbps_ = - std::max(min_send_bitrate_kbps_, estimated_bitrate_bps_ / 1000) * - kDefaultPaceMultiplier; - padding_budget_->set_target_rate_kbps(padding_bitrate / 1000); + // Don't set media bitrate here as it may be boosted in order to meet max + // queue time constraint. Just update max_bitrate_kbps_ and let media_budget_ + // be updated in Process(). + padding_budget_->set_target_rate_kbps(min_bitrate_kbps); + bitrate_bps_ = 1000 * bitrate_kbps; + max_bitrate_kbps_ = max_bitrate_kbps; } void PacedSender::InsertPacket(RtpPacketSender::Priority priority, @@ -315,7 +306,7 @@ void PacedSender::InsertPacket(RtpPacketSender::Priority priority, if (probing_enabled_ && !prober_->IsProbing()) prober_->SetEnabled(true); int64_t now_ms = clock_->TimeInMilliseconds(); - prober_->OnIncomingPacket(estimated_bitrate_bps_, bytes, now_ms); + prober_->OnIncomingPacket(bitrate_bps_, bytes, now_ms); if (capture_time_ms < 0) capture_time_ms = now_ms; @@ -327,9 +318,8 @@ void PacedSender::InsertPacket(RtpPacketSender::Priority priority, int64_t PacedSender::ExpectedQueueTimeMs() const { CriticalSectionScoped cs(critsect_.get()); - RTC_DCHECK_GT(pacing_bitrate_kbps_, 0u); - return static_cast(packets_->SizeInBytes() * 8 / - pacing_bitrate_kbps_); + RTC_DCHECK_GT(max_bitrate_kbps_, 0); + return static_cast(packets_->SizeInBytes() * 8 / max_bitrate_kbps_); } size_t PacedSender::QueueSizePackets() const { @@ -370,7 +360,7 @@ void PacedSender::Process() { CriticalSectionScoped cs(critsect_.get()); int64_t elapsed_time_ms = (now_us - time_last_update_us_ + 500) / 1000; time_last_update_us_ = now_us; - int target_bitrate_kbps = pacing_bitrate_kbps_; + int target_bitrate_kbps = max_bitrate_kbps_; // TODO(holmer): Remove the !paused_ check when issue 5307 has been fixed. if (!paused_ && elapsed_time_ms > 0) { size_t queue_size_bytes = packets_->SizeInBytes(); @@ -435,9 +425,10 @@ bool PacedSender::SendPacket(const paced_sender::Packet& packet) { if (paused_ && packet.priority != kHighPriority) return false; critsect_->Leave(); - const bool success = packet_sender_->TimeToSendPacket( - packet.ssrc, packet.sequence_number, packet.capture_time_ms, - packet.retransmission); + const bool success = callback_->TimeToSendPacket(packet.ssrc, + packet.sequence_number, + packet.capture_time_ms, + packet.retransmission); critsect_->Enter(); if (success) { @@ -456,7 +447,7 @@ bool PacedSender::SendPacket(const paced_sender::Packet& packet) { void PacedSender::SendPadding(size_t padding_needed) { critsect_->Leave(); - size_t bytes_sent = packet_sender_->TimeToSendPadding(padding_needed); + size_t bytes_sent = callback_->TimeToSendPadding(padding_needed); critsect_->Enter(); if (bytes_sent > 0) { diff --git a/webrtc/modules/pacing/paced_sender.h b/webrtc/modules/pacing/paced_sender.h index 52b63e53bc..16569b0404 100644 --- a/webrtc/modules/pacing/paced_sender.h +++ b/webrtc/modules/pacing/paced_sender.h @@ -33,7 +33,7 @@ class PacketQueue; class PacedSender : public Module, public RtpPacketSender { public: - class PacketSender { + class Callback { public: // Note: packets sent as a result of a callback should not pass by this // module again. @@ -48,7 +48,7 @@ class PacedSender : public Module, public RtpPacketSender { virtual size_t TimeToSendPadding(size_t bytes) = 0; protected: - virtual ~PacketSender() {} + virtual ~Callback() {} }; // Expected max pacer delay in ms. If ExpectedQueueTimeMs() is higher than @@ -68,8 +68,10 @@ class PacedSender : public Module, public RtpPacketSender { static const size_t kMinProbePacketSize = 200; PacedSender(Clock* clock, - PacketSender* packet_sender, - int target_bitrate_bps); + Callback* callback, + int bitrate_kbps, + int max_bitrate_kbps, + int min_bitrate_kbps); virtual ~PacedSender(); @@ -84,19 +86,14 @@ class PacedSender : public Module, public RtpPacketSender { // effect. void SetProbingEnabled(bool enabled); - // Sets the estimated capacity of the network. - // |bitrate_bps| is our estimate of what we are allowed to send on average. - // We will pace out bursts of packets at a bitrate of - // |bitrate_bps| * kDefaultPaceMultiplier. - void SetEstimatedBitrate(uint32_t bitrate_bps); - - // Sets the bitrate that has been allocated for encoders. - // |allocated_bitrate| might be higher that the estimated available network - // bitrate and if so, the pacer will send with |allocated_bitrate|. - // Padding packets will be utilized to reach |padding_bitrate| unless enough - // media packets are available. - void SetAllocatedSendBitrate(int allocated_bitrate_bps, - int padding_bitrate_bps); + // Set target bitrates for the pacer. + // We will pace out bursts of packets at a bitrate of |max_bitrate_kbps|. + // |bitrate_kbps| is our estimate of what we are allowed to send on average. + // Padding packets will be utilized to reach |min_bitrate| unless enough media + // packets are available. + void UpdateBitrate(int bitrate_kbps, + int max_bitrate_kbps, + int min_bitrate_kbps); // Returns true if we send the packet now, else it will add the packet // information to the queue and call TimeToSendPacket when it's time to send. @@ -137,7 +134,7 @@ class PacedSender : public Module, public RtpPacketSender { void SendPadding(size_t padding_needed) EXCLUSIVE_LOCKS_REQUIRED(critsect_); Clock* const clock_; - PacketSender* const packet_sender_; + Callback* const callback_; std::unique_ptr critsect_; bool paused_ GUARDED_BY(critsect_); @@ -155,9 +152,8 @@ class PacedSender : public Module, public RtpPacketSender { std::unique_ptr prober_ GUARDED_BY(critsect_); // Actual configured bitrates (media_budget_ may temporarily be higher in // order to meet pace time constraint). - uint32_t estimated_bitrate_bps_ GUARDED_BY(critsect_); - uint32_t min_send_bitrate_kbps_ GUARDED_BY(critsect_); - uint32_t pacing_bitrate_kbps_ GUARDED_BY(critsect_); + int bitrate_bps_ GUARDED_BY(critsect_); + int max_bitrate_kbps_ GUARDED_BY(critsect_); int64_t time_last_update_us_ GUARDED_BY(critsect_); diff --git a/webrtc/modules/pacing/paced_sender_unittest.cc b/webrtc/modules/pacing/paced_sender_unittest.cc index 15bb462949..941c81335b 100644 --- a/webrtc/modules/pacing/paced_sender_unittest.cc +++ b/webrtc/modules/pacing/paced_sender_unittest.cc @@ -22,9 +22,10 @@ using testing::Return; namespace webrtc { namespace test { -static const int kTargetBitrateBps = 800000; +static const int kTargetBitrate = 800; +static const float kPaceMultiplier = 1.5f; -class MockPacedSenderCallback : public PacedSender::PacketSender { +class MockPacedSenderCallback : public PacedSender::Callback { public: MOCK_METHOD4(TimeToSendPacket, bool(uint32_t ssrc, @@ -35,7 +36,7 @@ class MockPacedSenderCallback : public PacedSender::PacketSender { size_t(size_t bytes)); }; -class PacedSenderPadding : public PacedSender::PacketSender { +class PacedSenderPadding : public PacedSender::Callback { public: PacedSenderPadding() : padding_sent_(0) {} @@ -59,7 +60,7 @@ class PacedSenderPadding : public PacedSender::PacketSender { size_t padding_sent_; }; -class PacedSenderProbing : public PacedSender::PacketSender { +class PacedSenderProbing : public PacedSender::Callback { public: PacedSenderProbing(const std::list& expected_deltas, Clock* clock) : prev_packet_time_ms_(-1), @@ -107,7 +108,11 @@ class PacedSenderTest : public ::testing::Test { PacedSenderTest() : clock_(123456) { srand(0); // Need to initialize PacedSender after we initialize clock. - send_bucket_.reset(new PacedSender(&clock_, &callback_, kTargetBitrateBps)); + send_bucket_.reset(new PacedSender(&clock_, + &callback_, + kTargetBitrate, + kPaceMultiplier * kTargetBitrate, + 0)); // Default to bitrate probing disabled for testing purposes. Probing tests // have to enable probing, either by creating a new PacedSender instance or // by calling SetProbingEnabled(true). @@ -136,21 +141,29 @@ class PacedSenderTest : public ::testing::Test { TEST_F(PacedSenderTest, QueuePacket) { uint32_t ssrc = 12345; uint16_t sequence_number = 1234; - // Due to the multiplicative factor we can send 5 packets during a send - // interval. (network capacity * multiplier / (8 bits per byte * - // (packet size * #send intervals per second) - const size_t packets_to_send = - kTargetBitrateBps * PacedSender::kDefaultPaceMultiplier / (8 * 250 * 200); - for (size_t i = 0; i < packets_to_send; ++i) { - SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++, - clock_.TimeInMilliseconds(), 250, false); - } - + // Due to the multiplicative factor we can send 3 packets not 2 packets. + SendAndExpectPacket(PacedSender::kNormalPriority, + ssrc, + sequence_number++, + clock_.TimeInMilliseconds(), + 250, + false); + SendAndExpectPacket(PacedSender::kNormalPriority, + ssrc, + sequence_number++, + clock_.TimeInMilliseconds(), + 250, + false); + SendAndExpectPacket(PacedSender::kNormalPriority, + ssrc, + sequence_number++, + clock_.TimeInMilliseconds(), + 250, + false); int64_t queued_packet_timestamp = clock_.TimeInMilliseconds(); send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc, sequence_number, queued_packet_timestamp, 250, false); - EXPECT_EQ(packets_to_send + 1, send_bucket_->QueueSizePackets()); send_bucket_->Process(); EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess()); EXPECT_CALL(callback_, TimeToSendPadding(_)).Times(0); @@ -158,79 +171,86 @@ TEST_F(PacedSenderTest, QueuePacket) { EXPECT_EQ(1, send_bucket_->TimeUntilNextProcess()); clock_.AdvanceTimeMilliseconds(1); EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess()); - EXPECT_EQ(1u, send_bucket_->QueueSizePackets()); - EXPECT_CALL(callback_, TimeToSendPacket(ssrc, sequence_number, - queued_packet_timestamp, false)) + EXPECT_CALL( + callback_, + TimeToSendPacket(ssrc, sequence_number++, queued_packet_timestamp, false)) .Times(1) .WillRepeatedly(Return(true)); send_bucket_->Process(); sequence_number++; - EXPECT_EQ(0u, send_bucket_->QueueSizePackets()); - - // We can send packets_to_send -1 packets of size 250 during the current - // interval since one packet has already been sent. - for (size_t i = 0; i < packets_to_send - 1; ++i) { - SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++, - clock_.TimeInMilliseconds(), 250, false); - } + SendAndExpectPacket(PacedSender::kNormalPriority, + ssrc, + sequence_number++, + clock_.TimeInMilliseconds(), + 250, + false); + SendAndExpectPacket(PacedSender::kNormalPriority, + ssrc, + sequence_number++, + clock_.TimeInMilliseconds(), + 250, + false); send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc, sequence_number++, clock_.TimeInMilliseconds(), 250, false); - EXPECT_EQ(packets_to_send, send_bucket_->QueueSizePackets()); send_bucket_->Process(); - EXPECT_EQ(1u, send_bucket_->QueueSizePackets()); } TEST_F(PacedSenderTest, PaceQueuedPackets) { uint32_t ssrc = 12345; uint16_t sequence_number = 1234; - // Due to the multiplicative factor we can send 5 packets during a send - // interval. (network capacity * multiplier / (8 bits per byte * - // (packet size * #send intervals per second) - const size_t packets_to_send_per_interval = - kTargetBitrateBps * PacedSender::kDefaultPaceMultiplier / (8 * 250 * 200); - for (size_t i = 0; i < packets_to_send_per_interval; ++i) { - SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++, - clock_.TimeInMilliseconds(), 250, false); + // Due to the multiplicative factor we can send 3 packets not 2 packets. + for (int i = 0; i < 3; ++i) { + SendAndExpectPacket(PacedSender::kNormalPriority, + ssrc, + sequence_number++, + clock_.TimeInMilliseconds(), + 250, + false); } - - for (size_t j = 0; j < packets_to_send_per_interval * 10; ++j) { + for (int j = 0; j < 30; ++j) { send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc, sequence_number++, clock_.TimeInMilliseconds(), 250, false); } - EXPECT_EQ(packets_to_send_per_interval + packets_to_send_per_interval * 10, - send_bucket_->QueueSizePackets()); send_bucket_->Process(); - EXPECT_EQ(packets_to_send_per_interval * 10, - send_bucket_->QueueSizePackets()); EXPECT_CALL(callback_, TimeToSendPadding(_)).Times(0); for (int k = 0; k < 10; ++k) { EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess()); clock_.AdvanceTimeMilliseconds(5); EXPECT_CALL(callback_, TimeToSendPacket(ssrc, _, _, false)) - .Times(packets_to_send_per_interval) + .Times(3) .WillRepeatedly(Return(true)); EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess()); send_bucket_->Process(); } - EXPECT_EQ(0u, send_bucket_->QueueSizePackets()); EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess()); clock_.AdvanceTimeMilliseconds(5); EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess()); - EXPECT_EQ(0u, send_bucket_->QueueSizePackets()); send_bucket_->Process(); - - for (size_t i = 0; i < packets_to_send_per_interval; ++i) { - SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++, - clock_.TimeInMilliseconds(), 250, false); - } + SendAndExpectPacket(PacedSender::kNormalPriority, + ssrc, + sequence_number++, + clock_.TimeInMilliseconds(), + 250, + false); + SendAndExpectPacket(PacedSender::kNormalPriority, + ssrc, + sequence_number++, + clock_.TimeInMilliseconds(), + 250, + false); + SendAndExpectPacket(PacedSender::kNormalPriority, + ssrc, + sequence_number++, + clock_.TimeInMilliseconds(), + 250, + false); send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc, sequence_number, clock_.TimeInMilliseconds(), 250, false); send_bucket_->Process(); - EXPECT_EQ(1u, send_bucket_->QueueSizePackets()); } TEST_F(PacedSenderTest, PaceQueuedPacketsWithDuplicates) { @@ -238,18 +258,18 @@ TEST_F(PacedSenderTest, PaceQueuedPacketsWithDuplicates) { uint16_t sequence_number = 1234; uint16_t queued_sequence_number; - // Due to the multiplicative factor we can send 5 packets during a send - // interval. (network capacity * multiplier / (8 bits per byte * - // (packet size * #send intervals per second) - const size_t packets_to_send_per_interval = - kTargetBitrateBps * PacedSender::kDefaultPaceMultiplier / (8 * 250 * 200); - for (size_t i = 0; i < packets_to_send_per_interval; ++i) { - SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++, - clock_.TimeInMilliseconds(), 250, false); + // Due to the multiplicative factor we can send 3 packets not 2 packets. + for (int i = 0; i < 3; ++i) { + SendAndExpectPacket(PacedSender::kNormalPriority, + ssrc, + sequence_number++, + clock_.TimeInMilliseconds(), + 250, + false); } queued_sequence_number = sequence_number; - for (size_t j = 0; j < packets_to_send_per_interval * 10; ++j) { + for (int j = 0; j < 30; ++j) { // Send in duplicate packets. send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc, sequence_number, clock_.TimeInMilliseconds(), @@ -264,7 +284,7 @@ TEST_F(PacedSenderTest, PaceQueuedPacketsWithDuplicates) { EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess()); clock_.AdvanceTimeMilliseconds(5); - for (size_t i = 0; i < packets_to_send_per_interval; ++i) { + for (int i = 0; i < 3; ++i) { EXPECT_CALL(callback_, TimeToSendPacket(ssrc, queued_sequence_number++, _, false)) .Times(1) @@ -277,16 +297,28 @@ TEST_F(PacedSenderTest, PaceQueuedPacketsWithDuplicates) { clock_.AdvanceTimeMilliseconds(5); EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess()); send_bucket_->Process(); - - for (size_t i = 0; i < packets_to_send_per_interval; ++i) { - SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++, - clock_.TimeInMilliseconds(), 250, false); - } + SendAndExpectPacket(PacedSender::kNormalPriority, + ssrc, + sequence_number++, + clock_.TimeInMilliseconds(), + 250, + false); + SendAndExpectPacket(PacedSender::kNormalPriority, + ssrc, + sequence_number++, + clock_.TimeInMilliseconds(), + 250, + false); + SendAndExpectPacket(PacedSender::kNormalPriority, + ssrc, + sequence_number++, + clock_.TimeInMilliseconds(), + 250, + false); send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc, sequence_number++, clock_.TimeInMilliseconds(), 250, false); send_bucket_->Process(); - EXPECT_EQ(1u, send_bucket_->QueueSizePackets()); } TEST_F(PacedSenderTest, CanQueuePacketsWithSameSequenceNumberOnDifferentSsrcs) { @@ -316,33 +348,33 @@ TEST_F(PacedSenderTest, Padding) { uint32_t ssrc = 12345; uint16_t sequence_number = 1234; - send_bucket_->SetEstimatedBitrate(kTargetBitrateBps); - send_bucket_->SetAllocatedSendBitrate(kTargetBitrateBps, kTargetBitrateBps); - - // Due to the multiplicative factor we can send 5 packets during a send - // interval. (network capacity * multiplier / (8 bits per byte * - // (packet size * #send intervals per second) - const size_t packets_to_send_per_interval = - kTargetBitrateBps * PacedSender::kDefaultPaceMultiplier / (8 * 250 * 200); - for (size_t i = 0; i < packets_to_send_per_interval; ++i) { - SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++, - clock_.TimeInMilliseconds(), 250, false); - } + send_bucket_->UpdateBitrate( + kTargetBitrate, kPaceMultiplier * kTargetBitrate, kTargetBitrate); + // Due to the multiplicative factor we can send 3 packets not 2 packets. + SendAndExpectPacket(PacedSender::kNormalPriority, + ssrc, + sequence_number++, + clock_.TimeInMilliseconds(), + 250, + false); + SendAndExpectPacket(PacedSender::kNormalPriority, + ssrc, + sequence_number++, + clock_.TimeInMilliseconds(), + 250, + false); + SendAndExpectPacket(PacedSender::kNormalPriority, + ssrc, + sequence_number++, + clock_.TimeInMilliseconds(), + 250, + false); // No padding is expected since we have sent too much already. EXPECT_CALL(callback_, TimeToSendPadding(_)).Times(0); EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess()); clock_.AdvanceTimeMilliseconds(5); EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess()); send_bucket_->Process(); - EXPECT_EQ(0u, send_bucket_->QueueSizePackets()); - - // 5 milliseconds later should not send padding since we filled the buffers - // initially. - EXPECT_CALL(callback_, TimeToSendPadding(250)).Times(0); - EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess()); - clock_.AdvanceTimeMilliseconds(5); - EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess()); - send_bucket_->Process(); // 5 milliseconds later we have enough budget to send some padding. EXPECT_CALL(callback_, TimeToSendPadding(250)).Times(1). @@ -359,9 +391,8 @@ TEST_F(PacedSenderTest, VerifyPaddingUpToBitrate) { int64_t capture_time_ms = 56789; const int kTimeStep = 5; const int64_t kBitrateWindow = 100; - send_bucket_->SetEstimatedBitrate(kTargetBitrateBps); - send_bucket_->SetAllocatedSendBitrate(kTargetBitrateBps, kTargetBitrateBps); - + send_bucket_->UpdateBitrate( + kTargetBitrate, kPaceMultiplier * kTargetBitrate, kTargetBitrate); int64_t start_time = clock_.TimeInMilliseconds(); while (clock_.TimeInMilliseconds() - start_time < kBitrateWindow) { SendAndExpectPacket(PacedSender::kNormalPriority, @@ -384,11 +415,11 @@ TEST_F(PacedSenderTest, VerifyAverageBitrateVaryingMediaPayload) { const int kTimeStep = 5; const int64_t kBitrateWindow = 10000; PacedSenderPadding callback; - send_bucket_.reset(new PacedSender(&clock_, &callback, kTargetBitrateBps)); + send_bucket_.reset(new PacedSender( + &clock_, &callback, kTargetBitrate, kPaceMultiplier * kTargetBitrate, 0)); send_bucket_->SetProbingEnabled(false); - send_bucket_->SetEstimatedBitrate(kTargetBitrateBps); - send_bucket_->SetAllocatedSendBitrate(kTargetBitrateBps, kTargetBitrateBps); - + send_bucket_->UpdateBitrate( + kTargetBitrate, kPaceMultiplier * kTargetBitrate, kTargetBitrate); int64_t start_time = clock_.TimeInMilliseconds(); size_t media_bytes = 0; while (clock_.TimeInMilliseconds() - start_time < kBitrateWindow) { @@ -401,10 +432,9 @@ TEST_F(PacedSenderTest, VerifyAverageBitrateVaryingMediaPayload) { clock_.AdvanceTimeMilliseconds(kTimeStep); send_bucket_->Process(); } - EXPECT_NEAR(kTargetBitrateBps / 1000, + EXPECT_NEAR(kTargetBitrate, static_cast(8 * (media_bytes + callback.padding_sent()) / - kBitrateWindow), - 1); + kBitrateWindow), 1); } TEST_F(PacedSenderTest, Priority) { @@ -414,41 +444,50 @@ TEST_F(PacedSenderTest, Priority) { int64_t capture_time_ms = 56789; int64_t capture_time_ms_low_priority = 1234567; - // Due to the multiplicative factor we can send 5 packets during a send - // interval. (network capacity * multiplier / (8 bits per byte * - // (packet size * #send intervals per second) - const size_t packets_to_send_per_interval = - kTargetBitrateBps * PacedSender::kDefaultPaceMultiplier / (8 * 250 * 200); - for (size_t i = 0; i < packets_to_send_per_interval; ++i) { - SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++, - clock_.TimeInMilliseconds(), 250, false); - } + // Due to the multiplicative factor we can send 3 packets not 2 packets. + SendAndExpectPacket(PacedSender::kLowPriority, + ssrc, + sequence_number++, + capture_time_ms, + 250, + false); + SendAndExpectPacket(PacedSender::kNormalPriority, + ssrc, + sequence_number++, + capture_time_ms, + 250, + false); + SendAndExpectPacket(PacedSender::kNormalPriority, + ssrc, + sequence_number++, + capture_time_ms, + 250, + false); send_bucket_->Process(); - EXPECT_EQ(0u, send_bucket_->QueueSizePackets()); // Expect normal and low priority to be queued and high to pass through. send_bucket_->InsertPacket(PacedSender::kLowPriority, ssrc_low_priority, sequence_number++, capture_time_ms_low_priority, 250, false); - - for (size_t i = 0; i < packets_to_send_per_interval; ++i) { - send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc, - sequence_number++, capture_time_ms, 250, false); - } + send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc, + sequence_number++, capture_time_ms, 250, false); + send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc, + sequence_number++, capture_time_ms, 250, false); + send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc, + sequence_number++, capture_time_ms, 250, false); send_bucket_->InsertPacket(PacedSender::kHighPriority, ssrc, sequence_number++, capture_time_ms, 250, false); // Expect all high and normal priority to be sent out first. EXPECT_CALL(callback_, TimeToSendPadding(_)).Times(0); EXPECT_CALL(callback_, TimeToSendPacket(ssrc, _, capture_time_ms, false)) - .Times(packets_to_send_per_interval + 1) + .Times(4) .WillRepeatedly(Return(true)); EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess()); clock_.AdvanceTimeMilliseconds(5); EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess()); send_bucket_->Process(); - EXPECT_EQ(1u, send_bucket_->QueueSizePackets()); EXPECT_CALL(callback_, TimeToSendPacket( @@ -474,30 +513,23 @@ TEST_F(PacedSenderTest, HighPrioDoesntAffectBudget) { capture_time_ms, 250, false); } send_bucket_->Process(); - // Low prio packets does affect the budget. - // Due to the multiplicative factor we can send 5 packets during a send - // interval. (network capacity * multiplier / (8 bits per byte * - // (packet size * #send intervals per second) - const size_t packets_to_send_per_interval = - kTargetBitrateBps * PacedSender::kDefaultPaceMultiplier / (8 * 250 * 200); - for (size_t i = 0; i < packets_to_send_per_interval; ++i) { + // Low prio packets does affect the budget, so we should only be able to send + // 3 at once, the 4th should be queued. + for (int i = 0; i < 3; ++i) { SendAndExpectPacket(PacedSender::kLowPriority, ssrc, sequence_number++, - clock_.TimeInMilliseconds(), 250, false); + capture_time_ms, 250, false); } send_bucket_->InsertPacket(PacedSender::kLowPriority, ssrc, sequence_number, capture_time_ms, 250, false); EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess()); clock_.AdvanceTimeMilliseconds(5); send_bucket_->Process(); - EXPECT_EQ(1u, send_bucket_->QueueSizePackets()); EXPECT_CALL(callback_, TimeToSendPacket(ssrc, sequence_number++, capture_time_ms, false)) - .Times(1) - .WillRepeatedly(Return(true)); + .Times(1); EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess()); clock_.AdvanceTimeMilliseconds(5); send_bucket_->Process(); - EXPECT_EQ(0u, send_bucket_->QueueSizePackets()); } TEST_F(PacedSenderTest, Pause) { @@ -508,16 +540,25 @@ TEST_F(PacedSenderTest, Pause) { EXPECT_EQ(0, send_bucket_->QueueInMs()); - // Due to the multiplicative factor we can send 5 packets during a send - // interval. (network capacity * multiplier / (8 bits per byte * - // (packet size * #send intervals per second) - const size_t packets_to_send_per_interval = - kTargetBitrateBps * PacedSender::kDefaultPaceMultiplier / (8 * 250 * 200); - for (size_t i = 0; i < packets_to_send_per_interval; ++i) { - SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++, - clock_.TimeInMilliseconds(), 250, false); - } - + // Due to the multiplicative factor we can send 3 packets not 2 packets. + SendAndExpectPacket(PacedSender::kLowPriority, + ssrc, + sequence_number++, + capture_time_ms, + 250, + false); + SendAndExpectPacket(PacedSender::kNormalPriority, + ssrc, + sequence_number++, + capture_time_ms, + 250, + false); + SendAndExpectPacket(PacedSender::kNormalPriority, + ssrc, + sequence_number++, + capture_time_ms, + 250, + false); send_bucket_->Process(); send_bucket_->Pause(); @@ -627,18 +668,18 @@ TEST_F(PacedSenderTest, ExpectedQueueTimeMs) { uint16_t sequence_number = 1234; const size_t kNumPackets = 60; const size_t kPacketSize = 1200; - const int32_t kMaxBitrate = PacedSender::kDefaultPaceMultiplier * 30000; + const int32_t kMaxBitrate = kPaceMultiplier * 30; EXPECT_EQ(0, send_bucket_->ExpectedQueueTimeMs()); - send_bucket_->SetEstimatedBitrate(30000); + send_bucket_->UpdateBitrate(30, kMaxBitrate, 0); for (size_t i = 0; i < kNumPackets; ++i) { SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++, clock_.TimeInMilliseconds(), kPacketSize, false); } - // Queue in ms = 1000 * (bytes in queue) *8 / (bits per second) + // Queue in ms = 1000 * (bytes in queue) / (kbit per second * 1000 / 8) int64_t queue_in_ms = - static_cast(1000 * kNumPackets * kPacketSize * 8 / kMaxBitrate); + static_cast(kNumPackets * kPacketSize * 8 / kMaxBitrate); EXPECT_EQ(queue_in_ms, send_bucket_->ExpectedQueueTimeMs()); int64_t time_start = clock_.TimeInMilliseconds(); @@ -656,7 +697,7 @@ TEST_F(PacedSenderTest, ExpectedQueueTimeMs) { // Allow for aliasing, duration should be within one pack of max time limit. EXPECT_NEAR(duration, PacedSender::kMaxQueueLengthMs, - static_cast(1000 * kPacketSize * 8 / kMaxBitrate)); + static_cast(kPacketSize * 8 / kMaxBitrate)); } TEST_F(PacedSenderTest, QueueTimeGrowsOverTime) { @@ -664,7 +705,7 @@ TEST_F(PacedSenderTest, QueueTimeGrowsOverTime) { uint16_t sequence_number = 1234; EXPECT_EQ(0, send_bucket_->QueueInMs()); - send_bucket_->SetEstimatedBitrate(30000); + send_bucket_->UpdateBitrate(30, kPaceMultiplier * 30, 0); SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number, @@ -682,22 +723,25 @@ TEST_F(PacedSenderTest, ProbingWithInitialFrame) { const int kNumPackets = 11; const int kNumDeltas = kNumPackets - 1; const size_t kPacketSize = 1200; - const int kInitialBitrateBps = 300000; + const int kInitialBitrateKbps = 300; uint32_t ssrc = 12346; uint16_t sequence_number = 1234; - const int expected_deltas[kNumDeltas] = {10, 10, 10, 10, 10, 5, 5, 5, 5, 5}; std::list expected_deltas_list(expected_deltas, expected_deltas + kNumDeltas); PacedSenderProbing callback(expected_deltas_list, &clock_); - send_bucket_.reset(new PacedSender(&clock_, &callback, kInitialBitrateBps)); + send_bucket_.reset( + new PacedSender(&clock_, + &callback, + kInitialBitrateKbps, + kPaceMultiplier * kInitialBitrateKbps, + 0)); for (int i = 0; i < kNumPackets; ++i) { send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc, sequence_number++, clock_.TimeInMilliseconds(), kPacketSize, false); } - while (callback.packets_sent() < kNumPackets) { int time_until_process = send_bucket_->TimeUntilNextProcess(); if (time_until_process <= 0) { @@ -712,14 +756,15 @@ TEST_F(PacedSenderTest, ProbingWithTooSmallInitialFrame) { const int kNumPackets = 11; const int kNumDeltas = kNumPackets - 1; const size_t kPacketSize = 1200; - const int kInitialBitrateBps = 300000; + const int kInitialBitrateKbps = 300; uint32_t ssrc = 12346; uint16_t sequence_number = 1234; const int expected_deltas[kNumDeltas] = {10, 10, 10, 10, 10, 5, 5, 5, 5, 5}; std::list expected_deltas_list(expected_deltas, expected_deltas + kNumDeltas); PacedSenderProbing callback(expected_deltas_list, &clock_); - send_bucket_.reset(new PacedSender(&clock_, &callback, kInitialBitrateBps)); + send_bucket_.reset(new PacedSender(&clock_, &callback, kInitialBitrateKbps, + kPaceMultiplier * kInitialBitrateKbps, 0)); for (int i = 0; i < kNumPackets - 5; ++i) { send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc, @@ -794,21 +839,21 @@ TEST_F(PacedSenderTest, PaddingOveruse) { uint16_t sequence_number = 1234; const size_t kPacketSize = 1200; - send_bucket_->SetEstimatedBitrate(60000); - send_bucket_->SetAllocatedSendBitrate(60000, 0); - + // Min bitrate 0 => no padding, padding budget will stay at 0. + send_bucket_->UpdateBitrate(60, 90, 0); SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++, clock_.TimeInMilliseconds(), kPacketSize, false); send_bucket_->Process(); // Add 30kbit padding. When increasing budget, media budget will increase from - // negative (overuse) while padding budget will increase from 0. + // negative (overuse) while padding budget will increase form 0. clock_.AdvanceTimeMilliseconds(5); - send_bucket_->SetAllocatedSendBitrate(60000, 30000); + send_bucket_->UpdateBitrate(60, 90, 30); + + send_bucket_->InsertPacket(PacedSender::kHighPriority, ssrc, + sequence_number++, clock_.TimeInMilliseconds(), + kPacketSize, false); - SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++, - clock_.TimeInMilliseconds(), kPacketSize, false); - EXPECT_LT(5u, send_bucket_->ExpectedQueueTimeMs()); // Don't send padding if queue is non-empty, even if padding budget > 0. EXPECT_CALL(callback_, TimeToSendPadding(_)).Times(0); send_bucket_->Process(); @@ -819,8 +864,9 @@ TEST_F(PacedSenderTest, AverageQueueTime) { uint16_t sequence_number = 1234; const size_t kPacketSize = 1200; const int kBitrateBps = 10 * kPacketSize * 8; // 10 packets per second. + const int kBitrateKbps = (kBitrateBps + 500) / 1000; - send_bucket_->SetEstimatedBitrate(kBitrateBps); + send_bucket_->UpdateBitrate(kBitrateKbps, kBitrateKbps, kBitrateKbps); EXPECT_EQ(0, send_bucket_->AverageQueueTimeMs()); diff --git a/webrtc/modules/pacing/packet_router.h b/webrtc/modules/pacing/packet_router.h index a6039fddd8..635b931225 100644 --- a/webrtc/modules/pacing/packet_router.h +++ b/webrtc/modules/pacing/packet_router.h @@ -30,7 +30,7 @@ class TransportFeedback; // PacketRouter routes outgoing data to the correct sending RTP module, based // on the simulcast layer in RTPVideoHeader. -class PacketRouter : public PacedSender::PacketSender, +class PacketRouter : public PacedSender::Callback, public TransportSequenceNumberAllocator { public: PacketRouter(); diff --git a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator.gypi b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator.gypi index 7b20cf7f9d..32663d729b 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator.gypi +++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator.gypi @@ -62,7 +62,6 @@ 'type': 'static_library', 'dependencies': [ '<(DEPTH)/testing/gtest.gyp:gtest', - '<(DEPTH)/testing/gmock.gyp:gmock', ], 'sources': [ 'test/bwe.cc', diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h index 51e29e34c2..e6a05684cc 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h +++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h @@ -14,6 +14,7 @@ #include #include +#include "webrtc/base/constructormagic.h" #include "webrtc/modules/remote_bitrate_estimator/include/send_time_history.h" #include "webrtc/modules/remote_bitrate_estimator/test/bwe.h" diff --git a/webrtc/modules/remote_bitrate_estimator/test/packet_sender.cc b/webrtc/modules/remote_bitrate_estimator/test/packet_sender.cc index 00e7a8b157..3bcbc0a071 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/packet_sender.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/packet_sender.cc @@ -157,7 +157,12 @@ PacedVideoSender::PacedVideoSender(PacketProcessorListener* listener, VideoSource* source, BandwidthEstimatorType estimator) : VideoSender(listener, source, estimator), - pacer_(&clock_, this, source->bits_per_second()) { + pacer_(&clock_, + this, + source->bits_per_second() / 1000, + PacedSender::kDefaultPaceMultiplier * source->bits_per_second() / + 1000, + 0) { modules_.push_back(&pacer_); } @@ -305,7 +310,9 @@ void PacedVideoSender::OnNetworkChanged(uint32_t target_bitrate_bps, uint8_t fraction_lost, int64_t rtt) { VideoSender::OnNetworkChanged(target_bitrate_bps, fraction_lost, rtt); - pacer_.SetEstimatedBitrate(target_bitrate_bps); + pacer_.UpdateBitrate( + target_bitrate_bps / 1000, + PacedSender::kDefaultPaceMultiplier * target_bitrate_bps / 1000, 0); } const int kNoLimit = std::numeric_limits::max(); diff --git a/webrtc/modules/remote_bitrate_estimator/test/packet_sender.h b/webrtc/modules/remote_bitrate_estimator/test/packet_sender.h index 1280138461..5ed4a3bc38 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/packet_sender.h +++ b/webrtc/modules/remote_bitrate_estimator/test/packet_sender.h @@ -100,7 +100,7 @@ class VideoSender : public PacketSender, public BitrateObserver { RTC_DISALLOW_COPY_AND_ASSIGN(VideoSender); }; -class PacedVideoSender : public VideoSender, public PacedSender::PacketSender { +class PacedVideoSender : public VideoSender, public PacedSender::Callback { public: PacedVideoSender(PacketProcessorListener* listener, VideoSource* source, diff --git a/webrtc/video/encoder_state_feedback_unittest.cc b/webrtc/video/encoder_state_feedback_unittest.cc index b9f27d575a..3bc45f1843 100644 --- a/webrtc/video/encoder_state_feedback_unittest.cc +++ b/webrtc/video/encoder_state_feedback_unittest.cc @@ -12,6 +12,8 @@ #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" +#include "webrtc/modules/pacing/packet_router.h" +#include "webrtc/modules/bitrate_controller/include/bitrate_controller.h" #include "webrtc/modules/utility/include/mock/mock_process_thread.h" #include "webrtc/video/vie_encoder.h" @@ -21,11 +23,13 @@ namespace webrtc { class MockVieEncoder : public ViEEncoder { public: - explicit MockVieEncoder(ProcessThread* process_thread) + explicit MockVieEncoder(ProcessThread* process_thread, PacedSender* pacer) : ViEEncoder(1, process_thread, nullptr, - nullptr) {} + nullptr, + nullptr, + pacer) {} ~MockVieEncoder() {} MOCK_METHOD1(OnReceivedIntraFrameRequest, void(size_t)); @@ -36,7 +40,13 @@ class MockVieEncoder : public ViEEncoder { class VieKeyRequestTest : public ::testing::Test { public: VieKeyRequestTest() - : encoder_(&process_thread_), + : pacer_(Clock::GetRealTimeClock(), + &router_, + BitrateController::kDefaultStartBitrateKbps, + PacedSender::kDefaultPaceMultiplier * + BitrateController::kDefaultStartBitrateKbps, + 0), + encoder_(&process_thread_, &pacer_), simulated_clock_(123456789), encoder_state_feedback_( &simulated_clock_, @@ -46,6 +56,8 @@ class VieKeyRequestTest : public ::testing::Test { protected: const uint32_t kSsrc = 1234; NiceMock process_thread_; + PacketRouter router_; + PacedSender pacer_; MockVieEncoder encoder_; SimulatedClock simulated_clock_; EncoderStateFeedback encoder_state_feedback_; diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index 840991b4ba..84eb836243 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -379,7 +379,9 @@ VideoSendStream::VideoSendStream( vie_encoder_(num_cpu_cores, module_process_thread_, &stats_proxy_, - &overuse_detector_), + config.pre_encode_callback, + &overuse_detector_, + congestion_controller_->pacer()), encoder_feedback_(Clock::GetRealTimeClock(), config.rtp.ssrcs, &vie_encoder_), @@ -588,14 +590,8 @@ void VideoSendStream::EncoderProcess() { } VideoFrame frame; - if (input_.GetVideoFrame(&frame)) { - // TODO(perkj): |pre_encode_callback| is only used by tests. Tests should - // register as a sink to the VideoSource instead. - if (config_.pre_encode_callback) { - config_.pre_encode_callback->OnFrame(frame); - } + if (input_.GetVideoFrame(&frame)) vie_encoder_.EncodeVideoFrame(frame); - } } vie_encoder_.DeRegisterExternalEncoder(config_.encoder_settings.payload_type); } diff --git a/webrtc/video/vie_encoder.cc b/webrtc/video/vie_encoder.cc index ec20ad95fd..2a7c21adc9 100644 --- a/webrtc/video/vie_encoder.cc +++ b/webrtc/video/vie_encoder.cc @@ -48,7 +48,9 @@ class QMVideoSettingsCallback : public VCMQMSettingsCallback { ViEEncoder::ViEEncoder(uint32_t number_of_cores, ProcessThread* module_process_thread, SendStatisticsProxy* stats_proxy, - OveruseFrameDetector* overuse_detector) + rtc::VideoSinkInterface* pre_encode_callback, + OveruseFrameDetector* overuse_detector, + PacedSender* pacer) : number_of_cores_(number_of_cores), vp_(VideoProcessing::Create()), qm_callback_(new QMVideoSettingsCallback(vp_.get())), @@ -58,7 +60,9 @@ ViEEncoder::ViEEncoder(uint32_t number_of_cores, qm_callback_.get(), this), stats_proxy_(stats_proxy), + pre_encode_callback_(pre_encode_callback), overuse_detector_(overuse_detector), + pacer_(pacer), time_of_last_frame_activity_ms_(0), encoder_config_(), min_transmit_bitrate_bps_(0), @@ -225,7 +229,8 @@ bool ViEEncoder::EncoderPaused() const { if (encoder_paused_) { return true; } - if (video_suspended_ || last_observed_bitrate_bps_ == 0) { + if (pacer_->ExpectedQueueTimeMs() > PacedSender::kMaxQueueLengthMs) { + // Too much data in pacer queue, drop frame. return true; } return !network_is_transmitting_; @@ -274,6 +279,10 @@ void ViEEncoder::EncodeVideoFrame(const VideoFrame& video_frame) { } } + if (pre_encode_callback_) { + pre_encode_callback_->OnFrame(*frame_to_send); + } + if (codec_type == webrtc::kVideoCodecVP8) { webrtc::CodecSpecificInfo codec_specific_info; codec_specific_info.codecType = webrtc::kVideoCodecVP8; @@ -368,7 +377,7 @@ void ViEEncoder::OnReceivedIntraFrameRequest(size_t stream_index) { void ViEEncoder::OnBitrateUpdated(uint32_t bitrate_bps, uint8_t fraction_lost, int64_t round_trip_time_ms) { - LOG(LS_VERBOSE) << "OnBitrateUpdated, bitrate " << bitrate_bps + LOG(LS_VERBOSE) << "OnBitrateUpdated, bitrate" << bitrate_bps << " packet loss " << static_cast(fraction_lost) << " rtt " << round_trip_time_ms; video_sender_.SetChannelParameters(bitrate_bps, fraction_lost, diff --git a/webrtc/video/vie_encoder.h b/webrtc/video/vie_encoder.h index e309dd5047..5db5e3f6ba 100644 --- a/webrtc/video/vie_encoder.h +++ b/webrtc/video/vie_encoder.h @@ -59,7 +59,10 @@ class ViEEncoder : public VideoEncoderRateObserver, ViEEncoder(uint32_t number_of_cores, ProcessThread* module_process_thread, SendStatisticsProxy* stats_proxy, - OveruseFrameDetector* overuse_detector); + // TODO(nisse): Used only for tests, delete? + rtc::VideoSinkInterface* pre_encode_callback, + OveruseFrameDetector* overuse_detector, + PacedSender* pacer); ~ViEEncoder(); vcm::VideoSender* video_sender(); @@ -130,7 +133,9 @@ class ViEEncoder : public VideoEncoderRateObserver, rtc::CriticalSection data_cs_; SendStatisticsProxy* const stats_proxy_; + rtc::VideoSinkInterface* const pre_encode_callback_; OveruseFrameDetector* const overuse_detector_; + PacedSender* const pacer_; // 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