Fix loop and position restoration in static AudioTracks
Allow restoration of loop and position.
Make position and loop synchronously readable.
Bug: 17964637
Change-Id: I8cfb5036e665f55fdff5c67d27e1363ce9a8665d
diff --git a/include/media/AudioTrack.h b/include/media/AudioTrack.h
index da6d4e3..0f9df4f 100644
--- a/include/media/AudioTrack.h
+++ b/include/media/AudioTrack.h
@@ -732,13 +732,16 @@
bool mRefreshRemaining; // processAudioBuffer() should refresh
// mRemainingFrames and mRetryOnPartialBuffer
+ // used for static track cbf and restoration
+ int32_t mLoopCount; // last setLoop loopCount; zero means disabled
+ uint32_t mLoopStart; // last setLoop loopStart
+ uint32_t mLoopEnd; // last setLoop loopEnd
+
// These are private to processAudioBuffer(), and are not protected by a lock
uint32_t mRemainingFrames; // number of frames to request in obtainBuffer()
bool mRetryOnPartialBuffer; // sleep and retry after partial obtainBuffer()
uint32_t mObservedSequence; // last observed value of mSequence
- uint32_t mLoopPeriod; // in frames, zero means looping is disabled
-
uint32_t mMarkerPosition; // in wrapping (overflow) frame units
bool mMarkerReached;
uint32_t mNewPosition; // in frames
diff --git a/include/private/media/AudioTrackShared.h b/include/private/media/AudioTrackShared.h
index 8389773..0d87cd2 100644
--- a/include/private/media/AudioTrackShared.h
+++ b/include/private/media/AudioTrackShared.h
@@ -85,13 +85,32 @@
typedef SingleStateQueue<StaticAudioTrackState> StaticAudioTrackSingleStateQueue;
+struct StaticAudioTrackPosLoop {
+ // Do not define constructors, destructors, or virtual methods as this is part of a
+ // union in shared memory and will not get called properly.
+
+ // These fields should both be size_t, but since they are located in shared memory we
+ // force to 32-bit. The client and server may have different typedefs for size_t.
+
+ // This struct information is stored in a single state queue to communicate the
+ // static AudioTrack server state to the client while data is consumed.
+ // It is smaller than StaticAudioTrackState to prevent unnecessary information from
+ // being sent.
+
+ uint32_t mBufferPosition;
+ int32_t mLoopCount;
+};
+
+typedef SingleStateQueue<StaticAudioTrackPosLoop> StaticAudioTrackPosLoopQueue;
+
struct AudioTrackSharedStatic {
+ // client requests to the server for loop or position changes.
StaticAudioTrackSingleStateQueue::Shared
mSingleStateQueue;
- // This field should be a size_t, but since it is located in shared memory we
- // force to 32-bit. The client and server may have different typedefs for size_t.
- uint32_t mBufferPosition; // updated asynchronously by server,
- // "for entertainment purposes only"
+ // position info updated asynchronously by server and read by client,
+ // "for entertainment purposes only"
+ StaticAudioTrackPosLoopQueue::Shared
+ mPosLoopQueue;
};
// ----------------------------------------------------------------------------
@@ -355,6 +374,9 @@
void setBufferPositionAndLoop(size_t position, size_t loopStart, size_t loopEnd,
int loopCount);
size_t getBufferPosition();
+ // getBufferPositionAndLoopCount() provides the proper snapshot of
+ // position and loopCount together.
+ void getBufferPositionAndLoopCount(size_t *position, int *loopCount);
virtual size_t getMisalignment() {
return 0;
@@ -366,8 +388,9 @@
private:
StaticAudioTrackSingleStateQueue::Mutator mMutator;
+ StaticAudioTrackPosLoopQueue::Observer mPosLoopObserver;
StaticAudioTrackState mState; // last communicated state to server
- size_t mBufferPosition; // so that getBufferPosition() appears to be synchronous
+ StaticAudioTrackPosLoop mPosLoop; // snapshot of position and loop.
};
// ----------------------------------------------------------------------------
@@ -494,8 +517,7 @@
const StaticAudioTrackState &update) const;
ssize_t pollPosition(); // poll for state queue update, and return current position
StaticAudioTrackSingleStateQueue::Observer mObserver;
- size_t mPosition; // server's current play position in frames, relative to 0
-
+ StaticAudioTrackPosLoopQueue::Mutator mPosLoopMutator;
size_t mFramesReadySafe; // Assuming size_t read/writes are atomic on 32 / 64 bit
// processors, this is a thread-safe version of
// mFramesReady.
diff --git a/media/libmedia/AudioTrack.cpp b/media/libmedia/AudioTrack.cpp
index fdc31a1..422ead1 100644
--- a/media/libmedia/AudioTrack.cpp
+++ b/media/libmedia/AudioTrack.cpp
@@ -38,6 +38,11 @@
namespace android {
// ---------------------------------------------------------------------------
+template <typename T>
+const T &min(const T &x, const T &y) {
+ return x < y ? x : y;
+}
+
static int64_t convertTimespecToUs(const struct timespec &tv)
{
return tv.tv_sec * 1000000ll + tv.tv_nsec / 1000;
@@ -420,7 +425,9 @@
mStatus = NO_ERROR;
mState = STATE_STOPPED;
mUserData = user;
- mLoopPeriod = 0;
+ mLoopCount = 0;
+ mLoopStart = 0;
+ mLoopEnd = 0;
mMarkerPosition = 0;
mMarkerReached = false;
mNewPosition = 0;
@@ -534,7 +541,6 @@
if (mSharedBuffer != 0) {
// clear buffer position and loop count.
- mLoopPeriod = 0;
mStaticProxy->setBufferPositionAndLoop(0 /* position */,
0 /* loopStart */, 0 /* loopEnd */, 0 /* loopCount */);
}
@@ -739,9 +745,11 @@
void AudioTrack::setLoop_l(uint32_t loopStart, uint32_t loopEnd, int loopCount)
{
- // Setting the loop will reset next notification update period (like setPosition).
- mNewPosition = updateAndGetPosition_l() + mUpdatePeriod;
- mLoopPeriod = loopCount != 0 ? loopEnd - loopStart : 0;
+ // We do not update the periodic notification point.
+ // mNewPosition = updateAndGetPosition_l() + mUpdatePeriod;
+ mLoopCount = loopCount;
+ mLoopEnd = loopEnd;
+ mLoopStart = loopStart;
mStaticProxy->setLoop(loopStart, loopEnd, loopCount);
}
@@ -889,7 +897,6 @@
return INVALID_OPERATION;
}
mNewPosition = mUpdatePeriod;
- mLoopPeriod = 0;
(void) updateAndGetPosition_l();
mPosition = 0;
// The documentation is not clear on the behavior of reload() and the restoration
@@ -1610,7 +1617,7 @@
}
// Cache other fields that will be needed soon
- uint32_t loopPeriod = mLoopPeriod;
+ uint32_t loopPeriod = mLoopCount != 0 ? mLoopEnd - mLoopStart : 0;
uint32_t sampleRate = mSampleRate;
uint32_t notificationFrames = mNotificationFramesAct;
if (mRefreshRemaining) {
@@ -1856,7 +1863,11 @@
}
// save the old static buffer position
- size_t bufferPosition = mStaticProxy != NULL ? mStaticProxy->getBufferPosition() : 0;
+ size_t bufferPosition = 0;
+ int loopCount = 0;
+ if (mStaticProxy != 0) {
+ mStaticProxy->getBufferPositionAndLoopCount(&bufferPosition, &loopCount);
+ }
// If a new IAudioTrack is successfully created, createTrack_l() will modify the
// following member variables: mAudioTrack, mCblkMemory and mCblk.
@@ -1873,27 +1884,15 @@
}
if (result == NO_ERROR) {
- // continue playback from last known position, but
- // don't attempt to restore loop after invalidation; it's difficult and not worthwhile
- if (mStaticProxy != NULL) {
- mLoopPeriod = 0;
- mStaticProxy->setBufferPositionAndLoop(bufferPosition,
- 0 /* loopStart */, 0 /* loopEnd */, 0 /* loopCount */);
- }
- // FIXME How do we simulate the fact that all frames present in the buffer at the time of
- // track destruction have been played? This is critical for SoundPool implementation
- // This must be broken, and needs to be tested/debugged.
-#if 0
- // restore write index and set other indexes to reflect empty buffer status
- if (!strcmp(from, "start")) {
- // Make sure that a client relying on callback events indicating underrun or
- // the actual amount of audio frames played (e.g SoundPool) receives them.
- if (mSharedBuffer == 0) {
- // restart playback even if buffer is not completely filled.
- android_atomic_or(CBLK_FORCEREADY, &mCblk->mFlags);
+ // Continue playback from last known position and restore loop.
+ if (mStaticProxy != 0) {
+ if (loopCount != 0) {
+ mStaticProxy->setBufferPositionAndLoop(bufferPosition,
+ mLoopStart, mLoopEnd, loopCount);
+ } else {
+ mStaticProxy->setBufferPosition(bufferPosition);
}
}
-#endif
if (mState == STATE_ACTIVE) {
result = mAudioTrack->start();
}
diff --git a/media/libmedia/AudioTrackShared.cpp b/media/libmedia/AudioTrackShared.cpp
index c44cdee..08241e2 100644
--- a/media/libmedia/AudioTrackShared.cpp
+++ b/media/libmedia/AudioTrackShared.cpp
@@ -499,9 +499,11 @@
StaticAudioTrackClientProxy::StaticAudioTrackClientProxy(audio_track_cblk_t* cblk, void *buffers,
size_t frameCount, size_t frameSize)
: AudioTrackClientProxy(cblk, buffers, frameCount, frameSize),
- mMutator(&cblk->u.mStatic.mSingleStateQueue), mBufferPosition(0)
+ mMutator(&cblk->u.mStatic.mSingleStateQueue),
+ mPosLoopObserver(&cblk->u.mStatic.mPosLoopQueue)
{
memset(&mState, 0, sizeof(mState));
+ memset(&mPosLoop, 0, sizeof(mPosLoop));
}
void StaticAudioTrackClientProxy::flush()
@@ -523,11 +525,12 @@
// set patch-up variables until the mState is acknowledged by the ServerProxy.
// observed buffer position and loop count will freeze until then to give the
// illusion of a synchronous change.
- size_t bufferPosition = getBufferPosition();
+ getBufferPositionAndLoopCount(NULL, NULL);
// preserve behavior to restart at mState.mLoopStart if position exceeds mState.mLoopEnd.
- if (loopCount != 0 && bufferPosition >= mState.mLoopEnd) {
- mBufferPosition = mState.mLoopStart;
+ if (mState.mLoopCount != 0 && mPosLoop.mBufferPosition >= mState.mLoopEnd) {
+ mPosLoop.mBufferPosition = mState.mLoopStart;
}
+ mPosLoop.mLoopCount = mState.mLoopCount;
(void) mMutator.push(mState);
}
@@ -540,7 +543,17 @@
}
mState.mPosition = (uint32_t) position;
mState.mPositionSequence = incrementSequence(mState.mPositionSequence, mState.mLoopSequence);
- mBufferPosition = position;
+ // set patch-up variables until the mState is acknowledged by the ServerProxy.
+ // observed buffer position and loop count will freeze until then to give the
+ // illusion of a synchronous change.
+ if (mState.mLoopCount > 0) { // only check if loop count is changing
+ getBufferPositionAndLoopCount(NULL, NULL); // get last position
+ }
+ mPosLoop.mBufferPosition = position;
+ if (position >= mState.mLoopEnd) {
+ // no ongoing loop is possible if position is greater than loopEnd.
+ mPosLoop.mLoopCount = 0;
+ }
(void) mMutator.push(mState);
}
@@ -553,18 +566,24 @@
size_t StaticAudioTrackClientProxy::getBufferPosition()
{
- size_t bufferPosition;
- if (mMutator.ack()) {
- // There is a race condition here as ack may be signaled before
- // the buffer position in mCblk is updated. Will be fixed in a later CL.
- bufferPosition = (size_t) mCblk->u.mStatic.mBufferPosition;
- if (bufferPosition > mFrameCount) {
- bufferPosition = mFrameCount;
- }
- } else {
- bufferPosition = mBufferPosition;
+ getBufferPositionAndLoopCount(NULL, NULL);
+ return mPosLoop.mBufferPosition;
+}
+
+void StaticAudioTrackClientProxy::getBufferPositionAndLoopCount(
+ size_t *position, int *loopCount)
+{
+ if (mMutator.ack() == StaticAudioTrackSingleStateQueue::SSQ_DONE) {
+ if (mPosLoopObserver.poll(mPosLoop)) {
+ ; // a valid mPosLoop should be available if ackDone is true.
+ }
}
- return bufferPosition;
+ if (position != NULL) {
+ *position = mPosLoop.mBufferPosition;
+ }
+ if (loopCount != NULL) {
+ *loopCount = mPosLoop.mLoopCount;
+ }
}
// ---------------------------------------------------------------------------
@@ -780,7 +799,8 @@
StaticAudioTrackServerProxy::StaticAudioTrackServerProxy(audio_track_cblk_t* cblk, void *buffers,
size_t frameCount, size_t frameSize)
: AudioTrackServerProxy(cblk, buffers, frameCount, frameSize),
- mObserver(&cblk->u.mStatic.mSingleStateQueue), mPosition(0),
+ mObserver(&cblk->u.mStatic.mSingleStateQueue),
+ mPosLoopMutator(&cblk->u.mStatic.mPosLoopQueue),
mFramesReadySafe(frameCount), mFramesReady(frameCount),
mFramesReadyIsCalledByMultipleThreads(false)
{
@@ -865,6 +885,7 @@
updateStateWithLoop(&trystate, state) == OK;
}
if (!result) {
+ mObserver.done();
// caution: no update occurs so server state will be inconsistent with client state.
ALOGE("%s client pushed an invalid state, shutting down", __func__);
mIsShutdown = true;
@@ -883,7 +904,12 @@
}
mFramesReadySafe = clampToSize(mFramesReady);
// This may overflow, but client is not supposed to rely on it
- mCblk->u.mStatic.mBufferPosition = (uint32_t) mState.mPosition;
+ StaticAudioTrackPosLoop posLoop;
+
+ posLoop.mLoopCount = (int32_t) mState.mLoopCount;
+ posLoop.mBufferPosition = (uint32_t) mState.mPosition;
+ mPosLoopMutator.push(posLoop);
+ mObserver.done(); // safe to read mStatic variables.
}
return (ssize_t) mState.mPosition;
}
@@ -969,7 +995,10 @@
cblk->mServer += stepCount;
// This may overflow, but client is not supposed to rely on it
- cblk->u.mStatic.mBufferPosition = (uint32_t) mState.mPosition;
+ StaticAudioTrackPosLoop posLoop;
+ posLoop.mBufferPosition = mState.mPosition;
+ posLoop.mLoopCount = mState.mLoopCount;
+ mPosLoopMutator.push(posLoop);
if (setFlags != 0) {
(void) android_atomic_or(setFlags, &cblk->mFlags);
// this would be a good place to wake a futex