Fix issue 3439872: video chat and bluetooth SCO
This change fixes the stability problems experienced when using
a bluetooth headset supporting both A2DP and SCO. Problems occur
when starting the video chat at which time the A2DP output is being
stopped to start SCO. At that time, active AudioTracks are invalidated
by AudioFlinger so that a new AudioTrack binder interface can be
recreated by the client process on the new mixer thread with correct parameters.
The problem was that the process to restore the binder interface was not
protected against concurrent requests which caused 2 binder interfaces
to be created sometimes. This could lead to permanent client deadlock
if one of the client threads was waiting for a condition of the first
created binder interface while the second one was created (as the AudioFlinger
would only signal conditions on the last one created).
This concurrent request situation is more likely to happen when a client
uses the JAVA AudioTrack as the JNI implementation uses simultaneously the
native AudioTrack callback and write push mechanisms. By doing so, the code
that checks if the binder interface should be restored (in obtainBuffer()) is
much more likely to be called concurrently from two different threads.
The fix consists in protecting the critical binder interface restore phase
with a flag in the AudioTrack control block. The first thread acting upon the binder
interface restore request will raise the flag and the second thread will just wait for
a condition to be signaled when the restore process is complete.
Also protected all accesses to the AudioTrack control block by a mutex to prevent
access while the track is being destroyed and restored. If a mutex cannot be held
(e.g because we call a callback function), acquire a strong reference on the IAudioTrack
to prevent its destruction while the cblk is being accessed.
Modified AudioTrack JNI to use GetByteArrayElements() instead of
GetPrimitiveArrayCritical() when writing audio buffers. Entering a critical section would
cause the JNI to abort if a mediaserver crash occurs during a write due to the AudioSystem
callback being called during the critical section when media server process restarts.
Anyway with current JNI implementation, either versions do not copy data most of the times
and the criticial version does not guaranty no data copy.
The same modifications have been made to AudioRecord.
Change-Id: Idc5aa711a04c3eee180cdd03f44fe17f3c4dcb52
diff --git a/media/libmedia/AudioTrack.cpp b/media/libmedia/AudioTrack.cpp
index c1bed59..8d8f67b 100644
--- a/media/libmedia/AudioTrack.cpp
+++ b/media/libmedia/AudioTrack.cpp
@@ -148,6 +148,7 @@
LOGV_IF(sharedBuffer != 0, "sharedBuffer: %p, size: %d", sharedBuffer->pointer(), sharedBuffer->size());
+ AutoMutex lock(mLock);
if (mAudioTrack != 0) {
LOGE("Track already in use");
return INVALID_OPERATION;
@@ -211,8 +212,15 @@
mAuxEffectId = 0;
// create the IAudioTrack
- status_t status = createTrack(streamType, sampleRate, format, channelCount,
- frameCount, flags, sharedBuffer, output, true);
+ status_t status = createTrack_l(streamType,
+ sampleRate,
+ format,
+ channelCount,
+ frameCount,
+ flags,
+ sharedBuffer,
+ output,
+ true);
if (status != NO_ERROR) {
return status;
@@ -312,37 +320,38 @@
}
AutoMutex lock(mLock);
+ // acquire a strong reference on the IMemory and IAudioTrack so that they cannot be destroyed
+ // while we are accessing the cblk
+ sp <IAudioTrack> audioTrack = mAudioTrack;
+ sp <IMemory> iMem = mCblkMemory;
+ audio_track_cblk_t* cblk = mCblk;
+
if (mActive == 0) {
mActive = 1;
- mNewPosition = mCblk->server + mUpdatePeriod;
- mCblk->bufferTimeoutMs = MAX_STARTUP_TIMEOUT_MS;
- mCblk->waitTimeMs = 0;
- mCblk->flags &= ~CBLK_DISABLED_ON;
+ mNewPosition = cblk->server + mUpdatePeriod;
+ cblk->bufferTimeoutMs = MAX_STARTUP_TIMEOUT_MS;
+ cblk->waitTimeMs = 0;
+ cblk->flags &= ~CBLK_DISABLED_ON;
if (t != 0) {
t->run("AudioTrackThread", THREAD_PRIORITY_AUDIO_CLIENT);
} else {
setpriority(PRIO_PROCESS, 0, THREAD_PRIORITY_AUDIO_CLIENT);
}
- if (mCblk->flags & CBLK_INVALID_MSK) {
- LOGW("start() track %p invalidated, creating a new one", this);
- // no need to clear the invalid flag as this cblk will not be used anymore
- // force new track creation
- status = DEAD_OBJECT;
- } else {
+ LOGV("start %p before lock cblk %p", this, mCblk);
+ cblk->lock.lock();
+ if (!(cblk->flags & CBLK_INVALID_MSK)) {
+ cblk->lock.unlock();
status = mAudioTrack->start();
- }
- if (status == DEAD_OBJECT) {
- LOGV("start() dead IAudioTrack: creating a new one");
- status = createTrack(mStreamType, mCblk->sampleRate, mFormat, mChannelCount,
- mFrameCount, mFlags, mSharedBuffer, getOutput(), false);
- if (status == NO_ERROR) {
- status = mAudioTrack->start();
- if (status == NO_ERROR) {
- mNewPosition = mCblk->server + mUpdatePeriod;
- }
+ cblk->lock.lock();
+ if (status == DEAD_OBJECT) {
+ cblk->flags |= CBLK_INVALID_MSK;
}
}
+ if (cblk->flags & CBLK_INVALID_MSK) {
+ status = restoreTrack_l(cblk, true);
+ }
+ cblk->lock.unlock();
if (status != NO_ERROR) {
LOGV("start() failed");
mActive = 0;
@@ -375,14 +384,14 @@
mAudioTrack->stop();
// Cancel loops (If we are in the middle of a loop, playback
// would not stop until loopCount reaches 0).
- setLoop(0, 0, 0);
+ setLoop_l(0, 0, 0);
// the playback head position will reset to 0, so if a marker is set, we need
// to activate it again
mMarkerReached = false;
// Force flush if a shared buffer is used otherwise audioflinger
// will not stop before end of buffer is reached.
if (mSharedBuffer != 0) {
- flush();
+ flush_l();
}
if (t != 0) {
t->requestExit();
@@ -403,6 +412,13 @@
void AudioTrack::flush()
{
+ AutoMutex lock(mLock);
+ flush_l();
+}
+
+// must be called with mLock held
+void AudioTrack::flush_l()
+{
LOGV("flush");
// clear playback marker and periodic update counter
@@ -445,6 +461,7 @@
return BAD_VALUE;
}
+ AutoMutex lock(mLock);
mVolume[LEFT] = left;
mVolume[RIGHT] = right;
@@ -470,6 +487,7 @@
if (level > 1.0f) {
return BAD_VALUE;
}
+ AutoMutex lock(mLock);
mSendLevel = level;
@@ -495,17 +513,26 @@
// Resampler implementation limits input sampling rate to 2 x output sampling rate.
if (rate <= 0 || rate > afSamplingRate*2 ) return BAD_VALUE;
+ AutoMutex lock(mLock);
mCblk->sampleRate = rate;
return NO_ERROR;
}
uint32_t AudioTrack::getSampleRate()
{
+ AutoMutex lock(mLock);
return mCblk->sampleRate;
}
status_t AudioTrack::setLoop(uint32_t loopStart, uint32_t loopEnd, int loopCount)
{
+ AutoMutex lock(mLock);
+ return setLoop_l(loopStart, loopEnd, loopCount);
+}
+
+// must be called with mLock held
+status_t AudioTrack::setLoop_l(uint32_t loopStart, uint32_t loopEnd, int loopCount)
+{
audio_track_cblk_t* cblk = mCblk;
Mutex::Autolock _l(cblk->lock);
@@ -540,6 +567,7 @@
status_t AudioTrack::getLoop(uint32_t *loopStart, uint32_t *loopEnd, int *loopCount)
{
+ AutoMutex lock(mLock);
if (loopStart != 0) {
*loopStart = mCblk->loopStart;
}
@@ -599,6 +627,7 @@
status_t AudioTrack::setPosition(uint32_t position)
{
+ AutoMutex lock(mLock);
Mutex::Autolock _l(mCblk->lock);
if (!stopped()) return INVALID_OPERATION;
@@ -614,7 +643,7 @@
status_t AudioTrack::getPosition(uint32_t *position)
{
if (position == 0) return BAD_VALUE;
-
+ AutoMutex lock(mLock);
*position = mCblk->server;
return NO_ERROR;
@@ -622,9 +651,11 @@
status_t AudioTrack::reload()
{
+ AutoMutex lock(mLock);
+
if (!stopped()) return INVALID_OPERATION;
- flush();
+ flush_l();
mCblk->stepUser(mCblk->frameCount);
@@ -633,6 +664,13 @@
audio_io_handle_t AudioTrack::getOutput()
{
+ AutoMutex lock(mLock);
+ return getOutput_l();
+}
+
+// must be called with mLock held
+audio_io_handle_t AudioTrack::getOutput_l()
+{
return AudioSystem::getOutput((AudioSystem::stream_type)mStreamType,
mCblk->sampleRate, mFormat, mChannels, (AudioSystem::output_flags)mFlags);
}
@@ -654,7 +692,8 @@
// -------------------------------------------------------------------------
-status_t AudioTrack::createTrack(
+// must be called with mLock held
+status_t AudioTrack::createTrack_l(
int streamType,
uint32_t sampleRate,
int format,
@@ -774,6 +813,7 @@
status_t AudioTrack::obtainBuffer(Buffer* audioBuffer, int32_t waitCount)
{
+ AutoMutex lock(mLock);
int active;
status_t result;
audio_track_cblk_t* cblk = mCblk;
@@ -800,12 +840,17 @@
return WOULD_BLOCK;
}
if (!(cblk->flags & CBLK_INVALID_MSK)) {
+ mLock.unlock();
result = cblk->cv.waitRelative(cblk->lock, milliseconds(waitTimeMs));
- }
- if (cblk->flags & CBLK_INVALID_MSK) {
- LOGW("obtainBuffer() track %p invalidated, creating a new one", this);
- // no need to clear the invalid flag as this cblk will not be used anymore
cblk->lock.unlock();
+ mLock.lock();
+ if (mActive == 0) {
+ return status_t(STOPPED);
+ }
+ cblk->lock.lock();
+ }
+
+ if (cblk->flags & CBLK_INVALID_MSK) {
goto create_new_track;
}
if (__builtin_expect(result!=NO_ERROR, false)) {
@@ -819,18 +864,17 @@
//unlock cblk mutex before calling mAudioTrack->start() (see issue #1617140)
cblk->lock.unlock();
result = mAudioTrack->start();
- if (result == DEAD_OBJECT) {
- LOGW("obtainBuffer() dead IAudioTrack: creating a new one");
-create_new_track:
- result = createTrack(mStreamType, cblk->sampleRate, mFormat, mChannelCount,
- mFrameCount, mFlags, mSharedBuffer, getOutput(), false);
- if (result == NO_ERROR) {
- cblk = mCblk;
- cblk->bufferTimeoutMs = MAX_RUN_TIMEOUT_MS;
- mAudioTrack->start();
- }
- }
cblk->lock.lock();
+ if (result == DEAD_OBJECT) {
+ cblk->flags |= CBLK_INVALID_MSK;
+create_new_track:
+ result = restoreTrack_l(cblk, false);
+ }
+ if (result != NO_ERROR) {
+ LOGW("obtainBuffer create Track error %d", result);
+ cblk->lock.unlock();
+ return result;
+ }
}
cblk->waitTimeMs = 0;
}
@@ -848,7 +892,7 @@
}
// restart track if it was disabled by audioflinger due to previous underrun
- if (cblk->flags & CBLK_DISABLED_MSK) {
+ if (mActive && (cblk->flags & CBLK_DISABLED_MSK)) {
cblk->flags &= ~CBLK_DISABLED_ON;
LOGW("obtainBuffer() track %p disabled, restarting", this);
mAudioTrack->start();
@@ -883,8 +927,8 @@
void AudioTrack::releaseBuffer(Buffer* audioBuffer)
{
- audio_track_cblk_t* cblk = mCblk;
- cblk->stepUser(audioBuffer->frameCount);
+ AutoMutex lock(mLock);
+ mCblk->stepUser(audioBuffer->frameCount);
}
// -------------------------------------------------------------------------
@@ -903,6 +947,13 @@
LOGV("write %p: %d bytes, mActive=%d", this, userSize, mActive);
+ // acquire a strong reference on the IMemory and IAudioTrack so that they cannot be destroyed
+ // while we are accessing the cblk
+ mLock.lock();
+ sp <IAudioTrack> audioTrack = mAudioTrack;
+ sp <IMemory> iMem = mCblkMemory;
+ mLock.unlock();
+
ssize_t written = 0;
const int8_t *src = (const int8_t *)buffer;
Buffer audioBuffer;
@@ -953,21 +1004,29 @@
uint32_t frames;
size_t writtenSize;
+ mLock.lock();
+ // acquire a strong reference on the IMemory and IAudioTrack so that they cannot be destroyed
+ // while we are accessing the cblk
+ sp <IAudioTrack> audioTrack = mAudioTrack;
+ sp <IMemory> iMem = mCblkMemory;
+ audio_track_cblk_t* cblk = mCblk;
+ mLock.unlock();
+
// Manage underrun callback
- if (mActive && (mCblk->framesReady() == 0)) {
- LOGV("Underrun user: %x, server: %x, flags %04x", mCblk->user, mCblk->server, mCblk->flags);
- if ((mCblk->flags & CBLK_UNDERRUN_MSK) == CBLK_UNDERRUN_OFF) {
+ if (mActive && (cblk->framesReady() == 0)) {
+ LOGV("Underrun user: %x, server: %x, flags %04x", cblk->user, cblk->server, cblk->flags);
+ if ((cblk->flags & CBLK_UNDERRUN_MSK) == CBLK_UNDERRUN_OFF) {
mCbf(EVENT_UNDERRUN, mUserData, 0);
- if (mCblk->server == mCblk->frameCount) {
+ if (cblk->server == cblk->frameCount) {
mCbf(EVENT_BUFFER_END, mUserData, 0);
}
- mCblk->flags |= CBLK_UNDERRUN_ON;
+ cblk->flags |= CBLK_UNDERRUN_ON;
if (mSharedBuffer != 0) return false;
}
}
// Manage loop end callback
- while (mLoopCount > mCblk->loopCount) {
+ while (mLoopCount > cblk->loopCount) {
int loopCount = -1;
mLoopCount--;
if (mLoopCount >= 0) loopCount = mLoopCount;
@@ -977,7 +1036,7 @@
// Manage marker callback
if (!mMarkerReached && (mMarkerPosition > 0)) {
- if (mCblk->server >= mMarkerPosition) {
+ if (cblk->server >= mMarkerPosition) {
mCbf(EVENT_MARKER, mUserData, (void *)&mMarkerPosition);
mMarkerReached = true;
}
@@ -985,7 +1044,7 @@
// Manage new position callback
if (mUpdatePeriod > 0) {
- while (mCblk->server >= mNewPosition) {
+ while (cblk->server >= mNewPosition) {
mCbf(EVENT_NEW_POS, mUserData, (void *)&mNewPosition);
mNewPosition += mUpdatePeriod;
}
@@ -1068,6 +1127,84 @@
return true;
}
+// must be called with mLock and cblk.lock held. Callers must also hold strong references on
+// the IAudioTrack and IMemory in case they are recreated here.
+// If the IAudioTrack is successfully restored, the cblk pointer is updated
+status_t AudioTrack::restoreTrack_l(audio_track_cblk_t*& cblk, bool fromStart)
+{
+ status_t result;
+
+ if (!(cblk->flags & CBLK_RESTORING_MSK)) {
+ LOGW("dead IAudioTrack, creating a new one from %s",
+ fromStart ? "start()" : "obtainBuffer()");
+
+ cblk->flags |= CBLK_RESTORING_ON;
+ // signal old cblk condition so that other threads waiting for available buffers stop
+ // waiting now
+ cblk->cv.broadcast();
+ cblk->lock.unlock();
+
+ // if the new IAudioTrack is created, createTrack_l() will modify the
+ // following member variables: mAudioTrack, mCblkMemory and mCblk.
+ // It will also delete the strong references on previous IAudioTrack and IMemory
+ result = createTrack_l(mStreamType,
+ cblk->sampleRate,
+ mFormat,
+ mChannelCount,
+ mFrameCount,
+ mFlags,
+ mSharedBuffer,
+ getOutput_l(),
+ false);
+
+ if (result == NO_ERROR) {
+ if (!fromStart) {
+ mCblk->bufferTimeoutMs = MAX_RUN_TIMEOUT_MS;
+ }
+ result = mAudioTrack->start();
+ if (fromStart && result == NO_ERROR) {
+ mNewPosition = mCblk->server + mUpdatePeriod;
+ }
+ }
+ if (result != NO_ERROR) {
+ mActive = false;
+ }
+
+ // signal old cblk condition for other threads waiting for restore completion
+ cblk->lock.lock();
+ cblk->flags |= CBLK_RESTORED_MSK;
+ cblk->cv.broadcast();
+ cblk->lock.unlock();
+ } else {
+ if (!(cblk->flags & CBLK_RESTORED_MSK)) {
+ LOGW("dead IAudioTrack, waiting for a new one");
+ mLock.unlock();
+ result = cblk->cv.waitRelative(cblk->lock, milliseconds(RESTORE_TIMEOUT_MS));
+ cblk->lock.unlock();
+ mLock.lock();
+ } else {
+ LOGW("dead IAudioTrack, already restored");
+ result = NO_ERROR;
+ cblk->lock.unlock();
+ }
+ if (result != NO_ERROR || mActive == 0) {
+ result = status_t(STOPPED);
+ }
+ }
+ LOGV("restoreTrack_l() status %d mActive %d cblk %p, old cblk %p flags %08x old flags %08x",
+ result, mActive, mCblk, cblk, mCblk->flags, cblk->flags);
+
+ if (result == NO_ERROR) {
+ // from now on we switch to the newly created cblk
+ cblk = mCblk;
+ }
+ cblk->lock.lock();
+
+ LOGW_IF(result != NO_ERROR, "restoreTrack_l() error %d", result);
+
+ return result;
+}
+
status_t AudioTrack::dump(int fd, const Vector<String16>& args) const
{
@@ -1197,7 +1334,9 @@
this->server = s;
- cv.signal();
+ if (!(flags & CBLK_INVALID_MSK)) {
+ cv.signal();
+ }
lock.unlock();
return true;
}