Created a test that reports the statistics for the duration of APM stream processing API calls.

webrtc/modules/audio_processing/audio_processing_lock_performance_unittest.cc

Index: webrtc/modules/audio_processing/audio_processing_lock_performance_unittest.cc
diff --git a/webrtc/modules/audio_processing/audio_processing_lock_performance_unittest.cc b/webrtc/modules/audio_processing/audio_processing_lock_performance_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..a2f23535554fbc7dc0eb0e5276b53349a5bcbc77
--- /dev/null
+++ b/webrtc/modules/audio_processing/audio_processing_lock_performance_unittest.cc
@@ -0,0 +1,672 @@
+/*
+ *  Copyright (c) 2014 The WebRTC project authors. All Rights Reserved.

[hlundin-webrtc, 2015/11/13 12:18:32]

2015

[peah-webrtc, 2015/11/18 10:27:26]

Done.

+ *
+ *  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 "webrtc/modules/audio_processing/audio_processing_impl.h"
+
+#include <math.h>
+
+#include <algorithm>
+#include <vector>
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "webrtc/base/array_view.h"
+#include "webrtc/base/criticalsection.h"
+#include "webrtc/config.h"
+#include "webrtc/modules/audio_processing/test/test_utils.h"
+#include "webrtc/modules/include/module_common_types.h"
+#include "webrtc/system_wrappers/include/clock.h"
+#include "webrtc/system_wrappers/include/event_wrapper.h"
+#include "webrtc/system_wrappers/include/sleep.h"
+#include "webrtc/system_wrappers/include/thread_wrapper.h"
+#include "webrtc/test/random.h"
+#include "webrtc/test/testsupport/perf_test.h"
+
+namespace webrtc {
+
+namespace {
+
+class CallSimulator;
+
+// Populates a float audio frame with random data.

[hlundin-webrtc, 2015/11/13 12:18:31]

It seems to populate more than one frame. Make the comment reflect that.

[peah-webrtc, 2015/11/18 10:27:26]

It supports multiple channel frames, and I think it would make sense to extend
this test in the (very near) future to use it to benchmark the complexity of the
APM submodules such as the beamformer, which requires stereo input.

I extended the comment a bit to reflect that it generates a multi-channel frame.
Please let me know whether it is fine!

Done.

+void PopulateAudioFrame(float** frame,

[hlundin-webrtc, 2015/11/13 12:18:31]

frame is an output parameter, it should go last in the parameter list.

[peah-webrtc, 2015/11/18 10:27:27]

Done.

+                        float amplitude,
+                        size_t num_channels,
+                        size_t samples_per_channel,
+                        test::Random* rand_gen) {
+  for (size_t ch = 0; ch < num_channels; ch++) {
+    for (size_t k = 0; k < samples_per_channel; k++) {
+      // Store random 16 bit quantized float number between +-amplitude.

[hlundin-webrtc, 2015/11/13 12:18:32]

Where does the 16-bit quantization happen? Is it implicit in Rand?

[peah-webrtc, 2015/11/18 10:27:27]

Sorry, that comment is completely wrong! Removed it!

Done.

+      frame[ch][k] = amplitude * (2 * rand_gen->Rand<float>() - 1);
+    }
+  }
+}
+
+// Variant of echo canceller settings to use in the test.
+enum class AecType { BasicWebRtcAecSettings, BasicWebRtcAecmSettings };
+
+// Variables related to the audio data and formats.
+struct AudioFrameData {
+  explicit AudioFrameData(int max_frame_size) {

[hlundin-webrtc, 2015/11/13 12:18:32]

size_t

[peah-webrtc, 2015/11/18 10:27:26]

Done.

+    // Set up the two-dimensional arrays needed for the APM API calls.
+    input_framechannels.resize(2 * max_frame_size);
+    input_frame.resize(2);
+    input_frame[0] = &input_framechannels[0];
+    input_frame[1] = &input_framechannels[max_frame_size];
+
+    output_frame_channels.resize(2 * max_frame_size);
+    output_frame.resize(2);
+    output_frame[0] = &output_frame_channels[0];
+    output_frame[1] = &output_frame_channels[max_frame_size];
+  }
+
+  std::vector<float> output_frame_channels;
+  std::vector<float*> output_frame;
+  std::vector<float> input_framechannels;
+  std::vector<float*> input_frame;
+  StreamConfig input_stream_config;
+  StreamConfig output_stream_config;
+};
+
+// The configuration for the test.
+struct SimulationConfig {
+  SimulationConfig(int sample_rate_hz,
+                   AecType aec_type,
+                   int min_number_of_calls,
+                   bool use_realtime_simulation)
+      : sample_rate_hz(sample_rate_hz),
+        aec_type(aec_type),
+        min_number_of_calls(min_number_of_calls),
+        use_realtime_simulation(use_realtime_simulation) {}
+  int sample_rate_hz = 16000;
+  AecType aec_type = AecType::BasicWebRtcAecSettings;
+  int min_number_of_calls = 300;
+  bool use_realtime_simulation;

[hlundin-webrtc, 2015/11/13 12:18:32]

All members but this one has default values.

[peah-webrtc, 2015/11/18 10:27:26]

Done.

+};
+
+// Handler for the frame counters.
+class FrameCounters {
+ public:
+  int IncreaseRenderCounter() {
+    rtc::CritScope cs(&crit_);
+    render_count++;
+    return render_count;
+  }
+
+  int IncreaseCaptureCounter() {
+    rtc::CritScope cs(&crit_);
+    capture_count++;
+    return capture_count;
+  }
+
+  int GetCaptureCounter() {
+    rtc::CritScope cs(&crit_);
+    return capture_count;
+  }
+
+  int GetRenderCounter() {
+    rtc::CritScope cs(&crit_);
+    return render_count;
+  }
+
+  int CaptureMinusRenderCounters() {
+    rtc::CritScope cs(&crit_);
+    return capture_count - render_count;
+  }
+
+  int RenderMinusCaptureCounters() {

[hlundin-webrtc, 2015/11/13 12:18:32]

Do you need both "minus" methods? And if you do, you could implement this one as
int RenderMinusCaptureCounters() {
  return -CaptureMinusRenderCounters();
}

[peah-webrtc, 2015/11/18 10:27:25]

It is mainly for convenience and to (tiny) increase the code readability in the
actual test code.

I will add the change in the function. Please let me know if you also want me to
instead move the -CaptureMinusRenderCounters() to the test code.

Done.

+    rtc::CritScope cs(&crit_);
+    return render_count - capture_count;
+  }
+
+  bool BothCountersExceedeThreshold(int threshold) {
+    rtc::CritScope cs(&crit_);
+    return (render_count > threshold && capture_count > threshold);
+  }
+
+ private:
+  rtc::CriticalSection crit_;
+  int render_count GUARDED_BY(crit_) = 0;

[hlundin-webrtc, 2015/11/13 12:18:32]

Trailing underscore on both counter variables.

[peah-webrtc, 2015/11/18 10:27:26]

Done.

+  int capture_count GUARDED_BY(crit_) = 0;
+};
+
+// Checker for whether the capture side has been called.
+class CaptureSideCalledChecker {

[hlundin-webrtc, 2015/11/13 12:18:32]

Generalize the name to make it shorter. Nothing within the class implementation
limits its use to the capture side. It is essentially a locked bool; maybe call
it LockedBool?

[peah-webrtc, 2015/11/18 10:27:26]

Good point! I called it LockedFlag since it is does not provide all the
functionality that a bool does.

Done.

+ public:
+  bool CaptureSideCalled() {

[hlundin-webrtc, 2015/11/13 12:18:32]

This is a simple "getter" function, albeit with a lock. I think you should name
it as such: 

bool capture_side_called() const {
 ...
}

Getters are normally const methods, but since you have a lock you cannot do that
unless you declare the lock mutable. Up to you.

[peah-webrtc, 2015/11/18 10:27:26]

Done.

+    rtc::CritScope cs(&crit_);
+    return capture_side_called_;
+  }
+
+  void FlagCaptureSideCalled() {

[hlundin-webrtc, 2015/11/13 12:18:32]

This is a "setter" with a lock. Name it such:
void set_capture_side_called(bool value) {
  ...
}

[peah-webrtc, 2015/11/18 10:27:26]

Done.

+    rtc::CritScope cs(&crit_);
+    capture_side_called_ = true;
+  }
+
+ private:
+  rtc::CriticalSection crit_;
+  bool capture_side_called_ GUARDED_BY(crit_) = false;
+};
+
+// Parent class for the thread processors.
+class TimedThreadApiProcessor {
+ public:
+  TimedThreadApiProcessor(int max_frame_size,
+                          test::Random* rand_gen,
+                          FrameCounters* shared_counters_state,
+                          CaptureSideCalledChecker* capture_call_checker,
+                          CallSimulator* test_framework,
+                          SimulationConfig* simulation_config,
+                          AudioProcessing* apm,
+                          int min_number_of_calls,
+                          float input_level);
+  virtual ~TimedThreadApiProcessor() {}
+
+  bool ProcessClocked10msQuantum();
+  std::vector<int64_t> get_api_call_durations() const {
+    return api_call_durations_;
+  }
+  int64_t GetDurationStandardDeviation() const;
+  int64_t GetDurationAverage() const;
+
+ protected:
+  static const int kMaxCallDifference = 10;
+
+  void PrepareFrame();
+  void AddJitter(int max_jitter_microseconds, bool random_jitter);
+  virtual int Process() = 0;
+  virtual bool ReadyToProcess() = 0;
+
+  test::Random* rand_gen_ = nullptr;
+  FrameCounters* frame_counters_ = nullptr;
+  CaptureSideCalledChecker* capture_call_checker_ = nullptr;
+  CallSimulator* test_ = nullptr;
+  SimulationConfig* simulation_config_ = nullptr;
+  AudioProcessing* apm_ = nullptr;
+  AudioFrameData frame_data_;
+  webrtc::Clock* clock_;
+  int num_durations_to_store_;
+  std::vector<int64_t> api_call_durations_;
+  const float input_level_;
+};
+
+// Class for handling the capture side processing.
+class CaptureProcessor : public TimedThreadApiProcessor {

[hlundin-webrtc, 2015/11/13 12:18:31]

Mark this class final.

[peah-webrtc, 2015/11/18 10:27:26]

Done.

+ public:
+  CaptureProcessor(int max_frame_size,
+                   test::Random* rand_gen,
+                   FrameCounters* shared_counters_state,
+                   CaptureSideCalledChecker* capture_call_checker,
+                   CallSimulator* test_framework,
+                   SimulationConfig* simulation_config,
+                   AudioProcessing* apm,
+                   int min_number_of_calls,
+                   float input_level);
+
+ private:
+  // Overrides from TimedThreadApiProcessor.
+  int Process() override;
+  bool ReadyToProcess() override;
+
+  bool first_capture_side_call_ = true;
+};
+
+// Class for handling the render side processing.
+class RenderProcessor : public TimedThreadApiProcessor {

[hlundin-webrtc, 2015/11/13 12:18:31]

Mark this class final.

[peah-webrtc, 2015/11/18 10:27:26]

Done.

+ public:
+  RenderProcessor(int max_frame_size,
+                  test::Random* rand_gen,
+                  FrameCounters* shared_counters_state,
+                  CaptureSideCalledChecker* capture_call_checker,
+                  CallSimulator* test_framework,
+                  SimulationConfig* simulation_config,
+                  AudioProcessing* apm,
+                  int min_number_of_calls,
+                  float input_level);
+
+ private:
+  // Overrides from TimedThreadApiProcessor
+  int Process() override;
+  bool ReadyToProcess() override;
+
+  bool first_render_side_call_ = true;
+};
+
+// Class for managing the test simulation.
+class CallSimulator {

[hlundin-webrtc, 2015/11/13 12:18:32]

I think this class should be made a gtest fixture class. That is, let it inherit
from ::testing::Test, and change the TEST()s below to TEST_F(CallSimulator,
...). That way, the gtest framework will invoke SetUp() and TearDown() for you.

You won't be able to pass arguments to the fixture's constructor. Instead, you
can create a new method in this class that allows the TEST_F to set the config.
Or create a value-parameterized test.

[peah-webrtc, 2015/11/18 10:27:26]

Done.

+ public:
+  explicit CallSimulator(SimulationConfig simulation_config);
+  EventTypeWrapper Run(int time_out_limit);
+
+  // Tests whether all the required render and capture side calls have been
+  // done.
+  void CheckIfSimulationDone() {
+    if (frame_counters_.BothCountersExceedeThreshold(
+            simulation_config_.min_number_of_calls)) {
+      test_complete_->Set();
+    }
+  }
+
+  const CaptureProcessor* GetCapture() const { return &capture_thread_state_; }
+  const RenderProcessor* GetRender() const { return &render_thread_state_; }
+  void SetUp();
+  void TearDown();
+
+ private:
+  static const int kMaxFrameSize = 480;
+  static const float kCaptureInputFloatLevel;
+  static const float kRenderInputFloatLevel;
+
+  // Thread callback for the render thread.
+  static bool RenderProcessorThreadFunc(void* context) {
+    return reinterpret_cast<CallSimulator*>(context)
+        ->render_thread_state_.ProcessClocked10msQuantum();
+  }
+
+  // Thread callback for the capture thread.
+  static bool CaptureProcessorThreadFunc(void* context) {
+    return reinterpret_cast<CallSimulator*>(context)
+        ->capture_thread_state_.ProcessClocked10msQuantum();
+  }
+
+  // Start the threads used in the test.
+  void StartThreads() {
+    ASSERT_TRUE(render_thread_->Start());
+    render_thread_->SetPriority(kRealtimePriority);
+    ASSERT_TRUE(capture_thread_->Start());
+    capture_thread_->SetPriority(kRealtimePriority);
+  }
+
+  // Event handler for the test.
+  const rtc::scoped_ptr<EventWrapper> test_complete_;
+
+  // Thread related variables.
+  rtc::scoped_ptr<ThreadWrapper> render_thread_;
+  rtc::scoped_ptr<ThreadWrapper> capture_thread_;
+  mutable test::Random rand_gen_;

[hlundin-webrtc, 2015/11/13 12:18:31]

Why is this mutable?

[peah-webrtc, 2015/11/18 10:27:26]

Done.

+
+  rtc::scoped_ptr<AudioProcessing> apm_;
+  SimulationConfig simulation_config_;
+  FrameCounters frame_counters_;
+  CaptureSideCalledChecker capture_call_checker_;
+  RenderProcessor render_thread_state_;
+  CaptureProcessor capture_thread_state_;
+};
+
+const float CallSimulator::kRenderInputFloatLevel = 0.5f;
+const float CallSimulator::kCaptureInputFloatLevel = 0.03125f;
+
+CallSimulator::CallSimulator(SimulationConfig simulation_config)
+    : test_complete_(EventWrapper::Create()),
+      render_thread_(ThreadWrapper::CreateThread(RenderProcessorThreadFunc,
+                                                 this,
+                                                 "render")),
+      capture_thread_(ThreadWrapper::CreateThread(CaptureProcessorThreadFunc,
+                                                  this,
+                                                  "capture")),
+      rand_gen_(42U),
+      apm_(AudioProcessingImpl::Create()),

[hlundin-webrtc, 2015/11/13 12:18:31]

Create() may return null in case of failure. You can place an ASSERT in SetUp().

[peah-webrtc, 2015/11/18 10:27:26]

Done.

+      simulation_config_(simulation_config),
+      render_thread_state_(kMaxFrameSize,
+                           &rand_gen_,
+                           &frame_counters_,
+                           &capture_call_checker_,
+                           this,
+                           &simulation_config_,
+                           apm_.get(),
+                           simulation_config_.min_number_of_calls,
+                           kRenderInputFloatLevel),
+      capture_thread_state_(kMaxFrameSize,
+                            &rand_gen_,
+                            &frame_counters_,
+                            &capture_call_checker_,
+                            this,
+                            &simulation_config_,
+                            apm_.get(),
+                            simulation_config_.min_number_of_calls,
+                            kCaptureInputFloatLevel) {}
+
+// Run the call simulation with a timeout.
+EventTypeWrapper CallSimulator::Run(int time_out_limit) {
+  SetUp();
+  StartThreads();

[hlundin-webrtc, 2015/11/13 12:18:31]

You should make sure that none of the ASSERT_TRUEs in StartThreads() failed.
Either
ASSERT_NO_FATAL_FAILURE(StartThreads());
- or -
StartThreads();
if (::testing::Test::HasFatalFailure()) {
  FAIL();
}

Note that you can drop the ::testing::Test:: prefix if you make CallSimulator
inherit from ::testing::Test.

[peah-webrtc, 2015/11/18 10:27:26]

Done.

+  EventTypeWrapper result = test_complete_->Wait(time_out_limit);
+  TearDown();
+  return result;
+}
+
+// Simulator and APM setup.
+void CallSimulator::SetUp() {
+  ASSERT_EQ(apm_->kNoError, apm_->level_estimator()->Enable(true));
+  ASSERT_EQ(apm_->kNoError, apm_->gain_control()->Enable(true));
+  ASSERT_EQ(apm_->kNoError,
+            apm_->gain_control()->set_mode(GainControl::kAdaptiveAnalog));
+  ASSERT_EQ(apm_->kNoError, apm_->gain_control()->Enable(true));
+  ASSERT_EQ(apm_->kNoError, apm_->noise_suppression()->Enable(true));
+  ASSERT_EQ(apm_->kNoError, apm_->voice_detection()->Enable(true));
+
+  Config config;

[hlundin-webrtc, 2015/11/13 12:18:31]

Move the declaration down to within the else-clause.

[peah-webrtc, 2015/11/18 10:27:26]

Done.

+  if (simulation_config_.aec_type == AecType::BasicWebRtcAecmSettings) {
+    ASSERT_EQ(apm_->kNoError, apm_->echo_control_mobile()->Enable(true));
+    ASSERT_EQ(apm_->kNoError, apm_->echo_cancellation()->Enable(false));
+  } else {
+    ASSERT_EQ(apm_->kNoError, apm_->echo_control_mobile()->Enable(false));
+    ASSERT_EQ(apm_->kNoError, apm_->echo_cancellation()->Enable(true));
+    ASSERT_EQ(apm_->kNoError, apm_->echo_cancellation()->enable_metrics(true));
+    ASSERT_EQ(apm_->kNoError,
+              apm_->echo_cancellation()->enable_delay_logging(true));
+
+    config.Set<ExtendedFilter>(new ExtendedFilter(true));
+    config.Set<DelayAgnostic>(new DelayAgnostic(true));
+    apm_->SetExtraOptions(config);
+  }
+}
+
+void CallSimulator::TearDown() {
+  render_thread_->Stop();
+  capture_thread_->Stop();
+}
+
+TimedThreadApiProcessor::TimedThreadApiProcessor(

[hlundin-webrtc, 2015/11/13 12:18:32]

I think you can define all the member methods within the class declaration
above.

[peah-webrtc, 2015/11/18 10:27:26]

Did this on all methods apart from the Process() method that requires the
CallSimulator implementation to be known in order to properly build.

Done.

+    int max_frame_size,
+    test::Random* rand_gen,
+    FrameCounters* shared_counters_state,
+    CaptureSideCalledChecker* capture_call_checker,
+    CallSimulator* test_framework,
+    SimulationConfig* simulation_config,
+    AudioProcessing* apm,
+    int num_durations_to_store,
+    float input_level)
+    : rand_gen_(rand_gen),
+      frame_counters_(shared_counters_state),
+      capture_call_checker_(capture_call_checker),
+      test_(test_framework),
+      simulation_config_(simulation_config),
+      apm_(apm),
+      frame_data_(max_frame_size),

[hlundin-webrtc, 2015/11/13 12:18:31]

No need to pass max_frame_size as a parameter; simply use kMaxFrameSize.
YAGNI...

[peah-webrtc, 2015/11/18 10:27:27]

Done.

+      clock_(webrtc::Clock::GetRealTimeClock()),
+      num_durations_to_store_(num_durations_to_store),
+      api_call_durations_(num_durations_to_store_),
+      input_level_(input_level) {}
+
+// Implements the callback functionality for the threads.
+bool TimedThreadApiProcessor::ProcessClocked10msQuantum() {
+  PrepareFrame();
+
+  while (!ReadyToProcess()) {
+    AddJitter(2, true);
+  }
+
+  const int64_t start_time = clock_->TimeInMicroseconds();
+  Process();
+  const int64_t end_time = clock_->TimeInMicroseconds();
+  const int duration = static_cast<int>(end_time - start_time);

[hlundin-webrtc, 2015/11/13 12:18:31]

duration_us, to convey the unit.

[hlundin-webrtc, 2015/11/13 12:18:32]

use rtc::checked_cast<int> from base/safe_conversions.h.

[peah-webrtc, 2015/11/18 10:27:26]

Done.

[peah-webrtc, 2015/11/18 10:27:26]

Done.

+  const int time_left_of_10_ms_quantum = 10000 - duration;
+
+  if (simulation_config_->use_realtime_simulation &&
+      time_left_of_10_ms_quantum > 0) {
+    AddJitter(time_left_of_10_ms_quantum, false);
+  }
+
+  test_->CheckIfSimulationDone();
+
+  return true;
+}
+
+int64_t TimedThreadApiProcessor::GetDurationStandardDeviation() const {
+  double variance = 0;
+  const int64_t average_duration = GetDurationAverage();
+  for (int64_t duration : api_call_durations_) {
+    int64_t tmp = duration - average_duration;
+    variance += static_cast<double>(tmp * tmp);
+  }
+  return static_cast<int64_t>(sqrt(variance / api_call_durations_.size()));

[hlundin-webrtc, 2015/11/13 12:18:32]

rtc::checked_cast

[peah-webrtc, 2015/11/18 10:27:27]

Done.

+}
+
+int64_t TimedThreadApiProcessor::GetDurationAverage() const {
+  int64_t average_duration = 0;
+  for (int64_t duration : api_call_durations_) {
+    average_duration += duration;
+  }
+  return average_duration / api_call_durations_.size();
+}
+
+void TimedThreadApiProcessor::PrepareFrame() {
+  // Prepare the audio input data and metadata.
+  frame_data_.input_stream_config.set_sample_rate_hz(
+      simulation_config_->sample_rate_hz);
+  frame_data_.input_stream_config.set_num_channels(1);
+  frame_data_.input_stream_config.set_has_keyboard(false);
+  PopulateAudioFrame(&frame_data_.input_frame[0], input_level_, 1,
+                     (simulation_config_->sample_rate_hz *
+                      AudioProcessing::kChunkSizeMs / 1000),
+                     rand_gen_);
+
+  // Prepare the float audio output data and metadata.
+  frame_data_.output_stream_config.set_sample_rate_hz(
+      simulation_config_->sample_rate_hz);
+  frame_data_.output_stream_config.set_num_channels(1);
+  frame_data_.output_stream_config.set_has_keyboard(false);
+}
+
+void TimedThreadApiProcessor::AddJitter(int max_jitter_microseconds,
+                                        bool random_jitter) {
+  int min_method_duration;
+  if (random_jitter) {
+    min_method_duration = rand_gen_->Rand(0, max_jitter_microseconds);
+  } else {
+    min_method_duration = max_jitter_microseconds;
+  }
+
+  const int64_t start_time = clock_->TimeInMicroseconds();

[hlundin-webrtc, 2015/11/13 12:18:32]

Please, comment on why you cannot just sleep.

[peah-webrtc, 2015/11/18 10:27:26]

Very good point. As I never use this in the test any more, and as I think it
will not be used, I removed this and replaced it with a trivial spinlock
functionality in the one place where it made sense to use it.

Done.

+  int sum = 0;
+  while ((clock_->TimeInMicroseconds() - start_time) < min_method_duration) {
+    for (int k = 0; k < 100; ++k)
+      for (int j = 1; j < 100; ++j) {
+        sum += k % j;
+        sum = sum % 79;
+      }
+    if (sum > 100) {
+      printf("Will not be print out, just present in order to ensure");
+      printf("that sum is evaluated by the compiler\n");
+    }
+  }
+}
+
+CaptureProcessor::CaptureProcessor(

[hlundin-webrtc, 2015/11/13 12:18:32]

There is a non-negligible overhead associated with the class structure of the
Processor classes. I don't know if the code will be easier/shorter, but you
might want to consider having only the base class, and then instantiate each
object with function pointers to file-scope functions for the two
specializations you need. I mean, the Process() method could just as well be two
different stand-alone functions taking the few (four?) member variables they
need as pointers/references. Similar for ReadyToProcess().

[peah-webrtc, 2015/11/18 10:27:25]

Good point!!!
Will implement it like that.

Done.

+    int max_frame_size,
+    test::Random* rand_gen,
+    FrameCounters* shared_counters_state,
+    CaptureSideCalledChecker* capture_call_checker,
+    CallSimulator* test_framework,
+    SimulationConfig* simulation_config,
+    AudioProcessing* apm,
+    int num_durations_to_store,
+    float input_level)
+    : TimedThreadApiProcessor(max_frame_size,
+                              rand_gen,
+                              shared_counters_state,
+                              capture_call_checker,
+                              test_framework,
+                              simulation_config,
+                              apm,
+                              num_durations_to_store,
+                              input_level) {}
+
+int CaptureProcessor::Process() {
+  // Set the stream delay
+  apm_->set_stream_delay_ms(30);
+
+  // Call and time the specified capture side API processing method.
+  const int64_t start_time = clock_->TimeInMicroseconds();
+  const int result = apm_->ProcessStream(
+      &frame_data_.input_frame[0], frame_data_.input_stream_config,
+      frame_data_.output_stream_config, &frame_data_.output_frame[0]);
+  const int64_t end_time = clock_->TimeInMicroseconds();
+
+  const int new_capture_counter = frame_counters_->IncreaseCaptureCounter();
+
+  if (new_capture_counter <= num_durations_to_store_) {
+    api_call_durations_[new_capture_counter - 1] = end_time - start_time;

[hlundin-webrtc, 2015/11/13 12:18:32]

Why not just api_call_durations_.push_back(...)? You can still allocate enough
memory to start with, if you are worried about performance hits while resizing
(I wouldn't be). Also, if you want to stop storing more durations after a
certain size, simply checking size() will do. You don't need
frame_counters_->IncreaseCaptureCounter() to return a value.

[peah-webrtc, 2015/11/18 10:27:26]

Good and very valid point!

Done.

+  }
+
+  if (first_capture_side_call_) {
+    // Flag that the capture side has been called at least once
+    // (needed to ensure that a capture call has been done
+    // before the first render call is performed (implicitly
+    // required by the APM API).
+    capture_call_checker_->FlagCaptureSideCalled();
+    first_capture_side_call_ = false;
+  }
+  return result;
+}
+
+bool CaptureProcessor::ReadyToProcess() {
+  return (frame_counters_->CaptureMinusRenderCounters() <= kMaxCallDifference);
+}
+
+RenderProcessor::RenderProcessor(int max_frame_size,
+                                 test::Random* rand_gen,
+                                 FrameCounters* shared_counters_state,
+                                 CaptureSideCalledChecker* capture_call_checker,
+                                 CallSimulator* test_framework,
+                                 SimulationConfig* simulation_config,
+                                 AudioProcessing* apm,
+                                 int num_durations_to_store,
+                                 float input_level)
+    : TimedThreadApiProcessor(max_frame_size,
+                              rand_gen,
+                              shared_counters_state,
+                              capture_call_checker,
+                              test_framework,
+                              simulation_config,
+                              apm,
+                              num_durations_to_store,
+                              input_level) {}
+
+int RenderProcessor::Process() {
+  // Call and time the specified render side API processing method.
+  const int64_t start_time = clock_->TimeInMicroseconds();
+  const int result = apm_->ProcessReverseStream(
+      &frame_data_.input_frame[0], frame_data_.input_stream_config,
+      frame_data_.output_stream_config, &frame_data_.output_frame[0]);
+  const int64_t end_time = clock_->TimeInMicroseconds();
+
+  const int new_render_counter = frame_counters_->IncreaseRenderCounter();
+
+  if (new_render_counter <= num_durations_to_store_) {
+    api_call_durations_[new_render_counter - 1] = end_time - start_time;
+  }
+
+  return result;
+}
+
+bool RenderProcessor::ReadyToProcess() {
+  // Do not process until at least one capture call has been done.
+  // (implicitly required by the APM API).
+  if (first_render_side_call_ && !capture_call_checker_->CaptureSideCalled()) {
+    return false;
+  }
+
+  // Ensure that the number of render and capture calls do not differ too much.
+  if (frame_counters_->RenderMinusCaptureCounters() > kMaxCallDifference) {
+    return false;
+  }
+
+  first_render_side_call_ = false;
+  return true;
+}
+
+// Runs the simulation and reports the results.
+void RunSimulation(SimulationConfig simulation_config,
+                   int timeout,
+                   bool print_durations) {
+  // Lambda function for printing out the simulation statistics.
+  auto print_simulation_statistics = [](
+      const TimedThreadApiProcessor* processor, std::string processor_name,
+      bool print_durations) {
+    const std::string modifier = "_api_call_duration";
+
+    // Lambda function for creating a test printout string.
+    auto create_mean_and_variance_string = [](int64_t average,
+                                              int64_t standard_dev) {
+      std::string s = std::to_string(average);
+      s += ",";
+      s += std::to_string(standard_dev);
+      return s;
+    };
+
+    webrtc::test::PrintResult(processor_name, modifier, "avg,stdev",
+                              create_mean_and_variance_string(
+                                  processor->GetDurationAverage(),
+                                  processor->GetDurationStandardDeviation()),
+                              "ms", false);
+
+    if (print_durations) {
+      std::vector<int64_t> durations = processor->get_api_call_durations();
+      std::string value_string = "";
+      for (int64_t duration : durations) {
+        value_string += std::to_string(duration) + ",";
+      }
+      webrtc::test::PrintResultList(processor_name, modifier, "durations",
+                                    value_string, "ms", false);
+    }
+  };
+
+  CallSimulator simulator(simulation_config);
+
+  // Run simulation and verify that it did not time out.
+  ASSERT_EQ(EventTypeWrapper::kEventSignaled, simulator.Run(timeout));

[hlundin-webrtc, 2015/11/13 12:18:32]

Maybe EXPECT_EQ instead. It might still be interesting to see the printouts of a
timed-out run.

[peah-webrtc, 2015/11/18 10:27:27]

Done.

+
+  print_simulation_statistics(simulator.GetRender(), "render", print_durations);

[hlundin-webrtc, 2015/11/13 12:18:32]

To me it looks like the print_simulation_statistics lambda is not using any of
the local variables in RunSimulations. But it is using TimedThreadApiProcessor
pointers. Why not make the print functionality a part of TimedThreadApiProcessor
instead?

[peah-webrtc, 2015/11/18 10:27:26]

Done.

+  print_simulation_statistics(simulator.GetCapture(), "capture",
+                              print_durations);
+}
+
+const int kMaxFramesToProcess = 300;
+const int32_t kTestTimeout = 3 * 10 * kMaxFramesToProcess;
+const bool kUseTimedSimulation = false;

[hlundin-webrtc, 2015/11/13 12:18:32]

I don't think it makes much sense to declare this constant. In the tests below,
the constant is either used (meaning false), or true is explicitly given.

Also, the name is confusing. When I read the invocation lines below, I read
"RunSimulation, blah, blah, and use timed simulation", which is the opposite of
what is going on.

I prefer code like
RunSimulation(SimulationConfig(..., false /*use_realtime_simulation*/));
for the constants that change between different runs. The kPrintAllDurations is
fine, even though I think it could be made a file constant that won't have to
clutter the test parameter list.

