[libcamera-devel] [PATCH v2 4/5] libcamera: pipeline: simple: Add internal request queue

[Nícolas F. R. A. Prado]

Add an internal queue that stores requests until there are internal
buffers and V4L2 buffer slots available. This avoids the need to cancel
requests when there is a shortage of said buffers.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado at collabora.com>

---
I wasn't able to test this patch as I don't have any of the target
hardware.

Changes in v2:
- New

 src/libcamera/pipeline/simple/simple.cpp | 101 ++++++++++++++++++-----
 1 file changed, 81 insertions(+), 20 deletions(-)

diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index 6deba5d7dd61..e9abe5e291f4 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -167,6 +167,9 @@ public:
 	int setupFormats(V4L2SubdeviceFormat *format,
 			 V4L2Subdevice::Whence whence);
 
+	void queuePendingRequests();
+	void cancelPendingRequests();
+
 	unsigned int streamIndex(const Stream *stream) const
 	{
 		return stream - &streams_.front();
@@ -198,6 +201,10 @@ public:
 	std::queue<std::map<unsigned int, FrameBuffer *>> converterQueue_;
 
 	const SimplePipelineInfo *deviceInfo;
+
+	std::queue<Request *> pendingRequests_;
+
+	unsigned int avaliableBufferSlotCount_;
 };
 
 class SimpleCameraConfiguration : public CameraConfiguration
@@ -546,6 +553,66 @@ int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,
 	return 0;
 }
 
+void SimpleCameraData::queuePendingRequests()
+{
+	while (!pendingRequests_.empty() && avaliableBufferSlotCount_) {
+		Request *request = pendingRequests_.front();
+
+		if (useConverter_) {
+			/*
+			 * If conversion is needed, push the buffer(s) to the
+			 * converter queue, it will be handed to the converter
+			 * in the capture completion handler.
+			 */
+			std::map<unsigned int, FrameBuffer *> buffers;
+
+			for (auto &[stream, buffer] : request->buffers())
+				buffers.emplace(streamIndex(stream), buffer);
+
+			converterQueue_.push(std::move(buffers));
+		} else {
+			/*
+			 * Since we're not using a converter we can assume
+			 * there's a single stream.
+			 */
+			FrameBuffer *buffer = request->buffers().at(0);
+
+			int ret = video_->queueBuffer(buffer);
+			if (ret < 0) {
+				LOG(SimplePipeline, Error)
+					<< "Failed to queue buffer with error "
+					<< ret << ". Cancelling buffer.";
+				buffer->cancel();
+				pipe()->completeBuffer(request, buffer);
+				pipe()->completeRequest(request);
+				pendingRequests_.pop();
+
+				continue;
+			}
+		}
+
+		avaliableBufferSlotCount_--;
+
+		pendingRequests_.pop();
+	}
+}
+
+void SimpleCameraData::cancelPendingRequests()
+{
+	while (!pendingRequests_.empty()) {
+		Request *request = pendingRequests_.front();
+
+		for (auto const &it : request->buffers()) {
+			it.second->cancel();
+			pipe()->completeBuffer(request, it.second);
+		}
+
+		pipe()->completeRequest(request);
+
+		pendingRequests_.pop();
+	}
+}
+
 /* -----------------------------------------------------------------------------
  * Camera Configuration
  */
@@ -852,6 +919,8 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL
 	if (ret < 0)
 		return ret;
 
+	data->avaliableBufferSlotCount_ = kSimpleBufferSlotCount;
+
 	ret = video->streamOn();
 	if (ret < 0) {
 		stop(camera);
@@ -885,6 +954,7 @@ void SimplePipelineHandler::stop(Camera *camera)
 		converter_->stop();
 
 	video->streamOff();
+	data->cancelPendingRequests();
 	video->releaseBuffers();
 
 	data->converterBuffers_.clear();
@@ -894,27 +964,9 @@ void SimplePipelineHandler::stop(Camera *camera)
 int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)
 {
 	SimpleCameraData *data = cameraData(camera);
-	int ret;
-
-	std::map<unsigned int, FrameBuffer *> buffers;
-
-	for (auto &[stream, buffer] : request->buffers()) {
-		/*
-		 * If conversion is needed, push the buffer to the converter
-		 * queue, it will be handed to the converter in the capture
-		 * completion handler.
-		 */
-		if (data->useConverter_) {
-			buffers.emplace(data->streamIndex(stream), buffer);
-		} else {
-			ret = data->video_->queueBuffer(buffer);
-			if (ret < 0)
-				return ret;
-		}
-	}
 
-	if (data->useConverter_)
-		data->converterQueue_.push(std::move(buffers));
+	data->pendingRequests_.push(request);
+	data->queuePendingRequests();
 
 	return 0;
 }
@@ -1151,6 +1203,8 @@ void SimplePipelineHandler::bufferReady(FrameBuffer *buffer)
 			Request *request = buffer->request();
 			completeBuffer(request, buffer);
 			completeRequest(request);
+			data->avaliableBufferSlotCount_++;
+			data->queuePendingRequests();
 			return;
 		}
 
@@ -1221,6 +1275,9 @@ void SimplePipelineHandler::bufferReady(FrameBuffer *buffer)
 	/* Otherwise simply complete the request. */
 	completeBuffer(request, buffer);
 	completeRequest(request);
+
+	data->avaliableBufferSlotCount_++;
+	data->queuePendingRequests();
 }
 
 void SimplePipelineHandler::converterInputDone(FrameBuffer *buffer)
@@ -1235,11 +1292,15 @@ void SimplePipelineHandler::converterInputDone(FrameBuffer *buffer)
 void SimplePipelineHandler::converterOutputDone(FrameBuffer *buffer)
 {
 	ASSERT(activeCamera_);
+	SimpleCameraData *data = cameraData(activeCamera_);
 
 	/* Complete the buffer and the request. */
 	Request *request = buffer->request();
 	if (completeBuffer(request, buffer))
 		completeRequest(request);
+
+	data->avaliableBufferSlotCount_++;
+	data->queuePendingRequests();
 }
 
 REGISTER_PIPELINE_HANDLER(SimplePipelineHandler)
-- 
2.33.0