APM: Make echo detector an optionally compilable and injectable component

Important: This change does not in any way affect echo cancellation or standardized stats. The user audio experience is unchanged. Only non-standard stats are affected. Echo return loss metrics are unchanged. Residual echo likelihood {recent max} will no longer be computed by default.

Important: The echo detector is no longer enabled by default.

API change, PSA: https://groups.google.com/g/discuss-webrtc/c/mJV5cDysBDI/m/7PTPBjVHCgAJ

This CL removes the default usage of the residual echo detector in APM.
It can now only be used via injection and the helper function webrtc::CreateEchoDetector. See how the function audio_processing_unittest.cc:CreateApm() changed, for an example.

The echo detector implementation is marked poisonous, to avoid accidental dependencies.

Some cleanup is done:
- EchoDetector::PackRenderAudioBuffer is declared in one target but is defined in another target. It is not necessary to keep in the API. It is made an implementation detail, and the echo detector input is documented in the API.
- The internal state of APM is large and difficult to track. Submodule pointers that are set permanently on construction are now appropriately marked const.

Tested:
- existing + new unit tests
- audioproc_f is bitexact on a large number of aecdumps

Bug: webrtc:11539
Change-Id: I00cc2ee112fedb06451a533409311605220064d0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/239652
Reviewed-by: Ivo Creusen <ivoc@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Sam Zackrisson <saza@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35550}

BUILD.gn

