[libcamera-devel] [PATCH 08/20] libcamera: pipeline: simple: converter: Decouple input and output completion

[Laurent Pinchart]

The SimpleConverter API signals completion of input and output buffer
pairs. This unnecessarily delays requeueing the input buffer to the
video capture queue until the output buffer completes, and also delays
signalling request completion until the input buffer completes. While
this shouldn't cause large delays in practice, it will also not scale
when multi-stream support will be added to the converter class.

To address the current issue and prepare for the future, decouple
signalling of input and output buffers completion.

Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
---
 src/libcamera/pipeline/simple/converter.cpp | 24 +++++--------------
 src/libcamera/pipeline/simple/converter.h   | 11 ++++-----
 src/libcamera/pipeline/simple/simple.cpp    | 26 +++++++++++++--------
 3 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp
index f782fbc63b09..8324baedc198 100644
--- a/src/libcamera/pipeline/simple/converter.cpp
+++ b/src/libcamera/pipeline/simple/converter.cpp
@@ -45,8 +45,8 @@ SimpleConverter::SimpleConverter(MediaDevice *media)
 		return;
 	}
 
-	m2m_->output()->bufferReady.connect(this, &SimpleConverter::outputBufferReady);
-	m2m_->capture()->bufferReady.connect(this, &SimpleConverter::captureBufferReady);
+	m2m_->output()->bufferReady.connect(this, &SimpleConverter::m2mInputBufferReady);
+	m2m_->capture()->bufferReady.connect(this, &SimpleConverter::m2mOutputBufferReady);
 }
 
 std::vector<PixelFormat> SimpleConverter::formats(PixelFormat input)
@@ -247,26 +247,14 @@ int SimpleConverter::queueBuffers(FrameBuffer *input, FrameBuffer *output)
 	return 0;
 }
 
-void SimpleConverter::captureBufferReady(FrameBuffer *buffer)
+void SimpleConverter::m2mInputBufferReady(FrameBuffer *buffer)
 {
-	if (!outputDoneQueue_.empty()) {
-		FrameBuffer *other = outputDoneQueue_.front();
-		outputDoneQueue_.pop();
-		bufferReady.emit(other, buffer);
-	} else {
-		captureDoneQueue_.push(buffer);
-	}
+	inputBufferReady.emit(buffer);
 }
 
-void SimpleConverter::outputBufferReady(FrameBuffer *buffer)
+void SimpleConverter::m2mOutputBufferReady(FrameBuffer *buffer)
 {
-	if (!captureDoneQueue_.empty()) {
-		FrameBuffer *other = captureDoneQueue_.front();
-		captureDoneQueue_.pop();
-		bufferReady.emit(buffer, other);
-	} else {
-		outputDoneQueue_.push(buffer);
-	}
+	outputBufferReady.emit(buffer);
 }
 
 } /* namespace libcamera */
diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h
index 780bfa8f7832..739b24df0200 100644
--- a/src/libcamera/pipeline/simple/converter.h
+++ b/src/libcamera/pipeline/simple/converter.h
@@ -9,7 +9,6 @@
 #define __LIBCAMERA_PIPELINE_SIMPLE_CONVERTER_H__
 
 #include <memory>
-#include <queue>
 #include <tuple>
 #include <vector>
 
@@ -47,17 +46,15 @@ public:
 
 	int queueBuffers(FrameBuffer *input, FrameBuffer *output);
 
-	Signal<FrameBuffer *, FrameBuffer *> bufferReady;
+	Signal<FrameBuffer *> inputBufferReady;
+	Signal<FrameBuffer *> outputBufferReady;
 
 private:
-	void captureBufferReady(FrameBuffer *buffer);
-	void outputBufferReady(FrameBuffer *buffer);
+	void m2mInputBufferReady(FrameBuffer *buffer);
+	void m2mOutputBufferReady(FrameBuffer *buffer);
 
 	std::unique_ptr<V4L2M2MDevice> m2m_;
 
-	std::queue<FrameBuffer *> captureDoneQueue_;
-	std::queue<FrameBuffer *> outputDoneQueue_;
-
 	unsigned int inputBufferCount_;
 	unsigned int outputBufferCount_;
 };
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index 20a4ebca94fd..7f9c57234256 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -144,7 +144,8 @@ private:
 	}
 
 	void bufferReady(FrameBuffer *buffer);
-	void converterDone(FrameBuffer *input, FrameBuffer *output);
+	void converterInputDone(FrameBuffer *buffer);
+	void converterOutputDone(FrameBuffer *buffer);
 
 	MediaDevice *media_;
 	std::map<const MediaEntity *, std::unique_ptr<V4L2VideoDevice>> videos_;
@@ -768,7 +769,8 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
 				<< "Failed to create converter, disabling format conversion";
 			converter_.reset();
 		} else {
-			converter_->bufferReady.connect(this, &SimplePipelineHandler::converterDone);
+			converter_->inputBufferReady.connect(this, &SimplePipelineHandler::converterInputDone);
+			converter_->outputBufferReady.connect(this, &SimplePipelineHandler::converterOutputDone);
 		}
 	}
 
@@ -925,19 +927,23 @@ void SimplePipelineHandler::bufferReady(FrameBuffer *buffer)
 	completeRequest(request);
 }
 
-void SimplePipelineHandler::converterDone(FrameBuffer *input,
-					  FrameBuffer *output)
+void SimplePipelineHandler::converterInputDone(FrameBuffer *buffer)
 {
 	ASSERT(activeCamera_);
 	SimpleCameraData *data = cameraData(activeCamera_);
 
-	/* Complete the request. */
-	Request *request = output->request();
-	completeBuffer(request, output);
-	completeRequest(request);
-
 	/* Queue the input buffer back for capture. */
-	data->video_->queueBuffer(input);
+	data->video_->queueBuffer(buffer);
+}
+
+void SimplePipelineHandler::converterOutputDone(FrameBuffer *buffer)
+{
+	ASSERT(activeCamera_);
+
+	/* Complete the request. */
+	Request *request = buffer->request();
+	completeBuffer(request, buffer);
+	completeRequest(request);
 }
 
 REGISTER_PIPELINE_HANDLER(SimplePipelineHandler)
-- 
Regards,

Laurent Pinchart