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

Commit 78014f32 authored by Antoine Labour's avatar Antoine Labour Committed by John Reck
Browse files

BufferQueue: release mutex while allocating. DO NOT MERGE 

BufferQueueProducer::allocateBuffers used to keep the BufferQueueCore
mutex while doing the buffer allocation, which would cause the consumer
(which also needs the mutex) to block if the allocation takes a long
time.
Instead, release the mutex while doing the allocation, and grab it again
before filling the slots. Keep a bool state and a condvar to prevent
other producers from trying to allocate the slots while the mutex is
released.

Bug: 11792166

Change-Id: I4ab1319995ef892be2beba892f1fdbf50ce0416d
(cherry picked from commit ea960444)
parent 7b3f48d2

include/gui/BufferQueueCore.h

+12 −0
Original line number Diff line number Diff line
@@ -120,6 +120,9 @@ private: 
    // in one of the slots.

    bool stillTracking(const BufferItem* item) const;



    // waitWhileAllocatingLocked blocks until mIsAllocating is false.

    void waitWhileAllocatingLocked() const;



    // mAllocator is the connection to SurfaceFlinger that is used to allocate

    // new GraphicBuffer objects.

    sp<IGraphicBufferAlloc> mAllocator;
@@ -234,6 +237,15 @@ private: 
    // mSidebandStream is a handle to the sideband buffer stream, if any

    sp<NativeHandle> mSidebandStream;



    // mIsAllocating indicates whether a producer is currently trying to allocate buffers (which

    // releases mMutex while doing the allocation proper). Producers should not modify any of the

    // FREE slots while this is true. mIsAllocatingCondition is signaled when this value changes to

    // false.

    bool mIsAllocating;



    // mIsAllocatingCondition is a condition variable used by producers to wait until mIsAllocating

    // becomes false.

    mutable Condition mIsAllocatingCondition;

}; // class BufferQueueCore



} // namespace android

libs/gui/BufferQueueCore.cpp

+10 −1
Original line number Diff line number Diff line
@@ -64,7 +64,9 @@ BufferQueueCore::BufferQueueCore(const sp<IGraphicBufferAlloc>& allocator) : 
    mMaxAcquiredBufferCount(1),

    mBufferHasBeenQueued(false),

    mFrameCounter(0),

    mTransformHint(0)

    mTransformHint(0),

    mIsAllocating(false),

    mIsAllocatingCondition()

