[RFC PATCH 2/3] test: ipa: libipa: Add CameraSensorHelper Gain Model tests

[Kieran Bingham]

Quoting Kieran Bingham (2024-02-27 17:12:32)
> Quoting Laurent Pinchart (2024-02-27 15:05:51)
> > Hi Kieran
> > 
> > On Mon, Feb 26, 2024 at 03:50:46PM +0100, Jacopo Mondi wrote:
> > > On Fri, Feb 23, 2024 at 03:59:53PM +0000, Kieran Bingham wrote:
> > > > Introduce a test that validates conversion of the gain model
> > > > in each CameraSensorHelper.
> > > >
> > > > It should be expected that if a gainCode produces a given gain
> > > > value, then converting that gain back should produce the same
> > > > result.
> > > >
> > > > This test fails on the following CameraSensorHelpers:
> > > >  - imx219 imx258 imx283 imx290 imx296 imx327 imx335 imx477
> > > >
> > > > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > > > ---
> > > >
> > > > This test also fails on the ar0521 helper as that has a defined code
> > > > limit of '63', which the test does not yet have a way to infer.
> > 
> > Where does that value come from ? I don't see it in the code.
> 
> It's what the test outputs as the maximum value. All values after this
> fail, and only 63 is returned for any value above this.
> 
> 
> I expect it's this:
> 
>  
> uint32_t CameraSensorHelperAr0521::gainCode(double gain) const
> {
>         gain = std::clamp(gain, 1.0, 15.5);
>         ...
> }
> 
> 
> > > > Adding a 'maxCode' helper to the base class may be the way to go here,
> > > > and would also let us define a maximum value for other helpers directly.
> > 
> > One option would be to retrieve the maximum by calling gainCode(very
> > high value). Or that would be an option if the helpers actually
> > implemented limits, which is currently not the case. If I recall
> > correctly, we get the gain code limits dynamically from the kernel
> > driver. Hardcoding limits in libcamera could be problematic.
> > 
> > How about creating subclasses of the camera sensor class in this test,
> > with different models and coefficients ? That would also remove the
> > requirement of exposing the factory names.

I'll give it a quick go now.

It means we won't catch issues in future camera sensor helpers - but
that already requires extending the current camera sensor helpers with
repetetive data anyway so maybe this is sufficient for now.

--
Kieran