AGC2 dummy module: fixed gain param, APM integration, audioproc_f adaptation

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 9b80318e909b980fbcadad3dbe46b66a0e11ad79..65fc63ad0e979644893369f9f2e933ccf6ac6f5e 100644
--- a/webrtc/modules/audio_processing/audio_processing_impl.cc
+++ b/webrtc/modules/audio_processing/audio_processing_impl.cc
@@ -250,7 +250,7 @@ bool AudioProcessingImpl::ApmSubmoduleStates::CaptureMultiBandProcessingActive()
 
 bool AudioProcessingImpl::ApmSubmoduleStates::CaptureFullBandProcessingActive()
     const {
-  return level_controller_enabled_;
+  return level_controller_enabled_ && gain_controller2_enabled_;
 }
 
 bool AudioProcessingImpl::ApmSubmoduleStates::RenderMultiBandSubModulesActive()
@@ -371,6 +371,10 @@ AudioProcessingImpl::AudioProcessingImpl(const webrtc::Config& config,
     // TODO(peah): Move this creation to happen only when the level controller
     // is enabled.
     private_submodules_->level_controller.reset(new LevelController());
+
+    // TODO(alessiob): Move this creation to happen only when the gain
+    // controller is enabled.

[AleBzk, 2017/09/14 09:21:56]

@Per: I replicated the TODO from LC, but the code below will change when we will
have AGC injection.

[peah-webrtc, 2017/09/15 07:44:25]

Acknowledged.

[AleBzk, 2017/09/29 09:39:06]

Done.

+    private_submodules_->gain_controller2.reset(new GainController2());
   }
 
   SetExtraOptions(config);
@@ -676,24 +680,18 @@ void AudioProcessingImpl::ApplyConfig(const AudioProcessing::Config& config) {
                  << capture_nonlocked_.echo_canceller3_enabled;
   }
 
-  config_ok = GainController2::Validate(config_.gain_controller2);
-  if (!config_ok) {
+  if (!GainController2::Validate(config_.gain_controller2)) {

[peah-webrtc, 2017/09/15 07:44:25]

Even though this works, I think it makes sense to follow the same pattern as
above, and not change this from how it was before.

I'm not sure of the background to why it is like that, but I think it is to
simplify debugging using a debugger.

[AleBzk, 2017/09/29 09:39:06]

Done.

     LOG(LS_ERROR) << "AudioProcessing module config error" << std::endl
-                  << "gain_controller2: "
+                  << "Gain Controller 2: "
                   << GainController2::ToString(config_.gain_controller2)
                   << std::endl
                   << "Reverting to default parameter set";
     config_.gain_controller2 = AudioProcessing::Config::GainController2();
   }
-
-  if (config.gain_controller2.enabled !=
-      capture_nonlocked_.gain_controller2_enabled) {
-    capture_nonlocked_.gain_controller2_enabled =
-        config_.gain_controller2.enabled;
-    InitializeGainController2();
-    LOG(LS_INFO) << "Gain controller 2 activated: "
-                 << capture_nonlocked_.gain_controller2_enabled;
-  }
+  InitializeGainController2();

[AleBzk, 2017/09/14 09:21:56]

@Per: every time that we call AudioProcessingImpl::ApplyConfig we both call
Initialize() and ApplyConfig() on the active APM sub-modules. But when we call
AudioProcessingImpl::Initialize() we only call Initialize() on the active APM
sub-modules. is this right in general?

[peah-webrtc, 2017/09/15 07:44:25]

I think so.

+  private_submodules_->gain_controller2->ApplyConfig(config_.gain_controller2);
+  LOG(LS_INFO) << "Gain Controller 2 activated: "
+               << config_.gain_controller2.enabled;
 }
 
 void AudioProcessingImpl::SetExtraOptions(const webrtc::Config& config) {
@@ -1270,7 +1268,7 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() {
         capture_.key_pressed);
   }
 
-  if (capture_nonlocked_.gain_controller2_enabled) {
+  if (config_.gain_controller2.enabled) {
     private_submodules_->gain_controller2->Process(capture_buffer);
   }
 
@@ -1618,7 +1616,7 @@ bool AudioProcessingImpl::UpdateActiveSubmoduleStates() {
       capture_nonlocked_.intelligibility_enabled,
       capture_nonlocked_.beamformer_enabled,
       public_submodules_->gain_control->is_enabled(),
-      capture_nonlocked_.gain_controller2_enabled,
+      config_.gain_controller2.enabled,
       capture_nonlocked_.level_controller_enabled,
       capture_nonlocked_.echo_canceller3_enabled,
       public_submodules_->voice_detection->is_enabled(),
@@ -1680,11 +1678,8 @@ void AudioProcessingImpl::InitializeEchoCanceller3() {
 }
 
 void AudioProcessingImpl::InitializeGainController2() {
-  if (capture_nonlocked_.gain_controller2_enabled) {
-    private_submodules_->gain_controller2.reset(
-        new GainController2(proc_sample_rate_hz()));
-  } else {
-    private_submodules_->gain_controller2.reset();
+  if (config_.gain_controller2.enabled) {
+    private_submodules_->gain_controller2->Initialize(proc_sample_rate_hz());
   }
 }
 
@@ -1700,7 +1695,7 @@ void AudioProcessingImpl::MaybeUpdateHistograms() {
   static const int kMinDiffDelayMs = 60;
 
   if (echo_cancellation()->is_enabled()) {
-    // Activate delay_jumps_ counters if we know echo_cancellation is runnning.
+    // Activate delay_jumps_ counters if we know echo_cancellation is running.
     // If a stream has echo we know that the echo_cancellation is in process.
     if (capture_.stream_delay_jumps == -1 &&
         echo_cancellation()->stream_has_echo()) {
@@ -1786,6 +1781,9 @@ void AudioProcessingImpl::WriteAecDumpConfigMessage(bool forced) {
   if (capture_nonlocked_.echo_canceller3_enabled) {
     experiments_description += "EchoCanceller3;";
   }
+  if (config_.gain_controller2.enabled) {
+    experiments_description += "GainController2;";
+  }
 
   InternalAPMConfig apm_config;