diff --git a/webrtc/audio/audio_receive_stream_unittest.cc b/webrtc/audio/audio_receive_stream_unittest.cc index 300ab20054..a5318d5150 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 24efaada10..a94034c649 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/bitrate_controller/include/mock/mock_bitrate_controller.h" +#include "webrtc/modules/congestion_controller/include/mock/mock_congestion_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 097378f02a..4c8d2a0c44 100644 --- a/webrtc/call/bitrate_allocator.cc +++ b/webrtc/call/bitrate_allocator.cc @@ -54,7 +54,9 @@ 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_ <= sum_min_bitrates) + if (last_bitrate_bps_ == 0) + return ZeroRateAllocation(); + else if (last_bitrate_bps_ <= sum_min_bitrates) return LowRateAllocation(last_bitrate_bps_); else return NormalRateAllocation(last_bitrate_bps_, sum_min_bitrates); @@ -104,18 +106,6 @@ 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) { @@ -170,6 +160,14 @@ 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 404a312dad..fc88b783b5 100644 --- a/webrtc/call/bitrate_allocator.h +++ b/webrtc/call/bitrate_allocator.h @@ -60,9 +60,6 @@ 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). @@ -97,6 +94,7 @@ 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 6e0cdd4d78..63149acbe3 100644 --- a/webrtc/call/bitrate_allocator_unittest.cc +++ b/webrtc/call/bitrate_allocator_unittest.cc @@ -96,6 +96,12 @@ 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 { @@ -171,6 +177,13 @@ 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 82ca630ad2..f7c66db4b2 100644 --- a/webrtc/call/call.cc +++ b/webrtc/call/call.cc @@ -52,8 +52,9 @@ const int Call::Config::kDefaultStartBitrateBps = 300000; namespace internal { -class Call : public webrtc::Call, public PacketReceiver, - public BitrateObserver { +class Call : public webrtc::Call, + public PacketReceiver, + public CongestionController::Observer { public: explicit Call(const Call::Config& config); virtual ~Call(); @@ -699,10 +700,8 @@ 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_->UpdatePacerBitrate( - target_bitrate_bps / 1000, - PacedSender::kDefaultPaceMultiplier * pacer_bitrate_bps / 1000, - pad_up_to_bitrate_bps / 1000); + congestion_controller_->SetAllocatedSendBitrate(allocated_bitrate_bps, + pad_up_to_bitrate_bps); } 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 3c0d37c3b0..09652d8419 100644 --- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc +++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc @@ -83,6 +83,10 @@ 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), @@ -94,8 +98,8 @@ BitrateControllerImpl::BitrateControllerImpl(Clock* clock, last_fraction_loss_(0), last_rtt_ms_(0), last_reserved_bitrate_bps_(0) { - // 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 + // 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 // controller. We do this to avoid having to keep synchronized initial values // in both the controller and the allocator. MaybeTriggerOnNetworkChanged(); @@ -199,11 +203,15 @@ void BitrateControllerImpl::OnReceivedRtcpReceiverReport( } void BitrateControllerImpl::MaybeTriggerOnNetworkChanged() { - uint32_t bitrate; + if (!observer_) + return; + + uint32_t bitrate_bps; uint8_t fraction_loss; int64_t rtt; - if (GetNetworkParameters(&bitrate, &fraction_loss, &rtt)) - observer_->OnNetworkChanged(bitrate, fraction_loss, rtt); + + if (GetNetworkParameters(&bitrate_bps, &fraction_loss, &rtt)) + observer_->OnNetworkChanged(bitrate_bps, 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 a9661212d1..5a61379ce0 100644 --- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.h +++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.h @@ -28,6 +28,8 @@ 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() {} @@ -50,6 +52,11 @@ 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; @@ -64,20 +71,16 @@ 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* clock_; - BitrateObserver* observer_; + Clock* const clock_; + BitrateObserver* const 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 3f467ef8a4..4f92a3884b 100644 --- a/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc +++ b/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc @@ -14,11 +14,16 @@ #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 webrtc::RtcpBandwidthObserver; -using webrtc::BitrateObserver; +using ::testing::Exactly; +using ::testing::Return; + using webrtc::BitrateController; +using webrtc::BitrateObserver; +using webrtc::PacedSender; +using webrtc::RtcpBandwidthObserver; 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 d6cbc02d18..a61cf6a7a7 100644 --- a/webrtc/modules/bitrate_controller/include/bitrate_controller.h +++ b/webrtc/modules/bitrate_controller/include/bitrate_controller.h @@ -18,6 +18,7 @@ #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 { @@ -26,6 +27,8 @@ 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 @@ -46,10 +49,15 @@ class BitrateController : public Module { // estimation and divide the available bitrate between all its registered // BitrateObservers. public: - static const int kDefaultStartBitrateKbps = 300; + static const int kDefaultStartBitratebps = 300000; + // 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; @@ -71,6 +79,10 @@ 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 5290b01106..da6169e748 100644 --- a/webrtc/modules/bitrate_controller/include/mock/mock_bitrate_controller.h +++ b/webrtc/modules/bitrate_controller/include/mock/mock_bitrate_controller.h @@ -39,6 +39,8 @@ 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 14a73fe1cd..157258291a 100644 --- a/webrtc/modules/congestion_controller/congestion_controller.cc +++ b/webrtc/modules/congestion_controller/congestion_controller.cc @@ -20,7 +20,6 @@ #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" @@ -141,30 +140,71 @@ CongestionController::CongestionController( BitrateObserver* bitrate_observer, RemoteBitrateObserver* remote_bitrate_observer) : clock_(clock), + observer_(nullptr), + packet_router_(new PacketRouter()), pacer_(new PacedSender(clock_, - &packet_router_, - BitrateController::kDefaultStartBitrateKbps, - PacedSender::kDefaultPaceMultiplier * - BitrateController::kDefaultStartBitrateKbps, - 0)), + packet_router_.get())), + 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())), + 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)), 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_, bitrate_observer)), - remote_estimator_proxy_(clock_, &packet_router_), + bitrate_controller_(BitrateController::CreateBitrateController(clock_)), + remote_estimator_proxy_(clock_, packet_router_.get()), transport_feedback_adapter_(bitrate_controller_.get(), clock_), - min_bitrate_bps_(RemoteBitrateEstimator::kDefaultMinBitrateBps) { + min_bitrate_bps_(RemoteBitrateEstimator::kDefaultMinBitrateBps), + send_queue_is_full_(false) { + Init(); +} + +CongestionController::~CongestionController() {} + +void CongestionController::Init() { transport_feedback_adapter_.SetBitrateEstimator( new RemoteBitrateEstimatorAbsSendTime(&transport_feedback_adapter_)); transport_feedback_adapter_.GetBitrateEstimator()->SetMinBitrate( min_bitrate_bps_); } -CongestionController::~CongestionController() { -} - void CongestionController::SetBweBitrates(int min_bitrate_bps, int start_bitrate_bps, @@ -189,6 +229,7 @@ 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 { @@ -209,10 +250,9 @@ CongestionController::GetTransportFeedbackObserver() { return &transport_feedback_adapter_; } -void CongestionController::UpdatePacerBitrate(int bitrate_kbps, - int max_bitrate_kbps, - int min_bitrate_kbps) { - pacer_->UpdateBitrate(bitrate_kbps, max_bitrate_kbps, min_bitrate_kbps); +void CongestionController::SetAllocatedSendBitrate(int allocated_bitrate_bps, + int padding_bitrate_bps) { + pacer_->SetAllocatedSendBitrate(allocated_bitrate_bps, padding_bitrate_bps); } int64_t CongestionController::GetPacerQueuingDelayMs() const { @@ -245,6 +285,36 @@ 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 new file mode 100644 index 0000000000..20bf1a88e6 --- /dev/null +++ b/webrtc/modules/congestion_controller/congestion_controller_unittest.cc @@ -0,0 +1,117 @@ +/* + * 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 { + 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))); + bandwidth_observer_.reset( + controller_->GetBitrateController()->CreateRtcpBandwidthObserver()); + + // Set the initial bitrate estimate and expect the |observer| and |pacer_| + // to be updated. + EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps, _, _)); + EXPECT_CALL(*pacer_, SetEstimatedBitrate(kInitialBitrateBps)); + controller_->SetBweBitrates(0, kInitialBitrateBps, 5 * kInitialBitrateBps); + } + + SimulatedClock clock_; + StrictMock observer_; + NiceMock* pacer_; + NiceMock remote_bitrate_observer_; + std::unique_ptr bandwidth_observer_; + std::unique_ptr controller_; + const uint32_t kInitialBitrateBps = 60000; +}; + +TEST_F(CongestionControllerTest, OnNetworkChanged) { + // Test no change. + clock_.AdvanceTimeMilliseconds(25); + controller_->Process(); + + EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps * 2, _, _)); + EXPECT_CALL(*pacer_, SetEstimatedBitrate(kInitialBitrateBps * 2)); + bandwidth_observer_->OnReceivedEstimatedBitrate(kInitialBitrateBps * 2); + clock_.AdvanceTimeMilliseconds(25); + controller_->Process(); + + EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps, _, _)); + EXPECT_CALL(*pacer_, SetEstimatedBitrate(kInitialBitrateBps)); + bandwidth_observer_->OnReceivedEstimatedBitrate(kInitialBitrateBps); + 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(kInitialBitrateBps, _, _)); + 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(kInitialBitrateBps * 2); + EXPECT_CALL(*pacer_, ExpectedQueueTimeMs()) + .WillOnce(Return(PacedSender::kMaxQueueLengthMs + 1)); + // The send pacer should get the new estimate though. + EXPECT_CALL(*pacer_, SetEstimatedBitrate(kInitialBitrateBps * 2)); + 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(kInitialBitrateBps * 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 284070cc21..a3b672e3bc 100644 --- a/webrtc/modules/congestion_controller/include/congestion_controller.h +++ b/webrtc/modules/congestion_controller/include/congestion_controller.h @@ -19,6 +19,7 @@ #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" @@ -31,7 +32,6 @@ namespace webrtc { class BitrateController; class BitrateObserver; class Clock; -class PacedSender; class ProcessThread; class RemoteBitrateEstimator; class RemoteBitrateObserver; @@ -39,9 +39,33 @@ 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, @@ -53,12 +77,11 @@ 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_; } + virtual PacketRouter* packet_router() { return packet_router_.get(); } virtual TransportFeedbackObserver* GetTransportFeedbackObserver(); - virtual void UpdatePacerBitrate(int bitrate_kbps, - int max_bitrate_kbps, - int min_bitrate_kbps); + void SetAllocatedSendBitrate(int allocated_bitrate_bps, + int padding_bitrate_bps); virtual void OnSentPacket(const rtc::SentPacket& sent_packet); @@ -70,14 +93,23 @@ 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 c5b2412845..20955ea81a 100644 --- a/webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h +++ b/webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h @@ -19,14 +19,20 @@ 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, - BitrateObserver* bitrate_observer, + Observer* observer, RemoteBitrateObserver* remote_bitrate_observer) - : CongestionController(clock, - bitrate_observer, - remote_bitrate_observer) {} + : CongestionController(clock, 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 921697e528..4365c653f0 100644 --- a/webrtc/modules/modules.gyp +++ b/webrtc/modules/modules.gyp @@ -270,6 +270,7 @@ '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 c710dbcbea..bd7d7aaa49 100644 --- a/webrtc/modules/pacing/mock/mock_paced_sender.h +++ b/webrtc/modules/pacing/mock/mock_paced_sender.h @@ -22,15 +22,17 @@ namespace webrtc { class MockPacedSender : public PacedSender { public: - MockPacedSender() : PacedSender(Clock::GetRealTimeClock(), NULL, 0, 0, 0) {} + MockPacedSender() : PacedSender(Clock::GetRealTimeClock(), nullptr) {} MOCK_METHOD6(SendPacket, bool(Priority priority, uint32_t ssrc, uint16_t sequence_number, int64_t capture_time_ms, size_t bytes, bool retransmission)); + MOCK_METHOD1(SetEstimatedBitrate, void(uint32_t)); 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 b56d28510f..b14230cc3a 100644 --- a/webrtc/modules/pacing/paced_sender.cc +++ b/webrtc/modules/pacing/paced_sender.cc @@ -246,20 +246,18 @@ const int64_t PacedSender::kMaxQueueLengthMs = 2000; const float PacedSender::kDefaultPaceMultiplier = 2.5f; PacedSender::PacedSender(Clock* clock, - Callback* callback, - int bitrate_kbps, - int max_bitrate_kbps, - int min_bitrate_kbps) + PacketSender* packet_sender) : clock_(clock), - callback_(callback), + packet_sender_(packet_sender), critsect_(CriticalSectionWrapper::CreateCriticalSection()), paused_(false), probing_enabled_(true), - media_budget_(new paced_sender::IntervalBudget(max_bitrate_kbps)), - padding_budget_(new paced_sender::IntervalBudget(min_bitrate_kbps)), + media_budget_(new paced_sender::IntervalBudget(0)), + padding_budget_(new paced_sender::IntervalBudget(0)), prober_(new BitrateProber()), - bitrate_bps_(1000 * bitrate_kbps), - max_bitrate_kbps_(max_bitrate_kbps), + estimated_bitrate_bps_(0), + min_send_bitrate_kbps_(0u), + pacing_bitrate_kbps_(0), time_last_update_us_(clock->TimeInMicroseconds()), packets_(new paced_sender::PacketQueue(clock)), packet_counter_(0) { @@ -283,16 +281,24 @@ void PacedSender::SetProbingEnabled(bool enabled) { probing_enabled_ = enabled; } -void PacedSender::UpdateBitrate(int bitrate_kbps, - int max_bitrate_kbps, - int min_bitrate_kbps) { +void PacedSender::SetEstimatedBitrate(uint32_t bitrate_bps) { + LOG(LS_INFO) << "SetNetWorkEstimateTargetBitrate, bitrate " << bitrate_bps; + CriticalSectionScoped cs(critsect_.get()); - // 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; + 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); } void PacedSender::InsertPacket(RtpPacketSender::Priority priority, @@ -302,11 +308,13 @@ void PacedSender::InsertPacket(RtpPacketSender::Priority priority, size_t bytes, bool retransmission) { CriticalSectionScoped cs(critsect_.get()); + RTC_DCHECK(estimated_bitrate_bps_ > 0) + << "SetEstimatedBitrate must be called before InsertPacket."; if (probing_enabled_ && !prober_->IsProbing()) prober_->SetEnabled(true); int64_t now_ms = clock_->TimeInMilliseconds(); - prober_->OnIncomingPacket(bitrate_bps_, bytes, now_ms); + prober_->OnIncomingPacket(estimated_bitrate_bps_, bytes, now_ms); if (capture_time_ms < 0) capture_time_ms = now_ms; @@ -318,8 +326,9 @@ void PacedSender::InsertPacket(RtpPacketSender::Priority priority, int64_t PacedSender::ExpectedQueueTimeMs() const { CriticalSectionScoped cs(critsect_.get()); - RTC_DCHECK_GT(max_bitrate_kbps_, 0); - return static_cast(packets_->SizeInBytes() * 8 / max_bitrate_kbps_); + RTC_DCHECK_GT(pacing_bitrate_kbps_, 0u); + return static_cast(packets_->SizeInBytes() * 8 / + pacing_bitrate_kbps_); } size_t PacedSender::QueueSizePackets() const { @@ -360,7 +369,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 = max_bitrate_kbps_; + int target_bitrate_kbps = pacing_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(); @@ -425,10 +434,9 @@ bool PacedSender::SendPacket(const paced_sender::Packet& packet) { if (paused_ && packet.priority != kHighPriority) return false; critsect_->Leave(); - const bool success = callback_->TimeToSendPacket(packet.ssrc, - packet.sequence_number, - packet.capture_time_ms, - packet.retransmission); + const bool success = packet_sender_->TimeToSendPacket( + packet.ssrc, packet.sequence_number, packet.capture_time_ms, + packet.retransmission); critsect_->Enter(); if (success) { @@ -447,7 +455,7 @@ bool PacedSender::SendPacket(const paced_sender::Packet& packet) { void PacedSender::SendPadding(size_t padding_needed) { critsect_->Leave(); - size_t bytes_sent = callback_->TimeToSendPadding(padding_needed); + size_t bytes_sent = packet_sender_->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 16569b0404..e0da10f559 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 Callback { + class PacketSender { 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 ~Callback() {} + virtual ~PacketSender() {} }; // Expected max pacer delay in ms. If ExpectedQueueTimeMs() is higher than @@ -56,8 +56,6 @@ class PacedSender : public Module, public RtpPacketSender { // encoding them). Bitrate sent may temporarily exceed target set by // UpdateBitrate() so that this limit will be upheld. static const int64_t kMaxQueueLengthMs; - // Pace in kbits/s until we receive first estimate. - static const int kDefaultInitialPaceKbps = 2000; // Pacing-rate relative to our target send rate. // Multiplicative factor that is applied to the target bitrate to calculate // the number of bytes that can be transmitted per interval. @@ -68,10 +66,7 @@ class PacedSender : public Module, public RtpPacketSender { static const size_t kMinProbePacketSize = 200; PacedSender(Clock* clock, - Callback* callback, - int bitrate_kbps, - int max_bitrate_kbps, - int min_bitrate_kbps); + PacketSender* packet_sender); virtual ~PacedSender(); @@ -86,14 +81,20 @@ class PacedSender : public Module, public RtpPacketSender { // effect. void SetProbingEnabled(bool enabled); - // 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); + // Sets the estimated capacity of the network. Must be called once before + // packets can be sent. + // |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. + virtual void SetEstimatedBitrate(uint32_t bitrate_bps); + + // Sets the bitrate that has been allocated for encoders. + // |allocated_bitrate| might be higher that the estimated available network + // bitrate and if so, the pacer will send with |allocated_bitrate|. + // Padding packets will be utilized to reach |padding_bitrate| unless enough + // media packets are available. + void SetAllocatedSendBitrate(int allocated_bitrate_bps, + int padding_bitrate_bps); // 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. @@ -134,7 +135,7 @@ class PacedSender : public Module, public RtpPacketSender { void SendPadding(size_t padding_needed) EXCLUSIVE_LOCKS_REQUIRED(critsect_); Clock* const clock_; - Callback* const callback_; + PacketSender* const packet_sender_; std::unique_ptr critsect_; bool paused_ GUARDED_BY(critsect_); @@ -152,8 +153,9 @@ 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). - int bitrate_bps_ GUARDED_BY(critsect_); - int max_bitrate_kbps_ GUARDED_BY(critsect_); + 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_); 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 941c81335b..4410377e5f 100644 --- a/webrtc/modules/pacing/paced_sender_unittest.cc +++ b/webrtc/modules/pacing/paced_sender_unittest.cc @@ -22,10 +22,9 @@ using testing::Return; namespace webrtc { namespace test { -static const int kTargetBitrate = 800; -static const float kPaceMultiplier = 1.5f; +static const int kTargetBitrateBps = 800000; -class MockPacedSenderCallback : public PacedSender::Callback { +class MockPacedSenderCallback : public PacedSender::PacketSender { public: MOCK_METHOD4(TimeToSendPacket, bool(uint32_t ssrc, @@ -36,7 +35,7 @@ class MockPacedSenderCallback : public PacedSender::Callback { size_t(size_t bytes)); }; -class PacedSenderPadding : public PacedSender::Callback { +class PacedSenderPadding : public PacedSender::PacketSender { public: PacedSenderPadding() : padding_sent_(0) {} @@ -60,7 +59,7 @@ class PacedSenderPadding : public PacedSender::Callback { size_t padding_sent_; }; -class PacedSenderProbing : public PacedSender::Callback { +class PacedSenderProbing : public PacedSender::PacketSender { public: PacedSenderProbing(const std::list& expected_deltas, Clock* clock) : prev_packet_time_ms_(-1), @@ -108,15 +107,14 @@ 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_, - kTargetBitrate, - kPaceMultiplier * kTargetBitrate, - 0)); + send_bucket_.reset(new PacedSender(&clock_, &callback_)); // 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). send_bucket_->SetProbingEnabled(false); + send_bucket_->SetEstimatedBitrate(kTargetBitrateBps); + + clock_.AdvanceTimeMilliseconds(send_bucket_->TimeUntilNextProcess()); } void SendAndExpectPacket(PacedSender::Priority priority, @@ -141,29 +139,21 @@ 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 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); + // 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); + } + 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); @@ -171,86 +161,79 @@ TEST_F(PacedSenderTest, QueuePacket) { EXPECT_EQ(1, send_bucket_->TimeUntilNextProcess()); clock_.AdvanceTimeMilliseconds(1); EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess()); - EXPECT_CALL( - callback_, - TimeToSendPacket(ssrc, sequence_number++, queued_packet_timestamp, false)) + EXPECT_EQ(1u, send_bucket_->QueueSizePackets()); + EXPECT_CALL(callback_, TimeToSendPacket(ssrc, sequence_number, + queued_packet_timestamp, false)) .Times(1) .WillRepeatedly(Return(true)); send_bucket_->Process(); sequence_number++; - SendAndExpectPacket(PacedSender::kNormalPriority, - ssrc, - sequence_number++, - clock_.TimeInMilliseconds(), - 250, - false); - SendAndExpectPacket(PacedSender::kNormalPriority, - ssrc, - sequence_number++, - clock_.TimeInMilliseconds(), - 250, - false); + 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); + } 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 3 packets not 2 packets. - for (int i = 0; i < 3; ++i) { - SendAndExpectPacket(PacedSender::kNormalPriority, - ssrc, - sequence_number++, - clock_.TimeInMilliseconds(), - 250, - false); + // 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); } - for (int j = 0; j < 30; ++j) { + + for (size_t j = 0; j < packets_to_send_per_interval * 10; ++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(3) + .Times(packets_to_send_per_interval) .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(); - 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); + + for (size_t i = 0; i < packets_to_send_per_interval; ++i) { + 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) { @@ -258,18 +241,18 @@ TEST_F(PacedSenderTest, PaceQueuedPacketsWithDuplicates) { uint16_t sequence_number = 1234; uint16_t queued_sequence_number; - // 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); + // 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); } queued_sequence_number = sequence_number; - for (int j = 0; j < 30; ++j) { + for (size_t j = 0; j < packets_to_send_per_interval * 10; ++j) { // Send in duplicate packets. send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc, sequence_number, clock_.TimeInMilliseconds(), @@ -284,7 +267,7 @@ TEST_F(PacedSenderTest, PaceQueuedPacketsWithDuplicates) { EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess()); clock_.AdvanceTimeMilliseconds(5); - for (int i = 0; i < 3; ++i) { + for (size_t i = 0; i < packets_to_send_per_interval; ++i) { EXPECT_CALL(callback_, TimeToSendPacket(ssrc, queued_sequence_number++, _, false)) .Times(1) @@ -297,28 +280,16 @@ TEST_F(PacedSenderTest, PaceQueuedPacketsWithDuplicates) { clock_.AdvanceTimeMilliseconds(5); EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess()); send_bucket_->Process(); - 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); + + for (size_t i = 0; i < packets_to_send_per_interval; ++i) { + 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) { @@ -348,29 +319,27 @@ TEST_F(PacedSenderTest, Padding) { uint32_t ssrc = 12345; uint16_t sequence_number = 1234; - 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); + 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); + } // No padding is expected since we have sent too much already. EXPECT_CALL(callback_, TimeToSendPadding(_)).Times(0); + 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()); @@ -391,8 +360,9 @@ TEST_F(PacedSenderTest, VerifyPaddingUpToBitrate) { int64_t capture_time_ms = 56789; const int kTimeStep = 5; const int64_t kBitrateWindow = 100; - send_bucket_->UpdateBitrate( - kTargetBitrate, kPaceMultiplier * kTargetBitrate, kTargetBitrate); + send_bucket_->SetEstimatedBitrate(kTargetBitrateBps); + send_bucket_->SetAllocatedSendBitrate(kTargetBitrateBps, kTargetBitrateBps); + int64_t start_time = clock_.TimeInMilliseconds(); while (clock_.TimeInMilliseconds() - start_time < kBitrateWindow) { SendAndExpectPacket(PacedSender::kNormalPriority, @@ -401,10 +371,10 @@ TEST_F(PacedSenderTest, VerifyPaddingUpToBitrate) { capture_time_ms, 250, false); - clock_.AdvanceTimeMilliseconds(kTimeStep); EXPECT_CALL(callback_, TimeToSendPadding(250)).Times(1). WillOnce(Return(250)); send_bucket_->Process(); + clock_.AdvanceTimeMilliseconds(kTimeStep); } } @@ -415,11 +385,11 @@ TEST_F(PacedSenderTest, VerifyAverageBitrateVaryingMediaPayload) { const int kTimeStep = 5; const int64_t kBitrateWindow = 10000; PacedSenderPadding callback; - send_bucket_.reset(new PacedSender( - &clock_, &callback, kTargetBitrate, kPaceMultiplier * kTargetBitrate, 0)); + send_bucket_.reset(new PacedSender(&clock_, &callback)); send_bucket_->SetProbingEnabled(false); - send_bucket_->UpdateBitrate( - kTargetBitrate, kPaceMultiplier * kTargetBitrate, kTargetBitrate); + send_bucket_->SetEstimatedBitrate(kTargetBitrateBps); + send_bucket_->SetAllocatedSendBitrate(kTargetBitrateBps, kTargetBitrateBps); + int64_t start_time = clock_.TimeInMilliseconds(); size_t media_bytes = 0; while (clock_.TimeInMilliseconds() - start_time < kBitrateWindow) { @@ -432,9 +402,10 @@ TEST_F(PacedSenderTest, VerifyAverageBitrateVaryingMediaPayload) { clock_.AdvanceTimeMilliseconds(kTimeStep); send_bucket_->Process(); } - EXPECT_NEAR(kTargetBitrate, + EXPECT_NEAR(kTargetBitrateBps / 1000, static_cast(8 * (media_bytes + callback.padding_sent()) / - kBitrateWindow), 1); + kBitrateWindow), + 1); } TEST_F(PacedSenderTest, Priority) { @@ -444,50 +415,41 @@ 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 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); + // 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_->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); - 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); + + 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::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(4) + .Times(packets_to_send_per_interval + 1) .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( @@ -513,23 +475,30 @@ TEST_F(PacedSenderTest, HighPrioDoesntAffectBudget) { capture_time_ms, 250, false); } send_bucket_->Process(); - // 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) { + // 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) { SendAndExpectPacket(PacedSender::kLowPriority, ssrc, sequence_number++, - capture_time_ms, 250, false); + clock_.TimeInMilliseconds(), 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); + .Times(1) + .WillRepeatedly(Return(true)); EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess()); clock_.AdvanceTimeMilliseconds(5); send_bucket_->Process(); + EXPECT_EQ(0u, send_bucket_->QueueSizePackets()); } TEST_F(PacedSenderTest, Pause) { @@ -540,25 +509,16 @@ TEST_F(PacedSenderTest, Pause) { EXPECT_EQ(0, send_bucket_->QueueInMs()); - // 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); + // 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_->Process(); send_bucket_->Pause(); @@ -668,18 +628,18 @@ TEST_F(PacedSenderTest, ExpectedQueueTimeMs) { uint16_t sequence_number = 1234; const size_t kNumPackets = 60; const size_t kPacketSize = 1200; - const int32_t kMaxBitrate = kPaceMultiplier * 30; + const int32_t kMaxBitrate = PacedSender::kDefaultPaceMultiplier * 30000; EXPECT_EQ(0, send_bucket_->ExpectedQueueTimeMs()); - send_bucket_->UpdateBitrate(30, kMaxBitrate, 0); + send_bucket_->SetEstimatedBitrate(30000); 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) / (kbit per second * 1000 / 8) + // Queue in ms = 1000 * (bytes in queue) *8 / (bits per second) int64_t queue_in_ms = - static_cast(kNumPackets * kPacketSize * 8 / kMaxBitrate); + static_cast(1000 * kNumPackets * kPacketSize * 8 / kMaxBitrate); EXPECT_EQ(queue_in_ms, send_bucket_->ExpectedQueueTimeMs()); int64_t time_start = clock_.TimeInMilliseconds(); @@ -697,7 +657,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(kPacketSize * 8 / kMaxBitrate)); + static_cast(1000 * kPacketSize * 8 / kMaxBitrate)); } TEST_F(PacedSenderTest, QueueTimeGrowsOverTime) { @@ -705,7 +665,7 @@ TEST_F(PacedSenderTest, QueueTimeGrowsOverTime) { uint16_t sequence_number = 1234; EXPECT_EQ(0, send_bucket_->QueueInMs()); - send_bucket_->UpdateBitrate(30, kPaceMultiplier * 30, 0); + send_bucket_->SetEstimatedBitrate(30000); SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number, @@ -723,25 +683,23 @@ TEST_F(PacedSenderTest, ProbingWithInitialFrame) { const int kNumPackets = 11; const int kNumDeltas = kNumPackets - 1; const size_t kPacketSize = 1200; - const int kInitialBitrateKbps = 300; + const int kInitialBitrateBps = 300000; 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, - kInitialBitrateKbps, - kPaceMultiplier * kInitialBitrateKbps, - 0)); + send_bucket_.reset(new PacedSender(&clock_, &callback)); + send_bucket_->SetEstimatedBitrate(kInitialBitrateBps); 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) { @@ -756,15 +714,15 @@ TEST_F(PacedSenderTest, ProbingWithTooSmallInitialFrame) { const int kNumPackets = 11; const int kNumDeltas = kNumPackets - 1; const size_t kPacketSize = 1200; - const int kInitialBitrateKbps = 300; + const int kInitialBitrateBps = 300000; 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, kInitialBitrateKbps, - kPaceMultiplier * kInitialBitrateKbps, 0)); + send_bucket_.reset(new PacedSender(&clock_, &callback)); + send_bucket_->SetEstimatedBitrate(kInitialBitrateBps); for (int i = 0; i < kNumPackets - 5; ++i) { send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc, @@ -839,21 +797,22 @@ TEST_F(PacedSenderTest, PaddingOveruse) { uint16_t sequence_number = 1234; const size_t kPacketSize = 1200; - // Min bitrate 0 => no padding, padding budget will stay at 0. - send_bucket_->UpdateBitrate(60, 90, 0); + send_bucket_->Process(); + send_bucket_->SetEstimatedBitrate(60000); + send_bucket_->SetAllocatedSendBitrate(60000, 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 form 0. + // negative (overuse) while padding budget will increase from 0. clock_.AdvanceTimeMilliseconds(5); - send_bucket_->UpdateBitrate(60, 90, 30); - - send_bucket_->InsertPacket(PacedSender::kHighPriority, ssrc, - sequence_number++, clock_.TimeInMilliseconds(), - kPacketSize, false); + send_bucket_->SetAllocatedSendBitrate(60000, 30000); + 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(); @@ -864,9 +823,8 @@ 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_->UpdateBitrate(kBitrateKbps, kBitrateKbps, kBitrateKbps); + send_bucket_->SetEstimatedBitrate(kBitrateBps); EXPECT_EQ(0, send_bucket_->AverageQueueTimeMs()); diff --git a/webrtc/modules/pacing/packet_router.h b/webrtc/modules/pacing/packet_router.h index 635b931225..a6039fddd8 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::Callback, +class PacketRouter : public PacedSender::PacketSender, 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 32663d729b..7b20cf7f9d 100644 --- a/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator.gypi +++ b/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator.gypi @@ -62,6 +62,7 @@ '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 e6a05684cc..51e29e34c2 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h +++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.h @@ -14,7 +14,6 @@ #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 3bcbc0a071..0bcec82482 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/packet_sender.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/packet_sender.cc @@ -157,13 +157,9 @@ PacedVideoSender::PacedVideoSender(PacketProcessorListener* listener, VideoSource* source, BandwidthEstimatorType estimator) : VideoSender(listener, source, estimator), - pacer_(&clock_, - this, - source->bits_per_second() / 1000, - PacedSender::kDefaultPaceMultiplier * source->bits_per_second() / - 1000, - 0) { + pacer_(&clock_, this) { modules_.push_back(&pacer_); + pacer_.SetEstimatedBitrate(source->bits_per_second()); } PacedVideoSender::~PacedVideoSender() { @@ -310,9 +306,7 @@ void PacedVideoSender::OnNetworkChanged(uint32_t target_bitrate_bps, uint8_t fraction_lost, int64_t rtt) { VideoSender::OnNetworkChanged(target_bitrate_bps, fraction_lost, rtt); - pacer_.UpdateBitrate( - target_bitrate_bps / 1000, - PacedSender::kDefaultPaceMultiplier * target_bitrate_bps / 1000, 0); + pacer_.SetEstimatedBitrate(target_bitrate_bps); } 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 5ed4a3bc38..1280138461 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::Callback { +class PacedVideoSender : public VideoSender, public PacedSender::PacketSender { 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 3bc45f1843..7e6ec71d6e 100644 --- a/webrtc/video/encoder_state_feedback_unittest.cc +++ b/webrtc/video/encoder_state_feedback_unittest.cc @@ -12,8 +12,6 @@ #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" @@ -23,13 +21,8 @@ namespace webrtc { class MockVieEncoder : public ViEEncoder { public: - explicit MockVieEncoder(ProcessThread* process_thread, PacedSender* pacer) - : ViEEncoder(1, - process_thread, - nullptr, - nullptr, - nullptr, - pacer) {} + explicit MockVieEncoder(ProcessThread* process_thread) + : ViEEncoder(1, process_thread, nullptr, nullptr) {} ~MockVieEncoder() {} MOCK_METHOD1(OnReceivedIntraFrameRequest, void(size_t)); @@ -40,13 +33,7 @@ class MockVieEncoder : public ViEEncoder { class VieKeyRequestTest : public ::testing::Test { public: VieKeyRequestTest() - : pacer_(Clock::GetRealTimeClock(), - &router_, - BitrateController::kDefaultStartBitrateKbps, - PacedSender::kDefaultPaceMultiplier * - BitrateController::kDefaultStartBitrateKbps, - 0), - encoder_(&process_thread_, &pacer_), + : encoder_(&process_thread_), simulated_clock_(123456789), encoder_state_feedback_( &simulated_clock_, @@ -56,8 +43,6 @@ 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 84eb836243..840991b4ba 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -379,9 +379,7 @@ VideoSendStream::VideoSendStream( vie_encoder_(num_cpu_cores, module_process_thread_, &stats_proxy_, - config.pre_encode_callback, - &overuse_detector_, - congestion_controller_->pacer()), + &overuse_detector_), encoder_feedback_(Clock::GetRealTimeClock(), config.rtp.ssrcs, &vie_encoder_), @@ -590,8 +588,14 @@ void VideoSendStream::EncoderProcess() { } VideoFrame frame; - if (input_.GetVideoFrame(&frame)) + if (input_.GetVideoFrame(&frame)) { + // TODO(perkj): |pre_encode_callback| is only used by tests. Tests should + // register as a sink to the VideoSource instead. + if (config_.pre_encode_callback) { + config_.pre_encode_callback->OnFrame(frame); + } vie_encoder_.EncodeVideoFrame(frame); + } } vie_encoder_.DeRegisterExternalEncoder(config_.encoder_settings.payload_type); } diff --git a/webrtc/video/vie_encoder.cc b/webrtc/video/vie_encoder.cc index b507372379..e2ac52126e 100644 --- a/webrtc/video/vie_encoder.cc +++ b/webrtc/video/vie_encoder.cc @@ -48,9 +48,7 @@ class QMVideoSettingsCallback : public VCMQMSettingsCallback { ViEEncoder::ViEEncoder(uint32_t number_of_cores, ProcessThread* module_process_thread, SendStatisticsProxy* stats_proxy, - rtc::VideoSinkInterface* pre_encode_callback, - OveruseFrameDetector* overuse_detector, - PacedSender* pacer) + OveruseFrameDetector* overuse_detector) : number_of_cores_(number_of_cores), vp_(VideoProcessing::Create()), qm_callback_(new QMVideoSettingsCallback(vp_.get())), @@ -60,9 +58,7 @@ 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), @@ -229,8 +225,7 @@ bool ViEEncoder::EncoderPaused() const { if (encoder_paused_) { return true; } - if (pacer_->ExpectedQueueTimeMs() > PacedSender::kMaxQueueLengthMs) { - // Too much data in pacer queue, drop frame. + if (video_suspended_ || last_observed_bitrate_bps_ == 0) { return true; } return !network_is_transmitting_; @@ -279,10 +274,6 @@ 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; @@ -377,7 +368,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 5db5e3f6ba..e309dd5047 100644 --- a/webrtc/video/vie_encoder.h +++ b/webrtc/video/vie_encoder.h @@ -59,10 +59,7 @@ class ViEEncoder : public VideoEncoderRateObserver, ViEEncoder(uint32_t number_of_cores, ProcessThread* module_process_thread, SendStatisticsProxy* stats_proxy, - // TODO(nisse): Used only for tests, delete? - rtc::VideoSinkInterface* pre_encode_callback, - OveruseFrameDetector* overuse_detector, - PacedSender* pacer); + OveruseFrameDetector* overuse_detector); ~ViEEncoder(); vcm::VideoSender* video_sender(); @@ -133,9 +130,7 @@ 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