[v2,1/5] utils: libtuning: modules: Add skeletal AGC module
diff mbox series

Message ID 20240517075751.3866269-2-paul.elder@ideasonboard.com
State Superseded
Headers show
Series
  • utils: tuning: Add AGC and CCM
Related show

Commit Message

Paul Elder May 17, 2024, 7:57 a.m. UTC 
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 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  | 112 ++++++++++++++++++
 3 files changed, 139 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

Comments

Dan Scally May 20, 2024, 1:19 p.m. UTC | #1 
Hi Paul, thanks for the patches

On 17/05/2024 08:57, Paul Elder wrote:
> 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 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  | 112 ++++++++++++++++++
>   3 files changed, 139 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 000000000..4db9ca371
> --- /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 000000000..9c8899bad
> --- /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)'
> +    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 000000000..4ac4d8ce7
> --- /dev/null
> +++ b/utils/tuning/libtuning/modules/agc/rkisp1.py
> @@ -0,0 +1,112 @@
> +# 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'
> +    # \todo Not sure if this is useful. Probably will remove later.
> +    compatible = ['rkisp1']

$ grep -Ir compatible utils/tuning/
utils/tuning/libtuning/modules/lsc/raspberrypi.py:    compatible = ['raspberrypi']
utils/tuning/libtuning/modules/lsc/rkisp1.py:    compatible = ['rkisp1']
utils/tuning/libtuning/generators/raspberrypi_output.py: raise RuntimeError('Incompatible JSON 
dictionary has been provided')


Nothing's using it - let's just remove it now.

> +
> +    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
> +
> +    def _generate_metering_modes(self) -> dict:
> +        centre_weighted = {
> +                'v10': [
> +                    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
> +                ],
> +
> +                'v12': [
> +                    0, 0, 0, 0,  0, 0, 0, 0, 0,
> +                    0, 0, 0, 0,  0, 0, 0, 0, 0,
> +                    0, 0, 0, 3,  4, 3, 0, 0, 0,
> +                    0, 0, 3, 6,  8, 6, 3, 0, 0,
> +                    0, 0, 4, 8, 16, 8, 4, 0, 0,
> +                    0, 0, 3, 6,  8, 6, 3, 0, 0,
> +                    0, 0, 0, 3,  4, 3, 0, 0, 0,
> +                    0, 0, 0, 0,  0, 0, 0, 0, 0,
> +                    0, 0, 0, 0,  0, 0, 0, 0, 0,
> +                ]
> +        }
> +
> +        spot = {
> +                'v10': [
> +                    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
> +                ],
> +
> +                'v12': [
> +                    0, 0, 0, 0,  0, 0, 0, 0, 0,
> +                    0, 0, 0, 0,  0, 0, 0, 0, 0,
> +                    0, 0, 0, 1,  2, 1, 0, 0, 0,
> +                    0, 0, 1, 2,  4, 2, 1, 0, 0,
> +                    0, 0, 2, 4, 16, 4, 2, 0, 0,
> +                    0, 0, 1, 2,  4, 2, 1, 0, 0,
> +                    0, 0, 0, 1,  2, 1, 0, 0, 0,
> +                    0, 0, 0, 0,  0, 0, 0, 0, 0,
> +                    0, 0, 0, 0,  0, 0, 0, 0, 0,
> +                ]
> +        }
> +
> +        matrix = {
> +                'v10': [1 for i in range(0, 25)],
> +                'v12': [1 for i in range(0, 81)]
> +        }
> +
> +        return {
> +            'MeteringCentreWeighted': centre_weighted,
> +            'MeteringSpot': spot,
> +            'MeteringMatrix': matrix
> +        }
> +
> +    def _generate_exposure_modes(self) -> dict:
> +        normal = { 'shutter': [100, 10000, 30000, 60000, 120000],
> +                   'gain': [1.0, 2.0, 4.0, 6.0, 6.0] }
> +        short = { 'shutter': [100, 5000, 10000, 20000, 120000],
> +                  'gain': [1.0, 2.0, 4.0, 6.0, 6.0]}

Two things spring to mind here. First; the first entry in the gain array should be > 1.0. I know 
that in the rpi implementation we cribbed this from they start with 1.0 but that never made sense to 
me, and doesn't align with the description of the algorithm from their tuning guide:


"First, the desired analogue gain is set to 1 and the shutter time is allowed to ramp to the first 
value in the list. Thereafter, the analogue gain is allowed to ramp to the first value in its list. 
The procedure then simply repeats with the second value in each of the lists"


The ExposureModeHelper works fine with the first value as 1.0, but it's a wasted cycle since it's 
assuming aGain to be 1.0 initially anyway. The second thought is that this is perhaps of slightly 
limited value since the script has no idea whether or not the values are appropriate for the sensor, 
and so perhaps we should generate a comment for the Yaml output that clarifies that the limits of 
the sensor must be investigated and the values replaced with ones that make more sense?...or perhaps 
we should start a live tuning version of the scripts that uses the python bindings to get the 
control limits :D

> +
> +        return { 'ExposureNormal': normal, 'ExposureShort': short }
> +
> +    def _generate_constraint_modes(self) -> dict:
> +        normal = { 'lower': { 'qLo': 0.98, 'qHi': 1.0, 'yTarget': [ 0, 0.5, 1000, 0.5 ] } }
> +        highlight = {
> +            'lower': { 'qLo': 0.98, 'qHi': 1.0, 'yTarget': [ 0, 0.5, 1000, 0.5 ] },
> +            'upper': { 'qLo': 0.98, 'qHi': 1.0, 'yTarget': [ 0, 0.8, 1000, 0.5 ] }
> +        }
> +
> +        return { 'ConstraintNormal': normal, 'ConstraintHighlight': highlight }
This is fine, but in that case this series depends on work to make the constraint Y target be a Pwl 
being merged before it
> +
> +    def _generate_y_target(self) -> list:
> +        return [0, 0.16, 1000, 0.165, 10000, 0.17]
> +
> +    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()


Cool!

> +
> +        # \todo Debug functionality
> +
> +        return output

Patch
diff mbox series 

diff --git a/utils/tuning/libtuning/modules/agc/__init__.py b/utils/tuning/libtuning/modules/agc/__init__.py
new file mode 100644
index 000000000..4db9ca371
--- /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 000000000..9c8899bad
--- /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)'
+    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 000000000..4ac4d8ce7
--- /dev/null
+++ b/utils/tuning/libtuning/modules/agc/rkisp1.py
@@ -0,0 +1,112 @@ 
+# 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'
+    # \todo Not sure if this is useful. Probably will remove later.
+    compatible = ['rkisp1']
+
+    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
+
+    def _generate_metering_modes(self) -> dict:
+        centre_weighted = {
+                'v10': [
+                    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
+                ],
+
+                'v12': [
+                    0, 0, 0, 0,  0, 0, 0, 0, 0,
+                    0, 0, 0, 0,  0, 0, 0, 0, 0,
+                    0, 0, 0, 3,  4, 3, 0, 0, 0,
+                    0, 0, 3, 6,  8, 6, 3, 0, 0,
+                    0, 0, 4, 8, 16, 8, 4, 0, 0,
+                    0, 0, 3, 6,  8, 6, 3, 0, 0,
+                    0, 0, 0, 3,  4, 3, 0, 0, 0,
+                    0, 0, 0, 0,  0, 0, 0, 0, 0,
+                    0, 0, 0, 0,  0, 0, 0, 0, 0,
+                ]
+        }
+
+        spot = {
+                'v10': [
+                    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
+                ],
+
+                'v12': [
+                    0, 0, 0, 0,  0, 0, 0, 0, 0,
+                    0, 0, 0, 0,  0, 0, 0, 0, 0,
+                    0, 0, 0, 1,  2, 1, 0, 0, 0,
+                    0, 0, 1, 2,  4, 2, 1, 0, 0,
+                    0, 0, 2, 4, 16, 4, 2, 0, 0,
+                    0, 0, 1, 2,  4, 2, 1, 0, 0,
+                    0, 0, 0, 1,  2, 1, 0, 0, 0,
+                    0, 0, 0, 0,  0, 0, 0, 0, 0,
+                    0, 0, 0, 0,  0, 0, 0, 0, 0,
+                ]
+        }
+
+        matrix = {
+                'v10': [1 for i in range(0, 25)],
+                'v12': [1 for i in range(0, 81)]
+        }
+
+        return {
+            'MeteringCentreWeighted': centre_weighted,
+            'MeteringSpot': spot,
+            'MeteringMatrix': matrix
+        }
+
+    def _generate_exposure_modes(self) -> dict:
+        normal = { 'shutter': [100, 10000, 30000, 60000, 120000],
+                   'gain': [1.0, 2.0, 4.0, 6.0, 6.0] }
+        short = { 'shutter': [100, 5000, 10000, 20000, 120000],
+                  'gain': [1.0, 2.0, 4.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, 0.5, 1000, 0.5 ] } }
+        highlight = {
+            'lower': { 'qLo': 0.98, 'qHi': 1.0, 'yTarget': [ 0, 0.5, 1000, 0.5 ] },
+            'upper': { 'qLo': 0.98, 'qHi': 1.0, 'yTarget': [ 0, 0.8, 1000, 0.5 ] }
+        }
+
+        return { 'ConstraintNormal': normal, 'ConstraintHighlight': highlight }
+
+    def _generate_y_target(self) -> list:
+        return [0, 0.16, 1000, 0.165, 10000, 0.17]
+
+    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