From ca81c05b5abe743d41687417139039aa25771fd5 Mon Sep 17 00:00:00 2001 From: Patrick Williams Date: Thu, 15 Aug 2024 12:38:34 -0500 Subject: Fix stale buffer release issue. BufferItemConsumer sometimes returns BufferItems where mIsStale is already set to true, breaking the logic for ignoring buffers that were released via a disconnect. This CL adds a new field for BLAST to use for this purpose. Bug: 294133380 Flag: com.android.graphics.libgui.flags.buffer_release_channel Test: CtsMediaDecoderTestCases, CtsDeqpTestCases Change-Id: Ie32c9c6e0e26e68b794a1ca78cdd3c2395ac0966 --- libs/gui/BLASTBufferQueue.cpp | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index cda49850f2..f065ffa611 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -510,7 +510,7 @@ void BLASTBufferQueue::releaseBuffer(const ReleaseCallbackId& callbackId, return; } #if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL) - if (!it->second.mIsStale) { + if (!it->second.disconnectedAfterAcquired) { mNumAcquired--; } #else @@ -566,7 +566,7 @@ status_t BLASTBufferQueue::acquireNextBufferLocked( applyTransaction = false; } - BufferItem bufferItem; + BLASTBufferItem bufferItem; status_t status = mBufferItemConsumer->acquireBuffer(&bufferItem, 0 /* expectedPresent */, false); @@ -1130,9 +1130,9 @@ public: // can be non-blocking when the producer is in the client process. class BBQBufferQueueProducer : public BufferQueueProducer { public: - BBQBufferQueueProducer(const sp& core, wp bbq) + BBQBufferQueueProducer(const sp& core, const wp& bbq) : BufferQueueProducer(core, false /* consumerIsSurfaceFlinger*/), - mBLASTBufferQueue(std::move(bbq)) {} + mBLASTBufferQueue(bbq) {} status_t connect(const sp& listener, int api, bool producerControlledByApp, QueueBufferOutput* output) override { @@ -1156,10 +1156,19 @@ public: return status; } + // We need to reset dequeued and acquired counts because BufferQueueProducer::disconnect + // calls BufferQueueCore::freeAllBuffersLocked which frees all dequeued and acquired + // buffers. We don't reset mNumFrameAvailable because these buffers are still available + // in BufferItemConsumer. bbq->mNumDequeued = 0; bbq->mNumAcquired = 0; + // SurfaceFlinger sends release callbacks for buffers that have been acquired after a + // disconnect. We set disconnectedAfterAcquired to true so that we can ignore any stale + // releases that come in after the producer is disconnected. Otherwise, releaseBuffer will + // decrement mNumAcquired for a buffer that was acquired before we reset mNumAcquired to + // zero. for (auto& [releaseId, bufferItem] : bbq->mSubmitted) { - bufferItem.mIsStale = true; + bufferItem.disconnectedAfterAcquired = true; } return OK; -- cgit v1.2.3-59-g8ed1b