AGC2: GainController::ApplyConfig removed
When `AudioProcessingImpl::ApplyConfig()` is called, AGC2 is initialized
and then the new config is applied. That is error prone and for example
breaks bit exactness in [1].
Changes:
- `GainController2` must be created by passing configuration,
sample rate and number of channels
- `GainController2::ApplyConfig()` removed
Bit exactness verified with audioproc_f on a collection of AEC dumps
and Wav files (42 recordings in total).
[1] https://webrtc-review.googlesource.com/c/src/+/234587.
Bug: webrtc:7494
Change-Id: I251e03603394a4fc8769b9b5c197a157893676a9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/235060
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35206}
diff --git a/modules/audio_processing/agc2/gain_applier.cc b/modules/audio_processing/agc2/gain_applier.cc
index a777cf3..f9e276d 100644
--- a/modules/audio_processing/agc2/gain_applier.cc
+++ b/modules/audio_processing/agc2/gain_applier.cc
@@ -88,6 +88,7 @@
}
}
+// TODO(bugs.webrtc.org/7494): Remove once switched to gains in dB.
void GainApplier::SetGainFactor(float gain_factor) {
RTC_DCHECK_GT(gain_factor, 0.f);
current_gain_factor_ = gain_factor;
diff --git a/modules/audio_processing/audio_processing_impl.cc b/modules/audio_processing/audio_processing_impl.cc
index 318314b..ee9f5c7 100644
--- a/modules/audio_processing/audio_processing_impl.cc
+++ b/modules/audio_processing/audio_processing_impl.cc
@@ -412,7 +412,7 @@
InitializeVoiceDetector();
InitializeResidualEchoDetector();
InitializeEchoController();
- InitializeGainController2();
+ InitializeGainController2(/*config_has_changed=*/true);
InitializeNoiseSuppressor();
InitializeAnalyzer();
InitializePostProcessor();
@@ -589,9 +589,7 @@
config_.gain_controller2 = AudioProcessing::Config::GainController2();
}
- if (agc2_config_changed) {
- InitializeGainController2();
- }
+ InitializeGainController2(agc2_config_changed);
if (pre_amplifier_config_changed || gain_adjustment_config_changed) {
InitializeCaptureLevelsAdjuster();
@@ -873,7 +871,7 @@
float value;
setting.GetFloat(&value);
config_.gain_controller2.fixed_digital.gain_db = value;
- submodules_.gain_controller2->ApplyConfig(config_.gain_controller2);
+ submodules_.gain_controller2->SetFixedGainDb(value);
}
break;
}
@@ -1936,19 +1934,18 @@
capture_.capture_output_used);
}
-void AudioProcessingImpl::InitializeGainController2() {
- if (config_.gain_controller2.enabled) {
- if (!submodules_.gain_controller2) {
- // TODO(alessiob): Move the injected gain controller once injection is
- // implemented.
- submodules_.gain_controller2.reset(new GainController2());
- }
-
- submodules_.gain_controller2->Initialize(proc_fullband_sample_rate_hz(),
- num_input_channels());
- submodules_.gain_controller2->ApplyConfig(config_.gain_controller2);
- } else {
+void AudioProcessingImpl::InitializeGainController2(bool config_has_changed) {
+ if (!config_has_changed) {
+ return;
+ }
+ if (!config_.gain_controller2.enabled) {
submodules_.gain_controller2.reset();
+ return;
+ }
+ if (!submodules_.gain_controller2 || config_has_changed) {
+ submodules_.gain_controller2 = std::make_unique<GainController2>(
+ config_.gain_controller2, proc_fullband_sample_rate_hz(),
+ num_input_channels());
}
}
diff --git a/modules/audio_processing/audio_processing_impl.h b/modules/audio_processing/audio_processing_impl.h
index 026152b..89f5d63 100644
--- a/modules/audio_processing/audio_processing_impl.h
+++ b/modules/audio_processing/audio_processing_impl.h
@@ -267,7 +267,7 @@
void InitializeEchoController()
RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_render_, mutex_capture_);
- // Initializations of capture-only submodules, requiring the capture lock
+ // Initializations of capture-only sub-modules, requiring the capture lock
// already acquired.
void InitializeHighPassFilter(bool forced_reset)
RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_capture_);
@@ -275,7 +275,10 @@
void InitializeGainController1() RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_capture_);
void InitializeTransientSuppressor()
RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_capture_);
- void InitializeGainController2() RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_capture_);
+ // Initializes the `GainController2` sub-module. If the sub-module is enabled
+ // and `config_has_changed` is true, recreates the sub-module.
+ void InitializeGainController2(bool config_has_changed)
+ RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_capture_);
void InitializeNoiseSuppressor() RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_capture_);
void InitializeCaptureLevelsAdjuster()
RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_capture_);
diff --git a/modules/audio_processing/gain_controller2.cc b/modules/audio_processing/gain_controller2.cc
index 92cda69..416bc78 100644
--- a/modules/audio_processing/gain_controller2.cc
+++ b/modules/audio_processing/gain_controller2.cc
@@ -10,6 +10,9 @@
#include "modules/audio_processing/gain_controller2.h"
+#include <memory>
+#include <utility>
+
#include "common_audio/include/audio_util.h"
#include "modules/audio_processing/audio_buffer.h"
#include "modules/audio_processing/include/audio_frame_view.h"
@@ -21,24 +24,53 @@
namespace webrtc {
namespace {
+
+using Agc2Config = AudioProcessing::Config::GainController2;
+
+constexpr int kUnspecifiedAnalogLevel = -1;
constexpr int kLogLimiterStatsPeriodMs = 30'000;
constexpr int kFrameLengthMs = 10;
constexpr int kLogLimiterStatsPeriodNumFrames =
kLogLimiterStatsPeriodMs / kFrameLengthMs;
+
+// Creates an adaptive digital gain controller if enabled.
+std::unique_ptr<AdaptiveAgc> CreateAdaptiveDigitalController(
+ const Agc2Config::AdaptiveDigital& config,
+ int sample_rate_hz,
+ int num_channels,
+ ApmDataDumper* data_dumper) {
+ if (config.enabled) {
+ // TODO(bugs.webrtc.org/7494): Also init with sample rate and num channels.
+ auto controller = std::make_unique<AdaptiveAgc>(data_dumper, config);
+ // TODO(bugs.webrtc.org/7494): Remove once passed to the ctor.
+ controller->Initialize(sample_rate_hz, num_channels);
+ return controller;
+ }
+ return nullptr;
+}
+
} // namespace
int GainController2::instance_count_ = 0;
-GainController2::GainController2()
+GainController2::GainController2(const Agc2Config& config,
+ int sample_rate_hz,
+ int num_channels)
: data_dumper_(rtc::AtomicOps::Increment(&instance_count_)),
fixed_gain_applier_(/*hard_clip_samples=*/false,
/*initial_gain_factor=*/0.0f),
- limiter_(static_cast<size_t>(48000), &data_dumper_, "Agc2"),
- calls_since_last_limiter_log_(0) {
- if (config_.adaptive_digital.enabled) {
- adaptive_digital_controller_ =
- std::make_unique<AdaptiveAgc>(&data_dumper_, config_.adaptive_digital);
- }
+ adaptive_digital_controller_(
+ CreateAdaptiveDigitalController(config.adaptive_digital,
+ sample_rate_hz,
+ num_channels,
+ &data_dumper_)),
+ limiter_(sample_rate_hz, &data_dumper_, /*histogram_name_prefix=*/"Agc2"),
+ calls_since_last_limiter_log_(0),
+ analog_level_(kUnspecifiedAnalogLevel) {
+ RTC_DCHECK(Validate(config));
+ data_dumper_.InitiateNewSetOfRecordings();
+ // TODO(bugs.webrtc.org/7494): Set gain when `fixed_gain_applier_` is init'd.
+ fixed_gain_applier_.SetGainFactor(DbToRatio(config.fixed_digital.gain_db));
}
GainController2::~GainController2() = default;
@@ -48,13 +80,24 @@
sample_rate_hz == AudioProcessing::kSampleRate16kHz ||
sample_rate_hz == AudioProcessing::kSampleRate32kHz ||
sample_rate_hz == AudioProcessing::kSampleRate48kHz);
+ // TODO(bugs.webrtc.org/7494): Initialize `fixed_gain_applier_`.
limiter_.SetSampleRate(sample_rate_hz);
if (adaptive_digital_controller_) {
adaptive_digital_controller_->Initialize(sample_rate_hz, num_channels);
}
data_dumper_.InitiateNewSetOfRecordings();
- data_dumper_.DumpRaw("sample_rate_hz", sample_rate_hz);
calls_since_last_limiter_log_ = 0;
+ analog_level_ = kUnspecifiedAnalogLevel;
+}
+
+void GainController2::SetFixedGainDb(float gain_db) {
+ const float gain_factor = DbToRatio(gain_db);
+ if (fixed_gain_applier_.GetGainFactor() != gain_factor) {
+ // Reset the limiter to quickly react on abrupt level changes caused by
+ // large changes of the fixed gain.
+ limiter_.Reset();
+ }
+ fixed_gain_applier_.SetGainFactor(gain_factor);
}
void GainController2::Process(AudioBuffer* audio) {
@@ -87,25 +130,6 @@
analog_level_ = level;
}
-void GainController2::ApplyConfig(
- const AudioProcessing::Config::GainController2& config) {
- RTC_DCHECK(Validate(config));
-
- config_ = config;
- if (config.fixed_digital.gain_db != config_.fixed_digital.gain_db) {
- // Reset the limiter to quickly react on abrupt level changes caused by
- // large changes of the fixed gain.
- limiter_.Reset();
- }
- fixed_gain_applier_.SetGainFactor(DbToRatio(config_.fixed_digital.gain_db));
- if (config_.adaptive_digital.enabled) {
- adaptive_digital_controller_ =
- std::make_unique<AdaptiveAgc>(&data_dumper_, config_.adaptive_digital);
- } else {
- adaptive_digital_controller_.reset();
- }
-}
-
bool GainController2::Validate(
const AudioProcessing::Config::GainController2& config) {
const auto& fixed = config.fixed_digital;
diff --git a/modules/audio_processing/gain_controller2.h b/modules/audio_processing/gain_controller2.h
index 1a382f6..cfe51c2 100644
--- a/modules/audio_processing/gain_controller2.h
+++ b/modules/audio_processing/gain_controller2.h
@@ -28,27 +28,35 @@
// microphone gain and/or applying digital gain.
class GainController2 {
public:
- GainController2();
+ GainController2(const AudioProcessing::Config::GainController2& config,
+ int sample_rate_hz,
+ int num_channels);
GainController2(const GainController2&) = delete;
GainController2& operator=(const GainController2&) = delete;
~GainController2();
+ // Detects and handles changes of sample rate and/or number of channels.
void Initialize(int sample_rate_hz, int num_channels);
+
+ // Sets the fixed digital gain.
+ void SetFixedGainDb(float gain_db);
+
+ // Applies fixed and adaptive digital gains to `audio` and runs a limiter.
void Process(AudioBuffer* audio);
+
+ // Handles analog level changes.
void NotifyAnalogLevel(int level);
- void ApplyConfig(const AudioProcessing::Config::GainController2& config);
static bool Validate(const AudioProcessing::Config::GainController2& config);
private:
static int instance_count_;
ApmDataDumper data_dumper_;
- AudioProcessing::Config::GainController2 config_;
GainApplier fixed_gain_applier_;
std::unique_ptr<AdaptiveAgc> adaptive_digital_controller_;
Limiter limiter_;
int calls_since_last_limiter_log_;
- int analog_level_ = -1;
+ int analog_level_;
};
} // namespace webrtc
diff --git a/modules/audio_processing/gain_controller2_unittest.cc b/modules/audio_processing/gain_controller2_unittest.cc
index d1c1f5b..b18d943 100644
--- a/modules/audio_processing/gain_controller2_unittest.cc
+++ b/modules/audio_processing/gain_controller2_unittest.cc
@@ -14,6 +14,7 @@
#include <cmath>
#include <memory>
#include <numeric>
+#include <tuple>
#include "api/array_view.h"
#include "modules/audio_processing/agc2/agc2_testing_common.h"
@@ -27,6 +28,8 @@
namespace test {
namespace {
+using Agc2Config = AudioProcessing::Config::GainController2;
+
// Sets all the samples in `ab` to `value`.
void SetAudioBufferSamples(float value, AudioBuffer& ab) {
for (size_t k = 0; k < ab.num_channels(); ++k) {
@@ -51,33 +54,26 @@
return ab.channels()[0][num_samples - 1];
}
-AudioProcessing::Config::GainController2 CreateAgc2FixedDigitalModeConfig(
- float fixed_gain_db) {
- AudioProcessing::Config::GainController2 config;
- config.adaptive_digital.enabled = false;
- config.fixed_digital.gain_db = fixed_gain_db;
- EXPECT_TRUE(GainController2::Validate(config));
- return config;
-}
-
std::unique_ptr<GainController2> CreateAgc2FixedDigitalMode(
float fixed_gain_db,
int sample_rate_hz) {
- auto agc2 = std::make_unique<GainController2>();
- agc2->ApplyConfig(CreateAgc2FixedDigitalModeConfig(fixed_gain_db));
- agc2->Initialize(sample_rate_hz, /*num_channels=*/1);
- return agc2;
+ Agc2Config config;
+ config.adaptive_digital.enabled = false;
+ config.fixed_digital.gain_db = fixed_gain_db;
+ EXPECT_TRUE(GainController2::Validate(config));
+ return std::make_unique<GainController2>(config, sample_rate_hz,
+ /*num_channels=*/1);
}
} // namespace
TEST(GainController2, CheckDefaultConfig) {
- AudioProcessing::Config::GainController2 config;
+ Agc2Config config;
EXPECT_TRUE(GainController2::Validate(config));
}
TEST(GainController2, CheckFixedDigitalConfig) {
- AudioProcessing::Config::GainController2 config;
+ Agc2Config config;
// Attenuation is not allowed.
config.fixed_digital.gain_db = -5.0f;
EXPECT_FALSE(GainController2::Validate(config));
@@ -90,7 +86,7 @@
}
TEST(GainController2, CheckHeadroomDb) {
- AudioProcessing::Config::GainController2 config;
+ Agc2Config config;
config.adaptive_digital.headroom_db = -1.0f;
EXPECT_FALSE(GainController2::Validate(config));
config.adaptive_digital.headroom_db = 0.0f;
@@ -100,7 +96,7 @@
}
TEST(GainController2, CheckMaxGainDb) {
- AudioProcessing::Config::GainController2 config;
+ Agc2Config config;
config.adaptive_digital.max_gain_db = -1.0f;
EXPECT_FALSE(GainController2::Validate(config));
config.adaptive_digital.max_gain_db = 0.0f;
@@ -110,7 +106,7 @@
}
TEST(GainController2, CheckInitialGainDb) {
- AudioProcessing::Config::GainController2 config;
+ Agc2Config config;
config.adaptive_digital.initial_gain_db = -1.0f;
EXPECT_FALSE(GainController2::Validate(config));
config.adaptive_digital.initial_gain_db = 0.0f;
@@ -120,7 +116,7 @@
}
TEST(GainController2, CheckAdaptiveDigitalMaxGainChangeSpeedConfig) {
- AudioProcessing::Config::GainController2 config;
+ Agc2Config config;
config.adaptive_digital.max_gain_change_db_per_second = -1.0f;
EXPECT_FALSE(GainController2::Validate(config));
config.adaptive_digital.max_gain_change_db_per_second = 0.0f;
@@ -130,7 +126,7 @@
}
TEST(GainController2, CheckAdaptiveDigitalMaxOutputNoiseLevelConfig) {
- AudioProcessing::Config::GainController2 config;
+ Agc2Config config;
config.adaptive_digital.max_output_noise_level_dbfs = 5.0f;
EXPECT_FALSE(GainController2::Validate(config));
config.adaptive_digital.max_output_noise_level_dbfs = 0.0f;
@@ -141,9 +137,9 @@
// Checks that the default config is applied.
TEST(GainController2, ApplyDefaultConfig) {
- auto gain_controller2 = std::make_unique<GainController2>();
- AudioProcessing::Config::GainController2 config;
- gain_controller2->ApplyConfig(config);
+ auto gain_controller2 = std::make_unique<GainController2>(
+ Agc2Config{}, /*sample_rate_hz=*/16000, /*num_channels=*/2);
+ EXPECT_TRUE(gain_controller2.get());
}
TEST(GainController2FixedDigital, GainShouldChangeOnSetGain) {
@@ -161,7 +157,7 @@
kInputLevel);
// +20 db should increase signal by a factor of 10.
- agc2_fixed->ApplyConfig(CreateAgc2FixedDigitalModeConfig(kGain20Db));
+ agc2_fixed->SetFixedGainDb(kGain20Db);
EXPECT_FLOAT_EQ(RunAgc2WithConstantInput(*agc2_fixed, kInputLevel, kNumFrames,
kSampleRateHz),
kInputLevel * 10);
@@ -185,51 +181,35 @@
*agc2_fixed, kInputLevel, kNumFrames, kSampleRateHz);
// Increase gain.
- agc2_fixed->ApplyConfig(CreateAgc2FixedDigitalModeConfig(kGainDbHigh));
+ agc2_fixed->SetFixedGainDb(kGainDbHigh);
static_cast<void>(RunAgc2WithConstantInput(*agc2_fixed, kInputLevel,
kNumFrames, kSampleRateHz));
// Back to the lower gain.
- agc2_fixed->ApplyConfig(CreateAgc2FixedDigitalModeConfig(kGainDbLow));
+ agc2_fixed->SetFixedGainDb(kGainDbLow);
const float output_level_post = RunAgc2WithConstantInput(
*agc2_fixed, kInputLevel, kNumFrames, kSampleRateHz);
EXPECT_EQ(output_level_pre, output_level_post);
}
-struct FixedDigitalTestParams {
- FixedDigitalTestParams(float gain_db_min,
- float gain_db_max,
- size_t sample_rate,
- bool saturation_expected)
- : gain_db_min(gain_db_min),
- gain_db_max(gain_db_max),
- sample_rate(sample_rate),
- saturation_expected(saturation_expected) {}
- float gain_db_min;
- float gain_db_max;
- size_t sample_rate;
- bool saturation_expected;
+class FixedDigitalTest
+ : public ::testing::TestWithParam<std::tuple<float, float, int, bool>> {
+ protected:
+ float gain_db_min() const { return std::get<0>(GetParam()); }
+ float gain_db_max() const { return std::get<1>(GetParam()); }
+ int sample_rate_hz() const { return std::get<2>(GetParam()); }
+ bool saturation_expected() const { return std::get<3>(GetParam()); }
};
-class FixedDigitalTest
- : public ::testing::Test,
- public ::testing::WithParamInterface<FixedDigitalTestParams> {};
-
TEST_P(FixedDigitalTest, CheckSaturationBehaviorWithLimiter) {
- const float kInputLevel = 32767.0f;
- const size_t kNumFrames = 5;
-
- const auto params = GetParam();
-
- const auto gains_db =
- test::LinSpace(params.gain_db_min, params.gain_db_max, 10);
- for (const auto gain_db : gains_db) {
- SCOPED_TRACE(std::to_string(gain_db));
- auto agc2_fixed = CreateAgc2FixedDigitalMode(gain_db, params.sample_rate);
- const float processed_sample = RunAgc2WithConstantInput(
- *agc2_fixed, kInputLevel, kNumFrames, params.sample_rate);
- if (params.saturation_expected) {
+ for (const float gain_db : test::LinSpace(gain_db_min(), gain_db_max(), 10)) {
+ SCOPED_TRACE(gain_db);
+ auto agc2_fixed = CreateAgc2FixedDigitalMode(gain_db, sample_rate_hz());
+ const float processed_sample =
+ RunAgc2WithConstantInput(*agc2_fixed, /*input_level=*/32767.0f,
+ /*num_frames=*/5, sample_rate_hz());
+ if (saturation_expected()) {
EXPECT_FLOAT_EQ(processed_sample, 32767.0f);
} else {
EXPECT_LT(processed_sample, 32767.0f);
@@ -244,47 +224,43 @@
::testing::Values(
// When gain < `test::kLimiterMaxInputLevelDbFs`, the limiter will not
// saturate the signal (at any sample rate).
- FixedDigitalTestParams(0.1f,
- test::kLimiterMaxInputLevelDbFs - 0.01f,
- 8000,
- false),
- FixedDigitalTestParams(0.1,
- test::kLimiterMaxInputLevelDbFs - 0.01f,
- 48000,
- false),
+ std::make_tuple(0.1f,
+ test::kLimiterMaxInputLevelDbFs - 0.01f,
+ 8000,
+ false),
+ std::make_tuple(0.1,
+ test::kLimiterMaxInputLevelDbFs - 0.01f,
+ 48000,
+ false),
// When gain > `test::kLimiterMaxInputLevelDbFs`, the limiter will
// saturate the signal (at any sample rate).
- FixedDigitalTestParams(test::kLimiterMaxInputLevelDbFs + 0.01f,
- 10.0f,
- 8000,
- true),
- FixedDigitalTestParams(test::kLimiterMaxInputLevelDbFs + 0.01f,
- 10.0f,
- 48000,
- true)));
+ std::make_tuple(test::kLimiterMaxInputLevelDbFs + 0.01f,
+ 10.0f,
+ 8000,
+ true),
+ std::make_tuple(test::kLimiterMaxInputLevelDbFs + 0.01f,
+ 10.0f,
+ 48000,
+ true)));
// Processes a test audio file and checks that the gain applied at the end of
// the recording is close to the expected value.
TEST(GainController2, CheckFinalGainWithAdaptiveDigitalController) {
- // Create AGC2 enabling only the adaptive digital controller.
- GainController2 agc2;
- AudioProcessing::Config::GainController2 config;
- config.fixed_digital.gain_db = 0.0f;
- config.adaptive_digital.enabled = true;
- agc2.ApplyConfig(config);
-
- // The input audio is a 48k stereo recording.
constexpr int kSampleRateHz = AudioProcessing::kSampleRate48kHz;
constexpr int kStereo = 2;
+
+ // Create AGC2 enabling only the adaptive digital controller.
+ Agc2Config config;
+ config.fixed_digital.gain_db = 0.0f;
+ config.adaptive_digital.enabled = true;
+ GainController2 agc2(config, kSampleRateHz, kStereo);
+
test::InputAudioFile input_file(
test::GetApmCaptureTestVectorFileName(kSampleRateHz),
/*loop_at_end=*/true);
const StreamConfig stream_config(kSampleRateHz, kStereo,
/*has_keyboard=*/false);
- // Initialize AGC2 for the used input.
- agc2.Initialize(kSampleRateHz, kStereo);
-
// Init buffers.
constexpr int kFrameDurationMs = 10;
std::vector<float> frame(kStereo * stream_config.num_frames());
