Remove APM limiter in Audio Mixer.
The FrameCombiner sub-module of the AudioMixer uses one of two
limiters. One is an AudioProcessingModule with AGC1 enabled and
configured as a limiter. The other is the limiter part of AGC2. This
change removes the APM-AGC1 limiter. This requires small changes to
FrameCombiner, AudioMixerImpl and tests.
We also stop using the finch experiment flag.
Bug: webrtc:8925
Change-Id: Id7b8349ec4720b6417b15eaf70ed1a850b6ddbed
Reviewed-on: https://webrtc-review.googlesource.com/84620
Reviewed-by: Sam Zackrisson <saza@webrtc.org>
Commit-Queue: Alex Loiko <aleloi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23727}diff --git a/modules/audio_mixer/BUILD.gn b/modules/audio_mixer/BUILD.gn
index b829407..6a310e9 100644
--- a/modules/audio_mixer/BUILD.gn
+++ b/modules/audio_mixer/BUILD.gn
@@ -39,6 +39,7 @@
"../..:webrtc_common",
"../../:typedefs",
"../../api:array_view",
+ "../../api/audio:audio_frame_api",
"../../api/audio:audio_mixer_api",
"../../audio/utility:audio_frame_operations",
"../../common_audio",
diff --git a/modules/audio_mixer/audio_mixer_impl.cc b/modules/audio_mixer/audio_mixer_impl.cc
index 14c4e35..0940c59 100644
--- a/modules/audio_mixer/audio_mixer_impl.cc
+++ b/modules/audio_mixer/audio_mixer_impl.cc
@@ -19,7 +19,6 @@
#include "modules/audio_mixer/default_output_rate_calculator.h"
#include "rtc_base/logging.h"
#include "rtc_base/refcountedobject.h"
-#include "system_wrappers/include/field_trial.h"
namespace webrtc {
namespace {
@@ -89,17 +88,6 @@
return p->audio_source == audio_source;
});
}
-
-FrameCombiner::LimiterType ChooseLimiterType(bool use_limiter) {
- using LimiterType = FrameCombiner::LimiterType;
- if (!use_limiter) {
- return LimiterType::kNoLimiter;
- } else if (field_trial::IsEnabled("WebRTC-ApmGainController2Limiter")) {
- return LimiterType::kApmAgc2Limiter;
- } else {
- return LimiterType::kApmAgcLimiter;
- }
-}
} // namespace
AudioMixerImpl::AudioMixerImpl(
@@ -109,7 +97,7 @@
output_frequency_(0),
sample_size_(0),
audio_source_list_(),
- frame_combiner_(ChooseLimiterType(use_limiter)) {}
+ frame_combiner_(use_limiter) {}
AudioMixerImpl::~AudioMixerImpl() {}
@@ -127,11 +115,6 @@
std::move(output_rate_calculator), use_limiter));
}
-void AudioMixerImpl::SetLimiterType(FrameCombiner::LimiterType limiter_type) {
- RTC_DCHECK_RUNS_SERIALIZED(&race_checker_);
- frame_combiner_.SetLimiterType(limiter_type);
-}
-
void AudioMixerImpl::Mix(size_t number_of_channels,
AudioFrame* audio_frame_for_mixing) {
RTC_DCHECK(number_of_channels == 1 || number_of_channels == 2);
diff --git a/modules/audio_mixer/audio_mixer_impl.h b/modules/audio_mixer/audio_mixer_impl.h
index 86fcc1b..49edd16 100644
--- a/modules/audio_mixer/audio_mixer_impl.h
+++ b/modules/audio_mixer/audio_mixer_impl.h
@@ -17,7 +17,6 @@
#include "api/audio/audio_mixer.h"
#include "modules/audio_mixer/frame_combiner.h"
#include "modules/audio_mixer/output_rate_calculator.h"
-#include "modules/audio_processing/include/audio_processing.h"
#include "rtc_base/race_checker.h"
#include "rtc_base/scoped_ref_ptr.h"
#include "rtc_base/thread_annotations.h"
@@ -54,8 +53,6 @@
~AudioMixerImpl() override;
- void SetLimiterType(FrameCombiner::LimiterType limiter_type);
-
// AudioMixer functions
bool AddSource(Source* audio_source) override;
void RemoveSource(Source* audio_source) override;
diff --git a/modules/audio_mixer/audio_mixer_test.cc b/modules/audio_mixer/audio_mixer_test.cc
index 614c47f..a2b41ab 100644
--- a/modules/audio_mixer/audio_mixer_test.cc
+++ b/modules/audio_mixer/audio_mixer_test.cc
@@ -28,7 +28,7 @@
false,
"Enable stereo (interleaved). Inputs need not be as this parameter.");
-DEFINE_int(limiter, 0, "0-2. No limiter, AGC1, AGC2");
+DEFINE_bool(limiter, true, "Enable limiter.");
DEFINE_string(output_file,
"mixed_file.wav",
"File in which to store the mixed result.");
@@ -115,9 +115,7 @@
webrtc::AudioMixerImpl::Create(
std::unique_ptr<webrtc::OutputRateCalculator>(
new webrtc::DefaultOutputRateCalculator()),
- false));
- mixer->SetLimiterType(
- static_cast<webrtc::FrameCombiner::LimiterType>(FLAG_limiter));
+ FLAG_limiter));
const std::vector<std::string> input_files = parse_input_files();
std::vector<webrtc::test::FilePlayingSource> sources;
@@ -143,10 +141,7 @@
}
// Print stats.
- std::cout << "Limiting is: "
- << (FLAG_limiter == 0 ? "off"
- : (FLAG_limiter == 1 ? "agc" : "agc2"))
- << "\n"
+ std::cout << "Limiting is: " << (FLAG_limiter ? "on" : "off") << "\n"
<< "Channels: " << num_channels << "\n"
<< "Rate: " << sample_rate << "\n"
<< "Number of input streams: " << input_files.size() << "\n";
diff --git a/modules/audio_mixer/frame_combiner.cc b/modules/audio_mixer/frame_combiner.cc
index 04f7499..25e2194 100644
--- a/modules/audio_mixer/frame_combiner.cc
+++ b/modules/audio_mixer/frame_combiner.cc
@@ -19,6 +19,7 @@
#include "common_audio/include/audio_util.h"
#include "modules/audio_mixer/audio_frame_manipulator.h"
#include "modules/audio_mixer/audio_mixer_impl.h"
+#include "modules/audio_processing/include/audio_processing.h"
#include "modules/audio_processing/logging/apm_data_dumper.h"
#include "rtc_base/arraysize.h"
#include "rtc_base/checks.h"
@@ -34,35 +35,6 @@
using OneChannelBuffer = std::array<float, kMaximumChannelSize>;
-std::unique_ptr<AudioProcessing> CreateLimiter() {
- Config config;
- config.Set<ExperimentalAgc>(new ExperimentalAgc(false));
-
- std::unique_ptr<AudioProcessing> limiter(
- AudioProcessingBuilder().Create(config));
- RTC_DCHECK(limiter);
- webrtc::AudioProcessing::Config apm_config;
- apm_config.residual_echo_detector.enabled = false;
- limiter->ApplyConfig(apm_config);
-
- const auto check_no_error = [](int x) {
- RTC_DCHECK_EQ(x, AudioProcessing::kNoError);
- };
- auto* const gain_control = limiter->gain_control();
- check_no_error(gain_control->set_mode(GainControl::kFixedDigital));
-
- // We smoothly limit the mixed frame to -7 dbFS. -6 would correspond to the
- // divide-by-2 but -7 is used instead to give a bit of headroom since the
- // AGC is not a hard limiter.
- check_no_error(gain_control->set_target_level_dbfs(7));
-
- check_no_error(gain_control->set_compression_gain_db(0));
- check_no_error(gain_control->enable_limiter(true));
- check_no_error(gain_control->Enable(true));
-
- return limiter;
-}
-
void SetAudioFrameFields(const std::vector<AudioFrame*>& mix_list,
size_t number_of_channels,
int sample_rate,
@@ -119,52 +91,12 @@
return mixing_buffer;
}
-void RunApmAgcLimiter(AudioFrameView<float> mixing_buffer_view,
- AudioProcessing* apm_agc_limiter) {
- // Halve all samples to avoid saturation before limiting. The input
- // format of APM is Float. Convert the samples from FloatS16 to
- // Float.
- for (size_t i = 0; i < mixing_buffer_view.num_channels(); ++i) {
- std::transform(mixing_buffer_view.channel(i).begin(),
- mixing_buffer_view.channel(i).end(),
- mixing_buffer_view.channel(i).begin(),
- [](float a) { return FloatS16ToFloat(a / 2); });
- }
-
- const int sample_rate =
- static_cast<int>(mixing_buffer_view.samples_per_channel()) * 1000 /
- AudioMixerImpl::kFrameDurationInMs;
- StreamConfig processing_config(sample_rate,
- mixing_buffer_view.num_channels());
-
- // Smoothly limit the audio.
- apm_agc_limiter->ProcessStream(mixing_buffer_view.data(), processing_config,
- processing_config, mixing_buffer_view.data());
-
- // And now we can safely restore the level. This procedure results in
- // some loss of resolution, deemed acceptable.
- //
- // It's possible to apply the gain in the AGC (with a target level of 0 dbFS
- // and compression gain of 6 dB). However, in the transition frame when this
- // is enabled (moving from one to two audio sources) it has the potential to
- // create discontinuities in the mixed frame.
- //
- // Instead we double the samples in the frame..
- // Also convert the samples back to FloatS16.
- for (size_t i = 0; i < mixing_buffer_view.num_channels(); ++i) {
- std::transform(mixing_buffer_view.channel(i).begin(),
- mixing_buffer_view.channel(i).end(),
- mixing_buffer_view.channel(i).begin(),
- [](float a) { return FloatToFloatS16(a * 2); });
- }
-}
-
-void RunApmAgc2Limiter(AudioFrameView<float> mixing_buffer_view,
- FixedGainController* apm_agc2_limiter) {
+void RunLimiter(AudioFrameView<float> mixing_buffer_view,
+ FixedGainController* limiter) {
const size_t sample_rate = mixing_buffer_view.samples_per_channel() * 1000 /
AudioMixerImpl::kFrameDurationInMs;
- apm_agc2_limiter->SetSampleRate(sample_rate);
- apm_agc2_limiter->Process(mixing_buffer_view);
+ limiter->SetSampleRate(sample_rate);
+ limiter->Process(mixing_buffer_view);
}
// Both interleaves and rounds.
@@ -182,32 +114,15 @@
}
} // namespace
-FrameCombiner::FrameCombiner(LimiterType limiter_type)
- : limiter_type_(limiter_type),
- apm_agc_limiter_(limiter_type_ == LimiterType::kApmAgcLimiter
- ? CreateLimiter()
- : nullptr),
- data_dumper_(new ApmDataDumper(0)),
- apm_agc2_limiter_(data_dumper_.get()) {
- apm_agc2_limiter_.SetGain(0.f);
-}
-
FrameCombiner::FrameCombiner(bool use_limiter)
- : FrameCombiner(use_limiter ? LimiterType::kApmAgcLimiter
- : LimiterType::kNoLimiter) {}
+ : data_dumper_(new ApmDataDumper(0)),
+ limiter_(data_dumper_.get()),
+ use_limiter_(use_limiter) {
+ limiter_.SetGain(0.f);
+}
FrameCombiner::~FrameCombiner() = default;
-void FrameCombiner::SetLimiterType(LimiterType limiter_type) {
- // TODO(aleloi): remove this method and make limiter_type_ const
- // when we have finished moved to APM-AGC2.
- limiter_type_ = limiter_type;
- if (limiter_type_ == LimiterType::kApmAgcLimiter &&
- apm_agc_limiter_ == nullptr) {
- apm_agc_limiter_ = CreateLimiter();
- }
-}
-
void FrameCombiner::Combine(const std::vector<AudioFrame*>& mix_list,
size_t number_of_channels,
int sample_rate,
@@ -250,10 +165,8 @@
AudioFrameView<float> mixing_buffer_view(
&channel_pointers[0], number_of_channels, samples_per_channel);
- if (limiter_type_ == LimiterType::kApmAgcLimiter) {
- RunApmAgcLimiter(mixing_buffer_view, apm_agc_limiter_.get());
- } else if (limiter_type_ == LimiterType::kApmAgc2Limiter) {
- RunApmAgc2Limiter(mixing_buffer_view, &apm_agc2_limiter_);
+ if (use_limiter_) {
+ RunLimiter(mixing_buffer_view, &limiter_);
}
InterleaveToAudioFrame(mixing_buffer_view, audio_frame_for_mixing);
diff --git a/modules/audio_mixer/frame_combiner.h b/modules/audio_mixer/frame_combiner.h
index 2b77d6e..70ac027 100644
--- a/modules/audio_mixer/frame_combiner.h
+++ b/modules/audio_mixer/frame_combiner.h
@@ -14,8 +14,8 @@
#include <memory>
#include <vector>
+#include "api/audio/audio_frame.h"
#include "modules/audio_processing/agc2/fixed_gain_controller.h"
-#include "modules/audio_processing/include/audio_processing.h"
namespace webrtc {
class ApmDataDumper;
@@ -24,12 +24,9 @@
class FrameCombiner {
public:
enum class LimiterType { kNoLimiter, kApmAgcLimiter, kApmAgc2Limiter };
- explicit FrameCombiner(LimiterType limiter_type);
explicit FrameCombiner(bool use_limiter);
~FrameCombiner();
- void SetLimiterType(LimiterType limiter_type);
-
// Combine several frames into one. Assumes sample_rate,
// samples_per_channel of the input frames match the parameters. The
// parameters 'number_of_channels' and 'sample_rate' are needed
@@ -47,10 +44,9 @@
int sample_rate,
size_t number_of_streams) const;
- LimiterType limiter_type_;
- std::unique_ptr<AudioProcessing> apm_agc_limiter_;
std::unique_ptr<ApmDataDumper> data_dumper_;
- FixedGainController apm_agc2_limiter_;
+ FixedGainController limiter_;
+ const bool use_limiter_;
mutable int uma_logging_counter_ = 0;
};
} // namespace webrtc
diff --git a/modules/audio_mixer/frame_combiner_unittest.cc b/modules/audio_mixer/frame_combiner_unittest.cc
index b78e981..c7bf9c3 100644
--- a/modules/audio_mixer/frame_combiner_unittest.cc
+++ b/modules/audio_mixer/frame_combiner_unittest.cc
@@ -24,10 +24,9 @@
namespace {
using LimiterType = FrameCombiner::LimiterType;
-using NativeRate = AudioProcessing::NativeRate;
struct FrameCombinerConfig {
- LimiterType limiter_type;
- NativeRate sample_rate_hz;
+ bool use_limiter;
+ int sample_rate_hz;
int number_of_channels;
float wave_frequency;
};
@@ -46,13 +45,7 @@
std::ostringstream ss;
ss << "Sample rate: " << config.sample_rate_hz << " ,";
ss << "number of channels: " << config.number_of_channels << " ,";
- ss << "limiter active: "
- << (config.limiter_type == LimiterType::kNoLimiter
- ? "off"
-
- : (config.limiter_type == LimiterType::kApmAgcLimiter ? "agc1"
- : "agc2"))
- << " ,";
+ ss << "limiter active: " << (config.use_limiter ? "on" : "off") << " ,";
ss << "wave frequency: " << config.wave_frequency << " ,";
return ss.str();
}
@@ -70,9 +63,10 @@
}
} // namespace
+// The limiter requires sample rate divisible by 2000.
TEST(FrameCombiner, BasicApiCallsLimiter) {
- FrameCombiner combiner(LimiterType::kApmAgcLimiter);
- for (const int rate : {8000, 16000, 32000, 48000}) {
+ FrameCombiner combiner(true);
+ for (const int rate : {8000, 18000, 34000, 48000}) {
for (const int number_of_channels : {1, 2}) {
const std::vector<AudioFrame*> all_frames = {&frame1, &frame2};
SetUpFrames(rate, number_of_channels);
@@ -89,11 +83,10 @@
}
}
-// No APM limiter means no AudioProcessing::NativeRate restriction
-// on rate. The rate has to be divisible by 100 since we use
-// 10 ms frames, though.
+// With no limiter, the rate has to be divisible by 100 since we use
+// 10 ms frames.
TEST(FrameCombiner, BasicApiCallsNoLimiter) {
- FrameCombiner combiner(LimiterType::kNoLimiter);
+ FrameCombiner combiner(false);
for (const int rate : {8000, 10000, 11000, 32000, 44100}) {
for (const int number_of_channels : {1, 2}) {
const std::vector<AudioFrame*> all_frames = {&frame1, &frame2};
@@ -112,7 +105,7 @@
}
TEST(FrameCombiner, CombiningZeroFramesShouldProduceSilence) {
- FrameCombiner combiner(LimiterType::kNoLimiter);
+ FrameCombiner combiner(false);
for (const int rate : {8000, 10000, 11000, 32000, 44100}) {
for (const int number_of_channels : {1, 2}) {
SCOPED_TRACE(ProduceDebugText(rate, number_of_channels, 0));
@@ -134,7 +127,7 @@
}
TEST(FrameCombiner, CombiningOneFrameShouldNotChangeFrame) {
- FrameCombiner combiner(LimiterType::kNoLimiter);
+ FrameCombiner combiner(false);
for (const int rate : {8000, 10000, 11000, 32000, 44100}) {
for (const int number_of_channels : {1, 2}) {
SCOPED_TRACE(ProduceDebugText(rate, number_of_channels, 1));
@@ -164,22 +157,17 @@
// that it is inside reasonable bounds. This is to catch issues like
// chromium:695993 and chromium:816875.
TEST(FrameCombiner, GainCurveIsSmoothForAlternatingNumberOfStreams) {
+ // Rates are divisible by 2000 when limiter is active.
std::vector<FrameCombinerConfig> configs = {
- {LimiterType::kNoLimiter, NativeRate::kSampleRate32kHz, 2, 50.f},
- {LimiterType::kNoLimiter, NativeRate::kSampleRate16kHz, 1, 3200.f},
- {LimiterType::kApmAgcLimiter, NativeRate::kSampleRate8kHz, 1, 3200.f},
- {LimiterType::kApmAgcLimiter, NativeRate::kSampleRate16kHz, 1, 50.f},
- {LimiterType::kApmAgcLimiter, NativeRate::kSampleRate16kHz, 2, 3200.f},
- {LimiterType::kApmAgcLimiter, NativeRate::kSampleRate8kHz, 2, 50.f},
- {LimiterType::kApmAgc2Limiter, NativeRate::kSampleRate8kHz, 1, 3200.f},
- {LimiterType::kApmAgc2Limiter, NativeRate::kSampleRate32kHz, 1, 50.f},
- {LimiterType::kApmAgc2Limiter, NativeRate::kSampleRate48kHz, 2, 3200.f},
+ {false, 30100, 2, 50.f}, {false, 16500, 1, 3200.f},
+ {true, 8000, 1, 3200.f}, {true, 16000, 1, 50.f},
+ {true, 18000, 2, 3200.f}, {true, 10000, 2, 50.f},
};
for (const auto& config : configs) {
SCOPED_TRACE(ProduceDebugText(config));
- FrameCombiner combiner(config.limiter_type);
+ FrameCombiner combiner(config.use_limiter);
constexpr int16_t wave_amplitude = 30000;
SineWaveGenerator wave_generator(config.wave_frequency, wave_amplitude);