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

Commit bf1b8b96 authored by Yin-Chia Yeh's avatar Yin-Chia Yeh
Browse files

Camera: fix long dequeueBuffer call

Fix the long (>1s) dequeueBuffer call when a stream is managed by
Camera3BufferManager and its consumer end discards free buffers.

Test: CTS, no more long dequeBuffer call in GCA mode switch
Bug: 126054873
Change-Id: I03d6526b076796bb44f15cc2c4a092ff3d04fc1d
parent 5f840f8d
Loading
Loading
Loading
Loading
+8 −3
Original line number Diff line number Diff line
@@ -237,7 +237,7 @@ status_t Camera3BufferManager::checkAndFreeBufferOnOtherStreamsLocked(
}

status_t Camera3BufferManager::getBufferForStream(int streamId, int streamSetId,
        sp<GraphicBuffer>* gb, int* fenceFd) {
        sp<GraphicBuffer>* gb, int* fenceFd, bool noFreeBufferAtConsumer) {
    ATRACE_CALL();

    Mutex::Autolock l(mLock);
@@ -253,14 +253,19 @@ status_t Camera3BufferManager::getBufferForStream(int streamId, int streamSetId,
    StreamSet &streamSet = mStreamSetMap.editValueFor(streamSetId);
    BufferCountMap& handOutBufferCounts = streamSet.handoutBufferCountMap;
    size_t& bufferCount = handOutBufferCounts.editValueFor(streamId);
    BufferCountMap& attachedBufferCounts = streamSet.attachedBufferCountMap;
    size_t& attachedBufferCount = attachedBufferCounts.editValueFor(streamId);

    if (noFreeBufferAtConsumer) {
        attachedBufferCount = bufferCount;
    }

    if (bufferCount >= streamSet.maxAllowedBufferCount) {
        ALOGE("%s: bufferCount (%zu) exceeds the max allowed buffer count (%zu) of this stream set",
                __FUNCTION__, bufferCount, streamSet.maxAllowedBufferCount);
        return INVALID_OPERATION;
    }

    BufferCountMap& attachedBufferCounts = streamSet.attachedBufferCountMap;
    size_t& attachedBufferCount = attachedBufferCounts.editValueFor(streamId);
    if (attachedBufferCount > bufferCount) {
        // We've already attached more buffers to this stream than we currently have
        // outstanding, so have the stream just use an already-attached buffer
+7 −1
Original line number Diff line number Diff line
@@ -112,6 +112,10 @@ public:
     *
     * After this call, the client takes over the ownership of this buffer if it is not freed.
     *
     * Sometimes free buffers are discarded from consumer side and the dequeueBuffer call returns
     * TIMED_OUT, in this case calling getBufferForStream again with noFreeBufferAtConsumer set to
     * true will notify buffer manager to update its states and also tries to allocate a new buffer.
     *
     * Return values:
     *
     *  OK:        Getting buffer for this stream was successful.
@@ -122,7 +126,9 @@ public:
     *             to this buffer manager before.
     *  NO_MEMORY: Unable to allocate a buffer for this stream at this time.
     */
    status_t getBufferForStream(int streamId, int streamSetId, sp<GraphicBuffer>* gb, int* fenceFd);
    status_t getBufferForStream(
            int streamId, int streamSetId, sp<GraphicBuffer>* gb, int* fenceFd,
            bool noFreeBufferAtConsumer = false);

    /**
     * This method notifies the manager that a buffer has been released by the consumer.
+66 −21
Original line number Diff line number Diff line
@@ -359,8 +359,19 @@ status_t Camera3OutputStream::configureQueueLocked() {
    // Set dequeueBuffer/attachBuffer timeout if the consumer is not hw composer or hw texture.
    // We need skip these cases as timeout will disable the non-blocking (async) mode.
    if (!(isConsumedByHWComposer() || isConsumedByHWTexture())) {
        if (mUseBufferManager) {
            // When buffer manager is handling the buffer, we should have available buffers in
            // buffer queue before we calls into dequeueBuffer because buffer manager is tracking
            // free buffers.
            // There are however some consumer side feature (ImageReader::discardFreeBuffers) that
            // can discard free buffers without notifying buffer manager. We want the timeout to
            // happen immediately here so buffer manager can try to update its internal state and
            // try to allocate a buffer instead of waiting.
            mConsumer->setDequeueTimeout(0);
        } else {
            mConsumer->setDequeueTimeout(kDequeueBufferTimeout);
        }
    }

    return OK;
}
@@ -526,6 +537,8 @@ status_t Camera3OutputStream::getBufferLockedCommon(ANativeWindowBuffer** anb, i
            if (res != OK) {
                ALOGE("%s: Stream %d: Can't attach the output buffer to this surface: %s (%d)",
                        __FUNCTION__, mId, strerror(-res), res);

                checkRetAndSetAbandonedLocked(res);
                return res;
            }
            gotBufferFromManager = true;
@@ -562,33 +575,68 @@ status_t Camera3OutputStream::getBufferLockedCommon(ANativeWindowBuffer** anb, i
        mDequeueBufferLatency.add(dequeueStart, dequeueEnd);

        mLock.lock();

        if (mUseBufferManager && res == TIMED_OUT) {
            checkRemovedBuffersLocked();

            sp<GraphicBuffer> gb;
            res = mBufferManager->getBufferForStream(
                    getId(), getStreamSetId(), &gb, fenceFd, /*noFreeBuffer*/true);

            if (res == OK) {
                // Attach this buffer to the bufferQueue: the buffer will be in dequeue state after
                // a successful return.
                *anb = gb.get();
                res = mConsumer->attachBuffer(*anb);
                gotBufferFromManager = true;
                ALOGV("Stream %d: Attached new buffer", getId());

                if (res != OK) {
            ALOGE("%s: Stream %d: Can't dequeue next output buffer: %s (%d)",
                    ALOGE("%s: Stream %d: Can't attach the output buffer to this surface: %s (%d)",
                            __FUNCTION__, mId, strerror(-res), res);

            // Only transition to STATE_ABANDONED from STATE_CONFIGURED. (If it is STATE_PREPARING,
            // let prepareNextBuffer handle the error.)
            if ((res == NO_INIT || res == DEAD_OBJECT) && mState == STATE_CONFIGURED) {
                mState = STATE_ABANDONED;
                    checkRetAndSetAbandonedLocked(res);
                    return res;
                }
            } else {
                ALOGE("%s: Stream %d: Can't get next output buffer from buffer manager:"
                        " %s (%d)", __FUNCTION__, mId, strerror(-res), res);
                return res;
            }
        } else if (res != OK) {
            ALOGE("%s: Stream %d: Can't dequeue next output buffer: %s (%d)",
                    __FUNCTION__, mId, strerror(-res), res);

            checkRetAndSetAbandonedLocked(res);
            return res;
        }
    }

    if (res == OK) {
        checkRemovedBuffersLocked();
    }

    return res;
}

void Camera3OutputStream::checkRemovedBuffersLocked(bool notifyBufferManager) {
    std::vector<sp<GraphicBuffer>> removedBuffers;
        res = mConsumer->getAndFlushRemovedBuffers(&removedBuffers);
    status_t res = mConsumer->getAndFlushRemovedBuffers(&removedBuffers);
    if (res == OK) {
        onBuffersRemovedLocked(removedBuffers);

            if (mUseBufferManager && removedBuffers.size() > 0) {
        if (notifyBufferManager && mUseBufferManager && removedBuffers.size() > 0) {
            mBufferManager->onBuffersRemoved(getId(), getStreamSetId(), removedBuffers.size());
        }
    }
}

    return res;
void Camera3OutputStream::checkRetAndSetAbandonedLocked(status_t res) {
    // Only transition to STATE_ABANDONED from STATE_CONFIGURED. (If it is
    // STATE_PREPARING, let prepareNextBuffer handle the error.)
    if ((res == NO_INIT || res == DEAD_OBJECT) && mState == STATE_CONFIGURED) {
        mState = STATE_ABANDONED;
    }
}

status_t Camera3OutputStream::disconnectLocked() {
@@ -803,11 +851,8 @@ status_t Camera3OutputStream::detachBufferLocked(sp<GraphicBuffer>* buffer, int*
        }
    }

    std::vector<sp<GraphicBuffer>> removedBuffers;
    res = mConsumer->getAndFlushRemovedBuffers(&removedBuffers);
    if (res == OK) {
        onBuffersRemovedLocked(removedBuffers);
    }
    // Here we assume detachBuffer is called by buffer manager so it doesn't need to be notified
    checkRemovedBuffersLocked(/*notifyBufferManager*/false);
    return res;
}

+7 −0
Original line number Diff line number Diff line
@@ -309,6 +309,13 @@ class Camera3OutputStream :
     */
    void onBuffersRemovedLocked(const std::vector<sp<GraphicBuffer>>&);
    status_t detachBufferLocked(sp<GraphicBuffer>* buffer, int* fenceFd);
    // Call this after each dequeueBuffer/attachBuffer/detachNextBuffer call to get update on
    // removed buffers. Set notifyBufferManager to false when the call is initiated by buffer
    // manager so buffer manager doesn't need to be notified.
    void checkRemovedBuffersLocked(bool notifyBufferManager = true);

    // Check return status of IGBP calls and set abandoned state accordingly
    void checkRetAndSetAbandonedLocked(status_t res);

    static const int32_t kDequeueLatencyBinSize = 5; // in ms
    CameraLatencyHistogram mDequeueBufferLatency;