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/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);
}
// ----------------------------------------------------------------------------