Unittest for the locking in APM

webrtc/modules/audio_processing/audio_processing_impl_locking_unittest.cc

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

[kwiberg-webrtc, 2015/10/08 13:25:22]

Use the present year without rounding down to the nearest integer multiple of
53.

[peah-webrtc, 2015/10/13 06:58:39]

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 <algorithm>
+#include <vector>
+
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "webrtc/config.h"
+#include "webrtc/base/criticalsection.h"

[ivoc, 2015/10/09 15:47:15]

These should be in alphabetical order, I think.

[peah-webrtc, 2015/10/13 06:58:40]

Done.

+#include "webrtc/modules/audio_processing/test/test_utils.h"
+#include "webrtc/modules/interface/module_common_types.h"
+#include "webrtc/system_wrappers/interface/event_wrapper.h"
+#include "webrtc/system_wrappers/interface/sleep.h"
+#include "webrtc/system_wrappers/interface/thread_wrapper.h"
+
+namespace webrtc {
+
+namespace {
+
+// Holds the type of the render thread APM API call to use in the test.
+enum class RenderApiFunction {
+  ProcessReverseStream1,
+  ProcessReverseStream2,
+  AnalyzeReverseStream1,
+  AnalyzeReverseStream2
+};
+// Holds the type of the capture thread APM API call to use in the test.
+enum class CaptureApiFunction {
+  ProcessStream1,
+  ProcessStream2,
+  ProcessStream3
+};
+// Holds the runtime parameter setting scheme to use in the test.
+enum class RuntimeParameterSettingScheme { Scheme1, Scheme2, Scheme3, Scheme4 };

[ivoc, 2015/10/09 15:47:15]

I think these enums are a bit cryptic. Is it possible to give descriptive names
or at least add some comments explaining the different schemes (this also
applies to the previous 2 enums).

[peah-webrtc, 2015/10/13 06:58:39]

Done.

[peah-webrtc, 2015/10/13 06:58:39]

Good point! Should be better now!

+enum class AecType { Aec, NoAec, AecExtFilter, AecDelayAgnostic, Aecm };
+
+// Holds the configuration for the test to use.

[kwiberg-webrtc, 2015/10/08 13:25:21]

You can probably drop the "Holds the" in all these descriptions.

[peah-webrtc, 2015/10/13 06:58:40]

Done.

+struct TestConfig {
+  RenderApiFunction render_api_function;
+  CaptureApiFunction capture_api_function;
+  RuntimeParameterSettingScheme runtime_parameter_setting_scheme;
+  int initial_sample_rate;
+  AecType aec_type;
+};
+
+// Class for implementing the tests of the locks in the audio processing module.
+class AudioProcessingImpLockTest : public ::testing::TestWithParam<TestConfig> {
+ public:
+  AudioProcessingImpLockTest()
+      : render_thread_(
+            ThreadWrapper::CreateThread(CbRenderThread, this, "render")),
+        capture_thread_(
+            ThreadWrapper::CreateThread(CbCaptureThread, this, "capture")),
+        stats_thread_(
+            ThreadWrapper::CreateThread(CbStatsThread, this, "stats")),
+        render_count_(0),
+        capture_count_(0),
+        first_render_side_call_(true),
+        capture_side_called_(false),
+        test_complete_(EventWrapper::Create()),
+        render_seed(42),
+        capture_seed(37),
+        stats_seed(75),
+        capture_input_sample_rate_hz_(16000),
+        capture_output_sample_rate_hz_(16000),
+        render_input_sample_rate_hz_(16000),
+        render_output_sample_rate_hz_(16000) {
+    // Create the dynamic two-dimensional arrays needed for the APM API calls.
+    capture_input_frame_ = new float*[2];
+    capture_input_frame_[0] = new float[480];
+    capture_input_frame_[1] = new float[480];
+    capture_output_frame_ = new float*[2];
+    capture_output_frame_[0] = new float[480];
+    capture_output_frame_[1] = new float[480];
+    render_input_frame_ = new float*[2];
+    render_input_frame_[0] = new float[480];
+    render_input_frame_[1] = new float[480];
+    render_output_frame_ = new float*[2];
+    render_output_frame_[0] = new float[480];
+    render_output_frame_[1] = new float[480];
+  }
+
+  virtual ~AudioProcessingImpLockTest() {
+    // Delete the dynamic two-dimensional arrays needed for the APM API calls.
+    delete[] capture_input_frame_[0];
+    delete[] capture_input_frame_[1];
+    delete[] capture_input_frame_;
+
+    delete[] capture_output_frame_[0];
+    delete[] capture_output_frame_[1];
+    delete[] capture_output_frame_;
+
+    delete[] render_input_frame_[0];
+    delete[] render_input_frame_[1];
+    delete[] render_input_frame_;
+
+    delete[] render_output_frame_[0];
+    delete[] render_output_frame_[1];
+    delete[] render_output_frame_;

[kwiberg-webrtc, 2015/10/08 13:25:22]

Use scoped_ptrs to hold these. Or even better: since the sizes are known at
compile time, don't allocate them separately.

[ivoc, 2015/10/09 15:47:15]

vectors are another option.

[peah-webrtc, 2015/10/13 06:58:39]

Done.

[peah-webrtc, 2015/10/13 06:58:39]

Good point! Now I changed to a scheme using vectors.

[peah-webrtc, 2015/10/13 06:58:40]

Done.

+  }
+
+  // Run the test with a timeout.
+  EventTypeWrapper RunTest() {
+    StartThreads();
+    return test_complete_->Wait(kTestTimeOutLimit);
+  }
+
+  virtual void SetUp() {
+    apm_.reset(AudioProcessingImpl::Create());
+    test_config_ = static_cast<TestConfig>(GetParam());
+
+    Config config;
+    bool use_config = false;

[kwiberg-webrtc, 2015/10/08 13:25:22]

Move line 123 to 144, to reduce the scope of use_config.

[peah-webrtc, 2015/10/13 06:58:40]

Done.

+
+    EXPECT_EQ(apm_->kNoError, apm_->level_estimator()->Enable(true));
+    EXPECT_EQ(apm_->kNoError, apm_->gain_control()->Enable(true));
+
+    EXPECT_EQ(apm_->kNoError,
+              apm_->gain_control()->set_mode(GainControl::kAdaptiveAnalog));
+    EXPECT_EQ(apm_->kNoError, apm_->gain_control()->Enable(true));
+    EXPECT_EQ(apm_->kNoError,
+              apm_->gain_control()->set_mode(GainControl::kFixedDigital));
+
+    EXPECT_EQ(apm_->kNoError, apm_->noise_suppression()->Enable(true));
+    EXPECT_EQ(apm_->kNoError, apm_->voice_detection()->Enable(true));
+
+    if (test_config_.aec_type == AecType::NoAec) {
+      EXPECT_EQ(apm_->kNoError, apm_->echo_control_mobile()->Enable(false));
+      EXPECT_EQ(apm_->kNoError, apm_->echo_cancellation()->Enable(false));
+    } else {
+      if (test_config_.aec_type == AecType::Aecm) {
+        EXPECT_EQ(apm_->kNoError, apm_->echo_control_mobile()->Enable(true));
+      } else {
+        EXPECT_EQ(apm_->kNoError, apm_->echo_cancellation()->Enable(true));
+        EXPECT_EQ(apm_->kNoError,
+                  apm_->echo_cancellation()->enable_metrics(true));
+        EXPECT_EQ(apm_->kNoError,
+                  apm_->echo_cancellation()->enable_delay_logging(true));
+
+        if (test_config_.aec_type == AecType::AecExtFilter) {
+          config.Set<ExtendedFilter>(new ExtendedFilter(true));
+          use_config = true;
+        }
+
+        if (test_config_.aec_type == AecType::AecDelayAgnostic) {
+          config.Set<DelayAgnostic>(new DelayAgnostic(true));
+          use_config = true;
+        }
+
+        if (use_config)
+          apm_->SetExtraOptions(config);
+      }
+    }
+  }
+
+  virtual void TearDown() {
+    render_thread_->Stop();
+    capture_thread_->Stop();
+    stats_thread_->Stop();
+  }
+
+  // Function for generating the test configurations to use in the tests
+  static std::vector<TestConfig> GenerateTestConfigs() {
+    std::vector<TestConfig> test_configs;
+    // Loop over all possible test configurations
+    for (int render =
+             static_cast<int>(RenderApiFunction::ProcessReverseStream1);
+         render <= static_cast<int>(RenderApiFunction::AnalyzeReverseStream2);
+         render++)
+      for (int capture = static_cast<int>(CaptureApiFunction::ProcessStream1);
+           capture <= static_cast<int>(CaptureApiFunction::ProcessStream3);
+           capture++)
+        for (int aec = static_cast<int>(AecType::Aec);
+             aec <= static_cast<int>(AecType::Aecm); aec++)
+          for (int scheme =
+                   static_cast<int>(RuntimeParameterSettingScheme::Scheme1);
+               scheme <=
+               static_cast<int>(RuntimeParameterSettingScheme::Scheme4);
+               scheme++) {
+            TestConfig test_config;
+            test_config.render_api_function =
+                static_cast<RenderApiFunction>(render);
+            test_config.capture_api_function =
+                static_cast<CaptureApiFunction>(capture);
+            test_config.aec_type = static_cast<AecType>(aec);
+
+            // Check that the selected render and capture API calls are
+            // compatible
+            if ((((test_config.render_api_function ==
+                   RenderApiFunction::ProcessReverseStream1) ||
+                  (test_config.render_api_function ==
+                   RenderApiFunction::AnalyzeReverseStream1)) &&
+                 (test_config.capture_api_function ==
+                  CaptureApiFunction::ProcessStream1)) ||
+                (((test_config.render_api_function !=
+                   RenderApiFunction::ProcessReverseStream1) &&
+                  (test_config.render_api_function !=
+                   RenderApiFunction::AnalyzeReverseStream1)) &&
+                 (test_config.capture_api_function !=
+                  CaptureApiFunction::ProcessStream1))) {
+              // For the compatible render and capture function combinations
+              // add test configs with different initial sample rates and
+              // parameter setting schemes
+              test_config.runtime_parameter_setting_scheme =
+                  static_cast<RuntimeParameterSettingScheme>(scheme);
+
+              test_config.initial_sample_rate = 8000;
+              test_configs.push_back(test_config);
+
+              test_config.initial_sample_rate = 16000;
+              test_configs.push_back(test_config);
+
+              if (test_config.aec_type != AecType::Aecm) {
+                test_config.initial_sample_rate = 32000;
+                test_configs.push_back(test_config);
+
+                test_config.initial_sample_rate = 48000;
+                test_configs.push_back(test_config);
+              }
+            }
+          }
+    // Return the created test configurations
+    return test_configs;
+  }
+
+ private:
+  const int kMinNumCalls = 10000;
+  const int kTestTimeOutLimit = 10 * 60 * 1000;
+  const int kMaxCallDifference = 10;
+  const float kRenderInputFloatLevel = 0.5f;
+  const float kCaptureInputFloatLevel = 0.03125f;
+  const int kRenderInputFixLevel = 16384;
+  const int kCaptureInputFixLevel = 1024;

[kwiberg-webrtc, 2015/10/08 13:25:21]

static const for all of these?

[peah-webrtc, 2015/10/13 06:58:39]

Done.

+
+  // Populates a float audio frame with random data.
+  static void PopulateAudioFrame(float** frame,
+                                 int max_absolute_value,

[the sun, 2015/10/08 12:38:10]

amplitude?

[peah-webrtc, 2015/10/13 06:58:39]

Done.

+                                 int num_channels,
+                                 int samples_per_channel,
+                                 unsigned int* seed) {
+    for (int ch = 0; ch < num_channels; ch++)

[the sun, 2015/10/08 12:38:10]

Please, always use braces.

[peah-webrtc, 2015/10/13 06:58:39]

Done.

+      for (int k = 0; k < samples_per_channel; k++) {
+        // Store random 16 bit quantized float number between the specified
+        // limits.
+        frame[ch][k] =
+            static_cast<float>((rand_r(seed) % (32768 + 32768 + 1)) - 32768) /

[ivoc, 2015/10/09 15:47:15]

I don't understand the "+ 1" here. A 16 bit value can have 2^16 different values
right? This value can have 2^16 + 1 values (normally the max value a 16 bit
unsigned value can take is 2^16 - 1).
IMO, a better way would be to do: (2*(rand_r(seed) % 65536) / 65535.f)-1.f

[peah-webrtc, 2015/10/26 07:34:40]

You are totally correct in that! It is now rewritten.

+            32768.0f;
+        frame[ch][k] *= max_absolute_value;
+      }
+  }
+
+  // Populates an audioframe frame of AudioFrame type with random data.
+  static void PopulateAudioFrame(AudioFrame* frame,
+                                 int max_absolute_value,
+                                 unsigned int* seed) {
+    for (int ch = 0; ch < frame->num_channels_; ch++)
+      for (int k = 0; k < static_cast<int>(frame->samples_per_channel_); k++)
+        // Store random 16 bit quantized float number between -1 and 1.

[the sun, 2015/10/08 12:38:09]

Assert on the range of max_absolute_value, plus change name to "amplitude".

[peah-webrtc, 2015/10/13 06:58:39]

Done.

+        frame->data_[k * ch] =
+            ((rand_r(seed) % (max_absolute_value + max_absolute_value + 1)) -
+             (max_absolute_value + 1));

[the sun, 2015/10/08 12:38:09]

This computation is not correct. Say that max_absolute_value is 32768:

If we after the modulo have the max, 65536, then subtracting (32768+1) yields
32767; fine.
If we after the module have the min, 0, then subtracting (32768+1) yields
-32769; not so fine, if we intend to stick it in an int16_t.

[kwiberg-webrtc, 2015/10/08 13:25:22]

I recognize this from 15 lines ago. Subroutine?

[ivoc, 2015/10/09 15:47:15]

Not exactly the same, there's no division and conversion to float here. but I
agree that this function doesn't look correct.

[peah-webrtc, 2015/10/13 06:58:39]

You are definitely correct. I now limited the range of amplitude by asserts so
that it is between 0 and 32767. Then the result should fit in an int16_t.

[peah-webrtc, 2015/10/13 06:58:39]

Done.

[peah-webrtc, 2015/10/13 06:58:39]

Done.

[peah-webrtc, 2015/10/13 06:58:39]

Done.

[peah-webrtc, 2015/10/13 06:58:40]

I think it should be correct now.

+  }
+
+  // Thread callback for the render thread
+  static bool CbRenderThread(void* context) {
+    return reinterpret_cast<AudioProcessingImpLockTest*>(context)
+        ->CbRenderImpl();
+  }
+
+  // Thread callback for the capture thread
+  static bool CbCaptureThread(void* context) {
+    return reinterpret_cast<AudioProcessingImpLockTest*>(context)
+        ->CbCaptureImpl();
+  }
+
+  // Thread callback for the stats thread
+  static bool CbStatsThread(void* context) {
+    return reinterpret_cast<AudioProcessingImpLockTest*>(context)
+        ->CbStatsImpl();
+  }
+
+  // Tests whether all the required render and capture side calls have been
+  // done.
+  bool TestDone() {
+    rtc::CritScope cs(&crit_);
+    if ((render_count_ > kMinNumCalls) && (capture_count_ > kMinNumCalls))
+      return true;
+    return false;

[kwiberg-webrtc, 2015/10/08 13:25:21]

Just

  return (render_count_ > kMinNumCalls) && (capture_count_ > kMinNumCalls);

[peah-webrtc, 2015/10/13 06:58:40]

Done.

+  }
+
+  // Sleeps a random time.

[kwiberg-webrtc, 2015/10/08 13:25:21]

Time unit?

[peah-webrtc, 2015/10/13 06:58:40]

Done.

+  static void SleepRandomTime(int max_sleep, unsigned int* seed) {
+    int sleeptime = rand_r(seed) % (max_sleep + 1);
+    SleepMs(sleeptime);
+  }
+
+  // Implements the callback functionality for the statistics
+  // collection thread.
+  bool CbStatsImpl() {
+    SleepRandomTime(100, &stats_seed);
+
+    (void)apm_->echo_cancellation()->is_enabled();
+    (void)apm_->echo_cancellation()->stream_drift_samples();
+    (void)apm_->echo_control_mobile()->is_enabled();
+    (void)apm_->gain_control()->is_enabled();
+    (void)apm_->gain_control()->stream_analog_level();
+    (void)apm_->noise_suppression()->is_enabled();
+    (void)apm_->noise_suppression()->speech_probability();
+    (void)apm_->voice_detection()->is_enabled();
+
+    return true;
+  }
+
+  // Implements the callback functionality for the render thread.
+  bool CbRenderImpl() {
+    // Conditional wait to ensure that a capture call has been done
+    // before the first render call is performed (implicitly
+    // required by the APM API).
+    if (first_render_side_call_) {
+      bool capture_side_called_local;
+      do {
+        {
+          rtc::CritScope cs(&crit_initial_sync_);
+          capture_side_called_local = capture_side_called_;
+        }
+        SleepRandomTime(3, &render_seed);
+      } while (!capture_side_called_local);
+
+      first_render_side_call_ = false;
+    }
+
+    // Sleep a random time to simulate thread jitter.
+    SleepRandomTime(3, &render_seed);
+
+    // End the test early if a fatal failure (ASSERT_*) has occurred.
+    if (HasFatalFailure())
+      test_complete_->Set();
+
+    // Ensure that the number of render and capture calls do not
+    // differ too much.
+    int frame_counter_difference;
+    do {
+      {
+        rtc::CritScope cs(&crit_);
+        frame_counter_difference =
+            render_count_ - (capture_count_ + kMaxCallDifference);
+      }
+      if (frame_counter_difference > 0)
+        SleepMs(1);
+    } while (frame_counter_difference > 0);
+
+    // End the test early if a fatal failure (ASSERT_*) has occurred.
+    if (HasFatalFailure())
+      test_complete_->Set();
+
+    // Apply any specified render side APM non-processing runtime calls.
+    ApplyRenderRuntimeSettingScheme();
+
+    // Apply the render side processing call.
+    CallRenderSide();
+
+    // End the test early if a fatal failure (ASSERT_*) has occurred.
+    if (HasFatalFailure())
+      test_complete_->Set();
+
+    // Increase the number of render-side calls.
+    rtc::CritScope cs(&crit_);
+    render_count_++;
+
+    return true;
+  }
+
+  // Makes the capture side processing API call.
+  void CallCaptureSide() {
+    // Prepare a proper capture side processing API call input.
+    PrepareCaptureFrame();
+
+    // Set the stream delay
+    (void)apm_->set_stream_delay_ms(30);

[ivoc, 2015/10/09 15:47:15]

What does this (void) thing do?

[peah-webrtc, 2015/10/13 06:58:40]

Done.

[peah-webrtc, 2015/10/13 06:58:40]

It explicitly discards the output of the function, thereby avoiding warnings for
not catching the output.

However, one of these were wrong so I added a check for that output.

+
+    // Call the specified capture side API processing method.
+    int result = AudioProcessing::kNoError;
+    switch (test_config_.capture_api_function) {
+      case CaptureApiFunction::ProcessStream1:
+        result = apm_->ProcessStream(&capture_frame_);
+        break;
+      case CaptureApiFunction::ProcessStream2:
+        result = apm_->ProcessStream(
+            capture_input_frame_, capture_input_samples_per_channel_,
+            capture_input_sample_rate_hz_, capture_input_channel_layout_,
+            capture_output_sample_rate_hz_, capture_output_channel_layout_,
+            capture_output_frame_);
+        break;
+      case CaptureApiFunction::ProcessStream3:
+        result = apm_->ProcessStream(
+            capture_input_frame_, capture_input_stream_config_,
+            capture_output_stream_config_, capture_output_frame_);
+        break;
+      default:
+        assert(false);

[ivoc, 2015/10/09 15:47:15]

Shouldn't this be something like ASSERT_TRUE(false), so we mark the test as
failed instead of crashing the executable?

[peah-webrtc, 2015/10/13 06:58:39]

Done.

[peah-webrtc, 2015/10/13 06:58:39]

Good point! Added that!

+    }
+
+    // Check the return code for error.
+    EXPECT_EQ(AudioProcessing::kNoError, result);
+  }
+
+  // Prepares the render side frame and the accompanying metadata
+  // with the appropriate information.
+  void PrepareRenderFrame() {
+    // Restrict to a common fixed sample rate if the AudioFrame interface is
+    // used.
+    if ((test_config_.render_api_function ==
+         RenderApiFunction::AnalyzeReverseStream1) ||
+        (test_config_.render_api_function ==
+         RenderApiFunction::ProcessReverseStream1) ||
+        (test_config_.aec_type != AecType::Aecm)) {
+      render_input_sample_rate_hz_ = test_config_.initial_sample_rate;
+      render_output_sample_rate_hz_ = test_config_.initial_sample_rate;
+    }
+
+    // Prepare the audioframe data and metadata
+    render_input_samples_per_channel_ =
+        render_input_sample_rate_hz_ * AudioProcessing::kChunkSizeMs / 1000;
+    render_frame_.sample_rate_hz_ = render_input_sample_rate_hz_;
+    render_frame_.num_channels_ = render_input_number_of_channels_;
+    render_frame_.samples_per_channel_ = render_input_samples_per_channel_;
+    memset(render_frame_.data_, 0,
+           render_input_samples_per_channel_ * sizeof(render_frame_.data_[0]));
+    PopulateAudioFrame(&render_frame_, kRenderInputFixLevel, &render_seed);
+
+    // Prepare the float audio input data and metadata.
+    render_input_stream_config_.set_sample_rate_hz(
+        render_input_sample_rate_hz_);
+    render_input_stream_config_.set_num_channels(
+        render_input_number_of_channels_);
+    render_input_stream_config_.set_has_keyboard(false);
+    PopulateAudioFrame(render_input_frame_, kRenderInputFloatLevel,
+                       render_input_number_of_channels_,
+                       render_input_samples_per_channel_, &render_seed);
+    render_input_channel_layout_ =
+        (render_input_number_of_channels_ == 1
+             ? AudioProcessing::ChannelLayout::kMono
+             : AudioProcessing::ChannelLayout::kStereo);
+
+    // Prepare the float audio output data and metadata.
+    render_output_samples_per_channel_ =
+        render_output_sample_rate_hz_ * AudioProcessing::kChunkSizeMs / 1000;
+    render_output_stream_config_.set_sample_rate_hz(
+        render_output_sample_rate_hz_);
+    render_output_stream_config_.set_num_channels(
+        render_output_number_of_channels_);
+    render_output_stream_config_.set_has_keyboard(false);
+    render_output_channel_layout_ =
+        (render_output_number_of_channels_ == 1
+             ? AudioProcessing::ChannelLayout::kMono
+             : AudioProcessing::ChannelLayout::kStereo);
+  }
+
+  void PrepareCaptureFrame() {
+    // Restrict to a common fixed sample rate if the AudioFrame
+    // interface is used.
+    if (test_config_.capture_api_function ==
+        CaptureApiFunction::ProcessStream1) {
+      capture_input_sample_rate_hz_ = test_config_.initial_sample_rate;
+      capture_output_sample_rate_hz_ = test_config_.initial_sample_rate;
+    }
+
+    // Prepare the audioframe data and metadata.
+    capture_input_samples_per_channel_ =
+        capture_input_sample_rate_hz_ * AudioProcessing::kChunkSizeMs / 1000;
+    capture_frame_.sample_rate_hz_ = capture_input_sample_rate_hz_;
+    capture_frame_.num_channels_ = capture_input_number_of_channels_;
+    capture_frame_.samples_per_channel_ = capture_input_samples_per_channel_;
+    memset(capture_frame_.data_, 0, capture_input_samples_per_channel_ *
+                                        sizeof(capture_frame_.data_[0]));
+    PopulateAudioFrame(&capture_frame_, kCaptureInputFixLevel, &capture_seed);
+
+    // Prepare the float audio input data and metadata.
+    capture_input_stream_config_.set_sample_rate_hz(
+        capture_input_sample_rate_hz_);
+    capture_input_stream_config_.set_num_channels(
+        capture_input_number_of_channels_);
+    capture_input_stream_config_.set_has_keyboard(false);
+    PopulateAudioFrame(capture_input_frame_, kCaptureInputFloatLevel,
+                       capture_input_number_of_channels_,
+                       capture_input_samples_per_channel_, &capture_seed);
+    capture_input_channel_layout_ =
+        (capture_input_number_of_channels_ == 1
+             ? AudioProcessing::ChannelLayout::kMonoAndKeyboard
+             : AudioProcessing::ChannelLayout::kStereoAndKeyboard);
+
+    // Prepare the float audio output data and metadata.
+    capture_output_samples_per_channel_ =
+        capture_output_sample_rate_hz_ * AudioProcessing::kChunkSizeMs / 1000;
+    capture_output_stream_config_.set_sample_rate_hz(
+        capture_output_sample_rate_hz_);
+    capture_output_stream_config_.set_num_channels(
+        capture_output_number_of_channels_);
+    capture_output_stream_config_.set_has_keyboard(false);
+    capture_output_channel_layout_ =
+        (capture_output_number_of_channels_ == 1
+             ? AudioProcessing::ChannelLayout::kMono
+             : AudioProcessing::ChannelLayout::kStereo);
+  }
+
+  // Applies any render capture APM API calls and audio stream characteristics
+  // specified by the scheme for the test.
+  void ApplyRenderRuntimeSettingScheme() {
+    int render_count_local;
+    {
+      rtc::CritScope cs(&crit_);
+      render_count_local = render_count_;
+    }

[kwiberg-webrtc, 2015/10/08 13:25:22]

If you want, you can write it like this:

  const int render_count_local = [&]{
    rtc::CritScope cs(&crit_);
    return render_count_;
  }();

This is easier to read, since you never have an uninitialized variable (and can
even make it const).

[peah-webrtc, 2015/10/13 06:58:39]

That looks awesome! Unfortunately it seems that we are not yet allowed to use
that feature so I reverted back to the old implementation. ("Default lambda
captures are an unapproved C++ feature.  [build/c++11] [4]").

[kwiberg-webrtc, 2015/10/13 09:35:12]

It looks like that rule is going to get the obvious exception for lambdas that
can't escape the scope where they're defined, but in the meantime you could
capture this explicitly if you want:

  const int render_count_local = [this]{
    rtc::CritScope cs(&crit_);
    return render_count_;
  }();

[peah-webrtc, 2015/10/14 07:57:13]

Great! That worked super!

+
+    // Update the number of channels and sample rates for the input and output.
+    switch (test_config_.runtime_parameter_setting_scheme) {
+      case RuntimeParameterSettingScheme::Scheme1:
+        if (render_count_local == 0)
+          render_input_sample_rate_hz_ = 16000;
+        else if ((render_count_local % 47) == 0)

[kwiberg-webrtc, 2015/10/08 13:25:22]

Drop the extra parentheses.

[peah-webrtc, 2015/10/13 06:58:39]

Done.

+          render_input_sample_rate_hz_ = 32000;
+        else if ((render_count_local % 71) == 0)
+          render_input_sample_rate_hz_ = 48000;
+        else if ((render_count_local % 79) == 0)
+          render_input_sample_rate_hz_ = 16000;
+        else if ((render_count_local % 83) == 0)
+          render_input_sample_rate_hz_ = 8000;

[kwiberg-webrtc, 2015/10/08 13:25:22]

Where do all these numbers come from?

[ivoc, 2015/10/09 15:47:15]

Looks very confusing indeed, needs some comments to explain what's going on.

[peah-webrtc, 2015/10/13 06:58:39]

Please let me know if the comment is sufficient!

[peah-webrtc, 2015/10/13 06:58:39]

They are prime numbers that are chosen in order to produce a varying permutation
scheme of the settings. I added a comment about that.

+
+        if (render_count_local == 0)
+          render_input_number_of_channels_ = 1;
+        else if ((render_count_local % 4) == 0)
+          render_input_number_of_channels_ =
+              (render_input_number_of_channels_ == 1 ? 2 : 1);
+
+        if (render_count_local == 0)
+          render_output_sample_rate_hz_ = 16000;
+        else if ((render_count_local % 17) == 0)
+          render_output_sample_rate_hz_ = 32000;
+        else if ((render_count_local % 19) == 0)
+          render_output_sample_rate_hz_ = 48000;
+        else if ((render_count_local % 29) == 0)
+          render_output_sample_rate_hz_ = 16000;
+        else if ((render_count_local % 61) == 0)
+          render_output_sample_rate_hz_ = 8000;
+
+        if (render_count_local == 0)
+          render_output_number_of_channels_ = 1;
+        else if ((render_count_local % 8) == 0)
+          render_output_number_of_channels_ =
+              (render_output_number_of_channels_ == 1 ? 2 : 1);
+        break;
+      case RuntimeParameterSettingScheme::Scheme2:
+        if (render_count_local == 0) {
+          render_input_number_of_channels_ = 1;
+          render_input_sample_rate_hz_ = 16000;
+          render_output_number_of_channels_ = 1;
+          render_output_sample_rate_hz_ = 16000;
+        } else {
+          render_input_number_of_channels_ =
+              (render_input_number_of_channels_ == 1 ? 2 : 1);
+          if (render_input_sample_rate_hz_ == 8000)
+            render_input_sample_rate_hz_ = 16000;
+          else if (render_input_sample_rate_hz_ == 16000)
+            render_input_sample_rate_hz_ = 32000;
+          else if (render_input_sample_rate_hz_ == 32000)
+            render_input_sample_rate_hz_ = 48000;
+          else if (render_input_sample_rate_hz_ == 48000)
+            render_input_sample_rate_hz_ = 8000;
+
+          render_output_number_of_channels_ =
+              (render_output_number_of_channels_ == 1 ? 2 : 1);
+          if (render_output_sample_rate_hz_ == 8000)
+            render_output_sample_rate_hz_ = 16000;
+          else if (render_output_sample_rate_hz_ == 16000)
+            render_output_sample_rate_hz_ = 32000;
+          else if (render_output_sample_rate_hz_ == 32000)
+            render_output_sample_rate_hz_ = 48000;
+          else if (render_output_sample_rate_hz_ == 48000)
+            render_output_sample_rate_hz_ = 8000;
+        }
+        break;
+      case RuntimeParameterSettingScheme::Scheme3:
+        if (render_count_local == 0) {
+          render_input_sample_rate_hz_ = 16000;
+          render_input_number_of_channels_ = 1;
+          render_output_sample_rate_hz_ = 16000;
+          render_output_number_of_channels_ = 1;
+        }
+        break;
+      case RuntimeParameterSettingScheme::Scheme4:
+        if (render_count_local == 0) {
+          render_input_sample_rate_hz_ = 16000;
+          render_input_number_of_channels_ = 2;
+          render_output_sample_rate_hz_ = 16000;
+          render_output_number_of_channels_ = 2;
+        }
+
+        break;
+      default:
+        assert(false);
+    }
+
+    // Restric the number of output channels not to exceed
+    // the number of input channels.
+    render_output_number_of_channels_ = std::min(
+        render_output_number_of_channels_, render_input_number_of_channels_);
+  }
+
+  // Applies any runtime capture APM API calls and audio stream characteristics
+  // specified by the scheme for the test.
+  void ApplyCaptureRuntimeSettingScheme() {
+    int capture_count_local;
+    {
+      rtc::CritScope cs(&crit_);
+      capture_count_local = capture_count_;
+    }
+
+    // Update the number of channels and sample rates for the input and output.
+    switch (test_config_.runtime_parameter_setting_scheme) {
+      case RuntimeParameterSettingScheme::Scheme1:
+        if (capture_count_local == 0)
+          capture_input_sample_rate_hz_ = 16000;
+        else if ((capture_count_local % 11) == 0)
+          capture_input_sample_rate_hz_ = 32000;
+        else if ((capture_count_local % 73) == 0)
+          capture_input_sample_rate_hz_ = 48000;
+        else if ((capture_count_local % 89) == 0)
+          capture_input_sample_rate_hz_ = 16000;
+        else if ((capture_count_local % 97) == 0)
+          capture_input_sample_rate_hz_ = 8000;
+
+        if (capture_count_local == 0)
+          capture_input_number_of_channels_ = 1;
+        else if ((capture_count_local % 4) == 0)
+          capture_input_number_of_channels_ =
+              (capture_input_number_of_channels_ == 1 ? 2 : 1);
+
+        if (capture_count_local == 0)
+          capture_output_sample_rate_hz_ = 16000;
+        else if ((capture_count_local % 5) == 0)
+          capture_output_sample_rate_hz_ = 32000;
+        else if ((capture_count_local % 47) == 0)
+          capture_output_sample_rate_hz_ = 48000;
+        else if ((capture_count_local % 53) == 0)
+          capture_output_sample_rate_hz_ = 16000;
+        else if ((capture_count_local % 71) == 0)
+          capture_output_sample_rate_hz_ = 8000;
+
+        if (capture_count_local == 0)
+          capture_output_number_of_channels_ = 1;
+        else if ((capture_count_local % 8) == 0)
+          capture_output_number_of_channels_ =
+              (capture_output_number_of_channels_ == 1 ? 2 : 1);
+        break;
+      case RuntimeParameterSettingScheme::Scheme2:
+        if ((capture_count_local % 2) == 0) {
+          capture_input_number_of_channels_ = 1;
+          capture_input_sample_rate_hz_ = 16000;
+          capture_output_number_of_channels_ = 1;
+          capture_output_sample_rate_hz_ = 16000;
+        } else {
+          capture_input_number_of_channels_ =
+              (capture_input_number_of_channels_ == 1 ? 2 : 1);
+          if (capture_input_sample_rate_hz_ == 8000)
+            capture_input_sample_rate_hz_ = 16000;
+          else if (capture_input_sample_rate_hz_ == 16000)
+            capture_input_sample_rate_hz_ = 32000;
+          else if (capture_input_sample_rate_hz_ == 32000)
+            capture_input_sample_rate_hz_ = 48000;
+          else if (capture_input_sample_rate_hz_ == 48000)
+            capture_input_sample_rate_hz_ = 8000;
+
+          capture_output_number_of_channels_ =
+              (capture_output_number_of_channels_ == 1 ? 2 : 1);
+          if (capture_output_sample_rate_hz_ == 8000)
+            capture_output_sample_rate_hz_ = 16000;
+          else if (capture_output_sample_rate_hz_ == 16000)
+            capture_output_sample_rate_hz_ = 32000;
+          else if (capture_output_sample_rate_hz_ == 32000)
+            capture_output_sample_rate_hz_ = 48000;
+          else if (capture_output_sample_rate_hz_ == 48000)
+            capture_output_sample_rate_hz_ = 8000;
+        }
+        break;
+      case RuntimeParameterSettingScheme::Scheme3:
+        if (capture_count_local == 0) {
+          capture_input_sample_rate_hz_ = 16000;
+          capture_input_number_of_channels_ = 1;
+          capture_output_sample_rate_hz_ = 16000;
+          capture_output_number_of_channels_ = 1;
+        }
+        break;
+      case RuntimeParameterSettingScheme::Scheme4:
+        if (capture_count_local == 0) {
+          capture_input_sample_rate_hz_ = 16000;
+          capture_input_number_of_channels_ = 2;
+          capture_output_sample_rate_hz_ = 16000;
+          capture_output_number_of_channels_ = 2;
+        }
+
+        break;
+      default:
+        assert(false);
+    }
+
+    // Call any specified runtime APM setter and
+    // getter calls.
+    switch (test_config_.runtime_parameter_setting_scheme) {
+      case RuntimeParameterSettingScheme::Scheme1:
+      case RuntimeParameterSettingScheme::Scheme3:
+        break;
+      case RuntimeParameterSettingScheme::Scheme2:
+      case RuntimeParameterSettingScheme::Scheme4:
+        if ((capture_count_local % 2) == 0) {
+          (void)apm_->set_stream_delay_ms(30);
+          apm_->set_stream_key_pressed(true);
+          apm_->set_output_will_be_muted(true);
+          apm_->set_delay_offset_ms(15);
+          (void)apm_->delay_offset_ms();
+          apm_->set_output_will_be_muted(true);
+          (void)apm_->num_reverse_channels();
+        } else {
+          (void)apm_->set_stream_delay_ms(50);
+          apm_->set_stream_key_pressed(false);
+          apm_->set_output_will_be_muted(false);
+          apm_->set_delay_offset_ms(20);
+          (void)apm_->delay_offset_ms();
+          apm_->set_output_will_be_muted(false);
+          (void)apm_->num_reverse_channels();
+        }
+        break;
+      default:
+        assert(false);
+    }
+
+    // Restric the number of output channels not to exceed
+    // the number of input channels.
+    capture_output_number_of_channels_ = std::min(
+        capture_output_number_of_channels_, capture_input_number_of_channels_);
+  }
+
+  // Makes the render side processing API call.
+  void CallRenderSide() {
+    // Prepare a proper render side processing API call input.
+    PrepareRenderFrame();
+
+    // Call the specified render side API processing method.
+    int result = AudioProcessing::kNoError;
+    switch (test_config_.render_api_function) {
+      case RenderApiFunction::ProcessReverseStream1:
+        result = apm_->ProcessReverseStream(&render_frame_);
+        break;
+      case RenderApiFunction::ProcessReverseStream2:
+        result = apm_->ProcessReverseStream(
+            render_input_frame_, render_input_stream_config_,
+            render_output_stream_config_, render_output_frame_);
+        break;
+      case RenderApiFunction::AnalyzeReverseStream1:
+        result = apm_->AnalyzeReverseStream(&render_frame_);
+        break;
+      case RenderApiFunction::AnalyzeReverseStream2:
+        result = apm_->AnalyzeReverseStream(
+            render_input_frame_, render_input_samples_per_channel_,
+            render_input_sample_rate_hz_, render_input_channel_layout_);
+        break;
+      default:
+        assert(false);
+    }
+
+    // Check the return code for error.
+    EXPECT_EQ(AudioProcessing::kNoError, result);
+  }
+
+  // Implements the callback functionality for the capture thread.
+  bool CbCaptureImpl() {
+    // Sleep a random time to simulate thread jitter.
+    SleepRandomTime(3, &capture_seed);
+
+    // End the test early if a fatal failure (ASSERT_*) has occurred.
+    if (HasFatalFailure())
+      test_complete_->Set();
+
+    // Ensure that there are not more capture side calls than render side
+    // calls.
+    int frame_counter_difference;
+    do {
+      {
+        rtc::CritScope cs(&crit_);
+        frame_counter_difference = capture_count_ - render_count_;
+      }
+      if (frame_counter_difference > 0)
+        SleepMs(1);
+    } while (frame_counter_difference > 0);
+
+    // End the test early if a fatal failure (ASSERT_*) has occurred.
+    if (HasFatalFailure())
+      test_complete_->Set();
+
+    // Apply any specified capture side APM non-processing runtime calls.
+    ApplyCaptureRuntimeSettingScheme();
+
+    // Apply the capture side processing call.
+    CallCaptureSide();
+
+    // End the test early if a fatal failure (ASSERT_*) has occurred.
+    if (HasFatalFailure())
+      test_complete_->Set();
+
+    // Increase the number of capture-side calls.
+    {
+      rtc::CritScope cs(&crit_);
+      capture_count_++;
+    }
+
+    // Check if the test is done.
+    if (TestDone())
+      test_complete_->Set();
+
+    // 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).
+    {
+      rtc::CritScope cs(&crit_initial_sync_);
+      capture_side_called_ = true;
+    }
+
+    return true;
+  }
+
+  // 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);
+    ASSERT_TRUE(stats_thread_->Start());
+    stats_thread_->SetPriority(kNormalPriority);
+  }
+
+  rtc::CriticalSection crit_;
+  rtc::CriticalSection crit_initial_sync_;
+  rtc::scoped_ptr<ThreadWrapper> render_thread_;
+  rtc::scoped_ptr<ThreadWrapper> capture_thread_;
+  rtc::scoped_ptr<ThreadWrapper> stats_thread_;
+  int render_count_ GUARDED_BY(crit_);
+  int capture_count_ GUARDED_BY(crit_);
+  bool first_render_side_call_;
+  bool capture_side_called_ GUARDED_BY(crit_initial_sync_);
+  const rtc::scoped_ptr<EventWrapper> test_complete_;
+  rtc::scoped_ptr<AudioProcessing> apm_;
+  TestConfig test_config_;
+  AudioFrame render_frame_;
+  AudioFrame capture_frame_;
+  unsigned int render_seed;
+  unsigned int capture_seed;
+  unsigned int stats_seed;
+
+  // Variables related to the capture side audio data and formats.
+  float** capture_output_frame_;
+  AudioProcessing::ChannelLayout capture_output_channel_layout_;
+  int capture_input_sample_rate_hz_;
+  int capture_input_number_of_channels_;
+  float** capture_input_frame_;
+  AudioProcessing::ChannelLayout capture_input_channel_layout_;
+  int capture_output_sample_rate_hz_;
+  int capture_output_number_of_channels_;
+  StreamConfig capture_input_stream_config_;
+  StreamConfig capture_output_stream_config_;
+  int capture_input_samples_per_channel_;
+  int capture_output_samples_per_channel_;
+
+  // Variables related to the render side audio data and formats.
+  float** render_output_frame_;
+  AudioProcessing::ChannelLayout render_output_channel_layout_;
+  int render_input_sample_rate_hz_;
+  int render_input_number_of_channels_;
+  float** render_input_frame_;
+  AudioProcessing::ChannelLayout render_input_channel_layout_;
+  int render_output_sample_rate_hz_;
+  int render_output_number_of_channels_;
+  StreamConfig render_input_stream_config_;
+  StreamConfig render_output_stream_config_;
+  int render_input_samples_per_channel_;
+  int render_output_samples_per_channel_;

[kwiberg-webrtc, 2015/10/08 13:25:22]

This is a large pile of member variables. Any chance they'll neatly split into,
say, one private object per thread (with no locking) plus one shared object?

[peah-webrtc, 2015/10/13 06:58:40]

Done.

+};
+
+}  // anonymous namespace
+
+TEST_P(AudioProcessingImpLockTest, LockTest) {
+  // Run test and verify that it did not time out.
+  EXPECT_EQ(kEventSignaled, RunTest());
+}
+
+// Instantiate tests from the test configurations provided by the generator.
+INSTANTIATE_TEST_CASE_P(
+    DISABLED_AudioProcessingImpLockTestAllCombinations,
+    AudioProcessingImpLockTest,
+    ::testing::ValuesIn(AudioProcessingImpLockTest::GenerateTestConfigs()));
+
+}  // namespace webrtc