Replaced the custom thread synchronization with rtc::Event

webrtc/modules/audio_processing/audio_processing_impl_locking_unittest.cc

 /*
  *  Copyright (c) 2015 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 "webrtc/modules/audio_processing/audio_processing_impl.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/base/event.h"
 #include "webrtc/base/platform_thread.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/event_wrapper.h"
 #include "webrtc/system_wrappers/include/sleep.h"
 #include "webrtc/test/random.h"
 
 namespace webrtc {
 
 namespace {
 
 class AudioProcessingImplLockTest;
 
 // Type of the render thread APM API call to use in the test.
@@ skipping 258 matching lines @@
     rtc::CritScope cs(&crit_);
     return (render_count > threshold && capture_count > threshold);
   }
 
  private:
   rtc::CriticalSection crit_;
   int render_count GUARDED_BY(crit_) = 0;
   int capture_count GUARDED_BY(crit_) = 0;
 };
 
-// Checker for whether the capture side has been called.
-class CaptureSideCalledChecker {
- public:
-  bool CaptureSideCalled() {
-    rtc::CritScope cs(&crit_);
-    return capture_side_called_;
-  }
-
-  void FlagCaptureSideCalled() {
-    rtc::CritScope cs(&crit_);
-    capture_side_called_ = true;
-  }
-
- private:
-  rtc::CriticalSection crit_;
-  bool capture_side_called_ GUARDED_BY(crit_) = false;
-};
-
 // Class for handling the capture side processing.
 class CaptureProcessor {
  public:
   CaptureProcessor(int max_frame_size,
                    RandomGenerator* rand_gen,
                    FrameCounters* shared_counters_state,
-                   CaptureSideCalledChecker* capture_call_checker,
                    AudioProcessingImplLockTest* test_framework,
                    TestConfig* test_config,
                    AudioProcessing* apm);
   bool Process();
 
  private:
   static const int kMaxCallDifference = 10;
   static const float kCaptureInputFloatLevel;
   static const int kCaptureInputFixLevel = 1024;
 
   void PrepareFrame();
   void CallApmCaptureSide();
   void ApplyRuntimeSettingScheme();
 
   RandomGenerator* rand_gen_ = nullptr;
   FrameCounters* frame_counters_ = nullptr;
-  CaptureSideCalledChecker* capture_call_checker_ = nullptr;
   AudioProcessingImplLockTest* test_ = nullptr;
   TestConfig* test_config_ = nullptr;
   AudioProcessing* apm_ = nullptr;
   AudioFrameData frame_data_;
 };
 
 // Class for handling the stats processing.
 class StatsProcessor {
  public:
   StatsProcessor(RandomGenerator* rand_gen,
                  TestConfig* test_config,
                  AudioProcessing* apm);
   bool Process();
 
  private:
   RandomGenerator* rand_gen_ = nullptr;
   TestConfig* test_config_ = nullptr;
   AudioProcessing* apm_ = nullptr;
 };
 
 // Class for handling the render side processing.
 class RenderProcessor {
  public:
   RenderProcessor(int max_frame_size,
                   RandomGenerator* rand_gen,
                   FrameCounters* shared_counters_state,
-                  CaptureSideCalledChecker* capture_call_checker,
                   AudioProcessingImplLockTest* test_framework,
                   TestConfig* test_config,
                   AudioProcessing* apm);
   bool Process();
 
  private:
   static const int kMaxCallDifference = 10;
   static const int kRenderInputFixLevel = 16384;
   static const float kRenderInputFloatLevel;
 
   void PrepareFrame();
   void CallApmRenderSide();
   void ApplyRuntimeSettingScheme();
 
   RandomGenerator* rand_gen_ = nullptr;
   FrameCounters* frame_counters_ = nullptr;
-  CaptureSideCalledChecker* capture_call_checker_ = nullptr;
   AudioProcessingImplLockTest* test_ = nullptr;
   TestConfig* test_config_ = nullptr;
   AudioProcessing* apm_ = nullptr;
-  bool first_render_side_call_ = true;
   AudioFrameData frame_data_;
+  bool first_render_call_ = true;
 };
 
 class AudioProcessingImplLockTest
     : public ::testing::TestWithParam<TestConfig> {
  public:
   AudioProcessingImplLockTest();
-  EventTypeWrapper RunTest();
-  void CheckTestCompleteness();
+  bool RunTest();
+  bool MaybeEndTest();
+
+  rtc::Event& get_render_call_event() { return render_call_event_; }

[pbos-webrtc, 2015/12/02 13:01:12]

no non-const references allowed (googlestyle), use ptr

[pbos-webrtc, 2015/12/02 13:01:12]

remove get_ prefix

[peah-webrtc, 2015/12/02 15:02:27]

Thanks! I revised this according to your feedback and now pass the pointers in
the constructor instead of calling the accessor method.

Acknowledged.

[peah-webrtc, 2015/12/02 15:02:27]

Acknowledged.

+  rtc::Event& get_capture_call_event() { return capture_call_event_; }
 
  private:
   static const int kTestTimeOutLimit = 10 * 60 * 1000;
   static const int kMaxFrameSize = 480;
 
   // ::testing::TestWithParam<> implementation
   void SetUp() override;
   void TearDown() override;
 
   // Thread callback for the render thread
@@ skipping 24 matching lines @@
   // Start the threads used in the test.
   void StartThreads() {
     render_thread_.Start();
     render_thread_.SetPriority(rtc::kRealtimePriority);
     capture_thread_.Start();
     capture_thread_.SetPriority(rtc::kRealtimePriority);
     stats_thread_.Start();
     stats_thread_.SetPriority(rtc::kNormalPriority);
   }
 
-  // Event handler for the test.
-  const rtc::scoped_ptr<EventWrapper> test_complete_;
+  // Event handlers for the test.
+  rtc::Event test_complete_;
+  rtc::Event render_call_event_;
+  rtc::Event capture_call_event_;
 
   // Thread related variables.
   rtc::PlatformThread render_thread_;
   rtc::PlatformThread capture_thread_;
   rtc::PlatformThread stats_thread_;
   mutable RandomGenerator rand_gen_;
 
   rtc::scoped_ptr<AudioProcessing> apm_;
   TestConfig test_config_;
   FrameCounters frame_counters_;
-  CaptureSideCalledChecker capture_call_checker_;
   RenderProcessor render_thread_state_;
   CaptureProcessor capture_thread_state_;
   StatsProcessor stats_thread_state_;
 };
 
 // Sleeps a random time between 0 and max_sleep milliseconds.
 void SleepRandomMs(int max_sleep, RandomGenerator* rand_gen) {
   int sleeptime = rand_gen->RandInt(0, max_sleep);
   SleepMs(sleeptime);
 }
@@ skipping 22 matching lines @@
     for (int k = 0; k < static_cast<int>(frame->samples_per_channel_); k++) {
       // Store random 16 bit number between -(amplitude+1) and
       // amplitude.
       frame->data_[k * ch] =
           rand_gen->RandInt(2 * amplitude + 1) - amplitude - 1;
     }
   }
 }
 
 AudioProcessingImplLockTest::AudioProcessingImplLockTest()
-    : test_complete_(EventWrapper::Create()),
+    : test_complete_(rtc::Event(false, false)),

[pbos-webrtc, 2015/12/02 13:01:12]

remove rtc::Event here and below (no copy-constructor)

[peah-webrtc, 2015/12/02 15:02:27]

Done.

+      render_call_event_(rtc::Event(false, false)),
+      capture_call_event_(rtc::Event(false, false)),
       render_thread_(RenderProcessorThreadFunc, this, "render"),
       capture_thread_(CaptureProcessorThreadFunc, this, "capture"),
       stats_thread_(StatsProcessorThreadFunc, this, "stats"),
       apm_(AudioProcessingImpl::Create()),
       render_thread_state_(kMaxFrameSize,
                            &rand_gen_,
                            &frame_counters_,
-                           &capture_call_checker_,
                            this,
                            &test_config_,
                            apm_.get()),
       capture_thread_state_(kMaxFrameSize,
                             &rand_gen_,
                             &frame_counters_,
-                            &capture_call_checker_,
                             this,
                             &test_config_,
                             apm_.get()),
       stats_thread_state_(&rand_gen_, &test_config_, apm_.get()) {}
 
 // Run the test with a timeout.
-EventTypeWrapper AudioProcessingImplLockTest::RunTest() {
+bool AudioProcessingImplLockTest::RunTest() {
   StartThreads();
-  return test_complete_->Wait(kTestTimeOutLimit);
+  return test_complete_.Wait(kTestTimeOutLimit);
 }
 
-void AudioProcessingImplLockTest::CheckTestCompleteness() {
+bool AudioProcessingImplLockTest::MaybeEndTest() {
   if (HasFatalFailure() || TestDone()) {
-    test_complete_->Set();
+    test_complete_.Set();
+    return true;
   }
+  return false;
 }
 
 // Setup of test and APM.
 void AudioProcessingImplLockTest::SetUp() {
   test_config_ = static_cast<TestConfig>(GetParam());
 
   ASSERT_EQ(apm_->kNoError, apm_->level_estimator()->Enable(true));
   ASSERT_EQ(apm_->kNoError, apm_->gain_control()->Enable(true));
 
   ASSERT_EQ(apm_->kNoError,
@@ skipping 24 matching lines @@
 
     config.Set<DelayAgnostic>(
         new DelayAgnostic(test_config_.aec_type ==
                           AecType::BasicWebRtcAecSettingsWithDelayAgnosticAec));
 
     apm_->SetExtraOptions(config);
   }
 }
 
 void AudioProcessingImplLockTest::TearDown() {
+  render_call_event_.Set();
+  capture_call_event_.Set();
   render_thread_.Stop();
   capture_thread_.Stop();
   stats_thread_.Stop();
 }
 
 StatsProcessor::StatsProcessor(RandomGenerator* rand_gen,
                                TestConfig* test_config,
                                AudioProcessing* apm)
     : rand_gen_(rand_gen), test_config_(test_config), apm_(apm) {}
 
@@ skipping 21 matching lines @@
 
   return true;
 }
 
 const float CaptureProcessor::kCaptureInputFloatLevel = 0.03125f;
 
 CaptureProcessor::CaptureProcessor(
     int max_frame_size,
     RandomGenerator* rand_gen,
     FrameCounters* shared_counters_state,
-    CaptureSideCalledChecker* capture_call_checker,
     AudioProcessingImplLockTest* test_framework,
     TestConfig* test_config,

[pbos-webrtc, 2015/12/02 13:01:12]

supply events on construction instead of having the public getter in test_, so
have a rtc::Event* const render_call_event_;

[peah-webrtc, 2015/12/02 15:02:27]

Done.

     AudioProcessing* apm)
     : rand_gen_(rand_gen),
       frame_counters_(shared_counters_state),
-      capture_call_checker_(capture_call_checker),
       test_(test_framework),
       test_config_(test_config),
       apm_(apm),
       frame_data_(max_frame_size) {}
 
 // Implements the callback functionality for the capture thread.
 bool CaptureProcessor::Process() {
   // Sleep a random time to simulate thread jitter.
   SleepRandomMs(3, rand_gen_);
 
-  // End the test if complete.
-  test_->CheckTestCompleteness();
-
   // Ensure that there are not more capture side calls than render side
   // calls.
-  if (capture_call_checker_->CaptureSideCalled()) {
-    while (kMaxCallDifference < frame_counters_->CaptureMinusRenderCounters()) {
-      SleepMs(1);
-    }
+  while (!test_->MaybeEndTest() &&
+         frame_counters_->CaptureMinusRenderCounters() > kMaxCallDifference) {
+    test_->get_render_call_event().Wait(1);
   }
 
   // Apply any specified capture side APM non-processing runtime calls.
   ApplyRuntimeSettingScheme();
 
   // Apply the capture side processing call.
   CallApmCaptureSide();
 
   // Increase the number of capture-side calls.
   frame_counters_->IncreaseCaptureCounter();
 
-  // 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();
+  // Set api call flag.
+  test_->get_capture_call_event().Set();
 
   return true;
 }
 
 // Prepares a frame with relevant audio data and metadata.
 void CaptureProcessor::PrepareFrame() {
   // Restrict to a common fixed sample rate if the AudioFrame
   // interface is used.
   if (test_config_->capture_api_function ==
       CaptureApiImpl::ProcessStreamImpl1) {
@@ skipping 199 matching lines @@
   frame_data_.output_number_of_channels =
       std::min(frame_data_.output_number_of_channels,
                frame_data_.input_number_of_channels);
 }
 
 const float RenderProcessor::kRenderInputFloatLevel = 0.5f;
 
 RenderProcessor::RenderProcessor(int max_frame_size,
                                  RandomGenerator* rand_gen,
                                  FrameCounters* shared_counters_state,
-                                 CaptureSideCalledChecker* capture_call_checker,
                                  AudioProcessingImplLockTest* test_framework,
                                  TestConfig* test_config,
                                  AudioProcessing* apm)
     : rand_gen_(rand_gen),
       frame_counters_(shared_counters_state),
-      capture_call_checker_(capture_call_checker),
       test_(test_framework),
       test_config_(test_config),
       apm_(apm),
       frame_data_(max_frame_size) {}
 
 // Implements the callback functionality for the render thread.
 bool RenderProcessor::Process() {
   // 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_) {
-    while (!capture_call_checker_->CaptureSideCalled()) {
-      SleepRandomMs(3, rand_gen_);
-    }
-
-    first_render_side_call_ = false;
+  if (first_render_call_) {
+    test_->get_capture_call_event().Wait(rtc::Event::kForever);
+    first_render_call_ = false;
   }
 
   // Sleep a random time to simulate thread jitter.
   SleepRandomMs(3, rand_gen_);
 
-  // End the test early if a fatal failure (ASSERT_*) has occurred.
-  test_->CheckTestCompleteness();
-
   // Ensure that the number of render and capture calls do not
   // differ too much.
-  while (kMaxCallDifference < -frame_counters_->CaptureMinusRenderCounters()) {
-    SleepMs(1);
+  while (!test_->MaybeEndTest() &&
+         -frame_counters_->CaptureMinusRenderCounters() > kMaxCallDifference) {

[pbos-webrtc, 2015/12/02 13:01:12]

document the unary minus, or a function that's renderminuscapture

[peah-webrtc, 2015/12/02 15:02:27]

Good point!
I instead created a function for that.

Done.

+    test_->get_capture_call_event().Wait(1);
   }
 
   // Apply any specified render side APM non-processing runtime calls.
   ApplyRuntimeSettingScheme();
 
   // Apply the render side processing call.
   CallApmRenderSide();
 
   // Increase the number of render-side calls.
   frame_counters_->IncreaseRenderCounter();
 
+  // Set api call flag.

[pbos-webrtc, 2015/12/02 13:01:12]

put in comment what this should do/trigger

[peah-webrtc, 2015/12/02 15:02:27]

Done.

+  test_->get_render_call_event().Set();
   return true;
 }
 
 // Prepares the render side frame and the accompanying metadata
 // with the appropriate information.
 void RenderProcessor::PrepareFrame() {
   // Restrict to a common fixed sample rate if the AudioFrame interface is
   // used.
   if ((test_config_->render_api_function ==
        RenderApiImpl::AnalyzeReverseStreamImpl1) ||
@@ skipping 174 matching lines @@
   // the number of input channels.
   frame_data_.output_number_of_channels =
       std::min(frame_data_.output_number_of_channels,
                frame_data_.input_number_of_channels);
 }
 
 }  // anonymous namespace
 
 TEST_P(AudioProcessingImplLockTest, LockTest) {
   // Run test and verify that it did not time out.
-  ASSERT_EQ(kEventSignaled, RunTest());
+  ASSERT_EQ(true, RunTest());

[pbos-webrtc, 2015/12/02 13:01:12]

ASSERT_TRUE(RunTest())

[peah-webrtc, 2015/12/02 15:02:27]

Done.

 }
 
 // Instantiate tests from the extreme test configuration set.
 INSTANTIATE_TEST_CASE_P(
     DISABLED_AudioProcessingImplLockExtensive,
     AudioProcessingImplLockTest,
     ::testing::ValuesIn(TestConfig::GenerateExtensiveTestConfigs()));
 
 INSTANTIATE_TEST_CASE_P(
     AudioProcessingImplLockBrief,
     AudioProcessingImplLockTest,
     ::testing::ValuesIn(TestConfig::GenerateBriefTestConfigs()));
 
 }  // namespace webrtc