[RFC,v1,13/27] libcamera: pipeline: virtual: Make copy of request's buffer map
diff mbox series

Message ID 20260618123844.656396-14-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • Misc. changes before request-buffer split
Related show

Commit Message

Barnabás Pőcze June 18, 2026, 12:38 p.m. UTC
The change that moved frame generation to a separate thread introduced
a race condition. 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 | 40 +++++++++++++---------
 1 file changed, 23 insertions(+), 17 deletions(-)

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp
index 81d2dddab8..a7cd76e226 100644
--- a/src/libcamera/pipeline/virtual/virtual.cpp
+++ b/src/libcamera/pipeline/virtual/virtual.cpp
@@ -130,33 +130,39 @@  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)