From 9de7293b0a1b01ebe6fb1ab4a498f144adc8029f Mon Sep 17 00:00:00 2001 From: Dan Stoza Date: Thu, 16 Apr 2015 17:28:43 -0700 Subject: [PATCH] libgui: Allow an IGBProducer to disable allocation Adds a new method IGBP::allowAllocation, which controls whether dequeueBuffer is permitted to allocate a new buffer. If allocation is disallowed, dequeueBuffer will block or return an error as it normally would (as controlled by *ControlledByApp). If there are free buffers, but they are not of the correct dimensions, format, or usage, they may be freed if a more suitable buffer is not found first. Bug: 19801715 Change-Id: I0d604958b78b2fd775c2547690301423f9a52165 --- include/gui/BufferQueueCore.h | 4 ++ include/gui/BufferQueueProducer.h | 3 + include/gui/IGraphicBufferProducer.h | 12 ++++ include/ui/GraphicBuffer.h | 3 + libs/gui/BufferQueueCore.cpp | 3 +- libs/gui/BufferQueueProducer.cpp | 68 +++++++++++++------ libs/gui/IGraphicBufferProducer.cpp | 23 ++++++- libs/gui/tests/BufferQueue_test.cpp | 36 ++++++++++ libs/ui/GraphicBuffer.cpp | 10 +++ .../DisplayHardware/VirtualDisplaySurface.cpp | 4 ++ .../DisplayHardware/VirtualDisplaySurface.h | 1 + services/surfaceflinger/MonitoredProducer.cpp | 4 ++ services/surfaceflinger/MonitoredProducer.h | 1 + 13 files changed, 150 insertions(+), 22 deletions(-) diff --git a/include/gui/BufferQueueCore.h b/include/gui/BufferQueueCore.h index a6065a9e9..9a43516d7 100644 --- a/include/gui/BufferQueueCore.h +++ b/include/gui/BufferQueueCore.h @@ -266,6 +266,10 @@ private: // mIsAllocatingCondition is a condition variable used by producers to wait until mIsAllocating // becomes false. mutable Condition mIsAllocatingCondition; + + // mAllowAllocation determines whether dequeueBuffer is allowed to allocate + // new buffers + bool mAllowAllocation; }; // class BufferQueueCore } // namespace android diff --git a/include/gui/BufferQueueProducer.h b/include/gui/BufferQueueProducer.h index f794ea34b..ed660fbf6 100644 --- a/include/gui/BufferQueueProducer.h +++ b/include/gui/BufferQueueProducer.h @@ -172,6 +172,9 @@ public: virtual void allocateBuffers(bool async, uint32_t width, uint32_t height, PixelFormat format, uint32_t usage); + // See IGraphicBufferProducer::allowAllocation + virtual status_t allowAllocation(bool allow); + private: // This is required by the IBinder::DeathRecipient interface virtual void binderDied(const wp& who); diff --git a/include/gui/IGraphicBufferProducer.h b/include/gui/IGraphicBufferProducer.h index 2d99f2452..5c50b2b39 100644 --- a/include/gui/IGraphicBufferProducer.h +++ b/include/gui/IGraphicBufferProducer.h @@ -458,6 +458,18 @@ public: // allocated, this function has no effect. virtual void allocateBuffers(bool async, uint32_t width, uint32_t height, PixelFormat format, uint32_t usage) = 0; + + // Sets whether dequeueBuffer is allowed to allocate new buffers. + // + // Normally dequeueBuffer does not discriminate between free slots which + // already have an allocated buffer and those which do not, and will + // allocate a new buffer if the slot doesn't have a buffer or if the slot's + // buffer doesn't match the requested size, format, or usage. This method + // allows the producer to restrict the eligible slots to those which already + // have an allocated buffer of the correct size, format, and usage. If no + // eligible slot is available, dequeueBuffer will block or return an error + // as usual. + virtual status_t allowAllocation(bool allow) = 0; }; // ---------------------------------------------------------------------------- diff --git a/include/ui/GraphicBuffer.h b/include/ui/GraphicBuffer.h index cea94fc7b..f91d1920c 100644 --- a/include/ui/GraphicBuffer.h +++ b/include/ui/GraphicBuffer.h @@ -97,6 +97,9 @@ public: status_t reallocate(uint32_t inWidth, uint32_t inHeight, PixelFormat inFormat, uint32_t inUsage); + bool needsReallocation(uint32_t inWidth, uint32_t inHeight, + PixelFormat inFormat, uint32_t inUsage); + status_t lock(uint32_t inUsage, void** vaddr); status_t lock(uint32_t inUsage, const Rect& rect, void** vaddr); // For HAL_PIXEL_FORMAT_YCbCr_420_888 diff --git a/libs/gui/BufferQueueCore.cpp b/libs/gui/BufferQueueCore.cpp index 29415c94c..bc75ca753 100644 --- a/libs/gui/BufferQueueCore.cpp +++ b/libs/gui/BufferQueueCore.cpp @@ -69,7 +69,8 @@ BufferQueueCore::BufferQueueCore(const sp& allocator) : mFrameCounter(0), mTransformHint(0), mIsAllocating(false), - mIsAllocatingCondition() + mIsAllocatingCondition(), + mAllowAllocation(true) { if (allocator == NULL) { sp composer(ComposerService::getComposerService()); diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp index a27d5f033..86e45c8b0 100644 --- a/libs/gui/BufferQueueProducer.cpp +++ b/libs/gui/BufferQueueProducer.cpp @@ -219,7 +219,7 @@ status_t BufferQueueProducer::waitForFreeSlotThenRelock(const char* caller, auto slot = mCore->mFreeBuffers.begin(); *found = *slot; mCore->mFreeBuffers.erase(slot); - } else if (!mCore->mFreeSlots.empty()) { + } else if (mCore->mAllowAllocation && !mCore->mFreeSlots.empty()) { auto slot = mCore->mFreeSlots.begin(); // Only return free slots up to the max buffer count if (*slot < maxBufferCount) { @@ -285,17 +285,39 @@ status_t BufferQueueProducer::dequeueBuffer(int *outSlot, // Enable the usage bits the consumer requested usage |= mCore->mConsumerUsageBits; - int found; - status_t status = waitForFreeSlotThenRelock("dequeueBuffer", async, - &found, &returnFlags); - if (status != NO_ERROR) { - return status; + const bool useDefaultSize = !width && !height; + if (useDefaultSize) { + width = mCore->mDefaultWidth; + height = mCore->mDefaultHeight; } - // This should not happen - if (found == BufferQueueCore::INVALID_BUFFER_SLOT) { - BQ_LOGE("dequeueBuffer: no available buffer slots"); - return -EBUSY; + int found = BufferItem::INVALID_BUFFER_SLOT; + while (found == BufferItem::INVALID_BUFFER_SLOT) { + status_t status = waitForFreeSlotThenRelock("dequeueBuffer", async, + &found, &returnFlags); + if (status != NO_ERROR) { + return status; + } + + // This should not happen + if (found == BufferQueueCore::INVALID_BUFFER_SLOT) { + BQ_LOGE("dequeueBuffer: no available buffer slots"); + return -EBUSY; + } + + const sp& buffer(mSlots[found].mGraphicBuffer); + + // If we are not allowed to allocate new buffers, + // waitForFreeSlotThenRelock must have returned a slot containing a + // buffer. If this buffer would require reallocation to meet the + // requested attributes, we free it and attempt to get another one. + if (!mCore->mAllowAllocation) { + if (buffer->needsReallocation(width, height, format, usage)) { + mCore->freeBufferLocked(found); + found = BufferItem::INVALID_BUFFER_SLOT; + continue; + } + } } *outSlot = found; @@ -303,20 +325,11 @@ status_t BufferQueueProducer::dequeueBuffer(int *outSlot, attachedByConsumer = mSlots[found].mAttachedByConsumer; - const bool useDefaultSize = !width && !height; - if (useDefaultSize) { - width = mCore->mDefaultWidth; - height = mCore->mDefaultHeight; - } - mSlots[found].mBufferState = BufferSlot::DEQUEUED; const sp& buffer(mSlots[found].mGraphicBuffer); if ((buffer == NULL) || - (static_cast(buffer->width) != width) || - (static_cast(buffer->height) != height) || - (buffer->format != format) || - ((static_cast(buffer->usage) & usage) != usage)) + buffer->needsReallocation(width, height, format, usage)) { mSlots[found].mAcquireCalled = false; mSlots[found].mGraphicBuffer = NULL; @@ -933,6 +946,12 @@ void BufferQueueProducer::allocateBuffers(bool async, uint32_t width, Mutex::Autolock lock(mCore->mMutex); mCore->waitWhileAllocatingLocked(); + if (!mCore->mAllowAllocation) { + BQ_LOGE("allocateBuffers: allocation is not allowed for this " + "BufferQueue"); + return; + } + int currentBufferCount = 0; for (int slot = 0; slot < BufferQueueDefs::NUM_BUFFER_SLOTS; ++slot) { if (mSlots[slot].mGraphicBuffer != NULL) { @@ -1027,6 +1046,15 @@ void BufferQueueProducer::allocateBuffers(bool async, uint32_t width, } } +status_t BufferQueueProducer::allowAllocation(bool allow) { + ATRACE_CALL(); + BQ_LOGV("allowAllocation: %s", allow ? "true" : "false"); + + Mutex::Autolock lock(mCore->mMutex); + mCore->mAllowAllocation = allow; + return NO_ERROR; +} + void BufferQueueProducer::binderDied(const wp& /* who */) { // If we're here, it means that a producer we were connected to died. // We're guaranteed that we are still connected to it because we remove diff --git a/libs/gui/IGraphicBufferProducer.cpp b/libs/gui/IGraphicBufferProducer.cpp index b7982a9f5..7093ffaa1 100644 --- a/libs/gui/IGraphicBufferProducer.cpp +++ b/libs/gui/IGraphicBufferProducer.cpp @@ -46,6 +46,7 @@ enum { DISCONNECT, SET_SIDEBAND_STREAM, ALLOCATE_BUFFERS, + ALLOW_ALLOCATION, }; class BpGraphicBufferProducer : public BpInterface @@ -271,6 +272,18 @@ public: ALOGE("allocateBuffers failed to transact: %d", result); } } + + virtual status_t allowAllocation(bool allow) { + Parcel data, reply; + data.writeInterfaceToken(IGraphicBufferProducer::getInterfaceDescriptor()); + data.writeInt32(static_cast(allow)); + status_t result = remote()->transact(ALLOW_ALLOCATION, data, &reply); + if (result != NO_ERROR) { + return result; + } + result = reply.readInt32(); + return result; + } }; // Out-of-line virtual method definition to trigger vtable emission in this @@ -418,7 +431,7 @@ status_t BnGraphicBufferProducer::onTransact( reply->writeInt32(result); return NO_ERROR; } - case ALLOCATE_BUFFERS: + case ALLOCATE_BUFFERS: { CHECK_INTERFACE(IGraphicBufferProducer, data, reply); bool async = static_cast(data.readInt32()); uint32_t width = data.readUint32(); @@ -427,6 +440,14 @@ status_t BnGraphicBufferProducer::onTransact( uint32_t usage = data.readUint32(); allocateBuffers(async, width, height, format, usage); return NO_ERROR; + } + case ALLOW_ALLOCATION: { + CHECK_INTERFACE(IGraphicBufferProducer, data, reply); + bool allow = static_cast(data.readInt32()); + status_t result = allowAllocation(allow); + reply->writeInt32(result); + return NO_ERROR; + } } return BBinder::onTransact(code, data, reply, flags); } diff --git a/libs/gui/tests/BufferQueue_test.cpp b/libs/gui/tests/BufferQueue_test.cpp index c38c79781..112dcb9c7 100644 --- a/libs/gui/tests/BufferQueue_test.cpp +++ b/libs/gui/tests/BufferQueue_test.cpp @@ -364,4 +364,40 @@ TEST_F(BufferQueueTest, MoveFromConsumerToProducer) { ASSERT_EQ(OK, item.mGraphicBuffer->unlock()); } +TEST_F(BufferQueueTest, TestDisallowingAllocation) { + createBufferQueue(); + sp dc(new DummyConsumer); + ASSERT_EQ(OK, mConsumer->consumerConnect(dc, true)); + IGraphicBufferProducer::QueueBufferOutput output; + ASSERT_EQ(OK, mProducer->connect(new DummyProducerListener, + NATIVE_WINDOW_API_CPU, true, &output)); + + static const uint32_t WIDTH = 320; + static const uint32_t HEIGHT = 240; + + ASSERT_EQ(OK, mConsumer->setDefaultBufferSize(WIDTH, HEIGHT)); + + int slot; + sp fence; + sp buffer; + // This should return an error since it would require an allocation + ASSERT_EQ(OK, mProducer->allowAllocation(false)); + ASSERT_EQ(WOULD_BLOCK, mProducer->dequeueBuffer(&slot, &fence, false, 0, 0, + 0, GRALLOC_USAGE_SW_WRITE_OFTEN)); + + // This should succeed, now that we've lifted the prohibition + ASSERT_EQ(OK, mProducer->allowAllocation(true)); + ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION, + mProducer->dequeueBuffer(&slot, &fence, false, 0, 0, 0, + GRALLOC_USAGE_SW_WRITE_OFTEN)); + + // Release the previous buffer back to the BufferQueue + mProducer->cancelBuffer(slot, fence); + + // This should fail since we're requesting a different size + ASSERT_EQ(OK, mProducer->allowAllocation(false)); + ASSERT_EQ(WOULD_BLOCK, mProducer->dequeueBuffer(&slot, &fence, false, + WIDTH * 2, HEIGHT * 2, 0, GRALLOC_USAGE_SW_WRITE_OFTEN)); +} + } // namespace android diff --git a/libs/ui/GraphicBuffer.cpp b/libs/ui/GraphicBuffer.cpp index 638ac6299..d51d514d6 100644 --- a/libs/ui/GraphicBuffer.cpp +++ b/libs/ui/GraphicBuffer.cpp @@ -152,6 +152,16 @@ status_t GraphicBuffer::reallocate(uint32_t inWidth, uint32_t inHeight, return initSize(inWidth, inHeight, inFormat, inUsage); } +bool GraphicBuffer::needsReallocation(uint32_t inWidth, uint32_t inHeight, + PixelFormat inFormat, uint32_t inUsage) +{ + if (static_cast(inWidth) != width) return true; + if (static_cast(inHeight) != height) return true; + if (inFormat != format) return true; + if ((static_cast(usage) & inUsage) != inUsage) return true; + return false; +} + status_t GraphicBuffer::initSize(uint32_t inWidth, uint32_t inHeight, PixelFormat inFormat, uint32_t inUsage) { diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp index 0e94f0d58..11cbdc6d9 100644 --- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp +++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp @@ -526,6 +526,10 @@ void VirtualDisplaySurface::allocateBuffers(bool /* async */, // TODO: Should we actually allocate buffers for a virtual display? } +status_t VirtualDisplaySurface::allowAllocation(bool /* allow */) { + return INVALID_OPERATION; +} + void VirtualDisplaySurface::updateQueueBufferOutput( const QueueBufferOutput& qbo) { uint32_t w, h, transformHint, numPendingBuffers; diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h index 0a3f4a157..97af9809a 100644 --- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h +++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h @@ -115,6 +115,7 @@ private: virtual status_t setSidebandStream(const sp& stream); virtual void allocateBuffers(bool async, uint32_t width, uint32_t height, PixelFormat format, uint32_t usage); + virtual status_t allowAllocation(bool allow); // // Utility methods diff --git a/services/surfaceflinger/MonitoredProducer.cpp b/services/surfaceflinger/MonitoredProducer.cpp index e4e7d4227..9fb555b47 100644 --- a/services/surfaceflinger/MonitoredProducer.cpp +++ b/services/surfaceflinger/MonitoredProducer.cpp @@ -110,6 +110,10 @@ void MonitoredProducer::allocateBuffers(bool async, uint32_t width, mProducer->allocateBuffers(async, width, height, format, usage); } +status_t MonitoredProducer::allowAllocation(bool allow) { + return mProducer->allowAllocation(allow); +} + IBinder* MonitoredProducer::onAsBinder() { return IInterface::asBinder(mProducer).get(); } diff --git a/services/surfaceflinger/MonitoredProducer.h b/services/surfaceflinger/MonitoredProducer.h index aec3e8502..b2f8293e6 100644 --- a/services/surfaceflinger/MonitoredProducer.h +++ b/services/surfaceflinger/MonitoredProducer.h @@ -53,6 +53,7 @@ public: virtual status_t setSidebandStream(const sp& stream); virtual void allocateBuffers(bool async, uint32_t width, uint32_t height, PixelFormat format, uint32_t usage); + virtual status_t allowAllocation(bool allow); virtual IBinder* onAsBinder(); private: