audioproc_f with simulated mic analog gain

webrtc/modules/audio_processing/test/aec_dump_based_simulator.cc

 /*
  *  Copyright (c) 2016 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.
  */
 
+#include <algorithm>
 #include <iostream>
+#include <utility>
 
 #include "webrtc/modules/audio_processing/test/aec_dump_based_simulator.h"
 
 #include "webrtc/base/checks.h"
 #include "webrtc/modules/audio_processing/test/protobuf_utils.h"
 #include "webrtc/test/testsupport/trace_to_stderr.h"
 
 namespace webrtc {
 namespace test {
 namespace {
@@ skipping 40 matching lines @@
 
 }  // namespace
 
 AecDumpBasedSimulator::AecDumpBasedSimulator(const SimulationSettings& settings)
     : AudioProcessingSimulator(settings) {}
 
 AecDumpBasedSimulator::~AecDumpBasedSimulator() = default;
 
 void AecDumpBasedSimulator::PrepareProcessStreamCall(
     const webrtc::audioproc::Stream& msg,
-    bool* set_stream_analog_level_called) {
+    bool* update_analog_level) {
   if (msg.has_input_data()) {
     // Fixed interface processing.
     // Verify interface invariance.
     RTC_CHECK(interface_used_ == InterfaceType::kFixedInterface ||
               interface_used_ == InterfaceType::kNotSpecified);
     interface_used_ = InterfaceType::kFixedInterface;
 
     // Populate input buffer.
     RTC_CHECK_EQ(sizeof(fwd_frame_.data_[0]) * fwd_frame_.samples_per_channel_ *
                      fwd_frame_.num_channels_,
@@ skipping 68 matching lines @@
   if (!settings_.use_ts) {
     if (msg.has_keypress()) {
       ap_->set_stream_key_pressed(msg.keypress());
     }
   } else {
     ap_->set_stream_key_pressed(*settings_.use_ts);
   }
 
   // TODO(peah): Add support for controlling the analog level via the
   // command-line.
-  if (msg.has_level()) {
-    RTC_CHECK_EQ(AudioProcessing::kNoError,
-                 ap_->gain_control()->set_stream_analog_level(msg.level()));
-    *set_stream_analog_level_called = true;
-  } else {
-    *set_stream_analog_level_called = false;
+  // If the AECdump does not include a level, AGC was disabled during the call.

[peah-webrtc, 2017/04/26 12:54:44]

No, I don't think it works like this. msg.has_level() is not added only when the
AGC is on. Afaics, the level is always stored in the aecdump for newer
recordings. (please correct me if I'm wrong though).

[aleloi, 2017/05/04 12:47:13]

I just checked, msg->set_level(gain_control()->stream_analog_level()); is always
called. Maybe we should to a simplification and not consider the case
!msg.has_level() ?

+  // If so and if the analog gain simulation is disabled, inform
+  // AudioProcessingSimulator::ProcessStream in order not to call
+  // GainControl::set_stream_analog_level() and
+  // GainControl::stream_analog_level() before and after
+  // AudioProcessing::ProcessStream respectively.
+  *update_analog_level = msg.has_level() || settings_.simulate_mic_gain;
+  if (!settings_.simulate_mic_gain && msg.has_level()) {
+    // The analog gain simulation is off, use the level stored in the AECdump.
+    last_specified_microphone_level_ = msg.level();

[peah-webrtc, 2017/04/26 12:54:44]

I think this CL would be easier to review with an explanation of how you plan to
add the simulation of the analog gain. 

To me it would make sense to have it in the same place as where the msg.level()
is used, so that it is clear where the selection is done, but you have not added
that part yet, right? 

Could you please explain how that is to be added? As it is now, the simulation
causes the reported analog gain to be bypassed, right?

[peah-webrtc, 2017/04/26 12:54:44]

I think the name last_specified_microphone_level_ is a bit misleading here.

It was before this CL present only in the wav-based simulator as a crude way of
mocking an analog gain via allowing the previously set level to be reported as
the current mic gain level.

Here, it is used in a different manner and I think the code would benefit from
it being renamed. For instance, on 172, last_specified_microphone_level is set
to the level in the aecdump which may not at all be the microphone level that
was specified by the AGC (which is how it was before used).

[aleloi, 2017/05/04 12:47:13]

Acknowledged.

   }
 }
 
 void AecDumpBasedSimulator::VerifyProcessStreamBitExactness(
     const webrtc::audioproc::Stream& msg) {
   if (bitexact_output_) {
     if (interface_used_ == InterfaceType::kFixedInterface) {
       bitexact_output_ = VerifyFixedBitExactness(msg, fwd_frame_);
     } else {
       bitexact_output_ = VerifyFloatBitExactness(msg, out_config_, *out_buf_);
@@ skipping 378 matching lines @@
   }
 
   SetupBuffersConfigsOutputs(
       msg.sample_rate(), output_sample_rate, msg.reverse_sample_rate(),
       reverse_output_sample_rate, msg.num_input_channels(), num_output_channels,
       msg.num_reverse_channels(), num_reverse_output_channels);
 }
 
 void AecDumpBasedSimulator::HandleMessage(
     const webrtc::audioproc::Stream& msg) {
-  bool set_stream_analog_level_called = false;
-  PrepareProcessStreamCall(msg, &set_stream_analog_level_called);
-  ProcessStream(interface_used_ == InterfaceType::kFixedInterface);
-  if (set_stream_analog_level_called) {
-    // Call stream analog level to ensure that any side-effects are triggered.
-    (void)ap_->gain_control()->stream_analog_level();
-  }
-
+  bool update_analog_level = true;
+  PrepareProcessStreamCall(msg, &update_analog_level);

[aleloi, 2017/05/04 12:47:13]

I think update_analog_level should be renamed in the method as well.

+  ProcessStream(interface_used_ == InterfaceType::kFixedInterface,
+                update_analog_level);
   VerifyProcessStreamBitExactness(msg);
 }
 
 void AecDumpBasedSimulator::HandleMessage(
     const webrtc::audioproc::ReverseStream& msg) {
   PrepareReverseProcessStreamCall(msg);
   ProcessReverseStream(interface_used_ == InterfaceType::kFixedInterface);
 }
 
 }  // namespace test
 }  // namespace webrtc