Cleaned up and revised the handling of resampling and bandsplitting in APM and

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 64 matching lines @@
     AudioProcessing::kSampleRate16kHz,
     AudioProcessing::kSampleRate32kHz,
     AudioProcessing::kSampleRate48kHz};
 const size_t AudioProcessing::kNumNativeSampleRates =
     arraysize(AudioProcessing::kNativeSampleRatesHz);
 const int AudioProcessing::kMaxNativeSampleRateHz = AudioProcessing::
     kNativeSampleRatesHz[AudioProcessing::kNumNativeSampleRates - 1];
 
 namespace {
 
-const int kInternalNativeRates[] = {AudioProcessing::kSampleRate8kHz,
-                                    AudioProcessing::kSampleRate16kHz,
-#ifdef WEBRTC_ARCH_ARM_FAMILY
-                                    AudioProcessing::kSampleRate32kHz};
-#else
-                                    AudioProcessing::kSampleRate32kHz,
-                                    AudioProcessing::kSampleRate48kHz};
-#endif  // WEBRTC_ARCH_ARM_FAMILY
-
 static bool LayoutHasKeyboard(AudioProcessing::ChannelLayout layout) {
   switch (layout) {
     case AudioProcessing::kMono:
     case AudioProcessing::kStereo:
       return false;
     case AudioProcessing::kMonoAndKeyboard:
     case AudioProcessing::kStereoAndKeyboard:
       return true;
   }
 
   assert(false);
   return false;
 }
 
