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

Commit 6f82372d authored by Emilian Peev's avatar Emilian Peev
Browse files

Camera: Detach shared buffers if they are still referenced 

Dynamic consumers removed by client could still hold one or
several buffer references after streaming stops and the
producer interface gets disconnected. Returning those buffers
in the input queue is not recommended as they can continue to get
accessed at the same time as the camera modifies them. To avoid this
try to detach all previously registered buffers. If the call
fails for some specific buffer, then try to detach it from both the
input queue and the remaining outputs.

Bug: 70223208
Test: Camera CTS
Change-Id: Ie35c7a2360d970ab5c860be7b580140dfb768934
parent 85359426

services/camera/libcameraservice/device3/Camera3StreamSplitter.cpp

+45 −11
Original line number Diff line number Diff line
@@ -275,9 +275,17 @@ status_t Camera3StreamSplitter::removeOutputLocked(size_t surfaceId) { 
    //still attached to the removed surface.

    std::vector<uint64_t> pendingBufferIds;

    auto& outputSlots = *mOutputSlots[gbp];

    for (const auto &it : outputSlots) {

        if (it.get() != nullptr) {

            pendingBufferIds.push_back(it->getId());

    for (size_t i = 0; i < outputSlots.size(); i++) {

        if (outputSlots[i] != nullptr) {

            pendingBufferIds.push_back(outputSlots[i]->getId());

            auto rc = gbp->detachBuffer(i);

            if (rc != NO_ERROR) {

                //Buffers that fail to detach here will be scheduled for detach in the

                //input buffer queue and the rest of the registered outputs instead.

                //This will help ensure that camera stops accessing buffers that still

                //can get referenced by the disconnected output.

                mDetachedBuffers.emplace(outputSlots[i]->getId());

            }

        }

    }

    mOutputs[surfaceId] = nullptr;
@@ -521,10 +529,11 @@ void Camera3StreamSplitter::decrementBufRefCountLocked(uint64_t id, size_t surfa 
    uint64_t bufferId = tracker_ptr->getBuffer()->getId();

    int consumerSlot = -1;

    uint64_t frameNumber;

    for (const auto &it : mInputSlots) {

        if (it.second.mGraphicBuffer->getId() == bufferId) {

            consumerSlot = it.second.mSlot;

            frameNumber = it.second.mFrameNumber;

    auto inputSlot = mInputSlots.begin();

    for (; inputSlot != mInputSlots.end(); inputSlot++) {

        if (inputSlot->second.mGraphicBuffer->getId() == bufferId) {

            consumerSlot = inputSlot->second.mSlot;

            frameNumber = inputSlot->second.mFrameNumber;

            break;

        }

    }
@@ -533,6 +542,12 @@ void Camera3StreamSplitter::decrementBufRefCountLocked(uint64_t id, size_t surfa 
        return;

    }



    auto detachBuffer = mDetachedBuffers.find(bufferId);

    bool detach = (detachBuffer != mDetachedBuffers.end());

    if (detach) {

        mDetachedBuffers.erase(detachBuffer);

        mInputSlots.erase(inputSlot);

    }

    // Temporarily unlock mutex to avoid circular lock:

    // 1. This function holds splitter lock, calls releaseBuffer which triggers

    // onBufferReleased in Camera3OutputStream. onBufferReleased waits on the
@@ -544,17 +559,25 @@ void Camera3StreamSplitter::decrementBufRefCountLocked(uint64_t id, size_t surfa 
    mMutex.unlock();

    int res = NO_ERROR;

    if (consumer != nullptr) {

        if (detach) {

            res = consumer->detachBuffer(consumerSlot);

        } else {

            res = consumer->releaseBuffer(consumerSlot, frameNumber,

                    EGL_NO_DISPLAY, EGL_NO_SYNC_KHR, tracker_ptr->getMergedFence());

        }

    } else {

        SP_LOGE("%s: consumer has become null!", __FUNCTION__);

    }

    mMutex.lock();

    // If the producer of this queue is disconnected, -22 error will occur



    if (res != NO_ERROR) {

        if (detach) {

            SP_LOGE("%s: detachBuffer returns %d", __FUNCTION__, res);

        } else {

            SP_LOGE("%s: releaseBuffer returns %d", __FUNCTION__, res);

        }

    }

}



void Camera3StreamSplitter::onBufferReleasedByOutput(

        const sp<IGraphicBufferProducer>& from) {
@@ -626,6 +649,17 @@ void Camera3StreamSplitter::onBufferReleasedByOutputLocked( 
    SP_LOGV("%s: dequeued buffer %" PRId64 " %p from output %p", __FUNCTION__,

            buffer->getId(), buffer.get(), from.get());



    auto detachBuffer = mDetachedBuffers.find(buffer->getId());

    bool detach = (detachBuffer != mDetachedBuffers.end());

    if (detach) {

        res = from->detachBuffer(slot);

        if (res == NO_ERROR) {

            outputSlots[slot] = nullptr;

        } else {

            SP_LOGE("%s: detach buffer from output failed (%d)", __FUNCTION__, res);

        }

    }



    // Check to see if this is the last outstanding reference to this buffer

    decrementBufRefCountLocked(buffer->getId(), surfaceId);

}

services/camera/libcameraservice/device3/Camera3StreamSplitter.h

+6 −0
Original line number Diff line number Diff line
@@ -17,6 +17,8 @@ 
#ifndef ANDROID_SERVERS_STREAMSPLITTER_H

#define ANDROID_SERVERS_STREAMSPLITTER_H



#include <unordered_set>



#include <gui/IConsumerListener.h>

#include <gui/IProducerListener.h>

#include <gui/BufferItemConsumer.h>
@@ -255,6 +257,10 @@ private: 
    std::unordered_map<sp<IGraphicBufferProducer>, std::unique_ptr<OutputSlots>,

            GBPHash> mOutputSlots;



    //A set of buffers that could potentially stay in some of the outputs after removal

    //and therefore should be detached from the input queue.

    std::unordered_set<uint64_t> mDetachedBuffers;



    // Latest onFrameAvailable return value

    std::atomic<status_t> mOnFrameAvailableRes{0};