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 232 matching lines @@
   void IncreaseRenderCounter() {
     rtc::CritScope cs(&crit_);
     render_count++;
   }
 
   void IncreaseCaptureCounter() {
     rtc::CritScope cs(&crit_);
     capture_count++;
   }
 
-  int GetCaptureCounter() {
+  int GetCaptureCounter() const {
     rtc::CritScope cs(&crit_);
     return capture_count;
   }
 
-  int GetRenderCounter() {
+  int GetRenderCounter() const {
     rtc::CritScope cs(&crit_);
     return render_count;
   }
 
-  int CaptureMinusRenderCounters() {
+  int CaptureMinusRenderCounters() const {
     rtc::CritScope cs(&crit_);
     return capture_count - render_count;
   }
 
+  int RenderMinusCaptureCounters() const {
+    return -CaptureMinusRenderCounters();
+  }
+
+
   bool BothCountersExceedeThreshold(int threshold) {
     rtc::CritScope cs(&crit_);
     return (render_count > threshold && capture_count > threshold);
   }
 
  private:
-  rtc::CriticalSection crit_;
+  mutable 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,
+                   rtc::Event* render_call_event,
+                   rtc::Event* capture_call_event,
                    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;
+  rtc::Event* render_call_event_;

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

* const

[peah-webrtc, 2015/12/02 22:44:41]

Added some more const specifiers as well.

Done.

+  rtc::Event* capture_call_event_;

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

* const

[peah-webrtc, 2015/12/02 22:44:41]

Added some more const specifiers as well.
Done.

   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,
+                  rtc::Event* render_call_event,
+                  rtc::Event* capture_call_event,
                   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;
+  rtc::Event* render_call_event_;

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

* const

[peah-webrtc, 2015/12/02 22:44:41]

Added some more const specifiers as well.
Done.

+  rtc::Event* capture_call_event_;

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

* const

[peah-webrtc, 2015/12/02 22:44:41]

Added some more const specifiers as well.
Done.

   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();
 
  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_(false, false),
+      render_call_event_(false, false),
+      capture_call_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_,
+                           &render_call_event_,
+                           &capture_call_event_,
                            &frame_counters_,
-                           &capture_call_checker_,
                            this,
                            &test_config_,
                            apm_.get()),
       capture_thread_state_(kMaxFrameSize,
                             &rand_gen_,
+                            &render_call_event_,
+                            &capture_call_event_,
                             &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 20 matching lines @@
   apm_->voice_detection()->is_enabled();
 
   return true;
 }
 
 const float CaptureProcessor::kCaptureInputFloatLevel = 0.03125f;
 
 CaptureProcessor::CaptureProcessor(
     int max_frame_size,
     RandomGenerator* rand_gen,
+    rtc::Event* render_call_event,
+    rtc::Event* capture_call_event,
     FrameCounters* shared_counters_state,
-    CaptureSideCalledChecker* capture_call_checker,
     AudioProcessingImplLockTest* test_framework,
     TestConfig* test_config,
     AudioProcessing* apm)
     : rand_gen_(rand_gen),
+      render_call_event_(render_call_event),
+      capture_call_event_(capture_call_event),
       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) {
+    render_call_event_->Wait(1);

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

This should wait indefinitely right? Any shutdown will set these events so we
should be able to shut down?

See implementation suggestion below, WDYT?

[peah-webrtc, 2015/12/02 22:44:41]

Awesome! Great! Made the change.

Done.

   }
 
   // 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();
+  // Flag to the render thread that another capture api call has occurred
+  // by triggering this threads call event.
+  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 198 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);
 }
 
 const float RenderProcessor::kRenderInputFloatLevel = 0.5f;
 
 RenderProcessor::RenderProcessor(int max_frame_size,
                                  RandomGenerator* rand_gen,
+                                 rtc::Event* render_call_event,
+                                 rtc::Event* capture_call_event,
                                  FrameCounters* shared_counters_state,
-                                 CaptureSideCalledChecker* capture_call_checker,
                                  AudioProcessingImplLockTest* test_framework,
                                  TestConfig* test_config,
                                  AudioProcessing* apm)
     : rand_gen_(rand_gen),
+      render_call_event_(render_call_event),
+      capture_call_event_(capture_call_event),
       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_) {
+    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() &&

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

shouldn't this be:

while (true) {
  if (test_->EndTest())
    return false;
  if (frame_counters_->RenderMinusCaptureCounters() > kMaxCallDifference)
    capture_call_event_->Wait(rtc::Event::kForever);
}

And corresponding above. You don't want to run the render side after the test
ends right?

[peah-webrtc, 2015/12/02 22:44:41]

I think you are right. That is the correct way to do it. 

Done.

+         frame_counters_->RenderMinusCaptureCounters() > kMaxCallDifference) {
+    capture_call_event_->Wait(1);

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

Same here, wait indefinitely?

[peah-webrtc, 2015/12/02 22:44:41]

Done.

   }
 
   // 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();
 
+  // Flag to the capture thread that another render api call as occurred

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

has occurred, render API

[peah-webrtc, 2015/12/02 22:44:41]

Done.

+  // by triggering this threads call event.
+  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_TRUE(RunTest());
 }
 
 // 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