Revert "Default enable WebRTC-SendSideBwe-WithOverhead."
This reverts commit 87c1950841c3f5e465e1663cc922717ce191e192.
Reason for revert: breaks downstream tests
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}
TBR=stefan@webrtc.org,jakobi@webrtc.org,alito@webrtc.org
Change-Id: If59fd41dcd8f6db76ea297c34c25fe19ae2ae973
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:6762
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/189973
Reviewed-by: Jakob Ivarsson <jakobi@webrtc.org>
Commit-Queue: Jakob Ivarsson <jakobi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32474}
diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc
index 1c0a32f..3fcdbf4 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::IsDisabled("WebRTC-SendSideBwe-WithOverhead")),
+ field_trial::IsEnabled("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 342b64b..ac1d29e 100644
--- a/call/call_perf_tests.cc
+++ b/call/call_perf_tests.cc
@@ -732,11 +732,6 @@
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 f5adae6..63bb937 100644
--- a/call/rtp_transport_controller_send.cc
+++ b/call/rtp_transport_controller_send.cc
@@ -64,11 +64,6 @@
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();
}
@@ -116,7 +111,7 @@
reset_feedback_on_route_change_(
!IsEnabled(trials, "WebRTC-Bwe-NoFeedbackReset")),
send_side_bwe_with_overhead_(
- !IsDisabled(trials, "WebRTC-SendSideBwe-WithOverhead")),
+ IsEnabled(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 9dad424..fd71248 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"),
- "Disabled")),
+ "Enabled")),
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 8bde0e3..d99e9c8 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::IsDisabled("WebRTC-SendSideBwe-WithOverhead");
+ field_trial::IsEnabled("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 8d1a734..2b16920 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::IsDisabled("WebRTC-SendSideBwe-WithOverhead")),
+ webrtc::field_trial::IsEnabled("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 0fe87bc..1cbc4a3 100644
--- a/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc
+++ b/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc
@@ -198,31 +198,22 @@
// 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 + kOverheadBps - 1,
- absl::nullopt);
+ states->encoder->OnReceivedUplinkBandwidth(kMinBitrateBps - 1, absl::nullopt);
EXPECT_EQ(kMinBitrateBps, states->encoder->GetTargetBitrate());
// Set a too high bitrate.
- states->encoder->OnReceivedUplinkBandwidth(kMaxBitrateBps + kOverheadBps + 1,
- absl::nullopt);
+ states->encoder->OnReceivedUplinkBandwidth(kMaxBitrateBps + 1, absl::nullopt);
EXPECT_EQ(kMaxBitrateBps, states->encoder->GetTargetBitrate());
// Set the minimum rate.
- states->encoder->OnReceivedUplinkBandwidth(kMinBitrateBps + kOverheadBps,
- absl::nullopt);
+ states->encoder->OnReceivedUplinkBandwidth(kMinBitrateBps, absl::nullopt);
EXPECT_EQ(kMinBitrateBps, states->encoder->GetTargetBitrate());
// Set the maximum rate.
- states->encoder->OnReceivedUplinkBandwidth(kMaxBitrateBps + kOverheadBps,
- absl::nullopt);
+ states->encoder->OnReceivedUplinkBandwidth(kMaxBitrateBps, absl::nullopt);
EXPECT_EQ(kMaxBitrateBps, states->encoder->GetTargetBitrate());
// Set rates from kMaxBitrateBps up to 32000 bps.
- for (int rate = kMinBitrateBps + kOverheadBps; rate <= 32000 + kOverheadBps;
- rate += 1000) {
+ for (int rate = kMinBitrateBps; rate <= 32000; rate += 1000) {
states->encoder->OnReceivedUplinkBandwidth(rate, absl::nullopt);
- EXPECT_EQ(rate - kOverheadBps, states->encoder->GetTargetBitrate());
+ EXPECT_EQ(rate, states->encoder->GetTargetBitrate());
}
}
@@ -385,6 +376,53 @@
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 56708ec..d1e1ec1 100644
--- a/modules/audio_coding/neteq/audio_decoder_unittest.cc
+++ b/modules/audio_coding/neteq/audio_decoder_unittest.cc
@@ -37,9 +37,6 @@
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.
@@ -359,7 +356,6 @@
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_;
@@ -379,7 +375,6 @@
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_;
@@ -399,7 +394,6 @@
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_;
@@ -457,7 +451,6 @@
? 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())};
@@ -543,18 +536,11 @@
}
TEST_F(AudioDecoderIsacFloatTest, SetTargetBitrate) {
- 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));
+ 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));
}
TEST_F(AudioDecoderIsacSwbTest, EncodeDecode) {
@@ -567,18 +553,11 @@
}
TEST_F(AudioDecoderIsacSwbTest, SetTargetBitrate) {
- 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));
+ 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));
}
TEST_F(AudioDecoderIsacFixTest, EncodeDecode) {
@@ -598,18 +577,11 @@
}
TEST_F(AudioDecoderIsacFixTest, SetTargetBitrate) {
- 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));
+ 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));
}
TEST_F(AudioDecoderG722Test, EncodeDecode) {
@@ -650,18 +622,11 @@
}
TEST_P(AudioDecoderOpusTest, SetTargetBitrate) {
- 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));
+ 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));
}
} // 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 6cb9d93..ba091cef 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 IsDisabled(absl::string_view name,
- const WebRtcKeyValueConfig* field_trials) {
+bool IsEnabled(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), "Disabled");
+ return absl::StartsWith(trials.Lookup(name), "Enabled");
}
} // namespace
@@ -63,7 +63,7 @@
: absl::nullopt),
populate_network2_timestamp_(config.populate_network2_timestamp),
send_side_bwe_with_overhead_(
- !IsDisabled("WebRTC-SendSideBwe-WithOverhead", config.field_trials)),
+ IsEnabled("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 aba23dd..4252851 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",
- "Disabled")),
+ IsTrialSetTo(config.field_trials,
+ "WebRTC-SendSideBwe-WithOverhead",
+ "Enabled")),
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 36793b6..300c4ac 100644
--- a/video/video_send_stream_tests.cc
+++ b/video/video_send_stream_tests.cc
@@ -2761,13 +2761,11 @@
static const int kMaxBitrateKbps = 413;
static const int kIncreasedStartBitrateKbps = 451;
static const int kIncreasedMaxBitrateKbps = 597;
- // 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.
+ // If these fields trial are on, we get lower bitrates than expected by this
+ // test, due to encoder pushback.
webrtc::test::ScopedFieldTrials field_trials(
std::string(field_trial::GetFieldTrialString()) +
- "WebRTC-VideoRateControl/bitrate_adjuster:false/"
- "WebRTC-SendSideBwe-WithOverhead/Disabled/");
+ "WebRTC-VideoRateControl/bitrate_adjuster:false/");
class EncoderBitrateThresholdObserver : public test::SendTest,
public VideoBitrateAllocatorFactory,
