From d15687d61234fd7e9e470903bc912f8f884af231 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Fri, 18 Jan 2019 10:47:07 +0100 Subject: [PATCH] Don't include packetization overhead in protection bitrate. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If we do, the bitrate allocator will assume there can be a lot a FEC and other things and bumps the max probing bitrate by 2x. This caused a bunch of perf tests to change in a non-obvious way. This is a follow-up to https://webrtc-review.googlesource.com/c/src/+/115410 Bug: webrtc:10155, chromium:922396 Change-Id: I51d3611cb21d98a8fab1bfab2d8f167ed859696d Reviewed-on: https://webrtc-review.googlesource.com/c/118043 Reviewed-by: Niels Moller Commit-Queue: Erik Språng Cr-Commit-Position: refs/heads/master@{#26319} --- call/rtp_video_sender.cc | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc index 6d13821e79..dbc8831e34 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc @@ -636,16 +636,15 @@ void RtpVideoSender::OnBitrateUpdated(uint32_t bitrate_bps, encoder_target_rate_bps_ = fec_controller_->UpdateFecRates( payload_bitrate_bps, framerate, fraction_loss, loss_mask_vector_, rtt); + uint32_t packetization_rate_bps = 0; if (account_for_packetization_overhead_) { - // Subtract packetization overhead from the encoder target. If rate is - // really low, cap the overhead at 50%. Since packetization is measured over - // an averaging window, it might intermittently be higher than encoder - // target (eg encoder pause event), so cap it to target. - const uint32_t packetization_rate_bps = - std::min(GetPacketizationOverheadRate(), encoder_target_rate_bps_); - encoder_target_rate_bps_ = - std::max(encoder_target_rate_bps_ - packetization_rate_bps, - encoder_target_rate_bps_ / 2); + // Subtract packetization overhead from the encoder target. If target rate + // is really low, cap the overhead at 50%. This also avoids the case where + // |encoder_target_rate_bps_| is 0 due to encoder pause event while the + // packetization rate is positive since packets are still flowing. + packetization_rate_bps = + std::min(GetPacketizationOverheadRate(), encoder_target_rate_bps_ / 2); + encoder_target_rate_bps_ -= packetization_rate_bps; } loss_mask_vector_.clear(); @@ -664,8 +663,11 @@ void RtpVideoSender::OnBitrateUpdated(uint32_t bitrate_bps, // When the field trial "WebRTC-SendSideBwe-WithOverhead" is enabled // protection_bitrate includes overhead. - protection_bitrate_bps_ = - bitrate_bps - (encoder_target_rate_bps_ + encoder_overhead_rate_bps); + 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; } uint32_t RtpVideoSender::GetPayloadBitrateBps() const {