[libcamera-devel] [PATCH v5 6/8] utils: raspberrypi: Add tuning file conversion script

[Laurent Pinchart]

One more comment.

On Fri, Jul 15, 2022 at 02:40:59AM +0300, Laurent Pinchart via libcamera-devel wrote:
> Hi Naush,
> 
> Thank you for the patch.
> 
> On Thu, Jul 14, 2022 at 04:24:07PM +0100, Naushir Patuck via libcamera-devel wrote:
> > Add a script to convert the Raspberry Pi camera tuning file format from version
> > 1.0 to 2.0.
> > 
> > The version 1.0 format was originally used with the boost JSON parser that
> > happen to provided algorithm ordering based on the ordering in the file. The new
> 
> s/provided/provide/
> 
> > format provides implicit ordering by having the algorithms listed in an array.
> > 
> > This script also adds a root level version key set to 2.0 to the config file,
> > allowing the controller to distinguish between the two formats.
> > 
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > ---
> >  utils/raspberrypi/ctt/convert_tuning.py | 117 ++++++++++++++++++++++++
> >  1 file changed, 117 insertions(+)
> >  create mode 100755 utils/raspberrypi/ctt/convert_tuning.py
> > 
> > diff --git a/utils/raspberrypi/ctt/convert_tuning.py b/utils/raspberrypi/ctt/convert_tuning.py
> > new file mode 100755
> > index 000000000000..c915bcb46f64
> > --- /dev/null
> > +++ b/utils/raspberrypi/ctt/convert_tuning.py
> > @@ -0,0 +1,117 @@
> > +#!/bin/python3

This should be

#!/usr/bin/env python3

or possibly

#!/usr/bin/python3

> Please add an SPDX header to specify the license.
> 
> > +# Script to convert version 1.0 Raspberry Pi camera tuning files to version 2.0
> > +# and later.
> > +#
> > +# Copyright 2022 Raspberry Pi Ltd.
> > +
> > +import argparse
> > +import json
> > +import textwrap
> > +
> > +
> > +class Encoder(json.JSONEncoder):
> > +
> > +    def __init__(self, *args, **kwargs):
> > +        super().__init__(*args, **kwargs)
> > +        self.indentation_level = 0
> > +        self.hard_break = 120
> > +        self.custom_elems = {
> > +            'table': 16,
> > +            'luminance_lut': 16,
> > +            'ct_curve': 3,
> > +            'ccm': 3
> > +        }
> > +
> > +    def encode(self, o, node_key=None):
> > +        if isinstance(o, (list, tuple)):
> > +            # Check if we are a flat list of numbers.
> > +            if not any(isinstance(el, (list, tuple, dict)) for el in o):
> > +                s = ', '.join(json.dumps(el) for el in o)
> > +                if node_key in self.custom_elems.keys():
> > +                    # Special case handling to specify number of elements in a row for tables, ccm, etc.
> > +                    self.indentation_level += 1
> > +                    sl = s.split(', ')
> > +                    num = self.custom_elems[node_key]
> > +                    chunk = [self.indent_str + ', '.join(sl[x:x + num]) for x in range(0, len(sl), num)]
> > +                    t = ',\n'.join(chunk)
> > +                    self.indentation_level -= 1
> > +                    output = f'\n{self.indent_str}[\n{t}\n{self.indent_str}]'
> > +                elif len(s) > self.hard_break - len(self.indent_str):
> > +                    # Break a long list with wraps.
> > +                    self.indentation_level += 1
> > +                    t = textwrap.fill(s, self.hard_break, break_long_words=False,
> > +                                      initial_indent=self.indent_str, subsequent_indent=self.indent_str)
> > +                    self.indentation_level -= 1
> > +                    output = f'\n{self.indent_str}[\n{t}\n{self.indent_str}]'
> > +                else:
> > +                    # Smaller lists can remain on a single line.
> > +                    output = f' [ {s} ]'
> > +                return output
> > +            else:
> > +                # Sub-structures in the list case.
> > +                self.indentation_level += 1
> > +                output = [self.indent_str + self.encode(el) for el in o]
> > +                self.indentation_level -= 1
> > +                output = ',\n'.join(output)
> > +                return f' [\n{output}\n{self.indent_str}]'
> > +
> > +        elif isinstance(o, dict):
> > +            self.indentation_level += 1
> > +            output = []
> > +            for k, v in o.items():
> > +                if isinstance(v, dict) and len(v) == 0:
> > +                    # Empty config block special case.
> > +                    output.append(self.indent_str + f'{json.dumps(k)}: {{ }}')
> > +                else:
> > +                    # Only linebreak if the next node is a config block.
> > +                    sep = f'\n{self.indent_str}' if isinstance(v, dict) else ''
> > +                    output.append(self.indent_str + f'{json.dumps(k)}:{sep}{self.encode(v, k)}')
> > +            output = ',\n'.join(output)
> > +            self.indentation_level -= 1
> > +            return f'{{\n{output}\n{self.indent_str}}}'
> > +
> > +        else:
> > +            return ' ' + json.dumps(o)
> > +
> > +    @property
> > +    def indent_str(self) -> str:
> > +        return ' ' * self.indentation_level * self.indent
> > +
> > +    def iterencode(self, o, **kwargs):
> > +        return self.encode(o)
> 
> That's nice, I like the output.
> 
> > +
> > +
> > +def convert_v2(in_json):
> > +
> > +    ver = 1.0 if 'version' not in in_json.keys() else in_json['version']
> > +
> > +    if ver == 1.0:
> > +        converted = {}
> > +        converted['version'] = 2.0
> > +        converted['algorithms'] = []
> > +
> > +        for k, v in in_json.items():
> > +            if k == 'version':
> > +                continue
> 
> I'd drop this check for the same reason as explained in the previous
> patch.
> 
> > +            converted['algorithms'].append(dict(zip([k], [v])))
> > +    else:
> > +        converted = in_json
> > +
> > +    return json.dumps(converted, cls=Encoder, indent=4, sort_keys=False)
> > +
> > +
> > +if __name__ == "__main__":
> > +    parser = argparse.ArgumentParser(formatter_class=argparse.RawTextHelpFormatter, description=
> > +                    'Convert the format of the Raspberry Pi camera tuning file from v1.0 to v2.0.\n'
> > +                    'If a v2.0 format file is provided, the tool prettifies its contents.')
> > +    parser.add_argument('input', type=str, nargs=1, help='Input tuning file')
> 
> If you drop nargs, you can replace args.input[0] with args.input below.
> 
> > +    parser.add_argument('output', type=str, nargs='?', help='Output converted tuning file', default=None)
> 
> It would be useful to indicate that if the output argument is not
> provided, the tool updates the input file in-place.
> 
> > +    args = parser.parse_args()
> > +
> > +    with open(args.input[0], 'r') as f:
> > +        in_json = json.load(f)
> > +
> > +    out_json = convert_v2(in_json)
> > +
> > +    with open(args.output if args.output is not None else args.input[0], 'w') as f:
> 
>     with open(args.output or args.input, 'w') as f:
> 
> I also wonder if it could be useful to support outputting to stdout by
> specifying '-' as the output file (and maybe the same thing for stdin),
> but that can always be done later if needed.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> 
> > +        f.write(out_json)

-- 
Regards,

Laurent Pinchart