| Message ID | 20260925151816.909725-1-alexi.birlinger@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi all, Forget to add v2 changes, sorry. Changes in v2: - Reject multiple processed streams in validate() On 9/25/26 17:18, Alexi Birlinger wrote: > The software ISP is defined with 2 streams. However, it only supports > a single processed stream, with an optional raw stream. > Return a null configuration early in generateConfiguration() when multiple > processed streams are requested while the software ISP is enabled. > > Additionally, adjust validate() to enforce the same restriction, as the user > is allowed to modify the configuration after it has been generated. > > Signed-off-by: Alexi Birlinger <alexi.birlinger@nxp.com> > --- > src/libcamera/pipeline/simple/simple.cpp | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index 35c29ceca..27eb728f3 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -1183,6 +1183,13 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() > return Invalid; > } > > + unsigned int processedCount = config_.size() - rawCount; > + if (data_->pipe()->swIspEnabled() && processedCount > 1) { > + LOG(SimplePipeline, Error) > + << "Software ISP does not support multiple processed streams"; > + return Invalid; > + } > + > /* > * Find the best configuration for the pipeline using a heuristic. > * First select the pixel format based on the raw streams followed by > @@ -1427,6 +1434,11 @@ SimplePipelineHandler::generateConfiguration(Camera *camera, std::span<const Str > } > rawRequested = true; > } else { > + if (swIspEnabled_ && processedRequested) { > + LOG(SimplePipeline, Error) > + << "Software ISP can't capture multiple processed streams"; > + return nullptr; > + } > processedRequested = true; > } >
Alexi Birlinger <alexi.birlinger@nxp.com> writes: > The software ISP is defined with 2 streams. However, it only supports > a single processed stream, with an optional raw stream. > Return a null configuration early in generateConfiguration() when multiple > processed streams are requested while the software ISP is enabled. > > Additionally, adjust validate() to enforce the same restriction, as the user > is allowed to modify the configuration after it has been generated. > > Signed-off-by: Alexi Birlinger <alexi.birlinger@nxp.com> Reviewed-by: Milan Zamazal <mzamazal@redhat.com> > --- > src/libcamera/pipeline/simple/simple.cpp | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index 35c29ceca..27eb728f3 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -1183,6 +1183,13 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() > return Invalid; > } > > + unsigned int processedCount = config_.size() - rawCount; > + if (data_->pipe()->swIspEnabled() && processedCount > 1) { > + LOG(SimplePipeline, Error) > + << "Software ISP does not support multiple processed streams"; > + return Invalid; > + } > + > /* > * Find the best configuration for the pipeline using a heuristic. > * First select the pixel format based on the raw streams followed by > @@ -1427,6 +1434,11 @@ SimplePipelineHandler::generateConfiguration(Camera *camera, std::span<const Str > } > rawRequested = true; > } else { > + if (swIspEnabled_ && processedRequested) { > + LOG(SimplePipeline, Error) > + << "Software ISP can't capture multiple processed streams"; > + return nullptr; > + } > processedRequested = true; > }
2026. 09. 25. 17:18 keltezéssel, Alexi Birlinger írta: > The software ISP is defined with 2 streams. However, it only supports > a single processed stream, with an optional raw stream. > Return a null configuration early in generateConfiguration() when multiple > processed streams are requested while the software ISP is enabled. > > Additionally, adjust validate() to enforce the same restriction, as the user > is allowed to modify the configuration after it has been generated. > > Signed-off-by: Alexi Birlinger <alexi.birlinger@nxp.com> > --- > src/libcamera/pipeline/simple/simple.cpp | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index 35c29ceca..27eb728f3 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -1183,6 +1183,13 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() > return Invalid; > } > > + unsigned int processedCount = config_.size() - rawCount; > + if (data_->pipe()->swIspEnabled() && processedCount > 1) { > + LOG(SimplePipeline, Error) > + << "Software ISP does not support multiple processed streams"; > + return Invalid; > + } > + > /* > * Find the best configuration for the pipeline using a heuristic. > * First select the pixel format based on the raw streams followed by > @@ -1427,6 +1434,11 @@ SimplePipelineHandler::generateConfiguration(Camera *camera, std::span<const Str > } > rawRequested = true; > } else { > + if (swIspEnabled_ && processedRequested) { > + LOG(SimplePipeline, Error) > + << "Software ISP can't capture multiple processed streams"; > + return nullptr; > + } > processedRequested = true; The changes look ok to me. But I wonder if we should adjust `generateConfiguration()` as follows: if (config->validate() == CameraConfiguration::Invalid) return nullptr; that could for example avoid the need to repeat this check twice (and possibly others). Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > } >
Barnabás Pőcze <barnabas.pocze@ideasonboard.com> writes: > 2026. 09. 25. 17:18 keltezéssel, Alexi Birlinger írta: >> The software ISP is defined with 2 streams. However, it only supports >> a single processed stream, with an optional raw stream. >> Return a null configuration early in generateConfiguration() when multiple >> processed streams are requested while the software ISP is enabled. >> Additionally, adjust validate() to enforce the same restriction, as the user >> is allowed to modify the configuration after it has been generated. >> Signed-off-by: Alexi Birlinger <alexi.birlinger@nxp.com> >> --- >> src/libcamera/pipeline/simple/simple.cpp | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> diff --git a/src/libcamera/pipeline/simple/simple.cpp >> b/src/libcamera/pipeline/simple/simple.cpp >> index 35c29ceca..27eb728f3 100644 >> --- a/src/libcamera/pipeline/simple/simple.cpp >> +++ b/src/libcamera/pipeline/simple/simple.cpp >> @@ -1183,6 +1183,13 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() >> return Invalid; >> } >> + unsigned int processedCount = config_.size() - rawCount; >> + if (data_->pipe()->swIspEnabled() && processedCount > 1) { >> + LOG(SimplePipeline, Error) >> + << "Software ISP does not support multiple processed streams"; >> + return Invalid; >> + } >> + >> /* >> * Find the best configuration for the pipeline using a heuristic. >> * First select the pixel format based on the raw streams followed by >> @@ -1427,6 +1434,11 @@ SimplePipelineHandler::generateConfiguration(Camera *camera, std::span<const Str >> } >> rawRequested = true; >> } else { >> + if (swIspEnabled_ && processedRequested) { >> + LOG(SimplePipeline, Error) >> + << "Software ISP can't capture multiple processed streams"; >> + return nullptr; >> + } >> processedRequested = true; > > The changes look ok to me. But I wonder if we should adjust `generateConfiguration()` as follows: > > if (config->validate() == CameraConfiguration::Invalid) > return nullptr; > > that could for example avoid the need to repeat this check twice (and possibly others). Probably yes, in a separate patch. I can't see anything that would prevent it, and another reason is that if validate() fails (which is supposed to never happen, but what guarantees that), we ignore the failure. > Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > >> } >>
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 35c29ceca..27eb728f3 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -1183,6 +1183,13 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() return Invalid; } + unsigned int processedCount = config_.size() - rawCount; + if (data_->pipe()->swIspEnabled() && processedCount > 1) { + LOG(SimplePipeline, Error) + << "Software ISP does not support multiple processed streams"; + return Invalid; + } + /* * Find the best configuration for the pipeline using a heuristic. * First select the pixel format based on the raw streams followed by @@ -1427,6 +1434,11 @@ SimplePipelineHandler::generateConfiguration(Camera *camera, std::span<const Str } rawRequested = true; } else { + if (swIspEnabled_ && processedRequested) { + LOG(SimplePipeline, Error) + << "Software ISP can't capture multiple processed streams"; + return nullptr; + } processedRequested = true; }
The software ISP is defined with 2 streams. However, it only supports a single processed stream, with an optional raw stream. Return a null configuration early in generateConfiguration() when multiple processed streams are requested while the software ISP is enabled. Additionally, adjust validate() to enforce the same restriction, as the user is allowed to modify the configuration after it has been generated. Signed-off-by: Alexi Birlinger <alexi.birlinger@nxp.com> --- src/libcamera/pipeline/simple/simple.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+)