From 0c83e15c6b614d44f7bd545d518d02c681dedd65 Mon Sep 17 00:00:00 2001 From: Alessio Bazzica Date: Wed, 14 Oct 2020 12:49:54 +0200 Subject: [PATCH] Move AGC2 config ToString to the right place and update Validate() The APM config to string mapping must be in one place (namely, in `audio_processing.cc`). This CL moves the AGC2 config to string impl to the right place. This CL also updates `GainController2::Validate()` and adds the missing unit tests for the parameters that have recently been added. Stack buffer size in `AudioProcessing::Config::ToString()` increased because of the extra params. Syntax near `multi_channel_capture` fixed. Output string format verified with a JS linter. Bug: webrtc:7494 Change-Id: I692e1549b7d40c970d88a14c8e83da16325fb54c Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/187080 Commit-Queue: Alessio Bazzica Reviewed-by: Minyue Li Cr-Commit-Position: refs/heads/master@{#32400} --- .../audio_processing/audio_processing_impl.cc | 6 +- modules/audio_processing/gain_controller2.cc | 65 +++------- modules/audio_processing/gain_controller2.h | 2 - .../gain_controller2_unittest.cc | 117 ++++++++++++++---- .../include/audio_processing.cc | 31 +++-- 5 files changed, 129 insertions(+), 92 deletions(-) diff --git a/modules/audio_processing/audio_processing_impl.cc b/modules/audio_processing/audio_processing_impl.cc index 67208dfadd..3b66ee532c 100644 --- a/modules/audio_processing/audio_processing_impl.cc +++ b/modules/audio_processing/audio_processing_impl.cc @@ -611,10 +611,8 @@ void AudioProcessingImpl::ApplyConfig(const AudioProcessing::Config& config) { const bool config_ok = GainController2::Validate(config_.gain_controller2); if (!config_ok) { - RTC_LOG(LS_ERROR) << "AudioProcessing module config error\n" - "Gain Controller 2: " - << GainController2::ToString(config_.gain_controller2) - << "\nReverting to default parameter set"; + RTC_LOG(LS_ERROR) + << "Invalid Gain Controller 2 config; using the default config."; config_.gain_controller2 = AudioProcessing::Config::GainController2(); } diff --git a/modules/audio_processing/gain_controller2.cc b/modules/audio_processing/gain_controller2.cc index 8e71d4020e..6561bebc6e 100644 --- a/modules/audio_processing/gain_controller2.cc +++ b/modules/audio_processing/gain_controller2.cc @@ -65,8 +65,7 @@ void GainController2::NotifyAnalogLevel(int level) { void GainController2::ApplyConfig( const AudioProcessing::Config::GainController2& config) { - RTC_DCHECK(Validate(config)) - << " the invalid config was " << ToString(config); + RTC_DCHECK(Validate(config)); config_ = config; if (config.fixed_digital.gain_db != config_.fixed_digital.gain_db) { @@ -84,55 +83,19 @@ void GainController2::ApplyConfig( bool GainController2::Validate( const AudioProcessing::Config::GainController2& config) { - return config.fixed_digital.gain_db >= 0.f && - config.fixed_digital.gain_db < 50.f && - config.adaptive_digital.extra_saturation_margin_db >= 0.f && - config.adaptive_digital.extra_saturation_margin_db <= 100.f; -} - -std::string GainController2::ToString( - const AudioProcessing::Config::GainController2& config) { - rtc::StringBuilder ss; - std::string adaptive_digital_level_estimator; - using LevelEstimatorType = - AudioProcessing::Config::GainController2::LevelEstimator; - switch (config.adaptive_digital.level_estimator) { - case LevelEstimatorType::kRms: - adaptive_digital_level_estimator = "RMS"; - break; - case LevelEstimatorType::kPeak: - adaptive_digital_level_estimator = "peak"; - break; - } - // clang-format off - // clang formatting doesn't respect custom nested style. - ss << "{" - "enabled: " << (config.enabled ? "true" : "false") << ", " - "fixed_digital: {gain_db: " << config.fixed_digital.gain_db << "}, " - "adaptive_digital: {" - "enabled: " - << (config.adaptive_digital.enabled ? "true" : "false") << ", " - "level_estimator: {" - "type: " << adaptive_digital_level_estimator << ", " - "adjacent_speech_frames_threshold: " - << config.adaptive_digital - .level_estimator_adjacent_speech_frames_threshold << ", " - "initial_saturation_margin_db: " - << config.adaptive_digital.initial_saturation_margin_db << ", " - "extra_saturation_margin_db: " - << config.adaptive_digital.extra_saturation_margin_db << "}, " - "gain_applier: {" - "adjacent_speech_frames_threshold: " - << config.adaptive_digital - .gain_applier_adjacent_speech_frames_threshold << ", " - "max_gain_change_db_per_second: " - << config.adaptive_digital.max_gain_change_db_per_second << ", " - "max_output_noise_level_dbfs: " - << config.adaptive_digital.max_output_noise_level_dbfs << "}" - "}" - "}"; - // clang-format on - return ss.Release(); + 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; } } // namespace webrtc diff --git a/modules/audio_processing/gain_controller2.h b/modules/audio_processing/gain_controller2.h index 7ed310ebf6..da27fdcc62 100644 --- a/modules/audio_processing/gain_controller2.h +++ b/modules/audio_processing/gain_controller2.h @@ -38,8 +38,6 @@ class GainController2 { void ApplyConfig(const AudioProcessing::Config::GainController2& config); static bool Validate(const AudioProcessing::Config::GainController2& config); - static std::string ToString( - const AudioProcessing::Config::GainController2& config); private: static int instance_count_; diff --git a/modules/audio_processing/gain_controller2_unittest.cc b/modules/audio_processing/gain_controller2_unittest.cc index 9369a8a8d9..09bad5087d 100644 --- a/modules/audio_processing/gain_controller2_unittest.cc +++ b/modules/audio_processing/gain_controller2_unittest.cc @@ -104,36 +104,107 @@ float GainAfterProcessingFile(GainController2* gain_controller) { } // namespace -TEST(GainController2, CreateApplyConfig) { - // Instances GainController2 and applies different configurations. - std::unique_ptr gain_controller2(new GainController2()); - - // Check that the default config is valid. +TEST(GainController2, CheckDefaultConfig) { AudioProcessing::Config::GainController2 config; EXPECT_TRUE(GainController2::Validate(config)); - gain_controller2->ApplyConfig(config); - - // Check that attenuation is not allowed. - config.fixed_digital.gain_db = -5.f; - EXPECT_FALSE(GainController2::Validate(config)); - - // Check that valid configurations are applied. - for (const float& fixed_gain_db : {0.f, 5.f, 10.f, 40.f}) { - config.fixed_digital.gain_db = fixed_gain_db; - EXPECT_TRUE(GainController2::Validate(config)); - gain_controller2->ApplyConfig(config); - } } -TEST(GainController2, ToString) { - // Tests GainController2::ToString(). Only test the enabled property. +TEST(GainController2, CheckFixedDigitalConfig) { AudioProcessing::Config::GainController2 config; + // Attenuation is not allowed. + config.fixed_digital.gain_db = -5.f; + EXPECT_FALSE(GainController2::Validate(config)); + // No gain is allowed. + config.fixed_digital.gain_db = 0.f; + EXPECT_TRUE(GainController2::Validate(config)); + // Positive gain is allowed. + config.fixed_digital.gain_db = 15.f; + EXPECT_TRUE(GainController2::Validate(config)); +} - config.enabled = false; - EXPECT_EQ("{enabled: false", GainController2::ToString(config).substr(0, 15)); +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)); +} - config.enabled = true; - EXPECT_EQ("{enabled: true", GainController2::ToString(config).substr(0, 14)); +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; + EXPECT_FALSE(GainController2::Validate(config)); + config.adaptive_digital.max_gain_change_db_per_second = 0.f; + EXPECT_FALSE(GainController2::Validate(config)); + config.adaptive_digital.max_gain_change_db_per_second = 5.f; + EXPECT_TRUE(GainController2::Validate(config)); +} + +TEST(GainController2, CheckAdaptiveDigitalMaxOutputNoiseLevelConfig) { + AudioProcessing::Config::GainController2 config; + config.adaptive_digital.max_output_noise_level_dbfs = 5.f; + EXPECT_FALSE(GainController2::Validate(config)); + config.adaptive_digital.max_output_noise_level_dbfs = 0.f; + EXPECT_TRUE(GainController2::Validate(config)); + config.adaptive_digital.max_output_noise_level_dbfs = -5.f; + EXPECT_TRUE(GainController2::Validate(config)); +} + +// Checks that the default config is applied. +TEST(GainController2, ApplyDefaultConfig) { + auto gain_controller2 = std::make_unique(); + AudioProcessing::Config::GainController2 config; + gain_controller2->ApplyConfig(config); } TEST(GainController2FixedDigital, GainShouldChangeOnSetGain) { diff --git a/modules/audio_processing/include/audio_processing.cc b/modules/audio_processing/include/audio_processing.cc index 88544159a4..30e16e490b 100644 --- a/modules/audio_processing/include/audio_processing.cc +++ b/modules/audio_processing/include/audio_processing.cc @@ -71,19 +71,15 @@ AudioProcessing::Config::Pipeline::Pipeline() : maximum_internal_processing_rate(GetDefaultMaxInternalRate()) {} std::string AudioProcessing::Config::ToString() const { - char buf[1024]; + char buf[2048]; rtc::SimpleStringBuilder builder(buf); builder << "AudioProcessing::Config{ " "pipeline: {" "maximum_internal_processing_rate: " << pipeline.maximum_internal_processing_rate << ", multi_channel_render: " << pipeline.multi_channel_render - << ", " - ", multi_channel_capture: " - << pipeline.multi_channel_capture - << "}, " - "pre_amplifier: { enabled: " - << pre_amplifier.enabled + << ", multi_channel_capture: " << pipeline.multi_channel_capture + << "}, pre_amplifier: { enabled: " << pre_amplifier.enabled << ", fixed_gain_factor: " << pre_amplifier.fixed_gain_factor << " }, high_pass_filter: { enabled: " << high_pass_filter.enabled << " }, echo_canceller: { enabled: " << echo_canceller.enabled @@ -106,18 +102,29 @@ std::string AudioProcessing::Config::ToString() const { << " }, gain_controller2: { enabled: " << gain_controller2.enabled << ", fixed_digital: { gain_db: " << gain_controller2.fixed_digital.gain_db - << " }, adaptive_digital: { enabled: " - << gain_controller2.adaptive_digital.enabled << ", level_estimator: " + << "}, adaptive_digital: { enabled: " + << gain_controller2.adaptive_digital.enabled + << ", level_estimator: { type: " << GainController2LevelEstimatorToString( gain_controller2.adaptive_digital.level_estimator) - << ", use_saturation_protector: " - << gain_controller2.adaptive_digital.use_saturation_protector + << ", adjacent_speech_frames_threshold: " + << gain_controller2.adaptive_digital + .level_estimator_adjacent_speech_frames_threshold + << ", initial_saturation_margin_db: " + << gain_controller2.adaptive_digital.initial_saturation_margin_db << ", extra_saturation_margin_db: " << gain_controller2.adaptive_digital.extra_saturation_margin_db + << "}, gain_applier: { adjacent_speech_frames_threshold: " + << gain_controller2.adaptive_digital + .gain_applier_adjacent_speech_frames_threshold + << ", max_gain_change_db_per_second: " + << gain_controller2.adaptive_digital.max_gain_change_db_per_second + << ", max_output_noise_level_dbfs: " + << gain_controller2.adaptive_digital.max_output_noise_level_dbfs << " } }, residual_echo_detector: { enabled: " << residual_echo_detector.enabled << " }, level_estimation: { enabled: " << level_estimation.enabled - << " } }"; + << " }}}"; return builder.str(); }