[libcamera-devel] [RFC PATCH v2 5/5] android: camera_stream: Run post-processor in a separate thread

[Umang Jain]

In CameraStream, introduce a new private PostProcessorWorker
class derived from Object. A instance of PostProcessorWorker
is moved to a separate thread instance which will be responsible
to run the post-processor.

Running PostProcessor asynchronously should entail that all the
data context needed by the PostProcessor should remain valid for
the entire duration of its run. Most of the context preserving
part has been addressed inthe previous patch, we just need to
ensure the source framebuffer data that comes via Camera::Request,
should remain valid for the entire duration of post-processing
running asynchronously. In order to so, we maintain a separate
copy of the framebuffer data and add it to the Camera3RequestDescriptor
in which we preserve rest of the context.

Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
---
 src/android/camera_device.cpp | 18 +++++++++++++++++-
 src/android/camera_device.h   |  4 ++++
 src/android/camera_stream.cpp | 28 +++++++++++++++++++++-------
 src/android/camera_stream.h   | 27 +++++++++++++++++++++++++++
 4 files changed, 69 insertions(+), 8 deletions(-)

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index fd07c9bd..58e6d705 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -1175,6 +1175,21 @@ void CameraDevice::requestComplete(Request *request)
 		descriptor.resultMetadata_ = std::move(resultMetadata);
 		descriptor.captureResult_ = captureResult;
 
+		FrameBuffer *source = src;
+		if (cameraStream->type() != CameraStream::Type::Internal) {
+			/*
+			 * The source buffer is owned by Request object which
+			 * can be reused for future capture request. Since
+			 * post-processor will run asynchrnously, we need to
+			 * copy the source buffer and use it as source buffer.
+			 */
+			for (const auto &plane : src->planes())
+				descriptor.srcPlanes_.push_back(plane);
+			descriptor.srcFramebuffer_ =
+				std::make_unique<FrameBuffer>(descriptor.srcPlanes_);
+			source = descriptor.srcFramebuffer_.get();
+		}
+
 		std::unique_ptr<Camera3RequestDescriptor> reqDescriptor =
 			std::make_unique<Camera3RequestDescriptor>();
 		*reqDescriptor = std::move(descriptor);
@@ -1188,7 +1203,7 @@ void CameraDevice::requestComplete(Request *request)
 							 metadata);
 			});
 
