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

webrtc/modules/audio_processing/include/audio_processing.h

Index: webrtc/modules/audio_processing/include/audio_processing.h
diff --git a/webrtc/modules/audio_processing/include/audio_processing.h b/webrtc/modules/audio_processing/include/audio_processing.h
index 09e5d5be1cba9b20a8b8210c690c3c23e1dc7709..33a8e6dc4f176ee655323299069206a932bb7c3b 100644
--- a/webrtc/modules/audio_processing/include/audio_processing.h
+++ b/webrtc/modules/audio_processing/include/audio_processing.h
@@ -91,14 +91,6 @@ struct RefinedAdaptiveFilter {
   bool enabled;
 };
 
-// Enables the adaptive level controller.
-struct LevelControl {
-  LevelControl() : enabled(false) {}
-  explicit LevelControl(bool enabled) : enabled(enabled) {}
-  static const ConfigOptionID identifier = ConfigOptionID::kLevelControl;
-  bool enabled;
-};
-
 // Enables delay-agnostic echo cancellation. This feature relies on internally
 // estimated delays between the process and reverse streams, thus not relying
 // on reported system delays. This configuration only applies to
@@ -205,6 +197,13 @@ struct Intelligibility {
 // Usage example, omitting error checking:
 // AudioProcessing* apm = AudioProcessing::Create(0);
 //
+// AudioProcessing::Config config;
+// config.level_controller.enable = true;
+// bool valid_config = config.Validate();
+// if (valid_config) {

[the sun, 2016/09/02 09:37:05]

This doesn't make sense to me. Why would a client do this?

[hlundin-webrtc, 2016/09/06 08:56:04]

I agree that it isn't necessary to do that before calling ApplyConfig. That
method could instead return an error, as it does now.

My thinking was a bit ahead of the current implementation. I suppose we want to
be able to provide a Config to the APM ctor in the future. And in that case, the
user must validate the config before passing it, since the ctor itself has no
option but to crash if the config is invalid.

[peah-webrtc, 2016/09/07 05:42:59]

See my comment below.

[peah-webrtc, 2016/09/07 05:43:00]

I think it makes sense to do it like that. Eventually we want the parameters to
be set at creation time,
and then it makes sense for the user to be able to fail in a graceful manner for
faulty parameters. Without the Validate method, the only way for the user to
know that the parameters are faulty would be via a crash due to a check, or via
some delayed error scheme.

+//   RTC_CHECK(apm->ApplyConfig(config));
+// }
+//
 // apm->high_pass_filter()->Enable(true);
 //
 // apm->echo_cancellation()->enable_drift_compensation(false);
@@ -244,6 +243,23 @@ struct Intelligibility {
 //
 class AudioProcessing {
  public:
+  // The struct below constitutes the new parameter scheme for the
+  // audio processing functionality. It is being introduced gradually and
+  // until it is fully introduced, it is prone to change.
+  //
+  // The parameters and behavior of the audio processing module are controlled
+  // by changing the default values in the AudioProcessing::ApmConfig struct.

[hlundin-webrtc, 2016/09/06 08:56:04]

Not called ApmConfig anymore.

[peah-webrtc, 2016/09/07 05:42:59]

Done.

+  // The config is applied by passing the struct to the ApplyConfig method in
+  // the

[hlundin-webrtc, 2016/09/06 08:56:04]

Incomplete sentence.

[peah-webrtc, 2016/09/07 05:43:00]

Done.

+  // It is possible to verify the specified parameters by calling the
+  // Validate() method.
+  struct Config {
+    struct LevelController {
+      bool enabled = false;
+    } level_controller;
+    bool Validate() const;
+  };
+
   // TODO(mgraczyk): Remove once all methods that use ChannelLayout are gone.
   enum ChannelLayout {
     kMono,
@@ -262,9 +278,9 @@ class AudioProcessing {
   // be one instance for every incoming stream.
   static AudioProcessing* Create();
   // Allows passing in an optional configuration at create-time.
-  static AudioProcessing* Create(const Config& config);
+  static AudioProcessing* Create(const webrtc::Config& config);
   // Only for testing.
-  static AudioProcessing* Create(const Config& config,
+  static AudioProcessing* Create(const webrtc::Config& config,
                                  NonlinearBeamformer* beamformer);
   virtual ~AudioProcessing() {}
 
@@ -300,9 +316,13 @@ class AudioProcessing {
                          ChannelLayout output_layout,
                          ChannelLayout reverse_layout) = 0;
 
+  // TODO(peah): This method is a temporary solution used to take control
+  // over the parameters in the audio processing module and is likely to change.
+  virtual bool ApplyConfig(const Config& config) = 0;
+
   // Pass down additional options which don't have explicit setters. This
   // ensures the options are applied immediately.
-  virtual void SetExtraOptions(const Config& config) = 0;
+  virtual void SetExtraOptions(const webrtc::Config& config) = 0;
 
   // TODO(ajm): Only intended for internal use. Make private and friend the
   // necessary classes?