Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 4ede21d9 authored by Andy Hung's avatar Andy Hung
Browse files

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
parent 9b461588
Loading
Loading
Loading
Loading
+5 −2
Original line number Diff line number Diff line
@@ -732,13 +732,16 @@ protected:
    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
+29 −7
Original line number Diff line number Diff line
@@ -85,13 +85,32 @@ struct StaticAudioTrackState {

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,
    // position info updated asynchronously by server and read by client,
    // "for entertainment purposes only"
    StaticAudioTrackPosLoopQueue::Shared
                    mPosLoopQueue;
};

// ----------------------------------------------------------------------------
@@ -355,6 +374,9 @@ public:
            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 @@ public:

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 @@ private:
                                                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.
+26 −27
Original line number Diff line number Diff line
@@ -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 @@ status_t AudioTrack::set(
    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 @@ void AudioTrack::stop()

    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 @@ status_t AudioTrack::setLoop(uint32_t loopStart, uint32_t loopEnd, int loopCount

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 @@ status_t AudioTrack::reload()
        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 @@ nsecs_t AudioTrack::processAudioBuffer()
    }

    // 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 @@ status_t AudioTrack::restoreTrack_l(const char *from)
    }

    // 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 @@ status_t AudioTrack::restoreTrack_l(const char *from)
    }

    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;
        // Continue playback from last known position and restore loop.
        if (mStaticProxy != 0) {
            if (loopCount != 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);
                        mLoopStart, mLoopEnd, loopCount);
            } else {
                mStaticProxy->setBufferPosition(bufferPosition);
            }
        }
#endif
        if (mState == STATE_ACTIVE) {
            result = mAudioTrack->start();
        }
+48 −19
Original line number Diff line number Diff line
@@ -499,9 +499,11 @@ end:
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 @@ void StaticAudioTrackClientProxy::setLoop(size_t loopStart, size_t loopEnd, int
    // 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 @@ void StaticAudioTrackClientProxy::setBufferPosition(size_t position)
    }
    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 @@ void StaticAudioTrackClientProxy::setBufferPositionAndLoop(size_t position, size

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;
    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.
         }
    } else {
        bufferPosition = mBufferPosition;
    }
    return bufferPosition;
    if (position != NULL) {
        *position = mPosLoop.mBufferPosition;
    }
    if (loopCount != NULL) {
        *loopCount = mPosLoop.mLoopCount;
    }
}

// ---------------------------------------------------------------------------
@@ -780,7 +799,8 @@ void AudioTrackServerProxy::tallyUnderrunFrames(uint32_t frameCount)
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 @@ ssize_t StaticAudioTrackServerProxy::pollPosition()
                    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 @@ ssize_t StaticAudioTrackServerProxy::pollPosition()
        }
        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 @@ void StaticAudioTrackServerProxy::releaseBuffer(Buffer* buffer)

    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