[v1] libcamera: pipeline: virtual: Make copy of request's buffer map
diff mbox series

Message ID 20260514150046.503654-1-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • [v1] libcamera: pipeline: virtual: Make copy of request's buffer map
Related show

Commit Message

Barnabás Pőcze May 14, 2026, 3 p.m. UTC
A `Request` must not be accessed by a pipeline handler after it has called
`completeRequest()` as that call essentially transfers the request back
to the application.

The change that moved frame generation to a separate thread violated this
requirement. After the last buffer of a request is completed, the loop in
`VirtualCameraData::processRequest()` will reference the buffer map in
the loop condition. This is problematic because concurrently the request
will be completed, then reused or destroyed. This was mostly hidden by
the fact that most application use `ReuseBuffers`, so the buffer map
nodes do not change.

Fix that by making a copy of the (stream, buffer) pairs locally.

Fixes: 6c251ae3ef0e ("libcamera: pipeline: virtual: Move image generation to separate thread")
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 src/libcamera/pipeline/virtual/virtual.cpp | 39 ++++++++++++----------
 1 file changed, 22 insertions(+), 17 deletions(-)

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp
index 81d2dddab..6e0cb6478 100644
--- a/src/libcamera/pipeline/virtual/virtual.cpp
+++ b/src/libcamera/pipeline/virtual/virtual.cpp
@@ -130,33 +130,38 @@  VirtualCameraData::VirtualCameraData(PipelineHandler *pipe,
 
 void VirtualCameraData::processRequest(Request *request)
 {
+	std::array<std::pair<StreamConfig *, FrameBuffer *>, kMaxStream> buffers;
+	size_t bufferCount = 0;
+
 	for (const auto &[stream, buffer] : request->buffers()) {
 		bool found = false;
-		/* map buffer and fill test patterns */
 		for (auto &streamConfig : streamConfigs_) {
 			if (stream == &streamConfig.stream) {
-				FrameMetadata &fmd = buffer->_d()->metadata();
-
-				fmd.status = FrameMetadata::Status::FrameSuccess;
-				fmd.sequence = streamConfig.seq++;
-				fmd.timestamp = currentTimestamp();
-
-				Span<const FrameBuffer::Plane> planes = buffer->planes();
-				for (const auto [i, p] : utils::enumerate(planes))
-					fmd.planes()[i].bytesused = p.length;
-
+				buffers[bufferCount++] = { &streamConfig, buffer };
 				found = true;
-
-				if (streamConfig.frameGenerator->generateFrame(
-					    stream->configuration().size, buffer))
-					fmd.status = FrameMetadata::Status::FrameError;
-
-				bufferCompleted.emit(buffer);
 				break;
 			}
 		}
 		ASSERT(found);
 	}
+
+	for (size_t i = 0; i < bufferCount; i++) {
+		const auto &[stream, buffer] = buffers[i];
+		FrameMetadata &fmd = buffer->_d()->metadata();
+
+		fmd.status = FrameMetadata::Status::FrameSuccess;
+		fmd.sequence = stream->seq++;
+		fmd.timestamp = currentTimestamp();
+
+		Span<const FrameBuffer::Plane> planes = buffer->planes();
+		for (const auto [j, p] : utils::enumerate(planes))
+			fmd.planes()[j].bytesused = p.length;
+
+		if (stream->frameGenerator->generateFrame(stream->stream.configuration().size, buffer))
+			fmd.status = FrameMetadata::Status::FrameError;
+
+		bufferCompleted.emit(buffer);
+	}
 }
 
 VirtualCameraConfiguration::VirtualCameraConfiguration(VirtualCameraData *data)