[PATCH v3 16/23] libtuning: modules: Add initial CCM module

[Paul Elder]

On Thu, Jul 04, 2024 at 01:16:58PM +0300, Laurent Pinchart wrote:
> On Thu, Jul 04, 2024 at 07:04:56PM +0900, Paul Elder wrote:
> > On Wed, Jul 03, 2024 at 04:17:05PM +0200, Stefan Klug wrote:
> > > From: Paul Elder <paul.elder at ideasonboard.com>
> > > 
> > > Implement a minimal ccm calibration module. For now it doesn't take the
> > > results from lsc into account and supports rkisp1 only.
> > > 
> > > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > > Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > ---
> > >  .../tuning/libtuning/modules/ccm/__init__.py  |  6 +++
> > >  utils/tuning/libtuning/modules/ccm/ccm.py     | 41 +++++++++++++++++++
> > >  utils/tuning/libtuning/modules/ccm/rkisp1.py  | 27 ++++++++++++
> > >  utils/tuning/rkisp1.py                        |  4 +-
> > >  4 files changed, 77 insertions(+), 1 deletion(-)
> > >  create mode 100644 utils/tuning/libtuning/modules/ccm/__init__.py
> > >  create mode 100644 utils/tuning/libtuning/modules/ccm/ccm.py
> > >  create mode 100644 utils/tuning/libtuning/modules/ccm/rkisp1.py
> > > 
> > > diff --git a/utils/tuning/libtuning/modules/ccm/__init__.py b/utils/tuning/libtuning/modules/ccm/__init__.py
> > > new file mode 100644
> > > index 000000000000..322602afe63b
> > > --- /dev/null
> > > +++ b/utils/tuning/libtuning/modules/ccm/__init__.py
> > > @@ -0,0 +1,6 @@
> > > +# SPDX-License-Identifier: GPL-2.0-or-later
> > > +#
> > > +# Copyright (C) 2024, Paul Elder <paul.elder at ideasonboard.com>
> > > +
> > > +from libtuning.modules.ccm.ccm import CCM
> > > +from libtuning.modules.ccm.rkisp1 import CCMRkISP1
> > > diff --git a/utils/tuning/libtuning/modules/ccm/ccm.py b/utils/tuning/libtuning/modules/ccm/ccm.py
> > > new file mode 100644
> > > index 000000000000..ba433404f069
> > > --- /dev/null
> > > +++ b/utils/tuning/libtuning/modules/ccm/ccm.py
> > > @@ -0,0 +1,41 @@
> > > +# SPDX-License-Identifier: GPL-2.0-or-later
> > > +#
> > > +# Copyright (C) 2024, Paul Elder <paul.elder at ideasonboard.com>
> > > +# Copyright (C) 2024, Ideas on Board
> > > +#
> > > +# Base Ccm tuning module
> > > +
> > > +from ..module import Module
> > > +
> > > +from libtuning.ctt_ccm import ccm
> > > +import logging
> > > +
> > > +logger = logging.getLogger(__name__)
> > > +
> > > +
> > > +class CCM(Module):
> > > +    type = 'ccm'
> > > +    hr_name = 'CCM (Base)'
> > > +    out_name = 'GenericCCM'
> > > +
> > > +    def __init__(self, debug: list):
> > > +        super().__init__()
> > > +
> > > +        self.debug = debug
> > > +
> > > +    def do_calibration(self, images):
> > > +        logger.info('Starting CCM calibration')
> > > +
> > > +        imgs = [img for img in images if img.macbeth is not None]
> > > +
> > > +        # todo: Take LSC calibration results into account.
> > > +        cal_cr_list = None
> > > +        cal_cb_list = None
> > > +
> > > +        try:
> > > +            ccms = ccm(imgs, cal_cr_list, cal_cb_list)
> > 
> > Ooh that's cool.
> > 
> > > +        except ArithmeticError:
> > > +            logger.error('CCM calibration failed')
> > > +            return 1
> > > +
> > > +        return ccms
> > > diff --git a/utils/tuning/libtuning/modules/ccm/rkisp1.py b/utils/tuning/libtuning/modules/ccm/rkisp1.py
> > > new file mode 100644
> > > index 000000000000..00381abe2893
> > > --- /dev/null
> > > +++ b/utils/tuning/libtuning/modules/ccm/rkisp1.py
> > > @@ -0,0 +1,27 @@
> > > +# SPDX-License-Identifier: GPL-2.0-or-later
> > > +#
> > > +# Copyright (C) 2024, Paul Elder <paul.elder at ideasonboard.com>
> > > +# Copyright (C) 2024, Ideas on Board
> > > +#
> > > +# Ccm module for tuning rkisp1
> > > +
> > > +from .ccm import CCM
> > > +
> > > +class CCMRkISP1(CCM):
> > > +    hr_name = 'Crosstalk Correction (RkISP1)'
> > > +    out_name = 'Ccm'
> > > +
> > > +    def __init__(self, **kwargs):
> > > +        super().__init__(**kwargs)
> > > +
> > > +    # We don't need anything from the config file.
> > > +    def validate_config(self, config: dict) -> bool:
> > > +        return True
> > > +
> > > +    def process(self, config: dict, images: list, outputs: dict) -> dict:
> > > +        output = {}
> > > +
> > > +        ccms = self.do_calibration(images)
> > 
> > Don't we need to error check this?
> > 
> > Otherwise we'd get a single 1 for the ccm; not even a matrix.
> 
> Returning None from do_calibration() when an error occurs seems better

Yeah that would just disable ccm, which I suppose is more efficient than
returning identities.

Paul

> than 1. Or we could use exceptions for error handling too, but that's a
> bigger rework.
> 
> > > +        output['ccms'] = ccms
> > > +
> > > +        return output
> > > diff --git a/utils/tuning/rkisp1.py b/utils/tuning/rkisp1.py
> > > index 2606e07a93f7..fae35783c656 100755
> > > --- a/utils/tuning/rkisp1.py
> > > +++ b/utils/tuning/rkisp1.py
> > > @@ -14,6 +14,7 @@ from libtuning.parsers import YamlParser
> > >  from libtuning.generators import YamlOutput
> > >  from libtuning.modules.lsc import LSCRkISP1
> > >  from libtuning.modules.agc import AGCRkISP1
> > > +from libtuning.modules.ccm import CCMRkISP1
> > >  
> > >  
> > >  coloredlogs.install(level=logging.INFO, fmt='%(name)s %(levelname)s %(message)s')
> > > @@ -39,9 +40,10 @@ tuner.add(LSCRkISP1(
> > >            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, LSCRkISP1])
> > > +tuner.set_output_order([AGCRkISP1, CCMRkISP1, LSCRkISP1])
> > >  
> > >  if __name__ == '__main__':
> > >      sys.exit(tuner.run(sys.argv))