Extension and refactoring of the audioproc_f tool to be a fully fledged tool for audio processing m…

webrtc/modules/audio_processing/test/audio_processing_simulator.h

+/*
+ *  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.
+ */
+
+#ifndef WEBRTC_MODULES_AUDIO_PROCESSING_TEST_AUDIO_PROCESSING_SIMULATOR_H_
+#define WEBRTC_MODULES_AUDIO_PROCESSING_TEST_AUDIO_PROCESSING_SIMULATOR_H_
+
+#include <algorithm>

[aluebs-webrtc, 2016/04/27 16:06:48]

Is this include really needed here?

[peah-webrtc, 2016/04/28 07:41:12]

git cl upload complains if it is not there. I think it is needed for the
std::min operation.

[aluebs-webrtc, 2016/04/30 02:08:04]

Acknowledged.

[peah-webrtc, 2016/05/09 11:37:30]

Acknowledged.

+#include <limits>
+#include <memory>
+#include <string>
+
+#include "webrtc/base/constructormagic.h"
+#include "webrtc/base/optional.h"
+#include "webrtc/common_audio/channel_buffer.h"
+#include "webrtc/modules/audio_processing/include/audio_processing.h"
+#include "webrtc/modules/audio_processing/test/test_utils.h"
+#include "webrtc/system_wrappers/include/tick_util.h"
+
+namespace webrtc {
+namespace test {
+
+// Holds all the parameters available for controlling the simulation.
+struct SimulationSettings {
+  rtc::Optional<int> stream_delay;
+  rtc::Optional<int> stream_drift_samples;
+  rtc::Optional<int> output_sample_rate_hz;
+  rtc::Optional<int> output_num_channels;
+  rtc::Optional<int> reverse_output_sample_rate_hz;
+  rtc::Optional<int> reverse_output_num_channels;
+  rtc::Optional<std::string> microphone_positions;
+  int target_angle_degrees = 90;

[aluebs-webrtc, 2016/04/27 16:06:48]

Why are there some settings that are not optional?

[peah-webrtc, 2016/04/28 07:41:12]

I tried to make those that did not need to be optional nonoptional.
For instance, the target_angle_degrees does not need to be optional since it
always needs to be specified if the beamformer is activated (correct?) so it can
piggyback on the use_bf optional. Furthermore, the target_angle_degrees is not
stored in any protobuf recording and therefore there is no need to use an
optional to determine whether the values in the protobuf recording should be
overridden.

There are similar arguments for the other non-optional values. In general I
tried to make as few things optional as possible. 
I'd be happy for any suggestions for changes in this scheme.

[aluebs-webrtc, 2016/04/30 02:08:04]

Sounds good.

[peah-webrtc, 2016/05/09 11:37:30]

Acknowledged.

+  rtc::Optional<std::string> output_filename;
+  rtc::Optional<std::string> reverse_output_filename;
+  rtc::Optional<std::string> input_filename;
+  rtc::Optional<std::string> reverse_input_filename;
+  rtc::Optional<bool> use_aec;
+  rtc::Optional<bool> use_aecm;
+  rtc::Optional<bool> use_agc;
+  rtc::Optional<bool> use_hpf;
+  rtc::Optional<bool> use_ns;
+  rtc::Optional<bool> use_ts;
+  rtc::Optional<bool> use_bf;
+  rtc::Optional<bool> use_ie;
+  rtc::Optional<bool> use_vad;
+  rtc::Optional<bool> use_le;
+  rtc::Optional<bool> use_all;
+  rtc::Optional<int> aec_suppression_level;
+  rtc::Optional<bool> use_delay_agnostic;
+  rtc::Optional<bool> use_extended_filter;
+  rtc::Optional<bool> use_drift_compensation;
+  rtc::Optional<bool> use_aec3;
+  rtc::Optional<int> aecm_routing_mode;
+  rtc::Optional<bool> use_aecm_comfort_noise;
+  rtc::Optional<int> agc_mode;
+  rtc::Optional<int> agc_target_level;
+  rtc::Optional<bool> use_agc_limiter;
+  rtc::Optional<int> agc_compression_gain;
+  rtc::Optional<int> vad_likelihood;
+  rtc::Optional<int> ns_level;
+  rtc::Optional<bool> use_refined_adaptive_filter;
+  bool report_performance = false;
+  bool report_bitexactness = false;
+  bool use_verbose_logging = false;
+  bool discard_all_settings_in_aecdump = true;
+  rtc::Optional<std::string> aec_dump_input_filename;
+  rtc::Optional<std::string> aec_dump_output_filename;
+  bool fixed_interface = false;
+  bool store_intermediate_output = false;
+};
+
+// Holds a few statistics about a series of TickIntervals.
+struct TickIntervalStats {
+  TickIntervalStats() : min(std::numeric_limits<int64_t>::max()) {}
+  TickInterval sum;
+  TickInterval max;
+  TickInterval min;
+};
+
+// Copies samples present in a ChannelBuffer into an AudioFrame.
+void CopyToAudioFrame(const ChannelBuffer<float>& src, AudioFrame* dest);
+
+// Provides common functionality for performing audioprocessing simulations.
+class AudioProcessingSimulator {
+ public:
+  static const int kChunksPerSecond = 1000 / AudioProcessing::kChunkSizeMs;
+
+  explicit AudioProcessingSimulator(const SimulationSettings& settings)
+      : settings_(settings) {}
+  virtual ~AudioProcessingSimulator() {}
+
+  // Processes the data in the input.
+  virtual void Process() = 0;
+
+  // Returns the execution time of all AudioProcessing calls.
+  const TickIntervalStats& proc_time() const { return proc_time_; }
+
+  // Reports whether the processed recording was bitexact.
+  bool OutputWasBitexact() { return bitexact_output_; }
+
+  size_t get_num_process_stream_calls() { return num_process_stream_calls_; }
+  size_t get_num_reverse_process_stream_calls() {
+    return num_reverse_process_stream_calls_;
+  }
+
+ protected:
+  // RAII class for execution time measurement. Updates the provided
+  // TickIntervalStats based on the time between ScopedTimer creation and
+  // leaving the enclosing scope.
+  class ScopedTimer {
+   public:
+    explicit ScopedTimer(TickIntervalStats* proc_time)
+        : proc_time_(proc_time), start_time_(TickTime::Now()) {}
+
+    ~ScopedTimer();
+
+   private:
+    TickIntervalStats* const proc_time_;
+    TickTime start_time_;
+  };
+
+  TickIntervalStats* mutable_proc_time() { return &proc_time_; }
+
+ private:
+  void SetupOutput();
+
+  size_t num_process_stream_calls_ = 0;
+  size_t num_reverse_process_stream_calls_ = 0;
+  size_t output_reset_counter_ = 0;

[aluebs-webrtc, 2016/04/27 16:06:48]

These default values should be set in the constructor instead.

[peah-webrtc, 2016/04/28 07:41:12]

I think the guideline is agnostic to that (right?). I've actually often gotten
feedback from others that it is better to put the default initializers here
rather than in the constructor.

I don't have a strong opinion on this but I prefer this way as it does not
clutter the constructor, produces fewer lines of code and keeps the
initialization close to where the variables are. 
Wdyt? Are you ok with keeping them as they are.

[aluebs-webrtc, 2016/04/30 02:08:04]

Agreed on leaving as is.

[peah-webrtc, 2016/05/09 11:37:30]

Acknowledged.

+  std::unique_ptr<ChannelBufferWavWriter> buffer_writer_;
+  std::unique_ptr<ChannelBufferWavWriter> reverse_buffer_writer_;
+
+ protected:

[aluebs-webrtc, 2016/04/27 16:06:48]

Why 2 protected and private blocks? It is confusing.

[peah-webrtc, 2016/04/28 07:41:11]

My mistake. 
Done.

+  void ProcessStream(bool fixed_interface);
+  void ProcessReverseStream(bool fixed_interface);
+  void CreateAudioProcessor();
+  void DestroyAudioProcessor();
+  void SetupBuffersConfigsOutputs(int input_sample_rate_hz,
+                                  int output_sample_rate_hz,
+                                  int reverse_input_sample_rate_hz,
+                                  int reverse_output_sample_rate_hz,
+                                  int input_num_channels,
+                                  int output_num_channels,
+                                  int reverse_input_num_channels,
+                                  int reverse_output_num_channels);
+
+  const SimulationSettings& settings_;

[aluebs-webrtc, 2016/04/27 16:06:48]

Why a reference? I think this is confusing.

[peah-webrtc, 2016/04/28 07:41:12]

Done.

+  std::unique_ptr<AudioProcessing> ap_;
+
+  std::unique_ptr<ChannelBuffer<float>> in_buf_;
+  std::unique_ptr<ChannelBuffer<float>> out_buf_;
+  std::unique_ptr<ChannelBuffer<float>> reverse_in_buf_;
+  std::unique_ptr<ChannelBuffer<float>> reverse_out_buf_;
+  StreamConfig in_config_;
+  StreamConfig out_config_;
+  StreamConfig reverse_in_config_;
+  StreamConfig reverse_out_config_;
+  std::unique_ptr<ChannelBufferWavReader> buffer_reader_;
+  std::unique_ptr<ChannelBufferWavReader> reverse_buffer_reader_;
+  AudioFrame rev_frame_;
+  AudioFrame fwd_frame_;
+  bool bitexact_output_ = true;
+
+ private:
+  TickIntervalStats proc_time_;
+
+  RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(AudioProcessingSimulator);
+};
+
+}  // namespace test
+}  // namespace webrtc
+
+#endif  // WEBRTC_MODULES_AUDIO_PROCESSING_TEST_AUDIO_PROCESSING_SIMULATOR_H_