Ensure that we always set values for min and max audio bitrate.
Use (in order from lowest to highest precedence):
-- fixed 32000bps
-- fixed target bitrate from codec
-- explicit values from the rtp encoding parameters
-- Final precedence is given to field trial values from
WebRTC-Audio-Allocation
Bug: webrtc:10487
Change-Id: I7e289f209a927785572058b6fbfdf60fa14edf05
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/126229
Reviewed-by: Minyue Li <minyue@google.com>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Commit-Queue: Daniel Lee <dklee@google.com>
Cr-Commit-Position: refs/heads/master@{#27667}diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc
index 283fd9a..c8730d7 100644
--- a/audio/audio_send_stream.cc
+++ b/audio/audio_send_stream.cc
@@ -446,18 +446,12 @@
}
uint32_t AudioSendStream::OnBitrateUpdated(BitrateAllocationUpdate update) {
- // A send stream may be allocated a bitrate of zero if the allocator decides
- // to disable it. For now we ignore this decision and keep sending on min
- // bitrate.
- if (update.target_bitrate.IsZero()) {
- update.target_bitrate = DataRate::bps(config_.min_bitrate_bps);
- }
- RTC_DCHECK_GE(update.target_bitrate.bps<int>(), config_.min_bitrate_bps);
- // The bitrate allocator might allocate an higher than max configured bitrate
- // if there is room, to allow for, as example, extra FEC. Ignore that for now.
- const DataRate max_bitrate = DataRate::bps(config_.max_bitrate_bps);
- if (update.target_bitrate > max_bitrate)
- update.target_bitrate = max_bitrate;
+ // Pick a target bitrate between the constraints. Overrules the allocator if
+ // it 1) allocated a bitrate of zero to disable the stream or 2) allocated a
+ // higher than max to allow for e.g. extra FEC. For now we ignore these the
+ // exceptions.
+ auto constraints = GetMinMaxBitrateConstraints();
+ update.target_bitrate.Clamp(constraints.min, constraints.max);
channel_send_->OnBitrateAllocation(update);
@@ -799,12 +793,14 @@
void AudioSendStream::ConfigureBitrateObserver() {
// This either updates the current observer or adds a new observer.
// TODO(srte): Add overhead compensation here.
+ auto constraints = GetMinMaxBitrateConstraints();
+
bitrate_allocator_->AddObserver(
- this, MediaStreamAllocationConfig{
- static_cast<uint32_t>(config_.min_bitrate_bps),
- static_cast<uint32_t>(config_.max_bitrate_bps), 0,
- allocation_settings_.DefaultPriorityBitrate().bps(), true,
- config_.track_id, config_.bitrate_priority});
+ this,
+ MediaStreamAllocationConfig{
+ constraints.min.bps<uint32_t>(), constraints.max.bps<uint32_t>(), 0,
+ allocation_settings_.DefaultPriorityBitrate().bps(), true,
+ config_.track_id, config_.bitrate_priority});
}
void AudioSendStream::RemoveBitrateObserver() {
@@ -819,6 +815,36 @@
thread_sync_event.Wait(rtc::Event::kForever);
}
+AudioSendStream::TargetAudioBitrateConstraints
+AudioSendStream::GetMinMaxBitrateConstraints() const {
+ TargetAudioBitrateConstraints constraints{
+ DataRate::bps(config_.min_bitrate_bps),
+ DataRate::bps(config_.max_bitrate_bps)};
+
+ // If bitrates were explicitly overriden via field trial, use those values.
+ if (allocation_settings_.MinBitrate())
+ constraints.min = *allocation_settings_.MinBitrate();
+ if (allocation_settings_.MaxBitrate())
+ constraints.max = *allocation_settings_.MaxBitrate();
+
+ RTC_DCHECK_GE(constraints.min.bps(), 0);
+ RTC_DCHECK_GE(constraints.max.bps(), 0);
+ RTC_DCHECK_GE(constraints.max.bps(), constraints.min.bps());
+
+ // TODO(srte,dklee): Replace these with proper overhead calculations.
+ if (allocation_settings_.IncludeOverheadInAudioAllocation()) {
+ // OverheadPerPacket = Ipv4(20B) + UDP(8B) + SRTP(10B) + RTP(12)
+ const DataSize kOverheadPerPacket = DataSize::bytes(20 + 8 + 10 + 12);
+ const TimeDelta kMaxFrameLength = TimeDelta::ms(60); // Based on Opus spec
+ const DataRate kMinOverhead = kOverheadPerPacket / kMaxFrameLength;
+ constraints.min += kMinOverhead;
+ // TODO(dklee): This is obviously overly conservative to avoid exceeding max
+ // bitrate. Carefully reconsider the logic when addressing todo above.
+ constraints.max += kMinOverhead;
+ }
+ return constraints;
+}
+
void AudioSendStream::RegisterCngPayloadType(int payload_type,
int clockrate_hz) {
channel_send_->RegisterCngPayloadType(payload_type, clockrate_hz);
diff --git a/audio/audio_send_stream.h b/audio/audio_send_stream.h
index 8bee352..9796e80 100644
--- a/audio/audio_send_stream.h
+++ b/audio/audio_send_stream.h
@@ -104,6 +104,11 @@
private:
class TimedTransport;
+ // Constraints including overhead.
+ struct TargetAudioBitrateConstraints {
+ DataRate min;
+ DataRate max;
+ };
internal::AudioState* audio_state();
const internal::AudioState* audio_state() const;
@@ -126,6 +131,10 @@
void ConfigureBitrateObserver() RTC_RUN_ON(worker_queue_);
void RemoveBitrateObserver();
+ // Returns bitrate constraints, maybe including overhead when enabled by
+ // field trial.
+ TargetAudioBitrateConstraints GetMinMaxBitrateConstraints() const;
+
// Sets per-packet overhead on encoded (for ANA) based on current known values
// of transport and packetization overheads.
void UpdateOverheadForEncoder()
diff --git a/audio/audio_send_stream_unittest.cc b/audio/audio_send_stream_unittest.cc
index 701df1c..61acc96 100644
--- a/audio/audio_send_stream_unittest.cc
+++ b/audio/audio_send_stream_unittest.cc
@@ -73,6 +73,13 @@
{kOpusFormat, {48000, 1, 32000, 6000, 510000}},
{kG722Format, {16000, 1, 64000}}};
+// TODO(dklee): This mirrors calculation in audio_send_stream.cc, which
+// should be made more precise in the future. This can be changed when that
+// logic is more accurate.
+const DataSize kOverheadPerPacket = DataSize::bytes(20 + 8 + 10 + 12);
+const TimeDelta kMaxFrameLength = TimeDelta::ms(60);
+const DataRate kOverheadRate = kOverheadPerPacket / kMaxFrameLength;
+
class MockLimitObserver : public BitrateAllocator::LimitObserver {
public:
MOCK_METHOD3(OnAllocationLimitsChanged,
@@ -483,6 +490,97 @@
send_stream->OnBitrateUpdated(update);
}
+TEST(AudioSendStreamTest, SSBweTargetInRangeRespected) {
+ ScopedFieldTrials field_trials("WebRTC-Audio-SendSideBwe/Enabled/");
+ ConfigHelper helper(true, true);
+ auto send_stream = helper.CreateAudioSendStream();
+ EXPECT_CALL(*helper.channel_send(),
+ OnBitrateAllocation(Field(
+ &BitrateAllocationUpdate::target_bitrate,
+ Eq(DataRate::bps(helper.config().max_bitrate_bps - 5000)))));
+ BitrateAllocationUpdate update;
+ update.target_bitrate = DataRate::bps(helper.config().max_bitrate_bps - 5000);
+ send_stream->OnBitrateUpdated(update);
+}
+
+TEST(AudioSendStreamTest, SSBweFieldTrialMinRespected) {
+ ScopedFieldTrials field_trials(
+ "WebRTC-Audio-SendSideBwe/Enabled/"
+ "WebRTC-Audio-Allocation/min:6kbps,max:64kbps/");
+ ConfigHelper helper(true, true);
+ auto send_stream = helper.CreateAudioSendStream();
+ EXPECT_CALL(
+ *helper.channel_send(),
+ OnBitrateAllocation(Field(&BitrateAllocationUpdate::target_bitrate,
+ Eq(DataRate::kbps(6)))));
+ BitrateAllocationUpdate update;
+ update.target_bitrate = DataRate::kbps(1);
+ send_stream->OnBitrateUpdated(update);
+}
+
+TEST(AudioSendStreamTest, SSBweFieldTrialMaxRespected) {
+ ScopedFieldTrials field_trials(
+ "WebRTC-Audio-SendSideBwe/Enabled/"
+ "WebRTC-Audio-Allocation/min:6kbps,max:64kbps/");
+ ConfigHelper helper(true, true);
+ auto send_stream = helper.CreateAudioSendStream();
+ EXPECT_CALL(
+ *helper.channel_send(),
+ OnBitrateAllocation(Field(&BitrateAllocationUpdate::target_bitrate,
+ Eq(DataRate::kbps(64)))));
+ BitrateAllocationUpdate update;
+ update.target_bitrate = DataRate::kbps(128);
+ send_stream->OnBitrateUpdated(update);
+}
+
+TEST(AudioSendStreamTest, SSBweWithOverhead) {
+ ScopedFieldTrials field_trials(
+ "WebRTC-Audio-SendSideBwe/Enabled/"
+ "WebRTC-SendSideBwe-WithOverhead/Enabled/");
+ ConfigHelper helper(true, true);
+ auto send_stream = helper.CreateAudioSendStream();
+ const DataRate bitrate =
+ DataRate::bps(helper.config().max_bitrate_bps) + kOverheadRate;
+ EXPECT_CALL(*helper.channel_send(),
+ OnBitrateAllocation(Field(
+ &BitrateAllocationUpdate::target_bitrate, Eq(bitrate))));
+ BitrateAllocationUpdate update;
+ update.target_bitrate = bitrate;
+ send_stream->OnBitrateUpdated(update);
+}
+
+TEST(AudioSendStreamTest, SSBweWithOverheadMinRespected) {
+ ScopedFieldTrials field_trials(
+ "WebRTC-Audio-SendSideBwe/Enabled/"
+ "WebRTC-SendSideBwe-WithOverhead/Enabled/"
+ "WebRTC-Audio-Allocation/min:6kbps,max:64kbps/");
+ ConfigHelper helper(true, true);
+ auto send_stream = helper.CreateAudioSendStream();
+ const DataRate bitrate = DataRate::kbps(6) + kOverheadRate;
+ EXPECT_CALL(*helper.channel_send(),
+ OnBitrateAllocation(Field(
+ &BitrateAllocationUpdate::target_bitrate, Eq(bitrate))));
+ BitrateAllocationUpdate update;
+ update.target_bitrate = DataRate::kbps(1);
+ send_stream->OnBitrateUpdated(update);
+}
+
+TEST(AudioSendStreamTest, SSBweWithOverheadMaxRespected) {
+ ScopedFieldTrials field_trials(
+ "WebRTC-Audio-SendSideBwe/Enabled/"
+ "WebRTC-SendSideBwe-WithOverhead/Enabled/"
+ "WebRTC-Audio-Allocation/min:6kbps,max:64kbps/");
+ ConfigHelper helper(true, true);
+ auto send_stream = helper.CreateAudioSendStream();
+ const DataRate bitrate = DataRate::kbps(64) + kOverheadRate;
+ EXPECT_CALL(*helper.channel_send(),
+ OnBitrateAllocation(Field(
+ &BitrateAllocationUpdate::target_bitrate, Eq(bitrate))));
+ BitrateAllocationUpdate update;
+ update.target_bitrate = DataRate::kbps(128);
+ send_stream->OnBitrateUpdated(update);
+}
+
TEST(AudioSendStreamTest, ProbingIntervalOnBitrateUpdated) {
ConfigHelper helper(false, true);
auto send_stream = helper.CreateAudioSendStream();
diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc
index 9110d55..b0c5b7b 100644
--- a/media/engine/webrtc_voice_engine.cc
+++ b/media/engine/webrtc_voice_engine.cc
@@ -977,10 +977,27 @@
config_.send_codec_spec &&
absl::EqualsIgnoreCase(config_.send_codec_spec->format.name,
kOpusCodecName);
- if (is_opus && allocation_settings_.ConfigureRateAllocationRange()) {
- config_.min_bitrate_bps = allocation_settings_.MinBitrateBps();
- config_.max_bitrate_bps = allocation_settings_.MaxBitrateBps(
- rtp_parameters_.encodings[0].max_bitrate_bps);
+ if (is_opus) {
+ // The order of precedence (from lowest to highest is)
+ // - a reasonable default of 32kbps min/max
+ // - fixed target bitrate from codec spec
+ // - bitrate configured in the rtp_parameter encodings settings
+ const int kDefaultBitrateBps = 32000;
+ config_.min_bitrate_bps = kDefaultBitrateBps;
+ config_.max_bitrate_bps = kDefaultBitrateBps;
+
+ if (config_.send_codec_spec &&
+ config_.send_codec_spec->target_bitrate_bps) {
+ config_.min_bitrate_bps = *config_.send_codec_spec->target_bitrate_bps;
+ config_.max_bitrate_bps = *config_.send_codec_spec->target_bitrate_bps;
+ }
+
+ if (rtp_parameters_.encodings[0].min_bitrate_bps) {
+ config_.min_bitrate_bps = *rtp_parameters_.encodings[0].min_bitrate_bps;
+ }
+ if (rtp_parameters_.encodings[0].max_bitrate_bps) {
+ config_.max_bitrate_bps = *rtp_parameters_.encodings[0].max_bitrate_bps;
+ }
}
}
diff --git a/media/engine/webrtc_voice_engine_unittest.cc b/media/engine/webrtc_voice_engine_unittest.cc
index 4ddcd43..a76186d 100644
--- a/media/engine/webrtc_voice_engine_unittest.cc
+++ b/media/engine/webrtc_voice_engine_unittest.cc
@@ -2405,72 +2405,11 @@
public:
WebRtcVoiceEngineWithSendSideBweWithOverheadTest()
: WebRtcVoiceEngineTestFake(
- "WebRTC-Audio-SendSideBwe/Enabled/WebRTC-SendSideBwe-WithOverhead/"
+ "WebRTC-Audio-SendSideBwe/Enabled/WebRTC-Audio-Allocation/"
+ "min:6000bps,max:32000bps/WebRTC-SendSideBwe-WithOverhead/"
"Enabled/") {}
};
-TEST_F(WebRtcVoiceEngineWithSendSideBweWithOverheadTest, MinAndMaxBitrate) {
- EXPECT_TRUE(SetupSendStream());
- cricket::AudioSendParameters parameters;
- parameters.codecs.push_back(kOpusCodec);
- SetSendParameters(parameters);
- const int initial_num = call_.GetNumCreatedSendStreams();
- EXPECT_EQ(initial_num, call_.GetNumCreatedSendStreams());
-
- // OverheadPerPacket = Ipv4(20B) + UDP(8B) + SRTP(10B) + RTP(12)
- constexpr int kOverheadPerPacket = 20 + 8 + 10 + 12;
- constexpr int kOpusMaxPtimeMs = WEBRTC_OPUS_SUPPORT_120MS_PTIME ? 120 : 60;
- constexpr int kMinOverheadBps =
- kOverheadPerPacket * 8 * 1000 / kOpusMaxPtimeMs;
-
- constexpr int kOpusMinBitrateBps = 6000;
- EXPECT_EQ(kOpusMinBitrateBps + kMinOverheadBps,
- GetSendStreamConfig(kSsrcX).min_bitrate_bps);
- constexpr int kOpusBitrateFbBps = 32000;
- EXPECT_EQ(kOpusBitrateFbBps + kMinOverheadBps,
- GetSendStreamConfig(kSsrcX).max_bitrate_bps);
-
- parameters.options.audio_network_adaptor = true;
- parameters.options.audio_network_adaptor_config = {"1234"};
- SetSendParameters(parameters);
-
- constexpr int kMinOverheadWithAnaBps =
- kOverheadPerPacket * 8 * 1000 / kOpusMaxPtimeMs;
-
- EXPECT_EQ(kOpusMinBitrateBps + kMinOverheadWithAnaBps,
- GetSendStreamConfig(kSsrcX).min_bitrate_bps);
-
- EXPECT_EQ(kOpusBitrateFbBps + kMinOverheadWithAnaBps,
- GetSendStreamConfig(kSsrcX).max_bitrate_bps);
-}
-
-// This test is similar to
-// WebRtcVoiceEngineTestFake.SetRtpSendParameterUpdatesMaxBitrate but with an
-// additional field trial.
-TEST_F(WebRtcVoiceEngineWithSendSideBweWithOverheadTest,
- SetRtpSendParameterUpdatesMaxBitrate) {
- EXPECT_TRUE(SetupSendStream());
- cricket::AudioSendParameters send_parameters;
- send_parameters.codecs.push_back(kOpusCodec);
- SetSendParameters(send_parameters);
-
- webrtc::RtpParameters rtp_parameters = channel_->GetRtpSendParameters(kSsrcX);
- // Expect empty on parameters.encodings[0].max_bitrate_bps;
- EXPECT_FALSE(rtp_parameters.encodings[0].max_bitrate_bps);
-
- constexpr int kMaxBitrateBps = 6000;
- rtp_parameters.encodings[0].max_bitrate_bps = kMaxBitrateBps;
- EXPECT_TRUE(channel_->SetRtpSendParameters(kSsrcX, rtp_parameters).ok());
-
- const int max_bitrate = GetSendStreamConfig(kSsrcX).max_bitrate_bps;
-#if WEBRTC_OPUS_SUPPORT_120MS_PTIME
- constexpr int kMinOverhead = 3333;
-#else
- constexpr int kMinOverhead = 6666;
-#endif
- EXPECT_EQ(max_bitrate, kMaxBitrateBps + kMinOverhead);
-}
-
// Test that we can set the outgoing SSRC properly.
// SSRC is set in SetupSendStream() by calling AddSendStream.
TEST_F(WebRtcVoiceEngineTestFake, SetSendSsrc) {
diff --git a/rtc_base/experiments/audio_allocation_settings.cc b/rtc_base/experiments/audio_allocation_settings.cc
index a601cce..6579531 100644
--- a/rtc_base/experiments/audio_allocation_settings.cc
+++ b/rtc_base/experiments/audio_allocation_settings.cc
@@ -12,9 +12,6 @@
namespace webrtc {
namespace {
-// For SendSideBwe, Opus bitrate should be in the range between 6000 and 32000.
-const int kOpusMinBitrateBps = 6000;
-const int kOpusBitrateFbBps = 32000;
// OverheadPerPacket = Ipv4(20B) + UDP(8B) + SRTP(10B) + RTP(12)
constexpr int kOverheadPerPacket = 20 + 8 + 10 + 12;
} // namespace
@@ -23,8 +20,8 @@
allocate_audio_without_feedback_("Enabled"),
force_no_audio_feedback_("Enabled"),
send_side_bwe_with_overhead_("Enabled"),
- default_min_bitrate_("min", DataRate::bps(kOpusMinBitrateBps)),
- default_max_bitrate_("max", DataRate::bps(kOpusBitrateFbBps)),
+ min_bitrate_("min"),
+ max_bitrate_("max"),
priority_bitrate_("prio", DataRate::Zero()) {
ParseFieldTrial({&audio_send_side_bwe_},
field_trial::FindFullName("WebRTC-Audio-SendSideBwe"));
@@ -35,9 +32,8 @@
ParseFieldTrial({&send_side_bwe_with_overhead_},
field_trial::FindFullName("WebRTC-SendSideBwe-WithOverhead"));
- ParseFieldTrial(
- {&default_min_bitrate_, &default_max_bitrate_, &priority_bitrate_},
- field_trial::FindFullName("WebRTC-Audio-Allocation"));
+ ParseFieldTrial({&min_bitrate_, &max_bitrate_, &priority_bitrate_},
+ field_trial::FindFullName("WebRTC-Audio-Allocation"));
// TODO(mflodman): Keep testing this and set proper values.
// Note: This is an early experiment currently only supported by Opus.
@@ -100,25 +96,15 @@
return true;
}
-int AudioAllocationSettings::MinBitrateBps() const {
- return default_min_bitrate_->bps() + min_overhead_bps_;
+bool AudioAllocationSettings::IncludeOverheadInAudioAllocation() const {
+ return send_side_bwe_with_overhead_;
}
-int AudioAllocationSettings::MaxBitrateBps(
- absl::optional<int> rtp_parameter_max_bitrate_bps) const {
- // We assume that the max is a hard limit on the payload bitrate, so we add
- // min_overhead_bps to it to ensure that, when overhead is deducted, the
- // payload rate never goes beyond the limit. Note: this also means that if a
- // higher overhead is forced, we cannot reach the limit.
- // TODO(minyue): Reconsider this when the signaling to BWE is done
- // through a dedicated API.
-
- // This means that when RtpParameters is reset, we may change the
- // encoder's bit rate immediately (through ReconfigureAudioSendStream()),
- // meanwhile change the cap to the output of BWE.
- if (rtp_parameter_max_bitrate_bps)
- return *rtp_parameter_max_bitrate_bps + min_overhead_bps_;
- return default_max_bitrate_->bps() + min_overhead_bps_;
+absl::optional<DataRate> AudioAllocationSettings::MinBitrate() const {
+ return min_bitrate_.GetOptional();
+}
+absl::optional<DataRate> AudioAllocationSettings::MaxBitrate() const {
+ return max_bitrate_.GetOptional();
}
DataRate AudioAllocationSettings::DefaultPriorityBitrate() const {
DataRate max_overhead = DataRate::Zero();
diff --git a/rtc_base/experiments/audio_allocation_settings.h b/rtc_base/experiments/audio_allocation_settings.h
index 32e11df..c099973 100644
--- a/rtc_base/experiments/audio_allocation_settings.h
+++ b/rtc_base/experiments/audio_allocation_settings.h
@@ -60,14 +60,13 @@
int max_bitrate_bps,
bool has_dscp,
int transport_seq_num_extension_header_id) const;
+ // Returns true if we should include packet overhead in audio allocation.
+ bool IncludeOverheadInAudioAllocation() const;
- // Returns the min bitrate for audio rate allocation, potentially including
- // overhead.
- int MinBitrateBps() const;
- // Returns the max bitrate for audio rate allocation, potentially including
- // overhead. |rtp_parameter_max_bitrate_bps| max bitrate as configured in rtp
- // parameters, excluding overhead.
- int MaxBitrateBps(absl::optional<int> rtp_parameter_max_bitrate_bps) const;
+ // Returns the min bitrate for audio rate allocation.
+ absl::optional<DataRate> MinBitrate() const;
+ // Returns the max bitrate for audio rate allocation.
+ absl::optional<DataRate> MaxBitrate() const;
// Indicates the default priority bitrate for audio streams. The bitrate
// allocator will prioritize audio until it reaches this bitrate and will
// divide bitrate evently between audio and video above this bitrate.
@@ -79,10 +78,10 @@
FieldTrialFlag force_no_audio_feedback_;
FieldTrialFlag send_side_bwe_with_overhead_;
int min_overhead_bps_ = 0;
- // Default bitrates to use as range if there's no user configured
- // bitrate range but audio bitrate allocation is enabled.
- FieldTrialParameter<DataRate> default_min_bitrate_;
- FieldTrialParameter<DataRate> default_max_bitrate_;
+ // Field Trial configured bitrates to use as overrides over default/user
+ // configured bitrate range when audio bitrate allocation is enabled.
+ FieldTrialOptional<DataRate> min_bitrate_;
+ FieldTrialOptional<DataRate> max_bitrate_;
FieldTrialParameter<DataRate> priority_bitrate_;
};
} // namespace webrtc
