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

[Jacopo Mondi]

Hi Han-Lin

On Fri, Aug 12, 2022 at 05:01:03PM +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 | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index f65db3c8..47311953 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -247,6 +247,7 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
>  	 */
>  	unsigned int rawCount = 0;
>  	unsigned int yuvCount = 0;
> +	Size rawRequirement;
>  	Size maxYuvSize;
>  	Size rawSize;
>
> @@ -255,10 +256,11 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
>
>  		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {
>  			rawCount++;
> -			rawSize.expandTo(cfg.size);
> +			rawSize = std::max(rawSize, cfg.size);
>  		} else {
>  			yuvCount++;
> -			maxYuvSize.expandTo(cfg.size);
> +			maxYuvSize = std::max(maxYuvSize, cfg.size);
> +			rawRequirement.expandTo(cfg.size);
>  		}
>  	}
>
> @@ -287,17 +289,17 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
>  	 * The output YUV streams will be limited in size to the maximum frame
>  	 * size requested for the RAW stream, if present.
>  	 *
> -	 * If no raw stream is requested generate a size as large as the maximum
> -	 * requested YUV size aligned to the ImgU constraints and bound by the
> -	 * sensor's maximum resolution. See
> +	 * If no raw stream is requested generate a size as large as the span
> +	 * of all requested YUV size aligned to the ImgU constraints and bound
> +	 * by the sensor's maximum resolution. See
>  	 * https://bugs.libcamera.org/show_bug.cgi?id=32
>  	 */
>  	if (rawSize.isNull())
> -		rawSize = maxYuvSize.expandedTo({ ImgUDevice::kIFMaxCropWidth,
> -						  ImgUDevice::kIFMaxCropHeight })
> -				    .grownBy({ ImgUDevice::kOutputMarginWidth,
> -					       ImgUDevice::kOutputMarginHeight })
> -				    .boundedTo(data_->cio2_.sensor()->resolution());
> +		rawSize = rawRequirement.expandedTo({ ImgUDevice::kIFMaxCropWidth,
> +						      ImgUDevice::kIFMaxCropHeight })
> +				  .grownBy({ ImgUDevice::kOutputMarginWidth,
> +					     ImgUDevice::kOutputMarginHeight })
> +				  .boundedTo(data_->cio2_.sensor()->resolution());

This seems to fix the issue indeed, and maintain the current behaviour
where the RAW stream size limits the YUV output size.

Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

>
>  	cio2Configuration_ = data_->cio2_.generateConfiguration(rawSize);
>  	if (!cio2Configuration_.pixelFormat.isValid())
> --
> 2.37.1.595.g718a3a8f04-goog
>