[PATCH v2 3/3] libipa: camera_sensor_helper: Fix rounding of gainCode

[Paul Elder]

Hi Kieran,

On Thu, Mar 27, 2025 at 08:39:10AM +0000, Kieran Bingham wrote:
> Hi Paul,
> 
> Quoting Paul Elder (2025-03-26 08:00:33)
> > From: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > 
> > The implementation of gainCode for both Exponential and Linear gain
> > models does not generate a gainCode that matches the result of the
> > reverse operation.
> > 
> > This can be seen by converting sequential gainCodes to a gain
> > and converting that back to the gainCode. The values do not
> > translate back due to rounding errors.
> > 
> > Correct the rounding error and ensure that gainCode translation
> > produces accurate bi-directional conversions from the perspective
> > of the gainCode.
> > 
> > This fixes the IMX290, IMX296, IMX327 and IMX335 which use the
> > Exponential gain model helpers, as well as IMX219 IMX258 and IMX477
> > which use the Linear gain model helpers.
> > 
> 
> Thanks for reviving this.
> 
> I think this one needs to be thought about carefully though - I recall
> some discussion with David, where the key/important information I missed
> when I worked on this last was that we shouldn't ever round up in these
> equations...
> 
> Could you review https://patchwork.libcamera.org/patch/20099/ please?

Ah, I didn't notice that. I'll take a look.

> 
> When I started this work - I mis-interpreted the underlying issue as a
> rounding issue - but it's not. And using round() causes different
> problems.
> 
> I made a playground for graphing all of this at 
>  https://git.uk.ideasonboard.com/kbingham/octave-gains

Oh neat, thanks,

> 
> But this should be re-implemented based on the feedback from David.

ack


Paul

> 
> 
> > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > 
> > ---
> > Changes in v2:
> > - recover from bitrot
> > ---
> >  src/ipa/libipa/camera_sensor_helper.cpp | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> > index 7c66cd57d685..5645dd14fda3 100644
> > --- a/src/ipa/libipa/camera_sensor_helper.cpp
> > +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> > @@ -90,12 +90,12 @@ uint32_t CameraSensorHelper::gainCode(double gain) const
> >         if (auto *l = std::get_if<AnalogueGainLinear>(&gain_)) {
> >                 ASSERT(l->m0 == 0 || l->m1 == 0);
> >  
> > -               return (l->c0 - l->c1 * gain) /
> > -                      (l->m1 * gain - l->m0);
> > +               return std::round((l->c0 - l->c1 * gain) /
> > +                                 (l->m1 * gain - l->m0));
> >         } else if (auto *e = std::get_if<AnalogueGainExp>(&gain_)) {
> >                 ASSERT(e->a != 0 && e->m != 0);
> >  
> > -               return std::log2(gain / e->a) / e->m;
> > +               return std::round(std::log2(gain / e->a) / e->m);
> >         } else {
> >                 ASSERT(false);
> >                 return 0;
> > -- 
> > 2.47.2
> >