[PATCH v2] libcamera: rkisp1: Clamp stream configuration to ISP limit on raw path

[Stefan Klug]

Hi Umang,

Thank you for the patch. 

On Wed, Oct 09, 2024 at 03:09:19PM +0530, Umang Jain wrote:
> Commit 761545407c76 ("pipeline: rkisp1: Filter out sensor sizes not
> supported by the pipeline") introduced a mechanism to determine maximum
> supported sensor resolution and filter out resolutions that cannot be
> supported by the ISP.
> 
> However, it missed to update the raw stream configuration path, where
> it should have clamped the raw stream configuration size to the maximum
> sensor supported resolution.
> 
> This patch fixes the above issue and can be confirmed with IMX283
> on i.MX8MP:
> 
> From:
> ($) cam -c1 -srole=raw,width=5472,height=3072
> INFO Camera camera.cpp:1197 configuring streams: (0) 5472x3648-SRGGB12
> ERROR RkISP1 rkisp1_path.cpp:425 Unable to configure capture in 5472x3648-SRGGB12
> Failed to configure camera
> Failed to start camera session
> 
> To:
> ($) cam -c1 -srole=raw,width=5472,height=3072
> INFO Camera camera.cpp:1197 configuring streams: (0) 4096x3072-SRGGB12
> cam0: Capture until user interrupts by SIGINT
> 536.082380 (0.00 fps) cam0-stream0 seq: 000000 bytesused: 25165824
> 536.182378 (10.00 fps) cam0-stream0 seq: 000001 bytesused: 25165824
> 536.282375 (10.00 fps) cam0-stream0 seq: 000002 bytesused: 25165824
> ...
> 
> Fixes: 761545407c76 ("pipeline: rkisp1: Filter out sensor sizes not supported by the pipeline")
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

Looks good to me.

Reviewed-by: Stefan Klug <stefan.klug at ideasonboard.com> 

> ---
> Changes in v2:
> - Extend the comment on what 'resolution' var is and denote that
>   sensor->getFormat() will never return something greter than
>  'resolution'
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> index 3b5bea96..1999094e 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> @@ -326,9 +326,15 @@ RkISP1Path::validate(const CameraSensor *sensor,
>  	if (isRaw) {
>  		/*
>  		 * Use the sensor output size closest to the requested stream
> -		 * size.
> +		 * size while ensuring the output size doesn't exceed ISP limits.
> +		 *
> +		 * As 'resolution' is the largest sensor resolution
> +		 * supported by the ISP, CameraSensor::getFormat() will never
> +		 * return a V4L2SubdeviceFormat with a larger size.
>  		 */
>  		uint32_t mbusCode = formatToMediaBus.at(cfg->pixelFormat);
> +		cfg->size.boundTo(resolution);
> +
>  		Size rawSize = sensorConfig ? sensorConfig->outputSize
>  					    : cfg->size;
>  
> -- 
> 2.45.2
>