From 69058fb42b64794bb86a8ed23693eaf3dcc686ea Mon Sep 17 00:00:00 2001 From: chaviw Date: Mon, 27 Sep 2021 09:37:30 -0500 Subject: Call release buffer if the buffer is overwritten in the client Clients can overwrite the buffer in a transaction before it's applied. When this happens, the buffer is then replaced, but the client doesn't get a callback that the buffer was released. This could cause the client to run out of buffers since the dropped ones become lost. This can happen in two situations: 1. Merging two transactions where each has a buffer for the same SurfaceControl 2. Transaction.setBuffer is called multiple times before an apply In both cases, call the release callback so the client can properly handle the dropped buffers. It's also possible that the merging occurs in a different process than the one that set the buffer. Because of that, we need to ensure we call the correct releaseBufferListener that's associated with the one that set the buffer. The transformHint is removed from the releaseCallback since it's already provided in the transaction callback. It was originally added in the release callback to give it to BBQ early so it can know the new transform hint before rendering a new frame. However, the transform hint is now provided by VRI to BBQ so it no longer needs to be sent back via release callback. Test: ReleaseBufferCallbackTest Change-Id: I9df0c713bf841f53e1a3d092f33179573a680dc4 Bug: 200285149 --- libs/gui/SurfaceComposerClient.cpp | 90 ++++++++++++++++++++++++++++++-------- 1 file changed, 71 insertions(+), 19 deletions(-) (limited to 'libs/gui/SurfaceComposerClient.cpp') diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index aca59b6d3b..2713be060a 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -214,12 +214,6 @@ void TransactionCompletedListener::setReleaseBufferCallback(const ReleaseCallbac mReleaseBufferCallbacks[callbackId] = listener; } -void TransactionCompletedListener::removeReleaseBufferCallback( - const ReleaseCallbackId& callbackId) { - std::scoped_lock lock(mMutex); - mReleaseBufferCallbacks.erase(callbackId); -} - void TransactionCompletedListener::addSurfaceStatsListener(void* context, void* cookie, sp surfaceControl, SurfaceStatsCallback listener) { std::scoped_lock lock(mSurfaceStatsListenerMutex); @@ -343,7 +337,6 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener surfaceStats.previousReleaseFence ? surfaceStats.previousReleaseFence : Fence::NO_FENCE, - surfaceStats.transformHint, surfaceStats.currentMaxAcquiredBufferCount); } } @@ -389,7 +382,7 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener } void TransactionCompletedListener::onReleaseBuffer(ReleaseCallbackId callbackId, - sp releaseFence, uint32_t transformHint, + sp releaseFence, uint32_t currentMaxAcquiredBufferCount) { ReleaseBufferCallback callback; { @@ -401,7 +394,11 @@ void TransactionCompletedListener::onReleaseBuffer(ReleaseCallbackId callbackId, callbackId.to_string().c_str()); return; } - callback(callbackId, releaseFence, transformHint, currentMaxAcquiredBufferCount); + std::optional optionalMaxAcquiredBufferCount = + currentMaxAcquiredBufferCount == UINT_MAX + ? std::nullopt + : std::make_optional(currentMaxAcquiredBufferCount); + callback(callbackId, releaseFence, optionalMaxAcquiredBufferCount); } ReleaseBufferCallback TransactionCompletedListener::popReleaseBufferCallbackLocked( @@ -712,11 +709,34 @@ status_t SurfaceComposerClient::Transaction::writeToParcel(Parcel* parcel) const return NO_ERROR; } +void SurfaceComposerClient::Transaction::releaseBufferIfOverwriting(const layer_state_t& state) { + if (!(state.what & layer_state_t::eBufferChanged)) { + return; + } + + auto listener = state.bufferData.releaseBufferListener; + sp fence = + state.bufferData.acquireFence ? state.bufferData.acquireFence : Fence::NO_FENCE; + if (state.bufferData.releaseBufferEndpoint == + IInterface::asBinder(TransactionCompletedListener::getIInstance())) { + // if the callback is in process, run on a different thread to avoid any lock contigency + // issues in the client. + SurfaceComposerClient::getDefault() + ->mReleaseCallbackThread.addReleaseCallback(state.bufferData.releaseCallbackId, + fence); + } else { + listener->onReleaseBuffer(state.bufferData.releaseCallbackId, fence, UINT_MAX); + } +} + SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::merge(Transaction&& other) { for (auto const& [handle, composerState] : other.mComposerStates) { if (mComposerStates.count(handle) == 0) { mComposerStates[handle] = composerState; } else { + if (composerState.state.what & layer_state_t::eBufferChanged) { + releaseBufferIfOverwriting(mComposerStates[handle].state); + } mComposerStates[handle].state.merge(composerState.state); } } @@ -1296,7 +1316,9 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setBuffe mStatus = BAD_INDEX; return *this; } - removeReleaseBufferCallback(s); + + releaseBufferIfOverwriting(*s); + BufferData bufferData; bufferData.buffer = buffer; if (frameNumber) { @@ -1321,15 +1343,6 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setBuffe return *this; } -void SurfaceComposerClient::Transaction::removeReleaseBufferCallback(layer_state_t* s) { - if (!(s->what & layer_state_t::eBufferChanged)) { - return; - } - - auto listener = TransactionCompletedListener::getInstance(); - listener->removeReleaseBufferCallback(s->bufferData.releaseCallbackId); -} - void SurfaceComposerClient::Transaction::setReleaseBufferCallback(BufferData* bufferData, const ReleaseCallbackId& id, ReleaseBufferCallback callback) { @@ -2210,4 +2223,43 @@ status_t ScreenshotClient::captureLayers(const LayerCaptureArgs& captureArgs, return s->captureLayers(captureArgs, captureListener); } +// --------------------------------------------------------------------------------- + +void ReleaseCallbackThread::addReleaseCallback(const ReleaseCallbackId callbackId, + sp releaseFence) { + std::scoped_lock lock(mMutex); + if (!mStarted) { + mThread = std::thread(&ReleaseCallbackThread::threadMain, this); + mStarted = true; + } + + mCallbackInfos.emplace(callbackId, std::move(releaseFence)); + mReleaseCallbackPending.notify_one(); +} + +void ReleaseCallbackThread::threadMain() { + const auto listener = TransactionCompletedListener::getInstance(); + std::queue>> callbackInfos; + while (true) { + { + std::unique_lock lock(mMutex); + callbackInfos = std::move(mCallbackInfos); + mCallbackInfos = {}; + } + + while (!callbackInfos.empty()) { + auto [callbackId, releaseFence] = callbackInfos.front(); + listener->onReleaseBuffer(callbackId, std::move(releaseFence), UINT_MAX); + callbackInfos.pop(); + } + + { + std::unique_lock lock(mMutex); + if (mCallbackInfos.size() == 0) { + mReleaseCallbackPending.wait(lock); + } + } + } +} + } // namespace android -- cgit v1.2.3-59-g8ed1b