[PATCH v1] utils: Add script and schema to validate tuning files
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Feb 24 00:58:33 CET 2025
On Wed, Jun 05, 2024 at 04:58:29PM +0900, Paul Elder wrote:
> Hi Stefan,
>
> On Wed, May 29, 2024 at 03:15:57PM +0200, Stefan Klug wrote:
> > Add a schema file to validate the rkisp1 tuning files. This allows us to
> > easily check if tuning files got outdated and can be used as implicit
> > documentation for the file format. For now only rkisp1 is supported.
>
> This is amazing.
It is :-)
Stefan, there's no urgency, but a v2 at some point would be nice. I
think this is very valuable.
> > To validate all tuning files, run
> > utils/validate-tuning-files.py --all
> >
> > Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > ---
> > src/ipa/rkisp1/data/tuning-rkisp1.schema.yaml | 376 ++++++++++++++++++
> > utils/validate-tuning-files.py | 112 ++++++
> > 2 files changed, 488 insertions(+)
> > create mode 100644 src/ipa/rkisp1/data/tuning-rkisp1.schema.yaml
> > create mode 100755 utils/validate-tuning-files.py
> >
> > diff --git a/src/ipa/rkisp1/data/tuning-rkisp1.schema.yaml b/src/ipa/rkisp1/data/tuning-rkisp1.schema.yaml
> > new file mode 100644
> > index 00000000..4f72f53e
> > --- /dev/null
> > +++ b/src/ipa/rkisp1/data/tuning-rkisp1.schema.yaml
> > @@ -0,0 +1,376 @@
> > +# SPDX-License-Identifier: CC0-1.0
> > +%YAML 1.1
> > +---
> > +$id: https://libcamera.org/tuning-rkisp1.schema.yaml
> > +$schema: https://json-schema.org/draft/2020-12/schema
> > +description: Tuning file schema for the RKISP1 IPA
> > +type: object
> > +required: ['version', 'algorithms']
> > +properties:
> > + version:
> > + const: 1
> > + algorithms:
> > + type: array
> > + items:
> > + oneOf:
> > + - $ref: '#/$defs/Agc'
> > + - $ref: '#/$defs/Awb'
> > + - $ref: '#/$defs/BlackLevelCorrection'
> > + - $ref: '#/$defs/Ccm'
> > + - $ref: '#/$defs/ColorProcessing'
> > + - $ref: '#/$defs/Dpf'
> > + - $ref: '#/$defs/Filter'
> > + - $ref: '#/$defs/GammaSensorLinearization'
> > + - $ref: '#/$defs/DefectPixelClusterCorrection'
> > + - $ref: '#/$defs/LensShadingCorrection'
>
> I suppose these are just links to other sections in the schema so it
> doesn't really apply to these directly, but I'm wondering if it's worth
> consolidating these names somewhere so that we don't have to update it
> in three places (tuning file parsing in C++, tuning file generator in
> libtuning, and schema validation here). Although considering the former
> two already duplicate and need manual synchronization of these names
> maybe it's fine? Or maybe we need a new solution.
>
> (Also if you meant for these to be in alphabetical order then
> DefectPixelClusterCorrection is out of order)
>
> > +$defs:
> > + Agc:
> > + type: object
> > + additionalProperties: false
> > + required: ['Agc']
> > + properties:
> > + Agc:
> > + type: ['object', 'null']
> > + additionalProperties: false
> > + required: ['AeMeteringMode']
>
> I didn't think anything was required... and now I see the code that I
> wrote does make it requied... I'll have to change that :)
>
> > + properties:
> > + AeMeteringMode:
> > + type: object
> > + additionalProperties: false
> > + required: ['MeteringCentreWeighted', 'MeteringSpot', 'MeteringMatrix']
>
> I think oneOf is fine for metering mode? (Technically the tuning file parser
> works fine with none but then what's the point)
>
> > + properties:
>
> I'm not familiar at all with yaml schemas so idrk but you don't need
> oneOf for maps?
>
> > + MeteringCentreWeighted: { $ref: '#/$defs/_AgcMeteringMode' }
> > + MeteringSpot: { $ref: '#/$defs/_AgcMeteringMode' }
> > + MeteringMatrix: { $ref: '#/$defs/_AgcMeteringMode' }
> > + MeteringCustom: { $ref: '#/$defs/_AgcMeteringMode' }
> > + AeExposureMode:
> > + type: object
> > + additionalProperties: false
> > + properties:
> > + ExposureNormal: { $ref: '#/$defs/_AgcExposureMode' }
> > + ExposureShort: { $ref: '#/$defs/_AgcExposureMode' }
> > + ExposureLong: { $ref: '#/$defs/_AgcExposureMode' }
> > + ExposureCustom: { $ref: '#/$defs/_AgcExposureMode' }
> > + AeConstraintMode:
> > + type: object
> > + additionalProperties: false
> > + properties:
> > + ConstraintNormal: { $ref: '#/$defs/_AgcConstraintMode' }
> > + ConstraintHighlight: { $ref: '#/$defs/_AgcConstraintMode' }
> > + ConstraintShadows: { $ref: '#/$defs/_AgcConstraintMode' }
> > + ConstraintCustom: { $ref: '#/$defs/_AgcConstraintMode' }
> > + relativeLuminanceTarget:
> > + type: array
> > + items:
> > + type: number
> > +
> > + # Ideally these should be defined inside Agc and referenced using relative paths
> > + # but apparently this is not supported by the spec.
> > + # See https://github.com/python-jsonschema/jsonschema/issues/1265
> > + _AgcMeteringMode:
> > + type: array
> > + items:
> > + type: integer
> > + minItems: 25
> > + maxItems: 25
>
> There's a new version that adds another level of map for v10 and v12...
>
> It might've fell through the cracks because I removed the patch that
> adds the tuning file itself.
>
> https://patchwork.libcamera.org/patch/20141/
>
> > + _AgcConstraintMode:
> > + type: object
> > + additionalProperties: false
> > + properties:
> > + lower:
> > + type: object
> > + additionalProperties: false
> > + properties:
> > + qLo:
> > + type: number
> > + qHi:
> > + type: number
> > + yTarget:
> > + type: array
> > + items:
> > + type: number
>
> This makes this patch dependent on v2 of "ipa: libipa: Change constraint
> Y target to piecewise linear function".
>
> Also it would be nice if there was a way to validate that the length is
> a multiple of 2.
>
> > + upper:
> > + type: object
> > + additionalProperties: false
> > + properties:
> > + qLo:
> > + type: number
> > + qHi:
> > + type: number
> > + yTarget:
> > + type: array
> > + items:
> > + type: number
> > + _AgcExposureMode:
> > + type: object
> > + additionalProperties: false
> > + required: ['shutter', 'gain']
> > + properties:
> > + shutter:
> > + type: array
> > + items:
> > + type: integer
> > + gain:
> > + type: array
> > + items:
> > + type: number
>
> Is there a way to validate that these two lengths match?
>
> > +
> > + Awb:
> > + type: object
> > + additionalProperties: false
> > + required: ['Awb']
> > + properties:
> > + Awb: { $ref: '#/$defs/_Empty' }
> > + BlackLevelCorrection:
> > + type: object
> > + additionalProperties: false
> > + required: ['BlackLevelCorrection']
> > + properties:
> > + BlackLevelCorrection:
> > + type: object
> > + additionalProperties: false
> > + required: ['B', 'Gb', 'Gr', 'R']
> > + properties:
> > + B:
> > + type: integer
> > + Gb:
> > + type: integer
> > + Gr:
> > + type: integer
> > + R:
> > + type: integer
> > + Ccm:
> > + type: object
> > + additionalProperties: false
> > + required: ['Ccm']
> > + properties:
> > + Ccm:
> > + type: object
> > + additionalProperties: false
> > + required: ['ccms']
> > + properties:
> > + ccms:
> > + type: array
> > + items:
> > + type: object
> > + additionalProperties: false
> > + required: ['ct', 'ccm']
> > + properties:
> > + ct:
> > + type: integer
> > + ccm:
> > + type: array
> > + items:
> > + type: number
> > + minItems: 9
> > + maxItems: 9
> > + offsets:
> > + type: array
> > + items:
> > + type: integer
> > + minItems: 3
> > + maxItems: 3
> > + ColorProcessing:
> > + type: object
> > + additionalProperties: false
> > + required: ['ColorProcessing']
> > + properties:
> > + ColorProcessing: { $ref: '#/$defs/_Empty' }
> > + DefectPixelClusterCorrection:
> > + type: object
> > + additionalProperties: false
> > + required: ['DefectPixelClusterCorrection']
> > + properties:
> > + DefectPixelClusterCorrection:
> > + type: object
> > + additionalProperties: false
> > + required: ['sets']
> > + properties:
> > + fixed-set:
> > + type: boolean
> > + sets:
> > + type: array
> > + maxItems: 3
> > + items:
> > + type: object
> > + additionalProperties: false
> > + # Todo clarify what is really required here ov5640 misses
> > + # 'rg-factor', 'rnd-offsets', 'rnd-threshold'
>
> The documentation says what's required (they seem to not be the same for
> all three sets), but I think we can probably just make these all
> optional for the purposes of the schema (or at least at-least-one-of?).
>
> > + required: ['line-mad-factor', 'line-threshold', 'pg-factor', 'ro-limits']
> > + properties:
> > + line-mad-factor: { $ref: '#/$defs/_DefectPixelClusterCorrectionFactor' }
> > + line-threshold: { $ref: '#/$defs/_DefectPixelClusterCorrectionFactor' }
> > + pg-factor: { $ref: '#/$defs/_DefectPixelClusterCorrectionFactor' }
> > + rg-factor: { $ref: '#/$defs/_DefectPixelClusterCorrectionFactor' }
> > + rnd-offsets: { $ref: '#/$defs/_DefectPixelClusterCorrectionFactor' }
> > + rnd-threshold: { $ref: '#/$defs/_DefectPixelClusterCorrectionFactor' }
> > + ro-limits: { $ref: '#/$defs/_DefectPixelClusterCorrectionFactor' }
> > + _DefectPixelClusterCorrectionFactor:
> > + type: object
> > + additionalProperties: false
> > + properties:
> > + green:
> > + type: integer
> > + red-blue:
> > + type: integer
> > + Dpf:
> > + type: object
> > + additionalProperties: false
> > + required: ['Dpf']
> > + properties:
> > + Dpf:
> > + type: object
> > + additionalProperties: false
> > + required: ['DomainFilter', 'FilterStrength', 'NoiseLevelFunction']
> > + properties:
> > + DomainFilter:
> > + type: object
> > + additionalProperties: false
> > + required: ['g', 'rb']
> > + properties:
> > + g:
> > + type: array
> > + items:
> > + type: integer
> > + minItems: 6
> > + maxItems: 6
> > + rb:
> > + type: array
> > + items:
> > + type: integer
> > + minItems: 6
> > + maxItems: 6
> > + FilterStrength:
> > + type: object
> > + additionalProperties: false
> > + required: ['b', 'g', 'r']
> > + properties:
> > + b:
> > + type: integer
> > + g:
> > + type: integer
> > + r:
> > + type: integer
> > + NoiseLevelFunction:
> > + type: object
> > + additionalProperties: false
> > + required: ['coeff', 'scale-mode']
> > + properties:
> > + coeff:
> > + type: array
> > + items:
> > + type: integer
> > + minItems: 17
> > + maxItems: 17
> > + scale-mode:
> > + type: string
> > + enum: ['linear', 'logarithmic']
> > + Filter:
> > + type: object
> > + additionalProperties: false
> > + required: ['Filter']
> > + properties:
> > + Filter: { $ref: '#/$defs/_Empty' }
> > + GammaSensorLinearization:
> > + type: object
> > + additionalProperties: false
> > + required: ['GammaSensorLinearization']
> > + properties:
> > + GammaSensorLinearization:
> > + type: object
> > + additionalProperties: false
> > + required: ['x-intervals', 'y']
> > + properties:
> > + x-intervals:
> > + type: array
> > + items:
> > + type: number
>
> I think this should be integer as well.
>
> > + minItems: 16
> > + maxItems: 16
> > + y:
> > + type: object
> > + additionalProperties: false
> > + required: ['red', 'green', 'blue']
> > + properties:
> > + red:
> > + type: array
> > + items:
> > + type: integer
> > + minItems: 17
> > + maxItems: 17
> > + green:
> > + type: array
> > + items:
> > + type: integer
> > + minItems: 17
> > + maxItems: 17
> > + blue:
> > + type: array
> > + items:
> > + type: integer
> > + minItems: 17
> > + maxItems: 17
> > + LensShadingCorrection:
> > + type: object
> > + additionalProperties: false
> > + required: ['LensShadingCorrection']
> > + properties:
> > + LensShadingCorrection:
> > + type: object
> > + additionalProperties: false
> > + required: [x-size, y-size, sets]
> > + properties:
> > + x-size:
> > + type: array
> > + items:
> > + type: number
> > + minItems: 8
> > + maxItems: 8
> > + y-size:
> > + type: array
> > + items:
> > + type: number
> > + minItems: 8
> > + maxItems: 8
> > + sets:
> > + type: array
> > + items:
> > + type: object
> > + additionalProperties: false
> > + required: ['ct', 'r', 'gr', 'gb', 'b']
> > + properties:
> > + ct:
> > + type: integer
> > + r:
> > + type: array
> > + items:
> > + type: integer
> > + minItems: 289
> > + maxItems: 289
> > + gr:
> > + type: array
> > + items:
> > + type: integer
> > + minItems: 289
> > + maxItems: 289
> > + gb:
> > + type: array
> > + items:
> > + type: integer
> > + minItems: 289
> > + maxItems: 289
> > + b:
> > + type: array
> > + items:
> > + type: integer
> > + minItems: 289
> > + maxItems: 289
> > + # ToDo: We have some tuning files with that property. Is this needed?
>
> Considering the lsc algorithm module in the rkisp1 IPA doesn't even
> check for it, I don't think we need it.
>
> > + resolution:
> > + type: string
> > + # special definition to denote algorithms that do not have any parameters
> > + _Empty:
> > + type: [ 'object', 'null' ]
> > + additionalProperties: false
> > + properties: {}
> > +...
> > diff --git a/utils/validate-tuning-files.py b/utils/validate-tuning-files.py
> > new file mode 100755
> > index 00000000..45f1d863
> > --- /dev/null
> > +++ b/utils/validate-tuning-files.py
> > @@ -0,0 +1,112 @@
> > +#!/usr/bin/env python3
> > +# SPDX-License-Identifier: GPL-2.0-or-later
> > +# Copyright (C) 2024, Ideas on Board Oy
> > +#
> > +# Author: Stefan Klug <klug.stefan at ideasonboard.com>
> > +#
> > +# Validate tuning files against schema
> > +
> > +import argparse
> > +import logging
> > +import jsonschema
> > +import yaml
> > +import sys
> > +import json
> > +import os
> > +from pathlib import Path
>
> (alphabetical order)
>
> > +
> > +try:
> > + import coloredlogs
> > + coloredlogs.install(level=logging.INFO, fmt='%(levelname)s: %(message)s')
> > +except ImportError:
> > + logging.basicConfig(level=logging.INFO,
> > + format="%(levelname)s: %(message)s")
>
> Oh that's fancy, I didn't realize this pattern.
>
> > +
> > +logger = logging.getLogger()
> > +
> > +
> > +def do_schema_check(schema_file, files):
> > + res = True
> > + with open(schema_file, 'r') as file:
> > + schema = yaml.safe_load(file)
> > +
> > + for file in files:
> > + with open(file, 'r') as f:
> > + logger.info(f"Validating file {file} against {schema_file}")
> > + data = yaml.safe_load(f)
> > + try:
> > + jsonschema.validate(instance=data, schema=schema)
> > + except jsonschema.exceptions.ValidationError as e:
> > + logging.error(f"Validation error in file {file}: {e}")
> > + res = False
> > + return res
> > +
> > +# Checks for the given ipa. If files is None, all files for that ipa get checked
> > +
> > +
> > +def do_schema_check_ipa(ipa, files=None):
> > + res = True
> > +
> > + if ipa != 'rkisp1':
> > + raise ValueError(f"Ipa '{ipa}' is not supported")
> > +
> > + logger.info(f"Checking ipa {ipa}")
> > +
> > + top_dir = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
> > + # Todo implement for other pipelines
> > + data_dir = 'src/ipa/rkisp1/data'
> > +
> > + full_dir = Path(os.path.join(top_dir, data_dir))
>
> Why not Path(top_dir) / Path(data_dir)?
>
> > +
> > + schema_path = os.path.join(full_dir, f'tuning-{ipa}.schema.yaml')
> > + if not os.path.isfile(schema_path):
> > + raise ValueError(f"Schema file {schema_path} doesn't exist")
> > +
> > + if files is None:
> > + files = [f for f in full_dir.glob(
> > + '*.yaml') if f.is_file() and not f.name.endswith('.schema.yaml')]
>
> I was going to say instead of doing all this we could take the files as
> parameters... and I see now that you do allow that and this is just a
> convenience "all" option :)
>
> I wonder if we need to check in some way that we're in a libcamera
> tree or if os.path.isfile() is sufficient.
>
> > +
> > + if not do_schema_check(schema_path, files):
> > + res = False
> > +
> > + return res
> > +
> > +
> > +def do_schema_check_all():
> > + # We only support rkisp1 for now
> > + return do_schema_check_ipa('rkisp1')
> > +
> > +
> > +def main():
> > +
> > + parser = argparse.ArgumentParser()
> > + group = parser.add_mutually_exclusive_group(required=True)
> > + group.add_argument('--all', '-a', action='store_true',
> > + help='automatically find and check all tuning files.')
> > + group.add_argument('--schema', '-s',
> > + help='schema file to check against')
> > + group.add_argument('--pipeline', '-p',
> > + help='pipeline to check against (Currently only rkisp1 is supported)')
> > + parser.add_argument('files', metavar='FILE', nargs='*',
> > + help='tuning files to check')
> > +
> > + args = parser.parse_args()
> > +
> > + if args.all:
> > + if args.files != []:
> > + parser.error("No files should be given when using --all")
>
> Should we not error out here?
>
>
> Thanks,
>
> Paul
>
> > +
> > + if not do_schema_check_all():
> > + return 1
> > + return 0
> > +
> > + if not args.files:
> > + argparse.error("No files to check")
> > + return 1
> > +
> > + if not do_schema_check(args.schema, args.files):
> > + return 1
> > +
> > +
> > +if __name__ == '__main__':
> > + sys.exit(main())
> > --
> > 2.43.0
> >
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list