[v1] libcamera: software_isp: Allow limiting output sizes

28434 diff mbox series

There are various scenarios where it can be useful to control the ousput
sizes of the SW-ISP inside libcamera without rebuilding the project.
Examples include:
 - Debugging and testing when one can't fully control the client
 - Forcing well working sizes on devices where the GPU/CPU are not strong
   enough for typically client-picked sizes. This can be especially
   useful on mobile devices

I plan to use this as convenience feature in Nura (formerly postmarketOS),
making it easy to ship device specific configuration files without needing
libcamera-specific downstream changes to the kernel. To me the feature
seems useful enough to upstream it, though.

>cat ~/.config/libcamera/configuration.yaml
>---
>version: 1
>configuration:
>  software_isp:
>    min_output_width: 800
>    min_output_height: 600
>    max_output_width: 1280
>    max_output_height: 720

Signed-off-by: Robert Mader <robert.mader@collabora.com>
---
 .../internal/software_isp/software_isp.h      |  1 +
 src/libcamera/software_isp/software_isp.cpp   | 20 +++++++++++++++++--
 2 files changed, 19 insertions(+), 2 deletions(-)

Hi Robert,

thank you for the patch.

Robert Mader <robert.mader@collabora.com> writes:

> There are various scenarios where it can be useful to control the ousput

s/ousput/output/

> sizes of the SW-ISP inside libcamera without rebuilding the project.
> Examples include:
>  - Debugging and testing when one can't fully control the client
>  - Forcing well working sizes on devices where the GPU/CPU are not strong
>    enough for typically client-picked sizes. This can be especially
>    useful on mobile devices
>
> I plan to use this as convenience feature in Nura (formerly postmarketOS),
> making it easy to ship device specific configuration files without needing
> libcamera-specific downstream changes to the kernel. To me the feature
> seems useful enough to upstream it, though.
>
>>cat ~/.config/libcamera/configuration.yaml
>>---
>>version: 1
>>configuration:
>>  software_isp:
>>    min_output_width: 800
>>    min_output_height: 600
>>    max_output_width: 1280
>>    max_output_height: 720

