From c12d41b747bdfda92fddbdb4876db2f484032bb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Wed, 9 Jan 2019 09:55:31 +0100 Subject: [PATCH] Add field trial kill switch for packetization overhead subtraction. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Just in case. Also slightly update picture id test to make it more clear. This is a follow-up to https://webrtc-review.googlesource.com/c/src/+/115410 Bug: webrtc:10155 Change-Id: I9a0239e474b79fe545738860983e1931e8b82eff Reviewed-on: https://webrtc-review.googlesource.com/c/116661 Commit-Queue: Erik Språng Reviewed-by: Niels Moller Cr-Commit-Position: refs/heads/master@{#26173} --- call/rtp_video_sender.cc | 22 +++++++++++++--------- call/rtp_video_sender.h | 1 + video/picture_id_tests.cc | 12 +++++++----- 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc index 5da7bb776b..9958d36e5e 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc @@ -193,6 +193,8 @@ RtpVideoSender::RtpVideoSender( const CryptoOptions& crypto_options) : send_side_bwe_with_overhead_( webrtc::field_trial::IsEnabled("WebRTC-SendSideBwe-WithOverhead")), + account_for_packetization_overhead_(!webrtc::field_trial::IsDisabled( + "WebRTC-SubtractPacketizationOverhead")), active_(false), module_process_thread_(nullptr), suspended_ssrcs_(std::move(suspended_ssrcs)), @@ -634,15 +636,17 @@ void RtpVideoSender::OnBitrateUpdated(uint32_t bitrate_bps, encoder_target_rate_bps_ = fec_controller_->UpdateFecRates( payload_bitrate_bps, framerate, fraction_loss, loss_mask_vector_, rtt); - // 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); + 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); + } loss_mask_vector_.clear(); diff --git a/call/rtp_video_sender.h b/call/rtp_video_sender.h index 2eab569678..d72ef0d8ff 100644 --- a/call/rtp_video_sender.h +++ b/call/rtp_video_sender.h @@ -131,6 +131,7 @@ class RtpVideoSender : public RtpVideoSenderInterface, uint32_t GetPacketizationOverheadRate() const; const bool send_side_bwe_with_overhead_; + const bool account_for_packetization_overhead_; // TODO(holmer): Remove crit_ once RtpVideoSender runs on the // transport task queue. diff --git a/video/picture_id_tests.cc b/video/picture_id_tests.cc index 57abb20874..c8f5cf3482 100644 --- a/video/picture_id_tests.cc +++ b/video/picture_id_tests.cc @@ -262,13 +262,15 @@ class VideoStreamFactory // Always divide the same total bitrate across all streams so that sending a // single stream avoids lowering the bitrate estimate and requiring a - // subsequent rampup. Also reduce the target by 10% to account for overhead - // that might sometimes otherwise cause streams to not be enabled. - const int encoder_stream_bps = rtc::checked_cast( - 0.9 * (kEncoderBitrateBps / encoder_config.number_of_streams)); + // subsequent rampup. + const int encoder_stream_bps = + kEncoderBitrateBps / + rtc::checked_cast(encoder_config.number_of_streams); for (size_t i = 0; i < encoder_config.number_of_streams; ++i) { - streams[i].min_bitrate_bps = encoder_stream_bps; + // Reduce the min bitrate by 10% to account for overhead that might + // otherwise cause streams to not be enabled. + streams[i].min_bitrate_bps = static_cast(encoder_stream_bps * 0.9); streams[i].target_bitrate_bps = encoder_stream_bps; streams[i].max_bitrate_bps = encoder_stream_bps; streams[i].num_temporal_layers = num_of_temporal_layers_;