[libcamera-devel] [PATCH 01/16] libcamera: ipu3: Use the optimal sensor size

[Jean-Michel Hautbois]

Hi Jacopo,

On 27/08/2021 14:07, Jacopo Mondi wrote:
> As reported by commit 7208e70211a6 ("libcamera: ipu3: Always use sensor
> full frame size") the current implementation of the IPU3 pipeline
> handler always use the sensor resolution as the ImgU input frame size in
> order to work around an issue with the ImgU configuration procedure.
> 
> Now that the frame selection policy has been modified in the CIO2Device
> class implementation to comply with the requirements of the ImgU
> configuration script we can remove the workaround and select the most
> opportune sensor size to feed the ImgU with.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 38 ++++++++++++++--------------
>  1 file changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index c287bf86e79a..b321c94e9cb0 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -229,7 +229,16 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
>  		status = Adjusted;
>  	}
>  
> -	/* Validate the requested stream configuration */
> +	/*
> +	 * Validate the requested stream configuration and select the sensor
> +	 * format by collecting the maximum RAW stream width and height and
> +	 * picking the closest larger match.
> +	 *
> +	 * If no RAW stream is requested use the one of the largest YUV stream,
> +	 * plus margin pixels for the IF and BDS rectangle to downscale.
> +	 *
> +	 * \todo Clarify the IF and BDS margins requirements.
> +	 */
>  	unsigned int rawCount = 0;
>  	unsigned int yuvCount = 0;
>  	Size maxYuvSize;
> @@ -240,7 +249,7 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
>  
>  		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {
>  			rawCount++;
> -			rawSize = cfg.size;
> +			rawSize.expandTo(cfg.size);
>  		} else {
>  			yuvCount++;
>  			maxYuvSize.expandTo(cfg.size);
> @@ -269,24 +278,15 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
>  	/*
>  	 * Generate raw configuration from CIO2.
>  	 *
> -	 * \todo The image sensor frame size should be selected to optimize
> -	 * operations based on the sizes of the requested streams. However such
> -	 * a selection makes the pipeline configuration procedure fail for small
> -	 * resolutions (for example: 640x480 with OV5670) and causes the capture
> -	 * operations to stall for some stream size combinations (see the
> -	 * commit message of the patch that introduced this comment for more
> -	 * failure examples).
> -	 *
> -	 * Until the sensor frame size calculation criteria are clarified, when
> -	 * capturing from ImgU always use the largest possible size which
> -	 * guarantees better results at the expense of the frame rate and CSI-2
> -	 * bus bandwidth. When only a raw stream is requested use the requested
> -	 * size instead, as the ImgU is not involved.
> +	 * The output YUV streams will be limited in size to the
> +	 * maximum frame size requested for the RAW stream.
>  	 */
> -	if (!yuvCount)
> -		cio2Configuration_ = data_->cio2_.generateConfiguration(rawSize);
> -	else
> -		cio2Configuration_ = data_->cio2_.generateConfiguration({});
> +	if (rawSize.isNull())
> +		rawSize = maxYuvSize.alignedUpTo(40, 540)
> +				    .alignedUpTo(IMGU_OUTPUT_WIDTH_MARGIN,
> +						 IMGU_OUTPUT_HEIGHT_MARGIN)

We have mixed magical values and defined ones... What are 40 and 540
exactly ?

> +				    .boundedTo(data_->cio2_.sensor()->resolution());
> +	cio2Configuration_ = data_->cio2_.generateConfiguration(rawSize);
>  	if (!cio2Configuration_.pixelFormat.isValid())
>  		return Invalid;
>  
>