AudioRecord client threading cleanup
Rename ClientRecordThread to AudioRecordThread to be more similar to
AudioTrack naming.
Only create the thread once, and use resume() and pause() for start()
and stop(). This will allow us to have a known client callback thread
tid that we can pass to AudioFlinger before start().
mActive:
Made mActive a bool not int.
mActive is protected by mLock; volatile is meaningless.
Fixed a few places where mActive was accessed without a lock:
- stopped()
- processAudioBuffer()
These aren't used internally, so no need for _l() versions.
Change-Id: I4b8a5c90f3a22d3894b344564cb1c5aef4f1fda2
diff --git a/include/media/AudioRecord.h b/include/media/AudioRecord.h
index 01ab8c7..f7ebd39 100644
--- a/include/media/AudioRecord.h
+++ b/include/media/AudioRecord.h
@@ -316,18 +316,31 @@
AudioRecord& operator = (const AudioRecord& other);
/* a small internal class to handle the callback */
- class ClientRecordThread : public Thread
+ class AudioRecordThread : public Thread
{
public:
- ClientRecordThread(AudioRecord& receiver, bool bCanCallJava = false);
+ AudioRecordThread(AudioRecord& receiver, bool bCanCallJava = false);
+
+ // Do not call Thread::requestExitAndWait() without first calling requestExit().
+ // Thread::requestExitAndWait() is not virtual, and the implementation doesn't do enough.
+ virtual void requestExit();
+
+ void pause(); // suspend thread from execution at next loop boundary
+ void resume(); // allow thread to execute, if not requested to exit
+
private:
friend class AudioRecord;
virtual bool threadLoop();
- virtual status_t readyToRun();
AudioRecord& mReceiver;
+ virtual ~AudioRecordThread();
+ Mutex mMyLock; // Thread::mLock is private
+ Condition mMyCond; // Thread::mThreadExitedCondition is private
+ bool mPaused; // whether thread is currently paused
};
- bool processAudioBuffer(const sp<ClientRecordThread>& thread);
+ // body of AudioRecordThread::threadLoop()
+ bool processAudioBuffer(const sp<AudioRecordThread>& thread);
+
status_t openRecord_l(uint32_t sampleRate,
audio_format_t format,
audio_channel_mask_t channelMask,
@@ -336,12 +349,10 @@
audio_io_handle_t getInput_l();
status_t restoreRecord_l(audio_track_cblk_t*& cblk);
- sp<ClientRecordThread> mClientRecordThread;
- status_t mReadyToRun;
+ sp<AudioRecordThread> mAudioRecordThread;
mutable Mutex mLock;
- Condition mCondition;
- volatile int32_t mActive;
+ bool mActive; // protected by mLock
// for client callback handler
callback_t mCbf;
diff --git a/media/libmedia/AudioRecord.cpp b/media/libmedia/AudioRecord.cpp
index 13e60a7..807f812 100644
--- a/media/libmedia/AudioRecord.cpp
+++ b/media/libmedia/AudioRecord.cpp
@@ -103,9 +103,10 @@
// it is looping on buffer empty condition in obtainBuffer().
// Otherwise the callback thread will never exit.
stop();
- if (mClientRecordThread != 0) {
- mClientRecordThread->requestExitAndWait();
- mClientRecordThread.clear();
+ if (mAudioRecordThread != 0) {
+ mAudioRecordThread->requestExit(); // see comment in AudioRecord.h
+ mAudioRecordThread->requestExitAndWait();
+ mAudioRecordThread.clear();
}
mAudioRecord.clear();
IPCThreadState::self()->flushCommands();
@@ -200,7 +201,8 @@
}
if (cbf != NULL) {
- mClientRecordThread = new ClientRecordThread(*this, threadCanCallJava);
+ mAudioRecordThread = new AudioRecordThread(*this, threadCanCallJava);
+ mAudioRecordThread->run("AudioRecord", ANDROID_PRIORITY_AUDIO);
}
mStatus = NO_ERROR;
@@ -210,7 +212,7 @@
mFrameCount = mCblk->frameCount;
mChannelCount = (uint8_t)channelCount;
mChannelMask = channelMask;
- mActive = 0;
+ mActive = false;
mCbf = cbf;
mNotificationFrames = notificationFrames;
mRemainingFrames = notificationFrames;
@@ -274,41 +276,19 @@
status_t AudioRecord::start(AudioSystem::sync_event_t event, int triggerSession)
{
status_t ret = NO_ERROR;
- sp<ClientRecordThread> t = mClientRecordThread;
+ sp<AudioRecordThread> t = mAudioRecordThread;
ALOGV("start, sync event %d trigger session %d", event, triggerSession);
- if (t != 0) {
- if (t->exitPending()) {
- if (t->requestExitAndWait() == WOULD_BLOCK) {
- ALOGE("AudioRecord::start called from thread");
- return WOULD_BLOCK;
- }
- }
- }
-
AutoMutex lock(mLock);
// acquire a strong reference on the IAudioRecord and IMemory so that they cannot be destroyed
// while we are accessing the cblk
sp<IAudioRecord> audioRecord = mAudioRecord;
sp<IMemory> iMem = mCblkMemory;
audio_track_cblk_t* cblk = mCblk;
- if (mActive == 0) {
- mActive = 1;
- pid_t tid;
- if (t != 0) {
- mReadyToRun = WOULD_BLOCK;
- t->run("AudioRecord", ANDROID_PRIORITY_AUDIO);
- tid = t->getTid(); // pid_t is unknown until run()
- ALOGV("getTid=%d", tid);
- if (tid == -1) {
- tid = 0;
- }
- // thread blocks in readyToRun()
- } else {
- tid = 0; // not gettid()
- }
+ if (!mActive) {
+ mActive = true;
cblk->lock.lock();
if (!(cblk->flags & CBLK_INVALID_MSK)) {
@@ -330,19 +310,14 @@
AudioSystem::kSyncRecordStartTimeOutMs;
cblk->waitTimeMs = 0;
if (t != 0) {
- // thread unblocks in readyToRun() and returns NO_ERROR
- mReadyToRun = NO_ERROR;
- mCondition.signal();
+ t->resume();
} else {
mPreviousPriority = getpriority(PRIO_PROCESS, 0);
get_sched_policy(0, &mPreviousSchedulingGroup);
androidSetThreadPriority(0, ANDROID_PRIORITY_AUDIO);
}
} else {
- mActive = 0;
- // thread unblocks in readyToRun() and returns NO_INIT
- mReadyToRun = NO_INIT;
- mCondition.signal();
+ mActive = false;
}
}
@@ -351,20 +326,20 @@
status_t AudioRecord::stop()
{
- sp<ClientRecordThread> t = mClientRecordThread;
+ sp<AudioRecordThread> t = mAudioRecordThread;
ALOGV("stop");
AutoMutex lock(mLock);
- if (mActive == 1) {
- mActive = 0;
+ if (mActive) {
+ mActive = false;
mCblk->cv.signal();
mAudioRecord->stop();
// the record head position will reset to 0, so if a marker is set, we need
// to activate it again
mMarkerReached = false;
if (t != 0) {
- t->requestExit();
+ t->pause();
} else {
setpriority(PRIO_PROCESS, 0, mPreviousPriority);
set_sched_policy(0, mPreviousSchedulingGroup);
@@ -376,6 +351,7 @@
bool AudioRecord::stopped() const
{
+ AutoMutex lock(mLock);
return !mActive;
}
@@ -493,7 +469,7 @@
status_t AudioRecord::obtainBuffer(Buffer* audioBuffer, int32_t waitCount)
{
AutoMutex lock(mLock);
- int active;
+ bool active;
status_t result = NO_ERROR;
audio_track_cblk_t* cblk = mCblk;
uint32_t framesReq = audioBuffer->frameCount;
@@ -522,7 +498,7 @@
result = cblk->cv.waitRelative(cblk->lock, milliseconds(waitTimeMs));
cblk->lock.unlock();
mLock.lock();
- if (mActive == 0) {
+ if (!mActive) {
return status_t(STOPPED);
}
cblk->lock.lock();
@@ -671,7 +647,7 @@
// -------------------------------------------------------------------------
-bool AudioRecord::processAudioBuffer(const sp<ClientRecordThread>& thread)
+bool AudioRecord::processAudioBuffer(const sp<AudioRecordThread>& thread)
{
Buffer audioBuffer;
uint32_t frames = mRemainingFrames;
@@ -683,6 +659,7 @@
sp<IAudioRecord> audioRecord = mAudioRecord;
sp<IMemory> iMem = mCblkMemory;
audio_track_cblk_t* cblk = mCblk;
+ bool active = mActive;
mLock.unlock();
// Manage marker callback
@@ -741,7 +718,9 @@
// Manage overrun callback
- if (mActive && (cblk->framesAvailable() == 0)) {
+ if (active && (cblk->framesAvailable() == 0)) {
+ // The value of active is stale, but we are almost sure to be active here because
+ // otherwise we would have exited when obtainBuffer returned STOPPED earlier.
ALOGV("Overrun user: %x, server: %x, flags %04x", cblk->user, cblk->server, cblk->flags);
if (!(android_atomic_or(CBLK_UNDERRUN_ON, &cblk->flags) & CBLK_UNDERRUN_MSK)) {
mCbf(EVENT_OVERRUN, mUserData, 0);
@@ -798,7 +777,7 @@
result = NO_ERROR;
cblk->lock.unlock();
}
- if (result != NO_ERROR || mActive == 0) {
+ if (result != NO_ERROR || !mActive) {
result = status_t(STOPPED);
}
}
@@ -818,23 +797,51 @@
// =========================================================================
-AudioRecord::ClientRecordThread::ClientRecordThread(AudioRecord& receiver, bool bCanCallJava)
- : Thread(bCanCallJava), mReceiver(receiver)
+AudioRecord::AudioRecordThread::AudioRecordThread(AudioRecord& receiver, bool bCanCallJava)
+ : Thread(bCanCallJava), mReceiver(receiver), mPaused(true)
{
}
-bool AudioRecord::ClientRecordThread::threadLoop()
+AudioRecord::AudioRecordThread::~AudioRecordThread()
{
- return mReceiver.processAudioBuffer(this);
}
-status_t AudioRecord::ClientRecordThread::readyToRun()
+bool AudioRecord::AudioRecordThread::threadLoop()
{
- AutoMutex(mReceiver.mLock);
- while (mReceiver.mReadyToRun == WOULD_BLOCK) {
- mReceiver.mCondition.wait(mReceiver.mLock);
+ {
+ AutoMutex _l(mMyLock);
+ if (mPaused) {
+ mMyCond.wait(mMyLock);
+ // caller will check for exitPending()
+ return true;
+ }
}
- return mReceiver.mReadyToRun;
+ if (!mReceiver.processAudioBuffer(this)) {
+ pause();
+ }
+ return true;
+}
+
+void AudioRecord::AudioRecordThread::requestExit()
+{
+ // must be in this order to avoid a race condition
+ Thread::requestExit();
+ resume();
+}
+
+void AudioRecord::AudioRecordThread::pause()
+{
+ AutoMutex _l(mMyLock);
+ mPaused = true;
+}
+
+void AudioRecord::AudioRecordThread::resume()
+{
+ AutoMutex _l(mMyLock);
+ if (mPaused) {
+ mPaused = false;
+ mMyCond.signal();
+ }
}
// -------------------------------------------------------------------------