{

    if (allocator == NULL) {

        sp<ISurfaceComposer> composer(ComposerService::getComposerService());
@@ -226,4 +228,11 @@ bool BufferQueueCore::stillTracking(const BufferItem* item) const { 
           (item->mGraphicBuffer->handle == slot.mGraphicBuffer->handle);

}



void BufferQueueCore::waitWhileAllocatingLocked() const {

    ATRACE_CALL();

    while (mIsAllocating) {

        mIsAllocatingCondition.wait(mMutex);

    }

}



} // namespace android

libs/gui/BufferQueueProducer.cpp

+91 −40
Original line number Diff line number Diff line
@@ -74,6 +74,7 @@ status_t BufferQueueProducer::setBufferCount(int bufferCount) { 
    sp<IConsumerListener> listener;

    { // Autolock scope

        Mutex::Autolock lock(mCore->mMutex);

        mCore->waitWhileAllocatingLocked();



        if (mCore->mIsAbandoned) {

            BQ_LOGE("setBufferCount: BufferQueue has been abandoned");
@@ -266,6 +267,7 @@ status_t BufferQueueProducer::dequeueBuffer(int *outSlot, 


    { // Autolock scope

        Mutex::Autolock lock(mCore->mMutex);

        mCore->waitWhileAllocatingLocked();



        if (format == 0) {

            format = mCore->mDefaultBufferFormat;
@@ -424,6 +426,7 @@ status_t BufferQueueProducer::detachNextBuffer(sp<GraphicBuffer>* outBuffer, 
    }



    Mutex::Autolock lock(mCore->mMutex);

    mCore->waitWhileAllocatingLocked();



    if (mCore->mIsAbandoned) {

        BQ_LOGE("detachNextBuffer: BufferQueue has been abandoned");
@@ -468,6 +471,7 @@ status_t BufferQueueProducer::attachBuffer(int* outSlot, 
    }



    Mutex::Autolock lock(mCore->mMutex);

    mCore->waitWhileAllocatingLocked();



    status_t returnFlags = NO_ERROR;

    int found;
@@ -796,6 +800,7 @@ status_t BufferQueueProducer::disconnect(int api) { 
    sp<IConsumerListener> listener;

    { // Autolock scope

        Mutex::Autolock lock(mCore->mMutex);

        mCore->waitWhileAllocatingLocked();



        if (mCore->mIsAbandoned) {

            // It's not really an error to disconnect after the surface has
@@ -862,9 +867,17 @@ status_t BufferQueueProducer::setSidebandStream(const sp<NativeHandle>& stream) 


void BufferQueueProducer::allocateBuffers(bool async, uint32_t width,

        uint32_t height, uint32_t format, uint32_t usage) {

    ATRACE_CALL();

    while (true) {

        Vector<int> freeSlots;



        size_t newBufferCount = 0;

        uint32_t allocWidth = 0;

        uint32_t allocHeight = 0;

        uint32_t allocFormat = 0;

        uint32_t allocUsage = 0;

        { // Autolock scope

            Mutex::Autolock lock(mCore->mMutex);

            mCore->waitWhileAllocatingLocked();



            int currentBufferCount = 0;

            for (int slot = 0; slot < BufferQueueDefs::NUM_BUFFER_SLOTS; ++slot) {
@@ -884,33 +897,71 @@ void BufferQueueProducer::allocateBuffers(bool async, uint32_t width, 
            int maxBufferCount = mCore->getMaxBufferCountLocked(async);

            BQ_LOGV("allocateBuffers: allocating from %d buffers up to %d buffers",

                    currentBufferCount, maxBufferCount);

    for (; currentBufferCount < maxBufferCount; ++currentBufferCount) {

        if (freeSlots.empty()) {

            if (maxBufferCount <= currentBufferCount)

                return;

            newBufferCount = maxBufferCount - currentBufferCount;

            if (freeSlots.size() < newBufferCount) {

                BQ_LOGE("allocateBuffers: ran out of free slots");

                return;

            }

            allocWidth = width > 0 ? width : mCore->mDefaultWidth;

            allocHeight = height > 0 ? height : mCore->mDefaultHeight;

            allocFormat = format != 0 ? format : mCore->mDefaultBufferFormat;

            allocUsage = usage | mCore->mConsumerUsageBits;



        width = width > 0 ? width : mCore->mDefaultWidth;

        height = height > 0 ? height : mCore->mDefaultHeight;

        format = format != 0 ? format : mCore->mDefaultBufferFormat;

        usage |= mCore->mConsumerUsageBits;

            mCore->mIsAllocating = true;

        } // Autolock scope



        Vector<sp<GraphicBuffer> > buffers;

        for (size_t i = 0; i <  newBufferCount; ++i) {

            status_t result = NO_ERROR;

            sp<GraphicBuffer> graphicBuffer(mCore->mAllocator->createGraphicBuffer(

                width, height, format, usage, &result));

                    allocWidth, allocHeight, allocFormat, allocUsage, &result));

            if (result != NO_ERROR) {

                BQ_LOGE("allocateBuffers: failed to allocate buffer (%u x %u, format"

                        " %u, usage %u)", width, height, format, usage);

                Mutex::Autolock lock(mCore->mMutex);

                mCore->mIsAllocating = false;

                mCore->mIsAllocatingCondition.broadcast();

                return;

            }

            buffers.push_back(graphicBuffer);

        }



        int slot = freeSlots[freeSlots.size() - 1];

        { // Autolock scope

            Mutex::Autolock lock(mCore->mMutex);

            uint32_t checkWidth = width > 0 ? width : mCore->mDefaultWidth;

            uint32_t checkHeight = height > 0 ? height : mCore->mDefaultHeight;

            uint32_t checkFormat = format != 0 ? format : mCore->mDefaultBufferFormat;

            uint32_t checkUsage = usage | mCore->mConsumerUsageBits;

            if (checkWidth != allocWidth || checkHeight != allocHeight ||

                checkFormat != allocFormat || checkUsage != allocUsage) {

                // Something changed while we released the lock. Retry.

                BQ_LOGV("allocateBuffers: size/format/usage changed while allocating. Retrying.");

                mCore->mIsAllocating = false;

                mCore->mIsAllocatingCondition.broadcast();

                continue;

            }



            for (size_t i = 0; i < newBufferCount; ++i) {

                int slot = freeSlots[i];

                if (mSlots[slot].mBufferState != BufferSlot::FREE) {

                    // A consumer allocated the FREE slot with attachBuffer. Discard the buffer we

                    // allocated.

                    BQ_LOGV("allocateBuffers: slot %d was acquired while allocating. "

                            "Dropping allocated buffer.", slot);

                    continue;

                }

                mCore->freeBufferLocked(slot); // Clean up the slot first

        mSlots[slot].mGraphicBuffer = graphicBuffer;

                mSlots[slot].mGraphicBuffer = buffers[i];

                mSlots[slot].mFrameNumber = 0;

                mSlots[slot].mFence = Fence::NO_FENCE;

                BQ_LOGV("allocateBuffers: allocated a new buffer in slot %d", slot);

        freeSlots.pop();

            }



            mCore->mIsAllocating = false;

            mCore->mIsAllocatingCondition.broadcast();

        } // Autolock scope

    }

}