| Message ID | 20240703141726.252368-21-stefan.klug@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
On Wed, Jul 03, 2024 at 04:17:09PM +0200, Stefan Klug wrote: > Add awb, blc, cproc, filter, and gamma by default to the tuning file. These > don't need any configuration. > > While at it, sort the modules alphabetically. > > Signed-off-by: Stefan Klug <stefan.klug@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( > 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])) This changes order of executing the tuning. I thought you want lsc to tune before ccm and agc? > + > 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]) This determines the order of output and thus the order of executing the algorithms. Paul > > if __name__ == '__main__': > sys.exit(tuner.run(sys.argv)) > -- > 2.43.0 >
Hi Paul, Thanks for the review. On Thu, Jul 04, 2024 at 07:07:32PM +0900, Paul Elder wrote: > On Wed, Jul 03, 2024 at 04:17:09PM +0200, Stefan Klug wrote: > > Add awb, blc, cproc, filter, and gamma by default to the tuning file. These > > don't need any configuration. > > > > While at it, sort the modules alphabetically. > > > > Signed-off-by: Stefan Klug <stefan.klug@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( > > 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])) > > This changes order of executing the tuning. I thought you want lsc to > tune before ccm and agc? This was discussed briefly here: https://patchwork.libcamera.org/patch/20479/#30202 Right now we don't have inner module dependencies. But I'm fine with keeping the old order if you prefer. > > > + > > 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]) > > This determines the order of output and thus the order of executing the > algorithms. Ah ok. I wasn't aware that this also has an influence on the order of processing inside the IPA. But now that you say it, it seems logical. From a user point of view I didn't expect that. Should the order of processing be defined/fixed inside the IPA? To get this patch in: Would it be ok for you if I order the modules here (and possibly above) in the order of processing inside the actual hardware and we postpone the discussion until we get there? Best regards, Stefan > > > Paul > > > > > if __name__ == '__main__': > > sys.exit(tuner.run(sys.argv)) > > -- > > 2.43.0 > >
On Fri, Jul 05, 2024 at 09:34:23AM +0200, Stefan Klug wrote: > Hi Paul, > > Thanks for the review. > > On Thu, Jul 04, 2024 at 07:07:32PM +0900, Paul Elder wrote: > > On Wed, Jul 03, 2024 at 04:17:09PM +0200, Stefan Klug wrote: > > > Add awb, blc, cproc, filter, and gamma by default to the tuning file. These > > > don't need any configuration. > > > > > > While at it, sort the modules alphabetically. > > > > > > Signed-off-by: Stefan Klug <stefan.klug@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( > > > 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])) > > > > This changes order of executing the tuning. I thought you want lsc to > > tune before ccm and agc? > > This was discussed briefly here: https://patchwork.libcamera.org/patch/20479/#30202 > Right now we don't have inner module dependencies. But I'm fine with > keeping the old order if you prefer. Ah ok I hadn' seen that. If there's no dependency *now* then yeah we can come up with a better dependency mechanism later when you do add it later. > > > > > > + > > > 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]) > > > > This determines the order of output and thus the order of executing the > > algorithms. > > Ah ok. I wasn't aware that this also has an influence on the order of > processing inside the IPA. But now that you say it, it seems logical. > > From a user point of view I didn't expect that. Should the order of > processing be defined/fixed inside the IPA? > I'm not sure. I thought it was fine for users to be able to modify it via the tuning file (that's how algos are enabled/disabled anyway). > To get this patch in: Would it be ok for you if I order the modules here > (and possibly above) in the order of processing inside the actual > hardware and we postpone the discussion until we get there? Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > > > > > > > > if __name__ == '__main__': > > > sys.exit(tuner.run(sys.argv)) > > > -- > > > 2.43.0 > > >
On Fri, Jul 05, 2024 at 05:02:52PM +0900, Paul Elder wrote: > On Fri, Jul 05, 2024 at 09:34:23AM +0200, Stefan Klug wrote: > > Hi Paul, > > > > Thanks for the review. > > > > On Thu, Jul 04, 2024 at 07:07:32PM +0900, Paul Elder wrote: > > > On Wed, Jul 03, 2024 at 04:17:09PM +0200, Stefan Klug wrote: > > > > Add awb, blc, cproc, filter, and gamma by default to the tuning file. These > > > > don't need any configuration. > > > > > > > > While at it, sort the modules alphabetically. > > > > > > > > Signed-off-by: Stefan Klug <stefan.klug@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( > > > > 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])) > > > > > > This changes order of executing the tuning. I thought you want lsc to > > > tune before ccm and agc? > > > > This was discussed briefly here: https://patchwork.libcamera.org/patch/20479/#30202 > > Right now we don't have inner module dependencies. But I'm fine with > > keeping the old order if you prefer. > > Ah ok I hadn' seen that. If there's no dependency *now* then yeah we can > come up with a better dependency mechanism later when you do add it > later. > > > > > + > > > > 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]) > > > > > > This determines the order of output and thus the order of executing the > > > algorithms. > > > > Ah ok. I wasn't aware that this also has an influence on the order of > > processing inside the IPA. But now that you say it, it seems logical. > > > > From a user point of view I didn't expect that. Should the order of > > processing be defined/fixed inside the IPA? > > > > I'm not sure. I thought it was fine for users to be able to modify it > via the tuning file (that's how algos are enabled/disabled anyway). The IPA design is that algorithms are controlled from tuning files, allowing the user to select which algorithms should run, but also the order. While this isn't currently doable without recompilation, one goal was to make it possible for users to implement additional algorithms. I'd rather keep the ordering controlled by the tuning file for now. > > To get this patch in: Would it be ok for you if I order the modules here > > (and possibly above) in the order of processing inside the actual > > hardware and we postpone the discussion until we get there? > > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > > > > > > > > > if __name__ == '__main__': > > > > sys.exit(tuner.run(sys.argv))
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( 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))
Add awb, blc, cproc, filter, and gamma by default to the tuning file. These don't need any configuration. While at it, sort the modules alphabetically. Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> --- utils/tuning/rkisp1.py | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-)