[PATCH v4 1/2] utils: libtuning: modules: Add skeletal AGC module
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sat Jun 15 19:45:50 CEST 2024
Hi Paul,
I know this has been merged, but I have a question.
On Fri, Jun 14, 2024 at 11:19:36PM +0900, Paul Elder wrote:
> On Fri, Jun 14, 2024 at 02:32:56PM +0100, Kieran Bingham wrote:
> > Quoting Paul Elder (2024-06-14 12:40:15)
> > > Add a skeletal AGC module just so that we can have some AGC tuning
> > > values that we can use to test during development of AGC in the IPAs. As
> > > rkisp1 is the main target, we only add support for rkisp1 for now.
> > >
> > > The parameters are mostly copied from the hardcoded values in ctt,
> > > except for the metering modes.
> > >
> > > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > > Reviewed-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > >
> > > ---
> > > Changes in v4:
> > > - remove support for both v10 and v12, and only support v10
> > > - remove piecewise linear function luminance targets
> > >
> > > Changes in v3:
> > > - remove unused compatible string
> > > - make gain start from 2 in the exposure modes
> > >
> > > Changes in v2:
> > > - remove unneccessary imports
> > > - add support for both v10 and v12
> > > ---
> > > .../tuning/libtuning/modules/agc/__init__.py | 6 ++
> > > utils/tuning/libtuning/modules/agc/agc.py | 21 +++++
> > > utils/tuning/libtuning/modules/agc/rkisp1.py | 79 +++++++++++++++++++
> > > 3 files changed, 106 insertions(+)
> > > create mode 100644 utils/tuning/libtuning/modules/agc/__init__.py
> > > create mode 100644 utils/tuning/libtuning/modules/agc/agc.py
> > > create mode 100644 utils/tuning/libtuning/modules/agc/rkisp1.py
> > >
> > > diff --git a/utils/tuning/libtuning/modules/agc/__init__.py b/utils/tuning/libtuning/modules/agc/__init__.py
> > > new file mode 100644
> > > index 000000000000..4db9ca371c68
> > > --- /dev/null
> > > +++ b/utils/tuning/libtuning/modules/agc/__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.agc.agc import AGC
> > > +from libtuning.modules.agc.rkisp1 import AGCRkISP1
> > > diff --git a/utils/tuning/libtuning/modules/agc/agc.py b/utils/tuning/libtuning/modules/agc/agc.py
> > > new file mode 100644
> > > index 000000000000..9c8899badc79
> > > --- /dev/null
> > > +++ b/utils/tuning/libtuning/modules/agc/agc.py
> > > @@ -0,0 +1,21 @@
> > > +# SPDX-License-Identifier: BSD-2-Clause
> > > +#
> > > +# Copyright (C) 2019, Raspberry Pi Ltd
> > > +# Copyright (C) 2024, Paul Elder <paul.elder at ideasonboard.com>
> > > +
> > > +from ..module import Module
> > > +
> > > +import libtuning as lt
> > > +
> > > +
> > > +class AGC(Module):
> > > + type = 'agc'
> > > + hr_name = 'AGC (Base)'
> >
> > What's an hr_name? (hr?)
>
> human-readable (which ironically itself is not very human-readable but)
>
> > > + out_name = 'GenericAGC'
> > > +
> > > + # \todo Add sector shapes and stuff just like lsc
> > > + def __init__(self, *,
> > > + debug: list):
> > > + super().__init__()
> > > +
> > > + self.debug = debug
> > > diff --git a/utils/tuning/libtuning/modules/agc/rkisp1.py b/utils/tuning/libtuning/modules/agc/rkisp1.py
> > > new file mode 100644
> > > index 000000000000..babb441a8300
> > > --- /dev/null
> > > +++ b/utils/tuning/libtuning/modules/agc/rkisp1.py
> > > @@ -0,0 +1,79 @@
> > > +# SPDX-License-Identifier: BSD-2-Clause
> > > +#
> > > +# Copyright (C) 2019, Raspberry Pi Ltd
> > > +# Copyright (C) 2024, Paul Elder <paul.elder at ideasonboard.com>
> > > +#
> > > +# rkisp1.py - AGC module for tuning rkisp1
> > > +
> > > +from .agc import AGC
> > > +
> > > +import libtuning as lt
> > > +
> > > +
> > > +class AGCRkISP1(AGC):
> > > + hr_name = 'AGC (RkISP1)'
> > > + out_name = 'Agc'
> > > +
> > > + def __init__(self, **kwargs):
> > > + super().__init__(**kwargs)
> > > +
> > > + # We don't actually need anything from the config file
> > > + def validate_config(self, config: dict) -> bool:
> > > + return True
> >
> > Would you ever anticipate putting these hardcoded values into a separate
> > config file?
>
> Oh maybe.
>
> >
> > Anyway ... I think this gets us moving forwards so:
> >
> >
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> >
>
> Thanks :)
>
>
> Paul
>
> > > +
> > > + def _generate_metering_modes(self) -> dict:
> > > + centre_weighted = [
> > > + 0, 0, 0, 0, 0,
> > > + 0, 6, 8, 6, 0,
> > > + 0, 8, 16, 8, 0,
> > > + 0, 6, 8, 6, 0,
> > > + 0, 0, 0, 0, 0
> > > + ]
> > > +
> > > + spot = [
> > > + 0, 0, 0, 0, 0,
> > > + 0, 2, 4, 2, 0,
> > > + 0, 4, 16, 4, 0,
> > > + 0, 2, 4, 2, 0,
> > > + 0, 0, 0, 0, 0
> > > + ]
How did you come up with those values ? The centre weighted matrix, in
particular, seems quite, well, centered :-) Downsampling the rpi4 matrix
from ctt I would have expected
[[ 0 0 5 0 0]
[ 0 10 10 10 0]
[ 5 10 16 10 5]
[ 0 10 10 10 0]
[ 0 0 5 0 0]]
Similarly, for spot metering, the downsampled rpi4 matrix is closer to
[[ 0 0 0 0 0]
[ 0 0 1 0 0]
[ 0 1 16 1 0]
[ 0 0 1 0 0]
[ 0 0 0 0 0]]
We can use other values, but I wonder where they come from.
> > > +
> > > + matrix = [1 for i in range(0, 25)]
Should this be 16, not 1, to fill up the histogram bins ? The IPA module
assumes weights of 16 when calculating the predivider.
OK, that was two questions :-)
> > > +
> > > + return {
> > > + 'MeteringCentreWeighted': centre_weighted,
> > > + 'MeteringSpot': spot,
> > > + 'MeteringMatrix': matrix
> > > + }
> > > +
> > > + def _generate_exposure_modes(self) -> dict:
> > > + normal = { 'shutter': [100, 10000, 30000, 60000, 120000],
> > > + 'gain': [2.0, 4.0, 6.0, 6.0, 6.0]}
> > > + short = { 'shutter': [100, 5000, 10000, 20000, 120000],
> > > + 'gain': [2.0, 4.0, 6.0, 6.0, 6.0]}
> > > +
> > > + return { 'ExposureNormal': normal, 'ExposureShort': short }
> > > +
> > > + def _generate_constraint_modes(self) -> dict:
> > > + normal = { 'lower': { 'qLo': 0.98, 'qHi': 1.0, 'yTarget': 0.5 } }
> > > + highlight = {
> > > + 'lower': { 'qLo': 0.98, 'qHi': 1.0, 'yTarget': 0.5 },
> > > + 'upper': { 'qLo': 0.98, 'qHi': 1.0, 'yTarget': 0.8 }
> > > + }
> > > +
> > > + return { 'ConstraintNormal': normal, 'ConstraintHighlight': highlight }
> > > +
> > > + def _generate_y_target(self) -> list:
> > > + return 0.16
> > > +
> > > + def process(self, config: dict, images: list, outputs: dict) -> dict:
> > > + output = {}
> > > +
> > > + output['AeMeteringMode'] = self._generate_metering_modes()
> > > + output['AeExposureMode'] = self._generate_exposure_modes()
> > > + output['AeConstraintMode'] = self._generate_constraint_modes()
> > > + output['relativeLuminanceTarget'] = self._generate_y_target()
> > > +
> > > + # \todo Debug functionality
> > > +
> > > + return output
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list