Adding APM configuration in AEC dump.

webrtc/modules/audio_processing/audio_processing_impl.cc

 /*
  *  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 446 matching lines @@
 void AudioProcessingImpl::SetExtraOptions(const Config& config) {
   CriticalSectionScoped crit_scoped(crit_);
   for (auto item : component_list_) {
     item->SetExtraOptions(config);
   }
 
   if (transient_suppressor_enabled_ != config.Get<ExperimentalNs>().enabled) {
     transient_suppressor_enabled_ = config.Get<ExperimentalNs>().enabled;
     InitializeTransient();
   }
+
+#ifdef WEBRTC_AUDIOPROC_DEBUG_DUMP
+  if (debug_file_->Open()) {
+    int err = WriteInitMessage();

[ivoc, 2015/09/22 11:18:54]

Can you explain why this call needs to be added here? Won't it lead to multiple
init messages being written?

[minyue-webrtc, 2015/09/22 11:39:28]

Yes, it may lead to multiple init messages, but won't miss any re-config.

BTW, even with current code, it gives a new init message when input/output
format (sample rate/channel) changes.

Since SetExtraOptions should only happen at beginning of a call, there should be
no harm to add an init here.

[peah-webrtc, 2015/09/22 12:05:29]

I'm very new to the protobuf concept, what is the problem of having multiple
init messages?

I don't think we can rely on that SetExtraOptions happens only at the beginning
of the call, or can we?

[minyue-webrtc, 2015/09/22 13:19:29]

I do not know better, and so I included Ivo in the reviewers. Init message is
defined here and not special to protobuf, you can call it anything. Messages are
serialized. So if there is a SetExtraOptions invoked during a WebRTC call, it
will stuff in a "init" message, and then behave as normal.  

There is problem with our unpack.cc is that it overwrite the audio files (input,
ref_out, reverse.wav/float/pcm) when a new init message is found. So we now
always see one set of audio files although there can be multiple init messages
in settings.txt. This can be fixed in unpack.cc.
There is no guarantee that SetExtraOptions should not happen during a call,
since APM is not fully RAII. But it is generally true. In addition, as said,
adding init messages should be no harm. (probably better with a fix in
unpack.cc)

[peah-webrtc, 2015/09/24 09:28:01]

Acknowledged.

[peah-webrtc, 2015/09/24 09:28:01]

It definitely makes sense to have unpack.cc restart the writing of the audio
after an init. And I think your extension for unpack to produce separate audio
outputs is great. 

+    if (err != kNoError) {
+      LOG(LS_ERROR) << "Failed to update debug dump during SetExtraOptions.";
+    }
+  }
+#endif
 }
 
 int AudioProcessingImpl::input_sample_rate_hz() const {
   CriticalSectionScoped crit_scoped(crit_);
   return api_format_.input_stream().sample_rate_hz();
 }
 
 int AudioProcessingImpl::sample_rate_hz() const {
   CriticalSectionScoped crit_scoped(crit_);
   return api_format_.input_stream().sample_rate_hz();
@@ skipping 755 matching lines @@
   msg->set_sample_rate(api_format_.input_stream().sample_rate_hz());
   msg->set_num_input_channels(api_format_.input_stream().num_channels());
   msg->set_num_output_channels(api_format_.output_stream().num_channels());
   msg->set_num_reverse_channels(
       api_format_.reverse_input_stream().num_channels());
   msg->set_reverse_sample_rate(
       api_format_.reverse_input_stream().sample_rate_hz());
   msg->set_output_sample_rate(api_format_.output_stream().sample_rate_hz());
   // TODO(ekmeyerson): Add reverse output fields to event_msg_.
 
+  // Acoustic echo canceler
+  msg->set_aec_enabled(echo_cancellation_->is_enabled());
+  msg->set_aec_delay_agnostic(echo_cancellation_->is_delay_logging_enabled());

[peah-webrtc, 2015/09/22 12:05:29]

The delay logging is set if the delay agnostic aec is active but it may also be
set explicitly without that being active. Therefore this is not the correct
indicator to tell whether the delay agnostic aec is active. Most likely we
should add a member function for reporting that.

[minyue-webrtc, 2015/09/22 13:19:29]

Sorry, it is my bad, I chose a wrong flag to report

+
+  msg->set_aec_drift_compensation(
+      echo_cancellation_->is_drift_compensation_enabled());
+  msg->set_aec_extended_filter(
+      echo_cancellation_->is_extended_filter_enabled());
+  msg->set_aec_suppression_level(echo_cancellation_->suppression_level());
+
+  // Mobile AEC
+  msg->set_aecm_enabled(echo_control_mobile_->is_enabled());
+  msg->set_aecm_comfort_noise(
+      echo_control_mobile_->is_comfort_noise_enabled());
+  msg->set_aecm_routing_mode(echo_control_mobile_->routing_mode());
+
+  // Automatic gain controller
+  msg->set_agc_enabled(gain_control_->is_enabled());
+  msg->set_agc_mode(gain_control_->mode());
+  msg->set_agc_limiter(gain_control_->is_limiter_enabled());
+
+  // High pass filter
+  msg->set_hpf_enabled(high_pass_filter_->is_enabled());
+
+  // Noise suppression
+  msg->set_ns_enabled(noise_suppression_->is_enabled());
+  msg->set_ns_experiment(noise_suppression_->is_enabled());

[minyue-webrtc, 2015/09/24 17:09:58]

This was not right. noise suppression experiment is used to determine transient
suppressor, so here it should be whether transient suppressor is on or not.

+  msg->set_ns_level(noise_suppression_->level());
+
   int err = WriteMessageToDebugFile();
   if (err != kNoError) {
     return err;
   }
 
   return kNoError;
 }
 #endif  // WEBRTC_AUDIOPROC_DEBUG_DUMP
 
 }  // namespace webrtc