From 4731f0062e070f7541e0a1a8f39e9d69144c7404 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Fri, 3 May 2019 09:34:24 +0200 Subject: [PATCH] Delete deprecated PlatformThread looping Bug: webrtc:10594, webrtc:7187 Change-Id: Icba3a5cf6dbe817ead427c27645b3ad7bc8819be Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/134642 Commit-Queue: Niels Moller Reviewed-by: Karl Wiberg Reviewed-by: Tommi Cr-Commit-Position: refs/heads/master@{#27833} --- .../acm2/audio_coding_module_unittest.cc | 96 ++++++++++++------- .../audio_device/dummy/file_audio_device.cc | 20 ++-- .../audio_device/dummy/file_audio_device.h | 4 +- .../linux/audio_device_alsa_linux.cc | 20 ++-- .../linux/audio_device_alsa_linux.h | 4 +- .../linux/audio_device_pulse_linux.cc | 38 +++++--- .../linux/audio_device_pulse_linux.h | 7 +- modules/audio_device/mac/audio_device_mac.cc | 22 +++-- modules/audio_device/mac/audio_device_mac.h | 4 +- .../audio_processing_impl_locking_unittest.cc | 82 +++++++--------- .../audio_processing_performance_unittest.cc | 25 ++--- modules/utility/source/process_thread_impl.cc | 6 +- modules/utility/source/process_thread_impl.h | 2 +- .../linux/video_capture_linux.cc | 74 ++++++++------ .../video_capture/linux/video_capture_linux.h | 4 +- rtc_base/cpu_time_unittest.cc | 3 +- rtc_base/critical_section_unittest.cc | 3 +- rtc_base/platform_thread.cc | 91 +----------------- rtc_base/platform_thread.h | 15 +-- rtc_base/platform_thread_unittest.cc | 66 ------------- video/video_analyzer.cc | 15 ++- video/video_analyzer.h | 3 +- 22 files changed, 251 insertions(+), 353 deletions(-) diff --git a/modules/audio_coding/acm2/audio_coding_module_unittest.cc b/modules/audio_coding/acm2/audio_coding_module_unittest.cc index 68ed6b6e7d3..c95091f1c37 100644 --- a/modules/audio_coding/acm2/audio_coding_module_unittest.cc +++ b/modules/audio_coding/acm2/audio_coding_module_unittest.cc @@ -10,6 +10,7 @@ #include #include +#include #include #include @@ -484,9 +485,15 @@ class AudioCodingModuleMtTestOldApi : public AudioCodingModuleTestOldApi { AudioCodingModuleMtTestOldApi() : AudioCodingModuleTestOldApi(), - send_thread_(CbSendThread, this, "send"), - insert_packet_thread_(CbInsertPacketThread, this, "insert_packet"), - pull_audio_thread_(CbPullAudioThread, this, "pull_audio"), + send_thread_(CbSendThread, this, "send", rtc::kRealtimePriority), + insert_packet_thread_(CbInsertPacketThread, + this, + "insert_packet", + rtc::kRealtimePriority), + pull_audio_thread_(CbPullAudioThread, + this, + "pull_audio", + rtc::kRealtimePriority), send_count_(0), insert_packet_count_(0), pull_audio_count_(0), @@ -502,16 +509,15 @@ class AudioCodingModuleMtTestOldApi : public AudioCodingModuleTestOldApi { } void StartThreads() { + quit_.store(false); send_thread_.Start(); - send_thread_.SetPriority(rtc::kRealtimePriority); insert_packet_thread_.Start(); - insert_packet_thread_.SetPriority(rtc::kRealtimePriority); pull_audio_thread_.Start(); - pull_audio_thread_.SetPriority(rtc::kRealtimePriority); } void TearDown() { AudioCodingModuleTestOldApi::TearDown(); + quit_.store(true); pull_audio_thread_.Stop(); send_thread_.Stop(); insert_packet_thread_.Stop(); @@ -532,14 +538,17 @@ class AudioCodingModuleMtTestOldApi : public AudioCodingModuleTestOldApi { return false; } - static bool CbSendThread(void* context) { - return reinterpret_cast(context) - ->CbSendImpl(); + static void CbSendThread(void* context) { + AudioCodingModuleMtTestOldApi* fixture = + reinterpret_cast(context); + while (!fixture->quit_.load()) { + fixture->CbSendImpl(); + } } // The send thread doesn't have to care about the current simulated time, // since only the AcmReceiver is using the clock. - bool CbSendImpl() { + void CbSendImpl() { SleepMs(1); if (HasFatalFailure()) { // End the test early if a fatal failure (ASSERT_*) has occurred. @@ -550,53 +559,59 @@ class AudioCodingModuleMtTestOldApi : public AudioCodingModuleTestOldApi { if (TestDone()) { test_complete_.Set(); } - return true; } - static bool CbInsertPacketThread(void* context) { - return reinterpret_cast(context) - ->CbInsertPacketImpl(); + static void CbInsertPacketThread(void* context) { + AudioCodingModuleMtTestOldApi* fixture = + reinterpret_cast(context); + while (!fixture->quit_.load()) { + fixture->CbInsertPacketImpl(); + } } - bool CbInsertPacketImpl() { + void CbInsertPacketImpl() { SleepMs(1); { rtc::CritScope lock(&crit_sect_); if (clock_->TimeInMilliseconds() < next_insert_packet_time_ms_) { - return true; + return; } next_insert_packet_time_ms_ += 10; } // Now we're not holding the crit sect when calling ACM. ++insert_packet_count_; InsertPacket(); - return true; } - static bool CbPullAudioThread(void* context) { - return reinterpret_cast(context) - ->CbPullAudioImpl(); + static void CbPullAudioThread(void* context) { + AudioCodingModuleMtTestOldApi* fixture = + reinterpret_cast(context); + while (!fixture->quit_.load()) { + fixture->CbPullAudioImpl(); + } } - bool CbPullAudioImpl() { + void CbPullAudioImpl() { SleepMs(1); { rtc::CritScope lock(&crit_sect_); // Don't let the insert thread fall behind. if (next_insert_packet_time_ms_ < clock_->TimeInMilliseconds()) { - return true; + return; } ++pull_audio_count_; } // Now we're not holding the crit sect when calling ACM. PullAudio(); fake_clock_->AdvanceTimeMilliseconds(10); - return true; } rtc::PlatformThread send_thread_; rtc::PlatformThread insert_packet_thread_; rtc::PlatformThread pull_audio_thread_; + // Used to force worker threads to stop looping. + std::atomic quit_; + rtc::Event test_complete_; int send_count_; int insert_packet_count_; @@ -734,10 +749,14 @@ class AcmReRegisterIsacMtTestOldApi : public AudioCodingModuleTestOldApi { AcmReRegisterIsacMtTestOldApi() : AudioCodingModuleTestOldApi(), - receive_thread_(CbReceiveThread, this, "receive"), + receive_thread_(CbReceiveThread, + this, + "receive", + rtc::kRealtimePriority), codec_registration_thread_(CbCodecRegistrationThread, this, - "codec_registration"), + "codec_registration", + rtc::kRealtimePriority), codec_registered_(false), receive_packet_count_(0), next_insert_packet_time_ms_(0), @@ -768,14 +787,14 @@ class AcmReRegisterIsacMtTestOldApi : public AudioCodingModuleTestOldApi { } void StartThreads() { + quit_.store(false); receive_thread_.Start(); - receive_thread_.SetPriority(rtc::kRealtimePriority); codec_registration_thread_.Start(); - codec_registration_thread_.SetPriority(rtc::kRealtimePriority); } void TearDown() override { AudioCodingModuleTestOldApi::TearDown(); + quit_.store(true); receive_thread_.Stop(); codec_registration_thread_.Stop(); } @@ -784,9 +803,11 @@ class AcmReRegisterIsacMtTestOldApi : public AudioCodingModuleTestOldApi { return test_complete_.Wait(10 * 60 * 1000); // 10 minutes' timeout. } - static bool CbReceiveThread(void* context) { - return reinterpret_cast(context) - ->CbReceiveImpl(); + static void CbReceiveThread(void* context) { + AcmReRegisterIsacMtTestOldApi* fixture = + reinterpret_cast(context); + while (!fixture->quit_.load() && fixture->CbReceiveImpl()) { + } } bool CbReceiveImpl() { @@ -834,12 +855,15 @@ class AcmReRegisterIsacMtTestOldApi : public AudioCodingModuleTestOldApi { return true; } - static bool CbCodecRegistrationThread(void* context) { - return reinterpret_cast(context) - ->CbCodecRegistrationImpl(); + static void CbCodecRegistrationThread(void* context) { + AcmReRegisterIsacMtTestOldApi* fixture = + reinterpret_cast(context); + while (!fixture->quit_.load()) { + fixture->CbCodecRegistrationImpl(); + } } - bool CbCodecRegistrationImpl() { + void CbCodecRegistrationImpl() { SleepMs(1); if (HasFatalFailure()) { // End the test early if a fatal failure (ASSERT_*) has occurred. @@ -856,11 +880,13 @@ class AcmReRegisterIsacMtTestOldApi : public AudioCodingModuleTestOldApi { if (codec_registered_ && receive_packet_count_ > kNumPackets) { test_complete_.Set(); } - return true; } rtc::PlatformThread receive_thread_; rtc::PlatformThread codec_registration_thread_; + // Used to force worker threads to stop looping. + std::atomic quit_; + rtc::Event test_complete_; rtc::CriticalSection crit_sect_; bool codec_registered_ RTC_GUARDED_BY(crit_sect_); diff --git a/modules/audio_device/dummy/file_audio_device.cc b/modules/audio_device/dummy/file_audio_device.cc index c5bdb3b4526..60ff9907bf1 100644 --- a/modules/audio_device/dummy/file_audio_device.cc +++ b/modules/audio_device/dummy/file_audio_device.cc @@ -217,9 +217,9 @@ int32_t FileAudioDevice::StartPlayout() { } _ptrThreadPlay.reset(new rtc::PlatformThread( - PlayThreadFunc, this, "webrtc_audio_module_play_thread")); + PlayThreadFunc, this, "webrtc_audio_module_play_thread", + rtc::kRealtimePriority)); _ptrThreadPlay->Start(); - _ptrThreadPlay->SetPriority(rtc::kRealtimePriority); RTC_LOG(LS_INFO) << "Started playout capture to output file: " << _outputFilename; @@ -277,10 +277,10 @@ int32_t FileAudioDevice::StartRecording() { } _ptrThreadRec.reset(new rtc::PlatformThread( - RecThreadFunc, this, "webrtc_audio_module_capture_thread")); + RecThreadFunc, this, "webrtc_audio_module_capture_thread", + rtc::kRealtimePriority)); _ptrThreadRec->Start(); - _ptrThreadRec->SetPriority(rtc::kRealtimePriority); RTC_LOG(LS_INFO) << "Started recording from input file: " << _inputFilename; @@ -439,12 +439,16 @@ void FileAudioDevice::AttachAudioBuffer(AudioDeviceBuffer* audioBuffer) { _ptrAudioBuffer->SetPlayoutChannels(0); } -bool FileAudioDevice::PlayThreadFunc(void* pThis) { - return (static_cast(pThis)->PlayThreadProcess()); +void FileAudioDevice::PlayThreadFunc(void* pThis) { + FileAudioDevice* device = static_cast(pThis); + while (device->PlayThreadProcess()) { + } } -bool FileAudioDevice::RecThreadFunc(void* pThis) { - return (static_cast(pThis)->RecThreadProcess()); +void FileAudioDevice::RecThreadFunc(void* pThis) { + FileAudioDevice* device = static_cast(pThis); + while (device->RecThreadProcess()) { + } } bool FileAudioDevice::PlayThreadProcess() { diff --git a/modules/audio_device/dummy/file_audio_device.h b/modules/audio_device/dummy/file_audio_device.h index 85dce077ef6..719d9a37822 100644 --- a/modules/audio_device/dummy/file_audio_device.h +++ b/modules/audio_device/dummy/file_audio_device.h @@ -127,8 +127,8 @@ class FileAudioDevice : public AudioDeviceGeneric { void AttachAudioBuffer(AudioDeviceBuffer* audioBuffer) override; private: - static bool RecThreadFunc(void*); - static bool PlayThreadFunc(void*); + static void RecThreadFunc(void*); + static void PlayThreadFunc(void*); bool RecThreadProcess(); bool PlayThreadProcess(); diff --git a/modules/audio_device/linux/audio_device_alsa_linux.cc b/modules/audio_device/linux/audio_device_alsa_linux.cc index ecf296398a0..99697e989b5 100644 --- a/modules/audio_device/linux/audio_device_alsa_linux.cc +++ b/modules/audio_device/linux/audio_device_alsa_linux.cc @@ -1027,10 +1027,10 @@ int32_t AudioDeviceLinuxALSA::StartRecording() { } // RECORDING _ptrThreadRec.reset(new rtc::PlatformThread( - RecThreadFunc, this, "webrtc_audio_module_capture_thread")); + RecThreadFunc, this, "webrtc_audio_module_capture_thread", + rtc::kRealtimePriority)); _ptrThreadRec->Start(); - _ptrThreadRec->SetPriority(rtc::kRealtimePriority); errVal = LATE(snd_pcm_prepare)(_handleRecord); if (errVal < 0) { @@ -1145,9 +1145,9 @@ int32_t AudioDeviceLinuxALSA::StartPlayout() { // PLAYOUT _ptrThreadPlay.reset(new rtc::PlatformThread( - PlayThreadFunc, this, "webrtc_audio_module_play_thread")); + PlayThreadFunc, this, "webrtc_audio_module_play_thread", + rtc::kRealtimePriority)); _ptrThreadPlay->Start(); - _ptrThreadPlay->SetPriority(rtc::kRealtimePriority); int errVal = LATE(snd_pcm_prepare)(_handlePlayout); if (errVal < 0) { @@ -1456,12 +1456,16 @@ int32_t AudioDeviceLinuxALSA::ErrorRecovery(int32_t error, // Thread Methods // ============================================================================ -bool AudioDeviceLinuxALSA::PlayThreadFunc(void* pThis) { - return (static_cast(pThis)->PlayThreadProcess()); +void AudioDeviceLinuxALSA::PlayThreadFunc(void* pThis) { + AudioDeviceLinuxALSA* device = static_cast(pThis); + while (device->PlayThreadProcess()) { + } } -bool AudioDeviceLinuxALSA::RecThreadFunc(void* pThis) { - return (static_cast(pThis)->RecThreadProcess()); +void AudioDeviceLinuxALSA::RecThreadFunc(void* pThis) { + AudioDeviceLinuxALSA* device = static_cast(pThis); + while (device->RecThreadProcess()) { + } } bool AudioDeviceLinuxALSA::PlayThreadProcess() { diff --git a/modules/audio_device/linux/audio_device_alsa_linux.h b/modules/audio_device/linux/audio_device_alsa_linux.h index d5202fb166c..4eb97afecc1 100644 --- a/modules/audio_device/linux/audio_device_alsa_linux.h +++ b/modules/audio_device/linux/audio_device_alsa_linux.h @@ -137,8 +137,8 @@ class AudioDeviceLinuxALSA : public AudioDeviceGeneric { inline int32_t InputSanityCheckAfterUnlockedPeriod() const; inline int32_t OutputSanityCheckAfterUnlockedPeriod() const; - static bool RecThreadFunc(void*); - static bool PlayThreadFunc(void*); + static void RecThreadFunc(void*); + static void PlayThreadFunc(void*); bool RecThreadProcess(); bool PlayThreadProcess(); diff --git a/modules/audio_device/linux/audio_device_pulse_linux.cc b/modules/audio_device/linux/audio_device_pulse_linux.cc index c29ef8b5e79..d93876c209d 100644 --- a/modules/audio_device/linux/audio_device_pulse_linux.cc +++ b/modules/audio_device/linux/audio_device_pulse_linux.cc @@ -45,10 +45,9 @@ AudioDeviceLinuxPulse::AudioDeviceLinuxPulse() _recIsInitialized(false), _playIsInitialized(false), _startRec(false), - _stopRec(false), _startPlay(false), - _stopPlay(false), update_speaker_volume_at_startup_(false), + quit_(false), _sndCardPlayDelay(0), _sndCardRecDelay(0), _writeErrors(0), @@ -159,17 +158,17 @@ AudioDeviceGeneric::InitStatus AudioDeviceLinuxPulse::Init() { #endif // RECORDING - _ptrThreadRec.reset(new rtc::PlatformThread( - RecThreadFunc, this, "webrtc_audio_module_rec_thread")); + _ptrThreadRec.reset(new rtc::PlatformThread(RecThreadFunc, this, + "webrtc_audio_module_rec_thread", + rtc::kRealtimePriority)); _ptrThreadRec->Start(); - _ptrThreadRec->SetPriority(rtc::kRealtimePriority); // PLAYOUT _ptrThreadPlay.reset(new rtc::PlatformThread( - PlayThreadFunc, this, "webrtc_audio_module_play_thread")); + PlayThreadFunc, this, "webrtc_audio_module_play_thread", + rtc::kRealtimePriority)); _ptrThreadPlay->Start(); - _ptrThreadPlay->SetPriority(rtc::kRealtimePriority); _initialized = true; @@ -181,7 +180,10 @@ int32_t AudioDeviceLinuxPulse::Terminate() { if (!_initialized) { return 0; } - + { + rtc::CritScope lock(&_critSect); + quit_ = true; + } _mixerManager.Close(); // RECORDING @@ -1977,12 +1979,16 @@ int32_t AudioDeviceLinuxPulse::ProcessRecordedData(int8_t* bufferData, return 0; } -bool AudioDeviceLinuxPulse::PlayThreadFunc(void* pThis) { - return (static_cast(pThis)->PlayThreadProcess()); +void AudioDeviceLinuxPulse::PlayThreadFunc(void* pThis) { + AudioDeviceLinuxPulse* device = static_cast(pThis); + while (device->PlayThreadProcess()) { + } } -bool AudioDeviceLinuxPulse::RecThreadFunc(void* pThis) { - return (static_cast(pThis)->RecThreadProcess()); +void AudioDeviceLinuxPulse::RecThreadFunc(void* pThis) { + AudioDeviceLinuxPulse* device = static_cast(pThis); + while (device->RecThreadProcess()) { + } } bool AudioDeviceLinuxPulse::PlayThreadProcess() { @@ -1992,6 +1998,10 @@ bool AudioDeviceLinuxPulse::PlayThreadProcess() { rtc::CritScope lock(&_critSect); + if (quit_) { + return false; + } + if (_startPlay) { RTC_LOG(LS_VERBOSE) << "_startPlay true, performing initial actions"; @@ -2159,7 +2169,9 @@ bool AudioDeviceLinuxPulse::RecThreadProcess() { } rtc::CritScope lock(&_critSect); - + if (quit_) { + return false; + } if (_startRec) { RTC_LOG(LS_VERBOSE) << "_startRec true, performing initial actions"; diff --git a/modules/audio_device/linux/audio_device_pulse_linux.h b/modules/audio_device/linux/audio_device_pulse_linux.h index c610892ba25..0ded20bfded 100644 --- a/modules/audio_device/linux/audio_device_pulse_linux.h +++ b/modules/audio_device/linux/audio_device_pulse_linux.h @@ -254,8 +254,8 @@ class AudioDeviceLinuxPulse : public AudioDeviceGeneric { void PaLock(); void PaUnLock(); - static bool RecThreadFunc(void*); - static bool PlayThreadFunc(void*); + static void RecThreadFunc(void*); + static void PlayThreadFunc(void*); bool RecThreadProcess(); bool PlayThreadProcess(); @@ -294,10 +294,9 @@ class AudioDeviceLinuxPulse : public AudioDeviceGeneric { bool _recIsInitialized; bool _playIsInitialized; bool _startRec; - bool _stopRec; bool _startPlay; - bool _stopPlay; bool update_speaker_volume_at_startup_; + bool quit_ RTC_GUARDED_BY(&_critSect); uint32_t _sndCardPlayDelay; uint32_t _sndCardRecDelay; diff --git a/modules/audio_device/mac/audio_device_mac.cc b/modules/audio_device/mac/audio_device_mac.cc index 9daf7bddf7b..5687dc7ff88 100644 --- a/modules/audio_device/mac/audio_device_mac.cc +++ b/modules/audio_device/mac/audio_device_mac.cc @@ -1295,11 +1295,10 @@ int32_t AudioDeviceMac::StartRecording() { } RTC_DCHECK(!capture_worker_thread_.get()); - capture_worker_thread_.reset( - new rtc::PlatformThread(RunCapture, this, "CaptureWorkerThread")); + capture_worker_thread_.reset(new rtc::PlatformThread( + RunCapture, this, "CaptureWorkerThread", rtc::kRealtimePriority)); RTC_DCHECK(capture_worker_thread_.get()); capture_worker_thread_->Start(); - capture_worker_thread_->SetPriority(rtc::kRealtimePriority); OSStatus err = noErr; if (_twoDevices) { @@ -1431,10 +1430,9 @@ int32_t AudioDeviceMac::StartPlayout() { } RTC_DCHECK(!render_worker_thread_.get()); - render_worker_thread_.reset( - new rtc::PlatformThread(RunRender, this, "RenderWorkerThread")); + render_worker_thread_.reset(new rtc::PlatformThread( + RunRender, this, "RenderWorkerThread", rtc::kRealtimePriority)); render_worker_thread_->Start(); - render_worker_thread_->SetPriority(rtc::kRealtimePriority); if (_twoDevices || !_recording) { OSStatus err = noErr; @@ -2361,8 +2359,10 @@ OSStatus AudioDeviceMac::implInConverterProc(UInt32* numberDataPackets, return 0; } -bool AudioDeviceMac::RunRender(void* ptrThis) { - return static_cast(ptrThis)->RenderWorkerThread(); +void AudioDeviceMac::RunRender(void* ptrThis) { + AudioDeviceMac* device = static_cast(ptrThis); + while (device->RenderWorkerThread()) { + } } bool AudioDeviceMac::RenderWorkerThread() { @@ -2430,8 +2430,10 @@ bool AudioDeviceMac::RenderWorkerThread() { return true; } -bool AudioDeviceMac::RunCapture(void* ptrThis) { - return static_cast(ptrThis)->CaptureWorkerThread(); +void AudioDeviceMac::RunCapture(void* ptrThis) { + AudioDeviceMac* device = static_cast(ptrThis); + while (device->CaptureWorkerThread()) { + } } bool AudioDeviceMac::CaptureWorkerThread() { diff --git a/modules/audio_device/mac/audio_device_mac.h b/modules/audio_device/mac/audio_device_mac.h index e9ea7a71e79..49c687d5ae6 100644 --- a/modules/audio_device/mac/audio_device_mac.h +++ b/modules/audio_device/mac/audio_device_mac.h @@ -240,8 +240,8 @@ class AudioDeviceMac : public AudioDeviceGeneric { OSStatus implInConverterProc(UInt32* numberDataPackets, AudioBufferList* data); - static bool RunCapture(void*); - static bool RunRender(void*); + static void RunCapture(void*); + static void RunRender(void*); bool CaptureWorkerThread(); bool RenderWorkerThread(); diff --git a/modules/audio_processing/audio_processing_impl_locking_unittest.cc b/modules/audio_processing/audio_processing_impl_locking_unittest.cc index 828e2e77212..906398075f7 100644 --- a/modules/audio_processing/audio_processing_impl_locking_unittest.cc +++ b/modules/audio_processing/audio_processing_impl_locking_unittest.cc @@ -310,10 +310,9 @@ class CaptureProcessor { rtc::Event* render_call_event, rtc::Event* capture_call_event, FrameCounters* shared_counters_state, - AudioProcessingImplLockTest* test_framework, TestConfig* test_config, AudioProcessing* apm); - bool Process(); + void Process(); private: static const int kMaxCallDifference = 10; @@ -328,7 +327,6 @@ class CaptureProcessor { rtc::Event* const render_call_event_ = nullptr; rtc::Event* const capture_call_event_ = nullptr; FrameCounters* const frame_counters_ = nullptr; - AudioProcessingImplLockTest* const test_ = nullptr; const TestConfig* const test_config_ = nullptr; AudioProcessing* const apm_ = nullptr; AudioFrameData frame_data_; @@ -340,7 +338,7 @@ class StatsProcessor { StatsProcessor(RandomGenerator* rand_gen, TestConfig* test_config, AudioProcessing* apm); - bool Process(); + void Process(); private: RandomGenerator* rand_gen_ = nullptr; @@ -356,10 +354,9 @@ class RenderProcessor { rtc::Event* render_call_event, rtc::Event* capture_call_event, FrameCounters* shared_counters_state, - AudioProcessingImplLockTest* test_framework, TestConfig* test_config, AudioProcessing* apm); - bool Process(); + void Process(); private: static const int kMaxCallDifference = 10; @@ -374,7 +371,6 @@ class RenderProcessor { rtc::Event* const render_call_event_ = nullptr; rtc::Event* const capture_call_event_ = nullptr; FrameCounters* const frame_counters_ = nullptr; - AudioProcessingImplLockTest* const test_ = nullptr; const TestConfig* const test_config_ = nullptr; AudioProcessing* const apm_ = nullptr; AudioFrameData frame_data_; @@ -397,21 +393,30 @@ class AudioProcessingImplLockTest void TearDown() override; // Thread callback for the render thread - static bool RenderProcessorThreadFunc(void* context) { - return reinterpret_cast(context) - ->render_thread_state_.Process(); + static void RenderProcessorThreadFunc(void* context) { + AudioProcessingImplLockTest* impl = + reinterpret_cast(context); + while (!impl->MaybeEndTest()) { + impl->render_thread_state_.Process(); + } } // Thread callback for the capture thread - static bool CaptureProcessorThreadFunc(void* context) { - return reinterpret_cast(context) - ->capture_thread_state_.Process(); + static void CaptureProcessorThreadFunc(void* context) { + AudioProcessingImplLockTest* impl = + reinterpret_cast(context); + while (!impl->MaybeEndTest()) { + impl->capture_thread_state_.Process(); + } } // Thread callback for the stats thread - static bool StatsProcessorThreadFunc(void* context) { - return reinterpret_cast(context) - ->stats_thread_state_.Process(); + static void StatsProcessorThreadFunc(void* context) { + AudioProcessingImplLockTest* impl = + reinterpret_cast(context); + while (!impl->MaybeEndTest()) { + impl->stats_thread_state_.Process(); + } } // Tests whether all the required render and capture side calls have been @@ -424,11 +429,8 @@ class AudioProcessingImplLockTest // 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 handlers for the test. @@ -487,16 +489,24 @@ void PopulateAudioFrame(AudioFrame* frame, } AudioProcessingImplLockTest::AudioProcessingImplLockTest() - : render_thread_(RenderProcessorThreadFunc, this, "render"), - capture_thread_(CaptureProcessorThreadFunc, this, "capture"), - stats_thread_(StatsProcessorThreadFunc, this, "stats"), + : render_thread_(RenderProcessorThreadFunc, + this, + "render", + rtc::kRealtimePriority), + capture_thread_(CaptureProcessorThreadFunc, + this, + "capture", + rtc::kRealtimePriority), + stats_thread_(StatsProcessorThreadFunc, + this, + "stats", + rtc::kNormalPriority), apm_(AudioProcessingBuilder().Create()), render_thread_state_(kMaxFrameSize, &rand_gen_, &render_call_event_, &capture_call_event_, &frame_counters_, - this, &test_config_, apm_.get()), capture_thread_state_(kMaxFrameSize, @@ -504,7 +514,6 @@ AudioProcessingImplLockTest::AudioProcessingImplLockTest() &render_call_event_, &capture_call_event_, &frame_counters_, - this, &test_config_, apm_.get()), stats_thread_state_(&rand_gen_, &test_config_, apm_.get()) {} @@ -570,7 +579,7 @@ StatsProcessor::StatsProcessor(RandomGenerator* rand_gen, // Implements the callback functionality for the statistics // collection thread. -bool StatsProcessor::Process() { +void StatsProcessor::Process() { SleepRandomMs(100, rand_gen_); AudioProcessing::Config apm_config = apm_->GetConfig(); @@ -590,8 +599,6 @@ bool StatsProcessor::Process() { apm_->voice_detection()->is_enabled(); apm_->GetStatistics(/*has_remote_tracks=*/true); - - return true; } const float CaptureProcessor::kCaptureInputFloatLevel = 0.03125f; @@ -601,28 +608,21 @@ CaptureProcessor::CaptureProcessor(int max_frame_size, rtc::Event* render_call_event, rtc::Event* capture_call_event, FrameCounters* shared_counters_state, - 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), - 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() { +void CaptureProcessor::Process() { // Sleep a random time to simulate thread jitter. SleepRandomMs(3, rand_gen_); - // Check whether the test is done. - if (test_->MaybeEndTest()) { - return false; - } - // Ensure that the number of render and capture calls do not // differ too much. if (frame_counters_->CaptureMinusRenderCounters() > kMaxCallDifference) { @@ -641,8 +641,6 @@ bool CaptureProcessor::Process() { // 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. @@ -868,20 +866,18 @@ RenderProcessor::RenderProcessor(int max_frame_size, rtc::Event* render_call_event, rtc::Event* capture_call_event, FrameCounters* shared_counters_state, - 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), - 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() { +void 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). @@ -893,11 +889,6 @@ bool RenderProcessor::Process() { // Sleep a random time to simulate thread jitter. SleepRandomMs(3, rand_gen_); - // Check whether the test is done. - if (test_->MaybeEndTest()) { - return false; - } - // Ensure that the number of render and capture calls do not // differ too much. if (frame_counters_->RenderMinusCaptureCounters() > kMaxCallDifference) { @@ -916,7 +907,6 @@ bool RenderProcessor::Process() { // Flag to the capture thread that another render API call has occurred // by triggering this threads call event. render_call_event_->Set(); - return true; } // Prepares the render side frame and the accompanying metadata diff --git a/modules/audio_processing/audio_processing_performance_unittest.cc b/modules/audio_processing/audio_processing_performance_unittest.cc index 70c91c4147d..993b8b6fb3a 100644 --- a/modules/audio_processing/audio_processing_performance_unittest.cc +++ b/modules/audio_processing/audio_processing_performance_unittest.cc @@ -391,11 +391,14 @@ class TimedThreadApiProcessor { class CallSimulator : public ::testing::TestWithParam { public: CallSimulator() - : render_thread_( - new rtc::PlatformThread(RenderProcessorThreadFunc, this, "render")), + : render_thread_(new rtc::PlatformThread(RenderProcessorThreadFunc, + this, + "render", + rtc::kRealtimePriority)), capture_thread_(new rtc::PlatformThread(CaptureProcessorThreadFunc, this, - "capture")), + "capture", + rtc::kRealtimePriority)), rand_gen_(42U), simulation_config_(static_cast(GetParam())) {} @@ -549,23 +552,23 @@ class CallSimulator : public ::testing::TestWithParam { } // Thread callback for the render thread. - static bool RenderProcessorThreadFunc(void* context) { - return reinterpret_cast(context) - ->render_thread_state_->Process(); + static void RenderProcessorThreadFunc(void* context) { + CallSimulator* call_simulator = reinterpret_cast(context); + while (call_simulator->render_thread_state_->Process()) { + } } // Thread callback for the capture thread. - static bool CaptureProcessorThreadFunc(void* context) { - return reinterpret_cast(context) - ->capture_thread_state_->Process(); + static void CaptureProcessorThreadFunc(void* context) { + CallSimulator* call_simulator = reinterpret_cast(context); + while (call_simulator->capture_thread_state_->Process()) { + } } // Start the threads used in the test. void StartThreads() { ASSERT_NO_FATAL_FAILURE(render_thread_->Start()); - render_thread_->SetPriority(rtc::kRealtimePriority); ASSERT_NO_FATAL_FAILURE(capture_thread_->Start()); - capture_thread_->SetPriority(rtc::kRealtimePriority); } // Event handler for the test. diff --git a/modules/utility/source/process_thread_impl.cc b/modules/utility/source/process_thread_impl.cc index b3fbaef7e3a..472ff33d2ef 100644 --- a/modules/utility/source/process_thread_impl.cc +++ b/modules/utility/source/process_thread_impl.cc @@ -162,8 +162,10 @@ void ProcessThreadImpl::DeRegisterModule(Module* module) { } // static -bool ProcessThreadImpl::Run(void* obj) { - return static_cast(obj)->Process(); +void ProcessThreadImpl::Run(void* obj) { + ProcessThreadImpl* impl = static_cast(obj); + while (impl->Process()) { + } } bool ProcessThreadImpl::Process() { diff --git a/modules/utility/source/process_thread_impl.h b/modules/utility/source/process_thread_impl.h index 7339fe1f87f..0b44340a2f0 100644 --- a/modules/utility/source/process_thread_impl.h +++ b/modules/utility/source/process_thread_impl.h @@ -42,7 +42,7 @@ class ProcessThreadImpl : public ProcessThread { void DeRegisterModule(Module* module) override; protected: - static bool Run(void* obj); + static void Run(void* obj); bool Process(); private: diff --git a/modules/video_capture/linux/video_capture_linux.cc b/modules/video_capture/linux/video_capture_linux.cc index d89046959c9..cfa47392dea 100644 --- a/modules/video_capture/linux/video_capture_linux.cc +++ b/modules/video_capture/linux/video_capture_linux.cc @@ -241,10 +241,11 @@ int32_t VideoCaptureModuleV4L2::StartCapture( // start capture thread; if (!_captureThread) { - _captureThread.reset(new rtc::PlatformThread( - VideoCaptureModuleV4L2::CaptureThread, this, "CaptureThread")); + quit_ = false; + _captureThread.reset( + new rtc::PlatformThread(VideoCaptureModuleV4L2::CaptureThread, this, + "CaptureThread", rtc::kHighPriority)); _captureThread->Start(); - _captureThread->SetPriority(rtc::kHighPriority); } // Needed to start UVC camera - from the uvcview application @@ -261,6 +262,10 @@ int32_t VideoCaptureModuleV4L2::StartCapture( int32_t VideoCaptureModuleV4L2::StopCapture() { if (_captureThread) { + { + rtc::CritScope cs(&_captureCritSect); + quit_ = true; + } // Make sure the capture thread stop stop using the critsect. _captureThread->Stop(); _captureThread.reset(); @@ -351,21 +356,22 @@ bool VideoCaptureModuleV4L2::CaptureStarted() { return _captureStarted; } -bool VideoCaptureModuleV4L2::CaptureThread(void* obj) { - return static_cast(obj)->CaptureProcess(); +void VideoCaptureModuleV4L2::CaptureThread(void* obj) { + VideoCaptureModuleV4L2* capture = static_cast(obj); + while (capture->CaptureProcess()) { + } } bool VideoCaptureModuleV4L2::CaptureProcess() { int retVal = 0; fd_set rSet; struct timeval timeout; - rtc::CritScope cs(&_captureCritSect); - FD_ZERO(&rSet); FD_SET(_deviceFd, &rSet); timeout.tv_sec = 1; timeout.tv_usec = 0; + // _deviceFd written only in StartCapture, when this thread isn't running. retVal = select(_deviceFd + 1, &rSet, NULL, NULL, &timeout); if (retVal < 0 && errno != EINTR) // continue if interrupted { @@ -379,30 +385,38 @@ bool VideoCaptureModuleV4L2::CaptureProcess() { return true; } - if (_captureStarted) { - struct v4l2_buffer buf; - memset(&buf, 0, sizeof(struct v4l2_buffer)); - buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE; - buf.memory = V4L2_MEMORY_MMAP; - // dequeue a buffer - repeat until dequeued properly! - while (ioctl(_deviceFd, VIDIOC_DQBUF, &buf) < 0) { - if (errno != EINTR) { - RTC_LOG(LS_INFO) << "could not sync on a buffer on device " - << strerror(errno); - return true; - } + { + rtc::CritScope cs(&_captureCritSect); + + if (quit_) { + return false; } - VideoCaptureCapability frameInfo; - frameInfo.width = _currentWidth; - frameInfo.height = _currentHeight; - frameInfo.videoType = _captureVideoType; - - // convert to to I420 if needed - IncomingFrame((unsigned char*)_pool[buf.index].start, buf.bytesused, - frameInfo); - // enqueue the buffer again - if (ioctl(_deviceFd, VIDIOC_QBUF, &buf) == -1) { - RTC_LOG(LS_INFO) << "Failed to enqueue capture buffer"; + + if (_captureStarted) { + struct v4l2_buffer buf; + memset(&buf, 0, sizeof(struct v4l2_buffer)); + buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE; + buf.memory = V4L2_MEMORY_MMAP; + // dequeue a buffer - repeat until dequeued properly! + while (ioctl(_deviceFd, VIDIOC_DQBUF, &buf) < 0) { + if (errno != EINTR) { + RTC_LOG(LS_INFO) << "could not sync on a buffer on device " + << strerror(errno); + return true; + } + } + VideoCaptureCapability frameInfo; + frameInfo.width = _currentWidth; + frameInfo.height = _currentHeight; + frameInfo.videoType = _captureVideoType; + + // convert to to I420 if needed + IncomingFrame((unsigned char*)_pool[buf.index].start, buf.bytesused, + frameInfo); + // enqueue the buffer again + if (ioctl(_deviceFd, VIDIOC_QBUF, &buf) == -1) { + RTC_LOG(LS_INFO) << "Failed to enqueue capture buffer"; + } } } usleep(0); diff --git a/modules/video_capture/linux/video_capture_linux.h b/modules/video_capture/linux/video_capture_linux.h index 55855833099..317b011b7f5 100644 --- a/modules/video_capture/linux/video_capture_linux.h +++ b/modules/video_capture/linux/video_capture_linux.h @@ -35,7 +35,7 @@ class VideoCaptureModuleV4L2 : public VideoCaptureImpl { private: enum { kNoOfV4L2Bufffers = 4 }; - static bool CaptureThread(void*); + static void CaptureThread(void*); bool CaptureProcess(); bool AllocateVideoBuffers(); bool DeAllocateVideoBuffers(); @@ -43,7 +43,7 @@ class VideoCaptureModuleV4L2 : public VideoCaptureImpl { // TODO(pbos): Stop using unique_ptr and resetting the thread. std::unique_ptr _captureThread; rtc::CriticalSection _captureCritSect; - + bool quit_ RTC_GUARDED_BY(_captureCritSect); int32_t _deviceId; int32_t _deviceFd; diff --git a/rtc_base/cpu_time_unittest.cc b/rtc_base/cpu_time_unittest.cc index b5e0f67c001..79f0a4036fc 100644 --- a/rtc_base/cpu_time_unittest.cc +++ b/rtc_base/cpu_time_unittest.cc @@ -30,7 +30,7 @@ const int kProcessingTimeMillisecs = 300; const int kWorkingThreads = 2; // Consumes approximately kProcessingTimeMillisecs of CPU time in single thread. -bool WorkingFunction(void* counter_pointer) { +void WorkingFunction(void* counter_pointer) { int64_t* counter = reinterpret_cast(counter_pointer); *counter = 0; int64_t stop_cpu_time = @@ -39,7 +39,6 @@ bool WorkingFunction(void* counter_pointer) { while (rtc::GetThreadCpuTimeNanos() < stop_cpu_time) { (*counter)++; } - return false; } } // namespace diff --git a/rtc_base/critical_section_unittest.cc b/rtc_base/critical_section_unittest.cc index 5405c61e2a1..cf9dfaf3bba 100644 --- a/rtc_base/critical_section_unittest.cc +++ b/rtc_base/critical_section_unittest.cc @@ -359,11 +359,10 @@ class PerfTestThread { } private: - static bool ThreadFunc(void* param) { + static void ThreadFunc(void* param) { PerfTestThread* me = static_cast(param); for (int i = 0; i < me->repeats_; ++i) me->data_->AddToCounter(me->my_id_); - return false; } PlatformThread thread_; diff --git a/rtc_base/platform_thread.cc b/rtc_base/platform_thread.cc index 16cf1cca07b..f2a1f00975a 100644 --- a/rtc_base/platform_thread.cc +++ b/rtc_base/platform_thread.cc @@ -17,9 +17,7 @@ #include #include -#include "rtc_base/atomic_ops.h" #include "rtc_base/checks.h" -#include "rtc_base/time_utils.h" namespace rtc { namespace { @@ -37,15 +35,6 @@ struct ThreadAttributes { #endif // defined(WEBRTC_WIN) } -PlatformThread::PlatformThread(ThreadRunFunctionDeprecated func, - void* obj, - absl::string_view thread_name) - : run_function_deprecated_(func), obj_(obj), name_(thread_name) { - RTC_DCHECK(func); - RTC_DCHECK(name_.length() < 64); - spawned_thread_checker_.Detach(); -} - PlatformThread::PlatformThread(ThreadRunFunction func, void* obj, absl::string_view thread_name, @@ -143,96 +132,22 @@ void PlatformThread::Stop() { thread_ = nullptr; thread_id_ = 0; #else - if (!run_function_) - RTC_CHECK_EQ(1, AtomicOps::Increment(&stop_flag_)); RTC_CHECK_EQ(0, pthread_join(thread_, nullptr)); - if (!run_function_) - AtomicOps::ReleaseStore(&stop_flag_, 0); thread_ = 0; #endif // defined(WEBRTC_WIN) spawned_thread_checker_.Detach(); } -// TODO(tommi): Deprecate the loop behavior in PlatformThread. -// * Introduce a new callback type that returns void. -// * Remove potential for a busy loop in PlatformThread. -// * Delegate the responsibility for how to stop the thread, to the -// implementation that actually uses the thread. -// All implementations will need to be aware of how the thread should be stopped -// and encouraging a busy polling loop, can be costly in terms of power and cpu. void PlatformThread::Run() { // Attach the worker thread checker to this thread. RTC_DCHECK(spawned_thread_checker_.IsCurrent()); rtc::SetCurrentThreadName(name_.c_str()); - - if (run_function_) { - SetPriority(priority_); - run_function_(obj_); - return; - } - -// TODO(tommi): Delete the rest of this function when looping isn't supported. -#if RTC_DCHECK_IS_ON - // These constants control the busy loop detection algorithm below. - // |kMaxLoopCount| controls the limit for how many times we allow the loop - // to run within a period, before DCHECKing. - // |kPeriodToMeasureMs| controls how long that period is. - static const int kMaxLoopCount = 1000; - static const int kPeriodToMeasureMs = 100; - int64_t loop_stamps[kMaxLoopCount] = {}; - int64_t sequence_nr = 0; -#endif - - do { - // The interface contract of Start/Stop is that for a successful call to - // Start, there should be at least one call to the run function. So we - // call the function before checking |stop_|. - if (!run_function_deprecated_(obj_)) - break; -#if RTC_DCHECK_IS_ON - auto id = sequence_nr % kMaxLoopCount; - loop_stamps[id] = rtc::TimeMillis(); - if (sequence_nr > kMaxLoopCount) { - auto compare_id = (id + 1) % kMaxLoopCount; - auto diff = loop_stamps[id] - loop_stamps[compare_id]; - RTC_DCHECK_GE(diff, 0); - if (diff < kPeriodToMeasureMs) { - RTC_NOTREACHED() << "This thread is too busy: " << name_ << " " << diff - << "ms sequence=" << sequence_nr << " " - << loop_stamps[id] << " vs " << loop_stamps[compare_id] - << ", " << id << " vs " << compare_id; - } - } - ++sequence_nr; -#endif -#if defined(WEBRTC_WIN) - // Alertable sleep to permit RaiseFlag to run and update |stop_|. - SleepEx(0, true); - } while (!stop_); -#else -#if defined(WEBRTC_MAC) || defined(WEBRTC_ANDROID) - sched_yield(); -#else - static const struct timespec ts_null = {0}; - nanosleep(&ts_null, nullptr); -#endif - } while (!AtomicOps::AcquireLoad(&stop_flag_)); -#endif // defined(WEBRTC_WIN) + SetPriority(priority_); + run_function_(obj_); } bool PlatformThread::SetPriority(ThreadPriority priority) { -#if RTC_DCHECK_IS_ON - if (run_function_) { - // The non-deprecated way of how this function gets called, is that it must - // be called on the worker thread itself. - RTC_DCHECK(spawned_thread_checker_.IsCurrent()); - } else { - // In the case of deprecated use of this method, it must be called on the - // same thread as the PlatformThread object is constructed on. - RTC_DCHECK(thread_checker_.IsCurrent()); - RTC_DCHECK(IsRunning()); - } -#endif + RTC_DCHECK(spawned_thread_checker_.IsCurrent()); #if defined(WEBRTC_WIN) return SetThreadPriority(thread_, priority) != FALSE; diff --git a/rtc_base/platform_thread.h b/rtc_base/platform_thread.h index 99c5f909fed..d7f9a49adf4 100644 --- a/rtc_base/platform_thread.h +++ b/rtc_base/platform_thread.h @@ -24,9 +24,6 @@ namespace rtc { // Callback function that the spawned thread will enter once spawned. -// A return value of false is interpreted as that the function has no -// more work to do and that the thread can be released. -typedef bool (*ThreadRunFunctionDeprecated)(void*); typedef void (*ThreadRunFunction)(void*); enum ThreadPriority { @@ -50,9 +47,6 @@ enum ThreadPriority { // called from the same thread, including instantiation. class PlatformThread { public: - PlatformThread(ThreadRunFunctionDeprecated func, - void* obj, - absl::string_view thread_name); PlatformThread(ThreadRunFunction func, void* obj, absl::string_view thread_name, @@ -74,10 +68,6 @@ class PlatformThread { // Stops (joins) the spawned thread. void Stop(); - // Set the priority of the thread. Must be called when thread is running. - // TODO(tommi): Make private and only allow public support via ctor. - bool SetPriority(ThreadPriority priority); - protected: #if defined(WEBRTC_WIN) // Exposed to derived classes to allow for special cases specific to Windows. @@ -86,8 +76,8 @@ class PlatformThread { private: void Run(); + bool SetPriority(ThreadPriority priority); - ThreadRunFunctionDeprecated const run_function_deprecated_ = nullptr; ThreadRunFunction const run_function_ = nullptr; const ThreadPriority priority_ = kNormalPriority; void* const obj_; @@ -105,9 +95,6 @@ class PlatformThread { #else static void* StartThread(void* param); - // An atomic flag that we use to stop the thread. Only modified on the - // controlling thread and checked on the worker thread. - volatile int stop_flag_ = 0; pthread_t thread_ = 0; #endif // defined(WEBRTC_WIN) RTC_DISALLOW_COPY_AND_ASSIGN(PlatformThread); diff --git a/rtc_base/platform_thread_unittest.cc b/rtc_base/platform_thread_unittest.cc index 1661b6a999a..3f0408aa4b3 100644 --- a/rtc_base/platform_thread_unittest.cc +++ b/rtc_base/platform_thread_unittest.cc @@ -15,27 +15,10 @@ namespace rtc { namespace { -// Function that does nothing, and reports success. -bool NullRunFunctionDeprecated(void* obj) { - webrtc::SleepMs(2); // Hand over timeslice, prevents busy looping. - return true; -} - -bool TooBusyRunFunction(void* obj) { - // Indentionally busy looping. - return true; -} void NullRunFunction(void* obj) {} // Function that sets a boolean. -bool SetFlagRunFunctionDeprecated(void* obj) { - bool* obj_as_bool = static_cast(obj); - *obj_as_bool = true; - webrtc::SleepMs(0); // Hand over timeslice, prevents busy looping. - return true; -} - void SetFlagRunFunction(void* obj) { bool* obj_as_bool = static_cast(obj); *obj_as_bool = true; @@ -43,43 +26,6 @@ void SetFlagRunFunction(void* obj) { } // namespace -TEST(PlatformThreadTest, StartStopDeprecated) { - PlatformThread thread(&NullRunFunctionDeprecated, nullptr, - "PlatformThreadTest"); - EXPECT_TRUE(thread.name() == "PlatformThreadTest"); - EXPECT_TRUE(thread.GetThreadRef() == 0); - thread.Start(); - EXPECT_TRUE(thread.GetThreadRef() != 0); - thread.Stop(); - EXPECT_TRUE(thread.GetThreadRef() == 0); -} - -TEST(PlatformThreadTest, StartStop2Deprecated) { - PlatformThread thread1(&NullRunFunctionDeprecated, nullptr, - "PlatformThreadTest1"); - PlatformThread thread2(&NullRunFunctionDeprecated, nullptr, - "PlatformThreadTest2"); - EXPECT_TRUE(thread1.GetThreadRef() == thread2.GetThreadRef()); - thread1.Start(); - thread2.Start(); - EXPECT_TRUE(thread1.GetThreadRef() != thread2.GetThreadRef()); - thread2.Stop(); - thread1.Stop(); -} - -TEST(PlatformThreadTest, RunFunctionIsCalledDeprecated) { - bool flag = false; - PlatformThread thread(&SetFlagRunFunctionDeprecated, &flag, - "RunFunctionIsCalled"); - thread.Start(); - - // At this point, the flag may be either true or false. - thread.Stop(); - - // We expect the thread to have run at least once. - EXPECT_TRUE(flag); -} - TEST(PlatformThreadTest, StartStop) { PlatformThread thread(&NullRunFunction, nullptr, "PlatformThreadTest"); EXPECT_TRUE(thread.name() == "PlatformThreadTest"); @@ -113,16 +59,4 @@ TEST(PlatformThreadTest, RunFunctionIsCalled) { EXPECT_TRUE(flag); } -// This test is disabled since it will cause a crash. -// There might be a way to implement this as a death test, but it looks like -// a death test requires an expression to be checked but does not allow a -// flag to be raised that says "some thread will crash after this point". -// TODO(tommi): Look into ways to enable the test by default. -TEST(PlatformThreadTest, DISABLED_TooBusyDeprecated) { - PlatformThread thread(&TooBusyRunFunction, nullptr, "BusyThread"); - thread.Start(); - webrtc::SleepMs(1000); - thread.Stop(); -} - } // namespace rtc diff --git a/video/video_analyzer.cc b/video/video_analyzer.cc index d74b79ab606..d3aa263d280 100644 --- a/video/video_analyzer.cc +++ b/video/video_analyzer.cc @@ -101,6 +101,7 @@ VideoAnalyzer::VideoAnalyzer( avg_psnr_threshold_(avg_psnr_threshold), avg_ssim_threshold_(avg_ssim_threshold), is_quick_test_enabled_(is_quick_test_enabled), + quit_(false), done_(true, false), clock_(clock), start_ms_(clock->TimeInMilliseconds()), @@ -139,6 +140,10 @@ VideoAnalyzer::VideoAnalyzer( } VideoAnalyzer::~VideoAnalyzer() { + { + rtc::CritScope crit(&comparison_lock_); + quit_ = true; + } for (rtc::PlatformThread* thread : comparison_thread_pool_) { thread->Stop(); delete thread; @@ -519,8 +524,10 @@ void VideoAnalyzer::PollStats() { [this]() { PollStats(); }, kSendStatsPollingIntervalMs); } -bool VideoAnalyzer::FrameComparisonThread(void* obj) { - return static_cast(obj)->CompareFrames(); +void VideoAnalyzer::FrameComparisonThread(void* obj) { + VideoAnalyzer* analyzer = static_cast(obj); + while (analyzer->CompareFrames()) { + } } bool VideoAnalyzer::CompareFrames() { @@ -579,8 +586,8 @@ void VideoAnalyzer::FrameRecorded() { bool VideoAnalyzer::AllFramesRecorded() { rtc::CritScope crit(&comparison_lock_); - assert(frames_recorded_ <= frames_to_process_); - return frames_recorded_ == frames_to_process_; + RTC_DCHECK(frames_recorded_ <= frames_to_process_); + return frames_recorded_ == frames_to_process_ || quit_; } bool VideoAnalyzer::FrameProcessed() { diff --git a/video/video_analyzer.h b/video/video_analyzer.h index 855034565c9..f3a4fd0b64a 100644 --- a/video/video_analyzer.h +++ b/video/video_analyzer.h @@ -180,7 +180,7 @@ class VideoAnalyzer : public PacketReceiver, RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_); void PollStats(); - static bool FrameComparisonThread(void* obj); + static void FrameComparisonThread(void* obj); bool CompareFrames(); bool PopComparison(FrameComparison* comparison); // Increment counter for number of frames received for comparison. @@ -275,6 +275,7 @@ class VideoAnalyzer : public PacketReceiver, std::vector comparison_thread_pool_; rtc::Event comparison_available_event_; std::deque comparisons_ RTC_GUARDED_BY(comparison_lock_); + bool quit_ RTC_GUARDED_BY(comparison_lock_); rtc::Event done_; test::SingleThreadedTaskQueueForTesting::TaskId stats_polling_task_id_ RTC_GUARDED_BY(comparison_lock_);