[PATCH] pipeline: simple: Consider output sizes when choosing pipe config

[Milan Zamazal]

Hi Robert,

thank you for the patch.

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

> In order to avoid having to adjust the size further down below which
> again can break user assumptions. Notably, without this the capture size
> of 1920x1080 gets adjusted to 1912x1080 when used with the swISP using a
> bayer pattern width of 4, breaking users like Gstreamer down the line.
>
> Closes https://bugs.libcamera.org/show_bug.cgi?id=236
>
> Signed-off-by: Robert Mader <robert.mader at collabora.com>
>
> ---
>
> I'm not really sure if this is the correct approach, but sending it out
> already for feedback. So far this gives me promissing results on tested
> devices.
> ---
>  src/libcamera/pipeline/simple/simple.cpp | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 3ddce71d..2d185b90 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -1048,7 +1048,8 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()
>  		const Size &size = pipeConfig->captureSize;
>  
>  		if (size.width >= maxStreamSize.width &&
> -		    size.height >= maxStreamSize.height) {
> +		    size.height >= maxStreamSize.height &&
> +		    pipeConfig->outputSizes.contains(maxStreamSize)) {

Some more explanation is needed.  The original code simply selects the
smallest size that can cover the output.  Now with this change, what if
`size' is sufficiently high but outputSizes doesn't contain exactly
maxStreamSize but it contains some larger value?  And what happens if
software ISP is not enabled for the pipeline?

>  			if (!pipeConfig_ || size < pipeConfig_->captureSize)
>  				pipeConfig_ = pipeConfig;
>  		}