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

[Umang Jain]

Hi Jacopo

On 8/30/21 2:22 PM, Jacopo Mondi wrote:
> On Mon, Aug 30, 2021 at 02:02:27PM +0530, Umang Jain wrote:
>> 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?
>>
> What about adding a YUV stream of the same size to the command ?


As of master, 7208e70211a6 ("libcamera: ipu3: Always use sensor full 
frame size") is in effect  right? So it's always selecting the highest 
sensor format resolution.

I cherry-picked your "DNI: libcamera: ipu3: Use the optimal sensor side" 
which generates the same results as the commit message, to double-check.

So I don't adding YUV stream helps. Probably I'll just need to drop 
these commands and explain everything in a text.

>
>>> 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;