Removing AudioAllocationSettings moving functionality to AudioSendStream.
This is a no-op change that just removes the AudioAllocationSettings
helper class that was previously introduced since the field trials in it
were used in several places. Those other usages has now been removed
and AudioSendStream is now the only user. By moving the trials directly
to AudioSendStream we reduce the reader overhead when trying to follow
what a particular field trial does.
The "WebRTC-Audio-ForceNoTWCC" trial was removed as it is always set
together with "WebRTC-Audio-ABWENoTWCC".
Bug: webrtc:9883
Change-Id: Ib63589255bfe7adb155ea41279bdcd153f1536c8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/155366
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Reviewed-by: Oskar Sundbom <ossu@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29371}
diff --git a/audio/BUILD.gn b/audio/BUILD.gn
index fa16bb7..ba4518d 100644
--- a/audio/BUILD.gn
+++ b/audio/BUILD.gn
@@ -82,7 +82,7 @@
"../rtc_base:rtc_base_approved",
"../rtc_base:rtc_task_queue",
"../rtc_base:safe_minmax",
- "../rtc_base/experiments:audio_allocation_settings",
+ "../rtc_base/experiments:field_trial_parser",
"../system_wrappers",
"../system_wrappers:field_trial",
"../system_wrappers:metrics",
diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc
index 1ec2766..7f14768 100644
--- a/audio/audio_send_stream.cc
+++ b/audio/audio_send_stream.cc
@@ -41,7 +41,6 @@
#include "system_wrappers/include/field_trial.h"
namespace webrtc {
-namespace internal {
namespace {
// TODO(eladalon): Subsequent CL will make these values experiment-dependent.
constexpr size_t kPacketLossTrackerMaxWindowSizeMs = 15000;
@@ -79,9 +78,28 @@
event_log->Log(std::make_unique<RtcEventAudioSendStreamConfig>(
std::move(rtclog_config)));
}
-
} // namespace
+constexpr char AudioAllocationConfig::kKey[];
+
+std::unique_ptr<StructParametersParser> AudioAllocationConfig::Parser() {
+ return StructParametersParser::Create( //
+ "min", &min_bitrate, //
+ "max", &max_bitrate, //
+ "prio_rate", &priority_bitrate, //
+ "prio_rate_raw", &priority_bitrate_raw, //
+ "rate_prio", &bitrate_priority);
+}
+
+AudioAllocationConfig::AudioAllocationConfig() {
+ Parser()->Parse(field_trial::FindFullName(kKey));
+ if (priority_bitrate_raw && !priority_bitrate.IsZero()) {
+ RTC_LOG(LS_WARNING) << "'priority_bitrate' and '_raw' are mutually "
+ "exclusive but both were configured.";
+ }
+}
+
+namespace internal {
AudioSendStream::AudioSendStream(
Clock* clock,
const webrtc::AudioSendStream::Config& config,
@@ -129,6 +147,13 @@
std::unique_ptr<voe::ChannelSendInterface> channel_send)
: clock_(clock),
worker_queue_(rtp_transport->GetWorkerQueue()),
+ audio_send_side_bwe_(field_trial::IsEnabled("WebRTC-Audio-SendSideBwe")),
+ allocate_audio_without_feedback_(
+ field_trial::IsEnabled("WebRTC-Audio-ABWENoTWCC")),
+ enable_audio_alr_probing_(
+ !field_trial::IsDisabled("WebRTC-Audio-AlrProbing")),
+ send_side_bwe_with_overhead_(
+ field_trial::IsEnabled("WebRTC-SendSideBwe-WithOverhead")),
config_(Config(/*send_transport=*/nullptr, MediaTransportConfig())),
audio_state_(audio_state),
channel_send_(std::move(channel_send)),
@@ -286,15 +311,16 @@
bool transport_seq_num_id_changed =
new_ids.transport_sequence_number != old_ids.transport_sequence_number;
if (first_time || (transport_seq_num_id_changed &&
- !stream->allocation_settings_.ForceNoAudioFeedback())) {
+ !stream->allocate_audio_without_feedback_)) {
if (!first_time) {
channel_send->ResetSenderCongestionControlObjects();
}
RtcpBandwidthObserver* bandwidth_observer = nullptr;
- if (stream->allocation_settings_.ShouldSendTransportSequenceNumber(
- new_ids.transport_sequence_number)) {
+ if (stream->audio_send_side_bwe_ &&
+ !stream->allocate_audio_without_feedback_ &&
+ new_ids.transport_sequence_number != 0) {
channel_send->EnableSendTransportSequenceNumber(
new_ids.transport_sequence_number);
// Probing in application limited region is only used in combination with
@@ -303,7 +329,7 @@
if (stream->rtp_transport_) {
// Optionally request ALR probing but do not override any existing
// request from other streams.
- if (stream->allocation_settings_.RequestAlrProbing()) {
+ if (stream->enable_audio_alr_probing_) {
stream->rtp_transport_->EnablePeriodicAlrProbing(true);
}
bandwidth_observer = stream->rtp_transport_->GetBandwidthObserver();
@@ -345,10 +371,12 @@
if (sending_) {
return;
}
-
- if (allocation_settings_.IncludeAudioInAllocationOnStart(
- config_.min_bitrate_bps, config_.max_bitrate_bps, config_.has_dscp,
- TransportSeqNumId(config_))) {
+ // TODO(srte): We should not add audio to allocation just because
+ // audio_send_side_bwe_ is false.
+ if (!config_.has_dscp && config_.min_bitrate_bps != -1 &&
+ config_.max_bitrate_bps != -1 &&
+ (allocate_audio_without_feedback_ || TransportSeqNumId(config_) != 0 ||
+ !audio_send_side_bwe_)) {
rtp_transport_->AccountForAudioPacketsInPacedSender(true);
rtp_rtcp_module_->SetAsPartOfAllocation(true);
rtc::Event thread_sync_event;
@@ -812,13 +840,15 @@
stream->config_.max_bitrate_bps == new_config.max_bitrate_bps &&
stream->config_.bitrate_priority == new_config.bitrate_priority &&
(TransportSeqNumId(stream->config_) == TransportSeqNumId(new_config) ||
- stream->allocation_settings_.IgnoreSeqNumIdChange())) {
+ !stream->audio_send_side_bwe_)) {
return;
}
- if (stream->allocation_settings_.IncludeAudioInAllocationOnReconfigure(
- new_config.min_bitrate_bps, new_config.max_bitrate_bps,
- new_config.has_dscp, TransportSeqNumId(new_config))) {
+ // TODO(srte): We should not add audio to allocation just because
+ // audio_send_side_bwe_ is false.
+ if (!new_config.has_dscp && new_config.min_bitrate_bps != -1 &&
+ new_config.max_bitrate_bps != -1 &&
+ (TransportSeqNumId(new_config) != 0 || !stream->audio_send_side_bwe_)) {
stream->rtp_transport_->AccountForAudioPacketsInPacedSender(true);
rtc::Event thread_sync_event;
stream->worker_queue_->PostTask([&] {
@@ -847,12 +877,26 @@
// TODO(srte): Add overhead compensation here.
auto constraints = GetMinMaxBitrateConstraints();
+ DataRate max_overhead = DataRate::Zero();
+ if (send_side_bwe_with_overhead_) {
+ // TODO(srte): Respect |use_legacy_overhead_calculation_| here as well.
+ // OverheadPerPacket = Ipv4(20B) + UDP(8B) + SRTP(10B) + RTP(12)
+ constexpr int kOverheadPerPacket = 20 + 8 + 10 + 12;
+ const TimeDelta kMinPacketDuration = TimeDelta::ms(20);
+ max_overhead = DataSize::bytes(kOverheadPerPacket) / kMinPacketDuration;
+ }
+ DataRate priority_bitrate =
+ allocation_settings_.priority_bitrate + max_overhead;
+
+ if (allocation_settings_.priority_bitrate_raw)
+ priority_bitrate = *allocation_settings_.priority_bitrate_raw;
+
bitrate_allocator_->AddObserver(
this,
MediaStreamAllocationConfig{
constraints.min.bps<uint32_t>(), constraints.max.bps<uint32_t>(), 0,
- allocation_settings_.DefaultPriorityBitrate().bps(), true,
- allocation_settings_.BitratePriority().value_or(
+ priority_bitrate.bps(), true,
+ allocation_settings_.bitrate_priority.value_or(
config_.bitrate_priority)});
}
@@ -875,15 +919,15 @@
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();
+ if (allocation_settings_.min_bitrate)
+ constraints.min = *allocation_settings_.min_bitrate;
+ if (allocation_settings_.max_bitrate)
+ constraints.max = *allocation_settings_.max_bitrate;
RTC_DCHECK_GE(constraints.min, DataRate::Zero());
RTC_DCHECK_GE(constraints.max, DataRate::Zero());
RTC_DCHECK_GE(constraints.max, constraints.min);
- if (allocation_settings_.IncludeOverheadInAudioAllocation()) {
+ if (send_side_bwe_with_overhead_) {
if (use_legacy_overhead_calculation_) {
// OverheadPerPacket = Ipv4(20B) + UDP(8B) + SRTP(10B) + RTP(12)
const DataSize kOverheadPerPacket = DataSize::bytes(20 + 8 + 10 + 12);
diff --git a/audio/audio_send_stream.h b/audio/audio_send_stream.h
index 80b5087..3cd3a6c 100644
--- a/audio/audio_send_stream.h
+++ b/audio/audio_send_stream.h
@@ -23,7 +23,7 @@
#include "call/bitrate_allocator.h"
#include "modules/rtp_rtcp/include/rtp_rtcp.h"
#include "rtc_base/constructor_magic.h"
-#include "rtc_base/experiments/audio_allocation_settings.h"
+#include "rtc_base/experiments/struct_parameters_parser.h"
#include "rtc_base/race_checker.h"
#include "rtc_base/task_queue.h"
#include "rtc_base/thread_checker.h"
@@ -34,6 +34,21 @@
class RtcpRttStats;
class RtpTransportControllerSendInterface;
+struct AudioAllocationConfig {
+ static constexpr char kKey[] = "WebRTC-Audio-Allocation";
+ // Field Trial configured bitrates to use as overrides over default/user
+ // configured bitrate range when audio bitrate allocation is enabled.
+ absl::optional<DataRate> min_bitrate;
+ absl::optional<DataRate> max_bitrate;
+ DataRate priority_bitrate = DataRate::Zero();
+ // By default the priority_bitrate is compensated for packet overhead.
+ // Use this flag to configure a raw value instead.
+ absl::optional<DataRate> priority_bitrate_raw;
+ absl::optional<double> bitrate_priority;
+
+ std::unique_ptr<StructParametersParser> Parser();
+ AudioAllocationConfig();
+};
namespace internal {
class AudioState;
@@ -153,7 +168,14 @@
rtc::ThreadChecker pacer_thread_checker_;
rtc::RaceChecker audio_capture_race_checker_;
rtc::TaskQueue* worker_queue_;
- const AudioAllocationSettings allocation_settings_;
+
+ const bool audio_send_side_bwe_;
+ const bool allocate_audio_without_feedback_;
+ const bool force_no_audio_feedback_ = allocate_audio_without_feedback_;
+ const bool enable_audio_alr_probing_;
+ const bool send_side_bwe_with_overhead_;
+ const AudioAllocationConfig allocation_settings_;
+
rtc::CriticalSection config_cs_;
webrtc::AudioSendStream::Config config_;
rtc::scoped_refptr<webrtc::AudioState> audio_state_;
diff --git a/rtc_base/experiments/BUILD.gn b/rtc_base/experiments/BUILD.gn
index bbb5345..68afd8e 100644
--- a/rtc_base/experiments/BUILD.gn
+++ b/rtc_base/experiments/BUILD.gn
@@ -21,24 +21,6 @@
]
}
-rtc_static_library("audio_allocation_settings") {
- sources = [
- "audio_allocation_settings.cc",
- "audio_allocation_settings.h",
- ]
- defines = []
- if (rtc_opus_support_120ms_ptime) {
- defines += [ "WEBRTC_OPUS_SUPPORT_120MS_PTIME=1" ]
- } else {
- defines += [ "WEBRTC_OPUS_SUPPORT_120MS_PTIME=0" ]
- }
- deps = [
- ":field_trial_parser",
- "../:rtc_base_approved",
- "../../system_wrappers:field_trial",
- ]
-}
-
rtc_static_library("field_trial_parser") {
sources = [
"field_trial_list.cc",
diff --git a/rtc_base/experiments/audio_allocation_settings.cc b/rtc_base/experiments/audio_allocation_settings.cc
deleted file mode 100644
index dff1aae..0000000
--- a/rtc_base/experiments/audio_allocation_settings.cc
+++ /dev/null
@@ -1,134 +0,0 @@
-/*
- * Copyright 2019 The WebRTC project authors. All Rights Reserved.
- *
- * Use of this source code is governed by a BSD-style license
- * that can be found in the LICENSE file in the root of the source
- * tree. An additional intellectual property rights grant can be found
- * in the file PATENTS. All contributing project authors may
- * be found in the AUTHORS file in the root of the source tree.
- */
-#include "rtc_base/experiments/audio_allocation_settings.h"
-
-#include "rtc_base/logging.h"
-#include "system_wrappers/include/field_trial.h"
-
-namespace webrtc {
-namespace {
-// OverheadPerPacket = Ipv4(20B) + UDP(8B) + SRTP(10B) + RTP(12)
-constexpr int kOverheadPerPacket = 20 + 8 + 10 + 12;
-} // namespace
-AudioAllocationSettings::AudioAllocationSettings()
- : audio_send_side_bwe_(field_trial::IsEnabled("WebRTC-Audio-SendSideBwe")),
- allocate_audio_without_feedback_(
- field_trial::IsEnabled("WebRTC-Audio-ABWENoTWCC")),
- force_no_audio_feedback_(
- field_trial::IsEnabled("WebRTC-Audio-ForceNoTWCC")),
- enable_audio_alr_probing_(
- !field_trial::IsDisabled("WebRTC-Audio-AlrProbing")),
- send_side_bwe_with_overhead_(
- field_trial::IsEnabled("WebRTC-SendSideBwe-WithOverhead")),
- min_bitrate_("min"),
- max_bitrate_("max"),
- priority_bitrate_("prio_rate", DataRate::Zero()),
- priority_bitrate_raw_("prio_rate_raw"),
- bitrate_priority_("rate_prio") {
- ParseFieldTrial({&min_bitrate_, &max_bitrate_, &priority_bitrate_,
- &priority_bitrate_raw_, &bitrate_priority_},
- 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.
- if (send_side_bwe_with_overhead_) {
- constexpr int kMaxPacketSizeMs = WEBRTC_OPUS_SUPPORT_120MS_PTIME ? 120 : 60;
- min_overhead_bps_ = kOverheadPerPacket * 8 * 1000 / kMaxPacketSizeMs;
- }
- // priority_bitrate_raw will override priority_bitrate.
- if (priority_bitrate_raw_ && !priority_bitrate_->IsZero()) {
- RTC_LOG(LS_WARNING) << "'priority_bitrate' and '_raw' are mutually "
- "exclusive but both were configured.";
- }
-}
-
-AudioAllocationSettings::~AudioAllocationSettings() {}
-
-bool AudioAllocationSettings::ForceNoAudioFeedback() const {
- return force_no_audio_feedback_;
-}
-
-bool AudioAllocationSettings::IgnoreSeqNumIdChange() const {
- return !audio_send_side_bwe_;
-}
-
-bool AudioAllocationSettings::ConfigureRateAllocationRange() const {
- return audio_send_side_bwe_;
-}
-
-bool AudioAllocationSettings::ShouldSendTransportSequenceNumber(
- int transport_seq_num_extension_header_id) const {
- if (force_no_audio_feedback_)
- return false;
- return audio_send_side_bwe_ && !allocate_audio_without_feedback_ &&
- transport_seq_num_extension_header_id != 0;
-}
-
-bool AudioAllocationSettings::RequestAlrProbing() const {
- return enable_audio_alr_probing_;
-}
-
-bool AudioAllocationSettings::IncludeAudioInAllocationOnStart(
- int min_bitrate_bps,
- int max_bitrate_bps,
- bool has_dscp,
- int transport_seq_num_extension_header_id) const {
- if (has_dscp || min_bitrate_bps == -1 || max_bitrate_bps == -1)
- return false;
- if (transport_seq_num_extension_header_id != 0 && !force_no_audio_feedback_)
- return true;
- if (allocate_audio_without_feedback_)
- return true;
- if (audio_send_side_bwe_)
- return false;
- return true;
-}
-
-bool AudioAllocationSettings::IncludeAudioInAllocationOnReconfigure(
- int min_bitrate_bps,
- int max_bitrate_bps,
- bool has_dscp,
- int transport_seq_num_extension_header_id) const {
- // TODO(srte): Make this match include_audio_in_allocation_on_start.
- if (has_dscp || min_bitrate_bps == -1 || max_bitrate_bps == -1)
- return false;
- if (transport_seq_num_extension_header_id != 0)
- return true;
- if (audio_send_side_bwe_)
- return false;
- return true;
-}
-
-bool AudioAllocationSettings::IncludeOverheadInAudioAllocation() const {
- return send_side_bwe_with_overhead_;
-}
-
-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();
- if (priority_bitrate_raw_) {
- return *priority_bitrate_raw_;
- }
- if (send_side_bwe_with_overhead_) {
- const TimeDelta kMinPacketDuration = TimeDelta::ms(20);
- max_overhead = DataSize::bytes(kOverheadPerPacket) / kMinPacketDuration;
- }
- return priority_bitrate_.Get() + max_overhead;
-}
-absl::optional<double> AudioAllocationSettings::BitratePriority() const {
- return bitrate_priority_.GetOptional();
-}
-
-} // namespace webrtc
diff --git a/rtc_base/experiments/audio_allocation_settings.h b/rtc_base/experiments/audio_allocation_settings.h
deleted file mode 100644
index 8c5259b..0000000
--- a/rtc_base/experiments/audio_allocation_settings.h
+++ /dev/null
@@ -1,100 +0,0 @@
-/*
- * Copyright 2019 The WebRTC project authors. All Rights Reserved.
- *
- * Use of this source code is governed by a BSD-style license
- * that can be found in the LICENSE file in the root of the source
- * tree. An additional intellectual property rights grant can be found
- * in the file PATENTS. All contributing project authors may
- * be found in the AUTHORS file in the root of the source tree.
- */
-#ifndef RTC_BASE_EXPERIMENTS_AUDIO_ALLOCATION_SETTINGS_H_
-#define RTC_BASE_EXPERIMENTS_AUDIO_ALLOCATION_SETTINGS_H_
-
-#include "rtc_base/experiments/field_trial_parser.h"
-#include "rtc_base/experiments/field_trial_units.h"
-namespace webrtc {
-// This class encapsulates the logic that controls how allocation of audio
-// bitrate is done. This is primarily based on field trials, but also on the
-// values of audio parameters.
-class AudioAllocationSettings {
- public:
- AudioAllocationSettings();
- ~AudioAllocationSettings();
- // Returns true if audio feedback should be force disabled.
- bool ForceNoAudioFeedback() const;
- // Returns true if changes in transport sequence number id should be ignored
- // as a trigger for reconfiguration.
- bool IgnoreSeqNumIdChange() const;
- // Returns true if the bitrate allocation range should be configured.
- bool ConfigureRateAllocationRange() const;
- // Returns true if sent audio packets should have transport wide sequence
- // numbers.
- // |transport_seq_num_extension_header_id| the extension header id for
- // transport sequence numbers. Set to 0 if not the extension is not
- // configured.
- bool ShouldSendTransportSequenceNumber(
- int transport_seq_num_extension_header_id) const;
- // Returns true if audio should request ALR probing from network controller.
- bool RequestAlrProbing() const;
- // Returns true if audio should be added to rate allocation when the audio
- // stream is started.
- // |min_bitrate_bps| the configured min bitrate, set to -1 if unset.
- // |max_bitrate_bps| the configured max bitrate, set to -1 if unset.
- // |has_dscp| true is dscp is enabled.
- // |transport_seq_num_extension_header_id| the extension header id for
- // transport sequence numbers. Set to 0 if not the extension is not
- // configured.
- bool IncludeAudioInAllocationOnStart(
- int min_bitrate_bps,
- int max_bitrate_bps,
- bool has_dscp,
- int transport_seq_num_extension_header_id) const;
- // Returns true if audio should be added to rate allocation when the audio
- // stream is reconfigured.
- // |min_bitrate_bps| the configured min bitrate, set to -1 if unset.
- // |max_bitrate_bps| the configured max bitrate, set to -1 if unset.
- // |has_dscp| true is dscp is enabled.
- // |transport_seq_num_extension_header_id| the extension header id for
- // transport sequence numbers. Set to 0 if not the extension is not
- // configured.
- bool IncludeAudioInAllocationOnReconfigure(
- int min_bitrate_bps,
- 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.
- 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.
- DataRate DefaultPriorityBitrate() const;
-
- // The bitrate priority is used to determine how much of the available bitrate
- // beyond the min or priority bitrate audio streams should receive.
- absl::optional<double> BitratePriority() const;
-
- private:
- const bool audio_send_side_bwe_;
- const bool allocate_audio_without_feedback_;
- const bool force_no_audio_feedback_;
- const bool enable_audio_alr_probing_;
- const bool send_side_bwe_with_overhead_;
- int min_overhead_bps_ = 0;
- // 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_;
- // By default the priority_bitrate is compensated for packet overhead.
- // Use this flag to configure a raw value instead.
- FieldTrialOptional<DataRate> priority_bitrate_raw_;
- FieldTrialOptional<double> bitrate_priority_;
-};
-} // namespace webrtc
-
-#endif // RTC_BASE_EXPERIMENTS_AUDIO_ALLOCATION_SETTINGS_H_
