C2BufferQueueBlockPoolData: remove C2BufferQueueBlockPool::Impl field
Originally, C2BufferQueueBlockPoolData has
C2BufferQueueBlockPool::Impl field for calling IGBP::cancelBuffer() at
destructor if the buffer is still held. However,
C2BufferQueueBlockPoolData already has a IGBP field. This CL removes
the unnecessary C2BufferQueueBlockPool::Impl field.
Bug: 174188958
Test: android.media.cts.MediaCodecPlayerTest#testPlaybackSwitchViews
Change-Id: I6baf5c4a1b9d1c27f95d18817628cd6ffcd35478
diff --git a/media/codec2/vndk/include/C2BqBufferPriv.h b/media/codec2/vndk/include/C2BqBufferPriv.h
index 5ff4128..57f4011 100644
--- a/media/codec2/vndk/include/C2BqBufferPriv.h
+++ b/media/codec2/vndk/include/C2BqBufferPriv.h
@@ -97,7 +97,7 @@
// Create a local BlockPoolData.
C2BufferQueueBlockPoolData(
uint32_t generation, uint64_t bqId, int32_t bqSlot,
- const std::shared_ptr<C2BufferQueueBlockPool::Impl>& pool);
+ const android::sp<HGraphicBufferProducer>& producer);
virtual ~C2BufferQueueBlockPoolData() override;
@@ -124,15 +124,23 @@
const bool mLocal;
bool mHeld;
+
+ // Data of the corresponding buffer.
uint32_t mGeneration;
uint64_t mBqId;
int32_t mBqSlot;
+
+ // Data of the current IGBP, updated at migrate(). If the values are
+ // mismatched, then the corresponding buffer will not be cancelled back to
+ // IGBP at the destructor.
+ uint32_t mCurrentGeneration;
+ uint64_t mCurrentBqId;
+
bool mTransfer; // local transfer to remote
bool mAttach; // attach on remote
bool mDisplay; // display on remote;
std::weak_ptr<int> mOwner;
android::sp<HGraphicBufferProducer> mIgbp;
- std::shared_ptr<C2BufferQueueBlockPool::Impl> mLocalPool;
mutable std::mutex mLock;
};
diff --git a/media/codec2/vndk/platform/C2BqBuffer.cpp b/media/codec2/vndk/platform/C2BqBuffer.cpp
index 51eea7a..1267a4a 100644
--- a/media/codec2/vndk/platform/C2BqBuffer.cpp
+++ b/media/codec2/vndk/platform/C2BqBuffer.cpp
@@ -369,7 +369,7 @@
std::make_shared<C2BufferQueueBlockPoolData>(
slotBuffer->getGenerationNumber(),
mProducerId, slot,
- shared_from_this());
+ mProducer);
mPoolDatas[slot] = poolData;
*block = _C2BlockFactory::CreateGraphicBlock(alloc, poolData);
return C2_OK;
@@ -437,7 +437,7 @@
}
std::shared_ptr<C2BufferQueueBlockPoolData> poolData =
std::make_shared<C2BufferQueueBlockPoolData>(
- 0, (uint64_t)0, ~0, shared_from_this());
+ 0, (uint64_t)0, ~0, nullptr);
*block = _C2BlockFactory::CreateGraphicBlock(alloc, poolData);
ALOGV("allocated a buffer successfully");
@@ -534,17 +534,6 @@
private:
friend struct C2BufferQueueBlockPoolData;
- void cancel(uint32_t generation, uint64_t igbp_id, int32_t igbp_slot) {
- bool cancelled = false;
- {
- std::scoped_lock<std::mutex> lock(mMutex);
- if (generation == mGeneration && igbp_id == mProducerId && mProducer) {
- (void)mProducer->cancelBuffer(igbp_slot, hidl_handle{}).isOk();
- cancelled = true;
- }
- }
- }
-
c2_status_t mInit;
uint64_t mProducerId;
uint32_t mGeneration;
@@ -570,30 +559,30 @@
const sp<HGraphicBufferProducer>& producer) :
mLocal(false), mHeld(producer && bqId != 0),
mGeneration(generation), mBqId(bqId), mBqSlot(bqSlot),
+ mCurrentGeneration(generation), mCurrentBqId(bqId),
mTransfer(false), mAttach(false), mDisplay(false),
- mOwner(owner), mIgbp(producer),
- mLocalPool() {
+ mOwner(owner), mIgbp(producer) {
}
C2BufferQueueBlockPoolData::C2BufferQueueBlockPoolData(
uint32_t generation, uint64_t bqId, int32_t bqSlot,
- const std::shared_ptr<C2BufferQueueBlockPool::Impl>& pool) :
+ const android::sp<HGraphicBufferProducer>& producer) :
mLocal(true), mHeld(true),
mGeneration(generation), mBqId(bqId), mBqSlot(bqSlot),
- mTransfer(false), mAttach(false), mDisplay(false),
- mIgbp(pool ? pool->mProducer : nullptr),
- mLocalPool(pool) {
+ mCurrentGeneration(generation), mCurrentBqId(bqId),
+ mTransfer(false), mAttach(false), mDisplay(false), mIgbp(producer) {
}
C2BufferQueueBlockPoolData::~C2BufferQueueBlockPoolData() {
- if (!mHeld || mBqId == 0) {
+ if (!mHeld || mBqId == 0 || !mIgbp) {
return;
}
+
if (mLocal) {
- if (mLocalPool) {
- mLocalPool->cancel(mGeneration, mBqId, mBqSlot);
+ if (mGeneration == mCurrentGeneration && mBqId == mCurrentBqId) {
+ mIgbp->cancelBuffer(mBqSlot, hidl_handle{}).isOk();
}
- } else if (mIgbp && !mOwner.expired()) {
+ } else if (!mOwner.expired()) {
mIgbp->cancelBuffer(mBqSlot, hidl_handle{}).isOk();
}
}
@@ -607,11 +596,15 @@
uint32_t toGeneration, uint64_t toBqId,
sp<GraphicBuffer> *buffers, uint32_t oldGeneration) {
std::scoped_lock<std::mutex> l(mLock);
+
+ mCurrentBqId = toBqId;
+ mCurrentGeneration = toGeneration;
+
if (!mHeld || mBqId == 0) {
ALOGV("buffer is not owned");
return -1;
}
- if (!mLocal || !mLocalPool) {
+ if (!mLocal) {
ALOGV("pool is not local");
return -1;
}
@@ -661,6 +654,7 @@
}
ALOGV("local migration from gen %u : %u slot %d : %d",
mGeneration, toGeneration, mBqSlot, slot);
+ mIgbp = producer;
mGeneration = toGeneration;
mBqId = toBqId;
mBqSlot = slot;