@@ -721,6 +721,9 @@ group("poison_audio_codecs") {
 group("poison_default_task_queue") {
 }
 
+group("poison_default_echo_detector") {
+}
+
 group("poison_rtc_json") {
 }
 

api/audio/BUILD.gn

@@ -95,6 +95,7 @@ rtc_source_set("echo_control") {
 
 rtc_source_set("echo_detector_creator") {
   visibility = [ "*" ]
+  allow_poison = [ "default_echo_detector" ]
   sources = [
     "echo_detector_creator.cc",
     "echo_detector_creator.h",
@@ -102,7 +103,7 @@ rtc_source_set("echo_detector_creator") {
   deps = [
     "../../api:scoped_refptr",
     "../../modules/audio_processing:api",
-    "../../modules/audio_processing:audio_processing",
+    "../../modules/audio_processing:residual_echo_detector",
     "../../rtc_base:refcount",
   ]
 }

api/audio_options.h

@@ -64,8 +64,11 @@ struct RTC_EXPORT AudioOptions {
   absl::optional<bool> typing_detection;
   absl::optional<bool> experimental_agc;
   absl::optional<bool> experimental_ns;
-  // Note that tx_agc_* only applies to non-experimental AGC.
+  // TODO(bugs.webrtc.org/11539): Deprecated, replaced by
+  // webrtc::CreateEchoDetector() and injection when creating the audio
+  // processing module.
   absl::optional<bool> residual_echo_detector;
+  // Note that tx_agc_* only applies to non-experimental AGC.
   absl::optional<uint16_t> tx_agc_target_dbov;
   absl::optional<uint16_t> tx_agc_digital_compression_gain;
   absl::optional<bool> tx_agc_limiter;

media/engine/webrtc_voice_engine.cc

@@ -411,7 +411,6 @@ void WebRtcVoiceEngine::Init() {
     options.audio_jitter_buffer_min_delay_ms = 0;
     options.audio_jitter_buffer_enable_rtx_handling = false;
     options.experimental_agc = false;
-    options.residual_echo_detector = true;
     bool error = ApplyOptions(options);
     RTC_DCHECK(error);
   }
@@ -625,10 +624,6 @@ bool WebRtcVoiceEngine::ApplyOptions(const AudioOptions& options_in) {
     apm_config.high_pass_filter.enabled = *options.highpass_filter;
   }
 
-  if (options.residual_echo_detector) {
-    apm_config.residual_echo_detector.enabled = *options.residual_echo_detector;
-  }
-
   if (options.noise_suppression) {
     const bool enabled = *options.noise_suppression;
     apm_config.noise_suppression.enabled = enabled;

modules/audio_processing/BUILD.gn

@@ -149,19 +149,9 @@ rtc_library("audio_processing") {
     "common.h",
     "echo_control_mobile_impl.cc",
     "echo_control_mobile_impl.h",
-    "echo_detector/circular_buffer.cc",
-    "echo_detector/circular_buffer.h",
-    "echo_detector/mean_variance_estimator.cc",
-    "echo_detector/mean_variance_estimator.h",
-    "echo_detector/moving_max.cc",
-    "echo_detector/moving_max.h",
-    "echo_detector/normalized_covariance_estimator.cc",
-    "echo_detector/normalized_covariance_estimator.h",
     "gain_control_impl.cc",
     "gain_control_impl.h",
     "render_queue_item_verifier.h",
-    "residual_echo_detector.cc",
-    "residual_echo_detector.h",
     "typing_detection.cc",
     "typing_detection.h",
   ]
@@ -243,6 +233,33 @@ rtc_library("voice_detection") {
   ]
 }
 
+rtc_library("residual_echo_detector") {
+  poisonous = [ "default_echo_detector" ]
+  configs += [ ":apm_debug_dump" ]
+  sources = [
+    "echo_detector/circular_buffer.cc",
+    "echo_detector/circular_buffer.h",
+    "echo_detector/mean_variance_estimator.cc",
+    "echo_detector/mean_variance_estimator.h",
+    "echo_detector/moving_max.cc",
+    "echo_detector/moving_max.h",
+    "echo_detector/normalized_covariance_estimator.cc",
+    "echo_detector/normalized_covariance_estimator.h",
+    "residual_echo_detector.cc",
+    "residual_echo_detector.h",
+  ]
+  deps = [
+    ":api",
+    ":apm_logging",
+    "../../api:array_view",
+    "../../rtc_base:checks",
+    "../../rtc_base:logging",
+    "../../rtc_base:rtc_base_approved",
+    "../../system_wrappers:metrics",
+  ]
+  absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ]
+}
+
 rtc_library("optionally_built_submodule_creators") {
   sources = [
     "optionally_built_submodule_creators.cc",
@@ -368,6 +385,7 @@ if (rtc_include_tests) {
         "../../api:scoped_refptr",
         "../../api/audio:aec3_config",
         "../../api/audio:aec3_factory",
+        "../../api/audio:echo_detector_creator",
         "../../common_audio",
         "../../common_audio:common_audio_c",
         "../../rtc_base",
@@ -425,6 +443,7 @@ if (rtc_include_tests) {
           ":audioproc_test_utils",
           ":audioproc_unittest_proto",
           ":optionally_built_submodule_creators",
+          ":residual_echo_detector",
           ":rms_level",
           ":runtime_settings_protobuf_utils",
           "../../api/audio:audio_frame_api",
@@ -524,6 +543,7 @@ if (rtc_include_tests) {
         ":runtime_settings_protobuf_utils",
         "../../api/audio:aec3_config_json",
         "../../api/audio:aec3_factory",
+        "../../api/audio:echo_detector_creator",
         "../../common_audio",
         "../../rtc_base:checks",
         "../../rtc_base:ignore_wundef",

modules/audio_processing/audio_processing_impl.cc

@@ -129,6 +129,13 @@ static const size_t kMaxAllowedValuesOfSamplesPerFrame = 480;
 // reverse and forward call numbers.
 static const size_t kMaxNumFramesToBuffer = 100;
 
+void PackRenderAudioBufferForEchoDetector(const AudioBuffer& audio,
+                                          std::vector<float>& packed_buffer) {
+  packed_buffer.clear();
+  packed_buffer.insert(packed_buffer.end(), audio.channels_const()[0],
+                       audio.channels_const()[0] + audio.num_frames());
+}
+
 }  // namespace
 
 // Throughout webrtc, it's assumed that success is represented by zero.
@@ -145,7 +152,6 @@ AudioProcessingImpl::SubmoduleStates::SubmoduleStates(
 bool AudioProcessingImpl::SubmoduleStates::Update(
     bool high_pass_filter_enabled,
     bool mobile_echo_controller_enabled,
-    bool residual_echo_detector_enabled,
     bool noise_suppressor_enabled,
     bool adaptive_gain_controller_enabled,
     bool gain_controller2_enabled,
@@ -157,8 +163,6 @@ bool AudioProcessingImpl::SubmoduleStates::Update(
   changed |= (high_pass_filter_enabled != high_pass_filter_enabled_);
   changed |=
       (mobile_echo_controller_enabled != mobile_echo_controller_enabled_);
-  changed |=
-      (residual_echo_detector_enabled != residual_echo_detector_enabled_);
   changed |= (noise_suppressor_enabled != noise_suppressor_enabled_);
   changed |=
       (adaptive_gain_controller_enabled != adaptive_gain_controller_enabled_);
@@ -170,7 +174,6 @@ bool AudioProcessingImpl::SubmoduleStates::Update(
   if (changed) {
     high_pass_filter_enabled_ = high_pass_filter_enabled;
     mobile_echo_controller_enabled_ = mobile_echo_controller_enabled;
-    residual_echo_detector_enabled_ = residual_echo_detector_enabled;
     noise_suppressor_enabled_ = noise_suppressor_enabled;
     adaptive_gain_controller_enabled_ = adaptive_gain_controller_enabled;
     gain_controller2_enabled_ = gain_controller2_enabled;
@@ -296,11 +299,6 @@ AudioProcessingImpl::AudioProcessingImpl(
   capture_nonlocked_.echo_controller_enabled =
       static_cast<bool>(echo_control_factory_);
 
-  // If no echo detector is injected, use the ResidualEchoDetector.
-  if (!submodules_.echo_detector) {
-    submodules_.echo_detector = rtc::make_ref_counted<ResidualEchoDetector>();
-  }
-
   Initialize();
 }
 
@@ -969,8 +967,9 @@ void AudioProcessingImpl::QueueBandedRenderAudio(AudioBuffer* audio) {
 }
 
 void AudioProcessingImpl::QueueNonbandedRenderAudio(AudioBuffer* audio) {
-  ResidualEchoDetector::PackRenderAudioBuffer(audio, &red_render_queue_buffer_);
+  if (submodules_.echo_detector) {
-
+    PackRenderAudioBufferForEchoDetector(*audio, red_render_queue_buffer_);
+    RTC_DCHECK(red_render_signal_queue_);
     // Insert the samples into the queue.
     if (!red_render_signal_queue_->Insert(&red_render_queue_buffer_)) {
       // The data queue is full and needs to be emptied.
@@ -980,6 +979,7 @@ void AudioProcessingImpl::QueueNonbandedRenderAudio(AudioBuffer* audio) {
       bool result = red_render_signal_queue_->Insert(&red_render_queue_buffer_);
       RTC_DCHECK(result);
     }
+  }
 }
 
 void AudioProcessingImpl::AllocateRenderQueue() {
@@ -1011,9 +1011,11 @@ void AudioProcessingImpl::AllocateRenderQueue() {
     agc_render_signal_queue_->Clear();
   }
 
+  if (submodules_.echo_detector) {
     if (red_render_queue_element_max_size_ <
         new_red_render_queue_element_max_size) {
-    red_render_queue_element_max_size_ = new_red_render_queue_element_max_size;
+      red_render_queue_element_max_size_ =
+          new_red_render_queue_element_max_size;
 
       std::vector<float> template_queue_element(
           red_render_queue_element_max_size_);
@@ -1029,6 +1031,7 @@ void AudioProcessingImpl::AllocateRenderQueue() {
     } else {
       red_render_signal_queue_->Clear();
     }
+  }
 }
 
 void AudioProcessingImpl::EmptyQueuedRenderAudio() {
@@ -1051,10 +1054,11 @@ void AudioProcessingImpl::EmptyQueuedRenderAudioLocked() {
     }
   }
 
+  if (submodules_.echo_detector) {
     while (red_render_signal_queue_->Remove(&red_capture_queue_buffer_)) {
-    RTC_DCHECK(submodules_.echo_detector);
       submodules_.echo_detector->AnalyzeRenderAudio(red_capture_queue_buffer_);
     }
+  }
 }
 
 int AudioProcessingImpl::ProcessStream(const int16_t* const src,
@@ -1289,8 +1293,7 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() {
       capture_buffer = capture_.capture_fullband_audio.get();
     }
 
-    if (config_.residual_echo_detector.enabled) {
+    if (submodules_.echo_detector) {
-      RTC_DCHECK(submodules_.echo_detector);
       submodules_.echo_detector->AnalyzeCaptureAudio(
           rtc::ArrayView<const float>(capture_buffer->channels()[0],
                                       capture_buffer->num_frames()));
@@ -1348,8 +1351,7 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() {
     }
 
     // Compute echo-detector stats.
-    if (config_.residual_echo_detector.enabled) {
+    if (submodules_.echo_detector) {
-      RTC_DCHECK(submodules_.echo_detector);
       auto ed_metrics = submodules_.echo_detector->GetMetrics();
       capture_.stats.residual_echo_likelihood = ed_metrics.echo_likelihood;
       capture_.stats.residual_echo_likelihood_recent_max =
@@ -1714,8 +1716,8 @@ AudioProcessing::Config AudioProcessingImpl::GetConfig() const {
 bool AudioProcessingImpl::UpdateActiveSubmoduleStates() {
   return submodule_states_.Update(
       config_.high_pass_filter.enabled, !!submodules_.echo_control_mobile,
-      config_.residual_echo_detector.enabled, !!submodules_.noise_suppressor,
+      !!submodules_.noise_suppressor, !!submodules_.gain_control,
-      !!submodules_.gain_control, !!submodules_.gain_controller2,
+      !!submodules_.gain_controller2,
       config_.pre_amplifier.enabled || config_.capture_level_adjustment.enabled,
       capture_nonlocked_.echo_controller_enabled,
       config_.voice_detection.enabled, !!submodules_.transient_suppressor);
@@ -1991,10 +1993,11 @@ void AudioProcessingImpl::InitializeCaptureLevelsAdjuster() {
 }
 
 void AudioProcessingImpl::InitializeResidualEchoDetector() {
-  RTC_DCHECK(submodules_.echo_detector);
+  if (submodules_.echo_detector) {
     submodules_.echo_detector->Initialize(
         proc_fullband_sample_rate_hz(), 1,
         formats_.render_processing_format.sample_rate_hz(), 1);
+  }
 }
 
 void AudioProcessingImpl::InitializeAnalyzer() {

modules/audio_processing/audio_processing_impl.h

@@ -36,7 +36,6 @@
 #include "modules/audio_processing/ns/noise_suppressor.h"
 #include "modules/audio_processing/optionally_built_submodule_creators.h"
 #include "modules/audio_processing/render_queue_item_verifier.h"
-#include "modules/audio_processing/residual_echo_detector.h"
 #include "modules/audio_processing/rms_level.h"
 #include "modules/audio_processing/transient/transient_suppressor.h"
 #include "modules/audio_processing/voice_detection.h"
@@ -182,7 +181,7 @@ class AudioProcessingImpl : public AudioProcessing {
     SwapQueue<RuntimeSetting>& runtime_settings_;
   };
 
-  std::unique_ptr<ApmDataDumper> data_dumper_;
+  const std::unique_ptr<ApmDataDumper> data_dumper_;
   static int instance_count_;
   const bool use_setup_specific_default_aec3_config_;
 
@@ -195,7 +194,7 @@ class AudioProcessingImpl : public AudioProcessing {
   RuntimeSettingEnqueuer render_runtime_settings_enqueuer_;
 
   // EchoControl factory.
-  std::unique_ptr<EchoControlFactory> echo_control_factory_;
+  const std::unique_ptr<EchoControlFactory> echo_control_factory_;
 
   class SubmoduleStates {
    public:
@@ -205,7 +204,6 @@ class AudioProcessingImpl : public AudioProcessing {
     // Updates the submodule state and returns true if it has changed.
     bool Update(bool high_pass_filter_enabled,
                 bool mobile_echo_controller_enabled,
-                bool residual_echo_detector_enabled,
                 bool noise_suppressor_enabled,
                 bool adaptive_gain_controller_enabled,
                 bool gain_controller2_enabled,
@@ -229,7 +227,6 @@ class AudioProcessingImpl : public AudioProcessing {
     const bool capture_analyzer_enabled_ = false;
     bool high_pass_filter_enabled_ = false;
     bool mobile_echo_controller_enabled_ = false;
-    bool residual_echo_detector_enabled_ = false;
     bool noise_suppressor_enabled_ = false;
     bool adaptive_gain_controller_enabled_ = false;
     bool gain_controller2_enabled_ = false;
@@ -392,18 +389,18 @@ class AudioProcessingImpl : public AudioProcessing {
           render_pre_processor(std::move(render_pre_processor)),
           capture_analyzer(std::move(capture_analyzer)) {}
     // Accessed internally from capture or during initialization.
+    const rtc::scoped_refptr<EchoDetector> echo_detector;
+    const std::unique_ptr<CustomProcessing> capture_post_processor;
+    const std::unique_ptr<CustomProcessing> render_pre_processor;
+    const std::unique_ptr<CustomAudioAnalyzer> capture_analyzer;
     std::unique_ptr<AgcManagerDirect> agc_manager;
     std::unique_ptr<GainControlImpl> gain_control;
     std::unique_ptr<GainController2> gain_controller2;
     std::unique_ptr<HighPassFilter> high_pass_filter;
-    rtc::scoped_refptr<EchoDetector> echo_detector;
     std::unique_ptr<EchoControl> echo_controller;
     std::unique_ptr<EchoControlMobileImpl> echo_control_mobile;
     std::unique_ptr<NoiseSuppressor> noise_suppressor;
     std::unique_ptr<TransientSuppressor> transient_suppressor;
-    std::unique_ptr<CustomProcessing> capture_post_processor;
-    std::unique_ptr<CustomProcessing> render_pre_processor;
-    std::unique_ptr<CustomAudioAnalyzer> capture_analyzer;
     std::unique_ptr<VoiceDetection> voice_detector;
     std::unique_ptr<CaptureLevelsAdjuster> capture_levels_adjuster;
   } submodules_;

modules/audio_processing/audio_processing_impl_unittest.cc

@@ -554,7 +554,6 @@ TEST(AudioProcessingImplTest, RenderPreProcessorBeforeEchoDetector) {
           .Create();
   webrtc::AudioProcessing::Config apm_config;
   apm_config.pre_amplifier.enabled = true;
-  apm_config.residual_echo_detector.enabled = true;
   apm->ApplyConfig(apm_config);
 
   constexpr int16_t kAudioLevel = 1000;

modules/audio_processing/audio_processing_unittest.cc

@@ -20,6 +20,7 @@
 #include <queue>
 
 #include "absl/flags/flag.h"
+#include "api/audio/echo_detector_creator.h"
 #include "common_audio/include/audio_util.h"
 #include "common_audio/resampler/include/push_resampler.h"
 #include "common_audio/resampler/push_sinc_resampler.h"
@@ -415,8 +416,8 @@ class ApmTest : public ::testing::Test {
   rtc::scoped_refptr<AudioProcessing> apm_;
   Int16FrameData frame_;
   Int16FrameData revframe_;
-  std::unique_ptr<ChannelBuffer<float> > float_cb_;
+  std::unique_ptr<ChannelBuffer<float>> float_cb_;
-  std::unique_ptr<ChannelBuffer<float> > revfloat_cb_;
+  std::unique_ptr<ChannelBuffer<float>> revfloat_cb_;
   int output_sample_rate_hz_;
   size_t num_output_channels_;
   FILE* far_file_;
@@ -1736,7 +1737,9 @@ TEST_F(ApmTest, Process) {
     if (test->num_input_channels() != test->num_output_channels())
       continue;
 
-    apm_ = AudioProcessingBuilderForTesting().Create();
+    apm_ = AudioProcessingBuilderForTesting()
+               .SetEchoDetector(CreateEchoDetector())
+               .Create();
     AudioProcessing::Config apm_config = apm_->GetConfig();
     apm_config.gain_controller1.analog_gain_controller.enabled = false;
     apm_->ApplyConfig(apm_config);
@@ -2649,9 +2652,75 @@ TEST(ApmConfiguration, EchoControlInjection) {
                      audio.data.data());
 }
 
-rtc::scoped_refptr<AudioProcessing> CreateApm(bool mobile_aec) {
+TEST(ApmConfiguration, EchoDetectorInjection) {
+  using ::testing::_;
+  rtc::scoped_refptr<test::MockEchoDetector> mock_echo_detector =
+      rtc::make_ref_counted<::testing::StrictMock<test::MockEchoDetector>>();
+  EXPECT_CALL(*mock_echo_detector,
+              Initialize(/*capture_sample_rate_hz=*/16000, _,
+                         /*render_sample_rate_hz=*/16000, _))
+      .Times(1);
   rtc::scoped_refptr<AudioProcessing> apm =
-      AudioProcessingBuilderForTesting().Create();
+      AudioProcessingBuilderForTesting()
+          .SetEchoDetector(mock_echo_detector)
+          .Create();
+
+  // The echo detector is included in processing when enabled.
+  EXPECT_CALL(*mock_echo_detector, AnalyzeRenderAudio(_))
+      .WillOnce([](rtc::ArrayView<const float> render_audio) {
+        EXPECT_EQ(render_audio.size(), 160u);
+      });
+  EXPECT_CALL(*mock_echo_detector, AnalyzeCaptureAudio(_))
+      .WillOnce([](rtc::ArrayView<const float> capture_audio) {
+        EXPECT_EQ(capture_audio.size(), 160u);
+      });
+  EXPECT_CALL(*mock_echo_detector, GetMetrics()).Times(1);
+
+  Int16FrameData frame;
+  frame.num_channels = 1;
+  SetFrameSampleRate(&frame, 16000);
+
+  apm->ProcessReverseStream(frame.data.data(), StreamConfig(16000, 1),
+                            StreamConfig(16000, 1), frame.data.data());
+  apm->ProcessStream(frame.data.data(), StreamConfig(16000, 1),
+                     StreamConfig(16000, 1), frame.data.data());
+
+  // When processing rates change, the echo detector is also reinitialized to
+  // match those.
+  EXPECT_CALL(*mock_echo_detector,
+              Initialize(/*capture_sample_rate_hz=*/48000, _,
+                         /*render_sample_rate_hz=*/16000, _))
+      .Times(1);
+  EXPECT_CALL(*mock_echo_detector,
+              Initialize(/*capture_sample_rate_hz=*/48000, _,
+                         /*render_sample_rate_hz=*/48000, _))
+      .Times(1);
+  EXPECT_CALL(*mock_echo_detector, AnalyzeRenderAudio(_))
+      .WillOnce([](rtc::ArrayView<const float> render_audio) {
+        EXPECT_EQ(render_audio.size(), 480u);
+      });
+  EXPECT_CALL(*mock_echo_detector, AnalyzeCaptureAudio(_))
+      .Times(2)
+      .WillRepeatedly([](rtc::ArrayView<const float> capture_audio) {
+        EXPECT_EQ(capture_audio.size(), 480u);
+      });
+  EXPECT_CALL(*mock_echo_detector, GetMetrics()).Times(2);
+
+  SetFrameSampleRate(&frame, 48000);
+  apm->ProcessStream(frame.data.data(), StreamConfig(48000, 1),
+                     StreamConfig(48000, 1), frame.data.data());
+  apm->ProcessReverseStream(frame.data.data(), StreamConfig(48000, 1),
+                            StreamConfig(48000, 1), frame.data.data());
+  apm->ProcessStream(frame.data.data(), StreamConfig(48000, 1),
+                     StreamConfig(48000, 1), frame.data.data());
+}
+
+rtc::scoped_refptr<AudioProcessing> CreateApm(bool mobile_aec) {
+  // Enable residual echo detection, for stats.
+  rtc::scoped_refptr<AudioProcessing> apm =
+      AudioProcessingBuilderForTesting()
+          .SetEchoDetector(CreateEchoDetector())
+          .Create();
   if (!apm) {
     return apm;
   }
@@ -2663,9 +2732,8 @@ rtc::scoped_refptr<AudioProcessing> CreateApm(bool mobile_aec) {
     return nullptr;
   }
 
-  // Disable all components except for an AEC and the residual echo detector.
+  // Disable all components except for an AEC.
   AudioProcessing::Config apm_config;
-  apm_config.residual_echo_detector.enabled = true;
   apm_config.high_pass_filter.enabled = false;
   apm_config.gain_controller1.enabled = false;
   apm_config.gain_controller2.enabled = false;
@@ -2722,15 +2790,15 @@ TEST(MAYBE_ApmStatistics, AECEnabledTest) {
   // Test statistics interface.
   AudioProcessingStats stats = apm->GetStatistics();
   // We expect all statistics to be set and have a sensible value.
-  ASSERT_TRUE(stats.residual_echo_likelihood);
+  ASSERT_TRUE(stats.residual_echo_likelihood.has_value());
   EXPECT_GE(*stats.residual_echo_likelihood, 0.0);
   EXPECT_LE(*stats.residual_echo_likelihood, 1.0);
-  ASSERT_TRUE(stats.residual_echo_likelihood_recent_max);
+  ASSERT_TRUE(stats.residual_echo_likelihood_recent_max.has_value());
   EXPECT_GE(*stats.residual_echo_likelihood_recent_max, 0.0);
   EXPECT_LE(*stats.residual_echo_likelihood_recent_max, 1.0);
-  ASSERT_TRUE(stats.echo_return_loss);
+  ASSERT_TRUE(stats.echo_return_loss.has_value());
   EXPECT_NE(*stats.echo_return_loss, -100.0);
-  ASSERT_TRUE(stats.echo_return_loss_enhancement);
+  ASSERT_TRUE(stats.echo_return_loss_enhancement.has_value());
   EXPECT_NE(*stats.echo_return_loss_enhancement, -100.0);
 }
 
@@ -2771,18 +2839,14 @@ TEST(MAYBE_ApmStatistics, AECMEnabledTest) {
   AudioProcessingStats stats = apm->GetStatistics();
   // We expect only the residual echo detector statistics to be set and have a
   // sensible value.
-  EXPECT_TRUE(stats.residual_echo_likelihood);
+  ASSERT_TRUE(stats.residual_echo_likelihood.has_value());
-  if (stats.residual_echo_likelihood) {
   EXPECT_GE(*stats.residual_echo_likelihood, 0.0);
   EXPECT_LE(*stats.residual_echo_likelihood, 1.0);
-  }
+  ASSERT_TRUE(stats.residual_echo_likelihood_recent_max.has_value());
-  EXPECT_TRUE(stats.residual_echo_likelihood_recent_max);
-  if (stats.residual_echo_likelihood_recent_max) {
   EXPECT_GE(*stats.residual_echo_likelihood_recent_max, 0.0);
   EXPECT_LE(*stats.residual_echo_likelihood_recent_max, 1.0);
-  }
+  EXPECT_FALSE(stats.echo_return_loss.has_value());
-  EXPECT_FALSE(stats.echo_return_loss);
+  EXPECT_FALSE(stats.echo_return_loss_enhancement.has_value());
-  EXPECT_FALSE(stats.echo_return_loss_enhancement);
 }
 
 TEST(ApmStatistics, ReportHasVoice) {
@@ -2838,6 +2902,45 @@ TEST(ApmStatistics, ReportHasVoice) {
   EXPECT_FALSE(apm->GetStatistics().voice_detected);
 }
 
+TEST(ApmStatistics, GetStatisticsReportsNoEchoDetectorStatsWhenDisabled) {
+  rtc::scoped_refptr<AudioProcessing> apm =
+      AudioProcessingBuilderForTesting().Create();
+  Int16FrameData frame;
+  frame.num_channels = 1;
+  SetFrameSampleRate(&frame, AudioProcessing::NativeRate::kSampleRate32kHz);
+  ASSERT_EQ(
+      apm->ProcessStream(frame.data.data(),
+                         StreamConfig(frame.sample_rate_hz, frame.num_channels),
+                         StreamConfig(frame.sample_rate_hz, frame.num_channels),
+                         frame.data.data()),
+      0);
+  // Echo detector is disabled by default, no stats reported.
+  AudioProcessingStats stats = apm->GetStatistics();
+  EXPECT_FALSE(stats.residual_echo_likelihood.has_value());
+  EXPECT_FALSE(stats.residual_echo_likelihood_recent_max.has_value());
+}
+
+TEST(ApmStatistics, GetStatisticsReportsEchoDetectorStatsWhenEnabled) {
+  // Create APM with an echo detector injected.
+  rtc::scoped_refptr<AudioProcessing> apm =
+      AudioProcessingBuilderForTesting()
+          .SetEchoDetector(CreateEchoDetector())
+          .Create();
+  Int16FrameData frame;
+  frame.num_channels = 1;
+  SetFrameSampleRate(&frame, AudioProcessing::NativeRate::kSampleRate32kHz);
+  // Echo detector enabled: Report stats.
+  ASSERT_EQ(
+      apm->ProcessStream(frame.data.data(),
+                         StreamConfig(frame.sample_rate_hz, frame.num_channels),
+                         StreamConfig(frame.sample_rate_hz, frame.num_channels),
+                         frame.data.data()),
+      0);
+  AudioProcessingStats stats = apm->GetStatistics();
+  EXPECT_TRUE(stats.residual_echo_likelihood.has_value());
+  EXPECT_TRUE(stats.residual_echo_likelihood_recent_max.has_value());
+}
+
 TEST(ApmConfiguration, HandlingOfRateAndChannelCombinations) {
   std::array<int, 3> sample_rates_hz = {16000, 32000, 48000};
   std::array<int, 2> render_channel_counts = {1, 7};

modules/audio_processing/include/audio_processing.cc

@@ -205,8 +205,7 @@ std::string AudioProcessing::Config::ToString() const {
           << 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 << " }}";
   return builder.str();
 }
 

modules/audio_processing/include/audio_processing.h

@@ -161,7 +161,6 @@ class RTC_EXPORT AudioProcessing : public rtc::RefCountInterface {
   // submodule resets, affecting the audio quality. Use the RuntimeSetting
   // construct for runtime configuration.
   struct RTC_EXPORT Config {
-
     // Sets the properties of the audio processing pipeline.
     struct RTC_EXPORT Pipeline {
       // Maximum allowed processing rate used internally. May only be set to
@@ -378,8 +377,10 @@ class RTC_EXPORT AudioProcessing : public rtc::RefCountInterface {
       } adaptive_digital;
     } gain_controller2;
 
+    // TODO(bugs.webrtc.org/11539): Deprecated. Delete this flag. Replaced by
+    // injectable submodule.
     struct ResidualEchoDetector {
-      bool enabled = true;
+      bool enabled = false;
     } residual_echo_detector;
 
     std::string ToString() const;
@@ -939,17 +940,13 @@ class EchoDetector : public rtc::RefCountInterface {
                           int render_sample_rate_hz,
                           int num_render_channels) = 0;
 
-  // Analysis (not changing) of the render signal.
+  // Analysis (not changing) of the first channel of the render signal.
   virtual void AnalyzeRenderAudio(rtc::ArrayView<const float> render_audio) = 0;
 
   // Analysis (not changing) of the capture signal.
   virtual void AnalyzeCaptureAudio(
       rtc::ArrayView<const float> capture_audio) = 0;
 
-  // Pack an AudioBuffer into a vector<float>.
-  static void PackRenderAudioBuffer(AudioBuffer* audio,
-                                    std::vector<float>* packed_buffer);
-
   struct Metrics {
     absl::optional<double> echo_likelihood;
     absl::optional<double> echo_likelihood_recent_max;

modules/audio_processing/include/mock_audio_processing.h

@@ -67,6 +67,27 @@ class MockEchoControl : public EchoControl {
   MOCK_METHOD(bool, ActiveProcessing, (), (const, override));
 };
 
+class MockEchoDetector : public EchoDetector {
+ public:
+  virtual ~MockEchoDetector() {}
+  MOCK_METHOD(void,
+              Initialize,
+              (int capture_sample_rate_hz,
+               int num_capture_channels,
+               int render_sample_rate_hz,
+               int num_render_channels),
+              (override));
+  MOCK_METHOD(void,
+              AnalyzeRenderAudio,
+              (rtc::ArrayView<const float> render_audio),
+              (override));
+  MOCK_METHOD(void,
+              AnalyzeCaptureAudio,
+              (rtc::ArrayView<const float> capture_audio),
+              (override));
+  MOCK_METHOD(Metrics, GetMetrics, (), (const, override));
+};
+
 class MockAudioProcessing : public AudioProcessing {
  public:
   MockAudioProcessing() {}

modules/audio_processing/residual_echo_detector.cc

@@ -14,7 +14,6 @@
 #include <numeric>
 
 #include "absl/types/optional.h"
-#include "modules/audio_processing/audio_buffer.h"
 #include "modules/audio_processing/logging/apm_data_dumper.h"
 #include "rtc_base/atomic_ops.h"
 #include "rtc_base/checks.h"
@@ -199,13 +198,6 @@ void ResidualEchoDetector::Initialize(int /*capture_sample_rate_hz*/,
   reliability_ = 0.f;
 }
 
-void EchoDetector::PackRenderAudioBuffer(AudioBuffer* audio,
-                                         std::vector<float>* packed_buffer) {
-  packed_buffer->clear();
-  packed_buffer->insert(packed_buffer->end(), audio->channels()[0],
-                        audio->channels()[0] + audio->num_frames());
-}
-
 EchoDetector::Metrics ResidualEchoDetector::GetMetrics() const {
   EchoDetector::Metrics metrics;
   metrics.echo_likelihood = echo_likelihood_;

modules/audio_processing/test/aec_dump_based_simulator.cc

@@ -505,10 +505,6 @@ void AecDumpBasedSimulator::HandleMessage(
                 << msg.experiments_description() << std::endl;
     }
 
-    if (settings_.use_ed) {
-      apm_config.residual_echo_detector.enabled = *settings_.use_ed;
-    }
-
     ap_->ApplyConfig(apm_config);
   }
 }

modules/audio_processing/test/audio_processing_simulator.cc

@@ -20,6 +20,7 @@
 
 #include "api/audio/echo_canceller3_config_json.h"
 #include "api/audio/echo_canceller3_factory.h"
+#include "api/audio/echo_detector_creator.h"
 #include "modules/audio_processing/aec_dump/aec_dump_factory.h"
 #include "modules/audio_processing/echo_control_mobile_impl.h"
 #include "modules/audio_processing/include/audio_processing.h"
@@ -188,6 +189,10 @@ AudioProcessingSimulator::AudioProcessingSimulator(
       builder->SetEchoControlFactory(std::move(echo_control_factory));
     }
 
+    if (settings_.use_ed && *settings.use_ed) {
+      builder->SetEchoDetector(CreateEchoDetector());
+    }
+
     // Create an audio processing object.
     ap_ = builder->Create();
     RTC_CHECK(ap_);
@@ -569,10 +574,6 @@ void AudioProcessingSimulator::ConfigureAudioProcessor() {
         *settings_.analog_agc_disable_digital_adaptive;
   }
 
-  if (settings_.use_ed) {
-    apm_config.residual_echo_detector.enabled = *settings_.use_ed;
-  }
-
   if (settings_.maximum_internal_processing_rate) {
     apm_config.pipeline.maximum_internal_processing_rate =
         *settings_.maximum_internal_processing_rate;

test/fuzzers/BUILD.gn

@@ -473,6 +473,7 @@ webrtc_fuzzer_test("audio_processing_fuzzer") {
     ":audio_processing_fuzzer_helper",
     "../../api:scoped_refptr",
     "../../api/audio:aec3_factory",
+    "../../api/audio:echo_detector_creator",
     "../../api/task_queue:default_task_queue_factory",
     "../../modules/audio_processing",
     "../../modules/audio_processing:api",

test/fuzzers/audio_processing_configs_fuzzer.cc

@@ -13,6 +13,7 @@
 
 #include "absl/memory/memory.h"
 #include "api/audio/echo_canceller3_factory.h"
+#include "api/audio/echo_detector_creator.h"
 #include "api/task_queue/default_task_queue_factory.h"
 #include "modules/audio_processing/aec_dump/aec_dump_factory.h"
 #include "modules/audio_processing/include/audio_processing.h"
@@ -44,9 +45,9 @@ rtc::scoped_refptr<AudioProcessing> CreateApm(test::FuzzDataHelper* fuzz_data,
   static_cast<void>(fuzz_data->ReadOrDefaultValue(true));
   static_cast<void>(fuzz_data->ReadOrDefaultValue(true));
   static_cast<void>(fuzz_data->ReadOrDefaultValue(true));
-  bool red = fuzz_data->ReadOrDefaultValue(true);
+  bool use_red = fuzz_data->ReadOrDefaultValue(true);
-  bool hpf = fuzz_data->ReadOrDefaultValue(true);
+  bool use_hpf = fuzz_data->ReadOrDefaultValue(true);
-  bool aec3 = fuzz_data->ReadOrDefaultValue(true);
+  bool use_aec3 = fuzz_data->ReadOrDefaultValue(true);
 
   bool use_aec = fuzz_data->ReadOrDefaultValue(true);
   bool use_aecm = fuzz_data->ReadOrDefaultValue(true);
@@ -89,13 +90,13 @@ rtc::scoped_refptr<AudioProcessing> CreateApm(test::FuzzDataHelper* fuzz_data,
 
   // Filter out incompatible settings that lead to CHECK failures.
   if ((use_aecm && use_aec) ||          // These settings cause CHECK failure.
-      (use_aecm && aec3 && use_ns)  // These settings trigger webrtc:9489.
+      (use_aecm && use_aec3 && use_ns)  // These settings trigger webrtc:9489.
   ) {
     return nullptr;
   }
 
   std::unique_ptr<EchoControlFactory> echo_control_factory;
-  if (aec3) {
+  if (use_aec3) {
     echo_control_factory.reset(new EchoCanceller3Factory());
   }
 
@@ -104,8 +105,7 @@ rtc::scoped_refptr<AudioProcessing> CreateApm(test::FuzzDataHelper* fuzz_data,
   apm_config.pipeline.multi_channel_capture = true;
   apm_config.echo_canceller.enabled = use_aec || use_aecm;
   apm_config.echo_canceller.mobile_mode = use_aecm;
-  apm_config.residual_echo_detector.enabled = red;
+  apm_config.high_pass_filter.enabled = use_hpf;
-  apm_config.high_pass_filter.enabled = hpf;
   apm_config.gain_controller1.enabled = use_agc;
   apm_config.gain_controller1.enable_limiter = use_agc_limiter;
   apm_config.gain_controller2.enabled = use_agc2;
@@ -119,6 +119,7 @@ rtc::scoped_refptr<AudioProcessing> CreateApm(test::FuzzDataHelper* fuzz_data,
   rtc::scoped_refptr<AudioProcessing> apm =
       AudioProcessingBuilderForTesting()
           .SetEchoControlFactory(std::move(echo_control_factory))
+          .SetEchoDetector(use_red ? CreateEchoDetector() : nullptr)
           .SetConfig(apm_config)
           .Create();
 

webrtc.gni

@@ -435,6 +435,9 @@ all_poison_types = [
   # Default task queue implementation.
   "default_task_queue",
 
+  # Default echo detector implementation.
+  "default_echo_detector",
+
   # JSON parsing should not be needed in the "slim and modular" WebRTC.
   "rtc_json",