Add field trial kill switch for packetization overhead subtraction.
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 <sprang@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26173}diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc
index 5da7bb7..9958d36 100644
--- a/call/rtp_video_sender.cc
+++ b/call/rtp_video_sender.cc
@@ -193,6 +193,8 @@
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 @@
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 2eab569..d72ef0d 100644
--- a/call/rtp_video_sender.h
+++ b/call/rtp_video_sender.h
@@ -131,6 +131,7 @@
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 57abb20..c8f5cf3 100644
--- a/video/picture_id_tests.cc
+++ b/video/picture_id_tests.cc
@@ -262,13 +262,15 @@
// 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<int>(
- 0.9 * (kEncoderBitrateBps / encoder_config.number_of_streams));
+ // subsequent rampup.
+ const int encoder_stream_bps =
+ kEncoderBitrateBps /
+ rtc::checked_cast<int>(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<int>(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_;
