[libcamera-devel] [PATCH 4/5] android: camera_worker: Notify fence wait failures

[Jacopo Mondi]

The CameraDevice class stores one CaptureRequestDescriptor instance
for each capture request queued to the Camera. Such descriptors are
cleared when the Camera completes the Request in the
CameraDevice::requestComplete() slot.

The CameraDevice class relies on an internal worker to off-load
waiting on synchronization fences, and does unconditionally store
the descriptor in the descriptors_ class member map to handle its
lifetime.

If waiting on a fence fails, the created descriptor remains in the
descriptors_ map until the next call to stop() or close(), as
requestComplete() will never be called, and the camera
service is never notified about the queueing failure.

Fix that by allowing the CameraWorker to notify to the CameraDevice if
waiting on a fence has failed, by installing a new requestQueueFailed
Signal.

The Signal is emitted by the CameraWorker internal worker if waiting on
a fence has failed. The CameraDevice is augmented with a slot connected
to the Signal that removes the descriptor from the map and notifies the
camera framework about the failure.

Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
---
 src/android/camera_device.cpp | 22 +++++++++++++++++++++-
 src/android/camera_device.h   |  1 +
 src/android/camera_worker.cpp | 10 ++++++----
 src/android/camera_worker.h   |  9 +++++++--
 4 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index a0ea138d9499..0ce9b699bfae 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -244,7 +244,7 @@ CameraDevice::Camera3RequestDescriptor::Camera3RequestDescriptor(
 	 * Create the CaptureRequest, stored as a unique_ptr<> to tie its
 	 * lifetime to the descriptor.
 	 */
-	request_ = std::make_unique<CaptureRequest>(camera);
+	request_ = std::make_unique<CaptureRequest>(camera, camera3Request);
 }
 
 /*
@@ -264,6 +264,7 @@ CameraDevice::CameraDevice(unsigned int id, std::shared_ptr<Camera> camera)
 	: id_(id), state_(State::Stopped), camera_(std::move(camera)),
 	  facing_(CAMERA_FACING_FRONT), orientation_(0)
 {
+	worker_.requestQueueFailed.connect(this, &CameraDevice::requestQueueFailed);
 	camera_->requestCompleted.connect(this, &CameraDevice::requestComplete);
 
 	maker_ = "libcamera";
@@ -1060,6 +1061,25 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 	return 0;
 }
 
+void CameraDevice::requestQueueFailed(CaptureRequest *request)
+{
+	/*
+	 * Remove the descriptor from the map if the worker has failed
+	 * to process it and notify the camera service about the failure.
+	 */
+	MutexLocker descriptorsLock(descriptorsMutex_);
+	auto it = descriptors_.find(request->cookie());
+	if (it == descriptors_.end()) {
+		LOG(HAL, Fatal)
+			<< "Unknown request: " << request->cookie();
+		return;
+	}
+
+	descriptors_.extract(it);
+
+	abortRequest(request->camera3Request());
+}
+
 void CameraDevice::requestComplete(Request *request)
 {
 	decltype(descriptors_)::node_type node;
diff --git a/src/android/camera_device.h b/src/android/camera_device.h
index 54c4cb9ab499..65456ecca7e3 100644
--- a/src/android/camera_device.h
+++ b/src/android/camera_device.h
@@ -83,6 +83,7 @@ private:
 		std::vector<std::unique_ptr<libcamera::FrameBuffer>> frameBuffers_;
 		CameraMetadata settings_;
 		std::unique_ptr<CaptureRequest> request_;
+		const camera3_capture_request_t *camera3Request_;
 	};
 
 	enum class State {
diff --git a/src/android/camera_worker.cpp b/src/android/camera_worker.cpp
index 98dddd9eb13b..7f39fab01638 100644
--- a/src/android/camera_worker.cpp
+++ b/src/android/camera_worker.cpp
@@ -27,8 +27,9 @@ LOG_DECLARE_CATEGORY(HAL)
  * by the CameraWorker which queues it to the libcamera::Camera after handling
  * fences.
  */
-CaptureRequest::CaptureRequest(libcamera::Camera *camera)
-	: camera_(camera)
+CaptureRequest::CaptureRequest(libcamera::Camera *camera,
+			       const camera3_capture_request_t *camera3Request)
+	: camera_(camera), camera3Request_(camera3Request)
 {
 	request_ = camera_->createRequest(reinterpret_cast<uint64_t>(this));
 }
@@ -77,7 +78,7 @@ void CameraWorker::queueRequest(CaptureRequest *request)
 {
 	/* Async process the request on the worker which runs its own thread. */
 	worker_.invokeMethod(&Worker::processRequest, ConnectionTypeQueued,
-			     request);
+			     this, request);
 }
 
 /*
@@ -109,7 +110,7 @@ int CameraWorker::Worker::waitFence(int fence)
 	return -errno;
 }
 
-void CameraWorker::Worker::processRequest(CaptureRequest *request)
+void CameraWorker::Worker::processRequest(CameraWorker *context, CaptureRequest *request)
 {
 	/* Wait on all fences before queuing the Request. */
 	for (int fence : request->fences()) {
@@ -121,6 +122,7 @@ void CameraWorker::Worker::processRequest(CaptureRequest *request)
 		if (ret < 0) {
 			LOG(HAL, Error) << "Failed waiting for fence: "
 					<< fence << ": " << strerror(-ret);
+			context->requestQueueFailed.emit(request);
 			return;
 		}
 	}
diff --git a/src/android/camera_worker.h b/src/android/camera_worker.h
index 67ae50bd9288..86f20f741e54 100644
--- a/src/android/camera_worker.h
+++ b/src/android/camera_worker.h
@@ -10,6 +10,7 @@
 #include <memory>
 
 #include <libcamera/base/object.h>
+#include <libcamera/base/signal.h>
 #include <libcamera/base/thread.h>
 
 #include <libcamera/camera.h>
@@ -22,7 +23,8 @@ class CameraDevice;
 class CaptureRequest
 {
 public:
-	CaptureRequest(libcamera::Camera *camera);
+	CaptureRequest(libcamera::Camera *camera,
+		       const camera3_capture_request_t *camera3Request);
 
 	const std::vector<int> &fences() const { return acquireFences_; }
 	libcamera::ControlList &controls() { return request_->controls(); }
@@ -31,6 +33,7 @@ public:
 		return request_->metadata();
 	}
 	unsigned long cookie() const { return request_->cookie(); }
+	const camera3_capture_request_t *camera3Request() const { return camera3Request_; }
 
 	void addBuffer(libcamera::Stream *stream,
 		       libcamera::FrameBuffer *buffer, int fence);
@@ -40,6 +43,7 @@ private:
 	libcamera::Camera *camera_;
 	std::vector<int> acquireFences_;
 	std::unique_ptr<libcamera::Request> request_;
+	const camera3_capture_request_t *camera3Request_;
 };
 
 class CameraWorker : private libcamera::Thread
@@ -51,6 +55,7 @@ public:
 	void stop();
 
 	void queueRequest(CaptureRequest *request);
+	libcamera::Signal<CaptureRequest *> requestQueueFailed;
 
 protected:
 	void run() override;
@@ -59,7 +64,7 @@ private:
 	class Worker : public libcamera::Object
 	{
 	public:
-		void processRequest(CaptureRequest *request);
+		void processRequest(CameraWorker *context, CaptureRequest *request);
 
 	private:
 		int waitFence(int fence);
-- 
2.32.0