[libcamera-devel] [PATCH 7/7] libcamera: ipu3: imgu: Change IF_CROP_MAX to 40

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Thu, Mar 18, 2021 at 11:39:41AM +0100, Jacopo Mondi wrote:
> From: Dave Olsthoorn <dave at bewaar.me>
> 
> The value of IF_CROP_MAX seems to be a typo. A resolution of 40x540 seems
> unlikely and excludes camera's with a 640x480 resolution, like the OV7251
> in several Microsoft Surface products, from working.

IF_CROP_MAX_H seems to be the maximum cropping that can be applied
vertically, and is used to limit the search space. I'm not sure if
that's a device limitation or a software limitation, but in any case,
it's not a minimum vertical size as this commit message seems to imply,
and I don't see why it would excluse a 640x480 input.

Could the problem be instead caused by

	unsigned int minIFHeight = iif.height - IF_CROP_MAX_H;

in calculateBDSHeight() that will be a very large positive value if
iif.height < IF_CROP_MAX_H ?

> This patch corrects the value to 40 since a minimal resolution of 40x40
> seems more logical.
> 
> Signed-off-by: Dave Olsthoorn <dave at bewaar.me>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/pipeline/ipu3/imgu.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp
> index b6a1fe34494e..dd7d2e947141 100644
> --- a/src/libcamera/pipeline/ipu3/imgu.cpp
> +++ b/src/libcamera/pipeline/ipu3/imgu.cpp
> @@ -44,7 +44,7 @@ static constexpr unsigned int BDS_ALIGN_W = 2;
>  static constexpr unsigned int BDS_ALIGN_H = 4;
>  
>  static constexpr unsigned int IF_CROP_MAX_W = 40;
> -static constexpr unsigned int IF_CROP_MAX_H = 540;
> +static constexpr unsigned int IF_CROP_MAX_H = 40;
>  
>  static constexpr float BDS_SF_MAX = 2.5;
>  static constexpr float BDS_SF_MIN = 1.0;

-- 
Regards,

Laurent Pinchart