Fix issue 4111672: AudioTrack control block flags
Make sure that all read/modify/write operations on the AudioTrack
and AudioRecord control block flags field are protected by the
control block's mutex.
Also fix potential infinite loop in AudioTrack::write() if the
written size is not a multiple of frame size.
Change-Id: Ib3d557eb45dcc3abeb32c9aa56058e2873afee27
diff --git a/media/libmedia/AudioRecord.cpp b/media/libmedia/AudioRecord.cpp
index a18bedb..cee1c75 100644
--- a/media/libmedia/AudioRecord.cpp
+++ b/media/libmedia/AudioRecord.cpp
@@ -722,9 +722,12 @@
// Manage overrun callback
if (mActive && (cblk->framesAvailable() == 0)) {
LOGV("Overrun user: %x, server: %x, flags %04x", cblk->user, cblk->server, cblk->flags);
+ AutoMutex _l(cblk->lock);
if ((cblk->flags & CBLK_UNDERRUN_MSK) == CBLK_UNDERRUN_OFF) {
- mCbf(EVENT_OVERRUN, mUserData, 0);
cblk->flags |= CBLK_UNDERRUN_ON;
+ cblk->lock.unlock();
+ mCbf(EVENT_OVERRUN, mUserData, 0);
+ cblk->lock.lock();
}
}
diff --git a/media/libmedia/AudioTrack.cpp b/media/libmedia/AudioTrack.cpp
index 8d8f67b..02e1570 100644
--- a/media/libmedia/AudioTrack.cpp
+++ b/media/libmedia/AudioTrack.cpp
@@ -329,6 +329,7 @@
if (mActive == 0) {
mActive = 1;
mNewPosition = cblk->server + mUpdatePeriod;
+ cblk->lock.lock();
cblk->bufferTimeoutMs = MAX_STARTUP_TIMEOUT_MS;
cblk->waitTimeMs = 0;
cblk->flags &= ~CBLK_DISABLED_ON;
@@ -339,7 +340,6 @@
}
LOGV("start %p before lock cblk %p", this, mCblk);
- cblk->lock.lock();
if (!(cblk->flags & CBLK_INVALID_MSK)) {
cblk->lock.unlock();
status = mAudioTrack->start();
@@ -893,9 +893,14 @@
// restart track if it was disabled by audioflinger due to previous underrun
if (mActive && (cblk->flags & CBLK_DISABLED_MSK)) {
- cblk->flags &= ~CBLK_DISABLED_ON;
- LOGW("obtainBuffer() track %p disabled, restarting", this);
- mAudioTrack->start();
+ AutoMutex _l(cblk->lock);
+ if (mActive && (cblk->flags & CBLK_DISABLED_MSK)) {
+ cblk->flags &= ~CBLK_DISABLED_ON;
+ cblk->lock.unlock();
+ LOGW("obtainBuffer() track %p disabled, restarting", this);
+ mAudioTrack->start();
+ cblk->lock.lock();
+ }
}
cblk->waitTimeMs = 0;
@@ -957,9 +962,10 @@
ssize_t written = 0;
const int8_t *src = (const int8_t *)buffer;
Buffer audioBuffer;
+ size_t frameSz = (size_t)frameSize();
do {
- audioBuffer.frameCount = userSize/frameSize();
+ audioBuffer.frameCount = userSize/frameSz;
// Calling obtainBuffer() with a negative wait count causes
// an (almost) infinite wait time.
@@ -991,7 +997,7 @@
written += toWrite;
releaseBuffer(&audioBuffer);
- } while (userSize);
+ } while (userSize >= frameSz);
return written;
}
@@ -1015,12 +1021,15 @@
// Manage underrun callback
if (mActive && (cblk->framesReady() == 0)) {
LOGV("Underrun user: %x, server: %x, flags %04x", cblk->user, cblk->server, cblk->flags);
+ AutoMutex _l(cblk->lock);
if ((cblk->flags & CBLK_UNDERRUN_MSK) == CBLK_UNDERRUN_OFF) {
+ cblk->flags |= CBLK_UNDERRUN_ON;
+ cblk->lock.unlock();
mCbf(EVENT_UNDERRUN, mUserData, 0);
if (cblk->server == cblk->frameCount) {
mCbf(EVENT_BUFFER_END, mUserData, 0);
}
- cblk->flags |= CBLK_UNDERRUN_ON;
+ cblk->lock.lock();
if (mSharedBuffer != 0) return false;
}
}
@@ -1279,7 +1288,12 @@
this->user = u;
// Clear flow control error condition as new data has been written/read to/from buffer.
- flags &= ~CBLK_UNDERRUN_MSK;
+ if (flags & CBLK_UNDERRUN_MSK) {
+ AutoMutex _l(lock);
+ if (flags & CBLK_UNDERRUN_MSK) {
+ flags &= ~CBLK_UNDERRUN_MSK;
+ }
+ }
return u;
}
diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp
index 2b08ab5..2702242 100644
--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -1738,7 +1738,10 @@
LOGV("BUFFER TIMEOUT: remove(%d) from active list on thread %p", track->name(), this);
tracksToRemove->add(track);
// indicate to client process that the track was disabled because of underrun
- cblk->flags |= CBLK_DISABLED_ON;
+ {
+ AutoMutex _l(cblk->lock);
+ cblk->flags |= CBLK_DISABLED_ON;
+ }
} else if (mixerStatus != MIXER_TRACKS_READY) {
mixerStatus = MIXER_TRACKS_ENABLED;
}
@@ -1787,10 +1790,9 @@
for (size_t i = 0; i < size; i++) {
sp<Track> t = mTracks[i];
if (t->type() == streamType) {
- t->mCblk->lock.lock();
+ AutoMutex _lcblk(t->mCblk->lock);
t->mCblk->flags |= CBLK_INVALID_ON;
t->mCblk->cv.signal();
- t->mCblk->lock.unlock();
}
}
}
@@ -2948,6 +2950,7 @@
if (mCblk->framesReady() >= mCblk->frameCount ||
(mCblk->flags & CBLK_FORCEREADY_MSK)) {
+ AutoMutex _l(mCblk->lock);
mFillingUpStatus = FS_FILLED;
mCblk->flags &= ~CBLK_FORCEREADY_MSK;
return true;
@@ -3063,19 +3066,18 @@
// STOPPED state
mState = STOPPED;
- mCblk->lock.lock();
// NOTE: reset() will reset cblk->user and cblk->server with
// the risk that at the same time, the AudioMixer is trying to read
// data. In this case, getNextBuffer() would return a NULL pointer
// as audio buffer => the AudioMixer code MUST always test that pointer
// returned by getNextBuffer() is not NULL!
reset();
- mCblk->lock.unlock();
}
}
void AudioFlinger::PlaybackThread::Track::reset()
{
+ AutoMutex _l(mCblk->lock);
// Do not reset twice to avoid discarding data written just after a flush and before
// the audioflinger thread detects the track is stopped.
if (!mResetDone) {
@@ -3209,10 +3211,13 @@
if (thread != 0) {
RecordThread *recordThread = (RecordThread *)thread.get();
recordThread->stop(this);
- TrackBase::reset();
- // Force overerrun condition to avoid false overrun callback until first data is
- // read from buffer
- mCblk->flags |= CBLK_UNDERRUN_ON;
+ {
+ AutoMutex _l(mCblk->lock);
+ TrackBase::reset();
+ // Force overerrun condition to avoid false overrun callback until first data is
+ // read from buffer
+ mCblk->flags |= CBLK_UNDERRUN_ON;
+ }
}
}