diff options
-rw-r--r-- | libs/gui/BLASTBufferQueue.cpp | 40 | ||||
-rw-r--r-- | libs/gui/ITransactionCompletedListener.cpp | 24 | ||||
-rw-r--r-- | libs/gui/SurfaceComposerClient.cpp | 43 | ||||
-rw-r--r-- | libs/gui/include/gui/BLASTBufferQueue.h | 7 | ||||
-rw-r--r-- | libs/gui/include/gui/ITransactionCompletedListener.h | 38 | ||||
-rw-r--r-- | libs/gui/include/gui/LayerState.h | 6 | ||||
-rw-r--r-- | libs/gui/include/gui/SurfaceComposerClient.h | 20 | ||||
-rw-r--r-- | services/surfaceflinger/BufferLayer.h | 1 | ||||
-rw-r--r-- | services/surfaceflinger/BufferStateLayer.cpp | 40 | ||||
-rw-r--r-- | services/surfaceflinger/BufferStateLayer.h | 9 | ||||
-rw-r--r-- | services/surfaceflinger/TransactionCallbackInvoker.cpp | 3 | ||||
-rw-r--r-- | services/surfaceflinger/TransactionCallbackInvoker.h | 2 | ||||
-rw-r--r-- | services/surfaceflinger/tests/ReleaseBufferCallback_test.cpp | 77 |
13 files changed, 192 insertions, 118 deletions
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<SurfaceControl>& 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<SurfaceCont GraphicBuffer::USAGE_HW_TEXTURE, 1, false); static int32_t id = 0; + mName = name + "#" + std::to_string(id); auto consumerName = mName + "(BLAST Consumer)" + std::to_string(id); mQueuedBufferTrace = "QueuedBuffer - " + mName + "BLAST#" + std::to_string(id); id++; @@ -313,24 +313,24 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp<Fence // BBQ. This is because if the BBQ is destroyed, then the buffers will be released by the client. // So we pass in a weak pointer to the BBQ and if it still alive, then we release the buffer. // Otherwise, this is a no-op. -static void releaseBufferCallbackThunk(wp<BLASTBufferQueue> context, uint64_t graphicBufferId, +static void releaseBufferCallbackThunk(wp<BLASTBufferQueue> context, const ReleaseCallbackId& id, const sp<Fence>& releaseFence, uint32_t transformHint, uint32_t currentMaxAcquiredBufferCount) { sp<BLASTBufferQueue> 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<Fence>& 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<BLASTBufferQueue>(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<ui::Dataspace>(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<uint32_t>(mPendingTransactions.size()), - bufferItem.mGraphicBuffer->getId()); + static_cast<uint32_t>(mPendingTransactions.size()), bufferItem.mGraphicBuffer->getId(), + bufferItem.mAutoRefresh ? " mAutoRefresh" : ""); } Rect BLASTBufferQueue::computeCrop(const BufferItem& item) { diff --git a/libs/gui/ITransactionCompletedListener.cpp b/libs/gui/ITransactionCompletedListener.cpp index 17499ecd50..98e8b548bd 100644 --- a/libs/gui/ITransactionCompletedListener.cpp +++ b/libs/gui/ITransactionCompletedListener.cpp @@ -125,7 +125,7 @@ status_t SurfaceStats::writeToParcel(Parcel* output) const { for (const auto& data : jankData) { SAFE_PARCEL(output->writeParcelable, data); } - SAFE_PARCEL(output->writeUint64, previousBufferId); + SAFE_PARCEL(output->writeParcelable, previousReleaseCallbackId); return NO_ERROR; } @@ -149,7 +149,7 @@ status_t SurfaceStats::readFromParcel(const Parcel* input) { SAFE_PARCEL(input->readParcelable, &data); jankData.push_back(data); } - SAFE_PARCEL(input->readUint64, &previousBufferId); + SAFE_PARCEL(input->readParcelable, &previousReleaseCallbackId); return NO_ERROR; } @@ -253,11 +253,11 @@ public: stats); } - void onReleaseBuffer(uint64_t graphicBufferId, sp<Fence> releaseFence, uint32_t transformHint, - uint32_t currentMaxAcquiredBufferCount) override { + void onReleaseBuffer(ReleaseCallbackId callbackId, sp<Fence> releaseFence, + uint32_t transformHint, uint32_t currentMaxAcquiredBufferCount) override { callRemoteAsync<decltype( &ITransactionCompletedListener::onReleaseBuffer)>(Tag::ON_RELEASE_BUFFER, - graphicBufferId, releaseFence, + callbackId, releaseFence, transformHint, currentMaxAcquiredBufferCount); } @@ -308,4 +308,18 @@ status_t CallbackId::readFromParcel(const Parcel* input) { return NO_ERROR; } +status_t ReleaseCallbackId::writeToParcel(Parcel* output) const { + SAFE_PARCEL(output->writeUint64, bufferId); + SAFE_PARCEL(output->writeUint64, framenumber); + return NO_ERROR; +} + +status_t ReleaseCallbackId::readFromParcel(const Parcel* input) { + SAFE_PARCEL(input->readUint64, &bufferId); + SAFE_PARCEL(input->readUint64, &framenumber); + return NO_ERROR; +} + +const ReleaseCallbackId ReleaseCallbackId::INVALID_ID = ReleaseCallbackId(0, 0); + }; // namespace android 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( diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index 26c7285ab0..0d6a673b02 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -89,7 +89,7 @@ public: void transactionCallback(nsecs_t latchTime, const sp<Fence>& presentFence, const std::vector<SurfaceControlStats>& stats); - void releaseBufferCallback(uint64_t graphicBufferId, const sp<Fence>& releaseFence, + void releaseBufferCallback(const ReleaseCallbackId& id, const sp<Fence>& releaseFence, uint32_t transformHint, uint32_t currentMaxAcquiredBufferCount); void setNextTransaction(SurfaceComposerClient::Transaction *t); void mergeWithNextTransaction(SurfaceComposerClient::Transaction* t, uint64_t frameNumber); @@ -144,13 +144,14 @@ private: // Keep a reference to the submitted buffers so we can release when surfaceflinger drops the // buffer or the buffer has been presented and a new buffer is ready to be presented. - std::unordered_map<uint64_t /* bufferId */, BufferItem> mSubmitted GUARDED_BY(mMutex); + std::unordered_map<ReleaseCallbackId, BufferItem, ReleaseBufferCallbackIdHash> mSubmitted + GUARDED_BY(mMutex); // Keep a queue of the released buffers instead of immediately releasing // the buffers back to the buffer queue. This would be controlled by SF // setting the max acquired buffer count. struct ReleasedBuffer { - uint64_t bufferId; + ReleaseCallbackId callbackId; sp<Fence> releaseFence; }; std::deque<ReleasedBuffer> mPendingRelease GUARDED_BY(mMutex); diff --git a/libs/gui/include/gui/ITransactionCompletedListener.h b/libs/gui/include/gui/ITransactionCompletedListener.h index d286c34ec8..937095c543 100644 --- a/libs/gui/include/gui/ITransactionCompletedListener.h +++ b/libs/gui/include/gui/ITransactionCompletedListener.h @@ -53,6 +53,36 @@ struct CallbackIdHash { std::size_t operator()(const CallbackId& key) const { return std::hash<int64_t>()(key.id); } }; +class ReleaseCallbackId : public Parcelable { +public: + static const ReleaseCallbackId INVALID_ID; + + uint64_t bufferId; + uint64_t framenumber; + ReleaseCallbackId() {} + ReleaseCallbackId(uint64_t bufferId, uint64_t framenumber) + : bufferId(bufferId), framenumber(framenumber) {} + status_t writeToParcel(Parcel* output) const override; + status_t readFromParcel(const Parcel* input) override; + + bool operator==(const ReleaseCallbackId& rhs) const { + return bufferId == rhs.bufferId && framenumber == rhs.framenumber; + } + bool operator!=(const ReleaseCallbackId& rhs) const { return !operator==(rhs); } + std::string to_string() const { + if (*this == INVALID_ID) return "INVALID_ID"; + + return "bufferId:" + std::to_string(bufferId) + + " framenumber:" + std::to_string(framenumber); + } +}; + +struct ReleaseBufferCallbackIdHash { + std::size_t operator()(const ReleaseCallbackId& key) const { + return std::hash<uint64_t>()(key.bufferId); + } +}; + class FrameEventHistoryStats : public Parcelable { public: status_t writeToParcel(Parcel* output) const override; @@ -103,7 +133,7 @@ public: SurfaceStats(const sp<IBinder>& sc, nsecs_t time, const sp<Fence>& prevReleaseFence, uint32_t hint, uint32_t currentMaxAcquiredBuffersCount, FrameEventHistoryStats frameEventStats, std::vector<JankData> jankData, - uint64_t previousBufferId) + ReleaseCallbackId previousReleaseCallbackId) : surfaceControl(sc), acquireTime(time), previousReleaseFence(prevReleaseFence), @@ -111,7 +141,7 @@ public: currentMaxAcquiredBufferCount(currentMaxAcquiredBuffersCount), eventStats(frameEventStats), jankData(std::move(jankData)), - previousBufferId(previousBufferId) {} + previousReleaseCallbackId(previousReleaseCallbackId) {} sp<IBinder> surfaceControl; nsecs_t acquireTime = -1; @@ -120,7 +150,7 @@ public: uint32_t currentMaxAcquiredBufferCount = 0; FrameEventHistoryStats eventStats; std::vector<JankData> jankData; - uint64_t previousBufferId; + ReleaseCallbackId previousReleaseCallbackId; }; class TransactionStats : public Parcelable { @@ -161,7 +191,7 @@ public: virtual void onTransactionCompleted(ListenerStats stats) = 0; - virtual void onReleaseBuffer(uint64_t graphicBufferId, sp<Fence> releaseFence, + virtual void onReleaseBuffer(ReleaseCallbackId callbackId, sp<Fence> releaseFence, uint32_t transformHint, uint32_t currentMaxAcquiredBufferCount) = 0; }; diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h index 16430b324d..8ac1e5d9e9 100644 --- a/libs/gui/include/gui/LayerState.h +++ b/libs/gui/include/gui/LayerState.h @@ -234,6 +234,12 @@ struct layer_state_t { // layers only. The callback includes a release fence as well as the graphic // buffer id to identify the buffer. sp<ITransactionCompletedListener> releaseBufferListener; + + // Keeps track of the release callback id associated with the listener. This + // is not sent to the server since the id can be reconstructed there. This + // is used to remove the old callback from the client process map if it is + // overwritten by another setBuffer call. + ReleaseCallbackId releaseCallbackId; }; struct ComposerState { diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h index 13994fd9eb..c2963b58df 100644 --- a/libs/gui/include/gui/SurfaceComposerClient.h +++ b/libs/gui/include/gui/SurfaceComposerClient.h @@ -82,7 +82,7 @@ using TransactionCompletedCallback = std::function<void(nsecs_t /*latchTime*/, const sp<Fence>& /*presentFence*/, const std::vector<SurfaceControlStats>& /*stats*/)>; using ReleaseBufferCallback = - std::function<void(uint64_t /* graphicsBufferId */, const sp<Fence>& /*releaseFence*/, + std::function<void(const ReleaseCallbackId&, const sp<Fence>& /*releaseFence*/, uint32_t transformHint, uint32_t currentMaxAcquiredBufferCount)>; using SurfaceStatsCallback = @@ -397,8 +397,9 @@ public: void cacheBuffers(); void registerSurfaceControlForCallback(const sp<SurfaceControl>& sc); - void setReleaseBufferCallback(layer_state_t* state, ReleaseBufferCallback callback); - void removeReleaseBufferCallback(layer_state_t* state); + void setReleaseBufferCallback(layer_state_t*, const ReleaseCallbackId&, + ReleaseBufferCallback); + void removeReleaseBufferCallback(layer_state_t*); public: Transaction(); @@ -470,6 +471,7 @@ public: Transaction& setTransformToDisplayInverse(const sp<SurfaceControl>& sc, bool transformToDisplayInverse); Transaction& setBuffer(const sp<SurfaceControl>& sc, const sp<GraphicBuffer>& buffer, + const ReleaseCallbackId& id = ReleaseCallbackId::INVALID_ID, ReleaseBufferCallback callback = nullptr); Transaction& setCachedBuffer(const sp<SurfaceControl>& sc, int32_t bufferId); Transaction& setAcquireFence(const sp<SurfaceControl>& sc, const sp<Fence>& fence); @@ -678,7 +680,7 @@ class TransactionCompletedListener : public BnTransactionCompletedListener { std::unordered_map<CallbackId, CallbackTranslation, CallbackIdHash> mCallbacks GUARDED_BY(mMutex); std::multimap<sp<IBinder>, sp<JankDataListener>> mJankListeners GUARDED_BY(mMutex); - std::unordered_map<uint64_t /* graphicsBufferId */, ReleaseBufferCallback> + std::unordered_map<ReleaseCallbackId, ReleaseBufferCallback, ReleaseBufferCallbackIdHash> mReleaseBufferCallbacks GUARDED_BY(mMutex); // This is protected by mSurfaceStatsListenerMutex, but GUARDED_BY isn't supported for @@ -717,16 +719,16 @@ public: SurfaceStatsCallback listener); void removeSurfaceStatsListener(void* context, void* cookie); - void setReleaseBufferCallback(uint64_t /* graphicsBufferId */, ReleaseBufferCallback); - void removeReleaseBufferCallback(uint64_t /* graphicsBufferId */); + void setReleaseBufferCallback(const ReleaseCallbackId&, ReleaseBufferCallback); + void removeReleaseBufferCallback(const ReleaseCallbackId&); // BnTransactionCompletedListener overrides void onTransactionCompleted(ListenerStats stats) override; - void onReleaseBuffer(uint64_t /* graphicsBufferId */, sp<Fence> releaseFence, - uint32_t transformHint, uint32_t currentMaxAcquiredBufferCount) override; + void onReleaseBuffer(ReleaseCallbackId, sp<Fence> releaseFence, uint32_t transformHint, + uint32_t currentMaxAcquiredBufferCount) override; private: - ReleaseBufferCallback popReleaseBufferCallbackLocked(uint64_t /* graphicsBufferId */); + ReleaseBufferCallback popReleaseBufferCallbackLocked(const ReleaseCallbackId&); }; } // namespace android diff --git a/services/surfaceflinger/BufferLayer.h b/services/surfaceflinger/BufferLayer.h index cd3d80e809..760c8b9f3c 100644 --- a/services/surfaceflinger/BufferLayer.h +++ b/services/surfaceflinger/BufferLayer.h @@ -133,6 +133,7 @@ protected: bool mTransformToDisplayInverse{false}; std::shared_ptr<renderengine::ExternalTexture> mBuffer; + uint64_t mFrameNumber; int mBufferSlot{BufferQueue::INVALID_BUFFER_SLOT}; bool mFrameLatencyNeeded{false}; diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp index 6b5cf04536..8cf0d66a08 100644 --- a/services/surfaceflinger/BufferStateLayer.cpp +++ b/services/surfaceflinger/BufferStateLayer.cpp @@ -44,25 +44,18 @@ namespace android { using PresentState = frametimeline::SurfaceFrame::PresentState; namespace { void callReleaseBufferCallback(const sp<ITransactionCompletedListener>& listener, - const sp<GraphicBuffer>& buffer, const sp<Fence>& releaseFence, - uint32_t transformHint, uint32_t currentMaxAcquiredBufferCount) { + const sp<GraphicBuffer>& buffer, uint64_t framenumber, + const sp<Fence>& releaseFence, uint32_t transformHint, + uint32_t currentMaxAcquiredBufferCount) { if (!listener) { return; } - listener->onReleaseBuffer(buffer->getId(), releaseFence ? releaseFence : Fence::NO_FENCE, - transformHint, currentMaxAcquiredBufferCount); + listener->onReleaseBuffer({buffer->getId(), framenumber}, + releaseFence ? releaseFence : Fence::NO_FENCE, transformHint, + currentMaxAcquiredBufferCount); } } // namespace -// clang-format off -const std::array<float, 16> BufferStateLayer::IDENTITY_MATRIX{ - 1, 0, 0, 0, - 0, 1, 0, 0, - 0, 0, 1, 0, - 0, 0, 0, 1 -}; -// clang-format on - BufferStateLayer::BufferStateLayer(const LayerCreationArgs& args) : BufferLayer(args), mHwcSlotGenerator(new HwcSlotGenerator()) { mDrawingState.dataspace = ui::Dataspace::V0_SRGB; @@ -75,8 +68,8 @@ BufferStateLayer::~BufferStateLayer() { // issue with the clone layer trying to use the texture. if (mBufferInfo.mBuffer != nullptr && !isClone()) { callReleaseBufferCallback(mDrawingState.releaseBufferListener, - mBufferInfo.mBuffer->getBuffer(), mBufferInfo.mFence, - mTransformHint, + mBufferInfo.mBuffer->getBuffer(), mBufferInfo.mFrameNumber, + mBufferInfo.mFence, mTransformHint, mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate( mOwnerUid)); } @@ -87,7 +80,7 @@ status_t BufferStateLayer::addReleaseFence(const sp<CallbackHandle>& ch, if (ch == nullptr) { return OK; } - ch->previousBufferId = mPreviousBufferId; + ch->previousReleaseCallbackId = mPreviousReleaseCallbackId; if (!ch->previousReleaseFence.get()) { ch->previousReleaseFence = fence; return OK; @@ -214,7 +207,7 @@ void BufferStateLayer::releasePendingBuffer(nsecs_t dequeueReadyTime) { // see BufferStateLayer::onLayerDisplayed. for (auto& handle : mDrawingState.callbackHandles) { if (handle->releasePreviousBuffer) { - handle->previousBufferId = mPreviousBufferId; + handle->previousReleaseCallbackId = mPreviousReleaseCallbackId; break; } } @@ -438,8 +431,8 @@ bool BufferStateLayer::setBuffer(const std::shared_ptr<renderengine::ExternalTex // dropped and we should decrement the pending buffer count and // call any release buffer callbacks if set. callReleaseBufferCallback(mDrawingState.releaseBufferListener, - mDrawingState.buffer->getBuffer(), mDrawingState.acquireFence, - mTransformHint, + mDrawingState.buffer->getBuffer(), mDrawingState.frameNumber, + mDrawingState.acquireFence, mTransformHint, mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate( mOwnerUid)); decrementPendingBufferCount(); @@ -684,7 +677,7 @@ uint64_t BufferStateLayer::getFrameNumber(nsecs_t /*expectedPresentTime*/) const * DeferTransactionUntil -> frameNumber = 2 * Random other stuff * } - * Now imagine getHeadFrameNumber returned mDrawingState.mFrameNumber (or mCurrentFrameNumber). + * Now imagine mFrameNumber returned mDrawingState.frameNumber (or mCurrentFrameNumber). * Prior to doTransaction SurfaceFlinger will call notifyAvailableFrames, but because we * haven't swapped mDrawingState to mDrawingState yet we will think the sync point * is not ready. So we will return false from applyPendingState and not swap @@ -792,9 +785,10 @@ status_t BufferStateLayer::updateActiveBuffer() { decrementPendingBufferCount(); } - mPreviousBufferId = getCurrentBufferId(); + mPreviousReleaseCallbackId = {getCurrentBufferId(), mBufferInfo.mFrameNumber}; mBufferInfo.mBuffer = s.buffer; mBufferInfo.mFence = s.acquireFence; + mBufferInfo.mFrameNumber = s.frameNumber; return NO_ERROR; } @@ -969,8 +963,8 @@ void BufferStateLayer::bufferMayChange(const sp<GraphicBuffer>& newBuffer) { // then we will drop a buffer and should decrement the pending buffer count and // call any release buffer callbacks if set. callReleaseBufferCallback(mDrawingState.releaseBufferListener, - mDrawingState.buffer->getBuffer(), mDrawingState.acquireFence, - mTransformHint, + mDrawingState.buffer->getBuffer(), mDrawingState.frameNumber, + mDrawingState.acquireFence, mTransformHint, mFlinger->getMaxAcquiredBufferCountForCurrentRefreshRate( mOwnerUid)); decrementPendingBufferCount(); diff --git a/services/surfaceflinger/BufferStateLayer.h b/services/surfaceflinger/BufferStateLayer.h index 27470182bf..e5674785af 100644 --- a/services/surfaceflinger/BufferStateLayer.h +++ b/services/surfaceflinger/BufferStateLayer.h @@ -143,15 +143,8 @@ private: bool bufferNeedsFiltering() const override; - static const std::array<float, 16> IDENTITY_MATRIX; - - std::unique_ptr<renderengine::Image> mTextureImage; - - mutable uint64_t mFrameNumber{0}; - uint64_t mFrameCounter{0}; - sp<Fence> mPreviousReleaseFence; - uint64_t mPreviousBufferId = 0; + ReleaseCallbackId mPreviousReleaseCallbackId = ReleaseCallbackId::INVALID_ID; uint64_t mPreviousReleasedFrameNumber = 0; bool mReleasePreviousBuffer = false; diff --git a/services/surfaceflinger/TransactionCallbackInvoker.cpp b/services/surfaceflinger/TransactionCallbackInvoker.cpp index fdf16a797f..6af69f0ef2 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.cpp +++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp @@ -237,7 +237,8 @@ status_t TransactionCallbackInvoker::addCallbackHandle(const sp<CallbackHandle>& handle->previousReleaseFence, handle->transformHint, handle->currentMaxAcquiredBufferCount, - eventStats, jankData, handle->previousBufferId); + eventStats, jankData, + handle->previousReleaseCallbackId); } return NO_ERROR; } diff --git a/services/surfaceflinger/TransactionCallbackInvoker.h b/services/surfaceflinger/TransactionCallbackInvoker.h index 444bec646e..6f4d812ec5 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.h +++ b/services/surfaceflinger/TransactionCallbackInvoker.h @@ -51,7 +51,7 @@ public: nsecs_t refreshStartTime = 0; nsecs_t dequeueReadyTime = 0; uint64_t frameNumber = 0; - uint64_t previousBufferId = 0; + ReleaseCallbackId previousReleaseCallbackId = ReleaseCallbackId::INVALID_ID; }; class TransactionCallbackInvoker { diff --git a/services/surfaceflinger/tests/ReleaseBufferCallback_test.cpp b/services/surfaceflinger/tests/ReleaseBufferCallback_test.cpp index 5aa809dc8b..579a26ebf4 100644 --- a/services/surfaceflinger/tests/ReleaseBufferCallback_test.cpp +++ b/services/surfaceflinger/tests/ReleaseBufferCallback_test.cpp @@ -29,7 +29,7 @@ using android::hardware::graphics::common::V1_1::BufferUsage; // b/181132765 - disabled until cuttlefish failures are investigated class ReleaseBufferCallbackHelper { public: - static void function(void* callbackContext, uint64_t graphicsBufferId, + static void function(void* callbackContext, ReleaseCallbackId callbackId, const sp<Fence>& releaseFence, uint32_t /*currentMaxAcquiredBufferCount*/) { if (!callbackContext) { @@ -38,11 +38,11 @@ public: ReleaseBufferCallbackHelper* helper = static_cast<ReleaseBufferCallbackHelper*>(callbackContext); std::lock_guard lock(helper->mMutex); - helper->mCallbackDataQueue.emplace(graphicsBufferId, releaseFence); + helper->mCallbackDataQueue.emplace(callbackId, releaseFence); helper->mConditionVariable.notify_all(); } - void getCallbackData(uint64_t* bufferId) { + void getCallbackData(ReleaseCallbackId* callbackId) { std::unique_lock lock(mMutex); if (mCallbackDataQueue.empty()) { if (!mConditionVariable.wait_for(lock, std::chrono::seconds(3), @@ -53,7 +53,7 @@ public: auto callbackData = mCallbackDataQueue.front(); mCallbackDataQueue.pop(); - *bufferId = callbackData.first; + *callbackId = callbackData.first; } void verifyNoCallbacks() { @@ -72,7 +72,7 @@ public: std::mutex mMutex; std::condition_variable mConditionVariable; - std::queue<std::pair<uint64_t, sp<Fence>>> mCallbackDataQueue; + std::queue<std::pair<ReleaseCallbackId, sp<Fence>>> mCallbackDataQueue; }; class ReleaseBufferCallbackTest : public LayerTransactionTest { @@ -82,10 +82,11 @@ public: } static void submitBuffer(const sp<SurfaceControl>& layer, sp<GraphicBuffer> buffer, - sp<Fence> fence, CallbackHelper& callback, + sp<Fence> fence, CallbackHelper& callback, const ReleaseCallbackId& id, ReleaseBufferCallbackHelper& releaseCallback) { Transaction t; - t.setBuffer(layer, buffer, releaseCallback.getCallback()); + t.setFrameNumber(layer, id.framenumber); + t.setBuffer(layer, buffer, id, releaseCallback.getCallback()); t.setAcquireFence(layer, fence); t.addTransactionCompletedCallback(callback.function, callback.getContext()); t.apply(); @@ -98,10 +99,10 @@ public: } static void waitForReleaseBufferCallback(ReleaseBufferCallbackHelper& releaseCallback, - uint64_t expectedReleaseBufferId) { - uint64_t actualReleaseBufferId; + const ReleaseCallbackId& expectedCallbackId) { + ReleaseCallbackId actualReleaseBufferId; releaseCallback.getCallbackData(&actualReleaseBufferId); - EXPECT_EQ(expectedReleaseBufferId, actualReleaseBufferId); + EXPECT_EQ(expectedCallbackId, actualReleaseBufferId); releaseCallback.verifyNoCallbacks(); } static ReleaseBufferCallbackHelper* getReleaseBufferCallbackHelper() { @@ -116,6 +117,10 @@ public: BufferUsage::COMPOSER_OVERLAY, "test"); } + static uint64_t generateFrameNumber() { + static uint64_t sFrameNumber = 0; + return ++sFrameNumber; + } }; TEST_F(ReleaseBufferCallbackTest, DISABLED_PresentBuffer) { @@ -125,7 +130,9 @@ TEST_F(ReleaseBufferCallbackTest, DISABLED_PresentBuffer) { // If a buffer is being presented, we should not emit a release callback. sp<GraphicBuffer> firstBuffer = getBuffer(); - submitBuffer(layer, firstBuffer, Fence::NO_FENCE, transactionCallback, *releaseCallback); + ReleaseCallbackId firstBufferCallbackId(firstBuffer->getId(), generateFrameNumber()); + submitBuffer(layer, firstBuffer, Fence::NO_FENCE, transactionCallback, firstBufferCallbackId, + *releaseCallback); ExpectedResult expected; expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer, ExpectedResult::Buffer::NOT_ACQUIRED); @@ -143,13 +150,15 @@ TEST_F(ReleaseBufferCallbackTest, DISABLED_PresentBuffer) { // If a presented buffer is replaced, we should emit a release callback for the // previously presented buffer. sp<GraphicBuffer> secondBuffer = getBuffer(); - submitBuffer(layer, secondBuffer, Fence::NO_FENCE, transactionCallback, *releaseCallback); + ReleaseCallbackId secondBufferCallbackId(secondBuffer->getId(), generateFrameNumber()); + submitBuffer(layer, secondBuffer, Fence::NO_FENCE, transactionCallback, secondBufferCallbackId, + *releaseCallback); expected = ExpectedResult(); expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer, ExpectedResult::Buffer::NOT_ACQUIRED, ExpectedResult::PreviousBuffer::RELEASED); ASSERT_NO_FATAL_FAILURE(waitForCallback(transactionCallback, expected)); - ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBuffer->getId())); + ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBufferCallbackId)); } TEST_F(ReleaseBufferCallbackTest, DISABLED_OffScreenLayer) { @@ -160,7 +169,9 @@ TEST_F(ReleaseBufferCallbackTest, DISABLED_OffScreenLayer) { // If a buffer is being presented, we should not emit a release callback. sp<GraphicBuffer> firstBuffer = getBuffer(); - submitBuffer(layer, firstBuffer, Fence::NO_FENCE, transactionCallback, *releaseCallback); + ReleaseCallbackId firstBufferCallbackId(firstBuffer->getId(), generateFrameNumber()); + submitBuffer(layer, firstBuffer, Fence::NO_FENCE, transactionCallback, firstBufferCallbackId, + *releaseCallback); ExpectedResult expected; expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer, ExpectedResult::Buffer::NOT_ACQUIRED); @@ -184,23 +195,27 @@ TEST_F(ReleaseBufferCallbackTest, DISABLED_OffScreenLayer) { // If a presented buffer is replaced, we should emit a release callback for the // previously presented buffer. sp<GraphicBuffer> secondBuffer = getBuffer(); - submitBuffer(layer, secondBuffer, Fence::NO_FENCE, transactionCallback, *releaseCallback); + ReleaseCallbackId secondBufferCallbackId(secondBuffer->getId(), generateFrameNumber()); + submitBuffer(layer, secondBuffer, Fence::NO_FENCE, transactionCallback, secondBufferCallbackId, + *releaseCallback); expected = ExpectedResult(); expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer, ExpectedResult::Buffer::NOT_ACQUIRED, ExpectedResult::PreviousBuffer::NOT_RELEASED); ASSERT_NO_FATAL_FAILURE(waitForCallback(transactionCallback, expected)); - ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBuffer->getId())); + ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBufferCallbackId)); // If continue to submit buffer we continue to get release callbacks sp<GraphicBuffer> thirdBuffer = getBuffer(); - submitBuffer(layer, thirdBuffer, Fence::NO_FENCE, transactionCallback, *releaseCallback); + ReleaseCallbackId thirdBufferCallbackId(secondBuffer->getId(), generateFrameNumber()); + submitBuffer(layer, thirdBuffer, Fence::NO_FENCE, transactionCallback, thirdBufferCallbackId, + *releaseCallback); expected = ExpectedResult(); expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer, ExpectedResult::Buffer::NOT_ACQUIRED, ExpectedResult::PreviousBuffer::NOT_RELEASED); ASSERT_NO_FATAL_FAILURE(waitForCallback(transactionCallback, expected)); - ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, secondBuffer->getId())); + ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, secondBufferCallbackId)); } TEST_F(ReleaseBufferCallbackTest, DISABLED_LayerLifecycle_layerdestroy) { @@ -210,7 +225,9 @@ TEST_F(ReleaseBufferCallbackTest, DISABLED_LayerLifecycle_layerdestroy) { // If a buffer is being presented, we should not emit a release callback. sp<GraphicBuffer> firstBuffer = getBuffer(); - submitBuffer(layer, firstBuffer, Fence::NO_FENCE, *transactionCallback, *releaseCallback); + ReleaseCallbackId firstBufferCallbackId(firstBuffer->getId(), generateFrameNumber()); + submitBuffer(layer, firstBuffer, Fence::NO_FENCE, *transactionCallback, firstBufferCallbackId, + *releaseCallback); { ExpectedResult expected; expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer, @@ -225,7 +242,7 @@ TEST_F(ReleaseBufferCallbackTest, DISABLED_LayerLifecycle_layerdestroy) { t.apply(); layer = nullptr; - ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBuffer->getId())); + ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBufferCallbackId)); } // Destroying a never presented layer emits a callback. @@ -242,7 +259,9 @@ TEST_F(ReleaseBufferCallbackTest, DISABLED_LayerLifecycle_OffScreenLayerDestroy) // Submitting a buffer does not emit a callback. sp<GraphicBuffer> firstBuffer = getBuffer(); - submitBuffer(layer, firstBuffer, Fence::NO_FENCE, *transactionCallback, *releaseCallback); + ReleaseCallbackId firstBufferCallbackId(firstBuffer->getId(), generateFrameNumber()); + submitBuffer(layer, firstBuffer, Fence::NO_FENCE, *transactionCallback, firstBufferCallbackId, + *releaseCallback); { ExpectedResult expected; expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer, @@ -253,19 +272,21 @@ TEST_F(ReleaseBufferCallbackTest, DISABLED_LayerLifecycle_OffScreenLayerDestroy) // Submitting a second buffer will replace the drawing state buffer and emit a callback. sp<GraphicBuffer> secondBuffer = getBuffer(); - submitBuffer(layer, secondBuffer, Fence::NO_FENCE, *transactionCallback, *releaseCallback); + ReleaseCallbackId secondBufferCallbackId(secondBuffer->getId(), generateFrameNumber()); + submitBuffer(layer, secondBuffer, Fence::NO_FENCE, *transactionCallback, secondBufferCallbackId, + *releaseCallback); { ExpectedResult expected; expected.addSurface(ExpectedResult::Transaction::PRESENTED, layer, ExpectedResult::Buffer::NOT_ACQUIRED); ASSERT_NO_FATAL_FAILURE(waitForCallback(*transactionCallback, expected)); ASSERT_NO_FATAL_FAILURE( - waitForReleaseBufferCallback(*releaseCallback, firstBuffer->getId())); + waitForReleaseBufferCallback(*releaseCallback, firstBufferCallbackId)); } // Destroying the offscreen layer emits a callback. layer = nullptr; - ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, secondBuffer->getId())); + ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, secondBufferCallbackId)); } TEST_F(ReleaseBufferCallbackTest, DISABLED_FrameDropping) { @@ -275,12 +296,13 @@ TEST_F(ReleaseBufferCallbackTest, DISABLED_FrameDropping) { // If a buffer is being presented, we should not emit a release callback. sp<GraphicBuffer> firstBuffer = getBuffer(); + ReleaseCallbackId firstBufferCallbackId(firstBuffer->getId(), generateFrameNumber()); // Try to present 100ms in the future nsecs_t time = systemTime() + std::chrono::nanoseconds(100ms).count(); Transaction t; - t.setBuffer(layer, firstBuffer, releaseCallback->getCallback()); + t.setBuffer(layer, firstBuffer, firstBufferCallbackId, releaseCallback->getCallback()); t.setAcquireFence(layer, Fence::NO_FENCE); t.addTransactionCompletedCallback(transactionCallback.function, transactionCallback.getContext()); @@ -295,7 +317,8 @@ TEST_F(ReleaseBufferCallbackTest, DISABLED_FrameDropping) { // Dropping frames in transaction queue emits a callback sp<GraphicBuffer> secondBuffer = getBuffer(); - t.setBuffer(layer, secondBuffer, releaseCallback->getCallback()); + ReleaseCallbackId secondBufferCallbackId(secondBuffer->getId(), generateFrameNumber()); + t.setBuffer(layer, secondBuffer, secondBufferCallbackId, releaseCallback->getCallback()); t.setAcquireFence(layer, Fence::NO_FENCE); t.addTransactionCompletedCallback(transactionCallback.function, transactionCallback.getContext()); @@ -307,7 +330,7 @@ TEST_F(ReleaseBufferCallbackTest, DISABLED_FrameDropping) { ExpectedResult::Buffer::NOT_ACQUIRED, ExpectedResult::PreviousBuffer::RELEASED); ASSERT_NO_FATAL_FAILURE(waitForCallback(transactionCallback, expected)); - ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBuffer->getId())); + ASSERT_NO_FATAL_FAILURE(waitForReleaseBufferCallback(*releaseCallback, firstBufferCallbackId)); } } // namespace android |