Send releaseCallbackId and releaseFence to correct listener
The scenario is the following:
1. Process1 creates a T1 with a new buffer, release callback, and
transaction callback. The release callback is stored locally in
mReleaseBufferCallbacks so it only lives in Process1.
2. Process2 creates a T2 with a transaction callback
3. When T1 and T2 are merged, the SC from T1 end up getting the
transaction callbacks from T2. SC will end up with multiple transaction
callbacks
4. In BufferStateLayer, we only added mPreviousReleaseCallbackId to one
transaction callback. There's a chance the release callback would only
end up on the transaction callback for Process2.
Process2 doesn't have the data about the callback since it was only
stored in Process1. When Process2 gets the transaction callback, it
will not handle the releaseCallbackId since it doesn't know what release
callback it represents. Process1 will get a transaction callback, but will
not contain a releaseCallbackId.
To fix, add releaseBufferEndpoint that is set when the client calls
setBuffer. This is used in SurfaceFlinger to recognize which listener
was the one that set the buffer so it can send the release information
to the correct listener.
Test: ReleaseBufferCallback_test
Bug: 193565751
Change-Id: I534fbde2a54608c2e30852e48dc0c75c86b22525
diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp
index 6253036..5a59b67 100644
--- a/services/surfaceflinger/BufferStateLayer.cpp
+++ b/services/surfaceflinger/BufferStateLayer.cpp
@@ -158,7 +158,8 @@
// transaction doesn't need a previous release fence.
sp<CallbackHandle> ch;
for (auto& handle : mDrawingState.callbackHandles) {
- if (handle->releasePreviousBuffer) {
+ if (handle->releasePreviousBuffer &&
+ mDrawingState.releaseBufferEndpoint == handle->listener) {
ch = handle;
break;
}
@@ -199,14 +200,9 @@
mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate(mOwnerUid);
}
- // If there are multiple transactions in this frame, set the previous id on the earliest
- // transacton. We don't need to pass in the released buffer id to multiple transactions.
- // The buffer id does not have to correspond to any particular transaction as long as the
- // listening end point is the same but the client expects the first transaction callback that
- // replaces the presented buffer to contain the release fence. This follows the same logic.
- // see BufferStateLayer::onLayerDisplayed.
for (auto& handle : mDrawingState.callbackHandles) {
- if (handle->releasePreviousBuffer) {
+ if (handle->releasePreviousBuffer &&
+ mDrawingState.releaseBufferEndpoint == handle->listener) {
handle->previousReleaseCallbackId = mPreviousReleaseCallbackId;
break;
}
@@ -420,7 +416,8 @@
nsecs_t desiredPresentTime, bool isAutoTimestamp,
const client_cache_t& clientCacheId, uint64_t frameNumber,
std::optional<nsecs_t> dequeueTime, const FrameTimelineInfo& info,
- const sp<ITransactionCompletedListener>& releaseBufferListener) {
+ const sp<ITransactionCompletedListener>& releaseBufferListener,
+ const sp<IBinder>& releaseBufferEndpoint) {
ATRACE_CALL();
if (mDrawingState.buffer) {
@@ -484,6 +481,7 @@
mDrawingState.width = mDrawingState.buffer->getBuffer()->getWidth();
mDrawingState.height = mDrawingState.buffer->getBuffer()->getHeight();
+ mDrawingState.releaseBufferEndpoint = releaseBufferEndpoint;
return true;
}