[libcamera-devel,v4,11/11] android: Remove CameraWorker

14971 diff mbox series

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index 1938b10509fa..3a2969c70a45 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -15,10 +15,12 @@ 
 
 #include <libcamera/base/log.h>
 #include <libcamera/base/thread.h>
+#include <libcamera/base/unique_fd.h>
 #include <libcamera/base/utils.h>
 
 #include <libcamera/control_ids.h>
 #include <libcamera/controls.h>
+#include <libcamera/fence.h>
 #include <libcamera/formats.h>
 #include <libcamera/property_ids.h>
 
@@ -406,7 +408,6 @@  void CameraDevice::flush()
 		state_ = State::Flushing;
 	}
 
-	worker_.stop();
 	camera_->stop();
 
 	MutexLocker stateLock(stateMutex_);
@@ -419,7 +420,6 @@  void CameraDevice::stop()
 	if (state_ == State::Stopped)
 		return;
 
-	worker_.stop();
 	camera_->stop();
 
 	descriptors_ = {};
@@ -912,13 +912,9 @@  int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 
 		/*
 		 * Inspect the camera stream type, create buffers opportunely
-		 * and add them to the Request if required. Only acquire fences
-		 * for streams of type Direct are handled by the CameraWorker,
-		 * while fences for streams of type Internal and Mapped are
-		 * handled at post-processing time.
+		 * and add them to the Request if required.
 		 */
 		FrameBuffer *frameBuffer = nullptr;
-		int acquireFence = -1;
 		switch (cameraStream->type()) {
 		case CameraStream::Type::Mapped:
 			/*
@@ -943,7 +939,6 @@  int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 						  cameraStream->configuration().pixelFormat,
 						  cameraStream->configuration().size);
 			frameBuffer = buffer.frameBuffer.get();
-			acquireFence = buffer.fence;
 			LOG(HAL, Debug) << ss.str() << " (direct)";
 			break;
 
@@ -970,12 +965,12 @@  int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 		}
 
 		descriptor->request_->addBuffer(cameraStream->stream(),
-						frameBuffer, acquireFence);
+			frameBuffer, std::make_unique<Fence>(std::move(buffer.fence)));
 	}
 
 	/*
 	 * Translate controls from Android to libcamera and queue the request
-	 * to the CameraWorker thread.
+	 * to the camera.
 	 */
 	int ret = processControls(descriptor.get());
 	if (ret)
@@ -1001,26 +996,23 @@  int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 	}
 
 	if (state_ == State::Stopped) {
-		worker_.start();
-
 		ret = camera_->start();
 		if (ret) {
 			LOG(HAL, Error) << "Failed to start camera";
-			worker_.stop();
 			return ret;
 		}
 
 		state_ = State::Running;
 	}
 
-	CaptureRequest *request = descriptor->request_.get();
+	Request *request = descriptor->request_.get();
 
 	{
 		MutexLocker descriptorsLock(descriptorsMutex_);
 		descriptors_.push(std::move(descriptor));
 	}
 
-	worker_.queueRequest(request);
+	camera_->queueRequest(request);
 
 	return 0;
 }
@@ -1042,16 +1034,17 @@  void CameraDevice::requestComplete(Request *request)
 		/*
 		 * Streams of type Direct have been queued to the
 		 * libcamera::Camera and their acquire fences have
-		 * already been waited on by the CameraWorker.
+		 * already been waited on by the library.
 		 *
 		 * Acquire fences of streams of type Internal and Mapped
 		 * will be handled during post-processing.
-		 *
-		 * \todo Instrument the CameraWorker to set the acquire
-		 * fence to -1 once it has handled it and remove this check.
 		 */
-		if (stream->type() == CameraStream::Type::Direct)
-			buffer.fence = -1;
+		if (stream->type() == CameraStream::Type::Direct) {
+			/* If handling of the fence has failed restore buffer.fence. */
+			std::unique_ptr<Fence> fence = buffer.frameBuffer->releaseFence();
+			if (fence)
+				buffer.fence = fence->release();
+		}
 		buffer.status = Camera3RequestDescriptor::Status::Success;
 	}
 
