[PATCH v1 2/2] utils: tuning: Change Tuner.add() to accept a list of modules
Stefan Klug
stefan.klug at ideasonboard.com
Mon Aug 5 10:55:23 CEST 2024
Hi Kieran,
Thank you for the review.
On Mon, Aug 05, 2024 at 08:39:04AM +0100, Kieran Bingham wrote:
> Quoting Stefan Klug (2024-08-05 08:06:33)
> > Change the first parameter of Tuner.add() to be a list of modules
> > instead of a single module. This allows more compact code and is in sync
> > with Tuner.set_output_order().
> >
> > Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > ---
> > utils/tuning/libtuning/libtuning.py | 4 ++--
> > utils/tuning/raspberrypi_alsc_only.py | 2 +-
> > utils/tuning/rkisp1.py | 10 +---------
> > 3 files changed, 4 insertions(+), 12 deletions(-)
> >
> > diff --git a/utils/tuning/libtuning/libtuning.py b/utils/tuning/libtuning/libtuning.py
> > index e7c63535fefd..a167e39ef08e 100644
> > --- a/utils/tuning/libtuning/libtuning.py
> > +++ b/utils/tuning/libtuning/libtuning.py
> > @@ -94,8 +94,8 @@ class Tuner(object):
> > self.config = {}
> > self.output = {}
> >
> > - def add(self, module):
> > - self.modules.append(module)
> > + def add(self, modules):
> > + self.modules.extend(modules)
>
> I guess it already supported a list, it's just we're only calling a
> single module and it had only been used that way.
Not really, it now calls extend() instead of append() on the list.
>
> The pluralisation here is certainly worthwhile though to convey this.
>
> >
> > def set_input_parser(self, parser):
> > self.parser = parser
> > diff --git a/utils/tuning/raspberrypi_alsc_only.py b/utils/tuning/raspberrypi_alsc_only.py
> > index 777d800765e0..fe2b5b8b5a24 100755
> > --- a/utils/tuning/raspberrypi_alsc_only.py
> > +++ b/utils/tuning/raspberrypi_alsc_only.py
> > @@ -14,7 +14,7 @@ from libtuning.generators import RaspberryPiOutput
> > from raspberrypi.alsc import ALSC
> >
> > tuner = lt.Tuner('Raspberry Pi (ALSC only)')
> > -tuner.add(ALSC)
> > +tuner.add([ALSC])
>
> Does this one need to be changed though? (It doesn't hurt I believe), I
> see we're in a module called _alsc_only.py, so this won't be expected to
> be added to..
As noted above, this is actually required (If we don't want to go for
more magic).
Regards,
Stefan
>
> But still ... it keeps the usages consistent...
>
> > tuner.set_input_parser(RaspberryPiParser())
> > tuner.set_output_formatter(RaspberryPiOutput())
> > tuner.set_output_order([ALSC])
> > diff --git a/utils/tuning/rkisp1.py b/utils/tuning/rkisp1.py
> > index 5d7a69fc4a13..f5c42a61d15e 100755
> > --- a/utils/tuning/rkisp1.py
> > +++ b/utils/tuning/rkisp1.py
> > @@ -45,15 +45,7 @@ lsc = LSCRkISP1(debug=[lt.Debug.Plot],
> > smoothing_function=lt.smoothing.MedianBlur(3),)
> >
> > tuner = lt.Tuner('RkISP1')
> > -tuner.add(agc)
> > -tuner.add(awb)
> > -tuner.add(blc)
> > -tuner.add(ccm)
> > -tuner.add(color_processing)
> > -tuner.add(filter)
> > -tuner.add(gamma_out)
> > -tuner.add(lsc)
> > -
> > +tuner.add([agc, awb, blc, ccm, color_processing, filter, gamma_out, lsc])
>
>
> Keeping them on separate lines would make it easier to add new entries
> ... but there's a finite number of items we'll add here and once
> 'feature complete' this line won't change - so it's not quite comparable
> to other lists that we might otherwise extend, so :
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> > tuner.set_input_parser(YamlParser())
> > tuner.set_output_formatter(YamlOutput())
> > tuner.set_output_order([agc, awb, blc, ccm, color_processing,
> > --
> > 2.43.0
> >
More information about the libcamera-devel
mailing list