-		int ret = cameraStream->process(src, *buffer.buffer,
+		int ret = cameraStream->process(source, *buffer.buffer,
 						currentDescriptor->settings_,
 						metadata);
 		return;
@@ -1262,6 +1277,7 @@ void CameraDevice::streamProcessingComplete(CameraStream *cameraStream,
 
 void CameraDevice::sendQueuedCaptureResults()
 {
+	MutexLocker lock(queuedDescriptorsMutex_);
 	while (!queuedDescriptor_.empty()) {
 		std::unique_ptr<Camera3RequestDescriptor> &d = queuedDescriptor_.front();
 		if (d->status_ != Camera3RequestDescriptor::NOT_FINISHED) {
diff --git a/src/android/camera_device.h b/src/android/camera_device.h
index 36425773..e29a6fa5 100644
--- a/src/android/camera_device.h
+++ b/src/android/camera_device.h
@@ -99,6 +99,9 @@ private:
 		camera3_capture_result_t captureResult_;
 		libcamera::FrameBuffer *internalBuffer_;
 		completionStatus status_;
+
+		std::unique_ptr<libcamera::FrameBuffer> srcFramebuffer_;
+		std::vector<libcamera::FrameBuffer::Plane> srcPlanes_;
 	};
 
 	enum class State {
@@ -146,6 +149,7 @@ private:
 	libcamera::Mutex descriptorsMutex_; /* Protects descriptors_. */
 	std::map<uint64_t, Camera3RequestDescriptor> descriptors_;
 
+	libcamera::Mutex queuedDescriptorsMutex_; /* Protects queuedDescriptor_. */
 	std::deque<std::unique_ptr<Camera3RequestDescriptor>> queuedDescriptor_;
 
 	std::string maker_;
diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
index 92dd745b..2ce0542d 100644
--- a/src/android/camera_stream.cpp
+++ b/src/android/camera_stream.cpp
@@ -55,6 +55,12 @@ CameraStream::CameraStream(CameraDevice *const cameraDevice,
 		 * is what we instantiate here.
 		 */
 		postProcessor_ = std::make_unique<PostProcessorJpeg>(cameraDevice_);
+		ppWorker_ = std::make_unique<PostProcessorWorker>(postProcessor_.get());
+
+		thread_ = std::make_unique<libcamera::Thread>();
+		ppWorker_->moveToThread(thread_.get());
+		/* \todo: Where/when would you stop this thread? */
+		thread_->start();
 	}
 
 	if (type == Type::Internal) {
@@ -108,19 +114,27 @@ int CameraStream::process(const libcamera::FrameBuffer *source,
 	if (!postProcessor_)
 		return 0;
 
+	const StreamConfiguration &output = configuration();
 	/*
-	 * \todo Buffer mapping and processing should be moved to a
-	 * separate thread.
+	 * \todo Should buffer mapping be moved to post-processor's thread
+	 *  (maybe as a separate task?)
+	 *
+	 * \todo dest_ is racy if another requests gets queued for processing
+	 *  before previous one is finished.
 	 */
-	const StreamConfiguration &output = configuration();
-	CameraBuffer dest(camera3Dest, formats::MJPEG, output.size,
-			  PROT_READ | PROT_WRITE);
-	if (!dest.isValid()) {
+	dest_.reset();
+	dest_ = std::make_unique<CameraBuffer>(camera3Dest, formats::MJPEG,
+					       output.size, PROT_READ | PROT_WRITE);
+	if (!dest_->isValid()) {
 		LOG(HAL, Error) << "Failed to map android blob buffer";
 		return -EINVAL;
 	}
 
-	return postProcessor_->process(source, &dest, requestMetadata, resultMetadata);
+	ppWorker_->invokeMethod(&PostProcessorWorker::process,
+				ConnectionTypeQueued, source, dest_.get(),
+				requestMetadata, resultMetadata);
+
+	return 0;
 }
 
 void CameraStream::handlePostProcessing(PostProcessor::Status status)
diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h
index f9b11e5d..1802f003 100644
--- a/src/android/camera_stream.h
+++ b/src/android/camera_stream.h
@@ -13,7 +13,9 @@
 
 #include <hardware/camera3.h>
 
+#include <libcamera/base/object.h>
 #include <libcamera/base/signal.h>
+#include <libcamera/base/thread.h>
 
 #include <libcamera/camera.h>
 #include <libcamera/framebuffer.h>
@@ -23,6 +25,7 @@
 
 #include "post_processor.h"
 
+class CameraBuffer;
 class CameraDevice;
 class CameraMetadata;
 
@@ -135,6 +138,27 @@ public:
 	libcamera::Signal<ProcessStatus> processComplete;
 
 private:
+	class PostProcessorWorker : public libcamera::Object
+	{
+	public:
+		PostProcessorWorker(PostProcessor *postProcessor)
+		{
+			postProcessor_ = postProcessor;
+		}
+
+		void process(const libcamera::FrameBuffer *source,
+			     CameraBuffer *destination,
+			     const CameraMetadata &requestMetadata,
+			     CameraMetadata *resultMetadata)
+		{
+			postProcessor_->process(source, destination,
+						requestMetadata, resultMetadata);
+		}
+
+	private:
+		PostProcessor *postProcessor_;
+	};
+
 	void handlePostProcessing(PostProcessor::Status status);
 
 	CameraDevice *const cameraDevice_;
@@ -151,6 +175,9 @@ private:
 	 */
 	std::unique_ptr<std::mutex> mutex_;
 	std::unique_ptr<PostProcessor> postProcessor_;
+	std::unique_ptr<PostProcessorWorker> ppWorker_;
+	std::unique_ptr<CameraBuffer> dest_;
+	std::unique_ptr<libcamera::Thread> thread_;
 };
 
 #endif /* __ANDROID_CAMERA_STREAM__ */
-- 
2.31.1