From 4ba0c2e2a2e8a4870b4d5055bd765d5514a35ebc Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Thu, 24 Jun 2021 11:27:17 -0700 Subject: Blast: Use a unique id to track buffers When buffer queue is configured in shared buffer mode, the client can queue the same buffer over and over again and the consumer can acquire the same buffer repeatedly. BBQ tracks acquired buffers by graphic buffer id and SCC tracks release buffer callbacks by graphic buffer ids. This breaks if a buffer is reused before release. Fix this by using graphic buffer id and framenumber to track acquired buffers and buffer release callbacks. Test: atest CtsDeqpTestCases:dEQP-VK.wsi.android.shared_presentable_image.scale_none.identity.inherit.demand CtsDeqpTestCases:dEQP-VK.wsi.android.colorspace#basic Bug: 191220669 Change-Id: Ibd54df9fa6780c26cd8de769bf9e43163abbed5a --- libs/gui/BLASTBufferQueue.cpp | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) (limited to 'libs/gui/BLASTBufferQueue.cpp') diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index e15e11cc20..29701168e7 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -132,8 +132,7 @@ void BLASTBufferItemConsumer::onSidebandStreamChanged() { BLASTBufferQueue::BLASTBufferQueue(const std::string& name, const sp& surface, int width, int height, int32_t format) - : mName(name), - mSurfaceControl(surface), + : mSurfaceControl(surface), mSize(width, height), mRequestedSize(mSize), mFormat(format), @@ -150,6 +149,7 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name, const sp context, uint64_t graphicBufferId, +static void releaseBufferCallbackThunk(wp context, const ReleaseCallbackId& id, const sp& releaseFence, uint32_t transformHint, uint32_t currentMaxAcquiredBufferCount) { sp blastBufferQueue = context.promote(); - ALOGV("releaseBufferCallbackThunk graphicBufferId=%" PRIu64 " blastBufferQueue=%s", - graphicBufferId, blastBufferQueue ? "alive" : "dead"); if (blastBufferQueue) { - blastBufferQueue->releaseBufferCallback(graphicBufferId, releaseFence, transformHint, + blastBufferQueue->releaseBufferCallback(id, releaseFence, transformHint, currentMaxAcquiredBufferCount); + } else { + ALOGV("releaseBufferCallbackThunk %s blastBufferQueue is dead", id.to_string().c_str()); } } -void BLASTBufferQueue::releaseBufferCallback(uint64_t graphicBufferId, +void BLASTBufferQueue::releaseBufferCallback(const ReleaseCallbackId& id, const sp& releaseFence, uint32_t transformHint, uint32_t currentMaxAcquiredBufferCount) { ATRACE_CALL(); std::unique_lock _lock{mMutex}; - BQA_LOGV("releaseBufferCallback graphicBufferId=%" PRIu64, graphicBufferId); + BQA_LOGV("releaseBufferCallback %s", id.to_string().c_str()); if (mSurfaceControl != nullptr) { mTransformHint = transformHint; @@ -343,25 +343,26 @@ void BLASTBufferQueue::releaseBufferCallback(uint64_t graphicBufferId, // on a lower refresh rate than the max supported. We only do that for EGL // clients as others don't care about latency const bool isEGL = [&] { - const auto it = mSubmitted.find(graphicBufferId); + const auto it = mSubmitted.find(id); return it != mSubmitted.end() && it->second.mApi == NATIVE_WINDOW_API_EGL; }(); const auto numPendingBuffersToHold = isEGL ? std::max(0u, mMaxAcquiredBuffers - currentMaxAcquiredBufferCount) : 0; - mPendingRelease.emplace_back(ReleasedBuffer{graphicBufferId, releaseFence}); + mPendingRelease.emplace_back(ReleasedBuffer{id, releaseFence}); // Release all buffers that are beyond the ones that we need to hold while (mPendingRelease.size() > numPendingBuffersToHold) { const auto releaseBuffer = mPendingRelease.front(); mPendingRelease.pop_front(); - auto it = mSubmitted.find(releaseBuffer.bufferId); + auto it = mSubmitted.find(releaseBuffer.callbackId); if (it == mSubmitted.end()) { - BQA_LOGE("ERROR: releaseBufferCallback without corresponding submitted buffer %" PRIu64, - graphicBufferId); + BQA_LOGE("ERROR: releaseBufferCallback without corresponding submitted buffer %s", + releaseBuffer.callbackId.to_string().c_str()); return; } mNumAcquired--; + BQA_LOGV("released %s", id.to_string().c_str()); mBufferItemConsumer->releaseBuffer(it->second, releaseBuffer.releaseFence); mSubmitted.erase(it); processNextBufferLocked(false /* useNextTransaction */); @@ -428,7 +429,9 @@ void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) { } mNumAcquired++; - mSubmitted[buffer->getId()] = bufferItem; + mLastAcquiredFrameNumber = bufferItem.mFrameNumber; + ReleaseCallbackId releaseCallbackId(buffer->getId(), mLastAcquiredFrameNumber); + mSubmitted[releaseCallbackId] = bufferItem; bool needsDisconnect = false; mBufferItemConsumer->getConnectionEvents(bufferItem.mFrameNumber, &needsDisconnect); @@ -442,7 +445,6 @@ void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) { incStrong((void*)transactionCallbackThunk); Rect crop = computeCrop(bufferItem); - mLastAcquiredFrameNumber = bufferItem.mFrameNumber; mLastBufferInfo.update(true /* hasBuffer */, bufferItem.mGraphicBuffer->getWidth(), bufferItem.mGraphicBuffer->getHeight(), bufferItem.mTransform, bufferItem.mScalingMode, crop); @@ -451,7 +453,7 @@ void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) { std::bind(releaseBufferCallbackThunk, wp(this) /* callbackContext */, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, std::placeholders::_4); - t->setBuffer(mSurfaceControl, buffer, releaseBufferCallback); + t->setBuffer(mSurfaceControl, buffer, releaseCallbackId, releaseBufferCallback); t->setDataspace(mSurfaceControl, static_cast(bufferItem.mDataSpace)); t->setHdrMetadata(mSurfaceControl, bufferItem.mHdrMetadata); t->setSurfaceDamageRegion(mSurfaceControl, bufferItem.mSurfaceDamage); @@ -510,11 +512,11 @@ void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) { BQA_LOGV("processNextBufferLocked size=%dx%d mFrameNumber=%" PRIu64 " applyTransaction=%s mTimestamp=%" PRId64 "%s mPendingTransactions.size=%d" - " graphicBufferId=%" PRIu64, + " graphicBufferId=%" PRIu64 "%s", mSize.width, mSize.height, bufferItem.mFrameNumber, toString(applyTransaction), bufferItem.mTimestamp, bufferItem.mIsAutoTimestamp ? "(auto)" : "", - static_cast(mPendingTransactions.size()), - bufferItem.mGraphicBuffer->getId()); + static_cast(mPendingTransactions.size()), bufferItem.mGraphicBuffer->getId(), + bufferItem.mAutoRefresh ? " mAutoRefresh" : ""); } Rect BLASTBufferQueue::computeCrop(const BufferItem& item) { -- cgit v1.2.3-59-g8ed1b