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

Commit 264bac95 authored by Lajos Molnar's avatar Lajos Molnar
Browse files

stagefright: prevent allocating stale buffers for OMX decoders 

Also fix some issues encountered once using generationNumbers:
- properly account outstanding buffers in MediaSync
- don't release arbitrary frame if attach fails

Bug: 11990461
Change-Id: Icee5ea188ca4eb856138feb5e6ec5d4ee5e44008
parent 064b2bf7

media/libstagefright/ACodec.cpp

+1 −0
Original line number Diff line number Diff line
@@ -5234,6 +5234,7 @@ void ACodec::BaseState::onOutputBufferDrained(const sp<AMessage> &msg) { 
        if (err == OK) {

            info->mStatus = BufferInfo::OWNED_BY_NATIVE_WINDOW;

        } else {

            ALOGE("queueBuffer failed in onOutputBufferDrained: %d", err);

            mCodec->signalError(OMX_ErrorUndefined, makeNoSideEffectStatus(err));

            info->mStatus = BufferInfo::OWNED_BY_US;

            // keeping read fence as write fence to avoid clobbering

media/libstagefright/MediaCodec.cpp

+21 −1
Original line number Diff line number Diff line
@@ -2528,7 +2528,25 @@ status_t MediaCodec::connectToSurface(const sp<Surface> &surface) { 
        err = native_window_api_connect(surface.get(), NATIVE_WINDOW_API_MEDIA);

        if (err == BAD_VALUE) {

            ALOGI("native window already connected. Assuming no change of surface");

        } else if (err != OK) {

        } else if (err == OK) {

            // Require a fresh set of buffers after each connect by using a unique generation

            // number. Rely on the fact that max supported process id by Linux is 2^22.

            // PID is never 0 so we don't have to worry that we use the default generation of 0.

            // TODO: come up with a unique scheme if other producers also set the generation number.

            static uint32_t mSurfaceGeneration = 0;

            uint32_t generation = (getpid() << 10) | (++mSurfaceGeneration & ((1 << 10) - 1));

            surface->setGenerationNumber(generation);

            ALOGI("[%s] setting surface generation to %u", mComponentName.c_str(), generation);



            // HACK: clear any free buffers. Remove when connect will automatically do this.

            // This is needed as the consumer may be holding onto stale frames that it can reattach

            // to this surface after disconnect/connect, and those free frames would inherit the new

            // generation number. Disconnecting after setting a unique generation prevents this.

            native_window_api_disconnect(surface.get(), NATIVE_WINDOW_API_MEDIA);

            err = native_window_api_connect(surface.get(), NATIVE_WINDOW_API_MEDIA);

        }



        if (err != OK) {

            ALOGE("native_window_api_connect returned an error: %s (%d)", strerror(-err), err);

        }

    }
@@ -2538,6 +2556,8 @@ status_t MediaCodec::connectToSurface(const sp<Surface> &surface) { 
status_t MediaCodec::disconnectFromSurface() {

    status_t err = OK;

    if (mSurface != NULL) {

        // Resetting generation is not technically needed, but there is no need to keep it either

        mSurface->setGenerationNumber(0);

        err = native_window_api_disconnect(mSurface.get(), NATIVE_WINDOW_API_MEDIA);

        if (err != OK) {

            ALOGW("native_window_api_disconnect returned an error: %s (%d)", strerror(-err), err);

media/libstagefright/MediaSync.cpp

+6 −8
Original line number Diff line number Diff line
@@ -558,7 +558,6 @@ void MediaSync::onFrameAvailableFromInput() { 
            return;

        }

    }

    ++mNumOutstandingBuffers;



    // Acquire and detach the buffer from the input.

    BufferItem bufferItem;
@@ -567,6 +566,7 @@ void MediaSync::onFrameAvailableFromInput() { 
        ALOGE("acquiring buffer from input failed (%d)", status);

        return;

    }

    ++mNumOutstandingBuffers;



    ALOGV("acquired buffer %#llx from input", (long long)bufferItem.mGraphicBuffer->getId());
@@ -608,6 +608,7 @@ void MediaSync::renderOneBufferItem_l( const BufferItem &bufferItem) { 


    // Attach and queue the buffer to the output.

    int slot;

    mOutput->setGenerationNumber(bufferItem.mGraphicBuffer->getGenerationNumber());

    status_t status = mOutput->attachBuffer(&slot, bufferItem.mGraphicBuffer);

    ALOGE_IF(status != NO_ERROR, "attaching buffer to output failed (%d)", status);

    if (status == NO_ERROR) {
@@ -695,16 +696,13 @@ void MediaSync::returnBufferToInput_l( 
        ALOGE_IF(status != NO_ERROR, "releasing buffer to input failed (%d)", status);

    }



    if (status != NO_ERROR) {

        // TODO: do we need to try to return this buffer later?

        return;

    }



    ALOGV("released buffer %#llx to input", (long long)oldBuffer->getId());



    // Notify any waiting onFrameAvailable calls.

    --mNumOutstandingBuffers;

    mReleaseCondition.signal();



    if (status == NO_ERROR) {

        ALOGV("released buffer %#llx to input", (long long)oldBuffer->getId());

    }

}



void MediaSync::onAbandoned_l(bool isInput) {

media/libstagefright/omx/GraphicBufferSource.cpp

+5 −3
Original line number Diff line number Diff line
@@ -836,13 +836,15 @@ void GraphicBufferSource::releaseBuffer( 
        mConsumer->detachBuffer(id);

        mBufferSlot[id] = NULL;



        mConsumer->attachBuffer(&id, buffer);

        if (mConsumer->attachBuffer(&id, buffer) == OK) {

            mConsumer->releaseBuffer(

                    id, 0, EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, fence);

        }

    } else {

        mConsumer->releaseBuffer(

                id, frameNum, EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, fence);

    }

    id = -1; // invalidate id

    mNumBufferAcquired--;

}