[v1] libcamera: pipeline: simple: Delay stream count for software ISP
diff mbox series

Message ID 20260821094147.2251802-1-alexi.birlinger@nxp.com
State New
Headers show
Series
  • [v1] libcamera: pipeline: simple: Delay stream count for software ISP
Related show

Commit Message

Alexi Birlinger Aug. 21, 2026, 9:41 a.m. UTC
When using the software ISP, the simple pipeline handler exposes 2
streams. However, the configuration file can disable the software ISP,
and the current code sets the stream count before reading the
configuration file, resulting in an incorrect number of streams.

Move the stream count assignment after the configuration file is
processed to ensure the correct number of streams is exposed based on
the actual software ISP usage.

Signed-off-by: Alexi Birlinger <alexi.birlinger@nxp.com>
---
 src/libcamera/pipeline/simple/simple.cpp | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

Comments

Barnabás Pőcze Aug. 21, 2026, 10:03 a.m. UTC | #1
2026. 08. 21. 11:41 keltezéssel, Alexi Birlinger írta:
> When using the software ISP, the simple pipeline handler exposes 2
> streams. However, the configuration file can disable the software ISP,
> and the current code sets the stream count before reading the
> configuration file, resulting in an incorrect number of streams.
> 
> Move the stream count assignment after the configuration file is
> processed to ensure the correct number of streams is exposed based on
> the actual software ISP usage.
> 
> Signed-off-by: Alexi Birlinger <alexi.birlinger@nxp.com>
> ---
>   src/libcamera/pipeline/simple/simple.cpp | 20 ++++++++++----------
>   1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 02a98f594..e20f08b67 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -1880,16 +1880,6 @@ bool SimplePipelineHandler::matchDevice(std::shared_ptr<MediaDevice> media,
>   		}
>   	}
>   
> -	if (info.swIspEnabled) {
> -		/*
> -		 * When the software ISP is enabled, the simple pipeline handler
> -		 * exposes the raw stream, giving a total of two streams. This
> -		 * is mutually exclusive with the presence of a converter.
> -		 */
> -		ASSERT(!converter_);
> -		numStreams = 2;
> -	}
> -
>   	swIspEnabled_ = info.swIspEnabled;
>   	const GlobalConfiguration &configuration = cameraManager()->_d()->configuration();
>   	for (const ValueNode &entry :
> @@ -1905,6 +1895,16 @@ bool SimplePipelineHandler::matchDevice(std::shared_ptr<MediaDevice> media,
>   		}
>   	}
>   
> +	if (swIspEnabled_) {
> +		/*
> +		 * When the software ISP is enabled, the simple pipeline handler
> +		 * exposes the raw stream, giving a total of two streams. This
> +		 * is mutually exclusive with the presence of a converter.
> +		 */
> +		ASSERT(!converter_);

I think this assertion needs more consideration. Especially if the software-isp can
be controlled by user configuration. But even if not, e.g `supportedDevices[1]`
specifies both converters and software-isp. I wonder how that is supposed to work.
(CC Milan Zamazal)

In any case, the change itself looks ok to me.

Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

And I think

   Fixes: a19f72569558 ("libcamera: simple: Set the number of software ISP streams to 2")

could be added.

> +		numStreams = 2;
> +	}
> +
>   	/* Locate the sensors. */
>   	std::vector<MediaEntity *> sensors = locateSensors(media.get());
>   	if (sensors.empty()) {

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index 02a98f594..e20f08b67 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -1880,16 +1880,6 @@  bool SimplePipelineHandler::matchDevice(std::shared_ptr<MediaDevice> media,
 		}
 	}
 
-	if (info.swIspEnabled) {
-		/*
-		 * When the software ISP is enabled, the simple pipeline handler
-		 * exposes the raw stream, giving a total of two streams. This
-		 * is mutually exclusive with the presence of a converter.
-		 */
-		ASSERT(!converter_);
-		numStreams = 2;
-	}
-
 	swIspEnabled_ = info.swIspEnabled;
 	const GlobalConfiguration &configuration = cameraManager()->_d()->configuration();
 	for (const ValueNode &entry :
@@ -1905,6 +1895,16 @@  bool SimplePipelineHandler::matchDevice(std::shared_ptr<MediaDevice> media,
 		}
 	}
 
+	if (swIspEnabled_) {
+		/*
+		 * When the software ISP is enabled, the simple pipeline handler
+		 * exposes the raw stream, giving a total of two streams. This
+		 * is mutually exclusive with the presence of a converter.
+		 */
+		ASSERT(!converter_);
+		numStreams = 2;
+	}
+
 	/* Locate the sensors. */
 	std::vector<MediaEntity *> sensors = locateSensors(media.get());
 	if (sensors.empty()) {