From 68627c5563425579da2800acf423483854126c88 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= <pobrn@protonmail.com>
Date: Thu, 7 Aug 2025 15:47:02 +0200
Subject: [PATCH] spa: libcamera: source: remove `impl::pendingRequests`

The handling of `impl::pendingRequests` is a bit problematic because,
for example, during startup, if `spa_libcamera_buffer_recycle()` observes
that `impl::active == false`, then it will try to append to `impl::pendingRequests`,
which is being iterated in the main thread in `spa_libcamera_stream_on()`.
That is not allowed on an `std::deque`.

So instead remove it altogether, and simply queue all requests when starting.
After `libcamera::Camera::stop()` returns, every request is guaranteed not
to be used by libcamera, and they can be freely reused, so this is safe to do.

This also removes the need for calling `spa_libcamera_buffer_recycle()` when
the buffers are set/unset on a port since that function no longer changes
anything apart from updating `buffer::flags` but `spa_libcamera_alloc_buffers()`
is modified appropriately to take care of that. And in the case of
`spa_libcamera_clear_buffers()` clearing the flag is not required because the
next `spa_libcamera_alloc_buffers()` call will reset reset the flags.
---
 spa/plugins/libcamera/libcamera-source.cpp | 29 ++++++----------------
 1 file changed, 7 insertions(+), 22 deletions(-)

diff --git a/spa/plugins/libcamera/libcamera-source.cpp b/spa/plugins/libcamera/libcamera-source.cpp
index 9feb761ad..41fda937b 100644
--- a/spa/plugins/libcamera/libcamera-source.cpp
+++ b/spa/plugins/libcamera/libcamera-source.cpp
@@ -6,7 +6,6 @@
 
 #include <array>
 #include <cstddef>
-#include <deque>
 #include <limits>
 #include <optional>
 #include <type_traits>
@@ -157,7 +156,6 @@ struct impl {
 
 	FrameBufferAllocator *allocator = nullptr;
 	std::vector<std::unique_ptr<libcamera::Request>> requestPool;
-	std::deque<libcamera::Request *> pendingRequests;
 
 	void requestComplete(libcamera::Request *request);
 
@@ -257,10 +255,7 @@ int spa_libcamera_buffer_recycle(struct impl *impl, struct port *port, uint32_t
 		return -EINVAL;
 	}
 	Request *request = impl->requestPool[buffer_id].get();
-	if (!impl->active) {
-		impl->pendingRequests.push_back(request);
-		return 0;
-	} else {
+	if (impl->active) {
 		request->controls().merge(impl->ctrls);
 		impl->ctrls.clear();
 		if ((res = impl->camera->queueRequest(request)) < 0) {
@@ -359,10 +354,6 @@ int spa_libcamera_clear_buffers(struct impl *impl, struct port *port)
 		b = &port->buffers[i];
 		d = b->outbuf->datas;
 
-		if (SPA_FLAG_IS_SET(b->flags, BUFFER_FLAG_OUTSTANDING)) {
-			spa_log_debug(impl->log, "queueing outstanding buffer %p", b);
-			spa_libcamera_buffer_recycle(impl, port, i);
-		}
 		if (SPA_FLAG_IS_SET(b->flags, BUFFER_FLAG_MAPPED)) {
 			munmap(SPA_PTROFF(b->ptr, -d[0].mapoffset, void),
 					d[0].maxsize - d[0].mapoffset);
@@ -371,7 +362,6 @@ int spa_libcamera_clear_buffers(struct impl *impl, struct port *port)
 		d[0].type = SPA_ID_INVALID;
 	}
 
-	impl->pendingRequests.clear();
 	port->n_buffers = 0;
 	port->ring = SPA_RINGBUFFER_INIT();
 
@@ -1151,7 +1141,7 @@ spa_libcamera_alloc_buffers(struct impl *impl, struct port *port,
 		b = &port->buffers[i];
 		b->id = i;
 		b->outbuf = buffers[i];
-		b->flags = BUFFER_FLAG_OUTSTANDING;
+		b->flags = 0;
 		b->h = (struct spa_meta_header*)spa_buffer_find_meta_data(buffers[i], SPA_META_Header, sizeof(*b->h));
 
 		b->videotransform = (struct spa_meta_videotransform*)spa_buffer_find_meta_data(
@@ -1224,8 +1214,6 @@ spa_libcamera_alloc_buffers(struct impl *impl, struct port *port,
 				return -EIO;
 			}
 		}
-
-		spa_libcamera_buffer_recycle(impl, port, i);
 	}
 
 	port->n_buffers = n_buffers;
@@ -1353,11 +1341,12 @@ int spa_libcamera_stream_on(struct impl *impl)
 
 	impl->camera->requestCompleted.connect(impl, &impl::requestComplete);
 
-	for (Request *req : impl->pendingRequests) {
-		if ((res = impl->camera->queueRequest(req)) < 0)
+	for (auto& req : impl->requestPool) {
+		req->reuse(libcamera::Request::ReuseFlag::ReuseBuffers);
+
+		if ((res = impl->camera->queueRequest(req.get())) < 0)
 			goto err_stop_camera;
 	}
-	impl->pendingRequests.clear();
 
 	impl->dll.bw = 0.0;
 	impl->active = true;
@@ -1380,15 +1369,11 @@ int spa_libcamera_stream_off(struct impl *impl)
 	struct port *port = &impl->out_ports[0];
 	int res;
 
-	if (!impl->active) {
-		for (std::unique_ptr<Request> &req : impl->requestPool)
-			req->reuse(libcamera::Request::ReuseFlag::ReuseBuffers);
+	if (!impl->active)
 		return 0;
-	}
 
 	impl->active = false;
 	spa_log_info(impl->log, "stopping camera %s", impl->camera->id().c_str());
-	impl->pendingRequests.clear();
 
 	if ((res = impl->camera->stop()) < 0) {
 		spa_log_warn(impl->log, "error stopping camera %s: %s",