From 45eff329e0a71647f56ce4455ba8356feaf4298d Mon Sep 17 00:00:00 2001 From: Patrick Rohr Date: Fri, 23 Jul 2021 14:56:53 +0200 Subject: [PATCH 1/2] Clean Up Playback Thread in DVR Test Implementation Bug: 194476544 Test: atest VtsHalTvTunerV1_0TargetTest && atest VtsHalTvTunerV1_1TargetTest Change-Id: I639e8314a499c07758c4927fa10cb4ff0e6dcb50 --- tv/tuner/1.0/default/Dvr.cpp | 25 +++++++++++-------------- tv/tuner/1.0/default/Dvr.h | 8 ++++---- tv/tuner/1.1/default/Dvr.cpp | 25 +++++++++---------------- tv/tuner/1.1/default/Dvr.h | 8 ++++---- 4 files changed, 28 insertions(+), 38 deletions(-) diff --git a/tv/tuner/1.0/default/Dvr.cpp b/tv/tuner/1.0/default/Dvr.cpp index c62b878891..40879f2041 100644 --- a/tv/tuner/1.0/default/Dvr.cpp +++ b/tv/tuner/1.0/default/Dvr.cpp @@ -37,7 +37,10 @@ Dvr::Dvr(DvrType type, uint32_t bufferSize, const sp& cb, sp Dvr::getQueueDesc(getQueueDesc_cb _hidl_cb) { ALOGV("%s", __FUNCTION__); @@ -112,8 +115,7 @@ Return Dvr::start() { } if (mType == DvrType::PLAYBACK) { - pthread_create(&mDvrThread, NULL, __threadLoopPlayback, this); - pthread_setname_np(mDvrThread, "playback_waiting_loop"); + mDvrThread = std::thread(&Dvr::playbackThreadLoop, this); } else if (mType == DvrType::RECORD) { mRecordStatus = RecordStatus::DATA_READY; mDemux->setIsRecording(mType == DvrType::RECORD); @@ -128,9 +130,11 @@ Return Dvr::stop() { ALOGV("%s", __FUNCTION__); mDvrThreadRunning = false; - - lock_guard lock(mDvrThreadLock); - + if (mDvrThread.joinable()) { + mDvrThread.join(); + } + // thread should always be joinable if it is running, + // so it should be safe to assume recording stopped. mDemux->setIsRecording(false); return Result::SUCCESS; @@ -146,7 +150,7 @@ Return Dvr::flush() { Return Dvr::close() { ALOGV("%s", __FUNCTION__); - + stop(); return Result::SUCCESS; } @@ -173,15 +177,8 @@ EventFlag* Dvr::getDvrEventFlag() { return mDvrEventFlag; } -void* Dvr::__threadLoopPlayback(void* user) { - Dvr* const self = static_cast(user); - self->playbackThreadLoop(); - return 0; -} - void Dvr::playbackThreadLoop() { ALOGD("[Dvr] playback threadLoop start."); - lock_guard lock(mDvrThreadLock); mDvrThreadRunning = true; while (mDvrThreadRunning) { diff --git a/tv/tuner/1.0/default/Dvr.h b/tv/tuner/1.0/default/Dvr.h index fc4cb21791..264268a264 100644 --- a/tv/tuner/1.0/default/Dvr.h +++ b/tv/tuner/1.0/default/Dvr.h @@ -20,7 +20,9 @@ #include #include #include +#include #include +#include #include "Demux.h" #include "Frontend.h" #include "Tuner.h" @@ -119,7 +121,6 @@ class Dvr : public IDvr { * Each filter handler handles the data filtering/output writing/filterEvent updating. */ void startTpidFilter(vector data); - static void* __threadLoopPlayback(void* user); static void* __threadLoopRecord(void* user); void playbackThreadLoop(); void recordThreadLoop(); @@ -133,7 +134,7 @@ class Dvr : public IDvr { DvrSettings mDvrSettings; // Thread handlers - pthread_t mDvrThread; + std::thread mDvrThread; // FMQ status local records PlaybackStatus mPlaybackStatus; @@ -141,7 +142,7 @@ class Dvr : public IDvr { /** * If a specific filter's writing loop is still running */ - bool mDvrThreadRunning; + std::atomic mDvrThreadRunning; bool mKeepFetchingDataFromFrontend; /** * Lock to protect writes to the FMQs @@ -152,7 +153,6 @@ class Dvr : public IDvr { */ std::mutex mPlaybackStatusLock; std::mutex mRecordStatusLock; - std::mutex mDvrThreadLock; const bool DEBUG_DVR = false; }; diff --git a/tv/tuner/1.1/default/Dvr.cpp b/tv/tuner/1.1/default/Dvr.cpp index fd84d49946..fdb66c1fd4 100644 --- a/tv/tuner/1.1/default/Dvr.cpp +++ b/tv/tuner/1.1/default/Dvr.cpp @@ -38,8 +38,8 @@ Dvr::Dvr(DvrType type, uint32_t bufferSize, const sp& cb, sp lock(mDvrThreadLock); + // make sure thread has joined + close(); } Return Dvr::getQueueDesc(getQueueDesc_cb _hidl_cb) { @@ -134,8 +134,7 @@ Return Dvr::start() { if (mType == DvrType::PLAYBACK) { mDvrThreadRunning = true; - pthread_create(&mDvrThread, NULL, __threadLoopPlayback, this); - pthread_setname_np(mDvrThread, "playback_waiting_loop"); + mDvrThread = std::thread(&Dvr::playbackThreadLoop, this); } else if (mType == DvrType::RECORD) { mRecordStatus = RecordStatus::DATA_READY; mDemux->setIsRecording(mType == DvrType::RECORD); @@ -150,8 +149,11 @@ Return Dvr::stop() { ALOGV("%s", __FUNCTION__); mDvrThreadRunning = false; - lock_guard lock(mDvrThreadLock); - + if (mDvrThread.joinable()) { + mDvrThread.join(); + } + // thread should always be joinable if it is running, + // so it should be safe to assume recording stopped. mDemux->setIsRecording(false); return Result::SUCCESS; @@ -167,9 +169,7 @@ Return Dvr::flush() { Return Dvr::close() { ALOGV("%s", __FUNCTION__); - - mDvrThreadRunning = false; - lock_guard lock(mDvrThreadLock); + stop(); return Result::SUCCESS; } @@ -196,15 +196,8 @@ EventFlag* Dvr::getDvrEventFlag() { return mDvrEventFlag; } -void* Dvr::__threadLoopPlayback(void* user) { - Dvr* const self = static_cast(user); - self->playbackThreadLoop(); - return 0; -} - void Dvr::playbackThreadLoop() { ALOGD("[Dvr] playback threadLoop start."); - lock_guard lock(mDvrThreadLock); while (mDvrThreadRunning) { uint32_t efState = 0; diff --git a/tv/tuner/1.1/default/Dvr.h b/tv/tuner/1.1/default/Dvr.h index 9fabb49da5..bf46c1e3cd 100644 --- a/tv/tuner/1.1/default/Dvr.h +++ b/tv/tuner/1.1/default/Dvr.h @@ -19,7 +19,9 @@ #include #include +#include #include +#include #include "Demux.h" #include "Frontend.h" #include "Tuner.h" @@ -115,7 +117,6 @@ class Dvr : public IDvr { * Each filter handler handles the data filtering/output writing/filterEvent updating. */ void startTpidFilter(vector data); - static void* __threadLoopPlayback(void* user); static void* __threadLoopRecord(void* user); void playbackThreadLoop(); void recordThreadLoop(); @@ -129,7 +130,7 @@ class Dvr : public IDvr { DvrSettings mDvrSettings; // Thread handlers - pthread_t mDvrThread; + std::thread mDvrThread; // FMQ status local records PlaybackStatus mPlaybackStatus; @@ -137,7 +138,7 @@ class Dvr : public IDvr { /** * If a specific filter's writing loop is still running */ - bool mDvrThreadRunning; + std::atomic mDvrThreadRunning; bool mKeepFetchingDataFromFrontend; /** * Lock to protect writes to the FMQs @@ -148,7 +149,6 @@ class Dvr : public IDvr { */ std::mutex mPlaybackStatusLock; std::mutex mRecordStatusLock; - std::mutex mDvrThreadLock; const bool DEBUG_DVR = false; }; From eae26b7608e5018320601742d1eb1910d53595e8 Mon Sep 17 00:00:00 2001 From: Patrick Rohr Date: Fri, 23 Jul 2021 15:00:27 +0200 Subject: [PATCH 2/2] Remove Unused Function Declarations from DVR Default Implementation Bug: 194476544 Test: atest VtsHalTvTunerV1_0TargetTest && atest VtsHalTvTunerV1_1TargetTest Change-Id: I388fc2b864763ca38f960de2698d292964d0c15b --- tv/tuner/1.0/default/Dvr.h | 2 -- tv/tuner/1.1/default/Dvr.h | 2 -- 2 files changed, 4 deletions(-) diff --git a/tv/tuner/1.0/default/Dvr.h b/tv/tuner/1.0/default/Dvr.h index 264268a264..fd7fba2c0a 100644 --- a/tv/tuner/1.0/default/Dvr.h +++ b/tv/tuner/1.0/default/Dvr.h @@ -121,9 +121,7 @@ class Dvr : public IDvr { * Each filter handler handles the data filtering/output writing/filterEvent updating. */ void startTpidFilter(vector data); - static void* __threadLoopRecord(void* user); void playbackThreadLoop(); - void recordThreadLoop(); unique_ptr mDvrMQ; EventFlag* mDvrEventFlag; diff --git a/tv/tuner/1.1/default/Dvr.h b/tv/tuner/1.1/default/Dvr.h index bf46c1e3cd..1bbb55c45e 100644 --- a/tv/tuner/1.1/default/Dvr.h +++ b/tv/tuner/1.1/default/Dvr.h @@ -117,9 +117,7 @@ class Dvr : public IDvr { * Each filter handler handles the data filtering/output writing/filterEvent updating. */ void startTpidFilter(vector data); - static void* __threadLoopRecord(void* user); void playbackThreadLoop(); - void recordThreadLoop(); unique_ptr mDvrMQ; EventFlag* mDvrEventFlag;