diff options
author | Glenn Kasten <gkasten@google.com> | 2012-07-09 14:31:33 -0700 |
---|---|---|
committer | Glenn Kasten <gkasten@google.com> | 2012-07-13 16:35:52 -0700 |
commit | 1ea6d23396118a9cfe912b7b8a4e6f231e318ea2 (patch) | |
tree | 31327ae0fe7ad1e34a3766bf6246c26752704ce7 /services | |
parent | 68337edf595a0c345ba4b8adcd4f1e541a1d7eb7 (diff) | |
download | frameworks_av-1ea6d23396118a9cfe912b7b8a4e6f231e318ea2.zip frameworks_av-1ea6d23396118a9cfe912b7b8a4e6f231e318ea2.tar.gz frameworks_av-1ea6d23396118a9cfe912b7b8a4e6f231e318ea2.tar.bz2 |
Use atomic ops for thread suspend count
There was a theoretical but unlikely race if two binder threads
executed suspend() or restore() concurrently. Also added comments.
Change-Id: I0908acc810b83bdd66455b27ca3429de1662a2cd
Diffstat (limited to 'services')
-rw-r--r-- | services/audioflinger/AudioFlinger.cpp | 6 | ||||
-rw-r--r-- | services/audioflinger/AudioFlinger.h | 25 |
2 files changed, 24 insertions, 7 deletions
diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp index fc4969e..e2b29f7 100644 --- a/services/audioflinger/AudioFlinger.cpp +++ b/services/audioflinger/AudioFlinger.cpp @@ -2513,7 +2513,7 @@ bool AudioFlinger::PlaybackThread::threadLoop() // put audio hardware into standby after short delay if (CC_UNLIKELY((!mActiveTracks.size() && systemTime() > standbyTime) || - mSuspended > 0)) { + isSuspended())) { if (!mStandby) { threadLoop_standby(); @@ -2567,7 +2567,7 @@ bool AudioFlinger::PlaybackThread::threadLoop() threadLoop_sleepTime(); } - if (mSuspended > 0) { + if (isSuspended()) { sleepTime = suspendSleepTimeUs(); } @@ -2752,7 +2752,7 @@ void AudioFlinger::MixerThread::threadLoop_standby() // shared by MIXER and DIRECT, overridden by DUPLICATING void AudioFlinger::PlaybackThread::threadLoop_standby() { - ALOGV("Audio hardware entering standby, mixer %p, suspend count %u", this, mSuspended); + ALOGV("Audio hardware entering standby, mixer %p, suspend count %d", this, mSuspended); mOutput->stream->common.standby(&mOutput->stream->common); } diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h index 648a8d2..a6fd0a0 100644 --- a/services/audioflinger/AudioFlinger.h +++ b/services/audioflinger/AudioFlinger.h @@ -1023,9 +1023,19 @@ public: AudioStreamOut* clearOutput(); virtual audio_stream_t* stream() const; - void suspend() { mSuspended++; } - void restore() { if (mSuspended > 0) mSuspended--; } - bool isSuspended() const { return (mSuspended > 0); } + // a very large number of suspend() will eventually wraparound, but unlikely + void suspend() { (void) android_atomic_inc(&mSuspended); } + void restore() + { + // if restore() is done without suspend(), get back into + // range so that the next suspend() will operate correctly + if (android_atomic_dec(&mSuspended) <= 0) { + android_atomic_release_store(0, &mSuspended); + } + } + bool isSuspended() const + { return android_atomic_acquire_load(&mSuspended) > 0; } + virtual String8 getParameters(const String8& keys); virtual void audioConfigChanged_l(int event, int param = 0); status_t getRenderPosition(uint32_t *halFrames, uint32_t *dspFrames); @@ -1050,7 +1060,14 @@ public: protected: int16_t* mMixBuffer; - uint32_t mSuspended; // suspend count, > 0 means suspended + + // suspend count, > 0 means suspended. While suspended, the thread continues to pull from + // tracks and mix, but doesn't write to HAL. A2DP and SCO HAL implementations can't handle + // concurrent use of both of them, so Audio Policy Service suspends one of the threads to + // workaround that restriction. + // 'volatile' means accessed via atomic operations and no lock. + volatile int32_t mSuspended; + int mBytesWritten; private: // mMasterMute is in both PlaybackThread and in AudioFlinger. When a |