From 48a619f8332e06ea1cd96d82719cdf5e05c69630 Mon Sep 17 00:00:00 2001 From: Yi Kong Date: Tue, 5 Jun 2018 16:34:59 -0700 Subject: Replace NULL/0 with nullptr Fixes -Wzero-as-null-pointer-constant warning. clang-tidy -checks=modernize-use-nullptr -p compile_commands.json -fix ... Test: m Bug: 68236239 Change-Id: I3a8e982ba40f9b029bafef78437b146a878f56a9 --- libs/gui/ConsumerBase.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'libs/gui/ConsumerBase.cpp') diff --git a/libs/gui/ConsumerBase.cpp b/libs/gui/ConsumerBase.cpp index f9e292e199..abd9921fa9 100644 --- a/libs/gui/ConsumerBase.cpp +++ b/libs/gui/ConsumerBase.cpp @@ -96,7 +96,7 @@ void ConsumerBase::onLastStrongRef(const void* id __attribute__((unused))) { void ConsumerBase::freeBufferLocked(int slotIndex) { CB_LOGV("freeBufferLocked: slotIndex=%d", slotIndex); - mSlots[slotIndex].mGraphicBuffer = 0; + mSlots[slotIndex].mGraphicBuffer = nullptr; mSlots[slotIndex].mFence = Fence::NO_FENCE; mSlots[slotIndex].mFrameNumber = 0; } @@ -110,7 +110,7 @@ void ConsumerBase::onFrameAvailable(const BufferItem& item) { listener = mFrameAvailableListener.promote(); } - if (listener != NULL) { + if (listener != nullptr) { CB_LOGV("actually calling onFrameAvailable"); listener->onFrameAvailable(item); } @@ -125,7 +125,7 @@ void ConsumerBase::onFrameReplaced(const BufferItem &item) { listener = mFrameAvailableListener.promote(); } - if (listener != NULL) { + if (listener != nullptr) { CB_LOGV("actually calling onFrameReplaced"); listener->onFrameReplaced(item); } @@ -352,8 +352,8 @@ status_t ConsumerBase::acquireBufferLocked(BufferItem *item, return err; } - if (item->mGraphicBuffer != NULL) { - if (mSlots[item->mSlot].mGraphicBuffer != NULL) { + if (item->mGraphicBuffer != nullptr) { + if (mSlots[item->mSlot].mGraphicBuffer != nullptr) { freeBufferLocked(item->mSlot); } mSlots[item->mSlot].mGraphicBuffer = item->mGraphicBuffer; @@ -468,7 +468,7 @@ bool ConsumerBase::stillTracking(int slot, if (slot < 0 || slot >= BufferQueue::NUM_BUFFER_SLOTS) { return false; } - return (mSlots[slot].mGraphicBuffer != NULL && + return (mSlots[slot].mGraphicBuffer != nullptr && mSlots[slot].mGraphicBuffer->handle == graphicBuffer->handle); } -- cgit v1.2.3-59-g8ed1b From d7b3a8bcf9946a32213812a46f9c88a910151686 Mon Sep 17 00:00:00 2001 From: Alec Mouri Date: Thu, 21 Mar 2019 11:44:18 -0700 Subject: Create EGLImages during buffer allocation EGLImage creation is now performed on an async binder thread, so now GPU composition should rarely be stalled by expensive image creation. Bug: 129008989 Test: systrace Change-Id: I9732f866933a8950a4c69ff51d5ac1622bbb3470 --- libs/gui/BufferQueue.cpp | 7 ++++ libs/gui/BufferQueueProducer.cpp | 14 +++++++ libs/gui/ConsumerBase.cpp | 2 + libs/gui/IConsumerListener.cpp | 10 ++++- libs/gui/include/gui/BufferQueue.h | 1 + libs/gui/include/gui/ConsumerBase.h | 1 + libs/gui/include/gui/IConsumerListener.h | 7 ++++ libs/renderengine/gl/GLESRenderEngine.cpp | 43 +++++++++++++++++----- libs/renderengine/gl/GLESRenderEngine.h | 13 +++++-- .../include/renderengine/RenderEngine.h | 10 ++++- .../include/renderengine/mock/RenderEngine.h | 4 +- libs/renderengine/tests/RenderEngineTest.cpp | 14 +++++++ services/surfaceflinger/BufferLayerConsumer.cpp | 32 +++++++++++++++- services/surfaceflinger/BufferLayerConsumer.h | 19 +++++++--- 14 files changed, 152 insertions(+), 25 deletions(-) (limited to 'libs/gui/ConsumerBase.cpp') diff --git a/libs/gui/BufferQueue.cpp b/libs/gui/BufferQueue.cpp index 5fb3f0b80f..d87228fbbd 100644 --- a/libs/gui/BufferQueue.cpp +++ b/libs/gui/BufferQueue.cpp @@ -59,6 +59,13 @@ void BufferQueue::ProxyConsumerListener::onFrameReplaced( } } +void BufferQueue::ProxyConsumerListener::onBufferAllocated(const BufferItem& item) { + sp listener(mConsumerListener.promote()); + if (listener != nullptr) { + listener->onBufferAllocated(item); + } +} + void BufferQueue::ProxyConsumerListener::onBuffersReleased() { sp listener(mConsumerListener.promote()); if (listener != nullptr) { diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp index a462b0362f..e657488969 100644 --- a/libs/gui/BufferQueueProducer.cpp +++ b/libs/gui/BufferQueueProducer.cpp @@ -530,6 +530,13 @@ status_t BufferQueueProducer::dequeueBuffer(int* outSlot, sp* ou return NO_INIT; } + if (mCore->mConsumerListener != nullptr) { + BufferItem item; + item.mGraphicBuffer = graphicBuffer; + item.mSlot = *outSlot; + mCore->mConsumerListener->onBufferAllocated(item); + } + VALIDATE_CONSISTENCY(); } // Autolock scope } @@ -1414,6 +1421,13 @@ void BufferQueueProducer::allocateBuffers(uint32_t width, uint32_t height, BQ_LOGV("allocateBuffers: allocated a new buffer in slot %d", *slot); + if (mCore->mConsumerListener != nullptr) { + BufferItem item; + item.mGraphicBuffer = buffers[i]; + item.mSlot = *slot; + mCore->mConsumerListener->onBufferAllocated(item); + } + // Make sure the erase is done after all uses of the slot // iterator since it will be invalid after this point. mCore->mFreeSlots.erase(slot); diff --git a/libs/gui/ConsumerBase.cpp b/libs/gui/ConsumerBase.cpp index abd9921fa9..1e94cc13cd 100644 --- a/libs/gui/ConsumerBase.cpp +++ b/libs/gui/ConsumerBase.cpp @@ -131,6 +131,8 @@ void ConsumerBase::onFrameReplaced(const BufferItem &item) { } } +void ConsumerBase::onBufferAllocated(const BufferItem& /*item*/) {} + void ConsumerBase::onBuffersReleased() { Mutex::Autolock lock(mMutex); diff --git a/libs/gui/IConsumerListener.cpp b/libs/gui/IConsumerListener.cpp index 85ac304ab8..ea9045cad9 100644 --- a/libs/gui/IConsumerListener.cpp +++ b/libs/gui/IConsumerListener.cpp @@ -28,7 +28,8 @@ enum class Tag : uint32_t { ON_FRAME_REPLACED, ON_BUFFERS_RELEASED, ON_SIDEBAND_STREAM_CHANGED, - LAST = ON_SIDEBAND_STREAM_CHANGED, + ON_BUFFER_ALLOCATED, + LAST = ON_BUFFER_ALLOCATED, }; } // Anonymous namespace @@ -54,6 +55,11 @@ public: item); } + void onBufferAllocated(const BufferItem& item) override { + callRemoteAsync(Tag::ON_BUFFER_ALLOCATED, + item); + } + void onBuffersReleased() override { callRemoteAsync(Tag::ON_BUFFERS_RELEASED); } @@ -89,6 +95,8 @@ status_t BnConsumerListener::onTransact(uint32_t code, const Parcel& data, Parce return callLocalAsync(data, reply, &IConsumerListener::onFrameAvailable); case Tag::ON_FRAME_REPLACED: return callLocalAsync(data, reply, &IConsumerListener::onFrameReplaced); + case Tag::ON_BUFFER_ALLOCATED: + return callLocalAsync(data, reply, &IConsumerListener::onBufferAllocated); case Tag::ON_BUFFERS_RELEASED: return callLocalAsync(data, reply, &IConsumerListener::onBuffersReleased); case Tag::ON_SIDEBAND_STREAM_CHANGED: diff --git a/libs/gui/include/gui/BufferQueue.h b/libs/gui/include/gui/BufferQueue.h index da952744f3..721427be7b 100644 --- a/libs/gui/include/gui/BufferQueue.h +++ b/libs/gui/include/gui/BufferQueue.h @@ -61,6 +61,7 @@ public: void onDisconnect() override; void onFrameAvailable(const BufferItem& item) override; void onFrameReplaced(const BufferItem& item) override; + void onBufferAllocated(const BufferItem& item) override; void onBuffersReleased() override; void onSidebandStreamChanged() override; void addAndGetFrameTimestamps( diff --git a/libs/gui/include/gui/ConsumerBase.h b/libs/gui/include/gui/ConsumerBase.h index 366ced380b..7c26482509 100644 --- a/libs/gui/include/gui/ConsumerBase.h +++ b/libs/gui/include/gui/ConsumerBase.h @@ -141,6 +141,7 @@ protected: // classes if they want the notification. virtual void onFrameAvailable(const BufferItem& item) override; virtual void onFrameReplaced(const BufferItem& item) override; + virtual void onBufferAllocated(const BufferItem& item) override; virtual void onBuffersReleased() override; virtual void onSidebandStreamChanged() override; diff --git a/libs/gui/include/gui/IConsumerListener.h b/libs/gui/include/gui/IConsumerListener.h index c0828820e3..03fefbe90c 100644 --- a/libs/gui/include/gui/IConsumerListener.h +++ b/libs/gui/include/gui/IConsumerListener.h @@ -61,6 +61,13 @@ public: // This is called without any lock held and can be called concurrently by multiple threads. virtual void onFrameReplaced(const BufferItem& /* item */) {} /* Asynchronous */ + // onBufferAllocated is called to notify the buffer consumer that the BufferQueue has allocated + // a GraphicBuffer for a particular slot. Only the GraphicBuffer pointer and the slot ID will + // be populated. + // + // This is called without any lock held and can be called concurrently by multiple threads. + virtual void onBufferAllocated(const BufferItem& /* item */) {} /* Asynchronous */ + // onBuffersReleased is called to notify the buffer consumer that the BufferQueue has released // its references to one or more GraphicBuffers contained in its slots. The buffer consumer // should then call BufferQueue::getReleasedBuffers to retrieve the list of buffers. diff --git a/libs/renderengine/gl/GLESRenderEngine.cpp b/libs/renderengine/gl/GLESRenderEngine.cpp index 1980f50bac..fb71ce5332 100644 --- a/libs/renderengine/gl/GLESRenderEngine.cpp +++ b/libs/renderengine/gl/GLESRenderEngine.cpp @@ -626,23 +626,26 @@ void GLESRenderEngine::bindExternalTextureImage(uint32_t texName, const Image& i } } -status_t GLESRenderEngine::bindExternalTextureBuffer(uint32_t texName, sp buffer, - sp bufferFence) { +status_t GLESRenderEngine::cacheExternalTextureBuffer(const sp& buffer) { + std::lock_guard lock(mRenderingMutex); + return cacheExternalTextureBufferLocked(buffer); +} + +status_t GLESRenderEngine::bindExternalTextureBuffer(uint32_t texName, + const sp& buffer, + const sp& bufferFence) { std::lock_guard lock(mRenderingMutex); return bindExternalTextureBufferLocked(texName, buffer, bufferFence); } -status_t GLESRenderEngine::bindExternalTextureBufferLocked(uint32_t texName, - sp buffer, - sp bufferFence) { +status_t GLESRenderEngine::cacheExternalTextureBufferLocked(const sp& buffer) { if (buffer == nullptr) { return BAD_VALUE; } + ATRACE_CALL(); - auto cachedImage = mImageCache.find(buffer->getId()); - if (cachedImage != mImageCache.end()) { - bindExternalTextureImage(texName, *cachedImage->second); + if (mImageCache.count(buffer->getId()) > 0) { return NO_ERROR; } @@ -654,11 +657,32 @@ status_t GLESRenderEngine::bindExternalTextureBufferLocked(uint32_t texName, ALOGE("Failed to create image. size=%ux%u st=%u usage=%#" PRIx64 " fmt=%d", buffer->getWidth(), buffer->getHeight(), buffer->getStride(), buffer->getUsage(), buffer->getPixelFormat()); + return NO_INIT; + } + mImageCache.insert(std::make_pair(buffer->getId(), std::move(newImage))); + + return NO_ERROR; +} + +status_t GLESRenderEngine::bindExternalTextureBufferLocked(uint32_t texName, + const sp& buffer, + const sp& bufferFence) { + ATRACE_CALL(); + status_t cacheResult = cacheExternalTextureBufferLocked(buffer); + + if (cacheResult != NO_ERROR) { + return cacheResult; + } + + auto cachedImage = mImageCache.find(buffer->getId()); + + if (cachedImage == mImageCache.end()) { + // We failed creating the image if we got here, so bail out. bindExternalTextureImage(texName, *createImage()); return NO_INIT; } - bindExternalTextureImage(texName, *newImage); + bindExternalTextureImage(texName, *cachedImage->second); // Wait for the new buffer to be ready. if (bufferFence != nullptr && bufferFence->isValid()) { @@ -680,7 +704,6 @@ status_t GLESRenderEngine::bindExternalTextureBufferLocked(uint32_t texName, } } } - mImageCache.insert(std::make_pair(buffer->getId(), std::move(newImage))); return NO_ERROR; } diff --git a/libs/renderengine/gl/GLESRenderEngine.h b/libs/renderengine/gl/GLESRenderEngine.h index 8c8f308d3b..613629e4a1 100644 --- a/libs/renderengine/gl/GLESRenderEngine.h +++ b/libs/renderengine/gl/GLESRenderEngine.h @@ -72,8 +72,9 @@ public: void genTextures(size_t count, uint32_t* names) override; void deleteTextures(size_t count, uint32_t const* names) override; void bindExternalTextureImage(uint32_t texName, const Image& image) override; - status_t bindExternalTextureBuffer(uint32_t texName, sp buffer, sp fence) - EXCLUDES(mRenderingMutex); + status_t bindExternalTextureBuffer(uint32_t texName, const sp& buffer, + const sp& fence) EXCLUDES(mRenderingMutex); + status_t cacheExternalTextureBuffer(const sp& buffer) EXCLUDES(mRenderingMutex); void unbindExternalTextureBuffer(uint64_t bufferId) EXCLUDES(mRenderingMutex); status_t bindFrameBuffer(Framebuffer* framebuffer) override; void unbindFrameBuffer(Framebuffer* framebuffer) override; @@ -219,8 +220,12 @@ private: // See bindExternalTextureBuffer above, but requiring that mRenderingMutex // is held. - status_t bindExternalTextureBufferLocked(uint32_t texName, sp buffer, - sp fence) REQUIRES(mRenderingMutex); + status_t bindExternalTextureBufferLocked(uint32_t texName, const sp& buffer, + const sp& fence) REQUIRES(mRenderingMutex); + // See cacheExternalTextureBuffer above, but requiring that mRenderingMutex + // is held. + status_t cacheExternalTextureBufferLocked(const sp& buffer) + REQUIRES(mRenderingMutex); std::unique_ptr mDrawingBuffer; diff --git a/libs/renderengine/include/renderengine/RenderEngine.h b/libs/renderengine/include/renderengine/RenderEngine.h index b211551348..6773859eb7 100644 --- a/libs/renderengine/include/renderengine/RenderEngine.h +++ b/libs/renderengine/include/renderengine/RenderEngine.h @@ -106,8 +106,14 @@ public: virtual void genTextures(size_t count, uint32_t* names) = 0; virtual void deleteTextures(size_t count, uint32_t const* names) = 0; virtual void bindExternalTextureImage(uint32_t texName, const Image& image) = 0; - virtual status_t bindExternalTextureBuffer(uint32_t texName, sp buffer, - sp fence) = 0; + // Legacy public method used by devices that don't support native fence + // synchronization in their GPU driver, as this method provides implicit + // synchronization for latching buffers. + virtual status_t bindExternalTextureBuffer(uint32_t texName, const sp& buffer, + const sp& fence) = 0; + // Caches Image resources for this buffer, but does not bind the buffer to + // a particular texture. + virtual status_t cacheExternalTextureBuffer(const sp& buffer) = 0; // Removes internal resources referenced by the bufferId. This method should be // invoked when the caller will no longer hold a reference to a GraphicBuffer // and needs to clean up its resources. diff --git a/libs/renderengine/include/renderengine/mock/RenderEngine.h b/libs/renderengine/include/renderengine/mock/RenderEngine.h index 479c7ac035..4f7d9f4352 100644 --- a/libs/renderengine/include/renderengine/mock/RenderEngine.h +++ b/libs/renderengine/include/renderengine/mock/RenderEngine.h @@ -51,7 +51,9 @@ public: MOCK_METHOD2(genTextures, void(size_t, uint32_t*)); MOCK_METHOD2(deleteTextures, void(size_t, uint32_t const*)); MOCK_METHOD2(bindExternalTextureImage, void(uint32_t, const renderengine::Image&)); - MOCK_METHOD3(bindExternalTextureBuffer, status_t(uint32_t, sp, sp)); + MOCK_METHOD1(cacheExternalTextureBuffer, status_t(const sp&)); + MOCK_METHOD3(bindExternalTextureBuffer, + status_t(uint32_t, const sp&, const sp&)); MOCK_METHOD1(unbindExternalTextureBuffer, void(uint64_t)); MOCK_CONST_METHOD0(checkErrors, void()); MOCK_METHOD4(setViewportAndProjection, diff --git a/libs/renderengine/tests/RenderEngineTest.cpp b/libs/renderengine/tests/RenderEngineTest.cpp index 8c93cf432c..24904cfaa7 100644 --- a/libs/renderengine/tests/RenderEngineTest.cpp +++ b/libs/renderengine/tests/RenderEngineTest.cpp @@ -1003,4 +1003,18 @@ TEST_F(RenderEngineTest, drawLayers_bindExternalBufferCachesImages) { EXPECT_FALSE(sRE->isImageCachedForTesting(bufferId)); } +TEST_F(RenderEngineTest, drawLayers_cacheExternalBufferWithNullBuffer) { + status_t result = sRE->cacheExternalTextureBuffer(nullptr); + ASSERT_EQ(BAD_VALUE, result); +} + +TEST_F(RenderEngineTest, drawLayers_cacheExternalBufferCachesImages) { + sp buf = allocateSourceBuffer(1, 1); + uint64_t bufferId = buf->getId(); + sRE->cacheExternalTextureBuffer(buf); + EXPECT_TRUE(sRE->isImageCachedForTesting(bufferId)); + sRE->unbindExternalTextureBuffer(bufferId); + EXPECT_FALSE(sRE->isImageCachedForTesting(bufferId)); +} + } // namespace android diff --git a/services/surfaceflinger/BufferLayerConsumer.cpp b/services/surfaceflinger/BufferLayerConsumer.cpp index f2d4c5113f..fc98dc836a 100644 --- a/services/surfaceflinger/BufferLayerConsumer.cpp +++ b/services/surfaceflinger/BufferLayerConsumer.cpp @@ -217,7 +217,11 @@ status_t BufferLayerConsumer::acquireBufferLocked(BufferItem* item, nsecs_t pres // If item->mGraphicBuffer is not null, this buffer has not been acquired // before, so we need to clean up old references. if (item->mGraphicBuffer != nullptr) { - mImages[item->mSlot] = std::make_shared(item->mGraphicBuffer, mRE); + std::lock_guard lock(mImagesMutex); + if (mImages[item->mSlot] == nullptr || mImages[item->mSlot]->graphicBuffer() == nullptr || + mImages[item->mSlot]->graphicBuffer()->getId() != item->mGraphicBuffer->getId()) { + mImages[item->mSlot] = std::make_shared(item->mGraphicBuffer, mRE); + } } return NO_ERROR; @@ -238,7 +242,12 @@ status_t BufferLayerConsumer::updateAndReleaseLocked(const BufferItem& item, // Hang onto the pointer so that it isn't freed in the call to // releaseBufferLocked() if we're in shared buffer mode and both buffers are // the same. - std::shared_ptr nextTextureBuffer = mImages[slot]; + + std::shared_ptr nextTextureBuffer; + { + std::lock_guard lock(mImagesMutex); + nextTextureBuffer = mImages[slot]; + } // release old buffer if (mCurrentTexture != BufferQueue::INVALID_BUFFER_SLOT) { @@ -436,6 +445,7 @@ status_t BufferLayerConsumer::doFenceWaitLocked() const { void BufferLayerConsumer::freeBufferLocked(int slotIndex) { BLC_LOGV("freeBufferLocked: slotIndex=%d", slotIndex); + std::lock_guard lock(mImagesMutex); if (slotIndex == mCurrentTexture) { mCurrentTexture = BufferQueue::INVALID_BUFFER_SLOT; } @@ -468,6 +478,23 @@ void BufferLayerConsumer::onSidebandStreamChanged() { } } +void BufferLayerConsumer::onBufferAllocated(const BufferItem& item) { + if (item.mGraphicBuffer != nullptr) { + std::shared_ptr image = std::make_shared(item.mGraphicBuffer, mRE); + std::shared_ptr oldImage; + { + std::lock_guard lock(mImagesMutex); + oldImage = mImages[item.mSlot]; + if (oldImage == nullptr || oldImage->graphicBuffer() == nullptr || + oldImage->graphicBuffer()->getId() != item.mGraphicBuffer->getId()) { + mImages[item.mSlot] = std::make_shared(item.mGraphicBuffer, mRE); + } + image = mImages[item.mSlot]; + } + mRE.cacheExternalTextureBuffer(image->graphicBuffer()); + } +} + void BufferLayerConsumer::addAndGetFrameTimestamps(const NewFrameEventsEntry* newTimestamps, FrameEventHistoryDelta* outDelta) { sp l = mLayer.promote(); @@ -480,6 +507,7 @@ void BufferLayerConsumer::abandonLocked() { BLC_LOGV("abandonLocked"); mCurrentTextureBuffer = nullptr; for (int i = 0; i < BufferQueue::NUM_BUFFER_SLOTS; i++) { + std::lock_guard lock(mImagesMutex); mImages[i] = nullptr; } ConsumerBase::abandonLocked(); diff --git a/services/surfaceflinger/BufferLayerConsumer.h b/services/surfaceflinger/BufferLayerConsumer.h index f4ca846ea3..0f0655d705 100644 --- a/services/surfaceflinger/BufferLayerConsumer.h +++ b/services/surfaceflinger/BufferLayerConsumer.h @@ -17,6 +17,7 @@ #ifndef ANDROID_BUFFERLAYERCONSUMER_H #define ANDROID_BUFFERLAYERCONSUMER_H +#include #include #include #include @@ -179,7 +180,7 @@ public: protected: // abandonLocked overrides the ConsumerBase method to clear // mCurrentTextureImage in addition to the ConsumerBase behavior. - virtual void abandonLocked(); + virtual void abandonLocked() EXCLUDES(mImagesMutex); // dumpLocked overrides the ConsumerBase method to dump BufferLayerConsumer- // specific info in addition to the ConsumerBase behavior. @@ -187,7 +188,8 @@ protected: // See ConsumerBase::acquireBufferLocked virtual status_t acquireBufferLocked(BufferItem* item, nsecs_t presentWhen, - uint64_t maxFrameNumber = 0) override; + uint64_t maxFrameNumber = 0) override + EXCLUDES(mImagesMutex); bool canUseImageCrop(const Rect& crop) const; @@ -206,7 +208,8 @@ protected: // completion of the method will instead be returned to the caller, so that // it may call releaseBufferLocked itself later. status_t updateAndReleaseLocked(const BufferItem& item, - PendingRelease* pendingRelease = nullptr); + PendingRelease* pendingRelease = nullptr) + EXCLUDES(mImagesMutex); // Binds mTexName and the current buffer to TEXTURE_EXTERNAL target. // If the bind succeeds, this calls doFenceWait. @@ -234,10 +237,11 @@ private: // that slot. Otherwise it has no effect. // // This method must be called with mMutex locked. - virtual void freeBufferLocked(int slotIndex); + virtual void freeBufferLocked(int slotIndex) EXCLUDES(mImagesMutex); // IConsumerListener interface void onDisconnect() override; + void onBufferAllocated(const BufferItem& item) override EXCLUDES(mImagesMutex); void onSidebandStreamChanged() override; void addAndGetFrameTimestamps(const NewFrameEventsEntry* newTimestamps, FrameEventHistoryDelta* outDelta) override; @@ -344,7 +348,12 @@ private: int mCurrentTexture; // Shadow buffer cache for cleaning up renderengine references. - std::shared_ptr mImages[BufferQueueDefs::NUM_BUFFER_SLOTS]; + std::shared_ptr mImages[BufferQueueDefs::NUM_BUFFER_SLOTS] GUARDED_BY(mImagesMutex); + + // Separate mutex guarding the shadow buffer cache. + // mImagesMutex can be manipulated with binder threads (e.g. onBuffersAllocated) + // which is contentious enough that we can't just use mMutex. + mutable std::mutex mImagesMutex; // A release that is pending on the receipt of a new release fence from // presentDisplay -- cgit v1.2.3-59-g8ed1b From 485e4c358b26f09ec0634e63c311131c9507779a Mon Sep 17 00:00:00 2001 From: Alec Mouri Date: Tue, 30 Apr 2019 18:24:05 -0700 Subject: Clean-up egl image preallocation Allocate EGL images in onFrameAvailable, instead of a custom onBuffersAllocated callback. This way we reduce traffic over binder while still performing GL work ahead of time in queueBuffer(). Bug: 130567928 Test: systrace Change-Id: I4070e9ddbd379dac3d809d0e7edb2855fc8b7a80 --- libs/gui/BufferQueue.cpp | 7 ------- libs/gui/BufferQueueProducer.cpp | 17 ----------------- libs/gui/ConsumerBase.cpp | 2 -- libs/gui/IConsumerListener.cpp | 10 +--------- libs/gui/include/gui/BufferQueue.h | 1 - libs/gui/include/gui/ConsumerBase.h | 1 - libs/gui/include/gui/IConsumerListener.h | 7 ------- services/surfaceflinger/BufferLayerConsumer.cpp | 22 ++++++++-------------- services/surfaceflinger/BufferLayerConsumer.h | 2 +- services/surfaceflinger/BufferQueueLayer.cpp | 7 +++++++ 10 files changed, 17 insertions(+), 59 deletions(-) (limited to 'libs/gui/ConsumerBase.cpp') diff --git a/libs/gui/BufferQueue.cpp b/libs/gui/BufferQueue.cpp index d87228fbbd..5fb3f0b80f 100644 --- a/libs/gui/BufferQueue.cpp +++ b/libs/gui/BufferQueue.cpp @@ -59,13 +59,6 @@ void BufferQueue::ProxyConsumerListener::onFrameReplaced( } } -void BufferQueue::ProxyConsumerListener::onBufferAllocated(const BufferItem& item) { - sp listener(mConsumerListener.promote()); - if (listener != nullptr) { - listener->onBufferAllocated(item); - } -} - void BufferQueue::ProxyConsumerListener::onBuffersReleased() { sp listener(mConsumerListener.promote()); if (listener != nullptr) { diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp index c94c6b31c6..3928bb9e40 100644 --- a/libs/gui/BufferQueueProducer.cpp +++ b/libs/gui/BufferQueueProducer.cpp @@ -539,13 +539,6 @@ status_t BufferQueueProducer::dequeueBuffer(int* outSlot, sp* ou return NO_INIT; } - if (mCore->mConsumerListener != nullptr) { - BufferItem item; - item.mGraphicBuffer = graphicBuffer; - item.mSlot = *outSlot; - mCore->mConsumerListener->onBufferAllocated(item); - } - VALIDATE_CONSISTENCY(); } // Autolock scope } @@ -975,9 +968,6 @@ status_t BufferQueueProducer::queueBuffer(int slot, item.mGraphicBuffer.clear(); } - // Don't send the slot number through the callback since the consumer shouldn't need it - item.mSlot = BufferItem::INVALID_BUFFER_SLOT; - // Call back without the main BufferQueue lock held, but with the callback // lock held so we can ensure that callbacks occur in order @@ -1433,13 +1423,6 @@ void BufferQueueProducer::allocateBuffers(uint32_t width, uint32_t height, BQ_LOGV("allocateBuffers: allocated a new buffer in slot %d", *slot); - if (mCore->mConsumerListener != nullptr) { - BufferItem item; - item.mGraphicBuffer = buffers[i]; - item.mSlot = *slot; - mCore->mConsumerListener->onBufferAllocated(item); - } - // Make sure the erase is done after all uses of the slot // iterator since it will be invalid after this point. mCore->mFreeSlots.erase(slot); diff --git a/libs/gui/ConsumerBase.cpp b/libs/gui/ConsumerBase.cpp index 1e94cc13cd..abd9921fa9 100644 --- a/libs/gui/ConsumerBase.cpp +++ b/libs/gui/ConsumerBase.cpp @@ -131,8 +131,6 @@ void ConsumerBase::onFrameReplaced(const BufferItem &item) { } } -void ConsumerBase::onBufferAllocated(const BufferItem& /*item*/) {} - void ConsumerBase::onBuffersReleased() { Mutex::Autolock lock(mMutex); diff --git a/libs/gui/IConsumerListener.cpp b/libs/gui/IConsumerListener.cpp index ea9045cad9..85ac304ab8 100644 --- a/libs/gui/IConsumerListener.cpp +++ b/libs/gui/IConsumerListener.cpp @@ -28,8 +28,7 @@ enum class Tag : uint32_t { ON_FRAME_REPLACED, ON_BUFFERS_RELEASED, ON_SIDEBAND_STREAM_CHANGED, - ON_BUFFER_ALLOCATED, - LAST = ON_BUFFER_ALLOCATED, + LAST = ON_SIDEBAND_STREAM_CHANGED, }; } // Anonymous namespace @@ -55,11 +54,6 @@ public: item); } - void onBufferAllocated(const BufferItem& item) override { - callRemoteAsync(Tag::ON_BUFFER_ALLOCATED, - item); - } - void onBuffersReleased() override { callRemoteAsync(Tag::ON_BUFFERS_RELEASED); } @@ -95,8 +89,6 @@ status_t BnConsumerListener::onTransact(uint32_t code, const Parcel& data, Parce return callLocalAsync(data, reply, &IConsumerListener::onFrameAvailable); case Tag::ON_FRAME_REPLACED: return callLocalAsync(data, reply, &IConsumerListener::onFrameReplaced); - case Tag::ON_BUFFER_ALLOCATED: - return callLocalAsync(data, reply, &IConsumerListener::onBufferAllocated); case Tag::ON_BUFFERS_RELEASED: return callLocalAsync(data, reply, &IConsumerListener::onBuffersReleased); case Tag::ON_SIDEBAND_STREAM_CHANGED: diff --git a/libs/gui/include/gui/BufferQueue.h b/libs/gui/include/gui/BufferQueue.h index 721427be7b..da952744f3 100644 --- a/libs/gui/include/gui/BufferQueue.h +++ b/libs/gui/include/gui/BufferQueue.h @@ -61,7 +61,6 @@ public: void onDisconnect() override; void onFrameAvailable(const BufferItem& item) override; void onFrameReplaced(const BufferItem& item) override; - void onBufferAllocated(const BufferItem& item) override; void onBuffersReleased() override; void onSidebandStreamChanged() override; void addAndGetFrameTimestamps( diff --git a/libs/gui/include/gui/ConsumerBase.h b/libs/gui/include/gui/ConsumerBase.h index 7c26482509..366ced380b 100644 --- a/libs/gui/include/gui/ConsumerBase.h +++ b/libs/gui/include/gui/ConsumerBase.h @@ -141,7 +141,6 @@ protected: // classes if they want the notification. virtual void onFrameAvailable(const BufferItem& item) override; virtual void onFrameReplaced(const BufferItem& item) override; - virtual void onBufferAllocated(const BufferItem& item) override; virtual void onBuffersReleased() override; virtual void onSidebandStreamChanged() override; diff --git a/libs/gui/include/gui/IConsumerListener.h b/libs/gui/include/gui/IConsumerListener.h index 03fefbe90c..c0828820e3 100644 --- a/libs/gui/include/gui/IConsumerListener.h +++ b/libs/gui/include/gui/IConsumerListener.h @@ -61,13 +61,6 @@ public: // This is called without any lock held and can be called concurrently by multiple threads. virtual void onFrameReplaced(const BufferItem& /* item */) {} /* Asynchronous */ - // onBufferAllocated is called to notify the buffer consumer that the BufferQueue has allocated - // a GraphicBuffer for a particular slot. Only the GraphicBuffer pointer and the slot ID will - // be populated. - // - // This is called without any lock held and can be called concurrently by multiple threads. - virtual void onBufferAllocated(const BufferItem& /* item */) {} /* Asynchronous */ - // onBuffersReleased is called to notify the buffer consumer that the BufferQueue has released // its references to one or more GraphicBuffers contained in its slots. The buffer consumer // should then call BufferQueue::getReleasedBuffers to retrieve the list of buffers. diff --git a/services/surfaceflinger/BufferLayerConsumer.cpp b/services/surfaceflinger/BufferLayerConsumer.cpp index fc98dc836a..6709fb4b48 100644 --- a/services/surfaceflinger/BufferLayerConsumer.cpp +++ b/services/surfaceflinger/BufferLayerConsumer.cpp @@ -478,20 +478,15 @@ void BufferLayerConsumer::onSidebandStreamChanged() { } } -void BufferLayerConsumer::onBufferAllocated(const BufferItem& item) { - if (item.mGraphicBuffer != nullptr) { - std::shared_ptr image = std::make_shared(item.mGraphicBuffer, mRE); - std::shared_ptr oldImage; - { - std::lock_guard lock(mImagesMutex); - oldImage = mImages[item.mSlot]; - if (oldImage == nullptr || oldImage->graphicBuffer() == nullptr || - oldImage->graphicBuffer()->getId() != item.mGraphicBuffer->getId()) { - mImages[item.mSlot] = std::make_shared(item.mGraphicBuffer, mRE); - } - image = mImages[item.mSlot]; +void BufferLayerConsumer::onBufferAvailable(const BufferItem& item) { + if (item.mGraphicBuffer != nullptr && item.mSlot != BufferQueue::INVALID_BUFFER_SLOT) { + std::lock_guard lock(mImagesMutex); + const std::shared_ptr& oldImage = mImages[item.mSlot]; + if (oldImage == nullptr || oldImage->graphicBuffer() == nullptr || + oldImage->graphicBuffer()->getId() != item.mGraphicBuffer->getId()) { + mImages[item.mSlot] = std::make_shared(item.mGraphicBuffer, mRE); + mRE.cacheExternalTextureBuffer(item.mGraphicBuffer); } - mRE.cacheExternalTextureBuffer(image->graphicBuffer()); } } @@ -533,5 +528,4 @@ BufferLayerConsumer::Image::~Image() { mRE.unbindExternalTextureBuffer(mGraphicBuffer->getId()); } } - }; // namespace android diff --git a/services/surfaceflinger/BufferLayerConsumer.h b/services/surfaceflinger/BufferLayerConsumer.h index 0f0655d705..e3f6100c35 100644 --- a/services/surfaceflinger/BufferLayerConsumer.h +++ b/services/surfaceflinger/BufferLayerConsumer.h @@ -176,6 +176,7 @@ public: // setConsumerUsageBits overrides the ConsumerBase method to OR // DEFAULT_USAGE_FLAGS to usage. status_t setConsumerUsageBits(uint64_t usage); + void onBufferAvailable(const BufferItem& item) EXCLUDES(mImagesMutex); protected: // abandonLocked overrides the ConsumerBase method to clear @@ -241,7 +242,6 @@ private: // IConsumerListener interface void onDisconnect() override; - void onBufferAllocated(const BufferItem& item) override EXCLUDES(mImagesMutex); void onSidebandStreamChanged() override; void addAndGetFrameTimestamps(const NewFrameEventsEntry* newTimestamps, FrameEventHistoryDelta* outDelta) override; diff --git a/services/surfaceflinger/BufferQueueLayer.cpp b/services/surfaceflinger/BufferQueueLayer.cpp index 5d729f5b4f..3d51ec33b2 100644 --- a/services/surfaceflinger/BufferQueueLayer.cpp +++ b/services/surfaceflinger/BufferQueueLayer.cpp @@ -14,6 +14,9 @@ * limitations under the License. */ +#undef LOG_TAG +#define LOG_TAG "BufferQueueLayer" +#define ATRACE_TAG ATRACE_TAG_GRAPHICS #include #include #include @@ -435,6 +438,7 @@ void BufferQueueLayer::fakeVsync() { } void BufferQueueLayer::onFrameAvailable(const BufferItem& item) { + ATRACE_CALL(); // Add this buffer from our internal queue tracker { // Autolock scope if (mFlinger->mUseSmart90ForVideo) { @@ -475,9 +479,11 @@ void BufferQueueLayer::onFrameAvailable(const BufferItem& item) { } else { mFlinger->signalLayerUpdate(); } + mConsumer->onBufferAvailable(item); } void BufferQueueLayer::onFrameReplaced(const BufferItem& item) { + ATRACE_CALL(); { // Autolock scope Mutex::Autolock lock(mQueueItemLock); @@ -499,6 +505,7 @@ void BufferQueueLayer::onFrameReplaced(const BufferItem& item) { mLastFrameNumberReceived = item.mFrameNumber; mQueueItemCondition.broadcast(); } + mConsumer->onBufferAvailable(item); } void BufferQueueLayer::onSidebandStreamChanged() { -- cgit v1.2.3-59-g8ed1b