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

[Umang Jain]

Hi Jacopo

On 8/9/21 9:40 PM, Jacopo Mondi wrote:
> Hi Umang,
>
> On Tue, Aug 03, 2021 at 07:02:05PM +0530, 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
>>
>> 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>
>> ---
>>   src/libcamera/pipeline/ipu3/cio2.cpp | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
>> index 9980b8bd..887b850d 100644
>> --- a/src/libcamera/pipeline/ipu3/cio2.cpp
>> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp
>> @@ -281,6 +281,13 @@ V4L2SubdeviceFormat CIO2Device::getSensorFormat(const std::vector<unsigned int>
>>   				continue;
>>
>>   			float ratio = static_cast<float>(sz.width) / sz.height;
>> +			/*
>> +			 * Comparing ratios with a single precision is enough.
>> +			 * This avoids the selection of a frame size which is
>> +			 * 2x or 3x than the requested size, having a slightly
>> +			 * better ratio w.r.t native resolution ratio.
> Well, the fact the 2x or 3x sizes selection happens is Nautilus
> specific. I would stay simple and just
>
>                          /*
>                           * Comparing rations with a single precision
>                           * digit is enough.
>                           */
>
> Be aware that this actually truncates instead of rounding.. is this
> the correct choice ? Also do we need to round the desired ratio too ?

I think it's the correct choice of truncating for now. No, desired ratio 
can stay as is, as it's constant. We need to nuke the subtle differences 
between "competing" sizes and either respective ratios

.

>
> Thanks
>     j
>
>> +			 */
>> +			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;
>> --
>> 2.31.0
>>