diff --git a/libs/gui/BufferQueueConsumer.cpp b/libs/gui/BufferQueueConsumer.cpp index 11f5174d76..910ede1f15 100644 --- a/libs/gui/BufferQueueConsumer.cpp +++ b/libs/gui/BufferQueueConsumer.cpp @@ -317,44 +317,35 @@ status_t BufferQueueConsumer::detachBuffer(int slot) { ATRACE_CALL(); ATRACE_BUFFER_INDEX(slot); BQ_LOGV("detachBuffer: slot %d", slot); - sp listener; - { - std::lock_guard lock(mCore->mMutex); - - if (mCore->mIsAbandoned) { - BQ_LOGE("detachBuffer: BufferQueue has been abandoned"); - return NO_INIT; - } - - if (mCore->mSharedBufferMode || slot == mCore->mSharedBufferSlot) { - BQ_LOGE("detachBuffer: detachBuffer not allowed in shared buffer mode"); - return BAD_VALUE; - } + std::lock_guard lock(mCore->mMutex); - if (slot < 0 || slot >= BufferQueueDefs::NUM_BUFFER_SLOTS) { - BQ_LOGE("detachBuffer: slot index %d out of range [0, %d)", - slot, BufferQueueDefs::NUM_BUFFER_SLOTS); - return BAD_VALUE; - } else if (!mSlots[slot].mBufferState.isAcquired()) { - BQ_LOGE("detachBuffer: slot %d is not owned by the consumer " - "(state = %s)", slot, mSlots[slot].mBufferState.string()); - return BAD_VALUE; - } - if (mCore->mBufferReleasedCbEnabled) { - listener = mCore->mConnectedProducerListener; - } + if (mCore->mIsAbandoned) { + BQ_LOGE("detachBuffer: BufferQueue has been abandoned"); + return NO_INIT; + } - mSlots[slot].mBufferState.detachConsumer(); - mCore->mActiveBuffers.erase(slot); - mCore->mFreeSlots.insert(slot); - mCore->clearBufferSlotLocked(slot); - mCore->mDequeueCondition.notify_all(); - VALIDATE_CONSISTENCY(); + if (mCore->mSharedBufferMode || slot == mCore->mSharedBufferSlot) { + BQ_LOGE("detachBuffer: detachBuffer not allowed in shared buffer mode"); + return BAD_VALUE; } - if (listener) { - listener->onBufferDetached(slot); + if (slot < 0 || slot >= BufferQueueDefs::NUM_BUFFER_SLOTS) { + BQ_LOGE("detachBuffer: slot index %d out of range [0, %d)", + slot, BufferQueueDefs::NUM_BUFFER_SLOTS); + return BAD_VALUE; + } else if (!mSlots[slot].mBufferState.isAcquired()) { + BQ_LOGE("detachBuffer: slot %d is not owned by the consumer " + "(state = %s)", slot, mSlots[slot].mBufferState.string()); + return BAD_VALUE; } + + mSlots[slot].mBufferState.detachConsumer(); + mCore->mActiveBuffers.erase(slot); + mCore->mFreeSlots.insert(slot); + mCore->clearBufferSlotLocked(slot); + mCore->mDequeueCondition.notify_all(); + VALIDATE_CONSISTENCY(); + return NO_ERROR; } diff --git a/libs/gui/IProducerListener.cpp b/libs/gui/IProducerListener.cpp index 0683087211..72cec7afad 100644 --- a/libs/gui/IProducerListener.cpp +++ b/libs/gui/IProducerListener.cpp @@ -25,6 +25,7 @@ enum { ON_BUFFER_RELEASED = IBinder::FIRST_CALL_TRANSACTION, NEEDS_RELEASE_NOTIFY, ON_BUFFERS_DISCARDED, + ON_BUFFER_DETACHED, }; class BpProducerListener : public BpInterface @@ -64,6 +65,13 @@ class BpProducerListener : public BpInterface data.writeInt32Vector(discardedSlots); remote()->transact(ON_BUFFERS_DISCARDED, data, &reply, IBinder::FLAG_ONEWAY); } + + virtual void onBufferDetached(int slot) { + Parcel data, reply; + data.writeInterfaceToken(IProducerListener::getInterfaceDescriptor()); + data.writeInt32(slot); + remote()->transact(ON_BUFFER_DETACHED, data, &reply, IBinder::FLAG_ONEWAY); + } }; // Out-of-line virtual method definition to trigger vtable emission in this @@ -88,6 +96,10 @@ class HpProducerListener : public HpInterface< virtual void onBuffersDiscarded(const std::vector& discardedSlots) override { return mBase->onBuffersDiscarded(discardedSlots); } + + virtual void onBufferDetached(int slot) { + mBase->onBufferDetached(slot); + } }; IMPLEMENT_HYBRID_META_INTERFACE(ProducerListener, @@ -115,6 +127,12 @@ status_t BnProducerListener::onTransact(uint32_t code, const Parcel& data, onBuffersDiscarded(discardedSlots); return NO_ERROR; } + case ON_BUFFER_DETACHED: + int slot = 0; + CHECK_INTERFACE(IProducerListener, data, reply); + data.readInt32(&slot); + onBufferDetached(slot); + return NO_ERROR; } return BBinder::onTransact(code, data, reply, flags); } @@ -128,4 +146,7 @@ bool BnProducerListener::needsReleaseNotify() { void BnProducerListener::onBuffersDiscarded(const std::vector& /*discardedSlots*/) { } +void BnProducerListener::onBufferDetached(int slot) { + ALOGE("BnProducerListener::onBufferDetached slot: %d",slot); +} } // namespace android diff --git a/libs/gui/bufferqueue/1.0/WProducerListener.cpp b/libs/gui/bufferqueue/1.0/WProducerListener.cpp index 56b64b9ddd..7a3e050561 100644 --- a/libs/gui/bufferqueue/1.0/WProducerListener.cpp +++ b/libs/gui/bufferqueue/1.0/WProducerListener.cpp @@ -49,4 +49,7 @@ bool LWProducerListener::needsReleaseNotify() { void LWProducerListener::onBuffersDiscarded(const std::vector& /*slots*/) { } +void LWProducerListener::onBufferDetached(int /*slot*/) { +} + } // namespace android diff --git a/libs/gui/bufferqueue/2.0/H2BProducerListener.cpp b/libs/gui/bufferqueue/2.0/H2BProducerListener.cpp index b2bd1172d6..745ffea75f 100644 --- a/libs/gui/bufferqueue/2.0/H2BProducerListener.cpp +++ b/libs/gui/bufferqueue/2.0/H2BProducerListener.cpp @@ -55,6 +55,9 @@ bool H2BProducerListener::needsReleaseNotify() { void H2BProducerListener::onBuffersDiscarded(const std::vector& /*slots*/) { } +void H2BProducerListener::onBufferDetached(int /*slot*/) { +} + } // namespace utils } // namespace V2_0 } // namespace bufferqueue diff --git a/libs/gui/include/gui/IProducerListener.h b/libs/gui/include/gui/IProducerListener.h index b15f501518..ee19cfd823 100644 --- a/libs/gui/include/gui/IProducerListener.h +++ b/libs/gui/include/gui/IProducerListener.h @@ -49,12 +49,8 @@ class ProducerListener : public virtual RefBase // onBuffersFreed is called from IGraphicBufferConsumer::discardFreeBuffers // to notify the producer that certain free buffers are discarded by the consumer. virtual void onBuffersDiscarded(const std::vector& slots) = 0; // Asynchronous - // onBufferDetached is called from IGraphicBufferConsumer::detachBuffer to - // notify the producer that a buffer slot is free and ready to be dequeued. - // - // This is called without any lock held and can be called concurrently by - // multiple threads. - virtual void onBufferDetached(int /*slot*/) {} // Asynchronous + + virtual void onBufferDetached(int slot) = 0; }; #ifndef NO_BINDER @@ -78,6 +74,7 @@ class BnProducerListener : public BnInterface Parcel* reply, uint32_t flags = 0); virtual bool needsReleaseNotify(); virtual void onBuffersDiscarded(const std::vector& slots); + virtual void onBufferDetached(int slot); }; #else @@ -91,6 +88,7 @@ class StubProducerListener : public BnProducerListener { virtual ~StubProducerListener(); virtual void onBufferReleased() {} virtual bool needsReleaseNotify() { return false; } + virtual void onBufferDetached(int /**slot**/) {} }; } // namespace android diff --git a/libs/gui/include/gui/Surface.h b/libs/gui/include/gui/Surface.h index 39a59e42aa..0c7ca70f47 100644 --- a/libs/gui/include/gui/Surface.h +++ b/libs/gui/include/gui/Surface.h @@ -56,6 +56,11 @@ class SurfaceListener : public virtual RefBase virtual bool needsReleaseNotify() = 0; virtual void onBuffersDiscarded(const std::vector>& buffers) = 0; + + virtual void onBufferDetached(int /**slot**/) { + //default do nothing + } + }; /* @@ -412,6 +417,11 @@ class Surface } virtual void onBuffersDiscarded(const std::vector& slots); + + virtual void onBufferDetached(int slot) { + mSurfaceListener->onBufferDetached(slot); + } + private: wp mParent; sp mSurfaceListener; diff --git a/libs/gui/include/gui/bufferqueue/1.0/WProducerListener.h b/libs/gui/include/gui/bufferqueue/1.0/WProducerListener.h index 197db26444..efdd5aad5a 100644 --- a/libs/gui/include/gui/bufferqueue/1.0/WProducerListener.h +++ b/libs/gui/include/gui/bufferqueue/1.0/WProducerListener.h @@ -55,6 +55,7 @@ class LWProducerListener : public BnProducerListener { void onBufferReleased() override; bool needsReleaseNotify() override; void onBuffersDiscarded(const std::vector& slots) override; + void onBufferDetached(int slot) override; }; } // namespace android diff --git a/libs/gui/include/gui/bufferqueue/2.0/H2BProducerListener.h b/libs/gui/include/gui/bufferqueue/2.0/H2BProducerListener.h index 92650b701b..d66c025fe3 100644 --- a/libs/gui/include/gui/bufferqueue/2.0/H2BProducerListener.h +++ b/libs/gui/include/gui/bufferqueue/2.0/H2BProducerListener.h @@ -47,6 +47,7 @@ class H2BProducerListener virtual void onBufferReleased() override; virtual bool needsReleaseNotify() override; virtual void onBuffersDiscarded(const std::vector& slots) override; + virtual void onBufferDetached(int slot) override; }; } // namespace utils diff --git a/libs/gui/tests/BufferQueue_test.cpp b/libs/gui/tests/BufferQueue_test.cpp index df7739c3fb..1410c7dce0 100644 --- a/libs/gui/tests/BufferQueue_test.cpp +++ b/libs/gui/tests/BufferQueue_test.cpp @@ -1187,76 +1187,6 @@ TEST_F(BufferQueueTest, TestBufferReplacedInQueueBuffer) { ASSERT_EQ(true, output.bufferReplaced); } -struct BufferDetachedListener : public BnProducerListener { -public: - BufferDetachedListener() = default; - virtual ~BufferDetachedListener() = default; - - virtual void onBufferReleased() {} - virtual bool needsReleaseNotify() { return true; } - virtual void onBufferDetached(int slot) { - mDetachedSlots.push_back(slot); - } - const std::vector& getDetachedSlots() const { return mDetachedSlots; } -private: - std::vector mDetachedSlots; -}; - -TEST_F(BufferQueueTest, TestConsumerDetachProducerListener) { - createBufferQueue(); - sp mc(new MockConsumer); - ASSERT_EQ(OK, mConsumer->consumerConnect(mc, true)); - IGraphicBufferProducer::QueueBufferOutput output; - sp pl(new BufferDetachedListener); - ASSERT_EQ(OK, mProducer->connect(pl, NATIVE_WINDOW_API_CPU, true, &output)); - ASSERT_EQ(OK, mProducer->setDequeueTimeout(0)); - ASSERT_EQ(OK, mConsumer->setMaxAcquiredBufferCount(1)); - - sp fence = Fence::NO_FENCE; - sp buffer = nullptr; - IGraphicBufferProducer::QueueBufferInput input(0ull, true, - HAL_DATASPACE_UNKNOWN, Rect::INVALID_RECT, - NATIVE_WINDOW_SCALING_MODE_FREEZE, 0, Fence::NO_FENCE); - - int slots[2] = {}; - status_t result = OK; - ASSERT_EQ(OK, mProducer->setMaxDequeuedBufferCount(2)); - - result = mProducer->dequeueBuffer(&slots[0], &fence, 0, 0, 0, - GRALLOC_USAGE_SW_READ_RARELY, nullptr, nullptr); - ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION, result); - ASSERT_EQ(OK, mProducer->requestBuffer(slots[0], &buffer)); - - result = mProducer->dequeueBuffer(&slots[1], &fence, 0, 0, 0, - GRALLOC_USAGE_SW_READ_RARELY, nullptr, nullptr); - ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION, result); - ASSERT_EQ(OK, mProducer->requestBuffer(slots[1], &buffer)); - - // Queue & detach one from two dequeued buffes. - ASSERT_EQ(OK, mProducer->queueBuffer(slots[1], input, &output)); - BufferItem item{}; - ASSERT_EQ(OK, mConsumer->acquireBuffer(&item, 0)); - ASSERT_EQ(OK, mConsumer->detachBuffer(item.mSlot)); - - // Check whether the slot from IProducerListener is same to the detached slot. - ASSERT_EQ(pl->getDetachedSlots().size(), 1); - ASSERT_EQ(pl->getDetachedSlots()[0], slots[1]); - - // Dequeue another buffer. - int slot; - result = mProducer->dequeueBuffer(&slot, &fence, 0, 0, 0, - GRALLOC_USAGE_SW_READ_RARELY, nullptr, nullptr); - ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION, result); - ASSERT_EQ(OK, mProducer->requestBuffer(slot, &buffer)); - - // Dequeue should fail here, since we dequeued 3 buffers and one buffer was - // detached from consumer(Two buffers are dequeued, and the current max - // dequeued buffer count is two). - result = mProducer->dequeueBuffer(&slot, &fence, 0, 0, 0, - GRALLOC_USAGE_SW_READ_RARELY, nullptr, nullptr); - ASSERT_TRUE(result == WOULD_BLOCK || result == TIMED_OUT || result == INVALID_OPERATION); -} - TEST_F(BufferQueueTest, TestStaleBufferHandleSentAfterDisconnect) { createBufferQueue(); sp mc(new MockConsumer);