Revise cpp codes (mutex/cond) 76/207376/10 accepted/tizen/unified/20200411.115541 submit/tizen/20200410.105914
authorSeungbae Shin <seungbae.shin@samsung.com>
Tue, 4 Jun 2019 05:31:25 +0000 (14:31 +0900)
committerSeungbae Shin <seungbae.shin@samsung.com>
Thu, 19 Mar 2020 07:01:22 +0000 (16:01 +0900)
use std::mutex instead of pthread_mutex for internal locking.
use lock_guard/unique_lock whenever possible, which is convenient RAII-style mechanism for owning a mutex.
use std::condition_variable instead of pthread_cond for internal wait/signal

[Version] 0.5.26
[Issue Type] Revise

Change-Id: I2aee6405c173c5df2b391998d8e483e1c528c453

CMakeLists.txt
include/CAudioIO.h
packaging/capi-media-audio-io.spec
src/cpp/CAudioIO.cpp
src/cpp/CAudioInput.cpp
src/cpp/CAudioOutput.cpp

index 297c610..3026dc5 100644 (file)
@@ -22,7 +22,7 @@ ENDFOREACH(flag)
 SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${EXTRA_CFLAGS} -fPIC -Wall -Werror")
 SET(CMAKE_C_FLAGS_DEBUG "-O0 -g")
 
-SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${EXTRA_CXXFLAGS} -fPIC -Wall -Werror -std=c++0x")
+SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${EXTRA_CXXFLAGS} -fPIC -Wall -Werror -std=c++14")
 SET(CMAKE_CXX_FLAGS_DEBUG "-O0 -g")
 
 IF("${ARCH}" STREQUAL "arm")
index 667d3ae..3a84a75 100644 (file)
 #ifndef __TIZEN_MEDIA_AUDIO_IO_CAUDIO_IO_H__
 #define __TIZEN_MEDIA_AUDIO_IO_CAUDIO_IO_H__
 
-#include <sound_manager.h>
-
 #ifdef __cplusplus
 
