From 5a6cd224d07c05b496b6aca050ce5ecf96f125af Mon Sep 17 00:00:00 2001 From: Glenn Kasten Date: Fri, 20 Sep 2013 09:20:45 -0700 Subject: Fix slow AudioTrack and AudioRecord destruction There were two causes for the slowness: When thread was paused, it used nanosleep and sleep. These usually run to completion (except for POSIX signal, which we avoid because it is low-level). Instead, replace the nanosleep and sleep by condition timed wait, as that can be made to return early by a condition signal. Another advantage of condition timed wait is that a condition wait was already being used at top of thread loop, so it is a simpler change. The AudioRecord destructor was missing a proxy interrupt that was correct in AudioTrack. This proxy interrupt is needed in case another thread is blocked in proxy obtainBuffer. Does not address the 1 second polling for NS_WHENEVER. Bug: 10822765 Change-Id: Id665994551e87e4d7da9c7b015f424fd7a0b5560 --- media/libmedia/AudioRecord.cpp | 54 +++++++++++++++++++++++------------------- media/libmedia/AudioTrack.cpp | 53 ++++++++++++++++++++++------------------- 2 files changed, 59 insertions(+), 48 deletions(-) (limited to 'media') diff --git a/media/libmedia/AudioRecord.cpp b/media/libmedia/AudioRecord.cpp index e934a3e..fb731b9 100644 --- a/media/libmedia/AudioRecord.cpp +++ b/media/libmedia/AudioRecord.cpp @@ -105,6 +105,7 @@ AudioRecord::~AudioRecord() // Otherwise the callback thread will never exit. stop(); if (mAudioRecordThread != 0) { + mProxy->interrupt(); mAudioRecordThread->requestExit(); // see comment in AudioRecord.h mAudioRecordThread->requestExitAndWait(); mAudioRecordThread.clear(); @@ -960,7 +961,7 @@ void AudioRecord::DeathNotifier::binderDied(const wp& who) // ========================================================================= AudioRecord::AudioRecordThread::AudioRecordThread(AudioRecord& receiver, bool bCanCallJava) - : Thread(bCanCallJava), mReceiver(receiver), mPaused(true), mResumeLatch(false) + : Thread(bCanCallJava), mReceiver(receiver), mPaused(true), mPausedInt(false), mPausedNs(0LL) { } @@ -977,25 +978,32 @@ bool AudioRecord::AudioRecordThread::threadLoop() // caller will check for exitPending() return true; } + if (mPausedInt) { + mPausedInt = false; + if (mPausedNs > 0) { + (void) mMyCond.waitRelative(mMyLock, mPausedNs); + } else { + mMyCond.wait(mMyLock); + } + return true; + } } nsecs_t ns = mReceiver.processAudioBuffer(this); switch (ns) { case 0: return true; - case NS_WHENEVER: - sleep(1); - return true; case NS_INACTIVE: - pauseConditional(); + pauseInternal(); return true; case NS_NEVER: return false; + case NS_WHENEVER: + // FIXME increase poll interval, or make event-driven + ns = 1000000000LL; + // fall through default: LOG_ALWAYS_FATAL_IF(ns < 0, "processAudioBuffer() returned %lld", ns); - struct timespec req; - req.tv_sec = ns / 1000000000LL; - req.tv_nsec = ns % 1000000000LL; - nanosleep(&req, NULL /*rem*/); + pauseInternal(ns); return true; } } @@ -1004,24 +1012,18 @@ void AudioRecord::AudioRecordThread::requestExit() { // must be in this order to avoid a race condition Thread::requestExit(); - resume(); + AutoMutex _l(mMyLock); + if (mPaused || mPausedInt) { + mPaused = false; + mPausedInt = false; + mMyCond.signal(); + } } void AudioRecord::AudioRecordThread::pause() { AutoMutex _l(mMyLock); mPaused = true; - mResumeLatch = false; -} - -void AudioRecord::AudioRecordThread::pauseConditional() -{ - AutoMutex _l(mMyLock); - if (mResumeLatch) { - mResumeLatch = false; - } else { - mPaused = true; - } } void AudioRecord::AudioRecordThread::resume() @@ -1029,13 +1031,17 @@ void AudioRecord::AudioRecordThread::resume() AutoMutex _l(mMyLock); if (mPaused) { mPaused = false; - mResumeLatch = false; mMyCond.signal(); - } else { - mResumeLatch = true; } } +void AudioRecord::AudioRecordThread::pauseInternal(nsecs_t ns) +{ + AutoMutex _l(mMyLock); + mPausedInt = true; + mPausedNs = ns; +} + // ------------------------------------------------------------------------- }; // namespace android diff --git a/media/libmedia/AudioTrack.cpp b/media/libmedia/AudioTrack.cpp index 15249a4..fdcf911 100644 --- a/media/libmedia/AudioTrack.cpp +++ b/media/libmedia/AudioTrack.cpp @@ -1782,7 +1782,7 @@ void AudioTrack::DeathNotifier::binderDied(const wp& who) // ========================================================================= AudioTrack::AudioTrackThread::AudioTrackThread(AudioTrack& receiver, bool bCanCallJava) - : Thread(bCanCallJava), mReceiver(receiver), mPaused(true), mResumeLatch(false) + : Thread(bCanCallJava), mReceiver(receiver), mPaused(true), mPausedInt(false), mPausedNs(0LL) { } @@ -1799,25 +1799,32 @@ bool AudioTrack::AudioTrackThread::threadLoop() // caller will check for exitPending() return true; } + if (mPausedInt) { + mPausedInt = false; + if (mPausedNs > 0) { + (void) mMyCond.waitRelative(mMyLock, mPausedNs); + } else { + mMyCond.wait(mMyLock); + } + return true; + } } nsecs_t ns = mReceiver.processAudioBuffer(this); switch (ns) { case 0: return true; - case NS_WHENEVER: - sleep(1); - return true; case NS_INACTIVE: - pauseConditional(); + pauseInternal(); return true; case NS_NEVER: return false; + case NS_WHENEVER: + // FIXME increase poll interval, or make event-driven + ns = 1000000000LL; + // fall through default: LOG_ALWAYS_FATAL_IF(ns < 0, "processAudioBuffer() returned %lld", ns); - struct timespec req; - req.tv_sec = ns / 1000000000LL; - req.tv_nsec = ns % 1000000000LL; - nanosleep(&req, NULL /*rem*/); + pauseInternal(ns); return true; } } @@ -1826,24 +1833,18 @@ void AudioTrack::AudioTrackThread::requestExit() { // must be in this order to avoid a race condition Thread::requestExit(); - resume(); + AutoMutex _l(mMyLock); + if (mPaused || mPausedInt) { + mPaused = false; + mPausedInt = false; + mMyCond.signal(); + } } void AudioTrack::AudioTrackThread::pause() { AutoMutex _l(mMyLock); mPaused = true; - mResumeLatch = false; -} - -void AudioTrack::AudioTrackThread::pauseConditional() -{ - AutoMutex _l(mMyLock); - if (mResumeLatch) { - mResumeLatch = false; - } else { - mPaused = true; - } } void AudioTrack::AudioTrackThread::resume() @@ -1851,11 +1852,15 @@ void AudioTrack::AudioTrackThread::resume() AutoMutex _l(mMyLock); if (mPaused) { mPaused = false; - mResumeLatch = false; mMyCond.signal(); - } else { - mResumeLatch = true; } } +void AudioTrack::AudioTrackThread::pauseInternal(nsecs_t ns) +{ + AutoMutex _l(mMyLock); + mPausedInt = true; + mPausedNs = ns; +} + }; // namespace android -- cgit v1.1