@@ -1172,7 +1165,7 @@  void CameraDevice::sendCaptureResults()
 		std::vector<camera3_stream_buffer_t> resultBuffers;
 		resultBuffers.reserve(descriptor->buffers_.size());
 
-		for (const auto &buffer : descriptor->buffers_) {
+		for (auto &buffer : descriptor->buffers_) {
 			camera3_buffer_status status = CAMERA3_BUFFER_STATUS_ERROR;
 
 			if (buffer.status == Camera3RequestDescriptor::Status::Success)
@@ -1186,7 +1179,7 @@  void CameraDevice::sendCaptureResults()
 			 */
 			resultBuffers.push_back({ buffer.stream->camera3Stream(),
 						  buffer.camera3Buffer, status,
-						  -1, buffer.fence });
+						  -1, buffer.fence.release() });
 		}
 
 		captureResult.num_output_buffers = resultBuffers.size();
diff --git a/src/android/camera_device.h b/src/android/camera_device.h
index 51fe7da2cb47..494d79d2a3ea 100644
--- a/src/android/camera_device.h
+++ b/src/android/camera_device.h
@@ -30,7 +30,6 @@ 
 #include "camera_capabilities.h"
 #include "camera_metadata.h"
 #include "camera_stream.h"
-#include "camera_worker.h"
 #include "jpeg/encoder.h"
 
 class Camera3RequestDescriptor;
@@ -105,8 +104,6 @@  private:
 	unsigned int id_;
 	camera3_device_t camera3Device_;
 
-	CameraWorker worker_;
-
 	libcamera::Mutex stateMutex_; /* Protects access to the camera state. */
 	State state_;
 
diff --git a/src/android/camera_request.cpp b/src/android/camera_request.cpp
index 8162aa78e953..c46689a52d6d 100644
--- a/src/android/camera_request.cpp
+++ b/src/android/camera_request.cpp
@@ -47,8 +47,7 @@  Camera3RequestDescriptor::Camera3RequestDescriptor(
 	 * Create the CaptureRequest, stored as a unique_ptr<> to tie its
 	 * lifetime to the descriptor.
 	 */
-	request_ = std::make_unique<CaptureRequest>(camera,
-						    reinterpret_cast<uint64_t>(this));
+	request_ = camera->createRequest(reinterpret_cast<uint64_t>(this));
 }
 
 Camera3RequestDescriptor::~Camera3RequestDescriptor() = default;
@@ -57,8 +56,11 @@  Camera3RequestDescriptor::StreamBuffer::StreamBuffer(
 	CameraStream *cameraStream, const camera3_stream_buffer_t &buffer,
 	Camera3RequestDescriptor *requestDescriptor)
 	: stream(cameraStream), camera3Buffer(buffer.buffer),
-	  fence(buffer.acquire_fence), request(requestDescriptor)
+	  request(requestDescriptor)
 {
+	/* Do not reset buffer.acquire_fence by moving it to fence. */
+	int fenceFd = buffer.acquire_fence;
+	fence = UniqueFD(std::move(fenceFd));
 }
 
 Camera3RequestDescriptor::StreamBuffer::~StreamBuffer() = default;
diff --git a/src/android/camera_request.h b/src/android/camera_request.h
index f3cb6d643961..4372ad775c19 100644
--- a/src/android/camera_request.h
+++ b/src/android/camera_request.h
@@ -13,6 +13,7 @@ 
 #include <vector>
 
 #include <libcamera/base/class.h>
+#include <libcamera/base/unique_fd.h>
 
 #include <libcamera/camera.h>
 #include <libcamera/framebuffer.h>
@@ -20,7 +21,6 @@ 
 #include <hardware/camera3.h>
 
 #include "camera_metadata.h"
-#include "camera_worker.h"
 
 class CameraBuffer;
 class CameraStream;
@@ -45,7 +45,7 @@  public:
 		CameraStream *stream;
 		buffer_handle_t *camera3Buffer;
 		std::unique_ptr<libcamera::FrameBuffer> frameBuffer;
