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

Commit 52c12390 authored by Jayasena Sangaraboina's avatar Jayasena Sangaraboina Committed by Steve Kondik
Browse files

libstagefright: Fix to free buffers in error scenarios

MediaBuffer objects are not freed if component transition from
IDLE->EXECUTING or EXECUTING->LOADED fails (or if the
OMX_CmdComplete messages are dropped due to an earlier error)
Additionally with native buffers, the corresponding
ANativeWindowBuffers are not cancelled back in the above scenarios.

Change-Id: Idad02f7bb6bbfcd30947624148ea996a4a8f18b3
parent 6865fb32
Loading
Loading
Loading
Loading
+2 −0
Original line number Diff line number Diff line
@@ -372,6 +372,7 @@ private:
            unsigned *profile, unsigned *level);

    status_t stopOmxComponent_l();
    status_t flushBuffersOnError(void);

    OMXCodec(const OMXCodec &);
    OMXCodec &operator=(const OMXCodec &);
@@ -380,6 +381,7 @@ private:

    bool mNumBFrames;
    bool mInSmoothStreamingMode;
    status_t releaseMediaBuffersOn(OMX_U32 portIndex);
};

struct CodecCapabilities {
+116 −0
Original line number Diff line number Diff line
@@ -1730,6 +1730,10 @@ OMXCodec::~OMXCodec() {
    CHECK_EQ(err, (status_t)OK);

    mNode = NULL;

    releaseMediaBuffersOn(kPortIndexOutput);
    releaseMediaBuffersOn(kPortIndexInput);

    setState(DEAD);

    clearCodecSpecificData();
@@ -1769,6 +1773,13 @@ status_t OMXCodec::init() {
        mAsyncCompletion.wait(mLock);
    }

    /*
     * If the native window is valid, we need to do the extra work of
     * cancelling buffers back.
     */
    if (mState == ERROR) {
        flushBuffersOnError();
    }
    return mState == ERROR ? UNKNOWN_ERROR : OK;
}

@@ -4138,6 +4149,7 @@ status_t OMXCodec::stopOmxComponent_l() {
    }

    bool isError = false;
    bool forceFlush = false;
    switch (mState) {
        case LOADED:
            break;
@@ -4166,6 +4178,7 @@ status_t OMXCodec::stopOmxComponent_l() {
                CHECK_EQ(err, (status_t)OK);

                if (state != OMX_StateExecuting) {
                    forceFlush = true;
                    break;
                }
                // else fall through to the idling code
@@ -4221,6 +4234,10 @@ status_t OMXCodec::stopOmxComponent_l() {
                mAsyncCompletion.wait(mLock);
            }

            if (mState == ERROR) {
                forceFlush = true;
            }

            if (isError) {
                // We were in the ERROR state coming in, so restore that now
                // that we've idled the OMX component.
@@ -4237,6 +4254,8 @@ status_t OMXCodec::stopOmxComponent_l() {
        }
    }

    if (forceFlush) flushBuffersOnError();

    if (mLeftOverBuffer) {
        mLeftOverBuffer->release();
        mLeftOverBuffer = NULL;
@@ -5232,4 +5251,101 @@ status_t getOMXChannelMapping(size_t numChannels, OMX_AUDIO_CHANNELTYPE map[]) {
    return OK;
}

status_t OMXCodec::releaseMediaBuffersOn(OMX_U32 portIndex) {
    if (mPortBuffers[portIndex].size() == 0) {
        return OK;
    }

    if (mState != ERROR) {
        CODEC_LOGE("assertion failure, needs to be investigated why %s "
              " buffers are still pending",
              portIndex == kPortIndexOutput ? "output" : "input");
        //CHECK(!"shouldn't be here");
    }

    Vector<BufferInfo> *buffers = &mPortBuffers[portIndex];

    for (size_t i = buffers->size(); i-- > 0;) {
        BufferInfo *info = &buffers->editItemAt(i);
        if (info->mMediaBuffer) {
            CHECK_EQ(portIndex, (OMX_U32)kPortIndexOutput);
            info->mMediaBuffer->setObserver(NULL);

            // Make sure nobody but us owns this buffer at this point.
            CHECK_EQ(info->mMediaBuffer->refcount(), 0);

            info->mMediaBuffer->release();
            info->mMediaBuffer = NULL;
        }
        buffers->removeAt(i);
    }
    return OK;
}

/*
 * Last resort to flush buffers and additionally cancel all native window buffers.
 *
 * lock _must_ be acquired in caller
 */
status_t OMXCodec::flushBuffersOnError() {
    if (mState != ERROR) {
        return INVALID_OPERATION;
    }

    OMX_STATETYPE state = OMX_StateInvalid;
    status_t err = mOMX->getState(mNode, &state);
    CHECK_EQ(err, (status_t)OK); //component is alive

    mPortStatus[kPortIndexOutput] = ENABLED;
    mPortStatus[kPortIndexInput] = ENABLED;

    setState(EXECUTING_TO_IDLE);

    flushPortAsync(kPortIndexOutput);
    flushPortAsync(kPortIndexInput);

    size_t kRetries = 15;

    bool outputBuffersPending =
        countBuffersWeOwn(mPortBuffers[kPortIndexOutput]) !=
        mPortBuffers[kPortIndexOutput].size();

    bool inputBuffersPending =
        countBuffersWeOwn(mPortBuffers[kPortIndexInput]) !=
        mPortBuffers[kPortIndexInput].size();

    setState(ERROR); //drop all except EBD/FBD
    while ((outputBuffersPending || inputBuffersPending) && --kRetries) {
        mLock.unlock();
        usleep(10000);
        mLock.lock();

        outputBuffersPending =
            countBuffersWeOwn(mPortBuffers[kPortIndexOutput]) !=
            mPortBuffers[kPortIndexOutput].size();

        inputBuffersPending =
            countBuffersWeOwn(mPortBuffers[kPortIndexInput]) !=
            mPortBuffers[kPortIndexInput].size();
    }

    if (inputBuffersPending || outputBuffersPending) {
        ALOGE("Timed out waiting for all input/output buffers to be returned, "
              "there might be a leak");
    }

    //additional work for native buffers
    if (mNativeWindow != NULL) {
        Vector<BufferInfo> *buffers = &mPortBuffers[kPortIndexOutput];
        for (size_t i = 0; i < buffers->size(); ++i) {
            BufferInfo *info = &buffers->editItemAt(i);
            if (info->mStatus == OWNED_BY_US) {
                cancelBufferToNativeWindow(info);
            }
        }
    }

    return OK;
}

}  // namespace android