Removed the AudioProcessing dependency in EchoCancellerImpl

webrtc/modules/audio_processing/audio_processing_impl.cc

 /*
  *  Copyright (c) 2012 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.
  */
 
@@ skipping 151 matching lines @@
 #endif
                config.Get<Beamforming>().array_geometry,
                config.Get<Beamforming>().target_direction),
       capture_nonlocked_(config.Get<Beamforming>().enabled)
 {
   {
     rtc::CritScope cs_render(&crit_render_);
     rtc::CritScope cs_capture(&crit_capture_);
 
     public_submodules_->echo_cancellation.reset(
-        new EchoCancellationImpl(this, &crit_render_, &crit_capture_));
+        new EchoCancellationImpl(&crit_render_, &crit_capture_));
     public_submodules_->echo_control_mobile =
         new EchoControlMobileImpl(this, &crit_render_, &crit_capture_);
     public_submodules_->gain_control =
         new GainControlImpl(this, &crit_capture_, &crit_capture_);
     public_submodules_->high_pass_filter.reset(
         new HighPassFilterImpl(&crit_capture_));
     public_submodules_->level_estimator.reset(
         new LevelEstimatorImpl(&crit_capture_));
     public_submodules_->noise_suppression.reset(
         new NoiseSuppressionImpl(&crit_capture_));
@@ skipping 525 matching lines @@
 
   if (capture_nonlocked_.beamformer_enabled) {
     private_submodules_->beamformer->ProcessChunk(*ca->split_data_f(),
                                                   ca->split_data_f());
     ca->set_num_channels(1);
   }
 
   public_submodules_->high_pass_filter->ProcessCaptureAudio(ca);
   RETURN_ON_ERR(public_submodules_->gain_control->AnalyzeCaptureAudio(ca));
   public_submodules_->noise_suppression->AnalyzeCaptureAudio(ca);
-  RETURN_ON_ERR(public_submodules_->echo_cancellation->ProcessCaptureAudio(ca));
+
+  // Ensure that not both the AEC and AECM are active at the same time.
+  // TODO(peah): Simplify once the public API Enable functions for these
+  // are moved to APM.
+  if (public_submodules_->echo_cancellation->is_enabled() &&

[the sun, 2016/03/15 09:38:55]

Make this a DCHECK and move it to the beginning of the method.

[peah-webrtc, 2016/03/15 10:32:10]

Done.

+      public_submodules_->echo_control_mobile->is_enabled()) {
+    return AudioProcessing::kBadParameterError;
+  }
+
+  // Ensure that the stream delay was set before the call to the
+  // AEC ProcessCaptureAudio function.
+  if (public_submodules_->echo_cancellation->is_enabled() &&
+      !was_stream_delay_set()) {

[the sun, 2016/03/15 09:38:55]

Likewise, this is a precondition, so move to beginning of method. It's also a
logic error (and one which we should make impossible by setting the property at
construction) so make it a DCHECK.

[peah-webrtc, 2016/03/15 10:32:10]

What you say makes sense! I think this case is a bit different though compared
to the above one as that one is actually a necessary change that also breaks the
API behavior.

In this case, there is actually a unittest that test for this error code though.
Possibly there may be external code that relies on this error message as well.

I definitely agree that this should be refactored but I think it makes sense to
postpone that to upcoming CLs as the current code construct maintains the APM
behavior that was before this CL.

Removing the unittest for this error code is trivial, and I'd be happy to do
that, but before doing that I just wanted to raise the above concerns.

whyt?

[the sun, 2016/03/15 14:22:14]

Ok, let's leave as is.

+    return AudioProcessing::kStreamParameterNotSetError;
+  }
+
+  RETURN_ON_ERR(public_submodules_->echo_cancellation->ProcessCaptureAudio(
+      ca, stream_delay_ms()));
 
   if (public_submodules_->echo_control_mobile->is_enabled() &&
       public_submodules_->noise_suppression->is_enabled()) {
     ca->CopyLowPassToReference();
   }
   public_submodules_->noise_suppression->ProcessCaptureAudio(ca);
   if (constants_.intelligibility_enabled) {
     RTC_DCHECK(public_submodules_->noise_suppression->is_enabled());
     public_submodules_->intelligibility_enhancer->SetCaptureNoiseEstimate(
         public_submodules_->noise_suppression->NoiseEstimate());
@@ skipping 515 matching lines @@
   public_submodules_->high_pass_filter->Initialize(num_proc_channels(),
                                                    proc_sample_rate_hz());
 }
 
 void AudioProcessingImpl::InitializeNoiseSuppression() {
   public_submodules_->noise_suppression->Initialize(num_proc_channels(),
                                                     proc_sample_rate_hz());
 }
 
 void AudioProcessingImpl::InitializeEchoCanceller() {
-  public_submodules_->echo_cancellation->Initialize();
+  public_submodules_->echo_cancellation->Initialize(
+      proc_sample_rate_hz(), num_reverse_channels(), num_output_channels(),
+      num_proc_channels());
 }
 
 void AudioProcessingImpl::InitializeLevelEstimator() {
   public_submodules_->level_estimator->Initialize();
 }
 
 void AudioProcessingImpl::InitializeVoiceDetection() {
   public_submodules_->voice_detection->Initialize(proc_split_sample_rate_hz());
 }
 
@@ skipping 192 matching lines @@
   debug_dump_.capture.event_msg->mutable_config()->CopyFrom(config);
 
   RETURN_ON_ERR(WriteMessageToDebugFile(debug_dump_.debug_file.get(),
                                         &debug_dump_.num_bytes_left_for_log_,
                                         &crit_debug_, &debug_dump_.capture));
   return kNoError;
 }
 #endif  // WEBRTC_AUDIOPROC_DEBUG_DUMP
 
 }  // namespace webrtc