Please document the new options in
Documentation/runtime_configuration.rst.
>
> Signed-off-by: Robert Mader <robert.mader@collabora.com>
> ---
>  .../internal/software_isp/software_isp.h      |  1 +
>  src/libcamera/software_isp/software_isp.cpp   | 20 +++++++++++++++++--
>  2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
> index b8b24bb1b113..9a14b20723e4 100644
> --- a/include/libcamera/internal/software_isp/software_isp.h
> +++ b/include/libcamera/internal/software_isp/software_isp.h
> @@ -101,6 +101,7 @@ private:
>  	DebayerParams debayerParams_;
>  	DmaBufAllocator dmaHeap_;
>  	bool ccmEnabled_;
> +	SizeRange outputSizeLimits_;
>  
>  	std::unique_ptr<ipa::softisp::IPAProxySoftIsp> ipa_;
>  	std::deque<FrameBuffer *> queuedInputBuffers_;
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index a40976055d63..334ba4ef90b1 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -105,8 +105,21 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
>  	}
>  	stats->statsReady.connect(this, &SoftwareIsp::statsReady);
>  
> -#if HAVE_DEBAYER_EGL
>  	const GlobalConfiguration &configuration = cm._d()->configuration();
> +
> +	unsigned int minOutputWidth =
> +		configuration.option<unsigned int>({ "software_isp", "min_output_width" }).value_or(0);
> +	unsigned int minOutputHeight =
> +		configuration.option<unsigned int>({ "software_isp", "min_output_height" }).value_or(0);
> +	outputSizeLimits_.min = Size(minOutputWidth, minOutputHeight);
> +
> +	unsigned int maxOutputWidth =
> +		configuration.option<unsigned int>({ "software_isp", "max_output_width" }).value_or(std::numeric_limits<unsigned int>::max());
> +	unsigned int maxOutputHeight =
> +		configuration.option<unsigned int>({ "software_isp", "max_output_height" }).value_or(std::numeric_limits<unsigned int>::max());
> +	outputSizeLimits_.max = Size(maxOutputWidth, maxOutputHeight);
> +
> +#if HAVE_DEBAYER_EGL
>  	std::optional<std::string> softISPMode = configuration.option<std::string>({ "software_isp", "mode" });
>  	if (softISPMode) {
>  		if (softISPMode != "gpu" && softISPMode != "cpu") {
> @@ -243,7 +256,10 @@ SizeRange SoftwareIsp::sizes(PixelFormat inputFormat, const Size &inputSize)
>  {
>  	ASSERT(debayer_);
>  
> -	return debayer_->sizes(inputFormat, inputSize);
> +	SizeRange sizes = debayer_->sizes(inputFormat, inputSize);
> +	sizes.min = sizes.min.expandTo(outputSizeLimits_.min).alignUpTo(sizes.hStep, sizes.vStep);
> +	sizes.max = sizes.max.boundTo(outputSizeLimits_.max).alignDownTo(sizes.hStep, sizes.vStep);
> +	return sizes;
>  }
>  
>  /**

Quoting Milan Zamazal (2026-10-12 14:13:14)
> Hi Robert,
> 
> thank you for the patch.
> 
> Robert Mader <robert.mader@collabora.com> writes:
> 
> > There are various scenarios where it can be useful to control the ousput
> 
> s/ousput/output/
> 
> > sizes of the SW-ISP inside libcamera without rebuilding the project.
> > Examples include:
> >  - Debugging and testing when one can't fully control the client
> >  - Forcing well working sizes on devices where the GPU/CPU are not strong
> >    enough for typically client-picked sizes. This can be especially
> >    useful on mobile devices
> >
> > I plan to use this as convenience feature in Nura (formerly postmarketOS),
> > making it easy to ship device specific configuration files without needing
> > libcamera-specific downstream changes to the kernel. To me the feature
> > seems useful enough to upstream it, though.
> >
> >>cat ~/.config/libcamera/configuration.yaml
> >>---
> >>version: 1
> >>configuration:
> >>  software_isp:
> >>    min_output_width: 800
> >>    min_output_height: 600
> >>    max_output_width: 1280
> >>    max_output_height: 720

Do we have any yaml helpers for sizes/rectangles?

    software_isp:
        min: [800, 600]
        max: [1280, 720]

might be more concise if we can read something conveniently.


> 
> Please document the new options in
> Documentation/runtime_configuration.rst.

Indeed, and we're really going to have to start working on a schema
validation too somehow.

--
Kieran

> >
> > Signed-off-by: Robert Mader <robert.mader@collabora.com>
> > ---
> >  .../internal/software_isp/software_isp.h      |  1 +
> >  src/libcamera/software_isp/software_isp.cpp   | 20 +++++++++++++++++--
> >  2 files changed, 19 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
> > index b8b24bb1b113..9a14b20723e4 100644
> > --- a/include/libcamera/internal/software_isp/software_isp.h
> > +++ b/include/libcamera/internal/software_isp/software_isp.h
> > @@ -101,6 +101,7 @@ private:
> >       DebayerParams debayerParams_;
> >       DmaBufAllocator dmaHeap_;
> >       bool ccmEnabled_;
> > +     SizeRange outputSizeLimits_;
> >  
> >       std::unique_ptr<ipa::softisp::IPAProxySoftIsp> ipa_;
> >       std::deque<FrameBuffer *> queuedInputBuffers_;
> > diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> > index a40976055d63..334ba4ef90b1 100644
> > --- a/src/libcamera/software_isp/software_isp.cpp
> > +++ b/src/libcamera/software_isp/software_isp.cpp
> > @@ -105,8 +105,21 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
> >       }
> >       stats->statsReady.connect(this, &SoftwareIsp::statsReady);
> >  
> > -#if HAVE_DEBAYER_EGL
> >       const GlobalConfiguration &configuration = cm._d()->configuration();
> > +
> > +     unsigned int minOutputWidth =
> > +             configuration.option<unsigned int>({ "software_isp", "min_output_width" }).value_or(0);
> > +     unsigned int minOutputHeight =
> > +             configuration.option<unsigned int>({ "software_isp", "min_output_height" }).value_or(0);
> > +     outputSizeLimits_.min = Size(minOutputWidth, minOutputHeight);
> > +
> > +     unsigned int maxOutputWidth =
> > +             configuration.option<unsigned int>({ "software_isp", "max_output_width" }).value_or(std::numeric_limits<unsigned int>::max());
> > +     unsigned int maxOutputHeight =
> > +             configuration.option<unsigned int>({ "software_isp", "max_output_height" }).value_or(std::numeric_limits<unsigned int>::max());
> > +     outputSizeLimits_.max = Size(maxOutputWidth, maxOutputHeight);
> > +
> > +#if HAVE_DEBAYER_EGL
> >       std::optional<std::string> softISPMode = configuration.option<std::string>({ "software_isp", "mode" });
> >       if (softISPMode) {
> >               if (softISPMode != "gpu" && softISPMode != "cpu") {
> > @@ -243,7 +256,10 @@ SizeRange SoftwareIsp::sizes(PixelFormat inputFormat, const Size &inputSize)
> >  {
> >       ASSERT(debayer_);
> >  
> > -     return debayer_->sizes(inputFormat, inputSize);
> > +     SizeRange sizes = debayer_->sizes(inputFormat, inputSize);
> > +     sizes.min = sizes.min.expandTo(outputSizeLimits_.min).alignUpTo(sizes.hStep, sizes.vStep);
> > +     sizes.max = sizes.max.boundTo(outputSizeLimits_.max).alignDownTo(sizes.hStep, sizes.vStep);
> > +     return sizes;
> >  }
> >  
> >  /**
>

Hi

2026. 10. 11. 12:34 keltezéssel, Robert Mader írta:
> There are various scenarios where it can be useful to control the ousput
> sizes of the SW-ISP inside libcamera without rebuilding the project.
> Examples include:
>   - Debugging and testing when one can't fully control the client
>   - Forcing well working sizes on devices where the GPU/CPU are not strong
>     enough for typically client-picked sizes. This can be especially
>     useful on mobile devices
> 
> I plan to use this as convenience feature in Nura (formerly postmarketOS),
> making it easy to ship device specific configuration files without needing
> libcamera-specific downstream changes to the kernel. To me the feature
> seems useful enough to upstream it, though.
> 
>> cat ~/.config/libcamera/configuration.yaml
>> ---
>> version: 1
>> configuration:
>>   software_isp:
>>     min_output_width: 800
>>     min_output_height: 600
>>     max_output_width: 1280
>>     max_output_height: 720
> 
> Signed-off-by: Robert Mader <robert.mader@collabora.com>
> ---

Unfortunately I'm not entirely convinced by this as is. Firstly,
the configuration should probably be something like this:

   output_size:
     min: [1, 2]
     max: [3, 4]

as suggested already.

Secondly, I'm not a fan of the idea itself in the first place. Although
I suppose I understand the need for enforcing a system-wide policy, but
even then, I have two concerns:

   1. the configuration should most likely be on a per-device basis;
   2. for many of the common configuration parameters (like size),
      these constraints could probably be handled in common code;

So wrt. (1), the first thing that comes to mind, wouldn't the tuning file
be a better place for such system-wide policy?


>   .../internal/software_isp/software_isp.h      |  1 +
>   src/libcamera/software_isp/software_isp.cpp   | 20 +++++++++++++++++--
>   2 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
> index b8b24bb1b113..9a14b20723e4 100644
> --- a/include/libcamera/internal/software_isp/software_isp.h
> +++ b/include/libcamera/internal/software_isp/software_isp.h
> @@ -101,6 +101,7 @@ private:
>   	DebayerParams debayerParams_;
>   	DmaBufAllocator dmaHeap_;
>   	bool ccmEnabled_;
> +	SizeRange outputSizeLimits_;
>   
>   	std::unique_ptr<ipa::softisp::IPAProxySoftIsp> ipa_;
>   	std::deque<FrameBuffer *> queuedInputBuffers_;
> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index a40976055d63..334ba4ef90b1 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -105,8 +105,21 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
>   	}
>   	stats->statsReady.connect(this, &SoftwareIsp::statsReady);
>   
> -#if HAVE_DEBAYER_EGL
>   	const GlobalConfiguration &configuration = cm._d()->configuration();
> +
> +	unsigned int minOutputWidth =
> +		configuration.option<unsigned int>({ "software_isp", "min_output_width" }).value_or(0);
> +	unsigned int minOutputHeight =
> +		configuration.option<unsigned int>({ "software_isp", "min_output_height" }).value_or(0);
> +	outputSizeLimits_.min = Size(minOutputWidth, minOutputHeight);
> +
> +	unsigned int maxOutputWidth =
> +		configuration.option<unsigned int>({ "software_isp", "max_output_width" }).value_or(std::numeric_limits<unsigned int>::max());
> +	unsigned int maxOutputHeight =
> +		configuration.option<unsigned int>({ "software_isp", "max_output_height" }).value_or(std::numeric_limits<unsigned int>::max());
> +	outputSizeLimits_.max = Size(maxOutputWidth, maxOutputHeight);
> +
> +#if HAVE_DEBAYER_EGL
>   	std::optional<std::string> softISPMode = configuration.option<std::string>({ "software_isp", "mode" });
>   	if (softISPMode) {
>   		if (softISPMode != "gpu" && softISPMode != "cpu") {
> @@ -243,7 +256,10 @@ SizeRange SoftwareIsp::sizes(PixelFormat inputFormat, const Size &inputSize)
>   {
>   	ASSERT(debayer_);
>   
> -	return debayer_->sizes(inputFormat, inputSize);
> +	SizeRange sizes = debayer_->sizes(inputFormat, inputSize);
> +	sizes.min = sizes.min.expandTo(outputSizeLimits_.min).alignUpTo(sizes.hStep, sizes.vStep);
> +	sizes.max = sizes.max.boundTo(outputSizeLimits_.max).alignDownTo(sizes.hStep, sizes.vStep);
> +	return sizes;
>   }
>   
>   /**