From c0e4d45ce0468487b20bed908484d3ab6b16bbbd Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Thu, 25 Oct 2018 15:08:32 +0200 Subject: [PATCH] Adds BitrateAllocation struct to OnBitrateUpdated. This prepares for adding parameters to OnBitrateUpdated. By using a struct, additional fields doesn't require a change in the signature and only the obeservers that use the new fields will be affected by the change. Bug: webrtc:9718 Change-Id: I7dd6c9577afd77af06da5f56aea312356f80f9c0 Reviewed-on: https://webrtc-review.googlesource.com/c/107727 Commit-Queue: Sebastian Jansson Reviewed-by: Karl Wiberg Cr-Commit-Position: refs/heads/master@{#25366} --- audio/audio_send_stream.cc | 18 ++++++++---------- audio/audio_send_stream.h | 5 +---- audio/audio_send_stream_unittest.cc | 15 ++++++++++++--- call/bitrate_allocator.cc | 11 ++++++----- call/bitrate_allocator.h | 11 +++++++---- call/bitrate_allocator_unittest.cc | 15 ++++++--------- video/video_send_stream_impl.cc | 10 ++++------ video/video_send_stream_impl.h | 5 +---- video/video_send_stream_impl_unittest.cc | 18 +++++++++++++----- 9 files changed, 58 insertions(+), 50 deletions(-) diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc index 052cd73ec9..c8af43ea4e 100644 --- a/audio/audio_send_stream.cc +++ b/audio/audio_send_stream.cc @@ -424,24 +424,22 @@ bool AudioSendStream::DeliverRtcp(const uint8_t* packet, size_t length) { return channel_proxy_->ReceivedRTCPPacket(packet, length); } -uint32_t AudioSendStream::OnBitrateUpdated(uint32_t bitrate_bps, - uint8_t fraction_loss, - int64_t rtt, - int64_t bwe_period_ms) { +uint32_t AudioSendStream::OnBitrateUpdated(BitrateAllocationUpdate update) { // A send stream may be allocated a bitrate of zero if the allocator decides // to disable it. For now we ignore this decision and keep sending on min // bitrate. - if (bitrate_bps == 0) { - bitrate_bps = config_.min_bitrate_bps; + if (update.bitrate_bps == 0) { + update.bitrate_bps = config_.min_bitrate_bps; } - RTC_DCHECK_GE(bitrate_bps, static_cast(config_.min_bitrate_bps)); + RTC_DCHECK_GE(update.bitrate_bps, + static_cast(config_.min_bitrate_bps)); // The bitrate allocator might allocate an higher than max configured bitrate // if there is room, to allow for, as example, extra FEC. Ignore that for now. const uint32_t max_bitrate_bps = config_.max_bitrate_bps; - if (bitrate_bps > max_bitrate_bps) - bitrate_bps = max_bitrate_bps; + if (update.bitrate_bps > max_bitrate_bps) + update.bitrate_bps = max_bitrate_bps; - channel_proxy_->SetBitrate(bitrate_bps, bwe_period_ms); + channel_proxy_->SetBitrate(update.bitrate_bps, update.bwe_period_ms); // The amount of audio protection is not exposed by the encoder, hence // always returning 0. diff --git a/audio/audio_send_stream.h b/audio/audio_send_stream.h index 46c68b3c75..3683c9154e 100644 --- a/audio/audio_send_stream.h +++ b/audio/audio_send_stream.h @@ -83,10 +83,7 @@ class AudioSendStream final : public webrtc::AudioSendStream, bool DeliverRtcp(const uint8_t* packet, size_t length); // Implements BitrateAllocatorObserver. - uint32_t OnBitrateUpdated(uint32_t bitrate_bps, - uint8_t fraction_loss, - int64_t rtt, - int64_t bwe_period_ms) override; + uint32_t OnBitrateUpdated(BitrateAllocationUpdate update) override; // From PacketFeedbackObserver. void OnPacketAdded(uint32_t ssrc, uint16_t seq_num) override; diff --git a/audio/audio_send_stream_unittest.cc b/audio/audio_send_stream_unittest.cc index 0a954f86d6..30d2c54f95 100644 --- a/audio/audio_send_stream_unittest.cc +++ b/audio/audio_send_stream_unittest.cc @@ -470,15 +470,24 @@ TEST(AudioSendStreamTest, DoesNotPassHigherBitrateThanMaxBitrate) { auto send_stream = helper.CreateAudioSendStream(); EXPECT_CALL(*helper.channel_proxy(), SetBitrate(helper.config().max_bitrate_bps, _)); - send_stream->OnBitrateUpdated(helper.config().max_bitrate_bps + 5000, 0.0, 50, - 6000); + BitrateAllocationUpdate update; + update.bitrate_bps = helper.config().max_bitrate_bps + 5000; + update.fraction_loss = 0; + update.rtt = 50; + update.bwe_period_ms = 6000; + send_stream->OnBitrateUpdated(update); } TEST(AudioSendStreamTest, ProbingIntervalOnBitrateUpdated) { ConfigHelper helper(false, true); auto send_stream = helper.CreateAudioSendStream(); EXPECT_CALL(*helper.channel_proxy(), SetBitrate(_, 5000)); - send_stream->OnBitrateUpdated(50000, 0.0, 50, 5000); + BitrateAllocationUpdate update; + update.bitrate_bps = helper.config().max_bitrate_bps + 5000; + update.fraction_loss = 0; + update.rtt = 50; + update.bwe_period_ms = 5000; + send_stream->OnBitrateUpdated(update); } // Test that AudioSendStream doesn't recreate the encoder unnecessarily. diff --git a/call/bitrate_allocator.cc b/call/bitrate_allocator.cc index fc2618baa1..64d1a4861e 100644 --- a/call/bitrate_allocator.cc +++ b/call/bitrate_allocator.cc @@ -111,7 +111,8 @@ void BitrateAllocator::OnNetworkChanged(uint32_t target_bitrate_bps, for (auto& config : bitrate_observer_configs_) { uint32_t allocated_bitrate = allocation[config.observer]; uint32_t protection_bitrate = config.observer->OnBitrateUpdated( - allocated_bitrate, last_fraction_loss_, last_rtt_, last_bwe_period_ms_); + BitrateAllocationUpdate{allocated_bitrate, last_fraction_loss_, + last_rtt_, last_bwe_period_ms_}); if (allocated_bitrate == 0 && config.allocated_bitrate_bps > 0) { if (target_bitrate_bps > 0) @@ -170,8 +171,8 @@ void BitrateAllocator::AddObserver(BitrateAllocatorObserver* observer, for (auto& config : bitrate_observer_configs_) { uint32_t allocated_bitrate = allocation[config.observer]; uint32_t protection_bitrate = config.observer->OnBitrateUpdated( - allocated_bitrate, last_fraction_loss_, last_rtt_, - last_bwe_period_ms_); + BitrateAllocationUpdate{allocated_bitrate, last_fraction_loss_, + last_rtt_, last_bwe_period_ms_}); config.allocated_bitrate_bps = allocated_bitrate; if (allocated_bitrate > 0) config.media_ratio = MediaRatio(allocated_bitrate, protection_bitrate); @@ -181,8 +182,8 @@ void BitrateAllocator::AddObserver(BitrateAllocatorObserver* observer, // But we still have to return the initial config bitrate + let the // observer know that it can not produce frames. allocation = AllocateBitrates(last_non_zero_bitrate_bps_); - observer->OnBitrateUpdated(0, last_fraction_loss_, last_rtt_, - last_bwe_period_ms_); + observer->OnBitrateUpdated(BitrateAllocationUpdate{ + 0, last_fraction_loss_, last_rtt_, last_bwe_period_ms_}); } UpdateAllocationLimits(); } diff --git a/call/bitrate_allocator.h b/call/bitrate_allocator.h index 8d0008631f..22fad7cc83 100644 --- a/call/bitrate_allocator.h +++ b/call/bitrate_allocator.h @@ -26,6 +26,12 @@ namespace webrtc { class Clock; +struct BitrateAllocationUpdate { + uint32_t bitrate_bps; + uint8_t fraction_loss; + int64_t rtt; + int64_t bwe_period_ms; +}; // Used by all send streams with adaptive bitrate, to get the currently // allocated bitrate for the send stream. The current network properties are // given at the same time, to let the send stream decide about possible loss @@ -34,10 +40,7 @@ class BitrateAllocatorObserver { public: // Returns the amount of protection used by the BitrateAllocatorObserver // implementation, as bitrate in bps. - virtual uint32_t OnBitrateUpdated(uint32_t bitrate_bps, - uint8_t fraction_loss, - int64_t rtt, - int64_t bwe_period_ms) = 0; + virtual uint32_t OnBitrateUpdated(BitrateAllocationUpdate update) = 0; protected: virtual ~BitrateAllocatorObserver() {} diff --git a/call/bitrate_allocator_unittest.cc b/call/bitrate_allocator_unittest.cc index 2961fd409c..1fc6cd6174 100644 --- a/call/bitrate_allocator_unittest.cc +++ b/call/bitrate_allocator_unittest.cc @@ -59,15 +59,12 @@ class TestBitrateObserver : public BitrateAllocatorObserver { protection_ratio_ = protection_ratio; } - uint32_t OnBitrateUpdated(uint32_t bitrate_bps, - uint8_t fraction_loss, - int64_t rtt, - int64_t probing_interval_ms) override { - last_bitrate_bps_ = bitrate_bps; - last_fraction_loss_ = fraction_loss; - last_rtt_ms_ = rtt; - last_probing_interval_ms_ = probing_interval_ms; - return bitrate_bps * protection_ratio_; + uint32_t OnBitrateUpdated(BitrateAllocationUpdate update) override { + last_bitrate_bps_ = update.bitrate_bps; + last_fraction_loss_ = update.fraction_loss; + last_rtt_ms_ = update.rtt; + last_probing_interval_ms_ = update.bwe_period_ms; + return update.bitrate_bps * protection_ratio_; } uint32_t last_bitrate_bps_; uint8_t last_fraction_loss_; diff --git a/video/video_send_stream_impl.cc b/video/video_send_stream_impl.cc index 84545978b0..e6d97e98bb 100644 --- a/video/video_send_stream_impl.cc +++ b/video/video_send_stream_impl.cc @@ -611,21 +611,19 @@ std::map VideoSendStreamImpl::GetRtpPayloadStates() return rtp_video_sender_->GetRtpPayloadStates(); } -uint32_t VideoSendStreamImpl::OnBitrateUpdated(uint32_t bitrate_bps, - uint8_t fraction_loss, - int64_t rtt, - int64_t probing_interval_ms) { +uint32_t VideoSendStreamImpl::OnBitrateUpdated(BitrateAllocationUpdate update) { RTC_DCHECK_RUN_ON(worker_queue_); RTC_DCHECK(rtp_video_sender_->IsActive()) << "VideoSendStream::Start has not been called."; - rtp_video_sender_->OnBitrateUpdated(bitrate_bps, fraction_loss, rtt, + rtp_video_sender_->OnBitrateUpdated(update.bitrate_bps, update.fraction_loss, + update.rtt, stats_proxy_->GetSendFrameRate()); encoder_target_rate_bps_ = rtp_video_sender_->GetPayloadBitrateBps(); encoder_target_rate_bps_ = std::min(encoder_max_bitrate_bps_, encoder_target_rate_bps_); video_stream_encoder_->OnBitrateUpdated(encoder_target_rate_bps_, - fraction_loss, rtt); + update.fraction_loss, update.rtt); stats_proxy_->OnSetEncoderTargetRate(encoder_target_rate_bps_); return rtp_video_sender_->GetProtectionBitrateBps(); } diff --git a/video/video_send_stream_impl.h b/video/video_send_stream_impl.h index eed9f567fb..6197a0498c 100644 --- a/video/video_send_stream_impl.h +++ b/video/video_send_stream_impl.h @@ -83,10 +83,7 @@ class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver, class CheckEncoderActivityTask; // Implements BitrateAllocatorObserver. - uint32_t OnBitrateUpdated(uint32_t bitrate_bps, - uint8_t fraction_loss, - int64_t rtt, - int64_t probing_interval_ms) override; + uint32_t OnBitrateUpdated(BitrateAllocationUpdate update) override; void OnEncoderConfigurationChanged(std::vector streams, int min_transmit_bitrate_bps) override; diff --git a/video/video_send_stream_impl_unittest.cc b/video/video_send_stream_impl_unittest.cc index 7d94c40383..c93f8ff6a2 100644 --- a/video/video_send_stream_impl_unittest.cc +++ b/video/video_send_stream_impl_unittest.cc @@ -69,6 +69,14 @@ class MockRtpVideoSender : public RtpVideoSenderInterface { MOCK_CONST_METHOD0(GetProtectionBitrateBps, uint32_t()); MOCK_METHOD3(SetEncodingData, void(size_t, size_t, size_t)); }; + +BitrateAllocationUpdate CreateAllocation(int bitrate_bps) { + BitrateAllocationUpdate update; + update.bitrate_bps = bitrate_bps; + update.fraction_loss = 0; + update.rtt = 0; + return update; +} } // namespace class VideoSendStreamImplTest : public ::testing::Test { @@ -367,7 +375,7 @@ TEST_F(VideoSendStreamImplTest, ForwardsVideoBitrateAllocationWhenEnabled) { .Times(1) .WillOnce(Return(kBitrateBps)); static_cast(vss_impl.get()) - ->OnBitrateUpdated(kBitrateBps, 0, 0, 0); + ->OnBitrateUpdated(CreateAllocation(kBitrateBps)); EXPECT_CALL(rtp_video_sender_, OnBitrateAllocationUpdated(alloc)).Times(1); observer->OnBitrateAllocationUpdated(alloc); @@ -376,7 +384,7 @@ TEST_F(VideoSendStreamImplTest, ForwardsVideoBitrateAllocationWhenEnabled) { .Times(1) .WillOnce(Return(0)); static_cast(vss_impl.get()) - ->OnBitrateUpdated(0, 0, 0, 0); + ->OnBitrateUpdated(CreateAllocation(0)); EXPECT_CALL(rtp_video_sender_, OnBitrateAllocationUpdated(alloc)).Times(0); observer->OnBitrateAllocationUpdated(alloc); @@ -396,7 +404,7 @@ TEST_F(VideoSendStreamImplTest, ThrottlesVideoBitrateAllocationWhenTooSimilar) { .Times(1) .WillOnce(Return(kBitrateBps)); static_cast(vss_impl.get()) - ->OnBitrateUpdated(kBitrateBps, 0, 0, 0); + ->OnBitrateUpdated(CreateAllocation(kBitrateBps)); VideoBitrateAllocationObserver* const observer = static_cast(vss_impl.get()); @@ -450,7 +458,7 @@ TEST_F(VideoSendStreamImplTest, ForwardsVideoBitrateAllocationOnLayerChange) { .Times(1) .WillOnce(Return(kBitrateBps)); static_cast(vss_impl.get()) - ->OnBitrateUpdated(kBitrateBps, 0, 0, 0); + ->OnBitrateUpdated(CreateAllocation(kBitrateBps)); VideoBitrateAllocationObserver* const observer = static_cast(vss_impl.get()); @@ -494,7 +502,7 @@ TEST_F(VideoSendStreamImplTest, ForwardsVideoBitrateAllocationAfterTimeout) { .Times(1) .WillRepeatedly(Return(kBitrateBps)); static_cast(vss_impl.get()) - ->OnBitrateUpdated(kBitrateBps, 0, 0, 0); + ->OnBitrateUpdated(CreateAllocation(kBitrateBps)); VideoBitrateAllocationObserver* const observer = static_cast(vss_impl.get());