diff options
author | Eric Laurent <elaurent@google.com> | 2011-03-17 09:36:51 -0700 |
---|---|---|
committer | Eric Laurent <elaurent@google.com> | 2011-03-18 08:56:45 -0700 |
commit | 33797ea64d067dfeaacbfd7ebe7f3383b73961b5 (patch) | |
tree | 03aad0239e15dbb12e56c29f4d886e72e111ee2f | |
parent | 00d48b9495265457dfb265e766296212b5447b0e (diff) | |
download | frameworks_av-33797ea64d067dfeaacbfd7ebe7f3383b73961b5.zip frameworks_av-33797ea64d067dfeaacbfd7ebe7f3383b73961b5.tar.gz frameworks_av-33797ea64d067dfeaacbfd7ebe7f3383b73961b5.tar.bz2 |
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
-rw-r--r-- | media/libmedia/AudioRecord.cpp | 5 | ||||
-rw-r--r-- | media/libmedia/AudioTrack.cpp | 30 | ||||
-rw-r--r-- | services/audioflinger/AudioFlinger.cpp | 23 |
3 files changed, 40 insertions, 18 deletions
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 @@ bool AudioRecord::processAudioBuffer(const sp<ClientRecordThread>& thread) // 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 @@ void AudioTrack::start() 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 @@ void AudioTrack::start() } 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 @@ create_new_track: // 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 AudioTrack::write(const void* buffer, size_t userSize) 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 @@ ssize_t AudioTrack::write(const void* buffer, size_t userSize) written += toWrite; releaseBuffer(&audioBuffer); - } while (userSize); + } while (userSize >= frameSz); return written; } @@ -1015,12 +1021,15 @@ bool AudioTrack::processAudioBuffer(const sp<AudioTrackThread>& thread) // 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 @@ uint32_t audio_track_cblk_t::stepUser(uint32_t frameCount) 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 @@ uint32_t AudioFlinger::MixerThread::prepareTracks_l(const SortedVector< wp<Track 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 @@ void AudioFlinger::MixerThread::invalidateTracks(int streamType) 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 @@ bool AudioFlinger::PlaybackThread::Track::isReady() const { 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 @@ void AudioFlinger::PlaybackThread::Track::flush() // 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 @@ void AudioFlinger::RecordThread::RecordTrack::stop() 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; + } } } |