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

[Jacopo Mondi]

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 ?

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