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

webrtc/modules/audio_processing/include/audio_processing.h

 /*
  *  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 73 matching lines @@
 // EchoControlMobile. It can be set in the constructor
 // or using AudioProcessing::SetExtraOptions().
 struct RefinedAdaptiveFilter {
   RefinedAdaptiveFilter() : enabled(false) {}
   explicit RefinedAdaptiveFilter(bool enabled) : enabled(enabled) {}
   static const ConfigOptionID identifier =
       ConfigOptionID::kAecRefinedAdaptiveFilter;
   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
 // EchoCancellation and not EchoControlMobile. It can be set in the constructor
 // or using AudioProcessing::SetExtraOptions().
 struct DelayAgnostic {
   DelayAgnostic() : enabled(false) {}
   explicit DelayAgnostic(bool enabled) : enabled(enabled) {}
   static const ConfigOptionID identifier = ConfigOptionID::kDelayAgnostic;
   bool enabled;
@@ skipping 86 matching lines @@
 //   2. Parameter getters are never called concurrently with the corresponding
 //      setter.
 //
 // APM accepts only linear PCM audio data in chunks of 10 ms. The int16
 // interfaces use interleaved data, while the float interfaces use deinterleaved
 // data.
 //
 // Usage example, omitting error checking:
 // AudioProcessing* apm = AudioProcessing::Create(0);
 //
+// AudioProcessing::Config config;
+// config.level_controller.enable = true;
+// bool config_ok = config.Validate();
+// RTC_DCHECK(config_ok);
+// if (config_ok {

[the sun, 2016/08/30 18:05:35]

No. The client should not be burdened with validating the struct. Just validate
internally in ApplyConfig() and return false if there's a problem (*before*
acting on anything in the struct).

[peah-webrtc, 2016/08/31 08:08:32]

Done.

+//   apm->ApplyConfig(config);
+// }
+//
 // apm->high_pass_filter()->Enable(true);
 //
 // apm->echo_cancellation()->enable_drift_compensation(false);
 // apm->echo_cancellation()->Enable(true);
 //
 // apm->noise_reduction()->set_level(kHighSuppression);
 // apm->noise_reduction()->Enable(true);
 //
 // apm->gain_control()->set_analog_level_limits(0, 255);
 // apm->gain_control()->set_mode(kAdaptiveAnalog);
@@ skipping 19 matching lines @@
 //
 // // Repeate render and capture processing for the duration of the call...
 // // Start a new call...
 // apm->Initialize();
 //
 // // Close the application...
 // delete apm;
 //
 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 ApmConfig struct. It is possible

[the sun, 2016/08/30 18:05:35]

Update comment.

[peah-webrtc, 2016/08/31 08:08:31]

Done.

+  // to verify the specified parameters by calling the IsOk method for each
+  // substruct. The config are applied by passing the struct to the
+  // ApplyConfig method in the audio processing interface. The IsOk()
+  // method of the main struct calls all of the IsOk() methods in the
+  // substructs.
+  struct Config {
+    struct LevelController {
+      bool enabled = false;
+      bool Validate() const;

[the sun, 2016/08/30 18:05:35]

Again, the *knowledge* about a sane state for each config should live in the
actual processing module/component. The monolithic APM should know as little
detail as possible about each of the processors.

[peah-webrtc, 2016/08/31 08:08:32]

Done.

+    } level_controller;
+    bool Validate() const;
+  };
+
   // TODO(mgraczyk): Remove once all methods that use ChannelLayout are gone.
   enum ChannelLayout {
     kMono,
     // Left, right.
     kStereo,
     // Mono, keyboard mic.
     kMonoAndKeyboard,
     // Left, right, keyboard mic.
     kStereoAndKeyboard
   };
 
   // Creates an APM instance. Use one instance for every primary audio stream
   // requiring processing. On the client-side, this would typically be one
   // instance for the near-end stream, and additional instances for each far-end
   // stream which requires processing. On the server-side, this would typically
   // 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() {}
 
+  // 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;

[the sun, 2016/08/30 18:05:35]

nit: Does it really belong before Initialize() ?

[peah-webrtc, 2016/08/31 08:08:31]

Good point! I'll put it by SetExtraOptions

Done.

+
   // Initializes internal states, while retaining all user settings. This
   // should be called before beginning to process a new audio stream. However,
   // it is not necessary to call before processing the first stream after
   // creation.
   //
   // It is also not necessary to call if the audio parameters (sample
   // rate and number of channels) have changed. Passing updated parameters
   // directly to |ProcessStream()| and |ProcessReverseStream()| is permissible.
   // If the parameters are known at init-time though, they may be provided.
   virtual int Initialize() = 0;
@@ skipping 14 matching lines @@
   // TODO(mgraczyk): Remove once clients are updated to use the new interface.
   virtual int 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) = 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?
   virtual int proc_sample_rate_hz() const = 0;
   virtual int proc_split_sample_rate_hz() const = 0;
   virtual size_t num_input_channels() const = 0;
   virtual size_t num_proc_channels() const = 0;
   virtual size_t num_output_channels() const = 0;
   virtual size_t num_reverse_channels() const = 0;
 
@@ skipping 658 matching lines @@
   // This does not impact the size of frames passed to |ProcessStream()|.
   virtual int set_frame_size_ms(int size) = 0;
   virtual int frame_size_ms() const = 0;
 
  protected:
   virtual ~VoiceDetection() {}
 };
 }  // namespace webrtc
 
 #endif  // WEBRTC_MODULES_AUDIO_PROCESSING_INCLUDE_AUDIO_PROCESSING_H_