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 697 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.
+  RETURN_ON_ERR((public_submodules_->echo_cancellation->is_enabled() &&

[hlundin-webrtc, 2016/03/07 16:03:08]

You don't have to use RETURN_ON_ERR here, since you are essentially creating
your return values in situ.

if (...)
  return AudioProcessing::kNoError;

[peah-webrtc, 2016/03/08 06:05:08]

You are definitely right on that. It is just that I wanted to use the macro
RETURN_ON_ERR for consistency, as it basically does exactly the if-statement
if (public_submodules_->echo_cancellation->is_enabled() &&
public_submodules_->echo_control_mobile->is_enabled()) {
return AudioProcessing::kBadParameterError;
}

The advantage of doing the full if-statement is that it is more explicit and the
right thing to do as the macro is really mainly intended for function error
return values,
but the disadvantage is that it clutters the code and that it mixes up two ways
of reporting errors.

I'll change, but please let me know if you in light of my comments above would
think it could make sense to do it the way it was.

[peah-webrtc, 2016/03/08 06:05:08]

Done.

[hlundin-webrtc, 2016/03/08 12:39:23]

I think the code is nicer without the macro.

+                         public_submodules_->echo_control_mobile->is_enabled()
+                     ? AudioProcessing::kBadParameterError
+                     : AudioProcessing::kNoError));
+
+  // Ensure that the stream delay was set before the call to the
+  // AEC ProcessCaptureAudio function.
+  RETURN_ON_ERR((was_stream_delay_set() ? AudioProcessing::kNoError

[hlundin-webrtc, 2016/03/07 16:03:08]

Same as above.

[peah-webrtc, 2016/03/08 06:05:08]

Done.

+                                        : AudioProcessing::kBadParameterError));
+
+  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