Merge "Blast: Use a unique id to track buffers" into sc-dev am: 848879ae6c
Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/native/+/15096591
Change-Id: I44b45d02d2b1b26145ce9c801b0458e5eee22dbb
diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp
index e15e11c..2970116 100644
--- a/libs/gui/BLASTBufferQueue.cpp
+++ b/libs/gui/BLASTBufferQueue.cpp
@@ -132,8 +132,7 @@
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 @@
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 @@
// 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 @@
// 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 @@
}
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 @@
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 @@
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 @@
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 17499ec..98e8b54 100644
--- a/libs/gui/ITransactionCompletedListener.cpp
+++ b/libs/gui/ITransactionCompletedListener.cpp
@@ -125,7 +125,7 @@
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 @@
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 @@
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 @@
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 38d6fef..c255303 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -197,15 +197,16 @@
}
}
-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 @@
// 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::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& 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 @@
if (mIsAutoTimestamp) {
mDesiredPresentTime = systemTime();
}
- setReleaseBufferCallback(s, callback);
+ setReleaseBufferCallback(s, id, callback);
registerSurfaceControlForCallback(sc);
@@ -1286,10 +1289,13 @@
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 @@
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 26c7285..0d6a673 100644
--- a/libs/gui/include/gui/BLASTBufferQueue.h
+++ b/libs/gui/include/gui/BLASTBufferQueue.h
@@ -89,7 +89,7 @@
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 @@
// 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 d286c34..937095c 100644
--- a/libs/gui/include/gui/ITransactionCompletedListener.h
+++ b/libs/gui/include/gui/ITransactionCompletedListener.h
@@ -53,6 +53,36 @@
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 @@
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 @@
currentMaxAcquiredBufferCount(currentMaxAcquiredBuffersCount),
eventStats(frameEventStats),
jankData(std::move(jankData)),
- previousBufferId(previousBufferId) {}
+ previousReleaseCallbackId(previousReleaseCallbackId) {}
sp<IBinder> surfaceControl;
nsecs_t acquireTime = -1;
@@ -120,7 +150,7 @@
uint32_t currentMaxAcquiredBufferCount = 0;
FrameEventHistoryStats eventStats;
std::vector<JankData> jankData;
- uint64_t previousBufferId;
+ ReleaseCallbackId previousReleaseCallbackId;
};
class TransactionStats : public Parcelable {
@@ -161,7 +191,7 @@
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 869c082..7961f4b 100644
--- a/libs/gui/include/gui/LayerState.h
+++ b/libs/gui/include/gui/LayerState.h
@@ -234,6 +234,12 @@
// 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 e99bb7d..eb81483 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -82,7 +82,7 @@
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 @@
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 @@
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);
@@ -680,7 +682,7 @@
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
@@ -719,16 +721,16 @@
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 cd3d80e..760c8b9 100644
--- a/services/surfaceflinger/BufferLayer.h
+++ b/services/surfaceflinger/BufferLayer.h
@@ -133,6 +133,7 @@
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 c662bc2..2a49a0a 100644
--- a/services/surfaceflinger/BufferStateLayer.cpp
+++ b/services/surfaceflinger/BufferStateLayer.cpp
@@ -44,25 +44,18 @@
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 @@
// 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 @@
if (ch == nullptr) {
return OK;
}
- ch->previousBufferId = mPreviousBufferId;
+ ch->previousReleaseCallbackId = mPreviousReleaseCallbackId;
if (!ch->previousReleaseFence.get()) {
ch->previousReleaseFence = fence;
return OK;
@@ -214,7 +207,7 @@
// see BufferStateLayer::onLayerDisplayed.
for (auto& handle : mDrawingState.callbackHandles) {
if (handle->releasePreviousBuffer) {
- handle->previousBufferId = mPreviousBufferId;
+ handle->previousReleaseCallbackId = mPreviousReleaseCallbackId;
break;
}
}
@@ -438,8 +431,8 @@
// 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 @@
* 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 @@
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 @@
// 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 2747018..e567478 100644
--- a/services/surfaceflinger/BufferStateLayer.h
+++ b/services/surfaceflinger/BufferStateLayer.h
@@ -143,15 +143,8 @@
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 fdf16a7..6af69f0 100644
--- a/services/surfaceflinger/TransactionCallbackInvoker.cpp
+++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp
@@ -237,7 +237,8 @@
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 444bec6..6f4d812 100644
--- a/services/surfaceflinger/TransactionCallbackInvoker.h
+++ b/services/surfaceflinger/TransactionCallbackInvoker.h
@@ -51,7 +51,7 @@
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 5aa809d..579a26e 100644
--- a/services/surfaceflinger/tests/ReleaseBufferCallback_test.cpp
+++ b/services/surfaceflinger/tests/ReleaseBufferCallback_test.cpp
@@ -29,7 +29,7 @@
// 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 @@
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 @@
auto callbackData = mCallbackDataQueue.front();
mCallbackDataQueue.pop();
- *bufferId = callbackData.first;
+ *callbackId = callbackData.first;
}
void verifyNoCallbacks() {
@@ -72,7 +72,7 @@
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 @@
}
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 @@
}
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 @@
BufferUsage::COMPOSER_OVERLAY,
"test");
}
+ static uint64_t generateFrameNumber() {
+ static uint64_t sFrameNumber = 0;
+ return ++sFrameNumber;
+ }
};
TEST_F(ReleaseBufferCallbackTest, DISABLED_PresentBuffer) {
@@ -125,7 +130,9 @@
// 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 @@
// 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 @@
// 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 @@
// 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 @@
// 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 @@
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 @@
// 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 @@
// 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 @@
// 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 @@
// 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 @@
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