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

Commit 4d231dc0 authored by Eric Laurent's avatar Eric Laurent
Browse files

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
parent f56a02a5

include/media/AudioTrack.h

+2 −0
Original line number Diff line number Diff line
@@ -836,6 +836,8 @@ protected: 
            // check sample rate and speed is compatible with AudioTrack

            bool     isSampleRateSpeedAllowed_l(uint32_t sampleRate, float speed) const;



            void     restartIfDisabled();



    // Next 4 fields may be changed if IAudioTrack is re-created, but always != 0

    sp<IAudioTrack>         mAudioTrack;

    sp<IMemory>             mCblkMemory;

include/private/media/AudioTrackShared.h

+2 −0
Original line number Diff line number Diff line
@@ -268,6 +268,8 @@ public: 
    //  DEAD_OBJECT Server has died or invalidated, caller should destroy this proxy and re-create.

    //  -EINTR      Call has been interrupted.  Look around to see why, and then perhaps try again.

    //  NO_INIT     Shared memory is corrupt.

    //  NOT_ENOUGH_DATA Server has disabled the track because of underrun: restart the track

    //              if still in active state.

    // Assertion failure on entry, if buffer == NULL or buffer->mFrameCount == 0.

    status_t    obtainBuffer(Buffer* buffer, const struct timespec *requested = NULL,

            struct timespec *elapsed = NULL);

media/libmedia/AudioTrack.cpp

+15 −9
Original line number Diff line number Diff line
@@ -1532,6 +1532,10 @@ status_t AudioTrack::obtainBuffer(Buffer* audioBuffer, const struct timespec *re 
            }

            oldSequence = newSequence;



            if (status == NOT_ENOUGH_DATA) {

                restartIfDisabled();

            }



            // Keep the extra references

            proxy = mProxy;

            iMem = mCblkMemory;
@@ -1554,8 +1558,7 @@ status_t AudioTrack::obtainBuffer(Buffer* audioBuffer, const struct timespec *re 
        buffer.mFrameCount = audioBuffer->frameCount;

        // FIXME starts the requested timeout and elapsed over from scratch

        status = proxy->obtainBuffer(&buffer, requested, elapsed);



    } while ((status == DEAD_OBJECT) && (tryCounter-- > 0));

    } while (((status == DEAD_OBJECT) || (status == NOT_ENOUGH_DATA)) && (tryCounter-- > 0));



    audioBuffer->frameCount = buffer.mFrameCount;

    audioBuffer->size = buffer.mFrameCount * mFrameSize;
@@ -1588,15 +1591,18 @@ void AudioTrack::releaseBuffer(const Buffer* audioBuffer) 
    mProxy->releaseBuffer(&buffer);



    // restart track if it was disabled by audioflinger due to previous underrun

    if (mState == STATE_ACTIVE) {

        audio_track_cblk_t* cblk = mCblk;

        if (android_atomic_and(~CBLK_DISABLED, &cblk->mFlags) & CBLK_DISABLED) {

    restartIfDisabled();

}



void AudioTrack::restartIfDisabled()

{

    int32_t flags = android_atomic_and(~CBLK_DISABLED, &mCblk->mFlags);

    if ((mState == STATE_ACTIVE) && (flags & CBLK_DISABLED)) {

        ALOGW("releaseBuffer() track %p disabled due to previous underrun, restarting", this);

        // FIXME ignoring status

        mAudioTrack->start();

    }

}

}



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

media/libmedia/AudioTrackShared.cpp

+7 −1
Original line number Diff line number Diff line
@@ -129,6 +129,11 @@ status_t ClientProxy::obtainBuffer(Buffer* buffer, const struct timespec *reques 
            status = DEAD_OBJECT;

            goto end;

        }

        if (flags & CBLK_DISABLED) {

            ALOGV("Track disabled");

            status = NOT_ENOUGH_DATA;

            goto end;

        }

        // check for obtainBuffer interrupted by client

        if (!ignoreInitialPendingInterrupt && (flags & CBLK_INTERRUPT)) {

            ALOGV("obtainBuffer() interrupted by client");
@@ -425,7 +430,8 @@ status_t AudioTrackClientProxy::waitStreamEndDone(const struct timespec *request 
            status = DEAD_OBJECT;

            goto end;

        }

        if (flags & CBLK_STREAM_END_DONE) {

        // a track is not supposed to underrun at this stage but consider it done

        if (flags & (CBLK_STREAM_END_DONE | CBLK_DISABLED)) {

            ALOGV("stream end received");

            status = NO_ERROR;

            goto end;

services/audioflinger/PlaybackTracks.h

+11 −0
Original line number Diff line number Diff line
@@ -110,10 +110,13 @@ protected: 
    // 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 @@ private: 
                                     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 @@ public: 
                                   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 @@ public: 
            void setPeerProxy(PatchProxyBufferProvider *proxy) { mPeerProxy = proxy; }



private:

            void restartIfDisabled();



    sp<ClientProxy>             mProxy;

    PatchProxyBufferProvider*   mPeerProxy;

    struct timespec             mPeerTimeout;