Merge "Surface: don't hold mMutex when queuing buffers" into main (305912a7) · Commits · e / os / android_frameworks_native

libs/gui/Surface.cpp

+113 −0

Original line number	Diff line number	Diff line
		@@ -1168,6 +1168,117 @@ void Surface::onBufferQueuedLocked(int slot, sp<Fence> fence,
		}
		}

		#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS)

		int Surface::queueBuffer(android_native_buffer_t* buffer, int fenceFd) {
		ATRACE_CALL();
		ALOGV("Surface::queueBuffer");

		IGraphicBufferProducer::QueueBufferOutput output;
		IGraphicBufferProducer::QueueBufferInput input;
		int slot;
		sp<Fence> fence;
		{
		Mutex::Autolock lock(mMutex);

		slot = getSlotFromBufferLocked(buffer);
		if (slot < 0) {
		if (fenceFd >= 0) {
		close(fenceFd);
		}
		return slot;
		}
		if (mSharedBufferSlot == slot && mSharedBufferHasBeenQueued) {
		if (fenceFd >= 0) {
		close(fenceFd);
		}
		return OK;
		}

		getQueueBufferInputLocked(buffer, fenceFd, mTimestamp, &input);
		applyGrallocMetadataLocked(buffer, input);
		fence = input.fence;
		}
		nsecs_t now = systemTime();
		// Drop the lock temporarily while we touch the underlying producer. In the case of a local
		// BufferQueue, the following should be allowable:
		//
		// Surface::queueBuffer
		// -> IConsumerListener::onFrameAvailable callback triggers automatically
		// -> implementation calls IGraphicBufferConsumer::acquire/release immediately
		// -> SurfaceListener::onBufferRelesed callback triggers automatically
		// -> implementation calls Surface::dequeueBuffer
		status_t err = mGraphicBufferProducer->queueBuffer(slot, input, &output);
		{
		Mutex::Autolock lock(mMutex);

		mLastQueueDuration = systemTime() - now;
		if (err != OK) {
		ALOGE("queueBuffer: error queuing buffer, %d", err);
		}

		onBufferQueuedLocked(slot, fence, output);
		}

		return err;
		}

		int Surface::queueBuffers(const std::vector<BatchQueuedBuffer>& buffers) {
		ATRACE_CALL();
		ALOGV("Surface::queueBuffers");

		size_t numBuffers = buffers.size();
		std::vector<IGraphicBufferProducer::QueueBufferInput> queueBufferInputs(numBuffers);
		std::vector<IGraphicBufferProducer::QueueBufferOutput> queueBufferOutputs;
		std::vector<int> bufferSlots(numBuffers, -1);
		std::vector<sp<Fence>> bufferFences(numBuffers);

		int err;
		{
		Mutex::Autolock lock(mMutex);

		if (mSharedBufferMode) {
		ALOGE("%s: batched operation is not supported in shared buffer mode", __FUNCTION__);
		return INVALID_OPERATION;
		}

		for (size_t batchIdx = 0; batchIdx < numBuffers; batchIdx++) {
		int i = getSlotFromBufferLocked(buffers[batchIdx].buffer);
		if (i < 0) {
		if (buffers[batchIdx].fenceFd >= 0) {
		close(buffers[batchIdx].fenceFd);
		}
		return i;
		}
		bufferSlots[batchIdx] = i;

		IGraphicBufferProducer::QueueBufferInput input;
		getQueueBufferInputLocked(buffers[batchIdx].buffer, buffers[batchIdx].fenceFd,
		buffers[batchIdx].timestamp, &input);
		bufferFences[batchIdx] = input.fence;
		queueBufferInputs[batchIdx] = input;
		}
		}
		nsecs_t now = systemTime();
		err = mGraphicBufferProducer->queueBuffers(queueBufferInputs, &queueBufferOutputs);
		{
		Mutex::Autolock lock(mMutex);
		mLastQueueDuration = systemTime() - now;
		if (err != OK) {
		ALOGE("%s: error queuing buffer, %d", __FUNCTION__, err);
		}

		for (size_t batchIdx = 0; batchIdx < numBuffers; batchIdx++) {
		onBufferQueuedLocked(bufferSlots[batchIdx], bufferFences[batchIdx],
		queueBufferOutputs[batchIdx]);
		}
		}

		return err;
		}

		#else

		int Surface::queueBuffer(android_native_buffer_t* buffer, int fenceFd) {
		ATRACE_CALL();
		ALOGV("Surface::queueBuffer");
		@@ -1255,6 +1366,8 @@ int Surface::queueBuffers(const std::vector<BatchQueuedBuffer>& buffers) {
		return err;
		}

		#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS)

		void Surface::querySupportedTimestampsLocked() const {
		// mMutex must be locked when calling this method.

libs/gui/tests/Surface_test.cpp

+76 −0

Original line number	Diff line number	Diff line
		@@ -23,12 +23,17 @@
		#include <android/gui/IDisplayEventConnection.h>
		#include <android/gui/ISurfaceComposer.h>
		#include <android/hardware/configstore/1.0/ISurfaceFlingerConfigs.h>
		#include <android/hardware_buffer.h>
		#include <binder/ProcessState.h>
		#include <com_android_graphics_libgui_flags.h>
		#include <configstore/Utils.h>
		#include <gui/AidlStatusUtil.h>
		#include <gui/BufferItemConsumer.h>
		#include <gui/BufferQueue.h>
		#include <gui/CpuConsumer.h>
		#include <gui/IConsumerListener.h>
		#include <gui/IGraphicBufferConsumer.h>
		#include <gui/IGraphicBufferProducer.h>
		#include <gui/ISurfaceComposer.h>
		#include <gui/Surface.h>
		#include <gui/SurfaceComposerClient.h>
		@@ -36,6 +41,7 @@
		#include <private/gui/ComposerService.h>
		#include <private/gui/ComposerServiceAIDL.h>
		#include <sys/types.h>
		#include <system/window.h>
		#include <ui/BufferQueueDefs.h>
		#include <ui/DisplayMode.h>
		#include <ui/GraphicBuffer.h>
		@@ -2312,6 +2318,76 @@ TEST_F(SurfaceTest, AllowAllocation) {
		EXPECT_EQ(OK, surface->allowAllocation(true));
		EXPECT_EQ(OK, surface->dequeueBuffer(&buffer, &fence));
		}

		TEST_F(SurfaceTest, QueueAcquireReleaseDequeue_CalledInStack_DoesNotDeadlock) {
		class DequeuingSurfaceListener : public SurfaceListener {
		public:
		DequeuingSurfaceListener(const wp<Surface>& surface) : mSurface(surface) {}

		virtual void onBufferReleased() override {
		sp<Surface> surface = mSurface.promote();
		ASSERT_NE(nullptr, surface);
		EXPECT_EQ(OK, surface->dequeueBuffer(&mBuffer, &mFence));
		}

		virtual bool needsReleaseNotify() override { return true; }
		virtual void onBuffersDiscarded(const std::vector<sp<GraphicBuffer>>&) override {}
		virtual void onBufferDetached(int) override {}

		sp<GraphicBuffer> mBuffer;
		sp<Fence> mFence;

		private:
		wp<Surface> mSurface;
		};

		class ImmediateReleaseConsumerListener : public BufferItemConsumer::FrameAvailableListener {
		public:
		ImmediateReleaseConsumerListener(const wp<BufferItemConsumer>& consumer)
		: mConsumer(consumer) {}

		virtual void onFrameAvailable(const BufferItem&) override {
		sp<BufferItemConsumer> consumer = mConsumer.promote();
		ASSERT_NE(nullptr, consumer);

		mCalls += 1;

		BufferItem buffer;
		EXPECT_EQ(OK, consumer->acquireBuffer(&buffer, 0));
		EXPECT_EQ(OK, consumer->releaseBuffer(buffer));
		}

		size_t mCalls = 0;

		private:
		wp<BufferItemConsumer> mConsumer;
		};

		sp<IGraphicBufferProducer> bqProducer;
		sp<IGraphicBufferConsumer> bqConsumer;
		BufferQueue::createBufferQueue(&bqProducer, &bqConsumer);

		sp<BufferItemConsumer> consumer = sp<BufferItemConsumer>::make(bqConsumer, 3);
		sp<Surface> surface = sp<Surface>::make(bqProducer);
		sp<ImmediateReleaseConsumerListener> consumerListener =
		sp<ImmediateReleaseConsumerListener>::make(consumer);
		consumer->setFrameAvailableListener(consumerListener);

		sp<DequeuingSurfaceListener> surfaceListener = sp<DequeuingSurfaceListener>::make(surface);
		EXPECT_EQ(OK, surface->connect(NATIVE_WINDOW_API_CPU, false, surfaceListener));

		EXPECT_EQ(OK, surface->setMaxDequeuedBufferCount(2));

		sp<GraphicBuffer> buffer;
		sp<Fence> fence;
		EXPECT_EQ(OK, surface->dequeueBuffer(&buffer, &fence));
		EXPECT_EQ(OK, surface->queueBuffer(buffer, fence));

		EXPECT_EQ(1u, consumerListener->mCalls);
		EXPECT_NE(nullptr, surfaceListener->mBuffer);

		EXPECT_EQ(OK, surface->disconnect(NATIVE_WINDOW_API_CPU));
		}
		#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_PLATFORM_API_IMPROVEMENTS)

		} // namespace android