[v4,1/2] utils: libtuning: modules: Add skeletal AGC module

20321 diff mbox series

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@ideasonboard.com>
Reviewed-by: Stefan Klug <stefan.klug@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

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@ideasonboard.com>
> Reviewed-by: Stefan Klug <stefan.klug@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@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@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?)

> +    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@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?

Anyway ... I think this gets us moving forwards so:


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> +
> +    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
> +        ]
> +
> +        matrix = [1 for i in range(0, 25)]
> +
> +        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
> -- 
> 2.39.2
>

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@ideasonboard.com>
> > Reviewed-by: Stefan Klug <stefan.klug@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@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@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@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@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
> > +        ]
> > +
> > +        matrix = [1 for i in range(0, 25)]
> > +
> > +        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
> > -- 
> > 2.39.2
> >

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@ideasonboard.com>
> > > Reviewed-by: Stefan Klug <stefan.klug@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@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@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@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@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