audioproc_f with simulated mic analog gain

webrtc/modules/audio_processing/test/fake_recording_device.h

+/*
+ *  Copyright (c) 2017 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_TEST_FAKE_RECORDING_DEVICE_H_
+#define WEBRTC_MODULES_AUDIO_PROCESSING_TEST_FAKE_RECORDING_DEVICE_H_
+
+#include <algorithm>
+#include <vector>
+
+#include "webrtc/base/array_view.h"
+#include "webrtc/base/checks.h"
+#include "webrtc/modules/include/module_common_types.h"
+
+namespace webrtc {
+
+// Class for simulating a microphone with analog gain. The class wraps a mapping

[peah-webrtc, 2017/05/05 20:25:21]

This seems out of date, could you please update.

[AleBzk, 2017/05/16 08:53:03]

Done.

+// from the current level to a floating point scaling factor.
+//
+// The intended mode of operation is the following:
+//
+// set_analog_level(|mic gain level|);
+// NotifyAudioDeviceLevel(|recorded level of real microphone|); // Optional!

[peah-webrtc, 2017/05/05 20:25:21]

NotifyAudioDeviceLevel Is no longer present, right?

[AleBzk, 2017/05/16 08:53:03]

Done.

+// SimulateAnalogGain(src, dest);
+//
+// In these three calls, the fake device optionally undoes the gain
+// applied by the real microphone, and then applies a new mic gain level.
+class FakeRecordingDevice final {
+ public:
+  static const int kRealDeviceLevelUnknown = -1;
+
+  enum class LevelToScalingMappingKind {

[peah-webrtc, 2017/05/05 20:25:21]

I think the mapping name is too specific. What if you want to simulate two
microphones that both have a linear mapping,  but that differ in the way the
soft-saturation works. I'd propose to call this something more general, like
MicrophoneKind.

[aleloi, 2017/05/08 10:15:23]

+1

[AleBzk, 2017/05/16 08:53:03]

Done.

+    kIdentity,  // Always producing 1.0f.
+    kLinear     // A level within [0, 255] is linearly scaled. 0 produces
+                // 0.f, and 255 is 1.0f.
+  };
+
+  explicit FakeRecordingDevice(LevelToScalingMappingKind mapping_kind);
+
+  ~FakeRecordingDevice();
+
+  // Simulates the analog gain on a std::vector<rtc::ArrayView<>> buffer.
+  // If |real_device_level| is a valid level, the unmodified mic signal is
+  // virtually restored. To skip the latter step set |real_device_level| to
+  // FakeRecordingDevice::kRealDeviceLevelUnknown.
+  void SimulateAnalogGain(std::vector<rtc::ArrayView<const float>> src,

[peah-webrtc, 2017/05/05 20:25:21]

Seeing how this is used, I don't see why there needs to be separate outputs and
inputs (now the same arrayview is used for input as for output). This is in
theory error-prone, if the implementation would rely on them being different. 
I'd rather prefer to have one input/output variable as that is how it is used.

[aleloi, 2017/05/08 10:15:23]

+1

[AleBzk, 2017/05/16 08:53:04]

Done.

+                          std::vector<rtc::ArrayView<float>> dest,

[peah-webrtc, 2017/05/05 20:25:21]

The style guide says that input-only parameters should come first in the
parameter list.

[AleBzk, 2017/05/16 08:53:03]

Done.

+                          int level,
+                          int real_device_level = kRealDeviceLevelUnknown);

[peah-webrtc, 2017/05/05 20:25:21]

As real_device_level is actually always used whenever FakeAudioDevice is used,
there is no need for a default parameter value. 

[AleBzk, 2017/05/16 08:53:03]

Done.

+
+  // Simulates the analog gain on an AudioFrame buffer.
+  // For further details, see the comment above.
+  void SimulateAnalogGain(const AudioFrame* src,
+                          AudioFrame* dest,
+                          int level,
+                          int real_device_level = kRealDeviceLevelUnknown);

[aleloi, 2017/05/08 10:15:23]

Note that the comment about ordering and default param comment applies here as
well.

[AleBzk, 2017/05/16 08:53:04]

Done.

+
+ private:
+  // Computes the PCM samples scaling factor based on the following:
+  // - gain curve (which depends on |mapping_kind_|),

[peah-webrtc, 2017/05/05 20:25:21]

The style guide says "Declaration comments describe use of the function (when it
is non-obvious); comments at the definition of a function describe operation."
As the method is private and the class is small, I'd prefer to move all comments
to the source file for this method.

[AleBzk, 2017/05/16 08:53:04]

Done.

+  // - the desired mic level |level|,
+  // - the real device mic level |real_device_level| (optional).
+  float ComputeCompoundScalingFactor(int level, int real_device_level) const;
+
+  // Compute scaling factor based on current gain curve, which depends
+  // on |mapping_kind_|.
+  float GetScalingFactor(int level) const;
+
+  const LevelToScalingMappingKind mapping_kind_;
+};
+
+}  // namespace webrtc
+
+#endif  // WEBRTC_MODULES_AUDIO_PROCESSING_TEST_FAKE_RECORDING_DEVICE_H_