From 1ac4f2a29eef9b406270b4bfd3a31b7f15dccf5e Mon Sep 17 00:00:00 2001 From: Alessio Bazzica Date: Fri, 24 Sep 2021 14:59:30 +0200 Subject: [PATCH] AGC2: Remove unused parameters - `NoiseEstimator` and `LevelEstimator` enums - `vad_probability_attack` - `level_estimator_adjacent_speech_frames_threshold` - `use_saturation_protector` - `gain_applier_adjacent_speech_frames_threshold` - `initial_saturation_margin_db` - `extra_saturation_margin_db` Bug: webrtc:7494 Change-Id: I12e40c8efe2d2126d7597ec18a78cf9d5d39baf2 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/232903 Reviewed-by: Sam Zackrisson Reviewed-by: Jesus de Vicente Pena Commit-Queue: Alessio Bazzica Cr-Commit-Position: refs/heads/main@{#35096} --- modules/audio_processing/agc2/adaptive_agc.cc | 5 -- .../agc2/adaptive_mode_level_estimator.cc | 3 - .../agc2/adaptive_mode_level_estimator.h | 7 +-- modules/audio_processing/gain_controller2.cc | 12 +--- .../gain_controller2_unittest.cc | 58 ------------------- .../include/audio_processing.h | 12 ---- .../test/audio_processing_simulator.cc | 2 - .../test/audio_processing_simulator.h | 2 - .../test/audioproc_float_impl.cc | 31 ---------- .../audio_processing_configs_fuzzer.cc | 14 +---- 10 files changed, 7 insertions(+), 139 deletions(-) diff --git a/modules/audio_processing/agc2/adaptive_agc.cc b/modules/audio_processing/agc2/adaptive_agc.cc index a9f9622d00..0df4a26008 100644 --- a/modules/audio_processing/agc2/adaptive_agc.cc +++ b/modules/audio_processing/agc2/adaptive_agc.cc @@ -22,8 +22,6 @@ namespace { using AdaptiveDigitalConfig = AudioProcessing::Config::GainController2::AdaptiveDigital; -using NoiseEstimatorType = - AudioProcessing::Config::GainController2::NoiseEstimator; // Detects the available CPU features and applies any kill-switches. AvailableCpuFeatures GetAllowedCpuFeatures( @@ -63,9 +61,6 @@ AdaptiveAgc::AdaptiveAgc(ApmDataDumper* apm_data_dumper, RTC_DCHECK(apm_data_dumper); RTC_DCHECK(noise_level_estimator_); RTC_DCHECK(saturation_protector_); - if (!config.use_saturation_protector) { - RTC_LOG(LS_WARNING) << "The saturation protector cannot be disabled."; - } } AdaptiveAgc::~AdaptiveAgc() = default; diff --git a/modules/audio_processing/agc2/adaptive_mode_level_estimator.cc b/modules/audio_processing/agc2/adaptive_mode_level_estimator.cc index 507aa12cb4..ca3279e24f 100644 --- a/modules/audio_processing/agc2/adaptive_mode_level_estimator.cc +++ b/modules/audio_processing/agc2/adaptive_mode_level_estimator.cc @@ -19,9 +19,6 @@ namespace webrtc { namespace { -using LevelEstimatorType = - AudioProcessing::Config::GainController2::LevelEstimator; - float ClampLevelEstimateDbfs(float level_estimate_dbfs) { return rtc::SafeClamp(level_estimate_dbfs, -90.f, 30.f); } diff --git a/modules/audio_processing/agc2/adaptive_mode_level_estimator.h b/modules/audio_processing/agc2/adaptive_mode_level_estimator.h index 6d44938587..e39b6cecd7 100644 --- a/modules/audio_processing/agc2/adaptive_mode_level_estimator.h +++ b/modules/audio_processing/agc2/adaptive_mode_level_estimator.h @@ -47,14 +47,13 @@ class AdaptiveModeLevelEstimator { inline bool operator!=(const LevelEstimatorState& s) const { return !(*this == s); } + // TODO(bugs.webrtc.org/7494): Remove `time_to_confidence_ms` if redundant. + int time_to_confidence_ms; struct Ratio { float numerator; float denominator; float GetRatio() const; - }; - // TODO(crbug.com/webrtc/7494): Remove time_to_confidence_ms if redundant. - int time_to_confidence_ms; - Ratio level_dbfs; + } level_dbfs; }; static_assert(std::is_trivially_copyable::value, ""); diff --git a/modules/audio_processing/gain_controller2.cc b/modules/audio_processing/gain_controller2.cc index 74b63c9432..195044adc2 100644 --- a/modules/audio_processing/gain_controller2.cc +++ b/modules/audio_processing/gain_controller2.cc @@ -106,16 +106,8 @@ bool GainController2::Validate( const auto& fixed = config.fixed_digital; const auto& adaptive = config.adaptive_digital; return fixed.gain_db >= 0.f && fixed.gain_db < 50.f && - adaptive.vad_probability_attack > 0.f && - adaptive.vad_probability_attack <= 1.f && - adaptive.level_estimator_adjacent_speech_frames_threshold >= 1 && - adaptive.initial_saturation_margin_db >= 0.f && - adaptive.initial_saturation_margin_db <= 100.f && - adaptive.extra_saturation_margin_db >= 0.f && - adaptive.extra_saturation_margin_db <= 100.f && - adaptive.gain_applier_adjacent_speech_frames_threshold >= 1 && - adaptive.max_gain_change_db_per_second > 0.f && - adaptive.max_output_noise_level_dbfs <= 0.f; + adaptive.max_gain_change_db_per_second > 0.0f && + adaptive.max_output_noise_level_dbfs <= 0.0f; } } // namespace webrtc diff --git a/modules/audio_processing/gain_controller2_unittest.cc b/modules/audio_processing/gain_controller2_unittest.cc index 8f65a89cde..a4a6462576 100644 --- a/modules/audio_processing/gain_controller2_unittest.cc +++ b/modules/audio_processing/gain_controller2_unittest.cc @@ -129,64 +129,6 @@ TEST(GainController2, CheckFixedDigitalConfig) { EXPECT_TRUE(GainController2::Validate(config)); } -TEST(GainController2, CheckAdaptiveDigitalVadProbabilityAttackConfig) { - AudioProcessing::Config::GainController2 config; - // Reject invalid attack. - config.adaptive_digital.vad_probability_attack = -123.f; - EXPECT_FALSE(GainController2::Validate(config)); - config.adaptive_digital.vad_probability_attack = 0.f; - EXPECT_FALSE(GainController2::Validate(config)); - config.adaptive_digital.vad_probability_attack = 42.f; - EXPECT_FALSE(GainController2::Validate(config)); - // Accept valid attack. - config.adaptive_digital.vad_probability_attack = 0.1f; - EXPECT_TRUE(GainController2::Validate(config)); - config.adaptive_digital.vad_probability_attack = 1.f; - EXPECT_TRUE(GainController2::Validate(config)); -} - -TEST(GainController2, - CheckAdaptiveDigitalLevelEstimatorSpeechFramesThresholdConfig) { - AudioProcessing::Config::GainController2 config; - config.adaptive_digital.level_estimator_adjacent_speech_frames_threshold = 0; - EXPECT_FALSE(GainController2::Validate(config)); - config.adaptive_digital.level_estimator_adjacent_speech_frames_threshold = 1; - EXPECT_TRUE(GainController2::Validate(config)); - config.adaptive_digital.level_estimator_adjacent_speech_frames_threshold = 7; - EXPECT_TRUE(GainController2::Validate(config)); -} - -TEST(GainController2, CheckAdaptiveDigitalInitialSaturationMarginConfig) { - AudioProcessing::Config::GainController2 config; - config.adaptive_digital.initial_saturation_margin_db = -1.f; - EXPECT_FALSE(GainController2::Validate(config)); - config.adaptive_digital.initial_saturation_margin_db = 0.f; - EXPECT_TRUE(GainController2::Validate(config)); - config.adaptive_digital.initial_saturation_margin_db = 50.f; - EXPECT_TRUE(GainController2::Validate(config)); -} - -TEST(GainController2, CheckAdaptiveDigitalExtraSaturationMarginConfig) { - AudioProcessing::Config::GainController2 config; - config.adaptive_digital.extra_saturation_margin_db = -1.f; - EXPECT_FALSE(GainController2::Validate(config)); - config.adaptive_digital.extra_saturation_margin_db = 0.f; - EXPECT_TRUE(GainController2::Validate(config)); - config.adaptive_digital.extra_saturation_margin_db = 50.f; - EXPECT_TRUE(GainController2::Validate(config)); -} - -TEST(GainController2, - CheckAdaptiveDigitalGainApplierSpeechFramesThresholdConfig) { - AudioProcessing::Config::GainController2 config; - config.adaptive_digital.gain_applier_adjacent_speech_frames_threshold = 0; - EXPECT_FALSE(GainController2::Validate(config)); - config.adaptive_digital.gain_applier_adjacent_speech_frames_threshold = 1; - EXPECT_TRUE(GainController2::Validate(config)); - config.adaptive_digital.gain_applier_adjacent_speech_frames_threshold = 7; - EXPECT_TRUE(GainController2::Validate(config)); -} - TEST(GainController2, CheckAdaptiveDigitalMaxGainChangeSpeedConfig) { AudioProcessing::Config::GainController2 config; config.adaptive_digital.max_gain_change_db_per_second = -1.f; diff --git a/modules/audio_processing/include/audio_processing.h b/modules/audio_processing/include/audio_processing.h index 15dcc39e0e..8f07c6e3b7 100644 --- a/modules/audio_processing/include/audio_processing.h +++ b/modules/audio_processing/include/audio_processing.h @@ -356,9 +356,6 @@ class RTC_EXPORT AudioProcessing : public rtc::RefCountInterface { return !(*this == rhs); } - // TODO(crbug.com/webrtc/7494): Remove `LevelEstimator`. - enum LevelEstimator { kRms, kPeak }; - enum NoiseEstimator { kStationaryNoise, kNoiseFloor }; bool enabled = false; struct FixedDigital { float gain_db = 0.0f; @@ -379,15 +376,6 @@ class RTC_EXPORT AudioProcessing : public rtc::RefCountInterface { bool sse2_allowed = true; bool avx2_allowed = true; bool neon_allowed = true; - // TODO(crbug.com/webrtc/7494): Remove deprecated settings below. - NoiseEstimator noise_estimator = kNoiseFloor; - float vad_probability_attack = 1.0f; - LevelEstimator level_estimator = kRms; - int level_estimator_adjacent_speech_frames_threshold = 12; - bool use_saturation_protector = true; - float initial_saturation_margin_db = 25.0f; - float extra_saturation_margin_db = 5.0f; - int gain_applier_adjacent_speech_frames_threshold = 12; } adaptive_digital; } gain_controller2; diff --git a/modules/audio_processing/test/audio_processing_simulator.cc b/modules/audio_processing/test/audio_processing_simulator.cc index c61110fff0..7a48c0f318 100644 --- a/modules/audio_processing/test/audio_processing_simulator.cc +++ b/modules/audio_processing/test/audio_processing_simulator.cc @@ -487,8 +487,6 @@ void AudioProcessingSimulator::ConfigureAudioProcessor() { if (settings_.agc2_use_adaptive_gain) { apm_config.gain_controller2.adaptive_digital.enabled = *settings_.agc2_use_adaptive_gain; - apm_config.gain_controller2.adaptive_digital.level_estimator = - settings_.agc2_adaptive_level_estimator; } } if (settings_.use_pre_amplifier) { diff --git a/modules/audio_processing/test/audio_processing_simulator.h b/modules/audio_processing/test/audio_processing_simulator.h index 9539e58b1b..4a11230c66 100644 --- a/modules/audio_processing/test/audio_processing_simulator.h +++ b/modules/audio_processing/test/audio_processing_simulator.h @@ -115,8 +115,6 @@ struct SimulationSettings { absl::optional agc_compression_gain; absl::optional agc2_use_adaptive_gain; absl::optional agc2_fixed_gain_db; - AudioProcessing::Config::GainController2::LevelEstimator - agc2_adaptive_level_estimator; absl::optional pre_amplifier_gain_factor; absl::optional pre_gain_factor; absl::optional post_gain_factor; diff --git a/modules/audio_processing/test/audioproc_float_impl.cc b/modules/audio_processing/test/audioproc_float_impl.cc index 1fc39bb6b9..7ece5d3471 100644 --- a/modules/audio_processing/test/audioproc_float_impl.cc +++ b/modules/audio_processing/test/audioproc_float_impl.cc @@ -159,10 +159,6 @@ ABSL_FLAG(float, agc2_fixed_gain_db, kParameterNotSpecifiedValue, "AGC2 fixed gain (dB) to apply"); -ABSL_FLAG(std::string, - agc2_adaptive_level_estimator, - "RMS", - "AGC2 adaptive digital level estimator to use [RMS, peak]"); ABSL_FLAG(float, pre_amplifier_gain_factor, kParameterNotSpecifiedValue, @@ -341,10 +337,6 @@ const char kUsageDescription[] = "processing module, either based on wav files or " "protobuf debug dump recordings.\n"; -std::vector GetAgc2AdaptiveLevelEstimatorNames() { - return {"RMS", "peak"}; -} - void SetSettingIfSpecified(const std::string& value, absl::optional* parameter) { if (value.compare("") != 0) { @@ -374,27 +366,6 @@ void SetSettingIfFlagSet(int32_t flag, absl::optional* parameter) { } } -AudioProcessing::Config::GainController2::LevelEstimator -MapAgc2AdaptiveLevelEstimator(absl::string_view name) { - if (name.compare("RMS") == 0) { - return AudioProcessing::Config::GainController2::LevelEstimator::kRms; - } - if (name.compare("peak") == 0) { - return AudioProcessing::Config::GainController2::LevelEstimator::kPeak; - } - auto concat_strings = - [](const std::vector& strings) -> std::string { - rtc::StringBuilder ss; - for (const auto& s : strings) { - ss << " " << s; - } - return ss.Release(); - }; - RTC_CHECK(false) - << "Invalid value for agc2_adaptive_level_estimator, valid options:" - << concat_strings(GetAgc2AdaptiveLevelEstimatorNames()) << "."; -} - SimulationSettings CreateSettings() { SimulationSettings settings; if (absl::GetFlag(FLAGS_all_default)) { @@ -467,8 +438,6 @@ SimulationSettings CreateSettings() { SetSettingIfSpecified(absl::GetFlag(FLAGS_agc2_fixed_gain_db), &settings.agc2_fixed_gain_db); - settings.agc2_adaptive_level_estimator = MapAgc2AdaptiveLevelEstimator( - absl::GetFlag(FLAGS_agc2_adaptive_level_estimator)); SetSettingIfSpecified(absl::GetFlag(FLAGS_pre_amplifier_gain_factor), &settings.pre_amplifier_gain_factor); SetSettingIfSpecified(absl::GetFlag(FLAGS_pre_gain_factor), diff --git a/test/fuzzers/audio_processing_configs_fuzzer.cc b/test/fuzzers/audio_processing_configs_fuzzer.cc index cb2f7a8bf4..9bed5c1691 100644 --- a/test/fuzzers/audio_processing_configs_fuzzer.cc +++ b/test/fuzzers/audio_processing_configs_fuzzer.cc @@ -76,10 +76,8 @@ rtc::scoped_refptr CreateApm(test::FuzzDataHelper* fuzz_data, field_trial::InitFieldTrialsFromString(field_trial_string->c_str()); bool use_agc2_adaptive_digital = fuzz_data->ReadOrDefaultValue(true); - bool use_agc2_adaptive_digital_rms_estimator = - fuzz_data->ReadOrDefaultValue(true); - bool use_agc2_adaptive_digital_saturation_protector = - fuzz_data->ReadOrDefaultValue(true); + static_cast(fuzz_data->ReadOrDefaultValue(true)); + static_cast(fuzz_data->ReadOrDefaultValue(true)); // Ignore a few bytes. Bytes from this segment will be used for // future config flag changes. We assume 40 bytes is enough for @@ -123,14 +121,6 @@ rtc::scoped_refptr CreateApm(test::FuzzDataHelper* fuzz_data, apm_config.gain_controller2.fixed_digital.gain_db = gain_controller2_gain_db; apm_config.gain_controller2.adaptive_digital.enabled = use_agc2_adaptive_digital; - apm_config.gain_controller2.adaptive_digital.level_estimator = - use_agc2_adaptive_digital_rms_estimator - ? webrtc::AudioProcessing::Config::GainController2::LevelEstimator:: - kRms - : webrtc::AudioProcessing::Config::GainController2::LevelEstimator:: - kPeak; - apm_config.gain_controller2.adaptive_digital.use_saturation_protector = - use_agc2_adaptive_digital_saturation_protector; apm_config.noise_suppression.enabled = use_ns; apm_config.transient_suppression.enabled = use_ts; apm_config.voice_detection.enabled = use_vad;