Removed the AudioProcessing dependency in EchoCancellerImpl

webrtc/modules/audio_processing/audio_processing_impl.cc

Index: webrtc/modules/audio_processing/audio_processing_impl.cc
diff --git a/webrtc/modules/audio_processing/audio_processing_impl.cc b/webrtc/modules/audio_processing/audio_processing_impl.cc
index 9c587c25c7940b1cebb230caba6d6ea807eba628..bbd161d2240118e861a6ba3fc038d74f87217a38 100644
--- a/webrtc/modules/audio_processing/audio_processing_impl.cc
+++ b/webrtc/modules/audio_processing/audio_processing_impl.cc
@@ -169,7 +169,7 @@ AudioProcessingImpl::AudioProcessingImpl(const Config& config,
     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 =
@@ -715,7 +715,24 @@ int AudioProcessingImpl::ProcessStreamLocked() {
   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()) {
@@ -1251,7 +1268,9 @@ void AudioProcessingImpl::InitializeNoiseSuppression() {
 }
 
 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() {