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

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

AudioTrackShared: Use atomic min for mUnreleased 

Flag: EXEMPT bugfix
Test: atest CtsMediaAudioTestCases
Bug: 419572928
Change-Id: I5a81ec1c601b85b5106ee66a35fa39394977c03d
parent 06d047f6

include/private/media/AudioTrackShared.h

+7 −1
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ 
#include <stdint.h>

#include <sys/types.h>



#include <audio_utils/atomic.h>

#include <audio_utils/minifloat.h>

#include <utils/threads.h>

#include <utils/Log.h>
@@ -315,7 +316,12 @@ protected: 
    const bool      mIsOut;             // true for AudioTrack, false for AudioRecord

    const bool      mClientInServer;    // true for OutputTrack, false for AudioTrack & AudioRecord

    bool            mIsShutdown;        // latch set to true when shared memory corruption detected

    size_t          mUnreleased;        // unreleased frames remaining from most recent obtainBuffer



    // mUnreleased is the number frames remaining from most recent obtainBuffer(s).

    // Generally accessed by a single thread, but for Java offload,

    // this variable may be accessed from multiple threads.  It is used to

    // bounds check the releaseBuffer call after the obtainBuffer.

    audio_utils::atomic<size_t, audio_utils::memory_order_relaxed>  mUnreleased;

};



// ----------------------------------------------------------------------------

media/libaudioclient/AudioTrackShared.cpp

+4 −4
Original line number Diff line number Diff line
@@ -271,7 +271,7 @@ status_t ClientProxy::obtainBuffer(Buffer* buffer, const struct timespec *reques 
            // take the maximum of the two mUnreleased (consistency check variable)

            // to avoid triggering an assertion on releaseBuffer.

            // TODO(b/419572928) improve this logic.

            mUnreleased = std::max(mUnreleased, part1);

            mUnreleased.max(part1);

            status = NO_ERROR;

            break;

        }
@@ -401,7 +401,7 @@ void ClientProxy::releaseBuffer(Buffer* buffer) 
    LOG_ALWAYS_FATAL_IF(!(stepCount <= mUnreleased && mUnreleased <= mFrameCount),

            "%s: mUnreleased out of range, "

            "!(stepCount:%zu <= mUnreleased:%zu <= mFrameCount:%zu), BufferSizeInFrames:%u",

            __func__, stepCount, mUnreleased, mFrameCount, getBufferSizeInFrames());

            __func__, stepCount, mUnreleased.load(), mFrameCount, getBufferSizeInFrames());

    mUnreleased -= stepCount;

    audio_track_cblk_t* cblk = mCblk;

    // Both of these barriers are required
@@ -922,7 +922,7 @@ void ServerProxy::releaseBuffer(Buffer* buffer) 
    LOG_ALWAYS_FATAL_IF(!(stepCount <= mUnreleased && mUnreleased <= mFrameCount),

            "%s: mUnreleased out of range, "

            "!(stepCount:%zu <= mUnreleased:%zu <= mFrameCount:%zu)",

            __func__, stepCount, mUnreleased, mFrameCount);

            __func__, stepCount, mUnreleased.load(), mFrameCount);

    mUnreleased -= stepCount;

    audio_track_cblk_t* cblk = mCblk;

    if (mIsOut) {
@@ -1235,7 +1235,7 @@ void StaticAudioTrackServerProxy::releaseBuffer(Buffer* buffer) 
    LOG_ALWAYS_FATAL_IF(!(stepCount <= mUnreleased),

            "%s: stepCount out of range, "

            "!(stepCount:%zu <= mUnreleased:%zu)",

            __func__, stepCount, mUnreleased);

            __func__, stepCount, mUnreleased.load());

    if (stepCount == 0) {

        // prevent accidental re-use of buffer

        buffer->mRaw = NULL;