Reland "Default enable WebRTC-SendSideBwe-WithOverhead."
This is a reland of 87c1950841c3f5e465e1663cc922717ce191e192
Original change's description:
> Default enable WebRTC-SendSideBwe-WithOverhead.
>
> Bug: webrtc:6762
> Change-Id: I18ace06a33b3b60d5a19796d4769f70cd977d604
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/188801
> Commit-Queue: Jakob Ivarsson <jakobi@webrtc.org>
> Reviewed-by: Stefan Holmer <stefan@webrtc.org>
> Reviewed-by: Ali Tofigh <alito@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#32472}
Bug: webrtc:6762
Change-Id: Icf096a8755d29600a13bd08b1f22f5a79de21e90
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/190143
Reviewed-by: Ali Tofigh <alito@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Commit-Queue: Jakob Ivarsson <jakobi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32492}
diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc
index 3fcdbf4..1c0a32f 100644
--- a/audio/audio_send_stream.cc
+++ b/audio/audio_send_stream.cc
@@ -149,7 +149,7 @@
enable_audio_alr_probing_(
!field_trial::IsDisabled("WebRTC-Audio-AlrProbing")),
send_side_bwe_with_overhead_(
- field_trial::IsEnabled("WebRTC-SendSideBwe-WithOverhead")),
+ !field_trial::IsDisabled("WebRTC-SendSideBwe-WithOverhead")),
config_(Config(/*send_transport=*/nullptr)),
audio_state_(audio_state),
channel_send_(std::move(channel_send)),
diff --git a/call/call_perf_tests.cc b/call/call_perf_tests.cc
index ac1d29e..342b64b 100644
--- a/call/call_perf_tests.cc
+++ b/call/call_perf_tests.cc
@@ -732,6 +732,11 @@
static const uint32_t kInitialBitrateKbps = 400;
static const uint32_t kReconfigureThresholdKbps = 600;
+ // We get lower bitrate than expected by this test if the following field
+ // trial is enabled.
+ test::ScopedFieldTrials field_trials(
+ "WebRTC-SendSideBwe-WithOverhead/Disabled/");
+
class VideoStreamFactory
: public VideoEncoderConfig::VideoStreamFactoryInterface {
public:
diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc
index 63bb937..f5adae6 100644
--- a/call/rtp_transport_controller_send.cc
+++ b/call/rtp_transport_controller_send.cc
@@ -64,6 +64,11 @@
return absl::StartsWith(trials->Lookup(key), "Enabled");
}
+bool IsDisabled(const WebRtcKeyValueConfig* trials, absl::string_view key) {
+ RTC_DCHECK(trials != nullptr);
+ return absl::StartsWith(trials->Lookup(key), "Disabled");
+}
+
bool IsRelayed(const rtc::NetworkRoute& route) {
return route.local.uses_turn() || route.remote.uses_turn();
}
@@ -111,7 +116,7 @@
reset_feedback_on_route_change_(
!IsEnabled(trials, "WebRTC-Bwe-NoFeedbackReset")),
send_side_bwe_with_overhead_(
- IsEnabled(trials, "WebRTC-SendSideBwe-WithOverhead")),
+ !IsDisabled(trials, "WebRTC-SendSideBwe-WithOverhead")),
add_pacing_to_cwin_(
IsEnabled(trials, "WebRTC-AddPacingToCongestionWindowPushback")),
relay_bandwidth_cap_("relay_cap", DataRate::PlusInfinity()),
diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc
index fd71248..9dad424 100644
--- a/call/rtp_video_sender.cc
+++ b/call/rtp_video_sender.cc
@@ -327,9 +327,9 @@
FrameEncryptorInterface* frame_encryptor,
const CryptoOptions& crypto_options,
rtc::scoped_refptr<FrameTransformerInterface> frame_transformer)
- : send_side_bwe_with_overhead_(absl::StartsWith(
+ : send_side_bwe_with_overhead_(!absl::StartsWith(
field_trials_.Lookup("WebRTC-SendSideBwe-WithOverhead"),
- "Enabled")),
+ "Disabled")),
has_packet_feedback_(TransportSeqNumExtensionConfigured(rtp_config)),
active_(false),
module_process_thread_(nullptr),
diff --git a/modules/audio_coding/codecs/isac/audio_encoder_isac_t.h b/modules/audio_coding/codecs/isac/audio_encoder_isac_t.h
index d99e9c8..8bde0e3 100644
--- a/modules/audio_coding/codecs/isac/audio_encoder_isac_t.h
+++ b/modules/audio_coding/codecs/isac/audio_encoder_isac_t.h
@@ -93,7 +93,7 @@
// Cache the value of the "WebRTC-SendSideBwe-WithOverhead" field trial.
const bool send_side_bwe_with_overhead_ =
- field_trial::IsEnabled("WebRTC-SendSideBwe-WithOverhead");
+ !field_trial::IsDisabled("WebRTC-SendSideBwe-WithOverhead");
// When we send a packet, expect this many bytes of headers to be added to it.
// Start out with a reasonable default that we can use until we receive a real
diff --git a/modules/audio_coding/codecs/opus/audio_encoder_opus.cc b/modules/audio_coding/codecs/opus/audio_encoder_opus.cc
index 2b16920..8d1a734 100644
--- a/modules/audio_coding/codecs/opus/audio_encoder_opus.cc
+++ b/modules/audio_coding/codecs/opus/audio_encoder_opus.cc
@@ -356,7 +356,7 @@
std::unique_ptr<SmoothingFilter> bitrate_smoother)
: payload_type_(payload_type),
send_side_bwe_with_overhead_(
- webrtc::field_trial::IsEnabled("WebRTC-SendSideBwe-WithOverhead")),
+ !webrtc::field_trial::IsDisabled("WebRTC-SendSideBwe-WithOverhead")),
use_stable_target_for_adaptation_(!webrtc::field_trial::IsDisabled(
"WebRTC-Audio-StableTargetAdaptation")),
adjust_bandwidth_(
diff --git a/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc b/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc
index 1cbc4a3..0fe87bc 100644
--- a/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc
+++ b/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc
@@ -198,22 +198,31 @@
// Constants are replicated from audio_states->encoderopus.cc.
const int kMinBitrateBps = 6000;
const int kMaxBitrateBps = 510000;
+ const int kOverheadBytesPerPacket = 64;
+ states->encoder->OnReceivedOverhead(kOverheadBytesPerPacket);
+ const int kOverheadBps = 8 * kOverheadBytesPerPacket *
+ rtc::CheckedDivExact(48000, kDefaultOpusPacSize);
// Set a too low bitrate.
- states->encoder->OnReceivedUplinkBandwidth(kMinBitrateBps - 1, absl::nullopt);
+ states->encoder->OnReceivedUplinkBandwidth(kMinBitrateBps + kOverheadBps - 1,
+ absl::nullopt);
EXPECT_EQ(kMinBitrateBps, states->encoder->GetTargetBitrate());
// Set a too high bitrate.
- states->encoder->OnReceivedUplinkBandwidth(kMaxBitrateBps + 1, absl::nullopt);
+ states->encoder->OnReceivedUplinkBandwidth(kMaxBitrateBps + kOverheadBps + 1,
+ absl::nullopt);
EXPECT_EQ(kMaxBitrateBps, states->encoder->GetTargetBitrate());
// Set the minimum rate.
- states->encoder->OnReceivedUplinkBandwidth(kMinBitrateBps, absl::nullopt);
+ states->encoder->OnReceivedUplinkBandwidth(kMinBitrateBps + kOverheadBps,
+ absl::nullopt);
EXPECT_EQ(kMinBitrateBps, states->encoder->GetTargetBitrate());
// Set the maximum rate.
- states->encoder->OnReceivedUplinkBandwidth(kMaxBitrateBps, absl::nullopt);
+ states->encoder->OnReceivedUplinkBandwidth(kMaxBitrateBps + kOverheadBps,
+ absl::nullopt);
EXPECT_EQ(kMaxBitrateBps, states->encoder->GetTargetBitrate());
// Set rates from kMaxBitrateBps up to 32000 bps.
- for (int rate = kMinBitrateBps; rate <= 32000; rate += 1000) {
+ for (int rate = kMinBitrateBps + kOverheadBps; rate <= 32000 + kOverheadBps;
+ rate += 1000) {
states->encoder->OnReceivedUplinkBandwidth(rate, absl::nullopt);
- EXPECT_EQ(rate, states->encoder->GetTargetBitrate());
+ EXPECT_EQ(rate - kOverheadBps, states->encoder->GetTargetBitrate());
}
}
@@ -376,53 +385,6 @@
EXPECT_EQ(kDefaultOpusRate, states->encoder->GetTargetBitrate());
}
-TEST_P(AudioEncoderOpusTest, OverheadRemovedFromTargetAudioBitrate) {
- test::ScopedFieldTrials override_field_trials(
- "WebRTC-SendSideBwe-WithOverhead/Enabled/");
-
- auto states = CreateCodec(sample_rate_hz_, 2);
-
- constexpr size_t kOverheadBytesPerPacket = 64;
- states->encoder->OnReceivedOverhead(kOverheadBytesPerPacket);
-
- constexpr int kTargetBitrateBps = 40000;
- states->encoder->OnReceivedUplinkBandwidth(kTargetBitrateBps, absl::nullopt);
-
- int packet_rate = rtc::CheckedDivExact(48000, kDefaultOpusPacSize);
- EXPECT_EQ(kTargetBitrateBps -
- 8 * static_cast<int>(kOverheadBytesPerPacket) * packet_rate,
- states->encoder->GetTargetBitrate());
-}
-
-TEST_P(AudioEncoderOpusTest, BitrateBounded) {
- test::ScopedFieldTrials override_field_trials(
- "WebRTC-SendSideBwe-WithOverhead/Enabled/");
-
- constexpr int kMinBitrateBps = 6000;
- constexpr int kMaxBitrateBps = 510000;
-
- auto states = CreateCodec(sample_rate_hz_, 2);
-
- constexpr size_t kOverheadBytesPerPacket = 64;
- states->encoder->OnReceivedOverhead(kOverheadBytesPerPacket);
-
- int packet_rate = rtc::CheckedDivExact(48000, kDefaultOpusPacSize);
-
- // Set a target rate that is smaller than |kMinBitrateBps| when overhead is
- // subtracted. The eventual codec rate should be bounded by |kMinBitrateBps|.
- int target_bitrate =
- kOverheadBytesPerPacket * 8 * packet_rate + kMinBitrateBps - 1;
- states->encoder->OnReceivedUplinkBandwidth(target_bitrate, absl::nullopt);
- EXPECT_EQ(kMinBitrateBps, states->encoder->GetTargetBitrate());
-
- // Set a target rate that is greater than |kMaxBitrateBps| when overhead is
- // subtracted. The eventual codec rate should be bounded by |kMaxBitrateBps|.
- target_bitrate =
- kOverheadBytesPerPacket * 8 * packet_rate + kMaxBitrateBps + 1;
- states->encoder->OnReceivedUplinkBandwidth(target_bitrate, absl::nullopt);
- EXPECT_EQ(kMaxBitrateBps, states->encoder->GetTargetBitrate());
-}
-
// Verifies that the complexity adaptation in the config works as intended.
TEST(AudioEncoderOpusTest, ConfigComplexityAdaptation) {
AudioEncoderOpusConfig config;
diff --git a/modules/audio_coding/neteq/audio_decoder_unittest.cc b/modules/audio_coding/neteq/audio_decoder_unittest.cc
index d1e1ec1..56708ec 100644
--- a/modules/audio_coding/neteq/audio_decoder_unittest.cc
+++ b/modules/audio_coding/neteq/audio_decoder_unittest.cc
@@ -37,6 +37,9 @@
namespace webrtc {
namespace {
+
+constexpr int kOverheadBytesPerPacket = 50;
+
// The absolute difference between the input and output (the first channel) is
// compared vs |tolerance|. The parameter |delay| is used to correct for codec
// delays.
@@ -356,6 +359,7 @@
config.frame_size_ms =
1000 * static_cast<int>(frame_size_) / codec_input_rate_hz_;
audio_encoder_.reset(new AudioEncoderIsacFloatImpl(config));
+ audio_encoder_->OnReceivedOverhead(kOverheadBytesPerPacket);
AudioDecoderIsacFloatImpl::Config decoder_config;
decoder_config.sample_rate_hz = codec_input_rate_hz_;
@@ -375,6 +379,7 @@
config.frame_size_ms =
1000 * static_cast<int>(frame_size_) / codec_input_rate_hz_;
audio_encoder_.reset(new AudioEncoderIsacFloatImpl(config));
+ audio_encoder_->OnReceivedOverhead(kOverheadBytesPerPacket);
AudioDecoderIsacFloatImpl::Config decoder_config;
decoder_config.sample_rate_hz = codec_input_rate_hz_;
@@ -394,6 +399,7 @@
config.frame_size_ms =
1000 * static_cast<int>(frame_size_) / codec_input_rate_hz_;
audio_encoder_.reset(new AudioEncoderIsacFixImpl(config));
+ audio_encoder_->OnReceivedOverhead(kOverheadBytesPerPacket);
AudioDecoderIsacFixImpl::Config decoder_config;
decoder_config.sample_rate_hz = codec_input_rate_hz_;
@@ -451,6 +457,7 @@
? AudioEncoderOpusConfig::ApplicationMode::kVoip
: AudioEncoderOpusConfig::ApplicationMode::kAudio;
audio_encoder_ = AudioEncoderOpus::MakeAudioEncoder(config, payload_type_);
+ audio_encoder_->OnReceivedOverhead(kOverheadBytesPerPacket);
}
const int opus_sample_rate_hz_{std::get<0>(GetParam())};
const int opus_num_channels_{std::get<1>(GetParam())};
@@ -536,11 +543,18 @@
}
TEST_F(AudioDecoderIsacFloatTest, SetTargetBitrate) {
- EXPECT_EQ(10000, SetAndGetTargetBitrate(audio_encoder_.get(), 9999));
- EXPECT_EQ(10000, SetAndGetTargetBitrate(audio_encoder_.get(), 10000));
- EXPECT_EQ(23456, SetAndGetTargetBitrate(audio_encoder_.get(), 23456));
- EXPECT_EQ(32000, SetAndGetTargetBitrate(audio_encoder_.get(), 32000));
- EXPECT_EQ(32000, SetAndGetTargetBitrate(audio_encoder_.get(), 32001));
+ const int overhead_rate =
+ 8 * kOverheadBytesPerPacket * codec_input_rate_hz_ / frame_size_;
+ EXPECT_EQ(10000,
+ SetAndGetTargetBitrate(audio_encoder_.get(), 9999 + overhead_rate));
+ EXPECT_EQ(10000, SetAndGetTargetBitrate(audio_encoder_.get(),
+ 10000 + overhead_rate));
+ EXPECT_EQ(23456, SetAndGetTargetBitrate(audio_encoder_.get(),
+ 23456 + overhead_rate));
+ EXPECT_EQ(32000, SetAndGetTargetBitrate(audio_encoder_.get(),
+ 32000 + overhead_rate));
+ EXPECT_EQ(32000, SetAndGetTargetBitrate(audio_encoder_.get(),
+ 32001 + overhead_rate));
}
TEST_F(AudioDecoderIsacSwbTest, EncodeDecode) {
@@ -553,11 +567,18 @@
}
TEST_F(AudioDecoderIsacSwbTest, SetTargetBitrate) {
- EXPECT_EQ(10000, SetAndGetTargetBitrate(audio_encoder_.get(), 9999));
- EXPECT_EQ(10000, SetAndGetTargetBitrate(audio_encoder_.get(), 10000));
- EXPECT_EQ(23456, SetAndGetTargetBitrate(audio_encoder_.get(), 23456));
- EXPECT_EQ(56000, SetAndGetTargetBitrate(audio_encoder_.get(), 56000));
- EXPECT_EQ(56000, SetAndGetTargetBitrate(audio_encoder_.get(), 56001));
+ const int overhead_rate =
+ 8 * kOverheadBytesPerPacket * codec_input_rate_hz_ / frame_size_;
+ EXPECT_EQ(10000,
+ SetAndGetTargetBitrate(audio_encoder_.get(), 9999 + overhead_rate));
+ EXPECT_EQ(10000, SetAndGetTargetBitrate(audio_encoder_.get(),
+ 10000 + overhead_rate));
+ EXPECT_EQ(23456, SetAndGetTargetBitrate(audio_encoder_.get(),
+ 23456 + overhead_rate));
+ EXPECT_EQ(56000, SetAndGetTargetBitrate(audio_encoder_.get(),
+ 56000 + overhead_rate));
+ EXPECT_EQ(56000, SetAndGetTargetBitrate(audio_encoder_.get(),
+ 56001 + overhead_rate));
}
TEST_F(AudioDecoderIsacFixTest, EncodeDecode) {
@@ -577,11 +598,18 @@
}
TEST_F(AudioDecoderIsacFixTest, SetTargetBitrate) {
- EXPECT_EQ(10000, SetAndGetTargetBitrate(audio_encoder_.get(), 9999));
- EXPECT_EQ(10000, SetAndGetTargetBitrate(audio_encoder_.get(), 10000));
- EXPECT_EQ(23456, SetAndGetTargetBitrate(audio_encoder_.get(), 23456));
- EXPECT_EQ(32000, SetAndGetTargetBitrate(audio_encoder_.get(), 32000));
- EXPECT_EQ(32000, SetAndGetTargetBitrate(audio_encoder_.get(), 32001));
+ const int overhead_rate =
+ 8 * kOverheadBytesPerPacket * codec_input_rate_hz_ / frame_size_;
+ EXPECT_EQ(10000,
+ SetAndGetTargetBitrate(audio_encoder_.get(), 9999 + overhead_rate));
+ EXPECT_EQ(10000, SetAndGetTargetBitrate(audio_encoder_.get(),
+ 10000 + overhead_rate));
+ EXPECT_EQ(23456, SetAndGetTargetBitrate(audio_encoder_.get(),
+ 23456 + overhead_rate));
+ EXPECT_EQ(32000, SetAndGetTargetBitrate(audio_encoder_.get(),
+ 32000 + overhead_rate));
+ EXPECT_EQ(32000, SetAndGetTargetBitrate(audio_encoder_.get(),
+ 32001 + overhead_rate));
}
TEST_F(AudioDecoderG722Test, EncodeDecode) {
@@ -622,11 +650,18 @@
}
TEST_P(AudioDecoderOpusTest, SetTargetBitrate) {
- EXPECT_EQ(6000, SetAndGetTargetBitrate(audio_encoder_.get(), 5999));
- EXPECT_EQ(6000, SetAndGetTargetBitrate(audio_encoder_.get(), 6000));
- EXPECT_EQ(32000, SetAndGetTargetBitrate(audio_encoder_.get(), 32000));
- EXPECT_EQ(510000, SetAndGetTargetBitrate(audio_encoder_.get(), 510000));
- EXPECT_EQ(510000, SetAndGetTargetBitrate(audio_encoder_.get(), 511000));
+ const int overhead_rate =
+ 8 * kOverheadBytesPerPacket * codec_input_rate_hz_ / frame_size_;
+ EXPECT_EQ(6000,
+ SetAndGetTargetBitrate(audio_encoder_.get(), 5999 + overhead_rate));
+ EXPECT_EQ(6000,
+ SetAndGetTargetBitrate(audio_encoder_.get(), 6000 + overhead_rate));
+ EXPECT_EQ(32000, SetAndGetTargetBitrate(audio_encoder_.get(),
+ 32000 + overhead_rate));
+ EXPECT_EQ(510000, SetAndGetTargetBitrate(audio_encoder_.get(),
+ 510000 + overhead_rate));
+ EXPECT_EQ(510000, SetAndGetTargetBitrate(audio_encoder_.get(),
+ 511000 + overhead_rate));
}
} // namespace webrtc
diff --git a/modules/rtp_rtcp/source/deprecated/deprecated_rtp_sender_egress.cc b/modules/rtp_rtcp/source/deprecated/deprecated_rtp_sender_egress.cc
index ba091cef..6cb9d93 100644
--- a/modules/rtp_rtcp/source/deprecated/deprecated_rtp_sender_egress.cc
+++ b/modules/rtp_rtcp/source/deprecated/deprecated_rtp_sender_egress.cc
@@ -27,11 +27,11 @@
constexpr int kBitrateStatisticsWindowMs = 1000;
constexpr size_t kRtpSequenceNumberMapMaxEntries = 1 << 13;
-bool IsEnabled(absl::string_view name,
- const WebRtcKeyValueConfig* field_trials) {
+bool IsDisabled(absl::string_view name,
+ const WebRtcKeyValueConfig* field_trials) {
FieldTrialBasedConfig default_trials;
auto& trials = field_trials ? *field_trials : default_trials;
- return absl::StartsWith(trials.Lookup(name), "Enabled");
+ return absl::StartsWith(trials.Lookup(name), "Disabled");
}
} // namespace
@@ -63,7 +63,7 @@
: absl::nullopt),
populate_network2_timestamp_(config.populate_network2_timestamp),
send_side_bwe_with_overhead_(
- IsEnabled("WebRTC-SendSideBwe-WithOverhead", config.field_trials)),
+ !IsDisabled("WebRTC-SendSideBwe-WithOverhead", config.field_trials)),
clock_(config.clock),
packet_history_(packet_history),
transport_(config.outgoing_transport),
diff --git a/modules/rtp_rtcp/source/rtp_sender_egress.cc b/modules/rtp_rtcp/source/rtp_sender_egress.cc
index 4252851..aba23dd 100644
--- a/modules/rtp_rtcp/source/rtp_sender_egress.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_egress.cc
@@ -90,9 +90,9 @@
: absl::nullopt),
populate_network2_timestamp_(config.populate_network2_timestamp),
send_side_bwe_with_overhead_(
- IsTrialSetTo(config.field_trials,
- "WebRTC-SendSideBwe-WithOverhead",
- "Enabled")),
+ !IsTrialSetTo(config.field_trials,
+ "WebRTC-SendSideBwe-WithOverhead",
+ "Disabled")),
clock_(config.clock),
packet_history_(packet_history),
transport_(config.outgoing_transport),
diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc
index c26041e..52e4ddb 100644
--- a/video/video_send_stream_tests.cc
+++ b/video/video_send_stream_tests.cc
@@ -2761,11 +2761,13 @@
static const int kMaxBitrateKbps = 413;
static const int kIncreasedStartBitrateKbps = 451;
static const int kIncreasedMaxBitrateKbps = 597;
- // If these fields trial are on, we get lower bitrates than expected by this
- // test, due to encoder pushback.
+ // TODO(bugs.webrtc.org/12058): If these fields trial are on, we get lower
+ // bitrates than expected by this test, due to encoder pushback and subtracted
+ // overhead.
webrtc::test::ScopedFieldTrials field_trials(
std::string(field_trial::GetFieldTrialString()) +
- "WebRTC-VideoRateControl/bitrate_adjuster:false/");
+ "WebRTC-VideoRateControl/bitrate_adjuster:false/"
+ "WebRTC-SendSideBwe-WithOverhead/Disabled/");
class EncoderBitrateThresholdObserver : public test::SendTest,
public VideoBitrateAllocatorFactory,
