AGC2 config change detection fixed
AGC2 is correctly (re)initialized when its config changes.
This CL also improves the `AudioProcessingImpl::ApplyConfig`
readability by defining operator!= also for the AGC1 config.
Bug: webrtc:7494
Change-Id: I62068de32c941e6b18d4618c656f569647042345
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/187120
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32402}
diff --git a/modules/audio_processing/audio_processing_impl.cc b/modules/audio_processing/audio_processing_impl.cc
index 3b66ee5..849a0feb 100644
--- a/modules/audio_processing/audio_processing_impl.cc
+++ b/modules/audio_processing/audio_processing_impl.cc
@@ -125,6 +125,7 @@
// TODO(peah): Decrease this once we properly handle hugely unbalanced
// reverse and forward call numbers.
static const size_t kMaxNumFramesToBuffer = 100;
+
} // namespace
// Throughout webrtc, it's assumed that success is represented by zero.
@@ -545,34 +546,10 @@
config_.echo_canceller.mobile_mode != config.echo_canceller.mobile_mode;
const bool agc1_config_changed =
- config_.gain_controller1.enabled != config.gain_controller1.enabled ||
- config_.gain_controller1.mode != config.gain_controller1.mode ||
- config_.gain_controller1.target_level_dbfs !=
- config.gain_controller1.target_level_dbfs ||
- config_.gain_controller1.compression_gain_db !=
- config.gain_controller1.compression_gain_db ||
- config_.gain_controller1.enable_limiter !=
- config.gain_controller1.enable_limiter ||
- config_.gain_controller1.analog_level_minimum !=
- config.gain_controller1.analog_level_minimum ||
- config_.gain_controller1.analog_level_maximum !=
- config.gain_controller1.analog_level_maximum ||
- config_.gain_controller1.analog_gain_controller.enabled !=
- config.gain_controller1.analog_gain_controller.enabled ||
- config_.gain_controller1.analog_gain_controller.startup_min_volume !=
- config.gain_controller1.analog_gain_controller.startup_min_volume ||
- config_.gain_controller1.analog_gain_controller.clipped_level_min !=
- config.gain_controller1.analog_gain_controller.clipped_level_min ||
- config_.gain_controller1.analog_gain_controller
- .enable_agc2_level_estimator !=
- config.gain_controller1.analog_gain_controller
- .enable_agc2_level_estimator ||
- config_.gain_controller1.analog_gain_controller.enable_digital_adaptive !=
- config.gain_controller1.analog_gain_controller
- .enable_digital_adaptive;
+ config_.gain_controller1 != config.gain_controller1;
const bool agc2_config_changed =
- config_.gain_controller2.enabled != config.gain_controller2.enabled;
+ config_.gain_controller2 != config.gain_controller2;
const bool voice_detection_config_changed =
config_.voice_detection.enabled != config.voice_detection.enabled;
diff --git a/modules/audio_processing/audio_processing_unittest.cc b/modules/audio_processing/audio_processing_unittest.cc
index bd18d4d..545c780 100644
--- a/modules/audio_processing/audio_processing_unittest.cc
+++ b/modules/audio_processing/audio_processing_unittest.cc
@@ -2417,6 +2417,10 @@
}
}
+constexpr void Toggle(bool& b) {
+ b ^= true;
+}
+
} // namespace
TEST(RuntimeSettingTest, TestDefaultCtor) {
@@ -2879,4 +2883,246 @@
SUCCEED(); // Real success is absence of defects from asan/msan/ubsan.
}
+TEST(AudioProcessing, GainController1ConfigEqual) {
+ AudioProcessing::Config::GainController1 a;
+ AudioProcessing::Config::GainController1 b;
+ EXPECT_EQ(a, b);
+
+ Toggle(a.enabled);
+ b.enabled = a.enabled;
+ EXPECT_EQ(a, b);
+
+ a.mode = AudioProcessing::Config::GainController1::Mode::kAdaptiveDigital;
+ b.mode = a.mode;
+ EXPECT_EQ(a, b);
+
+ a.target_level_dbfs++;
+ b.target_level_dbfs = a.target_level_dbfs;
+ EXPECT_EQ(a, b);
+
+ a.compression_gain_db++;
+ b.compression_gain_db = a.compression_gain_db;
+ EXPECT_EQ(a, b);
+
+ Toggle(a.enable_limiter);
+ b.enable_limiter = a.enable_limiter;
+ EXPECT_EQ(a, b);
+
+ a.analog_level_minimum++;
+ b.analog_level_minimum = a.analog_level_minimum;
+ EXPECT_EQ(a, b);
+
+ a.analog_level_maximum--;
+ b.analog_level_maximum = a.analog_level_maximum;
+ EXPECT_EQ(a, b);
+
+ auto& a_analog = a.analog_gain_controller;
+ auto& b_analog = b.analog_gain_controller;
+
+ Toggle(a_analog.enabled);
+ b_analog.enabled = a_analog.enabled;
+ EXPECT_EQ(a, b);
+
+ a_analog.startup_min_volume++;
+ b_analog.startup_min_volume = a_analog.startup_min_volume;
+ EXPECT_EQ(a, b);
+
+ a_analog.clipped_level_min++;
+ b_analog.clipped_level_min = a_analog.clipped_level_min;
+ EXPECT_EQ(a, b);
+
+ Toggle(a_analog.enable_agc2_level_estimator);
+ b_analog.enable_agc2_level_estimator = a_analog.enable_agc2_level_estimator;
+ EXPECT_EQ(a, b);
+
+ Toggle(a_analog.enable_digital_adaptive);
+ b_analog.enable_digital_adaptive = a_analog.enable_digital_adaptive;
+ EXPECT_EQ(a, b);
+}
+
+// Checks that one differing parameter is sufficient to make two configs
+// different.
+TEST(AudioProcessing, GainController1ConfigNotEqual) {
+ AudioProcessing::Config::GainController1 a;
+ const AudioProcessing::Config::GainController1 b;
+
+ Toggle(a.enabled);
+ EXPECT_NE(a, b);
+ a.enabled = b.enabled;
+
+ a.mode = AudioProcessing::Config::GainController1::Mode::kAdaptiveDigital;
+ EXPECT_NE(a, b);
+ a.mode = b.mode;
+
+ a.target_level_dbfs++;
+ EXPECT_NE(a, b);
+ a.target_level_dbfs = b.target_level_dbfs;
+
+ a.compression_gain_db++;
+ EXPECT_NE(a, b);
+ a.compression_gain_db = b.compression_gain_db;
+
+ Toggle(a.enable_limiter);
+ EXPECT_NE(a, b);
+ a.enable_limiter = b.enable_limiter;
+
+ a.analog_level_minimum++;
+ EXPECT_NE(a, b);
+ a.analog_level_minimum = b.analog_level_minimum;
+
+ a.analog_level_maximum--;
+ EXPECT_NE(a, b);
+ a.analog_level_maximum = b.analog_level_maximum;
+
+ auto& a_analog = a.analog_gain_controller;
+ const auto& b_analog = b.analog_gain_controller;
+
+ Toggle(a_analog.enabled);
+ EXPECT_NE(a, b);
+ a_analog.enabled = b_analog.enabled;
+
+ a_analog.startup_min_volume++;
+ EXPECT_NE(a, b);
+ a_analog.startup_min_volume = b_analog.startup_min_volume;
+
+ a_analog.clipped_level_min++;
+ EXPECT_NE(a, b);
+ a_analog.clipped_level_min = b_analog.clipped_level_min;
+
+ Toggle(a_analog.enable_agc2_level_estimator);
+ EXPECT_NE(a, b);
+ a_analog.enable_agc2_level_estimator = b_analog.enable_agc2_level_estimator;
+
+ Toggle(a_analog.enable_digital_adaptive);
+ EXPECT_NE(a, b);
+ a_analog.enable_digital_adaptive = b_analog.enable_digital_adaptive;
+}
+
+TEST(AudioProcessing, GainController2ConfigEqual) {
+ AudioProcessing::Config::GainController2 a;
+ AudioProcessing::Config::GainController2 b;
+ EXPECT_EQ(a, b);
+
+ Toggle(a.enabled);
+ b.enabled = a.enabled;
+ EXPECT_EQ(a, b);
+
+ a.fixed_digital.gain_db += 1.f;
+ b.fixed_digital.gain_db = a.fixed_digital.gain_db;
+ EXPECT_EQ(a, b);
+
+ auto& a_adaptive = a.adaptive_digital;
+ auto& b_adaptive = b.adaptive_digital;
+
+ Toggle(a_adaptive.enabled);
+ b_adaptive.enabled = a_adaptive.enabled;
+ EXPECT_EQ(a, b);
+
+ a_adaptive.vad_probability_attack += 1.f;
+ b_adaptive.vad_probability_attack = a_adaptive.vad_probability_attack;
+ EXPECT_EQ(a, b);
+
+ a_adaptive.level_estimator =
+ AudioProcessing::Config::GainController2::LevelEstimator::kPeak;
+ b_adaptive.level_estimator = a_adaptive.level_estimator;
+ EXPECT_EQ(a, b);
+
+ a_adaptive.level_estimator_adjacent_speech_frames_threshold++;
+ b_adaptive.level_estimator_adjacent_speech_frames_threshold =
+ a_adaptive.level_estimator_adjacent_speech_frames_threshold;
+ EXPECT_EQ(a, b);
+
+ Toggle(a_adaptive.use_saturation_protector);
+ b_adaptive.use_saturation_protector = a_adaptive.use_saturation_protector;
+ EXPECT_EQ(a, b);
+
+ a_adaptive.initial_saturation_margin_db += 1.f;
+ b_adaptive.initial_saturation_margin_db =
+ a_adaptive.initial_saturation_margin_db;
+ EXPECT_EQ(a, b);
+
+ a_adaptive.extra_saturation_margin_db += 1.f;
+ b_adaptive.extra_saturation_margin_db = a_adaptive.extra_saturation_margin_db;
+ EXPECT_EQ(a, b);
+
+ a_adaptive.gain_applier_adjacent_speech_frames_threshold++;
+ b_adaptive.gain_applier_adjacent_speech_frames_threshold =
+ a_adaptive.gain_applier_adjacent_speech_frames_threshold;
+ EXPECT_EQ(a, b);
+
+ a_adaptive.max_gain_change_db_per_second += 1.f;
+ b_adaptive.max_gain_change_db_per_second =
+ a_adaptive.max_gain_change_db_per_second;
+ EXPECT_EQ(a, b);
+
+ a_adaptive.max_output_noise_level_dbfs -= 1.f;
+ b_adaptive.max_output_noise_level_dbfs =
+ a_adaptive.max_output_noise_level_dbfs;
+ EXPECT_EQ(a, b);
+}
+
+// Checks that one differing parameter is sufficient to make two configs
+// different.
+TEST(AudioProcessing, GainController2ConfigNotEqual) {
+ AudioProcessing::Config::GainController2 a;
+ const AudioProcessing::Config::GainController2 b;
+
+ Toggle(a.enabled);
+ EXPECT_NE(a, b);
+ a.enabled = b.enabled;
+
+ a.fixed_digital.gain_db += 1.f;
+ EXPECT_NE(a, b);
+ a.fixed_digital.gain_db = b.fixed_digital.gain_db;
+
+ auto& a_adaptive = a.adaptive_digital;
+ const auto& b_adaptive = b.adaptive_digital;
+
+ Toggle(a_adaptive.enabled);
+ EXPECT_NE(a, b);
+ a_adaptive.enabled = b_adaptive.enabled;
+
+ a_adaptive.vad_probability_attack += 1.f;
+ EXPECT_NE(a, b);
+ a_adaptive.vad_probability_attack = b_adaptive.vad_probability_attack;
+
+ a_adaptive.level_estimator =
+ AudioProcessing::Config::GainController2::LevelEstimator::kPeak;
+ EXPECT_NE(a, b);
+ a_adaptive.level_estimator = b_adaptive.level_estimator;
+
+ a_adaptive.level_estimator_adjacent_speech_frames_threshold++;
+ EXPECT_NE(a, b);
+ a_adaptive.level_estimator_adjacent_speech_frames_threshold =
+ b_adaptive.level_estimator_adjacent_speech_frames_threshold;
+
+ Toggle(a_adaptive.use_saturation_protector);
+ EXPECT_NE(a, b);
+ a_adaptive.use_saturation_protector = b_adaptive.use_saturation_protector;
+
+ a_adaptive.initial_saturation_margin_db += 1.f;
+ EXPECT_NE(a, b);
+ a_adaptive.initial_saturation_margin_db =
+ b_adaptive.initial_saturation_margin_db;
+
+ a_adaptive.extra_saturation_margin_db += 1.f;
+ EXPECT_NE(a, b);
+ a_adaptive.extra_saturation_margin_db = b_adaptive.extra_saturation_margin_db;
+
+ a_adaptive.gain_applier_adjacent_speech_frames_threshold++;
+ EXPECT_NE(a, b);
+ a_adaptive.gain_applier_adjacent_speech_frames_threshold =
+ b_adaptive.gain_applier_adjacent_speech_frames_threshold;
+
+ a_adaptive.max_gain_change_db_per_second += 1.f;
+ EXPECT_NE(a, b);
+ a_adaptive.max_gain_change_db_per_second =
+ b_adaptive.max_gain_change_db_per_second;
+
+ a_adaptive.max_output_noise_level_dbfs -= 1.f;
+ EXPECT_NE(a, b);
+ a_adaptive.max_output_noise_level_dbfs =
+ b_adaptive.max_output_noise_level_dbfs;
+}
+
} // namespace webrtc
diff --git a/modules/audio_processing/include/audio_processing.cc b/modules/audio_processing/include/audio_processing.cc
index 30e16e4..3e5f6ed 100644
--- a/modules/audio_processing/include/audio_processing.cc
+++ b/modules/audio_processing/include/audio_processing.cc
@@ -16,6 +16,9 @@
namespace webrtc {
namespace {
+using Agc1Config = AudioProcessing::Config::GainController1;
+using Agc2Config = AudioProcessing::Config::GainController2;
+
std::string NoiseSuppressionLevelToString(
const AudioProcessing::Config::NoiseSuppression::Level& level) {
switch (level) {
@@ -30,24 +33,23 @@
}
}
-std::string GainController1ModeToString(
- const AudioProcessing::Config::GainController1::Mode& mode) {
+std::string GainController1ModeToString(const Agc1Config::Mode& mode) {
switch (mode) {
- case AudioProcessing::Config::GainController1::Mode::kAdaptiveAnalog:
+ case Agc1Config::Mode::kAdaptiveAnalog:
return "AdaptiveAnalog";
- case AudioProcessing::Config::GainController1::Mode::kAdaptiveDigital:
+ case Agc1Config::Mode::kAdaptiveDigital:
return "AdaptiveDigital";
- case AudioProcessing::Config::GainController1::Mode::kFixedDigital:
+ case Agc1Config::Mode::kFixedDigital:
return "FixedDigital";
}
}
std::string GainController2LevelEstimatorToString(
- const AudioProcessing::Config::GainController2::LevelEstimator& level) {
+ const Agc2Config::LevelEstimator& level) {
switch (level) {
- case AudioProcessing::Config::GainController2::LevelEstimator::kRms:
+ case Agc2Config::LevelEstimator::kRms:
return "Rms";
- case AudioProcessing::Config::GainController2::LevelEstimator::kPeak:
+ case Agc2Config::LevelEstimator::kPeak:
return "Peak";
}
}
@@ -70,6 +72,50 @@
AudioProcessing::Config::Pipeline::Pipeline()
: maximum_internal_processing_rate(GetDefaultMaxInternalRate()) {}
+bool Agc1Config::operator==(const Agc1Config& rhs) const {
+ const auto& analog_lhs = analog_gain_controller;
+ const auto& analog_rhs = rhs.analog_gain_controller;
+ return enabled == rhs.enabled && mode == rhs.mode &&
+ target_level_dbfs == rhs.target_level_dbfs &&
+ compression_gain_db == rhs.compression_gain_db &&
+ enable_limiter == rhs.enable_limiter &&
+ analog_level_minimum == rhs.analog_level_minimum &&
+ analog_level_maximum == rhs.analog_level_maximum &&
+ analog_lhs.enabled == analog_rhs.enabled &&
+ analog_lhs.startup_min_volume == analog_rhs.startup_min_volume &&
+ analog_lhs.clipped_level_min == analog_rhs.clipped_level_min &&
+ analog_lhs.enable_agc2_level_estimator ==
+ analog_rhs.enable_agc2_level_estimator &&
+ analog_lhs.enable_digital_adaptive ==
+ analog_rhs.enable_digital_adaptive;
+}
+
+bool Agc2Config::operator==(const Agc2Config& rhs) const {
+ const auto& adaptive_lhs = adaptive_digital;
+ const auto& adaptive_rhs = rhs.adaptive_digital;
+
+ return enabled == rhs.enabled &&
+ fixed_digital.gain_db == rhs.fixed_digital.gain_db &&
+ adaptive_lhs.enabled == adaptive_rhs.enabled &&
+ adaptive_lhs.vad_probability_attack ==
+ adaptive_rhs.vad_probability_attack &&
+ adaptive_lhs.level_estimator == adaptive_rhs.level_estimator &&
+ adaptive_lhs.level_estimator_adjacent_speech_frames_threshold ==
+ adaptive_rhs.level_estimator_adjacent_speech_frames_threshold &&
+ adaptive_lhs.use_saturation_protector ==
+ adaptive_rhs.use_saturation_protector &&
+ adaptive_lhs.initial_saturation_margin_db ==
+ adaptive_rhs.initial_saturation_margin_db &&
+ adaptive_lhs.extra_saturation_margin_db ==
+ adaptive_rhs.extra_saturation_margin_db &&
+ adaptive_lhs.gain_applier_adjacent_speech_frames_threshold ==
+ adaptive_rhs.gain_applier_adjacent_speech_frames_threshold &&
+ adaptive_lhs.max_gain_change_db_per_second ==
+ adaptive_rhs.max_gain_change_db_per_second &&
+ adaptive_lhs.max_output_noise_level_dbfs ==
+ adaptive_rhs.max_output_noise_level_dbfs;
+}
+
std::string AudioProcessing::Config::ToString() const {
char buf[2048];
rtc::SimpleStringBuilder builder(buf);
diff --git a/modules/audio_processing/include/audio_processing.h b/modules/audio_processing/include/audio_processing.h
index d09e2ba..e85ac0c 100644
--- a/modules/audio_processing/include/audio_processing.h
+++ b/modules/audio_processing/include/audio_processing.h
@@ -274,6 +274,11 @@
// HAL.
// Recommended to be enabled on the client-side.
struct GainController1 {
+ bool operator==(const GainController1& rhs) const;
+ bool operator!=(const GainController1& rhs) const {
+ return !(*this == rhs);
+ }
+
bool enabled = false;
enum Mode {
// Adaptive mode intended for use if an analog volume control is
@@ -338,6 +343,11 @@
// first applies a fixed gain. The adaptive digital AGC can be turned off by
// setting |adaptive_digital_mode=false|.
struct GainController2 {
+ bool operator==(const GainController2& rhs) const;
+ bool operator!=(const GainController2& rhs) const {
+ return !(*this == rhs);
+ }
+
enum LevelEstimator { kRms, kPeak };
bool enabled = false;
struct {
