Introduced the new parameter setting scheme for activating the high-pass filter in APM

webrtc/modules/audio_processing/audio_processing_impl.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.
  */
 
 #ifndef WEBRTC_MODULES_AUDIO_PROCESSING_AUDIO_PROCESSING_IMPL_H_
 #define WEBRTC_MODULES_AUDIO_PROCESSING_AUDIO_PROCESSING_IMPL_H_
 
 #include <list>
 #include <memory>
 #include <string>
 #include <vector>
 
 #include "webrtc/base/criticalsection.h"
 #include "webrtc/base/gtest_prod_util.h"
 #include "webrtc/base/ignore_wundef.h"
 #include "webrtc/base/swap_queue.h"
 #include "webrtc/base/thread_annotations.h"
 #include "webrtc/modules/audio_processing/audio_buffer.h"
+#include "webrtc/modules/audio_processing/high_pass_filter_impl.h"
 #include "webrtc/modules/audio_processing/include/audio_processing.h"
 #include "webrtc/modules/audio_processing/render_queue_item_verifier.h"
 #include "webrtc/system_wrappers/include/file_wrapper.h"
 
 #ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP
 // Files generated at build-time by the protobuf compiler.
 RTC_PUSH_IGNORING_WUNDEF()
 #ifdef WEBRTC_ANDROID_PLATFORM_BUILD
 #include "external/webrtc/webrtc/modules/audio_processing/debug.pb.h"
 #else
@@ skipping 82 matching lines @@
       EXCLUSIVE_LOCKS_REQUIRED(crit_capture_);
 
   // Methods returning pointers to APM submodules.
   // No locks are aquired in those, as those locks
   // would offer no protection (the submodules are
   // created only once in a single-treaded manner
   // during APM creation).
   EchoCancellation* echo_cancellation() const override;
   EchoControlMobile* echo_control_mobile() const override;
   GainControl* gain_control() const override;
+  // TODO(peah): Deprecate this API call.
   HighPassFilter* high_pass_filter() const override;
   LevelEstimator* level_estimator() const override;
   NoiseSuppression* noise_suppression() const override;
   VoiceDetection* voice_detection() const override;
 
+  // TODO(peah): Remove these two methods once the new API allows that.
+  template <typename MutatorFunction>
+  void MutateConfig(MutatorFunction mutator) {

[the sun, 2016/10/28 10:52:53]

Nice! Can we do without it being a template though? The function prototype
should be the same in all cases. You should probably use rtc::FunctionView<>, or
possibly std::function - check with Karl what is the preferred way.

(See
https://chromium.googlesource.com/external/webrtc/+/master/webrtc/modules/aud...
for an example use of rtc::FunctionView<>)

[kwiberg-webrtc, 2016/10/28 11:07:00]

rtc::FunctionView is the right thing to use in a case like this, when the callee
isn't allowed to save the function. However, since this method is simple and not
virtual, I think using a template is fine too (but consider using const& or &&
so that you pass the mutator by reference instead of by value).

[peah-webrtc, 2016/10/28 12:19:52]

Great suggestion!
Done.

[peah-webrtc, 2016/10/28 12:19:52]

Thanks! I think the FunctionView became really nice in this case so I used that.

Done.

[kwiberg-webrtc, 2016/10/28 12:28:56]

No, you used std::function instead (which will also work, although it's a bit
more expensive since it copies the lambda).

[peah-webrtc, 2016/10/28 12:36:12]

You are right! I tried both locally and removed the wrong one.
I changed to the FunctionView instead.

Thanks for checking!!!! 

+    rtc::CritScope cs_render(&crit_render_);
+    rtc::CritScope cs_capture(&crit_capture_);
+    mutator(&config_);
+    ApplyConfig(config_);
+  }
+
+  AudioProcessing::Config GetConfig() const {
+    rtc::CritScope cs_render(&crit_render_);
+    rtc::CritScope cs_capture(&crit_capture_);
+    return config_;
+  }
+
  protected:
   // Overridden in a mock.
   virtual int InitializeLocked()
       EXCLUSIVE_LOCKS_REQUIRED(crit_render_, crit_capture_);
 
  private:
   // TODO(peah): These friend classes should be removed as soon as the new
   // parameter setting scheme allows.
   FRIEND_TEST_ALL_PREFIXES(ApmConfiguration, DefaultBehavior);
   FRIEND_TEST_ALL_PREFIXES(ApmConfiguration, ValidConfigBehavior);
   FRIEND_TEST_ALL_PREFIXES(ApmConfiguration, InValidConfigBehavior);
   struct ApmPublicSubmodules;
   struct ApmPrivateSubmodules;
 
+  // Submodule interface implementations.
+  std::unique_ptr<HighPassFilterImpl> high_pass_filter_impl_;

[the sun, 2016/10/28 10:52:53]

I don't see why you couldn't have a pointer to a HighPassFilter here instead,
provided you make the protected dtor of HighPassFilter public. Then you could
hide the entire impl in the .cc.

[peah-webrtc, 2016/10/28 12:19:52]

That is an awesome suggestion!!!
Done.

+
   class ApmSubmoduleStates {
    public:
     ApmSubmoduleStates();
     // Updates the submodule state and returns true if it has changed.
     bool 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,
@@ skipping 71 matching lines @@
   // acquired.
   void InitializeTransient()
       EXCLUSIVE_LOCKS_REQUIRED(crit_render_, crit_capture_);
   void InitializeBeamformer()
       EXCLUSIVE_LOCKS_REQUIRED(crit_render_, crit_capture_);
   void InitializeIntelligibility()
       EXCLUSIVE_LOCKS_REQUIRED(crit_render_, crit_capture_);
   int InitializeLocked(const ProcessingConfig& config)
       EXCLUSIVE_LOCKS_REQUIRED(crit_render_, crit_capture_);
   void InitializeLevelController() EXCLUSIVE_LOCKS_REQUIRED(crit_capture_);
+  void InitializeHighPassFilter() EXCLUSIVE_LOCKS_REQUIRED(crit_capture_);
 
   void EmptyQueuedRenderAudio();
   void AllocateRenderQueue()
       EXCLUSIVE_LOCKS_REQUIRED(crit_render_, crit_capture_);
   void QueueRenderAudio(AudioBuffer* audio)
       EXCLUSIVE_LOCKS_REQUIRED(crit_render_);
 
   // Capture-side exclusive methods possibly running APM in a multi-threaded
   // manner that are called with the render lock already acquired.
   int ProcessCaptureStreamLocked() EXCLUSIVE_LOCKS_REQUIRED(crit_capture_);
@@ skipping 146 matching lines @@
       SwapQueue<std::vector<int16_t>, RenderQueueItemVerifier<int16_t>>>
       aecm_render_signal_queue_;
   std::unique_ptr<
       SwapQueue<std::vector<int16_t>, RenderQueueItemVerifier<int16_t>>>
       agc_render_signal_queue_;
 };
 
 }  // namespace webrtc
 
 #endif  // WEBRTC_MODULES_AUDIO_PROCESSING_AUDIO_PROCESSING_IMPL_H_