[libcamera-devel] [PATCH v3 4/4] ipu3: cio2: Tweak sensor size selection policy

[Umang Jain]

Hi Me,

On 8/30/21 1:51 PM, Umang Jain wrote:
> Do not compare higher precision of the ratios, as it might lead to
> absurd selection of sensor size for a relatively low requested
> resolution size.
>
> For example:
> The imx258 driver supports the following sensor resolutions:
>
>   - 4208x3118 = 1.349583066
>   - 2104x1560 = 1.348717949
>   - 1048x780  = 1.343589744
>
> It can be inferred that, that the aspect ratio only differs by a small
> factor with each other. It does not makes sense to select a 4208x3118
> for a requested size of say 640x480 or 1280x720, which is what is
> happening currently.
> ($) cam -c1 -swidth=640,height=480,role=raw
>      - CIO2 configuration: 4208x3118-SGRBG10_IPU3


This cam command is invalid as of today. It was fine when I wrote the 
series.

The invalidation comes from 4fec25a4b ("ipu3: Disallow raw only camera 
configuration"). I am tempted to remove this cam command (since it's not 
valid today), but I don't think I would have a better explanation than 
this. All I can do is to replace it with a block of text (essentially 
making the commit message still longer)

Any ideas anyone?

>
> In order to address this constraint, only compare the ratio with single
> precision to make a better decision on the sensor resolution policy
> selection.
>
> ($) cam -c1 -srole=raw,width=640,height=480
>      - CIO2 configuration: 1048x780-SGRBG10_IPU3
>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> Tested-by: Jacopo Mondi <jacopo at jmondi.org>
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>   src/libcamera/pipeline/ipu3/cio2.cpp | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
> index ceaf665e..9cedcb5b 100644
> --- a/src/libcamera/pipeline/ipu3/cio2.cpp
> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp
> @@ -295,6 +295,14 @@ V4L2SubdeviceFormat CIO2Device::getSensorFormat(const std::vector<unsigned int>
>   				continue;
>   
>   			float ratio = static_cast<float>(sz.width) / sz.height;
> +			/*
> +			 * Ratios can differ by small mantissa difference which
> +			 * can affect the selection of the sensor output size
> +			 * wildly. We are interested in selection of the closest
> +			 * size with respect to the desired output size, hence
> +			 * comparing it with a single precision digit is enough.
> +			 */
> +			ratio = static_cast<unsigned int>(ratio * 10) / 10.0;
>   			float ratioDiff = fabsf(ratio - desiredRatio);
>   			unsigned int area = sz.width * sz.height;
>   			unsigned int areaDiff = area - desiredArea;