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

Commit cb2129b3 authored by Andy Hung's avatar Andy Hung
Browse files

Fix missing loop count for static tracks. 

StaticAudioTrackServerProxy::framesReady() previously returned
only the contiguous frames, update to return the total
available frames. This resolves short-count looping in
SoundPool for FastTracks.

Also (1) Removes the racy condition of reading two variables
and (2) Fixes buffer->mNonContig to return the correct value
and (3) Restores behavior that loop count of 1 goes back to
loopStart once during playback.

Bug: 11830751
Bug: 12070295
Bug: 17456842
Change-Id: I64906e6036bb00a1d7375b03efe6deb69d6478ca
parent 97e6ca1a

include/private/media/AudioTrackShared.h

+8 −1
Original line number Diff line number Diff line
@@ -450,7 +450,14 @@ private: 
    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

    size_t              mEnd;       // cached value computed from mState, safe for asynchronous read



    size_t              mFramesReadySafe; // Assuming size_t read/writes are atomic on 32 / 64 bit

                                          // processors, this is a thread-safe version of

                                          // mFramesReady.

    int64_t             mFramesReady;     // The number of frames ready in the static buffer

                                          // including loops.  This is 64 bits since loop mode

                                          // can cause a track to appear to have a large number

                                          // of frames. INT64_MAX means an infinite loop.

    bool                mFramesReadyIsCalledByMultipleThreads;

    StaticAudioTrackState   mState;

};

media/libmedia/AudioTrackShared.cpp

+38 −22
Original line number Diff line number Diff line
@@ -25,6 +25,12 @@ 


namespace android {



// used to clamp a value to size_t.  TODO: move to another file.

template <typename T>

size_t clampToSize(T x) {

    return x > SIZE_MAX ? SIZE_MAX : x < 0 ? 0 : (size_t) x;

}



audio_track_cblk_t::audio_track_cblk_t()

    : mServer(0), mFutex(0), mMinimum(0),

    mVolumeLR(GAIN_MINIFLOAT_PACKED_UNITY), mSampleRate(0), mSendLevel(0), mFlags(0)
@@ -728,7 +734,8 @@ StaticAudioTrackServerProxy::StaticAudioTrackServerProxy(audio_track_cblk_t* cbl 
        size_t frameCount, size_t frameSize)

    : AudioTrackServerProxy(cblk, buffers, frameCount, frameSize),

      mObserver(&cblk->u.mStatic.mSingleStateQueue), mPosition(0),

      mEnd(frameCount), mFramesReadyIsCalledByMultipleThreads(false)

      mFramesReadySafe(frameCount), mFramesReady(frameCount),

      mFramesReadyIsCalledByMultipleThreads(false)

{

    mState.mLoopStart = 0;

    mState.mLoopEnd = 0;
@@ -742,20 +749,11 @@ void StaticAudioTrackServerProxy::framesReadyIsCalledByMultipleThreads() 


size_t StaticAudioTrackServerProxy::framesReady()

{

    // FIXME

    // This is racy if called by normal mixer thread,

    // as we're reading 2 independent variables without a lock.

    // Can't call mObserver.poll(), as we might be called from wrong thread.

    // If looping is enabled, should return a higher number (since includes non-contiguous).

    size_t position = mPosition;

    // Can't call pollPosition() from multiple threads.

    if (!mFramesReadyIsCalledByMultipleThreads) {

        ssize_t positionOrStatus = pollPosition();

        if (positionOrStatus >= 0) {

            position = (size_t) positionOrStatus;

        }

        (void) pollPosition();

    }

    size_t end = mEnd;

    return position < end ? end - position : 0;

    return mFramesReadySafe;

}



ssize_t StaticAudioTrackServerProxy::pollPosition()
@@ -772,25 +770,35 @@ ssize_t StaticAudioTrackServerProxy::pollPosition() 
            }

            // ignore loopEnd

            mPosition = position = loopStart;

            mEnd = mFrameCount;

            mFramesReady = mFrameCount - mPosition;

            mState.mLoopCount = 0;

