audio: Fix race condition in AudioTrack underrun.
When audio flinger mixer removes an AudioTrack from the
active list in case of underrun, it is possible that the
client has written a full buffer just after the underrun detection and
is blocked waiting for more space to write. In this case, the client
will never detect the DISABLED flag and the track never be restarted.
Also implement missing DISABLE flag detection in server side audio tracks
(OutputTrack and PatchTrack).
bug: 27567768
Change-Id: I8d0753429d4113498258b1f61bd8ac5939a612f0
diff --git a/include/media/AudioTrack.h b/include/media/AudioTrack.h
index 69dc062..4de55a2 100644
--- a/include/media/AudioTrack.h
+++ b/include/media/AudioTrack.h
@@ -836,6 +836,8 @@
// check sample rate and speed is compatible with AudioTrack
bool isSampleRateSpeedAllowed_l(uint32_t sampleRate, float speed) const;
+ void restartIfDisabled();
+
// Next 4 fields may be changed if IAudioTrack is re-created, but always != 0
sp<IAudioTrack> mAudioTrack;
sp<IMemory> mCblkMemory;
diff --git a/include/private/media/AudioTrackShared.h b/include/private/media/AudioTrackShared.h
index ea8a78e..2dfb850 100644
--- a/include/private/media/AudioTrackShared.h
+++ b/include/private/media/AudioTrackShared.h
@@ -268,6 +268,8 @@
// DEAD_OBJECT Server has died or invalidated, caller should destroy this proxy and re-create.
// -EINTR Call has been interrupted. Look around to see why, and then perhaps try again.
// NO_INIT Shared memory is corrupt.
+ // NOT_ENOUGH_DATA Server has disabled the track because of underrun: restart the track
+ // if still in active state.
// Assertion failure on entry, if buffer == NULL or buffer->mFrameCount == 0.
status_t obtainBuffer(Buffer* buffer, const struct timespec *requested = NULL,
struct timespec *elapsed = NULL);
diff --git a/media/libmedia/AudioTrack.cpp b/media/libmedia/AudioTrack.cpp
index 423273d..ab18c27 100644
--- a/media/libmedia/AudioTrack.cpp
+++ b/media/libmedia/AudioTrack.cpp
@@ -1532,6 +1532,10 @@
}
oldSequence = newSequence;
+ if (status == NOT_ENOUGH_DATA) {
+ restartIfDisabled();
+ }
+
// Keep the extra references
proxy = mProxy;
iMem = mCblkMemory;
@@ -1554,8 +1558,7 @@
buffer.mFrameCount = audioBuffer->frameCount;
// FIXME starts the requested timeout and elapsed over from scratch
status = proxy->obtainBuffer(&buffer, requested, elapsed);
-
- } while ((status == DEAD_OBJECT) && (tryCounter-- > 0));
+ } while (((status == DEAD_OBJECT) || (status == NOT_ENOUGH_DATA)) && (tryCounter-- > 0));
audioBuffer->frameCount = buffer.mFrameCount;
audioBuffer->size = buffer.mFrameCount * mFrameSize;
@@ -1588,13 +1591,16 @@
mProxy->releaseBuffer(&buffer);
// restart track if it was disabled by audioflinger due to previous underrun
- if (mState == STATE_ACTIVE) {
- audio_track_cblk_t* cblk = mCblk;
- if (android_atomic_and(~CBLK_DISABLED, &cblk->mFlags) & CBLK_DISABLED) {
- ALOGW("releaseBuffer() track %p disabled due to previous underrun, restarting", this);
- // FIXME ignoring status
- mAudioTrack->start();
- }
+ restartIfDisabled();
+}
+
+void AudioTrack::restartIfDisabled()
+{
+ int32_t flags = android_atomic_and(~CBLK_DISABLED, &mCblk->mFlags);
+ if ((mState == STATE_ACTIVE) && (flags & CBLK_DISABLED)) {
+ ALOGW("releaseBuffer() track %p disabled due to previous underrun, restarting", this);
+ // FIXME ignoring status
+ mAudioTrack->start();
}
}
diff --git a/media/libmedia/AudioTrackShared.cpp b/media/libmedia/AudioTrackShared.cpp
index 1d15495..6b6865b 100644
--- a/media/libmedia/AudioTrackShared.cpp
+++ b/media/libmedia/AudioTrackShared.cpp
@@ -129,6 +129,11 @@
status = DEAD_OBJECT;
goto end;
}
+ if (flags & CBLK_DISABLED) {
+ ALOGV("Track disabled");
+ status = NOT_ENOUGH_DATA;
+ goto end;
+ }
// check for obtainBuffer interrupted by client
if (!ignoreInitialPendingInterrupt && (flags & CBLK_INTERRUPT)) {
ALOGV("obtainBuffer() interrupted by client");
@@ -425,7 +430,8 @@
status = DEAD_OBJECT;
goto end;
}
- if (flags & CBLK_STREAM_END_DONE) {
+ // a track is not supposed to underrun at this stage but consider it done
+ if (flags & (CBLK_STREAM_END_DONE | CBLK_DISABLED)) {
ALOGV("stream end received");
status = NO_ERROR;
goto end;
diff --git a/services/audioflinger/PlaybackTracks.h b/services/audioflinger/PlaybackTracks.h
index fa61af2..6e0c46d 100644
--- a/services/audioflinger/PlaybackTracks.h
+++ b/services/audioflinger/PlaybackTracks.h
@@ -110,10 +110,13 @@
// audioHalFrames is derived from output latency
// FIXME parameters not needed, could get them from the thread
bool presentationComplete(int64_t framesWritten, size_t audioHalFrames);
+ void signalClientFlag(int32_t flag);
public:
void triggerEvents(AudioSystem::sync_event_t type);
void invalidate();
+ void disable();
+
bool isInvalid() const { return mIsInvalid; }
int fastIndex() const { return mFastIndex; }
@@ -200,6 +203,8 @@
uint32_t waitTimeMs);
void clearBufferQueue();
+ void restartIfDisabled();
+
// Maximum number of pending buffers allocated by OutputTrack::write()
static const uint8_t kMaxOverFlowBuffers = 10;
@@ -224,6 +229,10 @@
IAudioFlinger::track_flags_t flags);
virtual ~PatchTrack();
+ virtual status_t start(AudioSystem::sync_event_t event =
+ AudioSystem::SYNC_EVENT_NONE,
+ int triggerSession = 0);
+
// AudioBufferProvider interface
virtual status_t getNextBuffer(AudioBufferProvider::Buffer* buffer);
virtual void releaseBuffer(AudioBufferProvider::Buffer* buffer);
@@ -236,6 +245,8 @@
void setPeerProxy(PatchProxyBufferProvider *proxy) { mPeerProxy = proxy; }
private:
+ void restartIfDisabled();
+
sp<ClientProxy> mProxy;
PatchProxyBufferProvider* mPeerProxy;
struct timespec mPeerTimeout;
diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp
index 96c5f59..f206e96 100644
--- a/services/audioflinger/Threads.cpp
+++ b/services/audioflinger/Threads.cpp
@@ -1113,7 +1113,7 @@
status_t status;
status = mPowerManager->updateWakeLockUids(mWakeLockToken, uids.size(), uids.array(),
true /* FIXME force oneway contrary to .aidl */);
- ALOGV("acquireWakeLock_l() %s status %d", mThreadName, status);
+ ALOGV("updateWakeLockUids_l() %s status %d", mThreadName, status);
}
}
@@ -3961,7 +3961,7 @@
}
// indicate to client process that the track was disabled because of underrun;
// it will then automatically call start() when data is available
- android_atomic_or(CBLK_DISABLED, &track->mCblk->mFlags);
+ track->disable();
// remove from active list, but state remains ACTIVE [confusing but true]
isActive = false;
break;
@@ -4322,7 +4322,7 @@
tracksToRemove->add(track);
// indicate to client process that the track was disabled because of underrun;
// it will then automatically call start() when data is available
- android_atomic_or(CBLK_DISABLED, &cblk->mFlags);
+ track->disable();
// If one track is not ready, mark the mixer also not ready if:
// - the mixer was ready during previous round OR
// - no other track is ready
@@ -4869,7 +4869,7 @@
tracksToRemove->add(track);
// indicate to client process that the track was disabled because of underrun;
// it will then automatically call start() when data is available
- android_atomic_or(CBLK_DISABLED, &cblk->mFlags);
+ track->disable();
} else if (last) {
ALOGW("pause because of UNDERRUN, framesReady = %zu,"
"minFrames = %u, mFormat = %#x",
@@ -5423,7 +5423,7 @@
tracksToRemove->add(track);
// indicate to client process that the track was disabled because of underrun;
// it will then automatically call start() when data is available
- android_atomic_or(CBLK_DISABLED, &cblk->mFlags);
+ track->disable();
} else if (last){
mixerStatus = MIXER_TRACKS_ENABLED;
}
diff --git a/services/audioflinger/Tracks.cpp b/services/audioflinger/Tracks.cpp
index 8b49062..0c51e81 100644
--- a/services/audioflinger/Tracks.cpp
+++ b/services/audioflinger/Tracks.cpp
@@ -1027,13 +1027,23 @@
void AudioFlinger::PlaybackThread::Track::invalidate()
{
+ signalClientFlag(CBLK_INVALID);
+ mIsInvalid = true;
+}
+
+void AudioFlinger::PlaybackThread::Track::disable()
+{
+ signalClientFlag(CBLK_DISABLED);
+}
+
+void AudioFlinger::PlaybackThread::Track::signalClientFlag(int32_t flag)
+{
// FIXME should use proxy, and needs work
audio_track_cblk_t* cblk = mCblk;
- android_atomic_or(CBLK_INVALID, &cblk->mFlags);
+ android_atomic_or(flag, &cblk->mFlags);
android_atomic_release_store(0x40000000, &cblk->mFutex);
// client is not in server, so FUTEX_WAKE is needed instead of FUTEX_WAKE_PRIVATE
(void) syscall(__NR_futex, &cblk->mFutex, FUTEX_WAKE, INT_MAX);
- mIsInvalid = true;
}
void AudioFlinger::PlaybackThread::Track::signal()
@@ -1199,7 +1209,7 @@
mOutBuffer.frameCount = pInBuffer->frameCount;
nsecs_t startTime = systemTime();
status_t status = obtainBuffer(&mOutBuffer, waitTimeLeftMs);
- if (status != NO_ERROR) {
+ if (status != NO_ERROR && status != NOT_ENOUGH_DATA) {
ALOGV("OutputTrack::write() %p thread %p no more output buffers; status %d", this,
mThread.unsafe_get(), status);
outputBufferFull = true;
@@ -1211,6 +1221,10 @@
} else {
waitTimeLeftMs = 0;
}
+ if (status == NOT_ENOUGH_DATA) {
+ restartIfDisabled();
+ continue;
+ }
}
uint32_t outFrames = pInBuffer->frameCount > mOutBuffer.frameCount ? mOutBuffer.frameCount :
@@ -1220,6 +1234,7 @@
buf.mFrameCount = outFrames;
buf.mRaw = NULL;
mClientProxy->releaseBuffer(&buf);
+ restartIfDisabled();
pInBuffer->frameCount -= outFrames;
pInBuffer->raw = (int8_t *)pInBuffer->raw + outFrames * mFrameSize;
mOutBuffer.frameCount -= outFrames;
@@ -1293,6 +1308,13 @@
mBufferQueue.clear();
}
+void AudioFlinger::PlaybackThread::OutputTrack::restartIfDisabled()
+{
+ int32_t flags = android_atomic_and(~CBLK_DISABLED, &mCblk->mFlags);
+ if (mActive && (flags & CBLK_DISABLED)) {
+ start();
+ }
+}
AudioFlinger::PlaybackThread::PatchTrack::PatchTrack(PlaybackThread *playbackThread,
audio_stream_type_t streamType,
@@ -1322,6 +1344,17 @@
{
}
+status_t AudioFlinger::PlaybackThread::PatchTrack::start(AudioSystem::sync_event_t event,
+ int triggerSession)
+{
+ status_t status = Track::start(event, triggerSession);
+ if (status != NO_ERROR) {
+ return status;
+ }
+ android_atomic_and(~CBLK_DISABLED, &mCblk->mFlags);
+ return status;
+}
+
// AudioBufferProvider interface
status_t AudioFlinger::PlaybackThread::PatchTrack::getNextBuffer(
AudioBufferProvider::Buffer* buffer)
@@ -1352,17 +1385,31 @@
status_t AudioFlinger::PlaybackThread::PatchTrack::obtainBuffer(Proxy::Buffer* buffer,
const struct timespec *timeOut)
{
- return mProxy->obtainBuffer(buffer, timeOut);
+ status_t status = NO_ERROR;
+ static const int32_t kMaxTries = 5;
+ int32_t tryCounter = kMaxTries;
+ do {
+ if (status == NOT_ENOUGH_DATA) {
+ restartIfDisabled();
+ }
+ status = mProxy->obtainBuffer(buffer, timeOut);
+ } while ((status == NOT_ENOUGH_DATA) && (tryCounter-- > 0));
+ return status;
}
void AudioFlinger::PlaybackThread::PatchTrack::releaseBuffer(Proxy::Buffer* buffer)
{
mProxy->releaseBuffer(buffer);
+ restartIfDisabled();
+ android_atomic_or(CBLK_FORCEREADY, &mCblk->mFlags);
+}
+
+void AudioFlinger::PlaybackThread::PatchTrack::restartIfDisabled()
+{
if (android_atomic_and(~CBLK_DISABLED, &mCblk->mFlags) & CBLK_DISABLED) {
ALOGW("PatchTrack::releaseBuffer() disabled due to previous underrun, restarting");
start();
}
- android_atomic_or(CBLK_FORCEREADY, &mCblk->mFlags);
}
// ----------------------------------------------------------------------------
