From 2611164688750d3b5bde0a1b24dba70b34d1ae98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Tue, 26 Mar 2019 11:09:04 +0100 Subject: [PATCH] Subtract protection bitrate from link headroom bitrate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In case protection bitrate (fec/nack) is used, don't allow encoder (rate adjuster) to utilize those bits as that may result in pacer delay. Bug: webrtc:10155 Change-Id: I5bceb100396f0ae2131db51039e8524ca068d13f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/128873 Reviewed-by: Ilya Nikolaevskiy Commit-Queue: Erik Språng Cr-Commit-Position: refs/heads/master@{#27283} --- video/video_send_stream_impl.cc | 10 +++++--- video/video_send_stream_impl_unittest.cc | 32 ++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/video/video_send_stream_impl.cc b/video/video_send_stream_impl.cc index b2a69ef04c..ae905df95a 100644 --- a/video/video_send_stream_impl.cc +++ b/video/video_send_stream_impl.cc @@ -650,10 +650,14 @@ uint32_t VideoSendStreamImpl::OnBitrateUpdated(BitrateAllocationUpdate update) { rtc::dchecked_cast(update.packet_loss_ratio * 256), update.round_trip_time.ms(), stats_proxy_->GetSendFrameRate()); encoder_target_rate_bps_ = rtp_video_sender_->GetPayloadBitrateBps(); + const uint32_t protection_bitrate_bps = + rtp_video_sender_->GetProtectionBitrateBps(); DataRate headroom = DataRate::Zero(); - if (encoder_target_rate_bps_ > encoder_max_bitrate_bps_) { + if (encoder_target_rate_bps_ > + encoder_max_bitrate_bps_ + protection_bitrate_bps) { headroom = - DataRate::bps(encoder_target_rate_bps_ - encoder_max_bitrate_bps_); + DataRate::bps(encoder_target_rate_bps_ - + (encoder_max_bitrate_bps_ + protection_bitrate_bps)); } encoder_target_rate_bps_ = std::min(encoder_max_bitrate_bps_, encoder_target_rate_bps_); @@ -662,7 +666,7 @@ uint32_t VideoSendStreamImpl::OnBitrateUpdated(BitrateAllocationUpdate update) { rtc::dchecked_cast(update.packet_loss_ratio * 256), update.round_trip_time.ms()); stats_proxy_->OnSetEncoderTargetRate(encoder_target_rate_bps_); - return rtp_video_sender_->GetProtectionBitrateBps(); + return protection_bitrate_bps; } } // namespace internal diff --git a/video/video_send_stream_impl_unittest.cc b/video/video_send_stream_impl_unittest.cc index d874c4d1fc..e8eab842b6 100644 --- a/video/video_send_stream_impl_unittest.cc +++ b/video/video_send_stream_impl_unittest.cc @@ -678,6 +678,38 @@ TEST_F(VideoSendStreamImplTest, CallsVideoStreamEncoderOnBitrateUpdate) { static_cast(vss_impl.get()) ->OnBitrateUpdated(update); + // Add protection bitrate to the mix, this should be subtracted from the + // headroom. + const uint32_t protection_bitrate_bps = 10000; + 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_, GetPayloadBitrateBps()) + .WillOnce(Return(rate_with_headroom.bps())); + const DataRate headroom_minus_protection = + headroom - DataRate::bps(protection_bitrate_bps); + EXPECT_CALL( + video_stream_encoder_, + OnBitrateUpdated(qvga_max_bitrate, headroom_minus_protection, 0, _)); + static_cast(vss_impl.get()) + ->OnBitrateUpdated(update); + + // Protection bitrate exceeds headroom, make sure it is capped to 0. + 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_, GetPayloadBitrateBps()) + .WillOnce(Return(rate_with_headroom.bps())); + EXPECT_CALL(video_stream_encoder_, + OnBitrateUpdated(qvga_max_bitrate, DataRate::Zero(), 0, _)); + static_cast(vss_impl.get()) + ->OnBitrateUpdated(update); + // Set rates to zero on stop. EXPECT_CALL(video_stream_encoder_, OnBitrateUpdated(DataRate::Zero(), DataRate::Zero(), 0, 0));