[peah-webrtc, 2015/11/18 10:27:26]

Good points and I fully agree. This has been partly changed in the refactoring.
In that I also made kPrintAllDurations a file constant.

Done.

+const bool kPrintAllDurations = false;
+}  // anonymous namespace
+
+TEST(AudioProcessingLockPerformanceTest, Aec8kHz) {
+  RunSimulation(SimulationConfig(8000, AecType::BasicWebRtcAecSettings,
+                                 kMaxFramesToProcess, kUseTimedSimulation),
+                kTestTimeout, kPrintAllDurations);
+}
+
+TEST(AudioProcessingLockPerformanceTest, Aec16kHz) {
+  RunSimulation(SimulationConfig(16000, AecType::BasicWebRtcAecSettings,
+                                 kMaxFramesToProcess, kUseTimedSimulation),
+                kTestTimeout, kPrintAllDurations);
+}
+
+TEST(AudioProcessingLockPerformanceTest, Aec32kHz) {
+  RunSimulation(SimulationConfig(32000, AecType::BasicWebRtcAecSettings,
+                                 kMaxFramesToProcess, kUseTimedSimulation),
+                kTestTimeout, kPrintAllDurations);
+}
+
+TEST(AudioProcessingLockPerformanceTest, Aec48kHz) {
+  RunSimulation(SimulationConfig(48000, AecType::BasicWebRtcAecSettings,
+                                 kMaxFramesToProcess, kUseTimedSimulation),
+                kTestTimeout, kPrintAllDurations);
+}
+
+TEST(AudioProcessingLockPerformanceTest, Aecm8kHz) {
+  RunSimulation(SimulationConfig(8000, AecType::BasicWebRtcAecmSettings,
+                                 kMaxFramesToProcess, true),
+                kTestTimeout, kPrintAllDurations);
+}
+
+TEST(AudioProcessingLockPerformanceTest, Aecm16kHz) {
+  RunSimulation(SimulationConfig(16000, AecType::BasicWebRtcAecmSettings,
+                                 kMaxFramesToProcess, true),
+                kTestTimeout, kPrintAllDurations);
+}
+}  // namespace webrtc