From 3277faf86d598a2ee1080b0bb4dd476fe1fddba8 Mon Sep 17 00:00:00 2001 From: chaviw Date: Wed, 19 May 2021 16:45:23 -0500 Subject: Renamed and moved InputWindow and related files In preparation for the hierarchy listener interface, moved the InputWindow structs into libgui and have libinput dependant on libgui. Also renamed InputWindow to exclude Input since it will be used for more generic purposes. Test: Builds and flashes Bug: 188792659 Change-Id: I24262cbc14d409c00273de0024a672394a959e5f Merged-In: I24262cbc14d409c00273de0024a672394a959e5f --- libs/gui/BLASTBufferQueue.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 29701168e7..dbb1cb0c17 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -38,7 +38,7 @@ using namespace std::chrono_literals; namespace { -inline const char* toString(bool b) { +inline const char* boolToString(bool b) { return b ? "true" : "false"; } } // namespace @@ -513,7 +513,7 @@ void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) { BQA_LOGV("processNextBufferLocked size=%dx%d mFrameNumber=%" PRIu64 " applyTransaction=%s mTimestamp=%" PRId64 "%s mPendingTransactions.size=%d" " graphicBufferId=%" PRIu64 "%s", - mSize.width, mSize.height, bufferItem.mFrameNumber, toString(applyTransaction), + mSize.width, mSize.height, bufferItem.mFrameNumber, boolToString(applyTransaction), bufferItem.mTimestamp, bufferItem.mIsAutoTimestamp ? "(auto)" : "", static_cast(mPendingTransactions.size()), bufferItem.mGraphicBuffer->getId(), bufferItem.mAutoRefresh ? " mAutoRefresh" : ""); @@ -543,7 +543,7 @@ void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) { mNumFrameAvailable + mNumAcquired - mPendingRelease.size()); BQA_LOGV("onFrameAvailable framenumber=%" PRIu64 " nextTransactionSet=%s", item.mFrameNumber, - toString(nextTransactionSet)); + boolToString(nextTransactionSet)); processNextBufferLocked(nextTransactionSet /* useNextTransaction */); } -- cgit v1.2.3-59-g8ed1b From a4fbca56dac64a561adba7348bf55fb429887042 Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Wed, 7 Jul 2021 16:52:34 -0700 Subject: BlastBufferQueue: Keep transform hint in Surface consistent We should only update the transform hint in Surface when connecting to the queue or queuing a buffer. Otherwise, the transform hint value used when dequeuing the buffer will not align with the value queried from the Surface. This inconsistency was causing BBQ to reject buffers and we suspect this was causing an issue where BBQ will remain in this inconsistent state rejecting all buffers until the app changed orientation. Test: rotate device, check for rejected buffers, run through scenario in bug Bug: b/191841127 Change-Id: Id84a3a650ce68878e5638b942249c11558fd29eb --- libs/gui/BLASTBufferQueue.cpp | 21 +++++++-------- libs/gui/tests/BLASTBufferQueue_test.cpp | 46 ++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 12 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index ebc436f913..d9024fa672 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -171,6 +171,8 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name, const sp& surface, uint32_t width, uint32_t height, int32_t format) { std::unique_lock _lock{mMutex}; - BQA_LOGV("update width=%d height=%d format=%d", width, height, format); if (mFormat != format) { mFormat = format; mBufferItemConsumer->setDefaultBufferFormat(convertBufferFormat(format)); @@ -212,6 +213,8 @@ void BLASTBufferQueue::update(const sp& surface, uint32_t width, mTransformHint = mSurfaceControl->getTransformHint(); mBufferItemConsumer->setTransformHint(mTransformHint); } + BQA_LOGV("update width=%d height=%d format=%d mTransformHint=%d", width, height, format, + mTransformHint); ui::Size newSize(width, height); if (mRequestedSize != newSize) { @@ -267,6 +270,7 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const spsetTransformHint(mTransformHint); + BQA_LOGV("updated mTransformHint=%d", mTransformHint); // Update frametime stamps if the frame was latched and presented, indicated by a // valid latch time. if (stat.latchTime > 0) { @@ -339,6 +343,7 @@ void BLASTBufferQueue::releaseBufferCallback(const ReleaseCallbackId& id, mTransformHint = transformHint; mSurfaceControl->setTransformHint(transformHint); mBufferItemConsumer->setTransformHint(mTransformHint); + BQA_LOGV("updated mTransformHint=%d", mTransformHint); } // Calculate how many buffers we need to hold before we release them back @@ -422,7 +427,7 @@ void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) { } if (rejectBuffer(bufferItem)) { - BQA_LOGE("rejecting buffer:active_size=%dx%d, requested_size=%dx%d" + BQA_LOGE("rejecting buffer:active_size=%dx%d, requested_size=%dx%d " "buffer{size=%dx%d transform=%d}", mSize.width, mSize.height, mRequestedSize.width, mRequestedSize.height, buffer->getWidth(), buffer->getHeight(), bufferItem.mTransform); @@ -515,11 +520,11 @@ void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) { BQA_LOGV("processNextBufferLocked size=%dx%d mFrameNumber=%" PRIu64 " applyTransaction=%s mTimestamp=%" PRId64 "%s mPendingTransactions.size=%d" - " graphicBufferId=%" PRIu64 "%s", + " graphicBufferId=%" PRIu64 "%s transform=%d", mSize.width, mSize.height, bufferItem.mFrameNumber, boolToString(applyTransaction), bufferItem.mTimestamp, bufferItem.mIsAutoTimestamp ? "(auto)" : "", static_cast(mPendingTransactions.size()), bufferItem.mGraphicBuffer->getId(), - bufferItem.mAutoRefresh ? " mAutoRefresh" : ""); + bufferItem.mAutoRefresh ? " mAutoRefresh" : "", bufferItem.mTransform); } Rect BLASTBufferQueue::computeCrop(const BufferItem& item) { @@ -646,14 +651,6 @@ public: status_t setFrameTimelineInfo(const FrameTimelineInfo& frameTimelineInfo) override { return mBbq->setFrameTimelineInfo(frameTimelineInfo); } - protected: - uint32_t getTransformHint() const override { - if (mStickyTransform == 0 && !transformToDisplayInverse()) { - return mBbq->getLastTransformHint(); - } else { - return 0; - } - } }; // TODO: Can we coalesce this with frame updates? Need to confirm diff --git a/libs/gui/tests/BLASTBufferQueue_test.cpp b/libs/gui/tests/BLASTBufferQueue_test.cpp index 6ff67aa7cb..9082d275a2 100644 --- a/libs/gui/tests/BLASTBufferQueue_test.cpp +++ b/libs/gui/tests/BLASTBufferQueue_test.cpp @@ -71,6 +71,10 @@ public: return mBlastBufferQueueAdapter->mSurfaceControl; } + sp getSurface() { + return mBlastBufferQueueAdapter->getSurface(false /* includeSurfaceControlHandle */); + } + void waitForCallbacks() { std::unique_lock lock{mBlastBufferQueueAdapter->mMutex}; // Wait until all but one of the submitted buffers have been released. @@ -758,6 +762,48 @@ TEST_F(BLASTBufferQueueTest, OutOfOrderTransactionTest) { {0, 0, (int32_t)mDisplayWidth, (int32_t)mDisplayHeight / 2})); } +TEST_F(BLASTBufferQueueTest, TransformHint) { + // Transform hint is provided to BBQ via the surface control passed by WM + mSurfaceControl->setTransformHint(ui::Transform::ROT_90); + + BLASTBufferQueueHelper adapter(mSurfaceControl, mDisplayWidth, mDisplayHeight); + sp igbProducer = adapter.getIGraphicBufferProducer(); + ASSERT_NE(nullptr, igbProducer.get()); + ASSERT_EQ(NO_ERROR, igbProducer->setMaxDequeuedBufferCount(2)); + sp surface = adapter.getSurface(); + + // Before connecting to the surface, we do not get a valid transform hint + int transformHint; + surface->query(NATIVE_WINDOW_TRANSFORM_HINT, &transformHint); + ASSERT_EQ(ui::Transform::ROT_0, transformHint); + + ASSERT_EQ(NO_ERROR, + surface->connect(NATIVE_WINDOW_API_CPU, new TestProducerListener(igbProducer))); + + // After connecting to the surface, we should get the correct hint. + surface->query(NATIVE_WINDOW_TRANSFORM_HINT, &transformHint); + ASSERT_EQ(ui::Transform::ROT_90, transformHint); + + ANativeWindow_Buffer buffer; + surface->lock(&buffer, nullptr /* inOutDirtyBounds */); + + // Transform hint is updated via callbacks or surface control updates + mSurfaceControl->setTransformHint(ui::Transform::ROT_0); + adapter.update(mSurfaceControl, mDisplayWidth, mDisplayHeight); + + // The hint does not change and matches the value used when dequeueing the buffer. + surface->query(NATIVE_WINDOW_TRANSFORM_HINT, &transformHint); + ASSERT_EQ(ui::Transform::ROT_90, transformHint); + + surface->unlockAndPost(); + + // After queuing the buffer, we get the updated transform hint + surface->query(NATIVE_WINDOW_TRANSFORM_HINT, &transformHint); + ASSERT_EQ(ui::Transform::ROT_0, transformHint); + + adapter.waitForCallbacks(); +} + class BLASTBufferQueueTransformTest : public BLASTBufferQueueTest { public: void test(uint32_t tr) { -- cgit v1.2.3-59-g8ed1b From 95b6d51f68e1a7c0fc3c989a7c8cfcd2db6a6ea4 Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Mon, 30 Aug 2021 15:31:08 -0700 Subject: Surface: Release references to BlastBufferQueue and SurfaceControl on Surface#destroy SurfaceView clients may hold on to surface references. In S this means they would extend the lifetime of the SurfaceControl resulting in "leaking" buffers until the references are cleared or the app is terminated. Fix this by calling a new destroy function on Surface which will explicitly remove references to the SurfaceControl and BBQ it holds. This is safe because SurfaceView controls the lifecycle of the Surface and knows when the Surface will become invalid. Once invalid, the Surface cannot become valid again. Test: repro steps in bug Bug: 198133921 Change-Id: I88fe98fa275dd76e20a5403c24bd2cb3bee5346d --- libs/gui/BLASTBufferQueue.cpp | 19 +++++++++++++++++++ libs/gui/Surface.cpp | 10 ++++++++++ libs/gui/include/gui/Surface.h | 3 ++- 3 files changed, 31 insertions(+), 1 deletion(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 70f2ae770b..b681a0f23e 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -630,7 +630,10 @@ bool BLASTBufferQueue::maxBuffersAcquired(bool includeExtraAcquire) const { class BBQSurface : public Surface { private: + std::mutex mMutex; sp mBbq; + bool mDestroyed = false; + public: BBQSurface(const sp& igbp, bool controlledByApp, const sp& scHandle, const sp& bbq) @@ -650,6 +653,10 @@ public: status_t setFrameRate(float frameRate, int8_t compatibility, int8_t changeFrameRateStrategy) override { + std::unique_lock _lock{mMutex}; + if (mDestroyed) { + return DEAD_OBJECT; + } if (!ValidateFrameRate(frameRate, compatibility, changeFrameRateStrategy, "BBQSurface::setFrameRate")) { return BAD_VALUE; @@ -658,8 +665,20 @@ public: } status_t setFrameTimelineInfo(const FrameTimelineInfo& frameTimelineInfo) override { + std::unique_lock _lock{mMutex}; + if (mDestroyed) { + return DEAD_OBJECT; + } return mBbq->setFrameTimelineInfo(frameTimelineInfo); } + + void destroy() override { + Surface::destroy(); + + std::unique_lock _lock{mMutex}; + mDestroyed = true; + mBbq = nullptr; + } }; // TODO: Can we coalesce this with frame updates? Need to confirm diff --git a/libs/gui/Surface.cpp b/libs/gui/Surface.cpp index 2edb4e4ba4..353a91d062 100644 --- a/libs/gui/Surface.cpp +++ b/libs/gui/Surface.cpp @@ -2622,4 +2622,14 @@ status_t Surface::setFrameTimelineInfo(const FrameTimelineInfo& frameTimelineInf return composerService()->setFrameTimelineInfo(mGraphicBufferProducer, frameTimelineInfo); } +sp Surface::getSurfaceControlHandle() const { + Mutex::Autolock lock(mMutex); + return mSurfaceControlHandle; +} + +void Surface::destroy() { + Mutex::Autolock lock(mMutex); + mSurfaceControlHandle = nullptr; +} + }; // namespace android diff --git a/libs/gui/include/gui/Surface.h b/libs/gui/include/gui/Surface.h index 7e4143b1f3..e5403512a9 100644 --- a/libs/gui/include/gui/Surface.h +++ b/libs/gui/include/gui/Surface.h @@ -99,7 +99,7 @@ public: */ sp getIGraphicBufferProducer() const; - sp getSurfaceControlHandle() const { return mSurfaceControlHandle; } + sp getSurfaceControlHandle() const; /* convenience function to check that the given surface is non NULL as * well as its IGraphicBufferProducer */ @@ -333,6 +333,7 @@ public: virtual int connect( int api, bool reportBufferRemoval, const sp& sListener); + virtual void destroy(); // When client connects to Surface with reportBufferRemoval set to true, any buffers removed // from this Surface will be collected and returned here. Once this method returns, these -- cgit v1.2.3-59-g8ed1b From 932f6aee50abb4f4c71b172a99afd9440e7896ab Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Wed, 29 Sep 2021 17:33:10 -0700 Subject: BlastBufferQueue: Fix scaling when buffer scaling mode changes The current logic updates the target size of the buffer when a buffer arrives with FREEZE scaling mode (target size == buffer size) or when the target size is set and the previous buffer has a scaling mode !FREEZE. This is because if the previous scaling mode is freeze, we do not want to change the target size otherwise we will break the contract with the client and scale the currently presented buffer. In the case of YoutubeMusic on TV, the client sets a target size, then submits a buffer with scaling mode FREEZE, then changes the target size but submits a buffer with scaling mode SCALE_TO_WINDOW. The new target size never gets sent to SurfaceFlinger. To fix this we check if the target size has been updated when acquiring a buffer and update the target size. Test: atest BLASTBufferQueueTest SurfaceViewSyncTest Bug: 196339769 Change-Id: I6db3411b64552e5a4416d60420c92e698beda311 --- libs/gui/BLASTBufferQueue.cpp | 8 ++-- libs/gui/tests/BLASTBufferQueue_test.cpp | 77 ++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 5 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index d1f57b03bc..9f52487857 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -456,10 +456,10 @@ void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) { // Ensure BLASTBufferQueue stays alive until we receive the transaction complete callback. incStrong((void*)transactionCallbackThunk); + const bool sizeHasChanged = mRequestedSize != mSize; + mSize = mRequestedSize; + const bool updateDestinationFrame = sizeHasChanged || !mLastBufferInfo.hasBuffer; Rect crop = computeCrop(bufferItem); - const bool updateDestinationFrame = - bufferItem.mScalingMode == NATIVE_WINDOW_SCALING_MODE_FREEZE || - !mLastBufferInfo.hasBuffer; mLastBufferInfo.update(true /* hasBuffer */, bufferItem.mGraphicBuffer->getWidth(), bufferItem.mGraphicBuffer->getHeight(), bufferItem.mTransform, bufferItem.mScalingMode, crop); @@ -586,7 +586,6 @@ void BLASTBufferQueue::setNextTransaction(SurfaceComposerClient::Transaction* t) bool BLASTBufferQueue::rejectBuffer(const BufferItem& item) { if (item.mScalingMode != NATIVE_WINDOW_SCALING_MODE_FREEZE) { - mSize = mRequestedSize; // Only reject buffers if scaling mode is freeze. return false; } @@ -600,7 +599,6 @@ bool BLASTBufferQueue::rejectBuffer(const BufferItem& item) { } ui::Size bufferSize(bufWidth, bufHeight); if (mRequestedSize != mSize && mRequestedSize == bufferSize) { - mSize = mRequestedSize; return false; } diff --git a/libs/gui/tests/BLASTBufferQueue_test.cpp b/libs/gui/tests/BLASTBufferQueue_test.cpp index 9082d275a2..99777e94c6 100644 --- a/libs/gui/tests/BLASTBufferQueue_test.cpp +++ b/libs/gui/tests/BLASTBufferQueue_test.cpp @@ -672,6 +672,83 @@ TEST_F(BLASTBufferQueueTest, ScaleCroppedBufferToWindowSize) { /*border*/ 0, /*outsideRegion*/ true)); } +// b/196339769 verify we can can update the requested size while the in FREEZE scaling mode and +// scale the buffer properly when the mode changes to SCALE_TO_WINDOW +TEST_F(BLASTBufferQueueTest, ScalingModeChanges) { + uint8_t r = 255; + uint8_t g = 0; + uint8_t b = 0; + + BLASTBufferQueueHelper adapter(mSurfaceControl, mDisplayWidth, mDisplayHeight / 4); + sp igbProducer; + setUpProducer(adapter, igbProducer); + { + int slot; + sp fence; + sp buf; + auto ret = igbProducer->dequeueBuffer(&slot, &fence, mDisplayWidth, mDisplayHeight / 4, + PIXEL_FORMAT_RGBA_8888, GRALLOC_USAGE_SW_WRITE_OFTEN, + nullptr, nullptr); + ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION, ret); + ASSERT_EQ(OK, igbProducer->requestBuffer(slot, &buf)); + + uint32_t* bufData; + buf->lock(static_cast(GraphicBuffer::USAGE_SW_WRITE_OFTEN), + reinterpret_cast(&bufData)); + fillBuffer(bufData, Rect(buf->getWidth(), buf->getHeight()), buf->getStride(), r, g, b); + buf->unlock(); + + IGraphicBufferProducer::QueueBufferOutput qbOutput; + IGraphicBufferProducer::QueueBufferInput input(systemTime(), true /* autotimestamp */, + HAL_DATASPACE_UNKNOWN, {}, + NATIVE_WINDOW_SCALING_MODE_FREEZE, 0, + Fence::NO_FENCE); + igbProducer->queueBuffer(slot, input, &qbOutput); + adapter.waitForCallbacks(); + } + // capture screen and verify that it is red + ASSERT_EQ(NO_ERROR, captureDisplay(mCaptureArgs, mCaptureResults)); + + ASSERT_NO_FATAL_FAILURE( + checkScreenCapture(r, g, b, + {0, 0, (int32_t)mDisplayWidth, (int32_t)mDisplayHeight / 4})); + + // update the size to half the display and dequeue a buffer quarter of the display. + adapter.update(mSurfaceControl, mDisplayWidth, mDisplayHeight / 2); + + { + int slot; + sp fence; + sp buf; + auto ret = igbProducer->dequeueBuffer(&slot, &fence, mDisplayWidth, mDisplayHeight / 8, + PIXEL_FORMAT_RGBA_8888, GRALLOC_USAGE_SW_WRITE_OFTEN, + nullptr, nullptr); + ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION, ret); + ASSERT_EQ(OK, igbProducer->requestBuffer(slot, &buf)); + + uint32_t* bufData; + buf->lock(static_cast(GraphicBuffer::USAGE_SW_WRITE_OFTEN), + reinterpret_cast(&bufData)); + g = 255; + fillBuffer(bufData, Rect(buf->getWidth(), buf->getHeight()), buf->getStride(), r, g, b); + buf->unlock(); + + IGraphicBufferProducer::QueueBufferOutput qbOutput; + IGraphicBufferProducer::QueueBufferInput input(systemTime(), true /* autotimestamp */, + HAL_DATASPACE_UNKNOWN, {}, + NATIVE_WINDOW_SCALING_MODE_SCALE_TO_WINDOW, + 0, Fence::NO_FENCE); + igbProducer->queueBuffer(slot, input, &qbOutput); + adapter.waitForCallbacks(); + } + // capture screen and verify that it is red + ASSERT_EQ(NO_ERROR, captureDisplay(mCaptureArgs, mCaptureResults)); + // verify we still scale the buffer to the new size (half the screen height) + ASSERT_NO_FATAL_FAILURE( + checkScreenCapture(r, g, b, + {0, 0, (int32_t)mDisplayWidth, (int32_t)mDisplayHeight / 2})); +} + class TestProducerListener : public BnProducerListener { public: sp mIgbp; -- cgit v1.2.3-59-g8ed1b From 51e4dc89f0e9b93f0be3204181caa9bef5376746 Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Fri, 1 Oct 2021 15:32:33 -0700 Subject: BlastBufferQueue: Fix async worker deadlock The async onBufferReleased callback can trigger another onBufferReleased which will end up deadlocking the async worker thread. Fix this by executing the callbacks outside the lock. Test: atest android.media.cts.MediaSyncTest#testPlaybackRateDouble --rerun-util-failure 100 Fixes: 201604213 Change-Id: I40d163c3644c6a0128936cf41e8bf8969766d9da --- libs/gui/BLASTBufferQueue.cpp | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index d1f57b03bc..5b9d912723 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -738,14 +738,26 @@ private: std::unique_lock lock(mMutex); while (!mDone) { while (!mRunnables.empty()) { - std::function runnable = mRunnables.front(); - mRunnables.pop_front(); - runnable(); + std::deque> runnables = std::move(mRunnables); + mRunnables.clear(); + lock.unlock(); + // Run outside the lock since the runnable might trigger another + // post to the async worker. + execute(runnables); + lock.lock(); } mCv.wait(lock); } } + void execute(std::deque>& runnables) { + while (!runnables.empty()) { + std::function runnable = runnables.front(); + runnables.pop_front(); + runnable(); + } + } + public: AsyncWorker() : Singleton() { mThread = std::thread(&AsyncWorker::run, this); } -- cgit v1.2.3-59-g8ed1b From 2d2150edc25cdd03bdb5b25ec1a4cc5fb7aa0f33 Mon Sep 17 00:00:00 2001 From: chaviw Date: Wed, 6 Oct 2021 11:53:40 -0500 Subject: DO NOT MERGE: Move blast sync handling to BBQ Add logic in BBQ so it can handle waiting the transaction callback vs a sync transaction request. The following behavior will occur 1. If a nextTransaction (sync) was set, we will wait until the transaction callback for that frame before continuing to acquire new frames. Once the transaction callback for the sync transaction is invoked, BBQ will flush the shadow queue. It will try to process as many frames as it can that were queued up during the time BBQ was blocked from processing. 2. If BBQ is waiting on a sync transaction callback and then another sync transaction is requested afterwards, BBQ will allow it to acquire a buffer instead of just adding to the shadow queue. It will acquire the new frame in the new sync transaction and allow the caller that requested the sync to apply it. At this point, it's up to the callers to ensure they apply the two sync transactions in order to ensure frames are applied in order. 3. Similar to 2, but if there are queue requests in between the two sync requests that aren't trying to be synced. When the second sync frame is getting acquired, BBQ will acquire and release any frames that were requested in between. This is so we don't skip or have to wait in the first sync transaction callback. Test: BLASTBufferQueueTest Bug: 200285149 Change-Id: I8da8de1a3fe2a44ca2199ff92cfd4b60c7f01183 (cherry picked from commit d7deef7278f934a1750738b600c11c1771ae7ac6) --- libs/gui/BLASTBufferQueue.cpp | 160 ++++++++++-- libs/gui/include/gui/BLASTBufferQueue.h | 13 +- libs/gui/include/gui/test/CallbackUtils.h | 211 ++++++++++++++++ libs/gui/tests/BLASTBufferQueue_test.cpp | 272 ++++++++++++++++++++- services/surfaceflinger/tests/IPC_test.cpp | 2 +- .../surfaceflinger/tests/LayerCallback_test.cpp | 2 +- .../tests/ReleaseBufferCallback_test.cpp | 4 +- .../surfaceflinger/tests/utils/CallbackUtils.h | 213 ---------------- 8 files changed, 627 insertions(+), 250 deletions(-) create mode 100644 libs/gui/include/gui/test/CallbackUtils.h delete mode 100644 services/surfaceflinger/tests/utils/CallbackUtils.h (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index e8e664bd76..94864e53f3 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -46,6 +46,8 @@ inline const char* boolToString(bool b) { namespace android { // Macros to include adapter info in log messages +#define BQA_LOGD(x, ...) \ + ALOGD("[%s](f:%u,a:%u) " x, mName.c_str(), mNumFrameAvailable, mNumAcquired, ##__VA_ARGS__) #define BQA_LOGV(x, ...) \ ALOGV("[%s](f:%u,a:%u) " x, mName.c_str(), mNumFrameAvailable, mNumAcquired, ##__VA_ARGS__) // enable logs for a single layer @@ -243,6 +245,67 @@ void BLASTBufferQueue::update(const sp& surface, uint32_t width, } } +static std::optional findMatchingStat( + const std::vector& stats, const sp& sc) { + for (auto stat : stats) { + if (SurfaceControl::isSameSurface(sc, stat.surfaceControl)) { + return stat; + } + } + return std::nullopt; +} + +static void transactionCommittedCallbackThunk(void* context, nsecs_t latchTime, + const sp& presentFence, + const std::vector& stats) { + if (context == nullptr) { + return; + } + sp bq = static_cast(context); + bq->transactionCommittedCallback(latchTime, presentFence, stats); +} + +void BLASTBufferQueue::transactionCommittedCallback(nsecs_t /*latchTime*/, + const sp& /*presentFence*/, + const std::vector& stats) { + { + std::unique_lock _lock{mMutex}; + ATRACE_CALL(); + BQA_LOGV("transactionCommittedCallback"); + if (!mSurfaceControlsWithPendingCallback.empty()) { + sp pendingSC = mSurfaceControlsWithPendingCallback.front(); + std::optional stat = findMatchingStat(stats, pendingSC); + if (stat) { + uint64_t currFrameNumber = stat->frameEventStats.frameNumber; + + // We need to check if we were waiting for a transaction callback in order to + // process any pending buffers and unblock. It's possible to get transaction + // callbacks for previous requests so we need to ensure the frame from this + // transaction callback matches the last acquired buffer. Since acquireNextBuffer + // will stop processing buffers when mWaitForTransactionCallback is set, we know + // that mLastAcquiredFrameNumber is the frame we're waiting on. + // We also want to check if mNextTransaction is null because it's possible another + // sync request came in while waiting, but it hasn't started processing yet. In that + // case, we don't actually want to flush the frames in between since they will get + // processed and merged with the sync transaction and released earlier than if they + // were sent to SF + if (mWaitForTransactionCallback && mNextTransaction == nullptr && + currFrameNumber >= mLastAcquiredFrameNumber) { + mWaitForTransactionCallback = false; + flushShadowQueueLocked(); + } + } else { + BQA_LOGE("Failed to find matching SurfaceControl in transaction callback"); + } + } else { + BQA_LOGE("No matching SurfaceControls found: mSurfaceControlsWithPendingCallback was " + "empty."); + } + + decStrong((void*)transactionCommittedCallbackThunk); + } +} + static void transactionCallbackThunk(void* context, nsecs_t latchTime, const sp& presentFence, const std::vector& stats) { @@ -266,12 +329,9 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp pendingSC = mSurfaceControlsWithPendingCallback.front(); mSurfaceControlsWithPendingCallback.pop(); - bool found = false; - for (auto stat : stats) { - if (!SurfaceControl::isSameSurface(pendingSC, stat.surfaceControl)) { - continue; - } - + std::optional statsOptional = findMatchingStat(stats, pendingSC); + if (statsOptional) { + SurfaceControlStats stat = *statsOptional; mTransformHint = stat.transformHint; mBufferItemConsumer->setTransformHint(mTransformHint); BQA_LOGV("updated mTransformHint=%d", mTransformHint); @@ -299,12 +359,7 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp context, const Relea } } +void BLASTBufferQueue::flushShadowQueueLocked() { + BQA_LOGV("flushShadowQueueLocked"); + int numFramesToFlush = mNumFrameAvailable; + while (numFramesToFlush > 0) { + acquireNextBufferLocked(std::nullopt); + numFramesToFlush--; + } +} + +void BLASTBufferQueue::flushShadowQueue() { + std::unique_lock _lock{mMutex}; + flushShadowQueueLocked(); +} + void BLASTBufferQueue::releaseBufferCallback(const ReleaseCallbackId& id, const sp& releaseFence, uint32_t transformHint, uint32_t currentMaxAcquiredBufferCount) { @@ -377,7 +446,11 @@ void BLASTBufferQueue::releaseBufferCallback(const ReleaseCallbackId& id, BQA_LOGV("released %s", id.to_string().c_str()); mBufferItemConsumer->releaseBuffer(it->second, releaseBuffer.releaseFence); mSubmitted.erase(it); - processNextBufferLocked(false /* useNextTransaction */); + // Don't process the transactions here if mWaitForTransactionCallback is set. Instead, let + // onFrameAvailable handle processing them since it will merge with the nextTransaction. + if (!mWaitForTransactionCallback) { + acquireNextBufferLocked(std::nullopt); + } } ATRACE_INT("PendingRelease", mPendingRelease.size()); @@ -386,13 +459,15 @@ void BLASTBufferQueue::releaseBufferCallback(const ReleaseCallbackId& id, mCallbackCV.notify_all(); } -void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) { +void BLASTBufferQueue::acquireNextBufferLocked( + const std::optional transaction) { ATRACE_CALL(); // If the next transaction is set, we want to guarantee the our acquire will not fail, so don't // include the extra buffer when checking if we can acquire the next buffer. - const bool includeExtraAcquire = !useNextTransaction; - if (mNumFrameAvailable == 0 || maxBuffersAcquired(includeExtraAcquire)) { - mCallbackCV.notify_all(); + const bool includeExtraAcquire = !transaction; + const bool maxAcquired = maxBuffersAcquired(includeExtraAcquire); + if (mNumFrameAvailable == 0 || maxAcquired) { + BQA_LOGV("Can't process next buffer maxBuffersAcquired=%s", boolToString(maxAcquired)); return; } @@ -404,9 +479,8 @@ void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) { SurfaceComposerClient::Transaction localTransaction; bool applyTransaction = true; SurfaceComposerClient::Transaction* t = &localTransaction; - if (mNextTransaction != nullptr && useNextTransaction) { - t = mNextTransaction; - mNextTransaction = nullptr; + if (transaction) { + t = *transaction; applyTransaction = false; } @@ -436,7 +510,7 @@ void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) { mSize.width, mSize.height, mRequestedSize.width, mRequestedSize.height, buffer->getWidth(), buffer->getHeight(), bufferItem.mTransform); mBufferItemConsumer->releaseBuffer(bufferItem, Fence::NO_FENCE); - processNextBufferLocked(useNextTransaction); + acquireNextBufferLocked(transaction); return; } @@ -455,6 +529,7 @@ void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) { // Ensure BLASTBufferQueue stays alive until we receive the transaction complete callback. incStrong((void*)transactionCallbackThunk); + incStrong((void*)transactionCommittedCallbackThunk); const bool sizeHasChanged = mRequestedSize != mSize; mSize = mRequestedSize; @@ -475,6 +550,7 @@ void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) { t->setAcquireFence(mSurfaceControl, bufferItem.mFence ? new Fence(bufferItem.mFence->dup()) : Fence::NO_FENCE); t->addTransactionCompletedCallback(transactionCallbackThunk, static_cast(this)); + t->addTransactionCommittedCallback(transactionCommittedCallbackThunk, static_cast(this)); mSurfaceControlsWithPendingCallback.push(mSurfaceControl); if (updateDestinationFrame) { @@ -527,7 +603,7 @@ void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) { t->setApplyToken(mApplyToken).apply(); } - BQA_LOGV("processNextBufferLocked size=%dx%d mFrameNumber=%" PRIu64 + BQA_LOGV("acquireNextBufferLocked size=%dx%d mFrameNumber=%" PRIu64 " applyTransaction=%s mTimestamp=%" PRId64 "%s mPendingTransactions.size=%d" " graphicBufferId=%" PRIu64 "%s transform=%d", mSize.width, mSize.height, bufferItem.mFrameNumber, boolToString(applyTransaction), @@ -543,17 +619,44 @@ Rect BLASTBufferQueue::computeCrop(const BufferItem& item) { return item.mCrop; } +void BLASTBufferQueue::acquireAndReleaseBuffer() { + BufferItem bufferItem; + mBufferItemConsumer->acquireBuffer(&bufferItem, 0 /* expectedPresent */, false); + mBufferItemConsumer->releaseBuffer(bufferItem, Fence::NO_FENCE); + mNumFrameAvailable--; +} + void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) { ATRACE_CALL(); std::unique_lock _lock{mMutex}; const bool nextTransactionSet = mNextTransaction != nullptr; + BQA_LOGV("onFrameAvailable-start nextTransactionSet=%s", boolToString(nextTransactionSet)); if (nextTransactionSet) { - while (mNumFrameAvailable > 0 || maxBuffersAcquired(false /* includeExtraAcquire */)) { - BQA_LOGV("waiting in onFrameAvailable..."); + if (mWaitForTransactionCallback) { + // We are waiting on a previous sync's transaction callback so allow another sync + // transaction to proceed. + // + // We need to first flush out the transactions that were in between the two syncs. + // We do this by merging them into mNextTransaction so any buffer merging will get + // a release callback invoked. The release callback will be async so we need to wait + // on max acquired to make sure we have the capacity to acquire another buffer. + if (maxBuffersAcquired(false /* includeExtraAcquire */)) { + BQA_LOGD("waiting to flush shadow queue..."); + mCallbackCV.wait(_lock); + } + while (mNumFrameAvailable > 0) { + // flush out the shadow queue + acquireAndReleaseBuffer(); + } + } + + while (maxBuffersAcquired(false /* includeExtraAcquire */)) { + BQA_LOGD("waiting for free buffer."); mCallbackCV.wait(_lock); } } + // add to shadow queue mNumFrameAvailable++; ATRACE_INT(mQueuedBufferTrace.c_str(), @@ -561,7 +664,14 @@ void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) { BQA_LOGV("onFrameAvailable framenumber=%" PRIu64 " nextTransactionSet=%s", item.mFrameNumber, boolToString(nextTransactionSet)); - processNextBufferLocked(nextTransactionSet /* useNextTransaction */); + + if (nextTransactionSet) { + acquireNextBufferLocked(std::move(mNextTransaction)); + mNextTransaction = nullptr; + mWaitForTransactionCallback = true; + } else if (!mWaitForTransactionCallback) { + acquireNextBufferLocked(std::nullopt); + } } void BLASTBufferQueue::onFrameReplaced(const BufferItem& item) { diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index 6c5b2aa53c..615f284e42 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -88,6 +88,8 @@ public: void onFrameDequeued(const uint64_t) override; void onFrameCancelled(const uint64_t) override; + void transactionCommittedCallback(nsecs_t latchTime, const sp& presentFence, + const std::vector& stats); void transactionCallback(nsecs_t latchTime, const sp& presentFence, const std::vector& stats); void releaseBufferCallback(const ReleaseCallbackId& id, const sp& releaseFence, @@ -99,7 +101,6 @@ public: void update(const sp& surface, uint32_t width, uint32_t height, int32_t format, SurfaceComposerClient::Transaction* outTransaction = nullptr); - void flushShadowQueue() {} status_t setFrameRate(float frameRate, int8_t compatibility, bool shouldBeSeamless); status_t setFrameTimelineInfo(const FrameTimelineInfo& info); @@ -107,6 +108,7 @@ public: void setSidebandStream(const sp& stream); uint32_t getLastTransformHint() const; + void flushShadowQueue(); virtual ~BLASTBufferQueue(); @@ -119,13 +121,17 @@ private: void createBufferQueue(sp* outProducer, sp* outConsumer); - void processNextBufferLocked(bool useNextTransaction) REQUIRES(mMutex); + void acquireNextBufferLocked( + const std::optional transaction) REQUIRES(mMutex); Rect computeCrop(const BufferItem& item) REQUIRES(mMutex); // Return true if we need to reject the buffer based on the scaling mode and the buffer size. bool rejectBuffer(const BufferItem& item) REQUIRES(mMutex); bool maxBuffersAcquired(bool includeExtraAcquire) const REQUIRES(mMutex); static PixelFormat convertBufferFormat(PixelFormat& format); + void flushShadowQueueLocked() REQUIRES(mMutex); + void acquireAndReleaseBuffer() REQUIRES(mMutex); + std::string mName; // Represents the queued buffer count from buffer queue, // pre-BLAST. This is mNumFrameAvailable (buffers that queued to blast) + @@ -233,6 +239,9 @@ private: // Keep track of SurfaceControls that have submitted a transaction and BBQ is waiting on a // callback for them. std::queue> mSurfaceControlsWithPendingCallback GUARDED_BY(mMutex); + + uint32_t mCurrentMaxAcquiredBufferCount; + bool mWaitForTransactionCallback = false; }; } // namespace android diff --git a/libs/gui/include/gui/test/CallbackUtils.h b/libs/gui/include/gui/test/CallbackUtils.h new file mode 100644 index 0000000000..64032087eb --- /dev/null +++ b/libs/gui/include/gui/test/CallbackUtils.h @@ -0,0 +1,211 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#pragma once + +#include +#include +#include +#include +#include +#include +#include + +using ::std::literals::chrono_literals::operator""ms; +using ::std::literals::chrono_literals::operator""s; + +namespace android { + +namespace { + +struct CallbackData { + CallbackData() = default; + CallbackData(nsecs_t time, const sp& fence, + const std::vector& stats) + : latchTime(time), presentFence(fence), surfaceControlStats(stats) {} + + nsecs_t latchTime; + sp presentFence; + std::vector surfaceControlStats; +}; + +class ExpectedResult { +public: + enum Transaction { + NOT_PRESENTED = 0, + PRESENTED, + }; + + enum Buffer { + NOT_ACQUIRED = 0, + ACQUIRED, + }; + + enum PreviousBuffer { + NOT_RELEASED = 0, + RELEASED, + UNKNOWN, + }; + + void reset() { + mTransactionResult = ExpectedResult::Transaction::NOT_PRESENTED; + mExpectedSurfaceResults.clear(); + } + + void addSurface(ExpectedResult::Transaction transactionResult, const sp& layer, + ExpectedResult::Buffer bufferResult = ACQUIRED, + ExpectedResult::PreviousBuffer previousBufferResult = NOT_RELEASED) { + mTransactionResult = transactionResult; + mExpectedSurfaceResults.emplace(std::piecewise_construct, std::forward_as_tuple(layer), + std::forward_as_tuple(bufferResult, previousBufferResult)); + } + + void addSurfaces(ExpectedResult::Transaction transactionResult, + const std::vector>& layers, + ExpectedResult::Buffer bufferResult = ACQUIRED, + ExpectedResult::PreviousBuffer previousBufferResult = NOT_RELEASED) { + for (const auto& layer : layers) { + addSurface(transactionResult, layer, bufferResult, previousBufferResult); + } + } + + void addExpectedPresentTime(nsecs_t expectedPresentTime) { + mExpectedPresentTime = expectedPresentTime; + } + + void addExpectedPresentTimeForVsyncId(nsecs_t expectedPresentTime) { + mExpectedPresentTimeForVsyncId = expectedPresentTime; + } + + void verifyCallbackData(const CallbackData& callbackData) const { + const auto& [latchTime, presentFence, surfaceControlStats] = callbackData; + if (mTransactionResult == ExpectedResult::Transaction::PRESENTED) { + ASSERT_GE(latchTime, 0) << "bad latch time"; + ASSERT_NE(presentFence, nullptr); + if (mExpectedPresentTime >= 0) { + ASSERT_EQ(presentFence->wait(3000), NO_ERROR); + ASSERT_GE(presentFence->getSignalTime(), mExpectedPresentTime - nsecs_t(5 * 1e6)); + // if the panel is running at 30 hz, at the worst case, our expected time just + // misses vsync and we have to wait another 33.3ms + ASSERT_LE(presentFence->getSignalTime(), + mExpectedPresentTime + nsecs_t(66.666666 * 1e6)); + } else if (mExpectedPresentTimeForVsyncId >= 0) { + ASSERT_EQ(presentFence->wait(3000), NO_ERROR); + // We give 4ms for prediction error + ASSERT_GE(presentFence->getSignalTime(), + mExpectedPresentTimeForVsyncId - 4'000'000); + } + } else { + ASSERT_EQ(presentFence, nullptr) << "transaction shouldn't have been presented"; + ASSERT_EQ(latchTime, -1) << "unpresented transactions shouldn't be latched"; + } + + ASSERT_EQ(surfaceControlStats.size(), mExpectedSurfaceResults.size()) + << "wrong number of surfaces"; + + for (const auto& stats : surfaceControlStats) { + ASSERT_NE(stats.surfaceControl, nullptr) << "returned null surface control"; + + const auto& expectedSurfaceResult = mExpectedSurfaceResults.find(stats.surfaceControl); + ASSERT_NE(expectedSurfaceResult, mExpectedSurfaceResults.end()) + << "unexpected surface control"; + expectedSurfaceResult->second.verifySurfaceControlStats(stats, latchTime); + } + } + +private: + class ExpectedSurfaceResult { + public: + ExpectedSurfaceResult(ExpectedResult::Buffer bufferResult, + ExpectedResult::PreviousBuffer previousBufferResult) + : mBufferResult(bufferResult), mPreviousBufferResult(previousBufferResult) {} + + void verifySurfaceControlStats(const SurfaceControlStats& surfaceControlStats, + nsecs_t latchTime) const { + const auto& [surfaceControl, latch, acquireTime, presentFence, previousReleaseFence, + transformHint, frameEvents] = surfaceControlStats; + + ASSERT_EQ(acquireTime > 0, mBufferResult == ExpectedResult::Buffer::ACQUIRED) + << "bad acquire time"; + ASSERT_LE(acquireTime, latchTime) << "acquire time should be <= latch time"; + + if (mPreviousBufferResult == ExpectedResult::PreviousBuffer::RELEASED) { + ASSERT_NE(previousReleaseFence, nullptr) + << "failed to set release prev buffer fence"; + } else if (mPreviousBufferResult == ExpectedResult::PreviousBuffer::NOT_RELEASED) { + ASSERT_EQ(previousReleaseFence, nullptr) + << "should not have set released prev buffer fence"; + } + } + + private: + ExpectedResult::Buffer mBufferResult; + ExpectedResult::PreviousBuffer mPreviousBufferResult; + }; + + struct SCHash { + std::size_t operator()(const sp& sc) const { + return std::hash{}(sc->getHandle().get()); + } + }; + ExpectedResult::Transaction mTransactionResult = ExpectedResult::Transaction::NOT_PRESENTED; + nsecs_t mExpectedPresentTime = -1; + nsecs_t mExpectedPresentTimeForVsyncId = -1; + std::unordered_map, ExpectedSurfaceResult, SCHash> mExpectedSurfaceResults; +}; + +class CallbackHelper { +public: + static void function(void* callbackContext, nsecs_t latchTime, const sp& presentFence, + const std::vector& stats) { + if (!callbackContext) { + ALOGE("failed to get callback context"); + } + CallbackHelper* helper = static_cast(callbackContext); + std::lock_guard lock(helper->mMutex); + helper->mCallbackDataQueue.emplace(latchTime, presentFence, stats); + helper->mConditionVariable.notify_all(); + } + + void getCallbackData(CallbackData* outData) { + std::unique_lock lock(mMutex); + + if (mCallbackDataQueue.empty()) { + ASSERT_NE(mConditionVariable.wait_for(lock, std::chrono::seconds(3)), + std::cv_status::timeout) + << "did not receive callback"; + } + + *outData = std::move(mCallbackDataQueue.front()); + mCallbackDataQueue.pop(); + } + + void verifyFinalState() { + // Wait to see if there are extra callbacks + std::this_thread::sleep_for(500ms); + + std::lock_guard lock(mMutex); + EXPECT_EQ(mCallbackDataQueue.size(), 0U) << "extra callbacks received"; + mCallbackDataQueue = {}; + } + + void* getContext() { return static_cast(this); } + + std::mutex mMutex; + std::condition_variable mConditionVariable; + std::queue mCallbackDataQueue; +}; +} // namespace +} // namespace android diff --git a/libs/gui/tests/BLASTBufferQueue_test.cpp b/libs/gui/tests/BLASTBufferQueue_test.cpp index 99777e94c6..fc7548511d 100644 --- a/libs/gui/tests/BLASTBufferQueue_test.cpp +++ b/libs/gui/tests/BLASTBufferQueue_test.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -42,6 +43,29 @@ namespace android { using Transaction = SurfaceComposerClient::Transaction; using android::hardware::graphics::common::V1_2::BufferUsage; +class CountProducerListener : public BnProducerListener { +public: + void onBufferReleased() override { + std::scoped_lock lock(mMutex); + mNumReleased++; + mReleaseCallback.notify_one(); + } + + void waitOnNumberReleased(int32_t expectedNumReleased) { + std::unique_lock lock(mMutex); + while (mNumReleased < expectedNumReleased) { + ASSERT_NE(mReleaseCallback.wait_for(lock, std::chrono::seconds(3)), + std::cv_status::timeout) + << "did not receive release"; + } + } + +private: + std::mutex mMutex; + std::condition_variable mReleaseCallback; + int32_t mNumReleased GUARDED_BY(mMutex) = 0; +}; + class BLASTBufferQueueHelper { public: BLASTBufferQueueHelper(const sp& sc, int width, int height) { @@ -152,18 +176,19 @@ protected: mCaptureArgs.dataspace = ui::Dataspace::V0_SRGB; } - void setUpProducer(BLASTBufferQueueHelper& adapter, sp& producer) { + void setUpProducer(BLASTBufferQueueHelper& adapter, sp& producer, + int32_t maxBufferCount = 2) { producer = adapter.getIGraphicBufferProducer(); - setUpProducer(producer); + setUpProducer(producer, maxBufferCount); } - void setUpProducer(sp& igbProducer) { + void setUpProducer(sp& igbProducer, int32_t maxBufferCount) { ASSERT_NE(nullptr, igbProducer.get()); - ASSERT_EQ(NO_ERROR, igbProducer->setMaxDequeuedBufferCount(2)); + ASSERT_EQ(NO_ERROR, igbProducer->setMaxDequeuedBufferCount(maxBufferCount)); IGraphicBufferProducer::QueueBufferOutput qbOutput; + mProducerListener = new CountProducerListener(); ASSERT_EQ(NO_ERROR, - igbProducer->connect(new StubProducerListener, NATIVE_WINDOW_API_CPU, false, - &qbOutput)); + igbProducer->connect(mProducerListener, NATIVE_WINDOW_API_CPU, false, &qbOutput)); ASSERT_NE(ui::Transform::ROT_INVALID, qbOutput.transformHint); } @@ -287,6 +312,7 @@ protected: DisplayCaptureArgs mCaptureArgs; ScreenCaptureResults mCaptureResults; + sp mProducerListener; }; TEST_F(BLASTBufferQueueTest, CreateBLASTBufferQueue) { @@ -749,6 +775,240 @@ TEST_F(BLASTBufferQueueTest, ScalingModeChanges) { {0, 0, (int32_t)mDisplayWidth, (int32_t)mDisplayHeight / 2})); } +TEST_F(BLASTBufferQueueTest, SyncThenNoSync) { + uint8_t r = 255; + uint8_t g = 0; + uint8_t b = 0; + + BLASTBufferQueueHelper adapter(mSurfaceControl, mDisplayWidth, mDisplayHeight); + + sp igbProducer; + setUpProducer(adapter, igbProducer); + + Transaction next; + adapter.setNextTransaction(&next); + queueBuffer(igbProducer, 0, 255, 0, 0); + + // queue non sync buffer, so this one should get blocked + // Add a present delay to allow the first screenshot to get taken. + nsecs_t presentTimeDelay = std::chrono::nanoseconds(500ms).count(); + queueBuffer(igbProducer, r, g, b, presentTimeDelay); + + CallbackHelper transactionCallback; + next.addTransactionCompletedCallback(transactionCallback.function, + transactionCallback.getContext()) + .apply(); + + CallbackData callbackData; + transactionCallback.getCallbackData(&callbackData); + + // capture screen and verify that it is red + ASSERT_EQ(NO_ERROR, captureDisplay(mCaptureArgs, mCaptureResults)); + ASSERT_NO_FATAL_FAILURE( + checkScreenCapture(0, 255, 0, {0, 0, (int32_t)mDisplayWidth, (int32_t)mDisplayHeight})); + + mProducerListener->waitOnNumberReleased(1); + ASSERT_EQ(NO_ERROR, captureDisplay(mCaptureArgs, mCaptureResults)); + ASSERT_NO_FATAL_FAILURE( + checkScreenCapture(r, g, b, {0, 0, (int32_t)mDisplayWidth, (int32_t)mDisplayHeight})); +} + +TEST_F(BLASTBufferQueueTest, MultipleSyncTransactions) { + uint8_t r = 255; + uint8_t g = 0; + uint8_t b = 0; + + BLASTBufferQueueHelper adapter(mSurfaceControl, mDisplayWidth, mDisplayHeight); + + sp igbProducer; + setUpProducer(adapter, igbProducer); + + Transaction mainTransaction; + + Transaction next; + adapter.setNextTransaction(&next); + queueBuffer(igbProducer, 0, 255, 0, 0); + + mainTransaction.merge(std::move(next)); + + adapter.setNextTransaction(&next); + queueBuffer(igbProducer, r, g, b, 0); + + mainTransaction.merge(std::move(next)); + // Expect 1 buffer to be released even before sending to SurfaceFlinger + mProducerListener->waitOnNumberReleased(1); + + CallbackHelper transactionCallback; + mainTransaction + .addTransactionCompletedCallback(transactionCallback.function, + transactionCallback.getContext()) + .apply(); + + CallbackData callbackData; + transactionCallback.getCallbackData(&callbackData); + + // capture screen and verify that it is red + ASSERT_EQ(NO_ERROR, captureDisplay(mCaptureArgs, mCaptureResults)); + ASSERT_NO_FATAL_FAILURE( + checkScreenCapture(r, g, b, {0, 0, (int32_t)mDisplayWidth, (int32_t)mDisplayHeight})); +} + +TEST_F(BLASTBufferQueueTest, MultipleSyncTransactionWithNonSync) { + uint8_t r = 255; + uint8_t g = 0; + uint8_t b = 0; + + BLASTBufferQueueHelper adapter(mSurfaceControl, mDisplayWidth, mDisplayHeight); + + sp igbProducer; + setUpProducer(adapter, igbProducer); + + Transaction mainTransaction; + + Transaction next; + // queue a sync transaction + adapter.setNextTransaction(&next); + queueBuffer(igbProducer, 0, 255, 0, 0); + + mainTransaction.merge(std::move(next)); + + // queue another buffer without setting next transaction + queueBuffer(igbProducer, 0, 0, 255, 0); + + // queue another sync transaction + adapter.setNextTransaction(&next); + queueBuffer(igbProducer, r, g, b, 0); + // Expect 1 buffer to be released because the non sync transaction should merge + // with the sync + mProducerListener->waitOnNumberReleased(1); + + mainTransaction.merge(std::move(next)); + // Expect 2 buffers to be released due to merging the two syncs. + mProducerListener->waitOnNumberReleased(2); + + CallbackHelper transactionCallback; + mainTransaction + .addTransactionCompletedCallback(transactionCallback.function, + transactionCallback.getContext()) + .apply(); + + CallbackData callbackData; + transactionCallback.getCallbackData(&callbackData); + + // capture screen and verify that it is red + ASSERT_EQ(NO_ERROR, captureDisplay(mCaptureArgs, mCaptureResults)); + ASSERT_NO_FATAL_FAILURE( + checkScreenCapture(r, g, b, {0, 0, (int32_t)mDisplayWidth, (int32_t)mDisplayHeight})); +} + +TEST_F(BLASTBufferQueueTest, MultipleSyncRunOutOfBuffers) { + uint8_t r = 255; + uint8_t g = 0; + uint8_t b = 0; + + BLASTBufferQueueHelper adapter(mSurfaceControl, mDisplayWidth, mDisplayHeight); + + sp igbProducer; + setUpProducer(adapter, igbProducer, 3); + + Transaction mainTransaction; + + Transaction next; + // queue a sync transaction + adapter.setNextTransaction(&next); + queueBuffer(igbProducer, 0, 255, 0, 0); + + mainTransaction.merge(std::move(next)); + + // queue a few buffers without setting next transaction + queueBuffer(igbProducer, 0, 0, 255, 0); + queueBuffer(igbProducer, 0, 0, 255, 0); + queueBuffer(igbProducer, 0, 0, 255, 0); + + // queue another sync transaction + adapter.setNextTransaction(&next); + queueBuffer(igbProducer, r, g, b, 0); + // Expect 3 buffers to be released because the non sync transactions should merge + // with the sync + mProducerListener->waitOnNumberReleased(3); + + mainTransaction.merge(std::move(next)); + // Expect 4 buffers to be released due to merging the two syncs. + mProducerListener->waitOnNumberReleased(4); + + CallbackHelper transactionCallback; + mainTransaction + .addTransactionCompletedCallback(transactionCallback.function, + transactionCallback.getContext()) + .apply(); + + CallbackData callbackData; + transactionCallback.getCallbackData(&callbackData); + + // capture screen and verify that it is red + ASSERT_EQ(NO_ERROR, captureDisplay(mCaptureArgs, mCaptureResults)); + ASSERT_NO_FATAL_FAILURE( + checkScreenCapture(r, g, b, {0, 0, (int32_t)mDisplayWidth, (int32_t)mDisplayHeight})); +} + +// Tests BBQ with a sync transaction when the buffers acquired reaches max and the only way to +// continue processing is for a release callback from SurfaceFlinger. +// This is done by sending a buffer to SF so it can release the previous one and allow BBQ to +// continue acquiring buffers. +TEST_F(BLASTBufferQueueTest, RunOutOfBuffersWaitingOnSF) { + uint8_t r = 255; + uint8_t g = 0; + uint8_t b = 0; + + BLASTBufferQueueHelper adapter(mSurfaceControl, mDisplayWidth, mDisplayHeight); + + sp igbProducer; + setUpProducer(adapter, igbProducer, 4); + + Transaction mainTransaction; + + // Send a buffer to SF + queueBuffer(igbProducer, 0, 255, 0, 0); + + Transaction next; + // queue a sync transaction + adapter.setNextTransaction(&next); + queueBuffer(igbProducer, 0, 255, 0, 0); + + mainTransaction.merge(std::move(next)); + + // queue a few buffers without setting next transaction + queueBuffer(igbProducer, 0, 0, 255, 0); + queueBuffer(igbProducer, 0, 0, 255, 0); + queueBuffer(igbProducer, 0, 0, 255, 0); + + // apply the first synced buffer to ensure we have to wait on SF + mainTransaction.apply(); + + // queue another sync transaction + adapter.setNextTransaction(&next); + queueBuffer(igbProducer, r, g, b, 0); + // Expect 2 buffers to be released because the non sync transactions should merge + // with the sync + mProducerListener->waitOnNumberReleased(3); + + mainTransaction.merge(std::move(next)); + + CallbackHelper transactionCallback; + mainTransaction + .addTransactionCompletedCallback(transactionCallback.function, + transactionCallback.getContext()) + .apply(); + + CallbackData callbackData; + transactionCallback.getCallbackData(&callbackData); + + // capture screen and verify that it is red + ASSERT_EQ(NO_ERROR, captureDisplay(mCaptureArgs, mCaptureResults)); + ASSERT_NO_FATAL_FAILURE( + checkScreenCapture(r, g, b, {0, 0, (int32_t)mDisplayWidth, (int32_t)mDisplayHeight})); +} + class TestProducerListener : public BnProducerListener { public: sp mIgbp; diff --git a/services/surfaceflinger/tests/IPC_test.cpp b/services/surfaceflinger/tests/IPC_test.cpp index 9fa3d4c417..84fea6c751 100644 --- a/services/surfaceflinger/tests/IPC_test.cpp +++ b/services/surfaceflinger/tests/IPC_test.cpp @@ -28,8 +28,8 @@ #include +#include #include "BufferGenerator.h" -#include "utils/CallbackUtils.h" #include "utils/ColorUtils.h" #include "utils/TransactionUtils.h" diff --git a/services/surfaceflinger/tests/LayerCallback_test.cpp b/services/surfaceflinger/tests/LayerCallback_test.cpp index 965aac301d..7ff041ea57 100644 --- a/services/surfaceflinger/tests/LayerCallback_test.cpp +++ b/services/surfaceflinger/tests/LayerCallback_test.cpp @@ -18,8 +18,8 @@ #include +#include #include "LayerTransactionTest.h" -#include "utils/CallbackUtils.h" using namespace std::chrono_literals; diff --git a/services/surfaceflinger/tests/ReleaseBufferCallback_test.cpp b/services/surfaceflinger/tests/ReleaseBufferCallback_test.cpp index c4d42fad36..309ab2f0db 100644 --- a/services/surfaceflinger/tests/ReleaseBufferCallback_test.cpp +++ b/services/surfaceflinger/tests/ReleaseBufferCallback_test.cpp @@ -14,8 +14,8 @@ * limitations under the License. */ +#include #include "LayerTransactionTest.h" -#include "utils/CallbackUtils.h" using namespace std::chrono_literals; @@ -61,7 +61,7 @@ public: std::this_thread::sleep_for(300ms); std::lock_guard lock(mMutex); - EXPECT_EQ(mCallbackDataQueue.size(), 0) << "extra callbacks received"; + EXPECT_EQ(mCallbackDataQueue.size(), 0U) << "extra callbacks received"; mCallbackDataQueue = {}; } diff --git a/services/surfaceflinger/tests/utils/CallbackUtils.h b/services/surfaceflinger/tests/utils/CallbackUtils.h deleted file mode 100644 index f4a3425b6a..0000000000 --- a/services/surfaceflinger/tests/utils/CallbackUtils.h +++ /dev/null @@ -1,213 +0,0 @@ -/* - * Copyright (C) 2019 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -#pragma once - -#include -#include -#include -#include -#include -#include -#include - -using ::std::literals::chrono_literals::operator""ms; -using ::std::literals::chrono_literals::operator""s; - -namespace android { - -namespace { - -struct CallbackData { - CallbackData() = default; - CallbackData(nsecs_t time, const sp& fence, - const std::vector& stats) - : latchTime(time), presentFence(fence), surfaceControlStats(stats) {} - - nsecs_t latchTime; - sp presentFence; - std::vector surfaceControlStats; -}; - -class ExpectedResult { -public: - enum Transaction { - NOT_PRESENTED = 0, - PRESENTED, - }; - - enum Buffer { - NOT_ACQUIRED = 0, - ACQUIRED, - }; - - enum PreviousBuffer { - NOT_RELEASED = 0, - RELEASED, - UNKNOWN, - }; - - void reset() { - mTransactionResult = ExpectedResult::Transaction::NOT_PRESENTED; - mExpectedSurfaceResults.clear(); - } - - void addSurface(ExpectedResult::Transaction transactionResult, const sp& layer, - ExpectedResult::Buffer bufferResult = ACQUIRED, - ExpectedResult::PreviousBuffer previousBufferResult = NOT_RELEASED) { - mTransactionResult = transactionResult; - mExpectedSurfaceResults.emplace(std::piecewise_construct, std::forward_as_tuple(layer), - std::forward_as_tuple(bufferResult, previousBufferResult)); - } - - void addSurfaces(ExpectedResult::Transaction transactionResult, - const std::vector>& layers, - ExpectedResult::Buffer bufferResult = ACQUIRED, - ExpectedResult::PreviousBuffer previousBufferResult = NOT_RELEASED) { - for (const auto& layer : layers) { - addSurface(transactionResult, layer, bufferResult, previousBufferResult); - } - } - - void addExpectedPresentTime(nsecs_t expectedPresentTime) { - mExpectedPresentTime = expectedPresentTime; - } - - void addExpectedPresentTimeForVsyncId(nsecs_t expectedPresentTime) { - mExpectedPresentTimeForVsyncId = expectedPresentTime; - } - - void verifyCallbackData(const CallbackData& callbackData) const { - const auto& [latchTime, presentFence, surfaceControlStats] = callbackData; - if (mTransactionResult == ExpectedResult::Transaction::PRESENTED) { - ASSERT_GE(latchTime, 0) << "bad latch time"; - ASSERT_NE(presentFence, nullptr); - if (mExpectedPresentTime >= 0) { - ASSERT_EQ(presentFence->wait(3000), NO_ERROR); - ASSERT_GE(presentFence->getSignalTime(), mExpectedPresentTime - nsecs_t(5 * 1e6)); - // if the panel is running at 30 hz, at the worst case, our expected time just - // misses vsync and we have to wait another 33.3ms - ASSERT_LE(presentFence->getSignalTime(), - mExpectedPresentTime + nsecs_t(66.666666 * 1e6)); - } else if (mExpectedPresentTimeForVsyncId >= 0) { - ASSERT_EQ(presentFence->wait(3000), NO_ERROR); - // We give 4ms for prediction error - ASSERT_GE(presentFence->getSignalTime(), - mExpectedPresentTimeForVsyncId - 4'000'000); - } - } else { - ASSERT_EQ(presentFence, nullptr) << "transaction shouldn't have been presented"; - ASSERT_EQ(latchTime, -1) << "unpresented transactions shouldn't be latched"; - } - - ASSERT_EQ(surfaceControlStats.size(), mExpectedSurfaceResults.size()) - << "wrong number of surfaces"; - - for (const auto& stats : surfaceControlStats) { - ASSERT_NE(stats.surfaceControl, nullptr) << "returned null surface control"; - - const auto& expectedSurfaceResult = mExpectedSurfaceResults.find(stats.surfaceControl); - ASSERT_NE(expectedSurfaceResult, mExpectedSurfaceResults.end()) - << "unexpected surface control"; - expectedSurfaceResult->second.verifySurfaceControlStats(stats, latchTime); - } - } - -private: - class ExpectedSurfaceResult { - public: - ExpectedSurfaceResult(ExpectedResult::Buffer bufferResult, - ExpectedResult::PreviousBuffer previousBufferResult) - : mBufferResult(bufferResult), mPreviousBufferResult(previousBufferResult) {} - - void verifySurfaceControlStats(const SurfaceControlStats& surfaceControlStats, - nsecs_t latchTime) const { - const auto& - [surfaceControl, latch, acquireTime, presentFence, previousReleaseFence, - transformHint, - frameEvents] = surfaceControlStats; - - ASSERT_EQ(acquireTime > 0, mBufferResult == ExpectedResult::Buffer::ACQUIRED) - << "bad acquire time"; - ASSERT_LE(acquireTime, latchTime) << "acquire time should be <= latch time"; - - if (mPreviousBufferResult == ExpectedResult::PreviousBuffer::RELEASED) { - ASSERT_NE(previousReleaseFence, nullptr) - << "failed to set release prev buffer fence"; - } else if (mPreviousBufferResult == ExpectedResult::PreviousBuffer::NOT_RELEASED) { - ASSERT_EQ(previousReleaseFence, nullptr) - << "should not have set released prev buffer fence"; - } - } - - private: - ExpectedResult::Buffer mBufferResult; - ExpectedResult::PreviousBuffer mPreviousBufferResult; - }; - - struct SCHash { - std::size_t operator()(const sp& sc) const { - return std::hash{}(sc->getHandle().get()); - } - }; - ExpectedResult::Transaction mTransactionResult = ExpectedResult::Transaction::NOT_PRESENTED; - nsecs_t mExpectedPresentTime = -1; - nsecs_t mExpectedPresentTimeForVsyncId = -1; - std::unordered_map, ExpectedSurfaceResult, SCHash> mExpectedSurfaceResults; -}; - -class CallbackHelper { -public: - static void function(void* callbackContext, nsecs_t latchTime, const sp& presentFence, - const std::vector& stats) { - if (!callbackContext) { - ALOGE("failed to get callback context"); - } - CallbackHelper* helper = static_cast(callbackContext); - std::lock_guard lock(helper->mMutex); - helper->mCallbackDataQueue.emplace(latchTime, presentFence, stats); - helper->mConditionVariable.notify_all(); - } - - void getCallbackData(CallbackData* outData) { - std::unique_lock lock(mMutex); - - if (mCallbackDataQueue.empty()) { - ASSERT_NE(mConditionVariable.wait_for(lock, std::chrono::seconds(3)), - std::cv_status::timeout) - << "did not receive callback"; - } - - *outData = std::move(mCallbackDataQueue.front()); - mCallbackDataQueue.pop(); - } - - void verifyFinalState() { - // Wait to see if there are extra callbacks - std::this_thread::sleep_for(500ms); - - std::lock_guard lock(mMutex); - EXPECT_EQ(mCallbackDataQueue.size(), 0) << "extra callbacks received"; - mCallbackDataQueue = {}; - } - - void* getContext() { return static_cast(this); } - - std::mutex mMutex; - std::condition_variable mConditionVariable; - std::queue mCallbackDataQueue; -}; -} -} // namespace android -- cgit v1.2.3-59-g8ed1b From a840a12d0c581ef5417850034eb1e1d514d829f2 Mon Sep 17 00:00:00 2001 From: chaviw Date: Mon, 1 Nov 2021 09:50:57 -0500 Subject: Change log to match with function it's coming from Added the function name in the error log when SurfaceControl can't be found in the callbacks. Test: Log Bug: 200285149 Change-Id: I6c721a699c25c8659f5aa5e703de134c8c0a31b7 (cherry picked from commit 768bfa07e598608f135140f1001bf09c7de35b1e) --- libs/gui/BLASTBufferQueue.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 94864e53f3..b2cab8cda3 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -295,7 +295,7 @@ void BLASTBufferQueue::transactionCommittedCallback(nsecs_t /*latchTime*/, flushShadowQueueLocked(); } } else { - BQA_LOGE("Failed to find matching SurfaceControl in transaction callback"); + BQA_LOGE("Failed to find matching SurfaceControl in transactionCommittedCallback"); } } else { BQA_LOGE("No matching SurfaceControls found: mSurfaceControlsWithPendingCallback was " @@ -360,7 +360,7 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp Date: Thu, 14 Oct 2021 11:57:22 -0500 Subject: Use bufferItem's fence when calling release When we acquire and release immediately, we want to use the bufferItem's release fence instead of NO_FENCE. Also add an error log if there's a failure in acquireBuffer, which really should never happen. Test: BLASTBufferQueueTest Bug: 200285149 Change-Id: I5e869b0f66f37b15b9317985b30f539dbfea831c (cherry picked from commit 6ebdf5f0f7ca5354a1a9a0e6419275c7b873849d) --- libs/gui/BLASTBufferQueue.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 94864e53f3..5e26b3f0e8 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -621,9 +621,15 @@ Rect BLASTBufferQueue::computeCrop(const BufferItem& item) { void BLASTBufferQueue::acquireAndReleaseBuffer() { BufferItem bufferItem; - mBufferItemConsumer->acquireBuffer(&bufferItem, 0 /* expectedPresent */, false); - mBufferItemConsumer->releaseBuffer(bufferItem, Fence::NO_FENCE); + status_t status = + mBufferItemConsumer->acquireBuffer(&bufferItem, 0 /* expectedPresent */, false); + if (status != OK) { + BQA_LOGE("Failed to acquire a buffer in acquireAndReleaseBuffer, err=%s", + statusToString(status).c_str()); + return; + } mNumFrameAvailable--; + mBufferItemConsumer->releaseBuffer(bufferItem, bufferItem.mFence); } void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) { -- cgit v1.2.3-59-g8ed1b From 9d12adc77270be3215e3cd2b89b1b787493a11f6 Mon Sep 17 00:00:00 2001 From: chaviw Date: Wed, 17 Nov 2021 17:36:50 -0600 Subject: Only add commit callback when using sync transaction There's no need to add a commit callback when we're not syncing since the callback won't actually do anything. Instead only add a commit callback when a sync transaction has been requested. Test: BLASTBufferQueueTest Bug: 205278630 Change-Id: Ib7345f2581b6e4ce8923531aebcd457c14d86027 Merged-In: Ib7345f2581b6e4ce8923531aebcd457c14d86027 --- libs/gui/BLASTBufferQueue.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 39dd5d9552..085a11a88e 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -529,7 +529,6 @@ void BLASTBufferQueue::acquireNextBufferLocked( // Ensure BLASTBufferQueue stays alive until we receive the transaction complete callback. incStrong((void*)transactionCallbackThunk); - incStrong((void*)transactionCommittedCallbackThunk); const bool sizeHasChanged = mRequestedSize != mSize; mSize = mRequestedSize; @@ -550,7 +549,7 @@ void BLASTBufferQueue::acquireNextBufferLocked( t->setAcquireFence(mSurfaceControl, bufferItem.mFence ? new Fence(bufferItem.mFence->dup()) : Fence::NO_FENCE); t->addTransactionCompletedCallback(transactionCallbackThunk, static_cast(this)); - t->addTransactionCommittedCallback(transactionCommittedCallbackThunk, static_cast(this)); + mSurfaceControlsWithPendingCallback.push(mSurfaceControl); if (updateDestinationFrame) { @@ -673,6 +672,13 @@ void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) { if (nextTransactionSet) { acquireNextBufferLocked(std::move(mNextTransaction)); + + // Only need a commit callback when syncing to ensure the buffer that's synced has been sent + // to SF + incStrong((void*)transactionCommittedCallbackThunk); + mNextTransaction->addTransactionCommittedCallback(transactionCommittedCallbackThunk, + static_cast(this)); + mNextTransaction = nullptr; mWaitForTransactionCallback = true; } else if (!mWaitForTransactionCallback) { -- cgit v1.2.3-59-g8ed1b From 3d8a31967b6b27581215c94b9d1471996e117e6a Mon Sep 17 00:00:00 2001 From: chaviw Date: Fri, 20 Aug 2021 12:00:47 -0500 Subject: Added getLastAcquiredFrameNum This will allow VRI to ask BBQ what buffer was actually acquired on the last draw. Test: blast sync Bug: 195262673 Bug: 193634619 Change-Id: I492651e8e6d333ef11b682cec939d81057ae197d Merged-In: I492651e8e6d333ef11b682cec939d81057ae197d --- libs/gui/BLASTBufferQueue.cpp | 5 +++++ libs/gui/include/gui/BLASTBufferQueue.h | 2 ++ 2 files changed, 7 insertions(+) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 39dd5d9552..ae61ea8b39 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -982,4 +982,9 @@ uint32_t BLASTBufferQueue::getLastTransformHint() const { } } +uint64_t BLASTBufferQueue::getLastAcquiredFrameNum() { + std::unique_lock _lock{mMutex}; + return mLastAcquiredFrameNumber; +} + } // namespace android diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index 615f284e42..698844c849 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -110,6 +110,8 @@ public: uint32_t getLastTransformHint() const; void flushShadowQueue(); + uint64_t getLastAcquiredFrameNum(); + virtual ~BLASTBufferQueue(); private: -- cgit v1.2.3-59-g8ed1b From e9323b30ab14bad2a06a5cc67c326462cd11856b Mon Sep 17 00:00:00 2001 From: Robert Carr Date: Tue, 30 Nov 2021 14:47:02 -0800 Subject: BLASTBufferQueue: Cap shadow queue size during sync While waiting for the transaction commit callback on a previous sync transaction BLASTBufferQueue will halt buffer processing to ensure later frames do not arrive at SurfaceFlinger first. These buffers wait in the shadow queue (tracked by mNumFrameAvailable) to be acquired later. If we end up in a situation where all the buffers are either in a sync transaction or in the shadow queue then dequeue buffer will begin to block. This isn't ideal, as dequeue buffer blocking can cause UI thread to block, aka UI thread can block on RenderThread. However completing the sync transaction (from a previous frame) can also depend on UI thread, aka RenderThread can now block on UI thread (since we need the transaction to apply in order to thaw the shadow queue). In this CL we try and avoid that situation by only keeping 1 frame in the shadow queue while waiting for the sync to complete. If a second frame comes in we will acquire and release the first before acquiring the second. Bug: 200285149 Change-Id: I5072765e7b94820b3e66c557f5a96172ccef8172 Merged-In: I5072765e7b94820b3e66c557f5a96172ccef8172 --- libs/gui/BLASTBufferQueue.cpp | 3 +++ 1 file changed, 3 insertions(+) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index f357c17e28..60c2e2e675 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -664,6 +664,9 @@ void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) { // add to shadow queue mNumFrameAvailable++; + if (mWaitForTransactionCallback && mNumFrameAvailable == 2) { + acquireAndReleaseBuffer(); + } ATRACE_INT(mQueuedBufferTrace.c_str(), mNumFrameAvailable + mNumAcquired - mPendingRelease.size()); -- cgit v1.2.3-59-g8ed1b From 22b6d239c14a65af4b60d15ade8517e768c5f2b3 Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Mon, 6 Dec 2021 16:45:48 -0800 Subject: BBQ: Capture initial destframe change from BBQ Allow the caller to pass in a transaction that can capture the initial destination frame changes. This allows the caller to apply other destination frame changes via BBQ#update in order. Bug: 195443440 Test: atest BLASTBufferQueueTest Test: repro steps from bug Change-Id: Ibf53a0efdebb87291d081e48633c373a98d347b1 Merged-In: Ibf53a0efdebb87291d081e48633c373a98d347b1 --- libs/gui/BLASTBufferQueue.cpp | 36 ++++++++++++--------------------- libs/gui/include/gui/BLASTBufferQueue.h | 1 + 2 files changed, 14 insertions(+), 23 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 60c2e2e675..76636c2b9e 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -132,12 +132,11 @@ void BLASTBufferItemConsumer::onSidebandStreamChanged() { } } -BLASTBufferQueue::BLASTBufferQueue(const std::string& name, const sp& surface, - int width, int height, int32_t format) - : mSurfaceControl(surface), - mSize(width, height), +BLASTBufferQueue::BLASTBufferQueue(const std::string& name) + : mSurfaceControl(nullptr), + mSize(1, 1), mRequestedSize(mSize), - mFormat(format), + mFormat(PIXEL_FORMAT_RGBA_8888), mNextTransaction(nullptr) { createBufferQueue(&mProducer, &mConsumer); // since the adapter is in the client process, set dequeue timeout @@ -158,24 +157,19 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name, const spsetName(String8(consumerName.c_str())); mBufferItemConsumer->setFrameAvailableListener(this); mBufferItemConsumer->setBufferFreedListener(this); - mBufferItemConsumer->setDefaultBufferSize(mSize.width, mSize.height); - mBufferItemConsumer->setDefaultBufferFormat(convertBufferFormat(format)); mBufferItemConsumer->setBlastBufferQueue(this); ComposerService::getComposerService()->getMaxAcquiredBufferCount(&mMaxAcquiredBuffers); mBufferItemConsumer->setMaxAcquiredBufferCount(mMaxAcquiredBuffers); - - mTransformHint = mSurfaceControl->getTransformHint(); - mBufferItemConsumer->setTransformHint(mTransformHint); - SurfaceComposerClient::Transaction() - .setFlags(surface, layer_state_t::eEnableBackpressure, - layer_state_t::eEnableBackpressure) - .setApplyToken(mApplyToken) - .apply(); mNumAcquired = 0; mNumFrameAvailable = 0; - BQA_LOGV("BLASTBufferQueue created width=%d height=%d format=%d mTransformHint=%d", width, - height, format, mTransformHint); + BQA_LOGV("BLASTBufferQueue created"); +} + +BLASTBufferQueue::BLASTBufferQueue(const std::string& name, const sp& surface, + int width, int height, int32_t format) + : BLASTBufferQueue(name) { + update(surface, width, height, format); } BLASTBufferQueue::~BLASTBufferQueue() { @@ -227,12 +221,9 @@ void BLASTBufferQueue::update(const sp& surface, uint32_t width, // If the buffer supports scaling, update the frame immediately since the client may // want to scale the existing buffer to the new size. mSize = mRequestedSize; - // We only need to update the scale if we've received at least one buffer. The reason - // for this is the scale is calculated based on the requested size and buffer size. - // If there's no buffer, the scale will always be 1. SurfaceComposerClient::Transaction* destFrameTransaction = (outTransaction) ? outTransaction : &t; - if (mSurfaceControl != nullptr && mLastBufferInfo.hasBuffer) { + if (mSurfaceControl != nullptr) { destFrameTransaction->setDestinationFrame(mSurfaceControl, Rect(0, 0, newSize.getWidth(), newSize.getHeight())); @@ -530,9 +521,8 @@ void BLASTBufferQueue::acquireNextBufferLocked( // Ensure BLASTBufferQueue stays alive until we receive the transaction complete callback. incStrong((void*)transactionCallbackThunk); - const bool sizeHasChanged = mRequestedSize != mSize; + const bool updateDestinationFrame = mRequestedSize != mSize; mSize = mRequestedSize; - const bool updateDestinationFrame = sizeHasChanged || !mLastBufferInfo.hasBuffer; Rect crop = computeCrop(bufferItem); mLastBufferInfo.update(true /* hasBuffer */, bufferItem.mGraphicBuffer->getWidth(), bufferItem.mGraphicBuffer->getHeight(), bufferItem.mTransform, diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index 698844c849..c1d389160c 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -74,6 +74,7 @@ class BLASTBufferQueue : public ConsumerBase::FrameAvailableListener, public BufferItemConsumer::BufferFreedListener { public: + BLASTBufferQueue(const std::string& name); BLASTBufferQueue(const std::string& name, const sp& surface, int width, int height, int32_t format); -- cgit v1.2.3-59-g8ed1b From aca25f60537453114bfe7f0f8587e551438ca7a5 Mon Sep 17 00:00:00 2001 From: Robert Carr Date: Fri, 31 Dec 2021 16:59:34 -0800 Subject: DO NOT MERGE: BlastBufferQueue: Fake release if not received by complete Perfetto telemetry shows a cluster where the transactionComplete callback is arriving but the release buffer callback is not arriving. This leads to blocking and ANRs. We try to work around this by generating a fake release buffer callback for previous buffers when we receive a TransactionCompleteCallback. How can we know this is safe? The criteria to safely release buffers are as follows: Condition 1. SurfaceFlinger does not plan to use the buffer Condition 2. Have the acquire fence (or a later signalling fence)if dropped, have the HWC release fence if presented. Now lets break down cases where the transaction complete callback arrives before the release buffer callback. All release buffer callbacks fall in to two categories: Category 1. In band with the complete callback. In which case the client library in SurfaceComposerClient always sends release callbacks first. Category 2. Out of band, e.g. from setBuffer or merge when dropping buffers In category 2 there are two scenarios: Scenario 1: Release callback was never going to arrive (bug) Scenario 2. Release callback descheduled, e.g. a. Transaction is filled with buffer and held in VRI/WM/SysUI b. A second transaction with buffer is merged in, the release buffer callback is emitted but not scheduled (async binder) c. The merged transaction is applied d. SurfaceFlinger processes a frame and emits the transaction complete callback e. The transaction complete callback is scheduled before the release callback is ever scheduled (since the 1 way binders are from different processes). In both scenarios, we can satisfy Conditions 1 and 2, because the HWC present fence is a later signalling fence than the acquire fence which the out of band callback will pass. While we may block extra on this fence. We will be safe by condition 2. Receiving the transaction complete callback should indicate condition 1 is satisfied. In category 1 there should only be a single scenario, the release buffer callback for frame N-1 is emitted immediately before the transaction callback for frame N, and so if it hasn't come at that time (and isn't out of band/scheduled e.g. category 2) then it will never come. We can further break this down in to two scenarios: 1. stat.previousReleaseFence is set correctly. This is the expected case. In this case, this is the same fence we would receive from the release buffer callback, and so we can satisfy condition 1 and 2 and safely release. 2. stat.previousReleaseFence is not set correctly. There is no known reason for this to happen, but there is also no known reason why we would receive a complete callback without the corresponding release, and so we have to explore the option as a potential risk/behavior change from the change. Hypothetically in category 1 case 2 we could experience some tearing. In category 1 we are currently experiencing ANR, and so the tradeoff is a risk of tearing vs a certainty of ANR. To tear the following would have to happen: 1. Enter the case where we previously ANRed 2. Enter the subcase where the release fence is not set correctly 3. Be scheduled very fast on the client side, in practicality HWC present has already returned and the fence should be firing very soon 4. The previous frame not be GL comp, in which case we were already done with the buffer and don't need the fence anyway. Conditions 2 and 3 should already make the occurence of tearing much lower than the occurence of ANRs. Furthermore 100% of observed ANRs were during GL comp (rotation animation) and so the telemetry indicates the risk of introducing tearing is very low. To review, we have shown that we can break down the side effects from the change in to two buckets (category 1, and category 2). In category 2, the possible negative side effect is to use too late of a fence. However this is still safe, and should be exceedingly rare. In category 1 we have shown that there are two scenarios, and in one of these scenarios we can experience tearing. We have furthermore argued that the risk of tearing should be exceedingly low even in this scenario, while the risk of ANR in this scenario was nearly 100%. This issue is not appearing in git_master branches and so the DO_NOT_MERGE tag. Bug: 197269223 Bug: 212846697 Test: Existing tests pass Change-Id: I757ed132ac076aa811df816e04a68f57b38e47e7 --- libs/gui/BLASTBufferQueue.cpp | 24 ++++++++++++++++++++++-- libs/gui/SurfaceComposerClient.cpp | 4 ++-- libs/gui/include/gui/BLASTBufferQueue.h | 8 ++++++++ libs/gui/include/gui/SurfaceComposerClient.h | 6 ++++-- libs/gui/include/gui/test/CallbackUtils.h | 2 +- 5 files changed, 37 insertions(+), 7 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 76636c2b9e..2104c77275 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -350,6 +350,16 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp staleReleases; + for (const auto& [key, value]: mSubmitted) { + if (currFrameNumber > key.framenumber) { + staleReleases.push_back(key); + } + } + for (const auto& staleRelease : staleReleases) { + releaseBufferCallbackLocked(staleRelease, stat.previousReleaseFence ? stat.previousReleaseFence : Fence::NO_FENCE, + stat.transformHint, stat.currentMaxAcquiredBufferCount); + } } else { BQA_LOGE("Failed to find matching SurfaceControl in transactionCallback"); } @@ -358,6 +368,7 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp& releaseFence, uint32_t transformHint, uint32_t currentMaxAcquiredBufferCount) { - ATRACE_CALL(); std::unique_lock _lock{mMutex}; + releaseBufferCallbackLocked(id, releaseFence, transformHint, currentMaxAcquiredBufferCount); +} + +void BLASTBufferQueue::releaseBufferCallbackLocked(const ReleaseCallbackId& id, + const sp& releaseFence, uint32_t transformHint, + uint32_t currentMaxAcquiredBufferCount) { + ATRACE_CALL(); BQA_LOGV("releaseBufferCallback %s", id.to_string().c_str()); if (mSurfaceControl != nullptr) { @@ -421,7 +438,10 @@ void BLASTBufferQueue::releaseBufferCallback(const ReleaseCallbackId& id, const auto numPendingBuffersToHold = isEGL ? std::max(0u, mMaxAcquiredBuffers - currentMaxAcquiredBufferCount) : 0; - mPendingRelease.emplace_back(ReleasedBuffer{id, releaseFence}); + auto rb = ReleasedBuffer{id, releaseFence}; + if (std::find(mPendingRelease.begin(), mPendingRelease.end(), rb) == mPendingRelease.end()) { + mPendingRelease.emplace_back(rb); + } // Release all buffers that are beyond the ones that we need to hold while (mPendingRelease.size() > numPendingBuffersToHold) { diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index 922bceb191..725ea6571d 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -296,7 +296,7 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener transactionStats.latchTime, surfaceStats.acquireTime, transactionStats.presentFence, surfaceStats.previousReleaseFence, surfaceStats.transformHint, - surfaceStats.eventStats); + surfaceStats.eventStats, surfaceStats.currentMaxAcquiredBufferCount); } callbackFunction(transactionStats.latchTime, transactionStats.presentFence, @@ -321,7 +321,7 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener transactionStats.latchTime, surfaceStats.acquireTime, transactionStats.presentFence, surfaceStats.previousReleaseFence, surfaceStats.transformHint, - surfaceStats.eventStats); + surfaceStats.eventStats, surfaceStats.currentMaxAcquiredBufferCount); if (callbacksMap[callbackId].surfaceControls[surfaceStats.surfaceControl]) { callbacksMap[callbackId] .surfaceControls[surfaceStats.surfaceControl] diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index c1d389160c..24d978a7e9 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -95,6 +95,8 @@ public: const std::vector& stats); void releaseBufferCallback(const ReleaseCallbackId& id, const sp& releaseFence, uint32_t transformHint, uint32_t currentMaxAcquiredBufferCount); + void releaseBufferCallbackLocked(const ReleaseCallbackId& id, const sp& releaseFence, + uint32_t transformHint, uint32_t currentMaxAcquiredBufferCount); void setNextTransaction(SurfaceComposerClient::Transaction *t); void mergeWithNextTransaction(SurfaceComposerClient::Transaction* t, uint64_t frameNumber); void setTransactionCompleteCallback(uint64_t frameNumber, @@ -164,6 +166,12 @@ private: struct ReleasedBuffer { ReleaseCallbackId callbackId; sp releaseFence; + bool operator==(const ReleasedBuffer& rhs) const { + // Only compare Id so if we somehow got two callbacks + // with different fences we don't decrement mNumAcquired + // too far. + return rhs.callbackId == callbackId; + } }; std::deque mPendingRelease GUARDED_BY(mMutex); diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h index 9af9e64062..0d1d1a37bd 100644 --- a/libs/gui/include/gui/SurfaceComposerClient.h +++ b/libs/gui/include/gui/SurfaceComposerClient.h @@ -57,14 +57,15 @@ class Region; struct SurfaceControlStats { SurfaceControlStats(const sp& sc, nsecs_t latchTime, nsecs_t acquireTime, const sp& presentFence, const sp& prevReleaseFence, - uint32_t hint, FrameEventHistoryStats eventStats) + uint32_t hint, FrameEventHistoryStats eventStats, uint32_t currentMaxAcquiredBufferCount) : surfaceControl(sc), latchTime(latchTime), acquireTime(acquireTime), presentFence(presentFence), previousReleaseFence(prevReleaseFence), transformHint(hint), - frameEventStats(eventStats) {} + frameEventStats(eventStats), + currentMaxAcquiredBufferCount(currentMaxAcquiredBufferCount) {} sp surfaceControl; nsecs_t latchTime = -1; @@ -73,6 +74,7 @@ struct SurfaceControlStats { sp previousReleaseFence; uint32_t transformHint = 0; FrameEventHistoryStats frameEventStats; + uint32_t currentMaxAcquiredBufferCount = 0; }; using TransactionCompletedCallbackTakesContext = diff --git a/libs/gui/include/gui/test/CallbackUtils.h b/libs/gui/include/gui/test/CallbackUtils.h index 64032087eb..d2e04268dc 100644 --- a/libs/gui/include/gui/test/CallbackUtils.h +++ b/libs/gui/include/gui/test/CallbackUtils.h @@ -135,7 +135,7 @@ private: void verifySurfaceControlStats(const SurfaceControlStats& surfaceControlStats, nsecs_t latchTime) const { const auto& [surfaceControl, latch, acquireTime, presentFence, previousReleaseFence, - transformHint, frameEvents] = surfaceControlStats; + transformHint, frameEvents, ignore] = surfaceControlStats; ASSERT_EQ(acquireTime > 0, mBufferResult == ExpectedResult::Buffer::ACQUIRED) << "bad acquire time"; -- cgit v1.2.3-59-g8ed1b From b6f8479039c1462a0f2e041d6376e3e208bbb70f Mon Sep 17 00:00:00 2001 From: Ady Abraham Date: Wed, 15 Dec 2021 11:58:56 -0800 Subject: keep a wp in BufferItemConsumer Use a wp<> instead of raw pointer and remove the mutex around it to prevent potential deadlocks. Bug: 208121127 Test: stress test by partner Test: atest BLASTBufferQueueTest Change-Id: Iffed80410aeffc9b724d5c01ca2ec589c9622990 Merged-In: Iffed80410aeffc9b724d5c01ca2ec589c9622990 --- libs/gui/BLASTBufferQueue.cpp | 15 ++++----------- libs/gui/include/gui/BLASTBufferQueue.h | 11 +++++------ 2 files changed, 9 insertions(+), 17 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index 2104c77275..f034642681 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -119,16 +119,11 @@ void BLASTBufferItemConsumer::getConnectionEvents(uint64_t frameNumber, bool* ne if (needsDisconnect != nullptr) *needsDisconnect = disconnect; } -void BLASTBufferItemConsumer::setBlastBufferQueue(BLASTBufferQueue* blastbufferqueue) { - std::scoped_lock lock(mBufferQueueMutex); - mBLASTBufferQueue = blastbufferqueue; -} - void BLASTBufferItemConsumer::onSidebandStreamChanged() { - std::scoped_lock lock(mBufferQueueMutex); - if (mBLASTBufferQueue != nullptr) { + sp bbq = mBLASTBufferQueue.promote(); + if (bbq != nullptr) { sp stream = getSidebandStream(); - mBLASTBufferQueue->setSidebandStream(stream); + bbq->setSidebandStream(stream); } } @@ -148,7 +143,7 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name) mBufferItemConsumer = new BLASTBufferItemConsumer(mConsumer, GraphicBuffer::USAGE_HW_COMPOSER | GraphicBuffer::USAGE_HW_TEXTURE, - 1, false); + 1, false, this); static int32_t id = 0; mName = name + "#" + std::to_string(id); auto consumerName = mName + "(BLAST Consumer)" + std::to_string(id); @@ -157,7 +152,6 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name) mBufferItemConsumer->setName(String8(consumerName.c_str())); mBufferItemConsumer->setFrameAvailableListener(this); mBufferItemConsumer->setBufferFreedListener(this); - mBufferItemConsumer->setBlastBufferQueue(this); ComposerService::getComposerService()->getMaxAcquiredBufferCount(&mMaxAcquiredBuffers); mBufferItemConsumer->setMaxAcquiredBufferCount(mMaxAcquiredBuffers); @@ -173,7 +167,6 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name, const spsetBlastBufferQueue(nullptr); if (mPendingTransactions.empty()) { return; } diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index 24d978a7e9..f9e40ecb5b 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -38,11 +38,11 @@ class BufferItemConsumer; class BLASTBufferItemConsumer : public BufferItemConsumer { public: BLASTBufferItemConsumer(const sp& consumer, uint64_t consumerUsage, - int bufferCount, bool controlledByApp) + int bufferCount, bool controlledByApp, wp bbq) : BufferItemConsumer(consumer, consumerUsage, bufferCount, controlledByApp), + mBLASTBufferQueue(std::move(bbq)), mCurrentlyConnected(false), - mPreviouslyConnected(false), - mBLASTBufferQueue(nullptr) {} + mPreviouslyConnected(false) {} void onDisconnect() override; void addAndGetFrameTimestamps(const NewFrameEventsEntry* newTimestamps, @@ -53,21 +53,20 @@ public: CompositorTiming compositorTiming, nsecs_t latchTime, nsecs_t dequeueReadyTime) REQUIRES(mMutex); void getConnectionEvents(uint64_t frameNumber, bool* needsDisconnect); - void setBlastBufferQueue(BLASTBufferQueue* blastbufferqueue) REQUIRES(mMutex); protected: void onSidebandStreamChanged() override REQUIRES(mMutex); private: + const wp mBLASTBufferQueue; + uint64_t mCurrentFrameNumber = 0; Mutex mMutex; - std::mutex mBufferQueueMutex; ConsumerFrameEventHistory mFrameEventHistory GUARDED_BY(mMutex); std::queue mDisconnectEvents GUARDED_BY(mMutex); bool mCurrentlyConnected GUARDED_BY(mMutex); bool mPreviouslyConnected GUARDED_BY(mMutex); - BLASTBufferQueue* mBLASTBufferQueue GUARDED_BY(mBufferQueueMutex); }; class BLASTBufferQueue -- cgit v1.2.3-59-g8ed1b