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

Commit c235270e authored by Shuzhen Wang's avatar Shuzhen Wang
Browse files

Camera: Avoid dequeue too many buffers from buffer queue 

The PreviewFrameSpacer could defer 1 or 2 buffers to reduce jitter.
And we update the handout buffer count at the time of queuing to
the preview frame spacer, not at the time of queuing to the buffer
queue.

This results in accounting mismatch where the framework could allow
the HAL to dequeue more buffers than supported.

The fix is to track the framework cached buffer count in addition to the
handout buffer count.

Test: Camera CTS, vendor testing, observe logcat with GoogleCamera
Bug: 242674531
Change-Id: I579ae7465b59f5bd9e12dfda332998b401793eb4
parent c25c6ed7

services/camera/libcameraservice/device3/Camera3IOStreamBase.cpp

+12 −2
Original line number Diff line number Diff line
@@ -41,8 +41,10 @@ Camera3IOStreamBase::Camera3IOStreamBase(int id, camera_stream_type_t type, 
                physicalCameraId, sensorPixelModesUsed, setId, isMultiResolution,

                dynamicRangeProfile, streamUseCase, deviceTimeBaseIsRealtime, timestampBase),

        mTotalBufferCount(0),

        mMaxCachedBufferCount(0),

        mHandoutTotalBufferCount(0),

        mHandoutOutputBufferCount(0),

        mCachedOutputBufferCount(0),

        mFrameCount(0),

        mLastTimestamp(0) {
@@ -95,8 +97,8 @@ void Camera3IOStreamBase::dump(int fd, const Vector<String16> &args) const { 
    lines.appendFormat("      Timestamp base: %d\n", getTimestampBase());

    lines.appendFormat("      Frames produced: %d, last timestamp: %" PRId64 " ns\n",

            mFrameCount, mLastTimestamp);

    lines.appendFormat("      Total buffers: %zu, currently dequeued: %zu\n",

            mTotalBufferCount, mHandoutTotalBufferCount);

    lines.appendFormat("      Total buffers: %zu, currently dequeued: %zu, currently cached: %zu\n",

            mTotalBufferCount, mHandoutTotalBufferCount, mCachedOutputBufferCount);

    write(fd, lines.string(), lines.size());



    Camera3Stream::dump(fd, args);
@@ -135,6 +137,14 @@ size_t Camera3IOStreamBase::getHandoutInputBufferCountLocked() { 
    return (mHandoutTotalBufferCount - mHandoutOutputBufferCount);

}



size_t Camera3IOStreamBase::getCachedOutputBufferCountLocked() const {

    return mCachedOutputBufferCount;

}



size_t Camera3IOStreamBase::getMaxCachedOutputBuffersLocked() const {

    return mMaxCachedBufferCount;

}



status_t Camera3IOStreamBase::disconnectLocked() {

    switch (mState) {

        case STATE_IN_RECONFIG:

services/camera/libcameraservice/device3/Camera3IOStreamBase.h

+10 −0
Original line number Diff line number Diff line
@@ -56,11 +56,18 @@ class Camera3IOStreamBase : 
    int              getMaxTotalBuffers() const { return mTotalBufferCount; }

  protected:

    size_t            mTotalBufferCount;

    // The maximum number of cached buffers allowed for this stream

    size_t            mMaxCachedBufferCount;



    // sum of input and output buffers that are currently acquired by HAL

    size_t            mHandoutTotalBufferCount;

    // number of output buffers that are currently acquired by HAL. This will be

    // Redundant when camera3 streams are no longer bidirectional streams.

    size_t            mHandoutOutputBufferCount;

    // number of cached output buffers that are currently queued in the camera

    // server but not yet queued to the buffer queue.

    size_t            mCachedOutputBufferCount;



    uint32_t          mFrameCount;

    // Last received output buffer's timestamp

    nsecs_t           mLastTimestamp;
@@ -97,6 +104,9 @@ class Camera3IOStreamBase : 


    virtual size_t   getHandoutInputBufferCountLocked();



    virtual size_t   getCachedOutputBufferCountLocked() const;

    virtual size_t   getMaxCachedOutputBuffersLocked() const;



    virtual status_t getEndpointUsage(uint64_t *usage) const = 0;



    status_t getBufferPreconditionCheckLocked() const;

services/camera/libcameraservice/device3/Camera3OutputStream.cpp

+20 −1
Original line number Diff line number Diff line
@@ -419,6 +419,7 @@ status_t Camera3OutputStream::returnBufferCheckedLocked( 
    mLock.unlock();



    ANativeWindowBuffer *anwBuffer = container_of(buffer.buffer, ANativeWindowBuffer, handle);

    bool bufferDeferred = false;

    /**

     * Return buffer back to ANativeWindow

     */
@@ -478,6 +479,7 @@ status_t Camera3OutputStream::returnBufferCheckedLocked( 
                        __FUNCTION__, mId, strerror(-res), res);

                return res;

            }

            bufferDeferred = true;

        } else {

            nsecs_t presentTime = mSyncToDisplay ?

                    syncTimestampToDisplayLocked(captureTime) : captureTime;
@@ -501,6 +503,10 @@ status_t Camera3OutputStream::returnBufferCheckedLocked( 
    }

    mLock.lock();



    if (bufferDeferred) {

        mCachedOutputBufferCount++;

    }



    // Once a valid buffer has been returned to the queue, can no longer

    // dequeue all buffers for preallocation.

    if (buffer.status != CAMERA_BUFFER_STATUS_ERROR) {
@@ -696,10 +702,15 @@ status_t Camera3OutputStream::configureConsumerQueueLocked(bool allowPreviewResp 
                !isVideoStream());

        if (forceChoreographer || defaultToChoreographer) {

            mSyncToDisplay = true;

            // For choreographer synced stream, extra buffers aren't kept by

            // camera service. So no need to update mMaxCachedBufferCount.

            mTotalBufferCount += kDisplaySyncExtraBuffer;

        } else if (defaultToSpacer) {

            mPreviewFrameSpacer = new PreviewFrameSpacer(this, mConsumer);

            mTotalBufferCount ++;

            // For preview frame spacer, the extra buffer is kept by camera

            // service. So update mMaxCachedBufferCount.

            mMaxCachedBufferCount = 1;

            mTotalBufferCount += mMaxCachedBufferCount;

            res = mPreviewFrameSpacer->run(String8::format("PreviewSpacer-%d", mId).string());

            if (res != OK) {

                ALOGE("%s: Unable to start preview spacer", __FUNCTION__);
@@ -968,6 +979,14 @@ bool Camera3OutputStream::shouldLogError(status_t res, StreamState state) { 
    return true;

}



void Camera3OutputStream::onCachedBufferQueued() {

    Mutex::Autolock l(mLock);

    mCachedOutputBufferCount--;

    // Signal whoever is waiting for the buffer to be returned to the buffer

    // queue.

    mOutputBufferReturnedSignal.signal();

}



status_t Camera3OutputStream::disconnectLocked() {

    status_t res;

services/camera/libcameraservice/device3/Camera3OutputStream.h

+1 −0
Original line number Diff line number Diff line
@@ -259,6 +259,7 @@ class Camera3OutputStream : 


    void setImageDumpMask(int mask) { mImageDumpMask = mask; }

    bool shouldLogError(status_t res);

    void onCachedBufferQueued();



  protected:

    Camera3OutputStream(int id, camera_stream_type_t type,

services/camera/libcameraservice/device3/Camera3Stream.cpp

+37 −13
Original line number Diff line number Diff line
@@ -665,11 +665,19 @@ status_t Camera3Stream::getBuffer(camera_stream_buffer *buffer, 
        }

    }



    // Wait for new buffer returned back if we are running into the limit.

    // Wait for new buffer returned back if we are running into the limit. There

    // are 2 limits:

    // 1. The number of HAL buffers is greater than max_buffers

    // 2. The number of HAL buffers + cached buffers is greater than max_buffers

    //    + maxCachedBuffers

    size_t numOutstandingBuffers = getHandoutOutputBufferCountLocked();

    if (numOutstandingBuffers == camera_stream::max_buffers) {

        ALOGV("%s: Already dequeued max output buffers (%d), wait for next returned one.",

                        __FUNCTION__, camera_stream::max_buffers);

    size_t numCachedBuffers = getCachedOutputBufferCountLocked();

    size_t maxNumCachedBuffers = getMaxCachedOutputBuffersLocked();

    while (numOutstandingBuffers == camera_stream::max_buffers ||

            numOutstandingBuffers + numCachedBuffers ==

            camera_stream::max_buffers + maxNumCachedBuffers) {

        ALOGV("%s: Already dequeued max output buffers (%d(+%zu)), wait for next returned one.",

                        __FUNCTION__, camera_stream::max_buffers, maxNumCachedBuffers);

        nsecs_t waitStart = systemTime(SYSTEM_TIME_MONOTONIC);

        if (waitBufferTimeout < kWaitForBufferDuration) {

            waitBufferTimeout = kWaitForBufferDuration;
@@ -687,12 +695,16 @@ status_t Camera3Stream::getBuffer(camera_stream_buffer *buffer, 
        }



        size_t updatedNumOutstandingBuffers = getHandoutOutputBufferCountLocked();

        if (updatedNumOutstandingBuffers >= numOutstandingBuffers) {

            ALOGE("%s: outsanding buffer count goes from %zu to %zu, "

        size_t updatedNumCachedBuffers = getCachedOutputBufferCountLocked();

        if (updatedNumOutstandingBuffers >= numOutstandingBuffers &&

                updatedNumCachedBuffers == numCachedBuffers) {

            ALOGE("%s: outstanding buffer count goes from %zu to %zu, "

                    "getBuffer(s) call must not run in parallel!", __FUNCTION__,

                    numOutstandingBuffers, updatedNumOutstandingBuffers);

            return INVALID_OPERATION;

        }

        numOutstandingBuffers = updatedNumOutstandingBuffers;

        numCachedBuffers = updatedNumCachedBuffers;

    }



    res = getBufferLocked(buffer, surface_ids);
@@ -1057,11 +1069,20 @@ status_t Camera3Stream::getBuffers(std::vector<OutstandingBuffer>* buffers, 
    }



    size_t numOutstandingBuffers = getHandoutOutputBufferCountLocked();

    // Wait for new buffer returned back if we are running into the limit.

    while (numOutstandingBuffers + numBuffersRequested > camera_stream::max_buffers) {

        ALOGV("%s: Already dequeued %zu output buffers and requesting %zu (max is %d), waiting.",

                __FUNCTION__, numOutstandingBuffers, numBuffersRequested,

                camera_stream::max_buffers);

    size_t numCachedBuffers = getCachedOutputBufferCountLocked();

    size_t maxNumCachedBuffers = getMaxCachedOutputBuffersLocked();

    // Wait for new buffer returned back if we are running into the limit. There

    // are 2 limits:

    // 1. The number of HAL buffers is greater than max_buffers

    // 2. The number of HAL buffers + cached buffers is greater than max_buffers

    //    + maxCachedBuffers

    while (numOutstandingBuffers + numBuffersRequested > camera_stream::max_buffers ||

            numOutstandingBuffers + numCachedBuffers + numBuffersRequested >

            camera_stream::max_buffers + maxNumCachedBuffers) {

        ALOGV("%s: Already dequeued %zu(+%zu) output buffers and requesting %zu "

                "(max is %d(+%zu)), waiting.", __FUNCTION__, numOutstandingBuffers,

                numCachedBuffers, numBuffersRequested, camera_stream::max_buffers,

                maxNumCachedBuffers);

        nsecs_t waitStart = systemTime(SYSTEM_TIME_MONOTONIC);

        if (waitBufferTimeout < kWaitForBufferDuration) {

            waitBufferTimeout = kWaitForBufferDuration;
@@ -1078,13 +1099,16 @@ status_t Camera3Stream::getBuffers(std::vector<OutstandingBuffer>* buffers, 
            return res;

        }

        size_t updatedNumOutstandingBuffers = getHandoutOutputBufferCountLocked();

        if (updatedNumOutstandingBuffers >= numOutstandingBuffers) {

            ALOGE("%s: outsanding buffer count goes from %zu to %zu, "

        size_t updatedNumCachedBuffers = getCachedOutputBufferCountLocked();

        if (updatedNumOutstandingBuffers >= numOutstandingBuffers &&

                updatedNumCachedBuffers == numCachedBuffers) {

            ALOGE("%s: outstanding buffer count goes from %zu to %zu, "

                    "getBuffer(s) call must not run in parallel!", __FUNCTION__,

                    numOutstandingBuffers, updatedNumOutstandingBuffers);

            return INVALID_OPERATION;

        }

        numOutstandingBuffers = updatedNumOutstandingBuffers;

        numCachedBuffers = updatedNumCachedBuffers;

    }



    res = getBuffersLocked(buffers);