+#include <sound_manager.h>
+#include <mutex>
+#include <condition_variable>
 
 namespace tizen_media_audio {
 
@@ -108,11 +109,6 @@ namespace tizen_media_audio {
         virtual bool isInit();
         virtual bool IsReady();
 
-        void internalLock();
-        void internalUnlock();
-        void internalWait();
-        void internalSignal();
-
         CPulseAudioClient*    mpPulseAudioClient;
         CAudioInfo            mAudioInfo;
 
@@ -123,11 +119,11 @@ namespace tizen_media_audio {
         CAudioInfo::EAudioIOState mState;
         CAudioInfo::EAudioIOState mStatePrev;
         bool                  mByPolicy;
+        std::mutex            mMutex;
+        std::mutex            mCondMutex;
+        std::condition_variable mCond;
 
     private:
-        pthread_mutex_t       __mMutex;
-        pthread_mutex_t       __mCondMutex;
-        pthread_cond_t        __mCond;
         bool                  __mIsInit;
     };
 
index fe2c544..2fef947 100644 (file)
@@ -1,6 +1,6 @@
 Name:           capi-media-audio-io
 Summary:        An Audio Input & Audio Output library in Tizen Native API
-Version:        0.5.25
+Version:        0.5.26
 Release:        0
 Group:          Multimedia/API
 License:        Apache-2.0
index 00cfd59..3115d75 100644 (file)
@@ -15,7 +15,6 @@
  */
 
 
-#include <pthread.h>
 #include <assert.h>
 #include "CAudioIODef.h"
 #include <sound_manager_internal.h>
@@ -32,9 +31,6 @@ using namespace tizen_media_audio;
 //LCOV_EXCL_START
 CAudioIO::CAudioIO() :
     mpPulseAudioClient(nullptr),
-    __mMutex(PTHREAD_MUTEX_INITIALIZER),
-    __mCondMutex(PTHREAD_MUTEX_INITIALIZER),
-    __mCond(PTHREAD_COND_INITIALIZER),
     __mIsInit(false) {
     mDirection = CAudioInfo::EAudioDirection::AUDIO_DIRECTION_MAX;
     mState = CAudioInfo::EAudioIOState::AUDIO_IO_STATE_NONE;
@@ -45,9 +41,6 @@ CAudioIO::CAudioIO() :
 
 CAudioIO::CAudioIO(CAudioInfo& audioInfo) :
     mpPulseAudioClient(nullptr),
-    __mMutex(PTHREAD_MUTEX_INITIALIZER),
-    __mCondMutex(PTHREAD_MUTEX_INITIALIZER),
-    __mCond(PTHREAD_COND_INITIALIZER),
     __mIsInit(false) {
     mAudioInfo = audioInfo;
     mDirection = CAudioInfo::EAudioDirection::AUDIO_DIRECTION_MAX;
@@ -69,89 +62,11 @@ bool CAudioIO::IsReady() {
              mState == CAudioInfo::EAudioIOState::AUDIO_IO_STATE_PAUSED));
 }
 
-void CAudioIO::internalLock() {
-    if (!__mIsInit)
-        THROW_ERROR_MSG(CAudioError::EError::ERROR_NOT_INITIALIZED, "Doesn't initialize CAudioIO"); //LCOV_EXCL_LINE
-
-    if (pthread_mutex_lock(&__mMutex) != 0)
-        THROW_ERROR_MSG(CAudioError::EError::ERROR_INTERNAL_OPERATION, "Failed pthread_mutex_lock()"); //LCOV_EXCL_LINE
-
-#ifdef _AUDIO_IO_DEBUG_TIMING_
-    AUDIO_IO_LOGD(COLOR_RED "%p LOCKED" COLOR_END, &__mMutex);
-#endif
-}
-
-void CAudioIO::internalUnlock() {
-    if (!__mIsInit)
-        THROW_ERROR_MSG(CAudioError::EError::ERROR_NOT_INITIALIZED, "Doesn't initialize CAudioIO"); //LCOV_EXCL_LINE
-
-    if (pthread_mutex_unlock(&__mMutex) != 0)
-        THROW_ERROR_MSG(CAudioError::EError::ERROR_INTERNAL_OPERATION, "Failed pthread_mutex_lock()"); //LCOV_EXCL_LINE
-
-#ifdef _AUDIO_IO_DEBUG_TIMING_
-    AUDIO_IO_LOGD(COLOR_GREEN "%p UNLOCKED" COLOR_END, &__mMutex);
-#endif
-}
-
-void CAudioIO::internalWait() {
-    if (!__mIsInit)
-        THROW_ERROR_MSG(CAudioError::EError::ERROR_NOT_INITIALIZED, "Doesn't initialize CAudioIO"); //LCOV_EXCL_LINE
-
-#ifdef _AUDIO_IO_DEBUG_TIMING_
-    AUDIO_IO_LOGD(COLOR_RED "WAIT" COLOR_END);
-#endif
-    pthread_mutex_lock(&__mCondMutex);
-
-    struct timeval now = { 0, };
-    struct timeval to_wait = { 0, };
-    struct timeval until = { 0, };
-    struct timespec until_ts = { 0, };
-
-    constexpr int COND_TIMEOUT_MS = 200;
-
-    gettimeofday(&now, nullptr);
-    to_wait.tv_sec = COND_TIMEOUT_MS / 1000UL;
-    to_wait.tv_usec = (COND_TIMEOUT_MS % 1000UL) * 1000UL;
-    timeradd(&now, &to_wait, &until);
-    until_ts.tv_sec = until.tv_sec;
-    until_ts.tv_nsec = until.tv_usec * 1000UL;
-
-    int ret = pthread_cond_timedwait(&__mCond, &__mCondMutex, &until_ts);
-    if (ret != 0) {
-        char str_error[256];
-        AUDIO_IO_LOGE("pthread_cond_timedwait error=[%d][%s]", ret, strerror_r(ret, str_error, sizeof(str_error)));
-    }
-
-    pthread_mutex_unlock(&__mCondMutex);
-}
-
-void CAudioIO::internalSignal() {
-    if (!__mIsInit)
-        THROW_ERROR_MSG(CAudioError::EError::ERROR_NOT_INITIALIZED, "Doesn't initialize CAudioIO"); //LCOV_EXCL_LINE
-
-#ifdef _AUDIO_IO_DEBUG_TIMING_
-    AUDIO_IO_LOGD(COLOR_GREEN "SIGNAL" COLOR_END);
-#endif
-
-    pthread_mutex_lock(&__mCondMutex);
-    pthread_cond_signal(&__mCond);
-    pthread_mutex_unlock(&__mCondMutex);
-}
-
 void CAudioIO::initialize() {
     if (__mIsInit)
         return;
 
     AUDIO_IO_LOGD("initialize");
-
-    int ret = pthread_mutex_init(&__mMutex, NULL);
-    if (ret != 0)
-        THROW_ERROR_MSG(CAudioError::EError::ERROR_OUT_OF_MEMORY, "Failed pthread_mutex_init()"); //LCOV_EXCL_LINE
-
-    ret = pthread_cond_init(&__mCond, NULL);
-    if (ret != 0)
-        THROW_ERROR_MSG(CAudioError::EError::ERROR_OUT_OF_MEMORY, "Failed pthread_cond_init()"); //LCOV_EXCL_LINE
-
     __mIsInit = true;
 }
 
@@ -160,35 +75,6 @@ void CAudioIO::finalize() {
         return;
 
     AUDIO_IO_LOGD("finalize");
-
-    bool error_occured = false;
-    int ret = pthread_mutex_destroy(&__mMutex);
-    if (ret != 0) {
-//LCOV_EXCL_START
-        AUDIO_IO_LOGE("Failed pthread_mutex_destroy(%p) errno:%d", &__mMutex, ret);
-        error_occured = true;
-//LCOV_EXCL_STOP
-    }
-
-    ret = pthread_mutex_destroy(&__mCondMutex);
-    if (ret != 0) {
-//LCOV_EXCL_START
-        AUDIO_IO_LOGE("Failed cond pthread_mutex_destroy(%p) errno:%d", &__mCondMutex, ret);
-        error_occured = true;
-//LCOV_EXCL_STOP
-    }
-
-    ret = pthread_cond_destroy(&__mCond);
-    if (ret != 0) {
-//LCOV_EXCL_START
-        AUDIO_IO_LOGE("Failed pthread_cond_destroy(%p) errno:%d", &__mCond, ret);
-        error_occured = true;
-//LCOV_EXCL_STOP
-    }
-
-    if (error_occured)
-        THROW_ERROR_MSG_FORMAT(CAudioError::EError::ERROR_INTERNAL_OPERATION, "Finalize Failed"); //LCOV_EXCL_LINE
-
     __mIsInit = false;
 }
 
@@ -251,48 +137,27 @@ void CAudioIO::pause() {
     if (!__mIsInit || !IsReady())
         THROW_ERROR_MSG(CAudioError::EError::ERROR_NOT_INITIALIZED, "Did not initialize or prepare CAudioIO"); //LCOV_EXCL_LINE
 
-    try {
-        internalLock();
-        AUDIO_IO_LOGD("pause");
-        mpPulseAudioClient->cork(true);
-        internalUnlock();
-    } catch (const CAudioError& e) {
-        internalUnlock();
-        throw;
-    }
+    std::lock_guard<std::mutex> guard(mMutex);
+    AUDIO_IO_LOGD("pause");
+    mpPulseAudioClient->cork(true);
 }
 
 void CAudioIO::resume() {
     if (!__mIsInit || !IsReady())
         THROW_ERROR_MSG(CAudioError::EError::ERROR_NOT_INITIALIZED, "Did not initialize or prepare CAudioIO"); //LCOV_EXCL_LINE
 
-    try {
-        internalLock();
-        AUDIO_IO_LOGD("resume");
-        mpPulseAudioClient->cork(false);
-        internalUnlock();
-    } catch (const CAudioError& e) {
-        internalUnlock();
-        throw;
-    }
+    std::lock_guard<std::mutex> guard(mMutex);
+    mpPulseAudioClient->cork(false);
 }
+
 void CAudioIO::flush() {
     if (!__mIsInit || !IsReady())
         THROW_ERROR_MSG(CAudioError::EError::ERROR_NOT_INITIALIZED, "Did not initialize or prepare CAudioIO"); //LCOV_EXCL_LINE
 
-    try {
-        if (mpPulseAudioClient->isInThread()) {
-            mpPulseAudioClient->flush();
-        } else {
-            internalLock();
-            mpPulseAudioClient->flush();
-            internalUnlock();
-        }
-    } catch (const CAudioError& e) {
-        if (!mpPulseAudioClient->isInThread())
-            internalUnlock();
-        throw;
-    }
+    std::unique_lock<std::mutex> defer_mutex(mMutex, std::defer_lock);
+    if (!mpPulseAudioClient->isInThread())
+        defer_mutex.lock();
+    mpPulseAudioClient->flush();
 }
 
 CAudioInfo& CAudioIO::getAudioInfo() {
index 69cbd67..ee5f3b6 100644 (file)
@@ -102,7 +102,6 @@ void CAudioInput::finalize() {
     }
 
     CAudioIO::finalize();
-
     __setInit(false);
     __mVolume = 1.0;
 }
@@ -134,7 +133,7 @@ void CAudioInput::prepare() {
 
         CPulseStreamSpec spec(streamSpec, mAudioInfo);
 
-        internalLock();
+        std::unique_lock<std::mutex> mutex(mMutex);
         mpPulseAudioClient = new CPulseAudioClient(CPulseAudioClient::EStreamDirection::STREAM_DIRECTION_RECORD, spec, this);
         mpPulseAudioClient->initialize();
         mpPulseAudioClient->applyRecordVolume(__mVolume);
@@ -142,21 +141,17 @@ void CAudioInput::prepare() {
         /* Uncork stream which is created with CORKED flag */
         mpPulseAudioClient->cork(false);
 #endif
-        internalUnlock();
+        mutex.unlock();
 
         CAudioIO::prepare();
     } catch (const CAudioError& e) {
 //LCOV_EXCL_START
         SAFE_FINALIZE(mpPulseAudioClient);
         SAFE_DELETE(mpPulseAudioClient);
-        internalUnlock();
         throw;
 //LCOV_EXCL_STOP
     } catch (const std::bad_alloc&) {
-//LCOV_EXCL_START
-        internalUnlock();
         THROW_ERROR_MSG(CAudioError::EError::ERROR_OUT_OF_MEMORY, "Failed to allocate CPulseAudioClient object");
-//LCOV_EXCL_STOP
     }
 }
 
@@ -172,19 +167,12 @@ void CAudioInput::unprepare() {
 
     CAudioIO::unprepare();
 
-    try {
-        internalLock();
-        if (mpPulseAudioClient && mpPulseAudioClient->isInThread())
-            THROW_ERROR_MSG(CAudioError::EError::ERROR_INVALID_OPERATION, "Can't unprepare inside pulseaudio thread");
-        SAFE_FINALIZE(mpPulseAudioClient);
-        SAFE_DELETE(mpPulseAudioClient);
-        internalUnlock();
-    } catch (const CAudioError& e) {
-//LCOV_EXCL_START
-        internalUnlock();
-        throw;
-//LCOV_EXCL_STOP
-    }
+    std::unique_lock<std::mutex> mutex(mMutex);
+    if (mpPulseAudioClient && mpPulseAudioClient->isInThread())
+        THROW_ERROR_MSG(CAudioError::EError::ERROR_INVALID_OPERATION, "Can't unprepare inside pulseaudio thread");
+    SAFE_FINALIZE(mpPulseAudioClient);
+    SAFE_DELETE(mpPulseAudioClient);
+    mutex.unlock();
 
     CAudioIO::onStateChanged(CAudioInfo::EAudioIOState::AUDIO_IO_STATE_IDLE);
 }
@@ -268,25 +256,17 @@ size_t CAudioInput::read(void* buffer, size_t length) {
 
     int ret = 0;
 
-    try {
-        internalLock();
-
-        // If another thread did call unprepare, do not read
-        if (!mpPulseAudioClient)
-            THROW_ERROR_MSG(CAudioError::EError::ERROR_NOT_INITIALIZED, //LCOV_EXCL_LINE
-                            "Did not initialize CPulseAudioClient");    //LCOV_EXCL_LINE
+    std::unique_lock<std::mutex> mutex(mMutex);
+    // If another thread did call unprepare, do not read
+    if (!mpPulseAudioClient)
+        THROW_ERROR_MSG(CAudioError::EError::ERROR_NOT_INITIALIZED, //LCOV_EXCL_LINE
+                        "Did not initialize CPulseAudioClient");    //LCOV_EXCL_LINE
 
-        // Block until read done
-        ret = mpPulseAudioClient->read(buffer, length);
-        internalUnlock();
+    // Block until read done
+    ret = mpPulseAudioClient->read(buffer, length);
+    mutex.unlock();
 
-        sched_yield();
-    } catch (const CAudioError& e) {
-//LCOV_EXCL_START
-        internalUnlock();
-        throw;
-//LCOV_EXCL_STOP
-    }
+    sched_yield();
 
     return ret;
 }
@@ -325,21 +305,12 @@ void CAudioInput::setVolume(double volume) {
     if (!__IsInit())
         THROW_ERROR_MSG(CAudioError::EError::ERROR_NOT_INITIALIZED, "Not initialized"); //LCOV_EXCL_LINE
 
-    AUDIO_IO_LOGE("%d, %p", __IsInit(), mpPulseAudioClient);
-    try {
-        if (__IsReady()) {
-            if (mpPulseAudioClient->isInThread()) {
-                mpPulseAudioClient->applyRecordVolume(volume);
-            } else {
-                internalLock();
-                mpPulseAudioClient->applyRecordVolume(volume);
-                internalUnlock();
-            }
-        }
-    } catch (const CAudioError& e) {
+    if (__IsReady()) {
+        std::unique_lock<std::mutex> defer_mutex(mMutex, std::defer_lock);
         if (!mpPulseAudioClient->isInThread())
-            internalUnlock();
-        throw;
+            defer_mutex.lock();
+
+        mpPulseAudioClient->applyRecordVolume(volume);
     }
 
     __mVolume = volume;
index 3f7b79a..a3ca983 100644 (file)
 
 #include "CAudioIODef.h"
 #include <sched.h>
+#include <chrono>
 
 using namespace std;
 using namespace tizen_media_audio;
 
+static constexpr auto cond_wait_ms = 200ms;
+
 /**
  * class CAudioOutput
  */
@@ -45,7 +48,10 @@ void CAudioOutput::onStream(CPulseAudioClient* pClient, size_t length) {
 #ifdef _AUDIO_IO_DEBUG_TIMING_
         AUDIO_IO_LOGD("Sync Write Mode! - signal! - pClient:[%p], length:[%zu]", pClient, length);
 #endif
-        internalSignal();
+        {
+            std::lock_guard<std::mutex> cond_guard(mCondMutex);
+        }
+        mCond.notify_one();
         return;
     }
 
@@ -94,7 +100,6 @@ void CAudioOutput::finalize() {
     }
 
     CAudioIO::finalize();
-
     __setInit(false);
 }
 
@@ -137,28 +142,25 @@ void CAudioOutput::prepare() {
 
         CPulseStreamSpec spec(streamSpec, mAudioInfo);
 
-        internalLock();
-        mpPulseAudioClient = new CPulseAudioClient(CPulseAudioClient::EStreamDirection::STREAM_DIRECTION_PLAYBACK, spec, this);
+        std::unique_lock<std::mutex> mutex(mMutex);
+        mpPulseAudioClient = new CPulseAudioClient(CPulseAudioClient::EStreamDirection::STREAM_DIRECTION_PLAYBACK,
+                                                   spec, this);
         mpPulseAudioClient->initialize();
 #ifndef DISABLE_MOBILE_BACK_COMP
         /* Uncork stream which is created with CORKED flag */
         mpPulseAudioClient->cork(false);
 #endif
-        internalUnlock();
+        mutex.unlock();
 
         CAudioIO::prepare();
     } catch (const CAudioError& e) {
 //LCOV_EXCL_START
         SAFE_FINALIZE(mpPulseAudioClient);
         SAFE_DELETE(mpPulseAudioClient);
-        internalUnlock();
         throw;
 //LCOV_EXCL_STOP
     } catch (const std::bad_alloc&) {
-//LCOV_EXCL_START
-        internalUnlock();
         THROW_ERROR_MSG(CAudioError::EError::ERROR_OUT_OF_MEMORY, "Failed to allocate CPulseAudioClient object");
-//LCOV_EXCL_STOP
     }
 }
 
@@ -174,19 +176,12 @@ void CAudioOutput::unprepare() {
 
     CAudioIO::unprepare();
 
-    try {
-        internalLock();
-        if (mpPulseAudioClient && mpPulseAudioClient->isInThread())
-            THROW_ERROR_MSG(CAudioError::EError::ERROR_INVALID_OPERATION, "Can't unprepare inside pulseaudio thread");
-        SAFE_FINALIZE(mpPulseAudioClient);
-        SAFE_DELETE(mpPulseAudioClient);
-        internalUnlock();
-    } catch (const CAudioError& e) {
-//LCOV_EXCL_START
-        internalUnlock();
-        throw;
-//LCOV_EXCL_STOP
-    }
+    std::unique_lock<std::mutex> mutex(mMutex);
+    if (mpPulseAudioClient && mpPulseAudioClient->isInThread())
+        THROW_ERROR_MSG(CAudioError::EError::ERROR_INVALID_OPERATION, "Can't unprepare inside pulseaudio thread");
+    SAFE_FINALIZE(mpPulseAudioClient);
+    SAFE_DELETE(mpPulseAudioClient);
+    mutex.unlock();
 
     CAudioIO::onStateChanged(CAudioInfo::EAudioIOState::AUDIO_IO_STATE_IDLE);
 }
@@ -231,19 +226,11 @@ void CAudioOutput::drain() {
     if (mStreamCallback.onStream)
         THROW_ERROR_MSG(CAudioError::EError::ERROR_INVALID_OPERATION, "async type don't support drain");
 
-    try {
-        if (mpPulseAudioClient->isInThread()) {
-            mpPulseAudioClient->drain();
-        } else {
-            internalLock();
-            mpPulseAudioClient->drain();
-            internalUnlock();
-        }
-    } catch (const CAudioError& e) {
-        if (!mpPulseAudioClient->isInThread())
-            internalUnlock();
-        throw;
-    }
+    std::unique_lock<std::mutex> defer_mutex(mMutex, std::defer_lock);
+    if (!mpPulseAudioClient->isInThread())
+        defer_mutex.lock();
+
+    mpPulseAudioClient->drain();
 }
 
 void CAudioOutput::flush() {
@@ -290,7 +277,7 @@ size_t CAudioOutput::write(const void* buffer, size_t length) {
 
     try {
         /* For synchronization */
-        internalLock();
+        std::unique_lock<std::mutex> mutex(mMutex);
 
         // If another thread did call unprepare, do not write
         if (!mpPulseAudioClient)
@@ -299,7 +286,6 @@ size_t CAudioOutput::write(const void* buffer, size_t length) {
 
         // Sets synchronous flag
         __mIsUsedSyncWrite = true;
-
         size_t lengthIter = length;
 
         while (lengthIter > 0) {
@@ -309,7 +295,8 @@ size_t CAudioOutput::write(const void* buffer, size_t length) {
 #ifdef _AUDIO_IO_DEBUG_TIMING_
                 AUDIO_IO_LOGD("writableSize is [%zu].. wait", l);
 #endif
-                internalWait();
+                std::unique_lock<std::mutex> cond_mutex(mCondMutex);
+                mCond.wait_for(cond_mutex, cond_wait_ms);
             }
 
             if (l > lengthIter)
@@ -329,12 +316,11 @@ size_t CAudioOutput::write(const void* buffer, size_t length) {
         }  // End of while (length > 0)
 
         __mIsUsedSyncWrite = false;
-        internalUnlock();
+        mutex.unlock();
 
         sched_yield();
     } catch (const CAudioError& e) {
         __mIsUsedSyncWrite = false;
-        internalUnlock();
         throw;
     }