            valid = true;

        } else {

        } else if (state.mLoopCount >= -1) {

            if (loopStart < loopEnd && loopEnd <= mFrameCount &&

                    loopEnd - loopStart >= MIN_LOOP) {

                if (!(loopStart <= position && position < loopEnd)) {

                    mPosition = position = loopStart;

                }

                mEnd = loopEnd;

                if (state.mLoopCount == -1) {

                    mFramesReady = INT64_MAX;

                } else {

                    // mFramesReady is 64 bits to handle the effective number of frames

                    // that the static audio track contains, including loops.

                    // TODO: Later consider fixing overflow, but does not seem needed now

                    // as will not overflow if loopStart and loopEnd are Java "ints".

                    mFramesReady = int64_t(state.mLoopCount) * (loopEnd - loopStart)

                            + mFrameCount - mPosition;

                }

                mState = state;

                valid = true;

            }

        }

        if (!valid) {

        if (!valid || mPosition > mFrameCount) {

            ALOGE("%s client pushed an invalid state, shutting down", __func__);

            mIsShutdown = true;

            return (ssize_t) NO_INIT;

        }

        mFramesReadySafe = clampToSize(mFramesReady);

        // This may overflow, but client is not supposed to rely on it

        mCblk->u.mStatic.mBufferPosition = (uint32_t) position;

    }
@@ -815,9 +823,10 @@ status_t StaticAudioTrackServerProxy::obtainBuffer(Buffer* buffer, bool ackFlush 
        return (status_t) positionOrStatus;

    }

    size_t position = (size_t) positionOrStatus;

    size_t end = mState.mLoopCount != 0 ? mState.mLoopEnd : mFrameCount;

    size_t avail;

    if (position < mEnd) {

        avail = mEnd - position;

    if (position < end) {

        avail = end - position;

        size_t wanted = buffer->mFrameCount;

        if (avail < wanted) {

            buffer->mFrameCount = avail;
@@ -830,7 +839,10 @@ status_t StaticAudioTrackServerProxy::obtainBuffer(Buffer* buffer, bool ackFlush 
        buffer->mFrameCount = 0;

        buffer->mRaw = NULL;

    }

    buffer->mNonContig = 0;     // FIXME should be > 0 for looping

    // As mFramesReady is the total remaining frames in the static audio track,

    // it is always larger or equal to avail.

    LOG_ALWAYS_FATAL_IF(mFramesReady < avail);

    buffer->mNonContig = mFramesReady == INT64_MAX ? SIZE_MAX : clampToSize(mFramesReady - avail);

    mUnreleased = avail;

    return NO_ERROR;

}
@@ -838,6 +850,7 @@ status_t StaticAudioTrackServerProxy::obtainBuffer(Buffer* buffer, bool ackFlush 
void StaticAudioTrackServerProxy::releaseBuffer(Buffer* buffer)

{

    size_t stepCount = buffer->mFrameCount;

    LOG_ALWAYS_FATAL_IF(!(stepCount <= mFramesReady));

    LOG_ALWAYS_FATAL_IF(!(stepCount <= mUnreleased));

    if (stepCount == 0) {

        // prevent accidental re-use of buffer
@@ -854,11 +867,10 @@ void StaticAudioTrackServerProxy::releaseBuffer(Buffer* buffer) 
        ALOGW("%s newPosition %zu outside [%zu, %zu]", __func__, newPosition, position, mFrameCount);

        newPosition = mFrameCount;

    } else if (mState.mLoopCount != 0 && newPosition == mState.mLoopEnd) {

        if (mState.mLoopCount == -1 || --mState.mLoopCount != 0) {

        newPosition = mState.mLoopStart;

        if (mState.mLoopCount == -1 || --mState.mLoopCount != 0) {

            setFlags = CBLK_LOOP_CYCLE;

        } else {

            mEnd = mFrameCount;     // this is what allows playback to continue after the loop

            setFlags = CBLK_LOOP_FINAL;

        }

    }
@@ -866,6 +878,10 @@ void StaticAudioTrackServerProxy::releaseBuffer(Buffer* buffer) 
        setFlags |= CBLK_BUFFER_END;

    }

    mPosition = newPosition;

    if (mFramesReady != INT64_MAX) {

        mFramesReady -= stepCount;

    }

    mFramesReadySafe = clampToSize(mFramesReady);



    cblk->mServer += stepCount;

    // This may overflow, but client is not supposed to rely on it