Message ID | 20250123114204.79321-9-stefan.klug@ideasonboard.com |
---|---|
State | Accepted |
Headers | show |
Series |
|
Related | show |
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@ideasonboard.com> > Reviewed-by: Paul Elder <paul.elder@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). 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) > + 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 >
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@ideasonboard.com> > > Reviewed-by: Paul Elder <paul.elder@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'] 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 > >
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@ideasonboard.com> > > > Reviewed-by: Paul Elder <paul.elder@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@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 > > >
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'] + 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