-		int fence;
+		libcamera::UniqueFD fence;
 		Status status = Status::Success;
 		libcamera::FrameBuffer *internalBuffer = nullptr;
 		const libcamera::FrameBuffer *srcBuffer = nullptr;
@@ -71,7 +71,7 @@  public:
 	std::vector<StreamBuffer> buffers_;
 
 	CameraMetadata settings_;
-	std::unique_ptr<CaptureRequest> request_;
+	std::unique_ptr<libcamera::Request> request_;
 	std::unique_ptr<CameraMetadata> resultMetadata_;
 
 	bool complete_ = false;
diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
index 9023c13c40f0..08252341e9cf 100644
--- a/src/android/camera_stream.cpp
+++ b/src/android/camera_stream.cpp
@@ -169,16 +169,16 @@  int CameraStream::process(Camera3RequestDescriptor::StreamBuffer *streamBuffer)
 	ASSERT(type_ != Type::Direct);
 
 	/* Handle waiting on fences on the destination buffer. */
-	if (streamBuffer->fence != -1) {
-		int ret = waitFence(streamBuffer->fence);
+	if (streamBuffer->fence.isValid()) {
+		int ret = waitFence(streamBuffer->fence.get());
 		if (ret < 0) {
 			LOG(HAL, Error) << "Failed waiting for fence: "
-					<< streamBuffer->fence << ": " << strerror(-ret);
+					<< streamBuffer->fence.get() << ": "
+					<< strerror(-ret);
 			return ret;
 		}
 
-		::close(streamBuffer->fence);
-		streamBuffer->fence = -1;
+		streamBuffer->fence.reset();
 	}
 
 	const StreamConfiguration &output = configuration();
diff --git a/src/android/camera_worker.cpp b/src/android/camera_worker.cpp
deleted file mode 100644
index dabb305407a2..000000000000
--- a/src/android/camera_worker.cpp
+++ /dev/null
@@ -1,129 +0,0 @@ 
-/* SPDX-License-Identifier: LGPL-2.1-or-later */
-/*
- * Copyright (C) 2020, Google Inc.
- *
- * camera_worker.cpp - Process capture requests on behalf of the Camera HAL
- */
-
-#include "camera_worker.h"
-
-#include <errno.h>
-#include <string.h>
-#include <sys/poll.h>
-#include <unistd.h>
-
-#include "camera_device.h"
-
-using namespace libcamera;
-
-LOG_DECLARE_CATEGORY(HAL)
-
-/*
- * \class CaptureRequest
- * \brief Wrap a libcamera::Request associated with buffers and fences
- *
- * A CaptureRequest is constructed by the CameraDevice, filled with
- * buffers and fences provided by the camera3 framework and then processed
- * by the CameraWorker which queues it to the libcamera::Camera after handling
- * fences.
- */
-CaptureRequest::CaptureRequest(Camera *camera, uint64_t cookie)
-	: camera_(camera)
-{
-	request_ = camera_->createRequest(cookie);
-}
-
-void CaptureRequest::addBuffer(Stream *stream, FrameBuffer *buffer, int fence)
-{
-	request_->addBuffer(stream, buffer);
-	acquireFences_.push_back(fence);
-}
-
-void CaptureRequest::queue()
-{
-	camera_->queueRequest(request_.get());
-}
-
-/*
- * \class CameraWorker
- * \brief Process a CaptureRequest on an internal thread
- *
- * The CameraWorker class wraps a Worker that runs on an internal thread
- * and schedules processing of CaptureRequest through it.
- */
-CameraWorker::CameraWorker()
-{
-	worker_.moveToThread(this);
-}
-
-void CameraWorker::start()
-{
-	Thread::start();
-}
-
-void CameraWorker::stop()
-{
-	exit();
-	wait();
-}
-
-void CameraWorker::run()
-{
-	exec();
-	dispatchMessages(Message::Type::InvokeMessage);
-}
-
-void CameraWorker::queueRequest(CaptureRequest *request)
-{
-	/* Async process the request on the worker which runs its own thread. */
-	worker_.invokeMethod(&Worker::processRequest, ConnectionTypeQueued,
-			     request);
-}
-
-/*
- * \class CameraWorker::Worker
- * \brief Process a CaptureRequest handling acquisition fences
- */
-int CameraWorker::Worker::waitFence(int fence)
-{
-	/*
-	 * \todo Better characterize the timeout. Currently equal to the one
-	 * used by the Rockchip Camera HAL on ChromeOS.
-	 */
-	constexpr unsigned int timeoutMs = 300;
-	struct pollfd fds = { fence, POLLIN, 0 };
-
-	do {
-		int ret = poll(&fds, 1, timeoutMs);
-		if (ret == 0)
-			return -ETIME;
-
-		if (ret > 0) {
-			if (fds.revents & (POLLERR | POLLNVAL))
-				return -EINVAL;
-
-			return 0;
-		}
-	} while (errno == EINTR || errno == EAGAIN);
-
-	return -errno;
-}
-
-void CameraWorker::Worker::processRequest(CaptureRequest *request)
-{
-	/* Wait on all fences before queuing the Request. */
-	for (int fence : request->fences()) {
-		if (fence == -1)
-			continue;
-
-		int ret = waitFence(fence);
-		close(fence);
-		if (ret < 0) {
-			LOG(HAL, Error) << "Failed waiting for fence: "
-					<< fence << ": " << strerror(-ret);
-			return;
-		}
-	}
-
-	request->queue();
-}
diff --git a/src/android/camera_worker.h b/src/android/camera_worker.h
deleted file mode 100644
index 26ecc2732046..000000000000
--- a/src/android/camera_worker.h
+++ /dev/null
@@ -1,70 +0,0 @@ 
-/* SPDX-License-Identifier: LGPL-2.1-or-later */
-/*
- * Copyright (C) 2020, Google Inc.
- *
- * camera_worker.h - Process capture requests on behalf of the Camera HAL
- */
-
-#pragma once
-
-#include <memory>
-#include <stdint.h>
-
-#include <libcamera/base/object.h>
-#include <libcamera/base/thread.h>
-
-#include <libcamera/camera.h>
-#include <libcamera/framebuffer.h>
-#include <libcamera/request.h>
-#include <libcamera/stream.h>
-
-class CameraDevice;
-
-class CaptureRequest
-{
-public:
-	CaptureRequest(libcamera::Camera *camera, uint64_t cookie);
-
-	const std::vector<int> &fences() const { return acquireFences_; }
-	libcamera::ControlList &controls() { return request_->controls(); }
-	const libcamera::ControlList &metadata() const
-	{
-		return request_->metadata();
-	}
-	unsigned long cookie() const { return request_->cookie(); }
-
-	void addBuffer(libcamera::Stream *stream,
-		       libcamera::FrameBuffer *buffer, int fence);
-	void queue();
-
-private:
-	libcamera::Camera *camera_;
-	std::vector<int> acquireFences_;
-	std::unique_ptr<libcamera::Request> request_;
-};
-
-class CameraWorker : private libcamera::Thread
-{
-public:
-	CameraWorker();
-
-	void start();
-	void stop();
-
-	void queueRequest(CaptureRequest *request);
-
-protected:
-	void run() override;
-
-private:
-	class Worker : public libcamera::Object
-	{
-	public:
-		void processRequest(CaptureRequest *request);
-
-	private:
-		int waitFence(int fence);
-	};
-
-	Worker worker_;
-};
diff --git a/src/android/meson.build b/src/android/meson.build
index 332b177ca805..75b4bf207085 100644
--- a/src/android/meson.build
+++ b/src/android/meson.build
@@ -47,7 +47,6 @@  android_hal_sources = files([
     'camera_ops.cpp',
     'camera_request.cpp',
     'camera_stream.cpp',
-    'camera_worker.cpp',
     'jpeg/encoder_libjpeg.cpp',
     'jpeg/exif.cpp',
     'jpeg/post_processor_jpeg.cpp',