Merge "libgui: Fix attaching buffers without allocation" (927d64c2) · Commits · e / os / platform_frameworks_native

include/gui/BufferQueueProducer.h

+13 −1

Original line number	Diff line number	Diff line
		@@ -183,12 +183,24 @@ private:
		// This is required by the IBinder::DeathRecipient interface
		virtual void binderDied(const wp<IBinder>& who);

		// Returns the slot of the next free buffer if one is available or
		// BufferQueueCore::INVALID_BUFFER_SLOT otherwise
		int getFreeBufferLocked() const;

		// Returns the next free slot if one less than or equal to maxBufferCount
		// is available or BufferQueueCore::INVALID_BUFFER_SLOT otherwise
		int getFreeSlotLocked(int maxBufferCount) const;

		// waitForFreeSlotThenRelock finds the oldest slot in the FREE state. It may
		// block if there are no available slots and we are not in non-blocking
		// mode (producer and consumer controlled by the application). If it blocks,
		// it will release mCore->mMutex while blocked so that other operations on
		// the BufferQueue may succeed.
		status_t waitForFreeSlotThenRelock(const char* caller, int* found,
		enum class FreeSlotCaller {
		Dequeue,
		Attach,
		};
		status_t waitForFreeSlotThenRelock(FreeSlotCaller caller, int* found,
		status_t* returnFlags) const;

		sp<BufferQueueCore> mCore;

libs/gui/BufferQueueProducer.cpp

+47 −17

Original line number	Diff line number	Diff line
		@@ -184,12 +184,35 @@ status_t BufferQueueProducer::setAsyncMode(bool async) {
		return NO_ERROR;
		}

		status_t BufferQueueProducer::waitForFreeSlotThenRelock(const char* caller,
		int BufferQueueProducer::getFreeBufferLocked() const {
		if (mCore->mFreeBuffers.empty()) {
		return BufferQueueCore::INVALID_BUFFER_SLOT;
		}
		auto slot = mCore->mFreeBuffers.front();
		mCore->mFreeBuffers.pop_front();
		return slot;
		}

		int BufferQueueProducer::getFreeSlotLocked(int maxBufferCount) const {
		if (mCore->mFreeSlots.empty()) {
		return BufferQueueCore::INVALID_BUFFER_SLOT;
		}
		auto slot = *(mCore->mFreeSlots.begin());
		if (slot < maxBufferCount) {
		mCore->mFreeSlots.erase(slot);
		return slot;
		}
		return BufferQueueCore::INVALID_BUFFER_SLOT;
		}

		status_t BufferQueueProducer::waitForFreeSlotThenRelock(FreeSlotCaller caller,
		int* found, status_t* returnFlags) const {
		auto callerString = (caller == FreeSlotCaller::Dequeue) ?
		"dequeueBuffer" : "attachBuffer";
		bool tryAgain = true;
		while (tryAgain) {
		if (mCore->mIsAbandoned) {
		BQ_LOGE("%s: BufferQueue has been abandoned", caller);
		BQ_LOGE("%s: BufferQueue has been abandoned", callerString);
		return NO_INIT;
		}

		@@ -221,7 +244,7 @@ status_t BufferQueueProducer::waitForFreeSlotThenRelock(const char* caller,
		if (mCore->mBufferHasBeenQueued &&
		dequeuedCount >= mCore->mMaxDequeuedBufferCount) {
		BQ_LOGE("%s: attempting to exceed the max dequeued buffer count "
		"(%d)", caller, mCore->mMaxDequeuedBufferCount);
		"(%d)", callerString, mCore->mMaxDequeuedBufferCount);
		return INVALID_OPERATION;
		}

		@@ -234,7 +257,7 @@ status_t BufferQueueProducer::waitForFreeSlotThenRelock(const char* caller,
		bool tooManyBuffers = mCore->mQueue.size()
		> static_cast<size_t>(maxBufferCount);
		if (tooManyBuffers) {
		BQ_LOGV("%s: queue size is %zu, waiting", caller,
		BQ_LOGV("%s: queue size is %zu, waiting", callerString,
		mCore->mQueue.size());
		} else {
		// If in single buffer mode and a shared buffer exists, always
		@@ -242,16 +265,23 @@ status_t BufferQueueProducer::waitForFreeSlotThenRelock(const char* caller,
		if (mCore->mSingleBufferMode && mCore->mSingleBufferSlot !=
		BufferQueueCore::INVALID_BUFFER_SLOT) {
		*found = mCore->mSingleBufferSlot;
		} else if (!mCore->mFreeBuffers.empty()) {
		auto slot = mCore->mFreeBuffers.begin();
		found = slot;
		mCore->mFreeBuffers.erase(slot);
		} else if (mCore->mAllowAllocation && !mCore->mFreeSlots.empty()) {
		auto slot = mCore->mFreeSlots.begin();
		// Only return free slots up to the max buffer count
		if (*slot < maxBufferCount) {
		found = slot;
		mCore->mFreeSlots.erase(slot);
		} else {
		if (caller == FreeSlotCaller::Dequeue) {
		// If we're calling this from dequeue, prefer free buffers
		auto slot = getFreeBufferLocked();
		if (slot != BufferQueueCore::INVALID_BUFFER_SLOT) {
		*found = slot;
		} else if (mCore->mAllowAllocation) {
		*found = getFreeSlotLocked(maxBufferCount);
		}
		} else {
		// If we're calling this from attach, prefer free slots
		auto slot = getFreeSlotLocked(maxBufferCount);
		if (slot != BufferQueueCore::INVALID_BUFFER_SLOT) {
		*found = slot;
		} else {
		*found = getFreeBufferLocked();
		}
		}
		}
		}
		@@ -338,8 +368,8 @@ status_t BufferQueueProducer::dequeueBuffer(int *outSlot,

		int found = BufferItem::INVALID_BUFFER_SLOT;
		while (found == BufferItem::INVALID_BUFFER_SLOT) {
		status_t status = waitForFreeSlotThenRelock("dequeueBuffer", &found,
		&returnFlags);
		status_t status = waitForFreeSlotThenRelock(FreeSlotCaller::Dequeue,
		&found, &returnFlags);
		if (status != NO_ERROR) {
		return status;
		}
		@@ -614,7 +644,7 @@ status_t BufferQueueProducer::attachBuffer(int* outSlot,

		status_t returnFlags = NO_ERROR;
		int found;
		status_t status = waitForFreeSlotThenRelock("attachBuffer", &found,
		status_t status = waitForFreeSlotThenRelock(FreeSlotCaller::Attach, &found,
		&returnFlags);
		if (status != NO_ERROR) {
		return status;

libs/gui/tests/BufferQueue_test.cpp

+24 −2

Original line number	Diff line number	Diff line
		@@ -500,7 +500,7 @@ TEST_F(BufferQueueTest, TestSingleBufferMode) {
		ASSERT_EQ(HAL_DATASPACE_UNKNOWN, item.mDataSpace);
		ASSERT_EQ(Rect(0, 0, 1, 1), item.mCrop);
		ASSERT_EQ(NATIVE_WINDOW_SCALING_MODE_FREEZE, item.mScalingMode);
		ASSERT_EQ(0, item.mTransform);
		ASSERT_EQ(0u, item.mTransform);
		ASSERT_EQ(Fence::NO_FENCE, item.mFence);

		ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber,
		@@ -527,7 +527,7 @@ TEST_F(BufferQueueTest, TestSingleBufferMode) {
		ASSERT_EQ(HAL_DATASPACE_UNKNOWN, item.mDataSpace);
		ASSERT_EQ(Rect(0, 0, 1, 1), item.mCrop);
		ASSERT_EQ(NATIVE_WINDOW_SCALING_MODE_FREEZE, item.mScalingMode);
		ASSERT_EQ(0, item.mTransform);
		ASSERT_EQ(0u, item.mTransform);
		ASSERT_EQ(Fence::NO_FENCE, item.mFence);

		ASSERT_EQ(OK, mConsumer->releaseBuffer(item.mSlot, item.mFrameNumber,
		@@ -597,4 +597,26 @@ TEST_F(BufferQueueTest, TestTimeouts) {
		ASSERT_GE(systemTime() - startTime, TIMEOUT);
		}

		TEST_F(BufferQueueTest, CanAttachWhileDisallowingAllocation) {
		createBufferQueue();
		sp<DummyConsumer> dc(new DummyConsumer);
		ASSERT_EQ(OK, mConsumer->consumerConnect(dc, true));
		IGraphicBufferProducer::QueueBufferOutput output;
		ASSERT_EQ(OK, mProducer->connect(new DummyProducerListener,
		NATIVE_WINDOW_API_CPU, true, &output));

		int slot = BufferQueue::INVALID_BUFFER_SLOT;
		sp<Fence> sourceFence;
		ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION,
		mProducer->dequeueBuffer(&slot, &sourceFence, 0, 0, 0, 0));
		sp<GraphicBuffer> buffer;
		ASSERT_EQ(OK, mProducer->requestBuffer(slot, &buffer));
		ASSERT_EQ(OK, mProducer->detachBuffer(slot));

		ASSERT_EQ(OK, mProducer->allowAllocation(false));

		slot = BufferQueue::INVALID_BUFFER_SLOT;
		ASSERT_EQ(OK, mProducer->attachBuffer(&slot, buffer));
		}

		} // namespace android