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

1 files changed, 25 insertions, 18 deletions

diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index fd78309462..6ea070ea2f 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -197,15 +197,16 @@ void TransactionCompletedListener::removeJankListener(const sp<JankDataListener>
     }
 }
 
-void TransactionCompletedListener::setReleaseBufferCallback(uint64_t graphicBufferId,
+void TransactionCompletedListener::setReleaseBufferCallback(const ReleaseCallbackId& callbackId,
                                                             ReleaseBufferCallback listener) {
     std::scoped_lock<std::mutex> lock(mMutex);
-    mReleaseBufferCallbacks[graphicBufferId] = listener;
+    mReleaseBufferCallbacks[callbackId] = listener;
 }
 
-void TransactionCompletedListener::removeReleaseBufferCallback(uint64_t graphicBufferId) {
+void TransactionCompletedListener::removeReleaseBufferCallback(
+        const ReleaseCallbackId& callbackId) {
     std::scoped_lock<std::mutex> lock(mMutex);
-    mReleaseBufferCallbacks.erase(graphicBufferId);
+    mReleaseBufferCallbacks.erase(callbackId);
 }
 
 void TransactionCompletedListener::addSurfaceStatsListener(void* context, void* cookie,
@@ -319,14 +320,15 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener
                 // and call them. This is a performance optimization when we have a transaction
                 // callback and a release buffer callback happening at the same time to avoid an
                 // additional ipc call from the server.
-                if (surfaceStats.previousBufferId) {
+                if (surfaceStats.previousReleaseCallbackId != ReleaseCallbackId::INVALID_ID) {
                     ReleaseBufferCallback callback;
                     {
                         std::scoped_lock<std::mutex> lock(mMutex);
-                        callback = popReleaseBufferCallbackLocked(surfaceStats.previousBufferId);
+                        callback = popReleaseBufferCallbackLocked(
+                                surfaceStats.previousReleaseCallbackId);
                     }
                     if (callback) {
-                        callback(surfaceStats.previousBufferId,
+                        callback(surfaceStats.previousReleaseCallbackId,
                                  surfaceStats.previousReleaseFence
                                          ? surfaceStats.previousReleaseFence
                                          : Fence::NO_FENCE,
@@ -362,25 +364,26 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener
     }
 }
 
-void TransactionCompletedListener::onReleaseBuffer(uint64_t graphicBufferId, sp<Fence> releaseFence,
-                                                   uint32_t transformHint,
+void TransactionCompletedListener::onReleaseBuffer(ReleaseCallbackId callbackId,
+                                                   sp<Fence> releaseFence, uint32_t transformHint,
                                                    uint32_t currentMaxAcquiredBufferCount) {
     ReleaseBufferCallback callback;
     {
         std::scoped_lock<std::mutex> lock(mMutex);
-        callback = popReleaseBufferCallbackLocked(graphicBufferId);
+        callback = popReleaseBufferCallbackLocked(callbackId);
     }
     if (!callback) {
-        ALOGE("Could not call release buffer callback, buffer not found %" PRIu64, graphicBufferId);
+        ALOGE("Could not call release buffer callback, buffer not found %s",
+              callbackId.to_string().c_str());
         return;
     }
-    callback(graphicBufferId, releaseFence, transformHint, currentMaxAcquiredBufferCount);
+    callback(callbackId, releaseFence, transformHint, currentMaxAcquiredBufferCount);
 }
 
 ReleaseBufferCallback TransactionCompletedListener::popReleaseBufferCallbackLocked(
-        uint64_t graphicBufferId) {
+        const ReleaseCallbackId& callbackId) {
     ReleaseBufferCallback callback;
-    auto itr = mReleaseBufferCallbacks.find(graphicBufferId);
+    auto itr = mReleaseBufferCallbacks.find(callbackId);
     if (itr == mReleaseBufferCallbacks.end()) {
         return nullptr;
     }
@@ -1258,7 +1261,7 @@ SurfaceComposerClient::Transaction::setTransformToDisplayInverse(const sp<Surfac
 }
 
 SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setBuffer(
-        const sp<SurfaceControl>& sc, const sp<GraphicBuffer>& buffer,
+        const sp<SurfaceControl>& sc, const sp<GraphicBuffer>& buffer, const ReleaseCallbackId& id,
         ReleaseBufferCallback callback) {
     layer_state_t* s = getLayerState(sc);
     if (!s) {
@@ -1271,7 +1274,7 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setBuffe
     if (mIsAutoTimestamp) {
         mDesiredPresentTime = systemTime();
     }
-    setReleaseBufferCallback(s, callback);
+    setReleaseBufferCallback(s, id, callback);
 
     registerSurfaceControlForCallback(sc);
 
@@ -1286,10 +1289,13 @@ void SurfaceComposerClient::Transaction::removeReleaseBufferCallback(layer_state
 
     s->what &= ~static_cast<uint64_t>(layer_state_t::eReleaseBufferListenerChanged);
     s->releaseBufferListener = nullptr;
-    TransactionCompletedListener::getInstance()->removeReleaseBufferCallback(s->buffer->getId());
+    auto listener = TransactionCompletedListener::getInstance();
+    listener->removeReleaseBufferCallback(s->releaseCallbackId);
+    s->releaseCallbackId = ReleaseCallbackId::INVALID_ID;
 }
 
 void SurfaceComposerClient::Transaction::setReleaseBufferCallback(layer_state_t* s,
+                                                                  const ReleaseCallbackId& id,
                                                                   ReleaseBufferCallback callback) {
     if (!callback) {
         return;
@@ -1303,8 +1309,9 @@ void SurfaceComposerClient::Transaction::setReleaseBufferCallback(layer_state_t*
 
     s->what |= layer_state_t::eReleaseBufferListenerChanged;
     s->releaseBufferListener = TransactionCompletedListener::getIInstance();
+    s->releaseCallbackId = id;
     auto listener = TransactionCompletedListener::getInstance();
-    listener->setReleaseBufferCallback(s->buffer->getId(), callback);
+    listener->setReleaseBufferCallback(id, callback);
 }
 
 SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setAcquireFence(