-bool is_multi_band(int sample_rate_hz) {
+bool SampleRateSupportsMultiBand(int sample_rate_hz) {
   return sample_rate_hz == AudioProcessing::kSampleRate32kHz ||
          sample_rate_hz == AudioProcessing::kSampleRate48kHz;
 }
 
-int ClosestHigherNativeRate(int min_proc_rate) {
-  for (int rate : kInternalNativeRates) {
-    if (rate >= min_proc_rate) {
+int NativeProcessRateToUse(int minimum_rate, bool band_splitting_required) {

[the sun, 2016/09/08 08:10:15]

FindNativeProcessRateToUse()

also, this is the type of function which it would be very easy to write a test
for...

[peah-webrtc, 2016/09/08 08:44:03]

Changed the name.

It is indeed a good function to write a test for. But then I would need to
expose that outside audio_processing_impl.cc, right? Now it is a local method in
the local namespace. Is it worth doing that just for the purpose of adding a
test?

[the sun, 2016/09/08 09:20:48]

I think so. Unless the functionality is covered by the other tests? But I guess
to test that proper you'd need to expose the other state, and then through
audio_processing.h, which seems worse.

[peah-webrtc, 2016/09/08 18:57:50]

This is indirectly tested by the tests that operate on the APM API, for instance
the tests in audio_processing_unittest.cc and the tests in
audio_processing_performance_unittest.cc. 

Is it ok to keep this as it is like this and consider that testing sufficient? 

[the sun, 2016/09/08 20:36:38]

Very well. I assume the non-bitexact changes cannot possibly be caused by this
then...

[peah-webrtc, 2016/09/10 07:52:37]

Of course, they can. But we have at least coverage to detect when something
happens that should not happen.

+#ifdef WEBRTC_ARCH_ARM_FAMILY
+  const int kMaxSplittingNativeProcessRate = AudioProcessing::kSampleRate32kHz;
+#else
+  const int kMaxSplittingNativeProcessRate = AudioProcessing::kSampleRate48kHz;
+#endif
+  RTC_DCHECK_LE(kMaxSplittingNativeProcessRate,
+                AudioProcessing::kMaxNativeSampleRateHz);
+  const int uppermost_native_rate = band_splitting_required
+                                        ? kMaxSplittingNativeProcessRate
+                                        : AudioProcessing::kSampleRate48kHz;
+
+  for (auto rate : AudioProcessing::kNativeSampleRatesHz) {
+    if (rate >= uppermost_native_rate) {
+      return uppermost_native_rate;
+    }
+    if (rate >= minimum_rate) {
       return rate;
     }
   }
-  return kInternalNativeRates[arraysize(kInternalNativeRates) - 1];
+  RTC_NOTREACHED();
 }
 
 }  // namespace
 
 // Throughout webrtc, it's assumed that success is represented by zero.
 static_assert(AudioProcessing::kNoError == 0, "kNoError must be zero");
 
+AudioProcessingImpl::ApmSubmoduleStates::ApmSubmoduleStates() {}
+
+bool AudioProcessingImpl::ApmSubmoduleStates::Update(
+    bool high_pass_filter_enabled,
+    bool echo_canceller_enabled,
+    bool mobile_echo_controller_enabled,
+    bool noise_suppressor_enabled,
+    bool intelligibility_enhancer_enabled,
+    bool beamformer_enabled,
+    bool adaptive_gain_controller_enabled,
+    bool level_controller_enabled,
+    bool voice_activity_detector_enabled,
+    bool level_estimator_enabled,
+    bool transient_suppressor_enabled) {
+  bool changed = false;
+  changed |= (high_pass_filter_enabled != high_pass_filter_enabled_);
+  changed |= (echo_canceller_enabled != echo_canceller_enabled_);
+  changed |=
+      (mobile_echo_controller_enabled != mobile_echo_controller_enabled_);
+  changed |= (noise_suppressor_enabled != noise_suppressor_enabled_);
+  changed |=
+      (intelligibility_enhancer_enabled != intelligibility_enhancer_enabled_);
+  changed |= (beamformer_enabled != beamformer_enabled_);
+  changed |=
+      (adaptive_gain_controller_enabled != adaptive_gain_controller_enabled_);
+  changed |= (level_controller_enabled != level_controller_enabled_);
+  changed |= (level_estimator_enabled != level_estimator_enabled_);
+  changed |=
+      (voice_activity_detector_enabled != voice_activity_detector_enabled_);
+  changed |= (transient_suppressor_enabled != transient_suppressor_enabled_);
+  if (changed) {
+    high_pass_filter_enabled_ = high_pass_filter_enabled;
+    echo_canceller_enabled_ = echo_canceller_enabled;
+    mobile_echo_controller_enabled_ = mobile_echo_controller_enabled;
+    noise_suppressor_enabled_ = noise_suppressor_enabled;
+    intelligibility_enhancer_enabled_ = intelligibility_enhancer_enabled;
+    beamformer_enabled_ = beamformer_enabled;
+    adaptive_gain_controller_enabled_ = adaptive_gain_controller_enabled;
+    level_controller_enabled_ = level_controller_enabled;
+    level_estimator_enabled_ = level_estimator_enabled;
+    voice_activity_detector_enabled_ = voice_activity_detector_enabled;
+    transient_suppressor_enabled_ = transient_suppressor_enabled;
+  }
+
+  changed |= first_update_;
+  first_update_ = false;
+  return changed;
+}
+
+bool AudioProcessingImpl::ApmSubmoduleStates::CaptureMultiBandModulesActive()

[the sun, 2016/09/08 08:10:15]

Is this distinction between "modules" and "effects" widely known and agreed upon
terminology? Feels a bit ad hoc to me. I guess the difference is that an
"effect" will change the signal? Can we make it clearer? At least add comments
in the .h.

[peah-webrtc, 2016/09/08 08:44:04]

Good points!

It is definitely not an accepted terminology. And it should actually be
SubModules instead of modules to follow the rest of the code so I'll change
that.

I'll change the name Effects to Processing, which follows the terminology
currently inside APM.

PTAL!

+    const {
+  return CaptureMultiBandEffectsActive() || intelligibility_enhancer_enabled_ ||
+         voice_activity_detector_enabled_;
+}
+
+bool AudioProcessingImpl::ApmSubmoduleStates::CaptureMultiBandEffectsActive()
+    const {
+  return high_pass_filter_enabled_ || echo_canceller_enabled_ ||
+         mobile_echo_controller_enabled_ || noise_suppressor_enabled_ ||
+         beamformer_enabled_ || adaptive_gain_controller_enabled_;
+}
+
+bool AudioProcessingImpl::ApmSubmoduleStates::RenderMultiBandModulesActive()
+    const {
+  return RenderMultiBandEffectsActive() || echo_canceller_enabled_ ||
+         mobile_echo_controller_enabled_ || adaptive_gain_controller_enabled_;
+}
+
+bool AudioProcessingImpl::ApmSubmoduleStates::RenderMultiBandEffectsActive()
+    const {
+  return intelligibility_enhancer_enabled_;
+}
+
 struct AudioProcessingImpl::ApmPublicSubmodules {
   ApmPublicSubmodules() {}
   // Accessed externally of APM without any lock acquired.
   std::unique_ptr<EchoCancellationImpl> echo_cancellation;
   std::unique_ptr<EchoControlMobileImpl> echo_control_mobile;
   std::unique_ptr<GainControlImpl> gain_control;
   std::unique_ptr<HighPassFilterImpl> high_pass_filter;
   std::unique_ptr<LevelEstimatorImpl> level_estimator;
   std::unique_ptr<NoiseSuppressionImpl> noise_suppression;
   std::unique_ptr<VoiceDetectionImpl> voice_detection;
@@ skipping 131 matching lines @@
 
 int AudioProcessingImpl::Initialize(const ProcessingConfig& processing_config) {
   // Run in a single-threaded manner during initialization.
   rtc::CritScope cs_render(&crit_render_);
   rtc::CritScope cs_capture(&crit_capture_);
   return InitializeLocked(processing_config);
 }
 
 int AudioProcessingImpl::MaybeInitializeRender(
     const ProcessingConfig& processing_config) {
-  return MaybeInitialize(processing_config);
+  return MaybeInitialize(processing_config, false);
 }
 
 int AudioProcessingImpl::MaybeInitializeCapture(
-    const ProcessingConfig& processing_config) {
-  return MaybeInitialize(processing_config);
+    const ProcessingConfig& processing_config,
+    bool force_initialization) {
+  return MaybeInitialize(processing_config, force_initialization);
 }
 
 #ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP
 
 AudioProcessingImpl::ApmDebugDumpThreadState::ApmDebugDumpThreadState()
     : event_msg(new audioproc::Event()) {}
 
 AudioProcessingImpl::ApmDebugDumpThreadState::~ApmDebugDumpThreadState() {}
 
 AudioProcessingImpl::ApmDebugDumpState::ApmDebugDumpState()
     : debug_file(FileWrapper::Create()) {}
 
 AudioProcessingImpl::ApmDebugDumpState::~ApmDebugDumpState() {}
 
 #endif  // WEBRTC_AUDIOPROC_DEBUG_DUMP
 
 // Calls InitializeLocked() if any of the audio parameters have changed from
 // their current values (needs to be called while holding the crit_render_lock).
 int AudioProcessingImpl::MaybeInitialize(
-    const ProcessingConfig& processing_config) {
+    const ProcessingConfig& processing_config,
+    bool force_initialization) {
   // Called from both threads. Thread check is therefore not possible.
-  if (processing_config == formats_.api_format) {
+  if (processing_config == formats_.api_format && !force_initialization) {
     return kNoError;
   }
 
   rtc::CritScope cs_capture(&crit_capture_);
   return InitializeLocked(processing_config);
 }
 
 int AudioProcessingImpl::InitializeLocked() {
   const int fwd_audio_buffer_channels =
       capture_nonlocked_.beamformer_enabled
           ? formats_.api_format.input_stream().num_channels()
           : formats_.api_format.output_stream().num_channels();
   const int rev_audio_buffer_out_num_frames =
       formats_.api_format.reverse_output_stream().num_frames() == 0
           ? formats_.rev_proc_format.num_frames()
           : formats_.api_format.reverse_output_stream().num_frames();
   if (formats_.api_format.reverse_input_stream().num_channels() > 0) {
     render_.render_audio.reset(new AudioBuffer(
         formats_.api_format.reverse_input_stream().num_frames(),
         formats_.api_format.reverse_input_stream().num_channels(),
         formats_.rev_proc_format.num_frames(),
         formats_.rev_proc_format.num_channels(),
         rev_audio_buffer_out_num_frames));
-    if (rev_conversion_needed()) {
+    if (formats_.api_format.reverse_input_stream() !=
+        formats_.api_format.reverse_output_stream()) {
       render_.render_converter = AudioConverter::Create(
           formats_.api_format.reverse_input_stream().num_channels(),
           formats_.api_format.reverse_input_stream().num_frames(),
           formats_.api_format.reverse_output_stream().num_channels(),
           formats_.api_format.reverse_output_stream().num_frames());
     } else {
       render_.render_converter.reset(nullptr);
     }
   } else {
     render_.render_audio.reset(nullptr);
@@ skipping 50 matching lines @@
     return kBadNumberChannelsError;
   }
 
   if (capture_nonlocked_.beamformer_enabled &&
       num_in_channels != capture_.array_geometry.size()) {
     return kBadNumberChannelsError;
   }
 
   formats_.api_format = config;
 
-  capture_nonlocked_.fwd_proc_format = StreamConfig(ClosestHigherNativeRate(
+  int fwd_proc_rate = NativeProcessRateToUse(

[the sun, 2016/09/08 08:10:15]

I find it confusing that "fwd" and "rev" are used sometimes, and "Capture" and
"Render" in other places. Can we stick to a single nomenclature?

[peah-webrtc, 2016/09/08 08:44:03]

I agree, and that is something that has been present for quite a while (>2
years). The changes in this CL makes the changes towards the "render" and
"capture" nomenclature.

Personally, I like the "render" and "capture" naming. But the AudioProcessing
API uses "reverse" and "forward" so I think that is the terminology we should
use.

Would it be ok to keep the current CL as it is, and follow up with a renaming CL
which fixes this for all other places inside audio_processing_impl.*?

[the sun, 2016/09/08 09:20:48]

Personally I think Render and Capture makes much more sense so I'd rather stick
to that internally and in the long term change the API.

[peah-webrtc, 2016/09/08 18:57:50]

That sounds great to me. Note that I named the variable fwd_proc_rate in order
to harmonize with the rev_proc_rate that was there previously. 

I'd be delighted to do the name changes, but I'd prefer to keep those for a
separate CL, as this one is already big and introduce non-bitexact changes (in
the new reinitialization). 
WDYT?

[the sun, 2016/09/08 20:36:38]

sgtm

       std::min(formats_.api_format.input_stream().sample_rate_hz(),
-               formats_.api_format.output_stream().sample_rate_hz())));
+               formats_.api_format.output_stream().sample_rate_hz()),
+      submodule_states_.CaptureMultiBandModulesActive() ||
+          submodule_states_.RenderMultiBandModulesActive());
 
-  int rev_proc_rate = ClosestHigherNativeRate(std::min(
-      formats_.api_format.reverse_input_stream().sample_rate_hz(),
-      formats_.api_format.reverse_output_stream().sample_rate_hz()));
+  capture_nonlocked_.fwd_proc_format = StreamConfig(fwd_proc_rate);
+
+  int rev_proc_rate = NativeProcessRateToUse(
+      std::min(formats_.api_format.reverse_input_stream().sample_rate_hz(),
+               formats_.api_format.reverse_output_stream().sample_rate_hz()),
+      submodule_states_.CaptureMultiBandModulesActive() ||
+          submodule_states_.RenderMultiBandModulesActive());
   // TODO(aluebs): Remove this restriction once we figure out why the 3-band
   // splitting filter degrades the AEC performance.
   if (rev_proc_rate > kSampleRate32kHz) {
-    rev_proc_rate = is_rev_processed() ? kSampleRate32kHz : kSampleRate16kHz;
+    rev_proc_rate = submodule_states_.RenderMultiBandEffectsActive()
+                        ? kSampleRate32kHz
+                        : kSampleRate16kHz;
   }
   // If the forward sample rate is 8 kHz, the reverse stream is also processed
   // at this rate.
   if (capture_nonlocked_.fwd_proc_format.sample_rate_hz() == kSampleRate8kHz) {
     rev_proc_rate = kSampleRate8kHz;
   } else {
     rev_proc_rate = std::max(rev_proc_rate, static_cast<int>(kSampleRate16kHz));
   }
 
   // Always downmix the reverse stream to mono for analysis. This has been
@@ skipping 128 matching lines @@
   }
   return ProcessStream(src, input_stream, output_stream, dest);
 }
 
 int AudioProcessingImpl::ProcessStream(const float* const* src,
                                        const StreamConfig& input_config,
                                        const StreamConfig& output_config,
                                        float* const* dest) {
   TRACE_EVENT0("webrtc", "AudioProcessing::ProcessStream_StreamConfig");
   ProcessingConfig processing_config;
+  bool reinitialization_required;

[the sun, 2016/09/08 08:10:15]

Please, give me a default value, in case someone later moves code around...

[peah-webrtc, 2016/09/08 08:44:03]

Done.

   {
     // Acquire the capture lock in order to safely call the function
     // that retrieves the render side data. This function accesses apm
     // getters that need the capture lock held when being called.
     rtc::CritScope cs_capture(&crit_capture_);
     public_submodules_->echo_cancellation->ReadQueuedRenderData();
     public_submodules_->echo_control_mobile->ReadQueuedRenderData();
     public_submodules_->gain_control->ReadQueuedRenderData();
 
     if (!src || !dest) {
       return kNullPointerError;
     }
 
     processing_config = formats_.api_format;
+    reinitialization_required = UpdateActiveSubmoduleStates();
   }
 
   processing_config.input_stream() = input_config;
   processing_config.output_stream() = output_config;
 
   {
     // Do conditional reinitialization.
     rtc::CritScope cs_render(&crit_render_);
-    RETURN_ON_ERR(MaybeInitializeCapture(processing_config));
+    RETURN_ON_ERR(
+        MaybeInitializeCapture(processing_config, reinitialization_required));
   }
   rtc::CritScope cs_capture(&crit_capture_);
   assert(processing_config.input_stream().num_frames() ==
          formats_.api_format.input_stream().num_frames());
 
 #ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP
   if (debug_dump_.debug_file->is_open()) {
     RETURN_ON_ERR(WriteConfigMessage(false));
 
     debug_dump_.capture.event_msg->set_type(audioproc::Event::STREAM);
@@ skipping 47 matching lines @@
   }
   // Must be a native rate.
   if (frame->sample_rate_hz_ != kSampleRate8kHz &&
       frame->sample_rate_hz_ != kSampleRate16kHz &&
       frame->sample_rate_hz_ != kSampleRate32kHz &&
       frame->sample_rate_hz_ != kSampleRate48kHz) {
     return kBadSampleRateError;
   }
 
   ProcessingConfig processing_config;
+  bool reinitialization_required;

[the sun, 2016/09/08 08:10:15]

default init pls

[peah-webrtc, 2016/09/08 08:44:04]

Done.

   {
     // Aquire lock for the access of api_format.
     // The lock is released immediately due to the conditional
     // reinitialization.
     rtc::CritScope cs_capture(&crit_capture_);
     // TODO(ajm): The input and output rates and channels are currently
     // constrained to be identical in the int16 interface.
     processing_config = formats_.api_format;
+
+    reinitialization_required = UpdateActiveSubmoduleStates();
   }
   processing_config.input_stream().set_sample_rate_hz(frame->sample_rate_hz_);
   processing_config.input_stream().set_num_channels(frame->num_channels_);
   processing_config.output_stream().set_sample_rate_hz(frame->sample_rate_hz_);
   processing_config.output_stream().set_num_channels(frame->num_channels_);
 
   {
     // Do conditional reinitialization.
     rtc::CritScope cs_render(&crit_render_);
-    RETURN_ON_ERR(MaybeInitializeCapture(processing_config));
+    RETURN_ON_ERR(
+        MaybeInitializeCapture(processing_config, reinitialization_required));
   }
   rtc::CritScope cs_capture(&crit_capture_);
   if (frame->samples_per_channel_ !=
       formats_.api_format.input_stream().num_frames()) {
     return kBadDataLengthError;
   }
 
 #ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP
   if (debug_dump_.debug_file->is_open()) {
     RETURN_ON_ERR(WriteConfigMessage(false));
 
     debug_dump_.capture.event_msg->set_type(audioproc::Event::STREAM);
     audioproc::Stream* msg = debug_dump_.capture.event_msg->mutable_stream();
     const size_t data_size =
         sizeof(int16_t) * frame->samples_per_channel_ * frame->num_channels_;
     msg->set_input_data(frame->data_, data_size);
   }
 #endif
 
   capture_.capture_audio->DeinterleaveFrom(frame);
   RETURN_ON_ERR(ProcessStreamLocked());
-  capture_.capture_audio->InterleaveTo(frame, output_copy_needed());
+  capture_.capture_audio->InterleaveTo(frame, true);
 
 #ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP
   if (debug_dump_.debug_file->is_open()) {
     audioproc::Stream* msg = debug_dump_.capture.event_msg->mutable_stream();
     const size_t data_size =
         sizeof(int16_t) * frame->samples_per_channel_ * frame->num_channels_;
     msg->set_output_data(frame->data_, data_size);
     RETURN_ON_ERR(WriteMessageToDebugFile(debug_dump_.debug_file.get(),
                                           &debug_dump_.num_bytes_left_for_log_,
                                           &crit_debug_, &debug_dump_.capture));
@@ skipping 25 matching lines @@
 
   AudioBuffer* ca = capture_.capture_audio.get();  // For brevity.
 
   if (constants_.use_experimental_agc &&
       public_submodules_->gain_control->is_enabled()) {
     private_submodules_->agc_manager->AnalyzePreProcess(
         ca->channels()[0], ca->num_channels(),
         capture_nonlocked_.fwd_proc_format.num_frames());
   }
 
-  if (fwd_analysis_needed()) {
+  if (submodule_states_.CaptureMultiBandModulesActive() &&
+      SampleRateSupportsMultiBand(
+          capture_nonlocked_.fwd_proc_format.sample_rate_hz())) {
     ca->SplitIntoFrequencyBands();
   }
 
   if (capture_nonlocked_.beamformer_enabled) {
     private_submodules_->beamformer->AnalyzeChunk(*ca->split_data_f());
     // Discards all channels by the leftmost one.
     ca->set_num_channels(1);
   }
 
   public_submodules_->high_pass_filter->ProcessCaptureAudio(ca);
@@ skipping 50 matching lines @@
       public_submodules_->gain_control->is_enabled() &&
       (!capture_nonlocked_.beamformer_enabled ||
        private_submodules_->beamformer->is_target_present())) {
     private_submodules_->agc_manager->Process(
         ca->split_bands_const(0)[kBand0To8kHz], ca->num_frames_per_band(),
         capture_nonlocked_.split_rate);
   }
   RETURN_ON_ERR(public_submodules_->gain_control->ProcessCaptureAudio(
       ca, echo_cancellation()->stream_has_echo()));
 
-  if (fwd_synthesis_needed()) {
+  if (submodule_states_.CaptureMultiBandEffectsActive() &&
+      SampleRateSupportsMultiBand(
+          capture_nonlocked_.fwd_proc_format.sample_rate_hz())) {
     ca->MergeFrequencyBands();
   }
 
   // TODO(aluebs): Investigate if the transient suppression placement should be
   // before or after the AGC.
   if (capture_.transient_suppressor_enabled) {
     float voice_probability =
         private_submodules_->agc_manager.get()
             ? private_submodules_->agc_manager->voice_probability()
             : 1.f;
@@ skipping 33 matching lines @@
 
 int AudioProcessingImpl::ProcessReverseStream(
     const float* const* src,
     const StreamConfig& reverse_input_config,
     const StreamConfig& reverse_output_config,
     float* const* dest) {
   TRACE_EVENT0("webrtc", "AudioProcessing::ProcessReverseStream_StreamConfig");
   rtc::CritScope cs(&crit_render_);
   RETURN_ON_ERR(AnalyzeReverseStreamLocked(src, reverse_input_config,
                                            reverse_output_config));
-  if (is_rev_processed()) {
+  if (submodule_states_.RenderMultiBandEffectsActive()) {
     render_.render_audio->CopyTo(formats_.api_format.reverse_output_stream(),
                                  dest);
-  } else if (render_check_rev_conversion_needed()) {
+  } else if (formats_.api_format.reverse_input_stream() !=
+             formats_.api_format.reverse_output_stream()) {
     render_.render_converter->Convert(src, reverse_input_config.num_samples(),
                                       dest,
                                       reverse_output_config.num_samples());
   } else {
     CopyAudioIfNeeded(src, reverse_input_config.num_frames(),
                       reverse_input_config.num_channels(), dest);
   }
 
   return kNoError;
 }
@@ skipping 81 matching lines @@
     const size_t data_size =
         sizeof(int16_t) * frame->samples_per_channel_ * frame->num_channels_;
     msg->set_data(frame->data_, data_size);
     RETURN_ON_ERR(WriteMessageToDebugFile(debug_dump_.debug_file.get(),
                                           &debug_dump_.num_bytes_left_for_log_,
                                           &crit_debug_, &debug_dump_.render));
   }
 #endif
   render_.render_audio->DeinterleaveFrom(frame);
   RETURN_ON_ERR(ProcessReverseStreamLocked());
-  if (is_rev_processed()) {
-    render_.render_audio->InterleaveTo(frame, true);
-  }
+  render_.render_audio->InterleaveTo(frame, true);
   return kNoError;
 }
 
 int AudioProcessingImpl::ProcessReverseStreamLocked() {
   AudioBuffer* ra = render_.render_audio.get();  // For brevity.
-  if (rev_analysis_needed()) {
+  if (submodule_states_.RenderMultiBandModulesActive() &&
+      SampleRateSupportsMultiBand(formats_.rev_proc_format.sample_rate_hz())) {
     ra->SplitIntoFrequencyBands();
   }
 
 #if WEBRTC_INTELLIGIBILITY_ENHANCER
   if (capture_nonlocked_.intelligibility_enabled) {
     public_submodules_->intelligibility_enhancer->ProcessRenderAudio(
         ra->split_channels_f(kBand0To8kHz), capture_nonlocked_.split_rate,
         ra->num_channels());
   }
 #endif
 
   RETURN_ON_ERR(public_submodules_->echo_cancellation->ProcessRenderAudio(ra));
   RETURN_ON_ERR(
       public_submodules_->echo_control_mobile->ProcessRenderAudio(ra));
   if (!constants_.use_experimental_agc) {
     RETURN_ON_ERR(public_submodules_->gain_control->ProcessRenderAudio(ra));
   }
 
-  if (rev_synthesis_needed()) {
+  if (submodule_states_.RenderMultiBandEffectsActive() &&
+      SampleRateSupportsMultiBand(formats_.rev_proc_format.sample_rate_hz())) {
     ra->MergeFrequencyBands();
   }
 
   return kNoError;
 }
 
 int AudioProcessingImpl::set_stream_delay_ms(int delay) {
   rtc::CritScope cs(&crit_capture_);
   Error retval = kNoError;
   capture_.was_stream_delay_set = true;
@@ skipping 113 matching lines @@
 #ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP
   // We just return if recording hasn't started.
   debug_dump_.debug_file->CloseFile();
   return kNoError;
 #else
   return kUnsupportedFunctionError;
 #endif  // WEBRTC_AUDIOPROC_DEBUG_DUMP
 }
 
 EchoCancellation* AudioProcessingImpl::echo_cancellation() const {
-  // Adding a lock here has no effect as it allows any access to the submodule
-  // from the returned pointer.
   return public_submodules_->echo_cancellation.get();
 }
 
 EchoControlMobile* AudioProcessingImpl::echo_control_mobile() const {
-  // Adding a lock here has no effect as it allows any access to the submodule
-  // from the returned pointer.
   return public_submodules_->echo_control_mobile.get();
 }
 
 GainControl* AudioProcessingImpl::gain_control() const {
-  // Adding a lock here has no effect as it allows any access to the submodule
-  // from the returned pointer.
   if (constants_.use_experimental_agc) {
     return public_submodules_->gain_control_for_experimental_agc.get();
   }
   return public_submodules_->gain_control.get();
 }
 
 HighPassFilter* AudioProcessingImpl::high_pass_filter() const {
-  // Adding a lock here has no effect as it allows any access to the submodule
-  // from the returned pointer.
   return public_submodules_->high_pass_filter.get();
 }
 
 LevelEstimator* AudioProcessingImpl::level_estimator() const {
-  // Adding a lock here has no effect as it allows any access to the submodule
-  // from the returned pointer.
   return public_submodules_->level_estimator.get();
 }
 
 NoiseSuppression* AudioProcessingImpl::noise_suppression() const {
-  // Adding a lock here has no effect as it allows any access to the submodule
-  // from the returned pointer.
   return public_submodules_->noise_suppression.get();
 }
 
 VoiceDetection* AudioProcessingImpl::voice_detection() const {
-  // Adding a lock here has no effect as it allows any access to the submodule
-  // from the returned pointer.
   return public_submodules_->voice_detection.get();
 }
 
-bool AudioProcessingImpl::is_fwd_processed() const {
-  // The beamformer, noise suppressor and highpass filter
-  // modify the data.
-  if (capture_nonlocked_.beamformer_enabled ||
-      public_submodules_->high_pass_filter->is_enabled() ||
-      public_submodules_->noise_suppression->is_enabled() ||
-      public_submodules_->echo_cancellation->is_enabled() ||
-      public_submodules_->echo_control_mobile->is_enabled() ||
-      public_submodules_->gain_control->is_enabled()) {
-    return true;
-  }
-
-  // The capture data is otherwise unchanged.
-  return false;
-}
-
-bool AudioProcessingImpl::output_copy_needed() const {
-  // Check if we've upmixed or downmixed the audio.
-  return ((formats_.api_format.output_stream().num_channels() !=
-           formats_.api_format.input_stream().num_channels()) ||
-          is_fwd_processed() || capture_.transient_suppressor_enabled ||
-          capture_nonlocked_.level_controller_enabled);
-}
-
-bool AudioProcessingImpl::fwd_synthesis_needed() const {
-  return (is_fwd_processed() &&
-          is_multi_band(capture_nonlocked_.fwd_proc_format.sample_rate_hz()));
-}
-
-bool AudioProcessingImpl::fwd_analysis_needed() const {
-  if (!is_fwd_processed() &&
-      !public_submodules_->voice_detection->is_enabled() &&
-      !capture_.transient_suppressor_enabled) {
-    // Only public_submodules_->level_estimator is enabled.
-    return false;
-  } else if (is_multi_band(
-                 capture_nonlocked_.fwd_proc_format.sample_rate_hz())) {
-    // Something besides public_submodules_->level_estimator is enabled, and we
-    // have super-wb.
-    return true;
-  }
-  return false;
-}
-
-bool AudioProcessingImpl::is_rev_processed() const {
-#if WEBRTC_INTELLIGIBILITY_ENHANCER
-  return capture_nonlocked_.intelligibility_enabled;

[the sun, 2016/09/08 08:10:15]

This was previously if-deffed but now is not - intentional?

[peah-webrtc, 2016/09/08 08:44:04]

Good point!
Done.

-#else
-  return false;
-#endif
-}
-
-bool AudioProcessingImpl::rev_synthesis_needed() const {
-  return (is_rev_processed() &&
-          is_multi_band(formats_.rev_proc_format.sample_rate_hz()));
-}
-
-bool AudioProcessingImpl::rev_analysis_needed() const {
-  return is_multi_band(formats_.rev_proc_format.sample_rate_hz()) &&
-         (is_rev_processed() ||
-          public_submodules_->echo_cancellation
-              ->is_enabled_render_side_query() ||
-          public_submodules_->echo_control_mobile
-              ->is_enabled_render_side_query() ||
-          public_submodules_->gain_control->is_enabled_render_side_query());
-}
-
-bool AudioProcessingImpl::render_check_rev_conversion_needed() const {
-  return rev_conversion_needed();
-}
-
-bool AudioProcessingImpl::rev_conversion_needed() const {
-  return (formats_.api_format.reverse_input_stream() !=
-          formats_.api_format.reverse_output_stream());
+bool AudioProcessingImpl::UpdateActiveSubmoduleStates() {
+  return submodule_states_.Update(
+      public_submodules_->high_pass_filter->is_enabled(),
+      public_submodules_->echo_cancellation->is_enabled(),
+      public_submodules_->echo_control_mobile->is_enabled(),
+      public_submodules_->noise_suppression->is_enabled(),
+      capture_nonlocked_.intelligibility_enabled,
+      capture_nonlocked_.beamformer_enabled,
+      public_submodules_->gain_control->is_enabled(),
+      capture_nonlocked_.level_controller_enabled,
+      public_submodules_->voice_detection->is_enabled(),
+      public_submodules_->level_estimator->is_enabled(),
+      capture_.transient_suppressor_enabled);
 }
 
 void AudioProcessingImpl::InitializeExperimentalAgc() {
   if (constants_.use_experimental_agc) {
     if (!private_submodules_->agc_manager.get()) {
       private_submodules_->agc_manager.reset(new AgcManagerDirect(
           public_submodules_->gain_control.get(),
           public_submodules_->gain_control_for_experimental_agc.get(),
           constants_.agc_startup_min_volume));
     }
@@ skipping 308 matching lines @@
       fwd_proc_format(kSampleRate16kHz),
       split_rate(kSampleRate16kHz) {}
 
 AudioProcessingImpl::ApmCaptureState::~ApmCaptureState() = default;
 
 AudioProcessingImpl::ApmRenderState::ApmRenderState() = default;
 
 AudioProcessingImpl::ApmRenderState::~ApmRenderState() = default;
 
 }  // namespace webrtc