From 6928522db6fab5a1cb9f05ce1ba8868a38de7781 Mon Sep 17 00:00:00 2001
From: Paul Elder <paul.elder@ideasonboard.com>
Date: Tue, 13 May 2025 15:37:51 +0200
Subject: [PATCH] pipeline: simple: Fix matching with empty media graphs

The match() function currently reports that it is not possible to create
any cameras if it encounters an empty media graph.

Fix this by looping over all media graphs and only returning false when
all of them fail to create a camera.

It is worth noting that an issue does exist when on a partial match that
ends in an invalid match, any media devices that were acquired will stay
acquired. This is not a new issue though, as any acquired media devices
in general are not released until pipeline handler deconstruction. This
requires a rework of how we do matching and pipeline handler
construction, so it is captured in a comment.

In the meantime, this fix fixes a problem without increasing the net
number of problems.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Tested-by: Hui Fang <hui.fang@nxp.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/libcamera/pipeline/simple/simple.cpp | 62 +++++++++++++++++-------
 1 file changed, 45 insertions(+), 17 deletions(-)

diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index efb07051..4323472e 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -427,6 +427,9 @@ private:
 		return static_cast<SimpleCameraData *>(camera->_d());
 	}
 
+	bool matchDevice(MediaDevice *media, const SimplePipelineInfo &info,
+			 DeviceEnumerator *enumerator);
+
 	std::vector<MediaEntity *> locateSensors(MediaDevice *media);
 	static int resetRoutingTable(V4L2Subdevice *subdev);
 
@@ -1660,25 +1663,13 @@ int SimplePipelineHandler::resetRoutingTable(V4L2Subdevice *subdev)
 	return 0;
 }
 
-bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
+bool SimplePipelineHandler::matchDevice(MediaDevice *media,
+					const SimplePipelineInfo &info,
+					DeviceEnumerator *enumerator)
 {
-	const SimplePipelineInfo *info = nullptr;
 	unsigned int numStreams = 1;
-	MediaDevice *media;
 
-	for (const SimplePipelineInfo &inf : supportedDevices) {
-		DeviceMatch dm(inf.driver);
-		media = acquireMediaDevice(enumerator, dm);
-		if (media) {
-			info = &inf;
-			break;
-		}
-	}
-
-	if (!media)
-		return false;
-
-	for (const auto &[name, streams] : info->converters) {
+	for (const auto &[name, streams] : info.converters) {
 		DeviceMatch converterMatch(name);
 		converter_ = acquireMediaDevice(enumerator, converterMatch);
 		if (converter_) {
@@ -1687,7 +1678,7 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
 		}
 	}
 
-	swIspEnabled_ = info->swIspEnabled;
+	swIspEnabled_ = info.swIspEnabled;
 
 	/* Locate the sensors. */
 	std::vector<MediaEntity *> sensors = locateSensors(media);
@@ -1806,6 +1797,43 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
 	return registered;
 }
 
+bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
+{
+	MediaDevice *media;
+
+	for (const SimplePipelineInfo &inf : supportedDevices) {
+		DeviceMatch dm(inf.driver);
+		while ((media = acquireMediaDevice(enumerator, dm))) {
+			/*
+			 * If match succeeds, return true to let match() be
+			 * called again on a new instance of the pipeline
+			 * handler. Otherwise keep looping until we do
+			 * successfully match one (or run out).
+			 */
+			if (matchDevice(media, inf, enumerator)) {
+				LOG(SimplePipeline, Debug)
+					<< "Matched on device: "
+					<< media->deviceNode();
+				return true;
+			}
+
+			/*
+			 * \todo We need to clear the list of media devices
+			 * that we've already acquired in the event that we
+			 * fail to create a camera. This requires a rework of
+			 * DeviceEnumerator, or even how we create pipelines
+			 * handlers. This is because at the moment acquired
+			 * media devices are only released on pipeline handler
+			 * deconstruction, and if we release them any earlier
+			 * then DeviceEnumerator::search() will keep returning
+			 * the same media devices.
+			 */
+		}
+	}
+
+	return false;
+}
+
 V4L2VideoDevice *SimplePipelineHandler::video(const MediaEntity *entity)
 {
 	auto iter = entities_.find(entity);