From 82ed2e852f7676e8f1eee725682ec28d19c1bf7c Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Tue, 15 Oct 2019 15:58:37 +0200 Subject: [PATCH] Cleanup: Propagating BitrateAllocationUpdate to RtpVideoSender MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:9883 Change-Id: I12d342ecd5eb0cc859123fe31fc759f6f60f7c8b Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/156940 Commit-Queue: Sebastian Jansson Reviewed-by: Erik Språng Cr-Commit-Position: refs/heads/master@{#29492} --- call/BUILD.gn | 1 + call/rtp_video_sender.cc | 27 +++++++++++---------- call/rtp_video_sender.h | 5 +--- call/rtp_video_sender_interface.h | 5 ++-- call/rtp_video_sender_unittest.cc | 19 ++++++++------- video/video_send_stream_impl.cc | 5 +--- video/video_send_stream_impl_unittest.cc | 30 ++++++++++++------------ 7 files changed, 45 insertions(+), 47 deletions(-) diff --git a/call/BUILD.gn b/call/BUILD.gn index 09ef54050d..48a6504906 100644 --- a/call/BUILD.gn +++ b/call/BUILD.gn @@ -134,6 +134,7 @@ rtc_source_set("rtp_sender") { ":bitrate_configurator", ":rtp_interfaces", "../api:array_view", + "../api:bitrate_allocation", "../api:fec_controller_api", "../api:network_state_predictor_api", "../api:rtp_parameters", diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc index d6670728a2..77a13382f7 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc @@ -712,9 +712,7 @@ void RtpVideoSender::OnOverheadChanged(size_t overhead_bytes_per_packet) { overhead_bytes_per_packet_ = overhead_bytes_per_packet; } -void RtpVideoSender::OnBitrateUpdated(uint32_t bitrate_bps, - uint8_t fraction_loss, - int64_t rtt, +void RtpVideoSender::OnBitrateUpdated(BitrateAllocationUpdate update, int framerate) { // Substract overhead from bitrate. rtc::CritScope lock(&crit_); @@ -722,19 +720,22 @@ void RtpVideoSender::OnBitrateUpdated(uint32_t bitrate_bps, overhead_bytes_per_packet_ + transport_overhead_bytes_per_packet_); DataSize max_total_packet_size = DataSize::bytes( rtp_config_.max_packet_size + transport_overhead_bytes_per_packet_); - uint32_t payload_bitrate_bps = bitrate_bps; + uint32_t payload_bitrate_bps = update.target_bitrate.bps(); if (send_side_bwe_with_overhead_) { DataRate overhead_rate = CalculateOverheadRate( - DataRate::bps(bitrate_bps), max_total_packet_size, packet_overhead); + update.target_bitrate, max_total_packet_size, packet_overhead); // TODO(srte): We probably should not accept 0 payload bitrate here. - payload_bitrate_bps = - rtc::saturated_cast(bitrate_bps - overhead_rate.bps()); + payload_bitrate_bps = rtc::saturated_cast(payload_bitrate_bps - + overhead_rate.bps()); } // Get the encoder target rate. It is the estimated network rate - // protection overhead. + // TODO(srte): We should multiply with 255 here. encoder_target_rate_bps_ = fec_controller_->UpdateFecRates( - payload_bitrate_bps, framerate, fraction_loss, loss_mask_vector_, rtt); + payload_bitrate_bps, framerate, + rtc::saturated_cast(update.packet_loss_ratio * 256), + loss_mask_vector_, update.round_trip_time.ms()); if (!fec_allowed_) { encoder_target_rate_bps_ = payload_bitrate_bps; // fec_controller_->UpdateFecRates() was still called so as to allow @@ -765,17 +766,17 @@ void RtpVideoSender::OnBitrateUpdated(uint32_t bitrate_bps, DataRate::bps(encoder_target_rate_bps_), max_total_packet_size - DataSize::bytes(overhead_bytes_per_packet_), packet_overhead); - encoder_overhead_rate_bps = - std::min(encoder_overhead_rate.bps(), - bitrate_bps - encoder_target_rate_bps_); + encoder_overhead_rate_bps = std::min( + encoder_overhead_rate.bps(), + update.target_bitrate.bps() - encoder_target_rate_bps_); } // When the field trial "WebRTC-SendSideBwe-WithOverhead" is enabled // protection_bitrate includes overhead. const uint32_t media_rate = encoder_target_rate_bps_ + encoder_overhead_rate_bps + packetization_rate_bps; - RTC_DCHECK_GE(bitrate_bps, media_rate); - protection_bitrate_bps_ = bitrate_bps - media_rate; + RTC_DCHECK_GE(update.target_bitrate, DataRate::bps(media_rate)); + protection_bitrate_bps_ = update.target_bitrate.bps() - media_rate; } uint32_t RtpVideoSender::GetPayloadBitrateBps() const { diff --git a/call/rtp_video_sender.h b/call/rtp_video_sender.h index b5f8a8f64c..9458f13f84 100644 --- a/call/rtp_video_sender.h +++ b/call/rtp_video_sender.h @@ -136,10 +136,7 @@ class RtpVideoSender : public RtpVideoSenderInterface, size_t transport_overhead_bytes_per_packet) override; // Implements OverheadObserver. void OnOverheadChanged(size_t overhead_bytes_per_packet) override; - void OnBitrateUpdated(uint32_t bitrate_bps, - uint8_t fraction_loss, - int64_t rtt, - int framerate) override; + void OnBitrateUpdated(BitrateAllocationUpdate update, int framerate) override; uint32_t GetPayloadBitrateBps() const override; uint32_t GetProtectionBitrateBps() const override; void SetEncodingData(size_t width, diff --git a/call/rtp_video_sender_interface.h b/call/rtp_video_sender_interface.h index ae9cdaf71c..bb72eb5996 100644 --- a/call/rtp_video_sender_interface.h +++ b/call/rtp_video_sender_interface.h @@ -16,6 +16,7 @@ #include "absl/types/optional.h" #include "api/array_view.h" +#include "api/call/bitrate_allocation.h" #include "api/fec_controller_override.h" #include "call/rtp_config.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" @@ -49,9 +50,7 @@ class RtpVideoSenderInterface : public EncodedImageCallback, virtual void OnBitrateAllocationUpdated( const VideoBitrateAllocation& bitrate) = 0; - virtual void OnBitrateUpdated(uint32_t bitrate_bps, - uint8_t fraction_loss, - int64_t rtt, + virtual void OnBitrateUpdated(BitrateAllocationUpdate update, int framerate) = 0; virtual void OnTransportOverheadChanged( size_t transport_overhead_bytes_per_packet) = 0; diff --git a/call/rtp_video_sender_unittest.cc b/call/rtp_video_sender_unittest.cc index 39d25e4d66..bac60f8f9f 100644 --- a/call/rtp_video_sender_unittest.cc +++ b/call/rtp_video_sender_unittest.cc @@ -629,19 +629,22 @@ TEST(RtpVideoSenderTest, EarlyRetransmits) { TEST(RtpVideoSenderTest, CanSetZeroBitrateWithOverhead) { test::ScopedFieldTrials trials("WebRTC-SendSideBwe-WithOverhead/Enabled/"); RtpVideoSenderTestFixture test({kSsrc1}, {kRtxSsrc1}, kPayloadType, {}); + BitrateAllocationUpdate update; + update.target_bitrate = DataRate::Zero(); + update.packet_loss_ratio = 0; + update.round_trip_time = TimeDelta::Zero(); - test.router()->OnBitrateUpdated(/*bitrate_bps*/ 0, - /*fraction_loss*/ 0, - /*rtt*/ 0, - /*framerate*/ 0); + test.router()->OnBitrateUpdated(update, /*framerate*/ 0); } TEST(RtpVideoSenderTest, CanSetZeroBitrateWithoutOverhead) { RtpVideoSenderTestFixture test({kSsrc1}, {kRtxSsrc1}, kPayloadType, {}); - test.router()->OnBitrateUpdated(/*bitrate_bps*/ 0, - /*fraction_loss*/ 0, - /*rtt*/ 0, - /*framerate*/ 0); + BitrateAllocationUpdate update; + update.target_bitrate = DataRate::Zero(); + update.packet_loss_ratio = 0; + update.round_trip_time = TimeDelta::Zero(); + + test.router()->OnBitrateUpdated(update, /*framerate*/ 0); } } // namespace webrtc diff --git a/video/video_send_stream_impl.cc b/video/video_send_stream_impl.cc index f1c2d3fbc1..26f55fff2e 100644 --- a/video/video_send_stream_impl.cc +++ b/video/video_send_stream_impl.cc @@ -647,10 +647,7 @@ uint32_t VideoSendStreamImpl::OnBitrateUpdated(BitrateAllocationUpdate update) { update.stable_target_bitrate = update.target_bitrate; } - rtp_video_sender_->OnBitrateUpdated( - update.target_bitrate.bps(), - rtc::dchecked_cast(update.packet_loss_ratio * 256), - update.round_trip_time.ms(), stats_proxy_->GetSendFrameRate()); + rtp_video_sender_->OnBitrateUpdated(update, stats_proxy_->GetSendFrameRate()); encoder_target_rate_bps_ = rtp_video_sender_->GetPayloadBitrateBps(); const uint32_t protection_bitrate_bps = rtp_video_sender_->GetProtectionBitrateBps(); diff --git a/video/video_send_stream_impl_unittest.cc b/video/video_send_stream_impl_unittest.cc index fdf0024105..2bbdefb0c6 100644 --- a/video/video_send_stream_impl_unittest.cc +++ b/video/video_send_stream_impl_unittest.cc @@ -31,6 +31,14 @@ #include "video/test/mock_video_stream_encoder.h" namespace webrtc { + +bool operator==(const BitrateAllocationUpdate& a, + const BitrateAllocationUpdate& b) { + return a.target_bitrate == b.target_bitrate && + a.round_trip_time == b.round_trip_time && + a.packet_loss_ratio == b.packet_loss_ratio; +} + namespace internal { namespace { using ::testing::_; @@ -66,7 +74,7 @@ class MockRtpVideoSender : public RtpVideoSenderInterface { const RTPFragmentationHeader*)); MOCK_METHOD1(OnTransportOverheadChanged, void(size_t)); MOCK_METHOD1(OnOverheadChanged, void(size_t)); - MOCK_METHOD4(OnBitrateUpdated, void(uint32_t, uint8_t, int64_t, int)); + MOCK_METHOD2(OnBitrateUpdated, void(BitrateAllocationUpdate, int)); MOCK_CONST_METHOD0(GetPayloadBitrateBps, uint32_t()); MOCK_CONST_METHOD0(GetProtectionBitrateBps, uint32_t()); MOCK_METHOD3(SetEncodingData, void(size_t, size_t, size_t)); @@ -693,9 +701,7 @@ TEST_F(VideoSendStreamImplTest, CallsVideoStreamEncoderOnBitrateUpdate) { update.target_bitrate = network_constrained_rate; update.stable_target_bitrate = network_constrained_rate; update.round_trip_time = TimeDelta::ms(1); - EXPECT_CALL(rtp_video_sender_, - OnBitrateUpdated(network_constrained_rate.bps(), _, - update.round_trip_time.ms(), _)); + EXPECT_CALL(rtp_video_sender_, OnBitrateUpdated(update, _)); EXPECT_CALL(rtp_video_sender_, GetPayloadBitrateBps()) .WillOnce(Return(network_constrained_rate.bps())); EXPECT_CALL( @@ -711,16 +717,14 @@ TEST_F(VideoSendStreamImplTest, CallsVideoStreamEncoderOnBitrateUpdate) { DataRate::bps(qvga_stream.max_bitrate_bps); const DataRate headroom = DataRate::bps(50000); const DataRate rate_with_headroom = qvga_max_bitrate + headroom; - EXPECT_CALL(rtp_video_sender_, - OnBitrateUpdated(rate_with_headroom.bps(), _, - update.round_trip_time.ms(), _)); + update.target_bitrate = rate_with_headroom; + update.stable_target_bitrate = rate_with_headroom; + EXPECT_CALL(rtp_video_sender_, OnBitrateUpdated(update, _)); EXPECT_CALL(rtp_video_sender_, GetPayloadBitrateBps()) .WillOnce(Return(rate_with_headroom.bps())); EXPECT_CALL(video_stream_encoder_, OnBitrateUpdated(qvga_max_bitrate, qvga_max_bitrate, rate_with_headroom, 0, _)); - update.target_bitrate = rate_with_headroom; - update.stable_target_bitrate = rate_with_headroom; static_cast(vss_impl.get()) ->OnBitrateUpdated(update); @@ -730,9 +734,7 @@ TEST_F(VideoSendStreamImplTest, CallsVideoStreamEncoderOnBitrateUpdate) { EXPECT_CALL(rtp_video_sender_, GetProtectionBitrateBps()) .WillOnce(Return(protection_bitrate_bps)); - EXPECT_CALL(rtp_video_sender_, - OnBitrateUpdated(rate_with_headroom.bps(), _, - update.round_trip_time.ms(), _)); + EXPECT_CALL(rtp_video_sender_, OnBitrateUpdated(update, _)); EXPECT_CALL(rtp_video_sender_, GetPayloadBitrateBps()) .WillOnce(Return(rate_with_headroom.bps())); const DataRate headroom_minus_protection = @@ -747,9 +749,7 @@ TEST_F(VideoSendStreamImplTest, CallsVideoStreamEncoderOnBitrateUpdate) { // capped to target bitrate. EXPECT_CALL(rtp_video_sender_, GetProtectionBitrateBps()) .WillOnce(Return(headroom.bps() + 1000)); - EXPECT_CALL(rtp_video_sender_, - OnBitrateUpdated(rate_with_headroom.bps(), _, - update.round_trip_time.ms(), _)); + EXPECT_CALL(rtp_video_sender_, OnBitrateUpdated(update, _)); EXPECT_CALL(rtp_video_sender_, GetPayloadBitrateBps()) .WillOnce(Return(rate_with_headroom.bps())); EXPECT_CALL(video_stream_encoder_,