[PATCH v4 20/23] tuning: rkisp1: Add some static modules

[Stefan Klug]

Hi Dan,

Thank you for the review.

On Fri, Jul 05, 2024 at 08:19:04PM +0100, Dan Scally wrote:
> Hi Stefan
> 
> On 05/07/2024 15:41, Stefan Klug wrote:
> > Add awb, blc, cproc, filter, and gamma to the tuning file.  These don't
> > need any configuration.
> > 
> > At the moment there are no inter-module dependencies in the tuning
> > process. We can therefore safely sort them alphabetically. As soon as
> > the first dependency gets introduced (most likely lsc -> ccm) we will
> > see how to solve that.
> > 
> > The output order controls the order of processing in the IPA. It is now
> > also in alphabetical order which happens to be no change for the modules
> > that existed previously. For the others, there is no need for a specific
> > order.
> > 
> > Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> > ---
> >   utils/tuning/rkisp1.py | 21 +++++++++++++++++----
> >   1 file changed, 17 insertions(+), 4 deletions(-)
> > 
> > diff --git a/utils/tuning/rkisp1.py b/utils/tuning/rkisp1.py
> > index fae35783c656..0d279a39ab1b 100755
> > --- a/utils/tuning/rkisp1.py
> > +++ b/utils/tuning/rkisp1.py
> > @@ -15,11 +15,24 @@ from libtuning.generators import YamlOutput
> >   from libtuning.modules.lsc import LSCRkISP1
> >   from libtuning.modules.agc import AGCRkISP1
> >   from libtuning.modules.ccm import CCMRkISP1
> > -
> > +from libtuning.modules.static import StaticModule
> >   coloredlogs.install(level=logging.INFO, fmt='%(name)s %(levelname)s %(message)s')
> > +awb = StaticModule('Awb')
> > +blc = StaticModule('BlackLevelCorrection')
> > +color_processing = StaticModule('ColorProcessing')
> > +filter = StaticModule('Filter')
> > +gamma_out = StaticModule('GammaOutCorrection', {'gamma': 2.2})
> > +
> >   tuner = lt.Tuner('RkISP1')
> > +tuner.add(AGCRkISP1(debug=[lt.Debug.Plot]))
> > +tuner.add(awb)
> > +tuner.add(blc)
> > +tuner.add(CCMRkISP1(debug=[lt.Debug.Plot]))
> > +tuner.add(color_processing)
> > +tuner.add(filter)
> > +tuner.add(gamma_out)
> >   tuner.add(LSCRkISP1(
> 
> This is nitpicking so feel free to ignore me, but I think it'd be better to
> be consistent in using variables for the modules or not.

Honestly I don't know why I didn't do that. At first I wanted to add the
static modules the same way as the others, but I couldn't do that
because multiple entries of the StaticModule class in the output_order
would be detected as error. I then (unwillingly) had to go for the
intermediate variables. But I never thought about doing the same with
the others even though I really disliked the asymmetry.

So thanks for the hint :-). I'll add that as a separate patch if that's
ok for you.

Cheers,
Stefan

> 
> Reviewed-by: Daniel Scally <dan.scally at ideasonboard.com>
> 
> >             debug=[lt.Debug.Plot],
> >             # This is for the actual LSC tuning, and is part of the base LSC
> > @@ -39,11 +52,11 @@ tuner.add(LSCRkISP1(
> >             # values.  This can also be a custom function.
> >             smoothing_function=lt.smoothing.MedianBlur(3),
> >             ))
> > -tuner.add(AGCRkISP1(debug=[lt.Debug.Plot]))
> > -tuner.add(CCMRkISP1(debug=[lt.Debug.Plot]))
> > +
> >   tuner.set_input_parser(YamlParser())
> >   tuner.set_output_formatter(YamlOutput())
> > -tuner.set_output_order([AGCRkISP1, CCMRkISP1, LSCRkISP1])
> > +tuner.set_output_order([AGCRkISP1, awb, blc, CCMRkISP1, color_processing,
> > +                        filter, gamma_out, LSCRkISP1])
> >   if __name__ == '__main__':
> >       sys.exit(tuner.run(sys.argv))