Add noise suppression settings to AudioProcessing::Config
This Config configuration will eventually replace the AudioProcessing::noise_suppression() interface.
This also introduces a proxy NoiseSuppression, returned by AudioProcessing::noise_suppression.
Without this proxy, ApplyConfig could overwrite NS settings for clients who currently use noise_suppression(). For example, the following code will not preserve the noise suppression level:
apm->noise_suppression()->set_level(NoiseSuppression::kHigh);
auto cfg = apm->GetConfig();
apm->ApplyConfig(cfg);
The NoiseSuppression instance returned by noise_suppression() has no way to update the config inside APM, so GetConfig() will return an out-of-date config which is then re-applied. This CL adds a proxy that makes this update, by forwarding Enable() and set_level() calls to ApplyConfig().
Drive-by change: AudioProcessing::Config substructs are reordered to mirror the capture processing pipeline.
Tested: Ran ToT and this CL builds of audioproc_f and verified identical settings/aecdumps.
Bug: webrtc:9947
Change-Id: I823eade894be115c254d656562564108b2b63b1f
Reviewed-on: https://webrtc-review.googlesource.com/c/116521
Reviewed-by: Alex Loiko <aleloi@webrtc.org>
Commit-Queue: Sam Zackrisson <saza@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26248}diff --git a/modules/audio_processing/BUILD.gn b/modules/audio_processing/BUILD.gn
index 714ad4b..ab5a704 100644
--- a/modules/audio_processing/BUILD.gn
+++ b/modules/audio_processing/BUILD.gn
@@ -134,6 +134,7 @@
":audio_processing_statistics",
":config",
":gain_control_interface",
+ ":noise_suppression_proxy",
"../..:webrtc_common",
"../../api:array_view",
"../../api/audio:aec3_config",
@@ -189,6 +190,17 @@
]
}
+rtc_source_set("noise_suppression_proxy") {
+ sources = [
+ "noise_suppression_proxy.cc",
+ "noise_suppression_proxy.h",
+ ]
+ deps = [
+ ":api",
+ "../../rtc_base:macromagic",
+ ]
+}
+
rtc_source_set("audio_processing_statistics") {
visibility = [ "*" ]
sources = [
diff --git a/modules/audio_processing/audio_processing_impl.cc b/modules/audio_processing/audio_processing_impl.cc
index eec4c33..5f0d676 100644
--- a/modules/audio_processing/audio_processing_impl.cc
+++ b/modules/audio_processing/audio_processing_impl.cc
@@ -34,6 +34,7 @@
#include "modules/audio_processing/logging/apm_data_dumper.h"
#include "modules/audio_processing/low_cut_filter.h"
#include "modules/audio_processing/noise_suppression_impl.h"
+#include "modules/audio_processing/noise_suppression_proxy.h"
#include "modules/audio_processing/residual_echo_detector.h"
#include "modules/audio_processing/transient/transient_suppressor.h"
#include "modules/audio_processing/voice_detection_impl.h"
@@ -107,6 +108,23 @@
return uppermost_native_rate;
}
+NoiseSuppression::Level NsConfigLevelToInterfaceLevel(
+ AudioProcessing::Config::NoiseSuppression::Level level) {
+ using NsConfig = AudioProcessing::Config::NoiseSuppression;
+ switch (level) {
+ case NsConfig::kLow:
+ return NoiseSuppression::kLow;
+ case NsConfig::kModerate:
+ return NoiseSuppression::kModerate;
+ case NsConfig::kHigh:
+ return NoiseSuppression::kHigh;
+ case NsConfig::kVeryHigh:
+ return NoiseSuppression::kVeryHigh;
+ default:
+ RTC_NOTREACHED();
+ }
+}
+
// Maximum lengths that frame of samples being passed from the render side to
// the capture side can have (does not apply to AEC3).
static const size_t kMaxAllowedValuesOfSamplesPerBand = 160;
@@ -232,9 +250,12 @@
struct AudioProcessingImpl::ApmPublicSubmodules {
ApmPublicSubmodules() {}
// Accessed externally of APM without any lock acquired.
+ // TODO(bugs.webrtc.org/9947): Move these submodules into private_submodules_
+ // when their pointer-to-submodule API functions are gone.
std::unique_ptr<GainControlImpl> gain_control;
std::unique_ptr<LevelEstimatorImpl> level_estimator;
std::unique_ptr<NoiseSuppressionImpl> noise_suppression;
+ std::unique_ptr<NoiseSuppressionProxy> noise_suppression_proxy;
std::unique_ptr<VoiceDetectionImpl> voice_detection;
std::unique_ptr<GainControlForExperimentalAgc>
gain_control_for_experimental_agc;
@@ -380,6 +401,8 @@
new LevelEstimatorImpl(&crit_capture_));
public_submodules_->noise_suppression.reset(
new NoiseSuppressionImpl(&crit_capture_));
+ public_submodules_->noise_suppression_proxy.reset(new NoiseSuppressionProxy(
+ this, public_submodules_->noise_suppression.get()));
public_submodules_->voice_detection.reset(
new VoiceDetectionImpl(&crit_capture_));
public_submodules_->gain_control_for_experimental_agc.reset(
@@ -664,6 +687,11 @@
? EchoCancellationImpl::SuppressionLevel::kModerateSuppression
: EchoCancellationImpl::SuppressionLevel::kHighSuppression);
+ public_submodules_->noise_suppression->Enable(
+ config.noise_suppression.enabled);
+ public_submodules_->noise_suppression->set_level(
+ NsConfigLevelToInterfaceLevel(config.noise_suppression.level));
+
InitializeLowCutFilter();
RTC_LOG(LS_INFO) << "Highpass filter activated: "
@@ -1690,7 +1718,7 @@
}
NoiseSuppression* AudioProcessingImpl::noise_suppression() const {
- return public_submodules_->noise_suppression.get();
+ return public_submodules_->noise_suppression_proxy.get();
}
VoiceDetection* AudioProcessingImpl::voice_detection() const {
diff --git a/modules/audio_processing/audio_processing_impl_locking_unittest.cc b/modules/audio_processing/audio_processing_impl_locking_unittest.cc
index 3609124..6d1de62 100644
--- a/modules/audio_processing/audio_processing_impl_locking_unittest.cc
+++ b/modules/audio_processing/audio_processing_impl_locking_unittest.cc
@@ -527,21 +527,20 @@
void AudioProcessingImplLockTest::SetUp() {
test_config_ = static_cast<TestConfig>(GetParam());
- ASSERT_EQ(apm_->kNoError, apm_->level_estimator()->Enable(true));
ASSERT_EQ(apm_->kNoError, apm_->gain_control()->Enable(true));
ASSERT_EQ(apm_->kNoError,
apm_->gain_control()->set_mode(GainControl::kAdaptiveDigital));
ASSERT_EQ(apm_->kNoError, apm_->gain_control()->Enable(true));
- ASSERT_EQ(apm_->kNoError, apm_->noise_suppression()->Enable(true));
- ASSERT_EQ(apm_->kNoError, apm_->voice_detection()->Enable(true));
-
- AudioProcessing::Config apm_config;
+ AudioProcessing::Config apm_config = apm_->GetConfig();
apm_config.echo_canceller.enabled =
(test_config_.aec_type != AecType::AecTurnedOff);
apm_config.echo_canceller.mobile_mode =
(test_config_.aec_type == AecType::BasicWebRtcAecSettingsWithAecMobile);
+ apm_config.noise_suppression.enabled = true;
+ apm_config.voice_detection.enabled = true;
+ apm_config.level_estimation.enabled = true;
apm_->ApplyConfig(apm_config);
Config config;
@@ -584,7 +583,7 @@
EXPECT_FALSE(apm_config.echo_canceller.enabled);
}
EXPECT_TRUE(apm_->gain_control()->is_enabled());
- EXPECT_TRUE(apm_->noise_suppression()->is_enabled());
+ EXPECT_TRUE(apm_config.noise_suppression.enabled);
// The below return values are not testable.
apm_->noise_suppression()->speech_probability();
diff --git a/modules/audio_processing/include/audio_processing.h b/modules/audio_processing/include/audio_processing.h
index b88a86f..afe5f6c 100644
--- a/modules/audio_processing/include/audio_processing.h
+++ b/modules/audio_processing/include/audio_processing.h
@@ -240,6 +240,17 @@
// by changing the default values in the AudioProcessing::Config struct.
// The config is applied by passing the struct to the ApplyConfig method.
struct Config {
+ // Enabled the pre-amplifier. It amplifies the capture signal
+ // before any other processing is done.
+ struct PreAmplifier {
+ bool enabled = false;
+ float fixed_gain_factor = 1.f;
+ } pre_amplifier;
+
+ struct HighPassFilter {
+ bool enabled = false;
+ } high_pass_filter;
+
struct EchoCanceller {
bool enabled = false;
bool mobile_mode = false;
@@ -248,20 +259,17 @@
bool legacy_moderate_suppression_level = false;
} echo_canceller;
- struct ResidualEchoDetector {
- bool enabled = true;
- } residual_echo_detector;
-
- struct HighPassFilter {
+ // Enables background noise suppression.
+ struct NoiseSuppression {
bool enabled = false;
- } high_pass_filter;
+ enum Level { kLow, kModerate, kHigh, kVeryHigh };
+ Level level = kModerate;
+ } noise_suppression;
- // Enabled the pre-amplifier. It amplifies the capture signal
- // before any other processing is done.
- struct PreAmplifier {
+ // Enables reporting of |has_voice| in webrtc::AudioProcessingStats.
+ struct VoiceDetection {
bool enabled = false;
- float fixed_gain_factor = 1.f;
- } pre_amplifier;
+ } voice_detection;
// Enables the next generation AGC functionality. This feature replaces the
// standard methods of gain control in the previous AGC. Enabling this
@@ -283,16 +291,15 @@
} adaptive_digital;
} gain_controller2;
+ struct ResidualEchoDetector {
+ bool enabled = true;
+ } residual_echo_detector;
+
// Enables reporting of |output_rms_dbfs| in webrtc::AudioProcessingStats.
struct LevelEstimation {
bool enabled = false;
} level_estimation;
- // Enables reporting of |has_voice| in webrtc::AudioProcessingStats.
- struct VoiceDetection {
- bool enabled = false;
- } voice_detection;
-
// Explicit copy assignment implementation to avoid issues with memory
// sanitizer complaints in case of self-assignment.
// TODO(peah): Add buildflag to ensure that this is only included for memory
diff --git a/modules/audio_processing/noise_suppression_proxy.cc b/modules/audio_processing/noise_suppression_proxy.cc
new file mode 100644
index 0000000..a83c9b2
--- /dev/null
+++ b/modules/audio_processing/noise_suppression_proxy.cc
@@ -0,0 +1,71 @@
+/*
+ * Copyright (c) 2019 The WebRTC project authors. All Rights Reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include "modules/audio_processing/noise_suppression_proxy.h"
+
+namespace webrtc {
+NoiseSuppressionProxy::NoiseSuppressionProxy(AudioProcessing* apm,
+ NoiseSuppression* ns)
+ : apm_(apm), ns_(ns) {}
+
+NoiseSuppressionProxy::~NoiseSuppressionProxy() {}
+
+int NoiseSuppressionProxy::Enable(bool enable) {
+ AudioProcessing::Config config = apm_->GetConfig();
+ if (config.noise_suppression.enabled != enable) {
+ config.noise_suppression.enabled = enable;
+ apm_->ApplyConfig(config);
+ }
+ return AudioProcessing::kNoError;
+}
+
+bool NoiseSuppressionProxy::is_enabled() const {
+ return ns_->is_enabled();
+}
+
+int NoiseSuppressionProxy::set_level(Level level) {
+ AudioProcessing::Config config = apm_->GetConfig();
+ using NsConfig = AudioProcessing::Config::NoiseSuppression;
+ NsConfig::Level new_level;
+ switch (level) {
+ case NoiseSuppression::kLow:
+ new_level = NsConfig::kLow;
+ break;
+ case NoiseSuppression::kModerate:
+ new_level = NsConfig::kModerate;
+ break;
+ case NoiseSuppression::kHigh:
+ new_level = NsConfig::kHigh;
+ break;
+ case NoiseSuppression::kVeryHigh:
+ new_level = NsConfig::kVeryHigh;
+ break;
+ default:
+ RTC_NOTREACHED();
+ }
+ if (config.noise_suppression.level != new_level) {
+ config.noise_suppression.level = new_level;
+ apm_->ApplyConfig(config);
+ }
+ return AudioProcessing::kNoError;
+}
+
+NoiseSuppression::Level NoiseSuppressionProxy::level() const {
+ return ns_->level();
+}
+
+float NoiseSuppressionProxy::speech_probability() const {
+ return ns_->speech_probability();
+}
+
+std::vector<float> NoiseSuppressionProxy::NoiseEstimate() {
+ return ns_->NoiseEstimate();
+}
+} // namespace webrtc
diff --git a/modules/audio_processing/noise_suppression_proxy.h b/modules/audio_processing/noise_suppression_proxy.h
new file mode 100644
index 0000000..b5afe92
--- /dev/null
+++ b/modules/audio_processing/noise_suppression_proxy.h
@@ -0,0 +1,45 @@
+/*
+ * Copyright (c) 2019 The WebRTC project authors. All Rights Reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+#ifndef MODULES_AUDIO_PROCESSING_NOISE_SUPPRESSION_PROXY_H_
+#define MODULES_AUDIO_PROCESSING_NOISE_SUPPRESSION_PROXY_H_
+
+#include <vector>
+
+#include "modules/audio_processing/include/audio_processing.h"
+#include "rtc_base/constructormagic.h"
+
+namespace webrtc {
+// This class ensures interoperability with the pointer-to-submodule interface
+// AudioProcessing::noise_suppression() and AudioProcessing::ApplyConfig:
+// Enable(..) and set_level(..) calls are applied via
+// AudioProcessing::ApplyConfig, while all other function calls are forwarded
+// directly to a wrapped NoiseSuppression instance.
+class NoiseSuppressionProxy : public NoiseSuppression {
+ public:
+ NoiseSuppressionProxy(AudioProcessing* apm, NoiseSuppression* ns);
+ ~NoiseSuppressionProxy() override;
+
+ // NoiseSuppression implementation.
+ int Enable(bool enable) override;
+ bool is_enabled() const override;
+ int set_level(Level level) override;
+ Level level() const override;
+ float speech_probability() const override;
+ std::vector<float> NoiseEstimate() override;
+
+ private:
+ AudioProcessing* apm_;
+ NoiseSuppression* ns_;
+ RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(NoiseSuppressionProxy);
+};
+} // namespace webrtc
+
+#endif // MODULES_AUDIO_PROCESSING_NOISE_SUPPRESSION_PROXY_H_
diff --git a/modules/audio_processing/test/aec_dump_based_simulator.cc b/modules/audio_processing/test/aec_dump_based_simulator.cc
index c5cff96..230d56b 100644
--- a/modules/audio_processing/test/aec_dump_based_simulator.cc
+++ b/modules/audio_processing/test/aec_dump_based_simulator.cc
@@ -424,8 +424,7 @@
if (msg.has_ns_enabled() || settings_.use_ns) {
bool enable = settings_.use_ns ? *settings_.use_ns : msg.ns_enabled();
- RTC_CHECK_EQ(AudioProcessing::kNoError,
- ap_->noise_suppression()->Enable(enable));
+ apm_config.noise_suppression.enabled = enable;
if (settings_.use_verbose_logging) {
std::cout << " ns_enabled: " << (enable ? "true" : "false")
<< std::endl;
@@ -434,9 +433,8 @@
if (msg.has_ns_level() || settings_.ns_level) {
int level = settings_.ns_level ? *settings_.ns_level : msg.ns_level();
- RTC_CHECK_EQ(AudioProcessing::kNoError,
- ap_->noise_suppression()->set_level(
- static_cast<NoiseSuppression::Level>(level)));
+ apm_config.noise_suppression.level =
+ static_cast<AudioProcessing::Config::NoiseSuppression::Level>(level);
if (settings_.use_verbose_logging) {
std::cout << " ns_level: " << level << std::endl;
}
diff --git a/modules/audio_processing/test/debug_dump_replayer.cc b/modules/audio_processing/test/debug_dump_replayer.cc
index 9c4fdd9..9c1257a 100644
--- a/modules/audio_processing/test/debug_dump_replayer.cc
+++ b/modules/audio_processing/test/debug_dump_replayer.cc
@@ -219,6 +219,26 @@
msg.aec_suppression_level()) ==
EchoCancellationImpl::SuppressionLevel::kModerateSuppression;
+ // HPF configs.
+ RTC_CHECK(msg.has_hpf_enabled());
+ apm_config.high_pass_filter.enabled = msg.hpf_enabled();
+
+ // Preamp configs.
+ RTC_CHECK(msg.has_pre_amplifier_enabled());
+ apm_config.pre_amplifier.enabled = msg.pre_amplifier_enabled();
+ apm_config.pre_amplifier.fixed_gain_factor =
+ msg.pre_amplifier_fixed_gain_factor();
+
+ // NS configs.
+ RTC_CHECK(msg.has_ns_enabled());
+ RTC_CHECK(msg.has_ns_level());
+ apm_config.noise_suppression.enabled = msg.ns_enabled();
+ apm_config.noise_suppression.level =
+ static_cast<AudioProcessing::Config::NoiseSuppression::Level>(
+ msg.ns_level());
+
+ apm_->ApplyConfig(apm_config);
+
// AGC configs.
RTC_CHECK(msg.has_agc_enabled());
RTC_CHECK_EQ(AudioProcessing::kNoError,
@@ -232,27 +252,6 @@
RTC_CHECK(msg.has_agc_limiter_enabled());
RTC_CHECK_EQ(AudioProcessing::kNoError,
apm_->gain_control()->enable_limiter(msg.agc_limiter_enabled()));
-
- // HPF configs.
- RTC_CHECK(msg.has_hpf_enabled());
- apm_config.high_pass_filter.enabled = msg.hpf_enabled();
-
- // NS configs.
- RTC_CHECK(msg.has_ns_enabled());
- RTC_CHECK_EQ(AudioProcessing::kNoError,
- apm_->noise_suppression()->Enable(msg.ns_enabled()));
-
- RTC_CHECK(msg.has_ns_level());
- RTC_CHECK_EQ(AudioProcessing::kNoError,
- apm_->noise_suppression()->set_level(
- static_cast<NoiseSuppression::Level>(msg.ns_level())));
-
- RTC_CHECK(msg.has_pre_amplifier_enabled());
- apm_config.pre_amplifier.enabled = msg.pre_amplifier_enabled();
- apm_config.pre_amplifier.fixed_gain_factor =
- msg.pre_amplifier_fixed_gain_factor();
-
- apm_->ApplyConfig(apm_config);
}
void DebugDumpReplayer::LoadNextMessage() {
diff --git a/modules/audio_processing/test/debug_dump_test.cc b/modules/audio_processing/test/debug_dump_test.cc
index 7d0a2bd..0f3ebff 100644
--- a/modules/audio_processing/test/debug_dump_test.cc
+++ b/modules/audio_processing/test/debug_dump_test.cc
@@ -588,8 +588,9 @@
generator.StartRecording();
generator.Process(100);
- NoiseSuppression* ns = generator.apm()->noise_suppression();
- EXPECT_EQ(AudioProcessing::kNoError, ns->Enable(!ns->is_enabled()));
+ AudioProcessing::Config apm_config = generator.apm()->GetConfig();
+ apm_config.noise_suppression.enabled = !apm_config.noise_suppression.enabled;
+ generator.apm()->ApplyConfig(apm_config);
generator.Process(100);
generator.StopRecording();
diff --git a/test/fuzzers/audio_processing_configs_fuzzer.cc b/test/fuzzers/audio_processing_configs_fuzzer.cc
index d864854..6791071 100644
--- a/test/fuzzers/audio_processing_configs_fuzzer.cc
+++ b/test/fuzzers/audio_processing_configs_fuzzer.cc
@@ -171,12 +171,11 @@
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.voice_detection.enabled = use_vad;
-
apm->ApplyConfig(apm_config);
apm->gain_control()->Enable(use_agc);
- apm->noise_suppression()->Enable(use_ns);
apm->level_estimator()->Enable(use_le);
apm->voice_detection()->Enable(use_vad);
apm->gain_control()->enable_limiter(use_agc_limiter);
