[libcamera-devel] [PATCH] libcamera: ipu3: Use std::max() instead of expandTo() to get the max resolution

[Laurent Pinchart]

Hi Han-Lin,

Thank you for the patch.

On Thu, Aug 04, 2022 at 08:06:20PM +0800, Han-Lin Chen via libcamera-devel wrote:
> Using Size::expandTo() to find the max resolution might generate a non-existent
> resolution. For example, when application request streams for 1920x1080 and
> 1600x1200, the max resolution will be wrongly 1920x1200 and fails the
> configuration.
> 
> Bug: https://bugs.libcamera.org/show_bug.cgi?id=139
> Signed-off-by: Han-Lin Chen <hanlinchen at chromium.org>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 75231156..335b6c94 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -254,7 +254,7 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
>  			rawSize.expandTo(cfg.size);

Doesn't this need to be addressed too ?

>  		} else {
>  			yuvCount++;
> -			maxYuvSize.expandTo(cfg.size);
> +			maxYuvSize = std::max(maxYuvSize, cfg.size);

Is this enough though ? In the example above, if the user requests two
streams, in 1920x1080 and 1600x1200 resolutions, shouldn't we pick a
size from the sensor that is larger than both, and then crop at the
output ? Looking at the rest of the validate() function, the size
selection seems quite fragile.

>  		}
>  	}
>  

-- 
Regards,

Laurent Pinchart