Introduced new scheme for controlling the functionality inside the audio processing module

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 98 matching lines @@
   for (int rate : AudioProcessing::kNativeSampleRatesHz) {
     if (rate >= min_proc_rate) {
       return rate;
     }
   }
   return AudioProcessing::kMaxNativeSampleRateHz;
 }
 
 }  // namespace
 
+bool ApmSettings::LevelControllerSettings::IsOk() const {

[the sun, 2016/08/30 10:32:38]

I don't think this is the right place for this knowledge (assuming there will be
more conditions to enforce). Eventually, I think we'll have a
LevelController::Config instead, with the LevelController exposed as a separate
processing module. While we still have the monolithic APM, I'd suggest keeping
the struct almost where you've put it:

class AudioProcessing {
...
  struct Config {
    struct LevelController {
      ...
    } level_controller;
    ...
  };

...but to keep the config structs as PODs. Instead, add a "static bool
LevelController::ValidateConfig(const Config& config)" method so we keep as much
knowledge of each module locally.

[peah-webrtc, 2016/08/30 17:05:57]

That makes sense. I've rewritten this. PTAL.

+  return true;
+}
+
+bool ApmSettings::IsOk() const {
+  return level_controller.IsOk();
+}
+
 // Throughout webrtc, it's assumed that success is represented by zero.
 static_assert(AudioProcessing::kNoError == 0, "kNoError must be zero");
 
 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;
@@ skipping 53 matching lines @@
                  config.Get<ExperimentalAgc>().enabled),
 #endif
 #if defined(WEBRTC_ANDROID) || defined(WEBRTC_IOS)
       capture_(false,
 #else
       capture_(config.Get<ExperimentalNs>().enabled,
 #endif
                config.Get<Beamforming>().array_geometry,
                config.Get<Beamforming>().target_direction),
       capture_nonlocked_(config.Get<Beamforming>().enabled,
-                         config.Get<Intelligibility>().enabled,
-                         config.Get<LevelControl>().enabled) {
+                         config.Get<Intelligibility>().enabled) {
   {
     rtc::CritScope cs_render(&crit_render_);
     rtc::CritScope cs_capture(&crit_capture_);
 
     public_submodules_->echo_cancellation.reset(
         new EchoCancellationImpl(&crit_render_, &crit_capture_));
     public_submodules_->echo_control_mobile.reset(
         new EchoControlMobileImpl(&crit_render_, &crit_capture_));
     public_submodules_->gain_control.reset(
         new GainControlImpl(&crit_capture_, &crit_capture_));
@@ skipping 27 matching lines @@
 #endif
 }
 
 int AudioProcessingImpl::Initialize() {
   // Run in a single-threaded manner during initialization.
   rtc::CritScope cs_render(&crit_render_);
   rtc::CritScope cs_capture(&crit_capture_);
   return InitializeLocked();
 }
 
+int AudioProcessingImpl::ApplySettings(const ApmSettings& settings) {
+  bool settings_ok = settings.IsOk();
+  RTC_DCHECK(settings_ok);
+
+  if (!settings_ok) {

[hlundin-webrtc, 2016/08/30 11:26:26]

I think you can be more aggressive, and simply RTC_CHECK(settings.IsOk());

You do provide a method for the "user" to validate the settings before calling
ApplySettings.

Then this function cannot fail and should return void. If you do not like this,
then at least make the function return bool.

[peah-webrtc, 2016/08/30 17:05:57]

I think it is not absolutely necessary to CHECK here, so I'll keep the DCHECK. I
will add the bool return type.

+    return -1;
+  }
+
+  // Run in a single-threaded manner when applying the settings.
+  rtc::CritScope cs_render(&crit_render_);
+  rtc::CritScope cs_capture(&crit_capture_);
+
+  capture_nonlocked_.level_controller_enabled =
+      settings.level_controller.enabled;
+  if (capture_nonlocked_.level_controller_enabled) {
+    InitializeLevelController();
+  }
+  LOG(LS_INFO) << "Level controller activated: "
+               << capture_nonlocked_.level_controller_enabled;
+
+  return settings_ok ? 0 : -1;
+}
+
 int AudioProcessingImpl::Initialize(int input_sample_rate_hz,
                                     int output_sample_rate_hz,
                                     int reverse_sample_rate_hz,
                                     ChannelLayout input_layout,
                                     ChannelLayout output_layout,
                                     ChannelLayout reverse_layout) {
   const ProcessingConfig processing_config = {
       {{input_sample_rate_hz,
         ChannelsFromLayout(input_layout),
         LayoutHasKeyboard(input_layout)},
@@ skipping 169 matching lines @@
 
   public_submodules_->echo_cancellation->SetExtraOptions(config);
 
   if (capture_.transient_suppressor_enabled !=
       config.Get<ExperimentalNs>().enabled) {
     capture_.transient_suppressor_enabled =
         config.Get<ExperimentalNs>().enabled;
     InitializeTransient();
   }
 
-  if (capture_nonlocked_.level_controller_enabled !=
-      config.Get<LevelControl>().enabled) {
-    capture_nonlocked_.level_controller_enabled =
-        config.Get<LevelControl>().enabled;
-    LOG(LS_INFO) << "Level controller activated: "
-                 << config.Get<LevelControl>().enabled;
-
-    InitializeLevelController();
-  }
-
 #if WEBRTC_INTELLIGIBILITY_ENHANCER
   if(capture_nonlocked_.intelligibility_enabled !=
      config.Get<Intelligibility>().enabled) {
     capture_nonlocked_.intelligibility_enabled =
         config.Get<Intelligibility>().enabled;
     InitializeIntelligibility();
   }
 #endif
 
 #ifdef WEBRTC_ANDROID_PLATFORM_BUILD
@@ skipping 1068 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