Fix issue 3157123.
Use a Mutex wherever atomic operations were used in AudioTrack,
AudioRecord, AudioFlinger and AudioEffect classes.
Change-Id: I6f55b2cabdcd93d64ef19446735b8f33720f8dbc
diff --git a/include/media/AudioEffect.h b/include/media/AudioEffect.h
index c967efb..cda2be0 100644
--- a/include/media/AudioEffect.h
+++ b/include/media/AudioEffect.h
@@ -403,7 +403,7 @@
static status_t guidToString(const effect_uuid_t *guid, char *str, size_t maxLen);
protected:
- volatile int32_t mEnabled; // enable state
+ bool mEnabled; // enable state
int32_t mSessionId; // audio session ID
int32_t mPriority; // priority for effect control
status_t mStatus; // effect status
@@ -412,6 +412,7 @@
void* mUserData; // client context for callback function
effect_descriptor_t mDescriptor; // effect descriptor
int32_t mId; // system wide unique effect engine instance ID
+ Mutex mLock; // Mutex for mEnabled access
private:
diff --git a/include/media/AudioRecord.h b/include/media/AudioRecord.h
index 38e3d44..5f7cd90 100644
--- a/include/media/AudioRecord.h
+++ b/include/media/AudioRecord.h
@@ -356,7 +356,7 @@
sp<IAudioRecord> mAudioRecord;
sp<IMemory> mCblkMemory;
sp<ClientRecordThread> mClientRecordThread;
- Mutex mRecordThreadLock;
+ Mutex mLock;
uint32_t mFrameCount;
diff --git a/include/media/AudioTrack.h b/include/media/AudioTrack.h
index 4475d4a..813a905 100644
--- a/include/media/AudioTrack.h
+++ b/include/media/AudioTrack.h
@@ -480,6 +480,7 @@
uint32_t mFlags;
int mSessionId;
int mAuxEffectId;
+ Mutex mLock;
};
diff --git a/media/jni/soundpool/SoundPool.h b/media/jni/soundpool/SoundPool.h
index 42037af..1b0fd38 100644
--- a/media/jni/soundpool/SoundPool.h
+++ b/media/jni/soundpool/SoundPool.h
@@ -22,7 +22,6 @@
#include <utils/Vector.h>
#include <utils/KeyedVector.h>
#include <media/AudioTrack.h>
-#include <cutils/atomic.h>
namespace android {
diff --git a/media/libmedia/AudioEffect.cpp b/media/libmedia/AudioEffect.cpp
index 88b8c86..aadeba5 100644
--- a/media/libmedia/AudioEffect.cpp
+++ b/media/libmedia/AudioEffect.cpp
@@ -27,7 +27,6 @@
#include <media/AudioEffect.h>
#include <utils/Log.h>
-#include <cutils/atomic.h>
#include <binder/IPCThreadState.h>
@@ -207,18 +206,22 @@
return INVALID_OPERATION;
}
- if (enabled) {
- LOGV("enable %p", this);
- if (android_atomic_or(1, &mEnabled) == 0) {
- return mIEffect->enable();
+ status_t status = NO_ERROR;
+
+ AutoMutex lock(mLock);
+ if (enabled != mEnabled) {
+ if (enabled) {
+ LOGV("enable %p", this);
+ status = mIEffect->enable();
+ } else {
+ LOGV("disable %p", this);
+ status = mIEffect->disable();
}
- } else {
- LOGV("disable %p", this);
- if (android_atomic_and(~1, &mEnabled) == 1) {
- return mIEffect->disable();
+ if (status == NO_ERROR) {
+ mEnabled = enabled;
}
}
- return NO_ERROR;
+ return status;
}
status_t AudioEffect::command(uint32_t cmdCode,
@@ -232,26 +235,26 @@
return INVALID_OPERATION;
}
- if ((cmdCode == EFFECT_CMD_ENABLE || cmdCode == EFFECT_CMD_DISABLE) &&
- (replySize == NULL || *replySize != sizeof(status_t) || replyData == NULL)) {
- return BAD_VALUE;
+ if (cmdCode == EFFECT_CMD_ENABLE || cmdCode == EFFECT_CMD_DISABLE) {
+ if (mEnabled == (cmdCode == EFFECT_CMD_ENABLE)) {
+ return NO_ERROR;
+ }
+ if (replySize == NULL || *replySize != sizeof(status_t) || replyData == NULL) {
+ return BAD_VALUE;
+ }
+ mLock.lock();
}
status_t status = mIEffect->command(cmdCode, cmdSize, cmdData, replySize, replyData);
- if (status != NO_ERROR) {
- return status;
- }
if (cmdCode == EFFECT_CMD_ENABLE || cmdCode == EFFECT_CMD_DISABLE) {
- status = *(status_t *)replyData;
- if (status != NO_ERROR) {
- return status;
+ if (status == NO_ERROR) {
+ status = *(status_t *)replyData;
}
- if (cmdCode == EFFECT_CMD_ENABLE) {
- android_atomic_or(1, &mEnabled);
- } else {
- android_atomic_and(~1, &mEnabled);
+ if (status == NO_ERROR) {
+ mEnabled = (cmdCode == EFFECT_CMD_ENABLE);
}
+ mLock.unlock();
}
return status;
@@ -370,11 +373,7 @@
{
LOGV("enableStatusChanged %p enabled %d mCbf %p", this, enabled, mCbf);
if (mStatus == ALREADY_EXISTS) {
- if (enabled) {
- android_atomic_or(1, &mEnabled);
- } else {
- android_atomic_and(~1, &mEnabled);
- }
+ mEnabled = enabled;
if (mCbf) {
mCbf(EVENT_ENABLE_STATUS_CHANGED, mUserData, &enabled);
}
diff --git a/media/libmedia/AudioRecord.cpp b/media/libmedia/AudioRecord.cpp
index a6c515c..1d6ffa0 100644
--- a/media/libmedia/AudioRecord.cpp
+++ b/media/libmedia/AudioRecord.cpp
@@ -35,7 +35,6 @@
#include <binder/Parcel.h>
#include <binder/IPCThreadState.h>
#include <utils/Timers.h>
-#include <cutils/atomic.h>
#define LIKELY( exp ) (__builtin_expect( (exp) != 0, true ))
#define UNLIKELY( exp ) (__builtin_expect( (exp) != 0, false ))
@@ -282,7 +281,9 @@
t->mLock.lock();
}
- if (android_atomic_or(1, &mActive) == 0) {
+ AutoMutex lock(mLock);
+ if (mActive == 0) {
+ mActive = 1;
ret = mAudioRecord->start();
if (ret == DEAD_OBJECT) {
LOGV("start() dead IAudioRecord: creating a new one");
@@ -302,8 +303,7 @@
setpriority(PRIO_PROCESS, 0, THREAD_PRIORITY_AUDIO_CLIENT);
}
} else {
- LOGV("start() failed");
- android_atomic_and(~1, &mActive);
+ mActive = 0;
}
}
@@ -322,9 +322,11 @@
if (t != 0) {
t->mLock.lock();
- }
+ }
- if (android_atomic_and(~1, &mActive) == 1) {
+ AutoMutex lock(mLock);
+ if (mActive == 1) {
+ mActive = 0;
mCblk->cv.signal();
mAudioRecord->stop();
// the record head position will reset to 0, so if a marker is set, we need
diff --git a/media/libmedia/AudioTrack.cpp b/media/libmedia/AudioTrack.cpp
index 587c8ff..c1bed59 100644
--- a/media/libmedia/AudioTrack.cpp
+++ b/media/libmedia/AudioTrack.cpp
@@ -35,7 +35,6 @@
#include <binder/Parcel.h>
#include <binder/IPCThreadState.h>
#include <utils/Timers.h>
-#include <cutils/atomic.h>
#define LIKELY( exp ) (__builtin_expect( (exp) != 0, true ))
#define UNLIKELY( exp ) (__builtin_expect( (exp) != 0, false ))
@@ -312,7 +311,9 @@
t->mLock.lock();
}
- if (android_atomic_or(1, &mActive) == 0) {
+ AutoMutex lock(mLock);
+ if (mActive == 0) {
+ mActive = 1;
mNewPosition = mCblk->server + mUpdatePeriod;
mCblk->bufferTimeoutMs = MAX_STARTUP_TIMEOUT_MS;
mCblk->waitTimeMs = 0;
@@ -344,7 +345,7 @@
}
if (status != NO_ERROR) {
LOGV("start() failed");
- android_atomic_and(~1, &mActive);
+ mActive = 0;
if (t != 0) {
t->requestExit();
} else {
@@ -367,7 +368,9 @@
t->mLock.lock();
}
- if (android_atomic_and(~1, &mActive) == 1) {
+ AutoMutex lock(mLock);
+ if (mActive == 1) {
+ mActive = 0;
mCblk->cv.signal();
mAudioTrack->stop();
// Cancel loops (If we are in the middle of a loop, playback
@@ -407,7 +410,6 @@
mMarkerReached = false;
mUpdatePeriod = 0;
-
if (!mActive) {
mAudioTrack->flush();
// Release AudioTrack callback thread in case it was waiting for new buffers
@@ -419,7 +421,9 @@
void AudioTrack::pause()
{
LOGV("pause");
- if (android_atomic_and(~1, &mActive) == 1) {
+ AutoMutex lock(mLock);
+ if (mActive == 1) {
+ mActive = 0;
mAudioTrack->pause();
}
}
diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp
index 84dd022..51b5947 100644
--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -343,7 +343,7 @@
lSessionId = *sessionId;
} else {
// if no audio session id is provided, create one here
- lSessionId = nextUniqueId();
+ lSessionId = nextUniqueId_l();
if (sessionId != NULL) {
*sessionId = lSessionId;
}
@@ -3699,7 +3699,7 @@
if (sessionId != NULL && *sessionId != AudioSystem::SESSION_OUTPUT_MIX) {
lSessionId = *sessionId;
} else {
- lSessionId = nextUniqueId();
+ lSessionId = nextUniqueId_l();
if (sessionId != NULL) {
*sessionId = lSessionId;
}
@@ -4300,7 +4300,7 @@
mHardwareStatus = AUDIO_HW_IDLE;
if (output != 0) {
- int id = nextUniqueId();
+ int id = nextUniqueId_l();
if ((flags & AudioSystem::OUTPUT_FLAG_DIRECT) ||
(format != AudioSystem::PCM_16_BIT) ||
(channels != AudioSystem::CHANNEL_OUT_STEREO)) {
@@ -4348,7 +4348,7 @@
return 0;
}
- int id = nextUniqueId();
+ int id = nextUniqueId_l();
DuplicatingThread *thread = new DuplicatingThread(this, thread1, id);
thread->addOutputTrack(thread2);
mPlaybackThreads.add(id, thread);
@@ -4473,7 +4473,7 @@
}
if (input != 0) {
- int id = nextUniqueId();
+ int id = nextUniqueId_l();
// Start record thread
thread = new RecordThread(this, input, reqSamplingRate, reqChannels, id);
mRecordThreads.add(id, thread);
@@ -4543,7 +4543,8 @@
int AudioFlinger::newAudioSessionId()
{
- return nextUniqueId();
+ AutoMutex _l(mLock);
+ return nextUniqueId_l();
}
// checkPlaybackThread_l() must be called with AudioFlinger::mLock held
@@ -4578,9 +4579,10 @@
return thread;
}
-int AudioFlinger::nextUniqueId()
+// nextUniqueId_l() must be called with AudioFlinger::mLock held
+int AudioFlinger::nextUniqueId_l()
{
- return android_atomic_inc(&mNextUniqueId);
+ return mNextUniqueId++;
}
// ----------------------------------------------------------------------------
@@ -4967,7 +4969,7 @@
LOGV("createEffect_l() got effect %p on chain %p", effect == 0 ? 0 : effect.get(), chain.get());
if (effect == 0) {
- int id = mAudioFlinger->nextUniqueId();
+ int id = mAudioFlinger->nextUniqueId_l();
// Check CPU and memory usage
lStatus = AudioSystem::registerEffect(desc, mId, chain->strategy(), sessionId, id);
if (lStatus != NO_ERROR) {
diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h
index 5917632..f0ef867 100644
--- a/services/audioflinger/AudioFlinger.h
+++ b/services/audioflinger/AudioFlinger.h
@@ -785,7 +785,7 @@
float streamVolumeInternal(int stream) const { return mStreamTypes[stream].volume; }
void audioConfigChanged_l(int event, int ioHandle, void *param2);
- int nextUniqueId();
+ int nextUniqueId_l();
status_t moveEffectChain_l(int session,
AudioFlinger::PlaybackThread *srcThread,
AudioFlinger::PlaybackThread *dstThread,