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

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 011325f8ca1cc00a525d01e11ef296db4ecddf52..14138d50907e3f6dac90c33669b25d71eb10504a 100644
--- a/webrtc/modules/audio_processing/audio_processing_impl.cc
+++ b/webrtc/modules/audio_processing/audio_processing_impl.cc
@@ -116,6 +116,14 @@ int ClosestHigherNativeRate(int min_proc_rate) {
 
 }  // 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");
 
@@ -189,8 +197,7 @@ AudioProcessingImpl::AudioProcessingImpl(const Config& config,
                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_);
@@ -238,6 +245,29 @@ int AudioProcessingImpl::Initialize() {
   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,
@@ -427,16 +457,6 @@ void AudioProcessingImpl::SetExtraOptions(const Config& config) {
     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) {