[PATCH v2 08/17] libtuning: module: awb: Add bayes AWB support

Kieran Bingham kieran.bingham at ideasonboard.com
Thu Feb 20 13:02:15 CET 2025


Quoting Stefan Klug (2025-02-17 09:04:41)
> Hi Kieran,
> 
> Thank you for the review. 
> 
> On Sat, Feb 15, 2025 at 01:04:10PM +0000, Kieran Bingham wrote:
> > Quoting Stefan Klug (2025-01-23 11:40:58)
> > > To support the bayesian AWB algorithm in libtuning, the necessary data
> > > needs to be collected and written to the tuning file.
> > > 
> > > Extend libtuning to calculate and output that additional data.
> > > 
> > > Prior probabilities and AwbModes are manually specified and not
> > > calculated in the tuning process. Add sample values from the RaspberryPi
> > > tuning files to the example config file.
> > > 
> > > Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > > Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> > > 
> > > ---
> > > 
> > > Changes in v2:
> > > - Collected tags
> > > - Fixed missing space
> > > - Reworked commit message
> > > - Add example prior probabilities from RaspberryPi
> > > ---
> > >  utils/tuning/config-example.yaml             | 44 +++++++++++++++++++-
> > >  utils/tuning/libtuning/modules/awb/awb.py    | 16 ++++---
> > >  utils/tuning/libtuning/modules/awb/rkisp1.py | 21 +++++++---
> > >  3 files changed, 68 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/utils/tuning/config-example.yaml b/utils/tuning/config-example.yaml
> > > index 1b7f52cd2fff..1bbb275778dc 100644
> > > --- a/utils/tuning/config-example.yaml
> > > +++ b/utils/tuning/config-example.yaml
> > > @@ -5,7 +5,49 @@ general:
> > >      do_alsc_colour: 1
> > >      luminance_strength: 0.5
> > >    awb:
> > > -    greyworld: 0
> > > +    # Algorithm can either be 'grey' or 'bayes'
> > > +    algorithm: bayes
> > > +    # Priors is only used for the bayes algorithm. They are defined in
> > > +    # logarithmic space. A good staring point is:
> > > +    # - lux: 0
> > > +    #   ct: [ 2000, 3000, 13000 ]
> > > +    #   probability: [ 1.0, 0.0, 0.0 ]
> > > +    # - lux: 800
> > > +    #   ct: [ 2000, 6000, 13000 ]
> > > +    #   probability: [ 0.0, 2.0, 2.0 ]
> > > +    # - lux: 1500
> > > +    #   ct: [ 2000, 4000, 6000, 6500, 7000, 13000 ]
> > > +    #   probability: [ 0.0, 1.0, 6.0, 7.0, 1.0, 1.0 ]
> > > +    priors:
> > > +      - lux: 0
> > > +        ct: [ 2000, 13000 ]
> > > +        probability: [ 0.0, 0.0 ]
> > > +    AwbMode:
> > > +      AwbAuto:
> > > +        lo: 2500
> > > +        hi: 8000
> > > +      AwbIncandescent:
> > > +        lo: 2500
> > > +        hi: 3000
> > > +      AwbTungsten:
> > > +        lo: 3000
> > > +        hi: 3500
> > > +      AwbFluorescent:
> > > +        lo: 4000
> > > +        hi: 4700
> > > +      AwbIndoor:
> > > +        lo: 3000
> > > +        hi: 5000
> > > +      AwbDaylight:
> > > +        lo: 5500
> > > +        hi: 6500
> > > +      AwbCloudy:
> > > +        lo: 6500
> > > +        hi: 8000
> > > +      # One custom mode can be defined if needed
> > > +      #AwbCustom:
> > > +      #  lo: 2000
> > > +      #  hi: 1300
> > >    macbeth:
> > >      small: 1
> > >      show: 0
> > > diff --git a/utils/tuning/libtuning/modules/awb/awb.py b/utils/tuning/libtuning/modules/awb/awb.py
> > > index c154cf3b8609..0dc4f59dcb26 100644
> > > --- a/utils/tuning/libtuning/modules/awb/awb.py
> > > +++ b/utils/tuning/libtuning/modules/awb/awb.py
> > > @@ -27,10 +27,14 @@ class AWB(Module):
> > >  
> > >          imgs = [img for img in images if img.macbeth is not None]
> > >  
> > > -        gains, _, _ = awb(imgs, None, None, False)
> > > -        gains = np.reshape(gains, (-1, 3))
> > > +        ct_curve, transverse_pos, transverse_neg = awb(imgs, None, None, False)
> > > +        ct_curve = np.reshape(ct_curve, (-1, 3))
> > > +        gains = [{
> > > +            'ct': int(v[0]),
> > > +            'gains': [float(1.0 / v[1]), float(1.0 / v[2])]
> > > +        } for v in ct_curve]
> > > +
> > > +        return {'colourGains': gains,
> > > +                'transversePos': transverse_pos,
> > > +                'transverseNeg': transverse_neg}
> > >  
> > > -        return [{
> > > -                    'ct': int(v[0]),
> > > -                    'gains': [float(1.0 / v[1]), float(1.0 / v[2])]
> > > -                } for v in gains]
> > > diff --git a/utils/tuning/libtuning/modules/awb/rkisp1.py b/utils/tuning/libtuning/modules/awb/rkisp1.py
> > > index 0c95843b83d3..d562d26eb8cc 100644
> > > --- a/utils/tuning/libtuning/modules/awb/rkisp1.py
> > > +++ b/utils/tuning/libtuning/modules/awb/rkisp1.py
> > > @@ -6,9 +6,6 @@
> > >  
> > >  from .awb import AWB
> > >  
> > > -import libtuning as lt
> > > -
> > > -
> > >  class AWBRkISP1(AWB):
> > >      hr_name = 'AWB (RkISP1)'
> > >      out_name = 'Awb'
> > > @@ -20,8 +17,20 @@ class AWBRkISP1(AWB):
> > >          return True
> > >  
> > >      def process(self, config: dict, images: list, outputs: dict) -> dict:
> > > -        output = {}
> > > -
> > > -        output['colourGains'] = self.do_calculation(images)
> > > +        if not 'awb' in config['general']:
> > > +            raise ValueError('AWB configuration missing')
> > > +        awb_config = config['general']['awb']
> > > +        algorithm = awb_config['algorithm']
> > > +
> > > +        output = {'algorithm': algorithm}
> > > +        data = self.do_calculation(images)
> > > +        if algorithm == 'grey':
> > > +            output['colourGains'] = data['colourGains']
> > 
> > How come there's no output.update(data) on this code path ? (I don't
> > know what it does yet, just noticing that it's only on one path).
> 
> Arguably in the tuning code there is room for improvement :-). Reason is
> that in the grey world case, we only need the colourGains in the tuning
> file, in the bayes case we need all the results, so I just did
> output.update(data) without manually stating every key.
> 
> > 
> > Is there any argument to output the colourGains for greyworld as well as
> > bayes to the same file ? or is Manual ColourTemperature handled
> > distinctly with bayes (I presume it just overrides whatever the bayes
> > would have predicted to be the colour temperature)
> > 
> 
> I'm not sure if I completely understand what you mean here. colourGains
> are needed for both cases (grey and bayes). But bayes needs the rest.
> Would you prefer the following code?
> 
> if algorithm == 'grey':
>     output['colourGains'] = data['colourGains']
> elif algorithm == 'bayes':
>     output['AwbMode'] = awb_config['AwbMode']
>     output['priors'] = awb_config['priors']
>     output['colourGains'] = data['colourGains']
>     output['transversePos'] = data['transversePos']
>     output['transverseNeg'] = data['transverseNeg']

Aha, thanks I hadn't actually realsed that output.update(data) was just
simplifying the transverseNeg parts..

I guess my point before was ... why not output it all if we've
calculated it. In AWB case, there's not a lot of 'cost' to storing this
data, but I can imagine in LSC or other algorithms the data might be
more costly if it's not used.

But ultimately - as long as the data is stored to be available when it's
needed I'm happy ;-)


But theres nothing here that would block merging so:


Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> else:
> ...
> 
> 
> Best regards,
> Stefan
> 
> > 
> > > +        elif algorithm == 'bayes':
> > > +            output['AwbMode'] = awb_config['AwbMode']
> > > +            output['priors'] = awb_config['priors']
> > > +            output.update(data)
> > > +        else:
> > > +            raise ValueError(f"Unknown AWB algorithm {output['algorithm']}")
> > >  
> > >          return output
> > > -- 
> > > 2.43.0
> > >


More information about the libcamera-devel mailing list