[{"id":29575,"web_url":"https://patchwork.libcamera.org/comment/29575/","msgid":"<70df9566-fc3d-44fe-bb63-4ab7f2a60ad3@ideasonboard.com>","date":"2024-05-20T13:19:03","subject":"Re: [PATCH v2 1/5] utils: libtuning: modules: Add skeletal AGC\n\tmodule","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Paul, thanks for the patches\n\nOn 17/05/2024 08:57, Paul Elder wrote:\n> Add a skeletal AGC module just so that we can have some AGC tuning\n> values that we can use to test during development of AGC in the IPAs. As\n> rkisp1 is the main target, we only add support for rkisp1 for now.\n>\n> The parameters are mostly copied from the hardcoded values in ctt,\n> except for the metering modes.\n>\n> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>\n>\n> ---\n> Changes in v2:\n> - remove unneccessary imports\n> - add support for both v10 and v12\n> ---\n>   .../tuning/libtuning/modules/agc/__init__.py  |   6 +\n>   utils/tuning/libtuning/modules/agc/agc.py     |  21 ++++\n>   utils/tuning/libtuning/modules/agc/rkisp1.py  | 112 ++++++++++++++++++\n>   3 files changed, 139 insertions(+)\n>   create mode 100644 utils/tuning/libtuning/modules/agc/__init__.py\n>   create mode 100644 utils/tuning/libtuning/modules/agc/agc.py\n>   create mode 100644 utils/tuning/libtuning/modules/agc/rkisp1.py\n>\n> diff --git a/utils/tuning/libtuning/modules/agc/__init__.py b/utils/tuning/libtuning/modules/agc/__init__.py\n> new file mode 100644\n> index 000000000..4db9ca371\n> --- /dev/null\n> +++ b/utils/tuning/libtuning/modules/agc/__init__.py\n> @@ -0,0 +1,6 @@\n> +# SPDX-License-Identifier: GPL-2.0-or-later\n> +#\n> +# Copyright (C) 2024, Paul Elder <paul.elder@ideasonboard.com>\n> +\n> +from libtuning.modules.agc.agc import AGC\n> +from libtuning.modules.agc.rkisp1 import AGCRkISP1\n> diff --git a/utils/tuning/libtuning/modules/agc/agc.py b/utils/tuning/libtuning/modules/agc/agc.py\n> new file mode 100644\n> index 000000000..9c8899bad\n> --- /dev/null\n> +++ b/utils/tuning/libtuning/modules/agc/agc.py\n> @@ -0,0 +1,21 @@\n> +# SPDX-License-Identifier: BSD-2-Clause\n> +#\n> +# Copyright (C) 2019, Raspberry Pi Ltd\n> +# Copyright (C) 2024, Paul Elder <paul.elder@ideasonboard.com>\n> +\n> +from ..module import Module\n> +\n> +import libtuning as lt\n> +\n> +\n> +class AGC(Module):\n> +    type = 'agc'\n> +    hr_name = 'AGC (Base)'\n> +    out_name = 'GenericAGC'\n> +\n> +    # \\todo Add sector shapes and stuff just like lsc\n> +    def __init__(self, *,\n> +                 debug: list):\n> +        super().__init__()\n> +\n> +        self.debug = debug\n> diff --git a/utils/tuning/libtuning/modules/agc/rkisp1.py b/utils/tuning/libtuning/modules/agc/rkisp1.py\n> new file mode 100644\n> index 000000000..4ac4d8ce7\n> --- /dev/null\n> +++ b/utils/tuning/libtuning/modules/agc/rkisp1.py\n> @@ -0,0 +1,112 @@\n> +# SPDX-License-Identifier: BSD-2-Clause\n> +#\n> +# Copyright (C) 2019, Raspberry Pi Ltd\n> +# Copyright (C) 2024, Paul Elder <paul.elder@ideasonboard.com>\n> +#\n> +# rkisp1.py - AGC module for tuning rkisp1\n> +\n> +from .agc import AGC\n> +\n> +import libtuning as lt\n> +\n> +\n> +class AGCRkISP1(AGC):\n> +    hr_name = 'AGC (RkISP1)'\n> +    out_name = 'Agc'\n> +    # \\todo Not sure if this is useful. Probably will remove later.\n> +    compatible = ['rkisp1']\n\n$ grep -Ir compatible utils/tuning/\nutils/tuning/libtuning/modules/lsc/raspberrypi.py:    compatible = ['raspberrypi']\nutils/tuning/libtuning/modules/lsc/rkisp1.py:    compatible = ['rkisp1']\nutils/tuning/libtuning/generators/raspberrypi_output.py: raise RuntimeError('Incompatible JSON \ndictionary has been provided')\n\n\nNothing's using it - let's just remove it now.\n\n> +\n> +    def __init__(self, *, **kwargs):\n> +        super().__init__(**kwargs)\n> +\n> +    # We don't actually need anything from the config file\n> +    def validate_config(self, config: dict) -> bool:\n> +        return True\n> +\n> +    def _generate_metering_modes(self) -> dict:\n> +        centre_weighted = {\n> +                'v10': [\n> +                    0, 0,  0, 0, 0,\n> +                    0, 6,  8, 6, 0,\n> +                    0, 8, 16, 8, 0,\n> +                    0, 6,  8, 6, 0,\n> +                    0, 0,  0, 0, 0\n> +                ],\n> +\n> +                'v12': [\n> +                    0, 0, 0, 0,  0, 0, 0, 0, 0,\n> +                    0, 0, 0, 0,  0, 0, 0, 0, 0,\n> +                    0, 0, 0, 3,  4, 3, 0, 0, 0,\n> +                    0, 0, 3, 6,  8, 6, 3, 0, 0,\n> +                    0, 0, 4, 8, 16, 8, 4, 0, 0,\n> +                    0, 0, 3, 6,  8, 6, 3, 0, 0,\n> +                    0, 0, 0, 3,  4, 3, 0, 0, 0,\n> +                    0, 0, 0, 0,  0, 0, 0, 0, 0,\n> +                    0, 0, 0, 0,  0, 0, 0, 0, 0,\n> +                ]\n> +        }\n> +\n> +        spot = {\n> +                'v10': [\n> +                    0, 0,  0, 0, 0,\n> +                    0, 2,  4, 2, 0,\n> +                    0, 4, 16, 4, 0,\n> +                    0, 2,  4, 2, 0,\n> +                    0, 0,  0, 0, 0\n> +                ],\n> +\n> +                'v12': [\n> +                    0, 0, 0, 0,  0, 0, 0, 0, 0,\n> +                    0, 0, 0, 0,  0, 0, 0, 0, 0,\n> +                    0, 0, 0, 1,  2, 1, 0, 0, 0,\n> +                    0, 0, 1, 2,  4, 2, 1, 0, 0,\n> +                    0, 0, 2, 4, 16, 4, 2, 0, 0,\n> +                    0, 0, 1, 2,  4, 2, 1, 0, 0,\n> +                    0, 0, 0, 1,  2, 1, 0, 0, 0,\n> +                    0, 0, 0, 0,  0, 0, 0, 0, 0,\n> +                    0, 0, 0, 0,  0, 0, 0, 0, 0,\n> +                ]\n> +        }\n> +\n> +        matrix = {\n> +                'v10': [1 for i in range(0, 25)],\n> +                'v12': [1 for i in range(0, 81)]\n> +        }\n> +\n> +        return {\n> +            'MeteringCentreWeighted': centre_weighted,\n> +            'MeteringSpot': spot,\n> +            'MeteringMatrix': matrix\n> +        }\n> +\n> +    def _generate_exposure_modes(self) -> dict:\n> +        normal = { 'shutter': [100, 10000, 30000, 60000, 120000],\n> +                   'gain': [1.0, 2.0, 4.0, 6.0, 6.0] }\n> +        short = { 'shutter': [100, 5000, 10000, 20000, 120000],\n> +                  'gain': [1.0, 2.0, 4.0, 6.0, 6.0]}\n\nTwo things spring to mind here. First; the first entry in the gain array should be > 1.0. I know \nthat in the rpi implementation we cribbed this from they start with 1.0 but that never made sense to \nme, and doesn't align with the description of the algorithm from their tuning guide:\n\n\n\"First, the desired analogue gain is set to 1 and the shutter time is allowed to ramp to the first \nvalue in the list. Thereafter, the analogue gain is allowed to ramp to the first value in its list. \nThe procedure then simply repeats with the second value in each of the lists\"\n\n\nThe ExposureModeHelper works fine with the first value as 1.0, but it's a wasted cycle since it's \nassuming aGain to be 1.0 initially anyway. The second thought is that this is perhaps of slightly \nlimited value since the script has no idea whether or not the values are appropriate for the sensor, \nand so perhaps we should generate a comment for the Yaml output that clarifies that the limits of \nthe sensor must be investigated and the values replaced with ones that make more sense?...or perhaps \nwe should start a live tuning version of the scripts that uses the python bindings to get the \ncontrol limits :D\n\n> +\n> +        return { 'ExposureNormal': normal, 'ExposureShort': short }\n> +\n> +    def _generate_constraint_modes(self) -> dict:\n> +        normal = { 'lower': { 'qLo': 0.98, 'qHi': 1.0, 'yTarget': [ 0, 0.5, 1000, 0.5 ] } }\n> +        highlight = {\n> +            'lower': { 'qLo': 0.98, 'qHi': 1.0, 'yTarget': [ 0, 0.5, 1000, 0.5 ] },\n> +            'upper': { 'qLo': 0.98, 'qHi': 1.0, 'yTarget': [ 0, 0.8, 1000, 0.5 ] }\n> +        }\n> +\n> +        return { 'ConstraintNormal': normal, 'ConstraintHighlight': highlight }\nThis is fine, but in that case this series depends on work to make the constraint Y target be a Pwl \nbeing merged before it\n> +\n> +    def _generate_y_target(self) -> list:\n> +        return [0, 0.16, 1000, 0.165, 10000, 0.17]\n> +\n> +    def process(self, config: dict, images: list, outputs: dict) -> dict:\n> +        output = {}\n> +\n> +        output['AeMeteringMode'] = self._generate_metering_modes()\n> +        output['AeExposureMode'] = self._generate_exposure_modes()\n> +        output['AeConstraintMode'] = self._generate_constraint_modes()\n> +        output['relativeLuminanceTarget'] = self._generate_y_target()\n\n\nCool!\n\n> +\n> +        # \\todo Debug functionality\n> +\n> +        return output","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 73ECDBD78E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 20 May 2024 13:19:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5705A6347C;\n\tMon, 20 May 2024 15:19:08 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6968561A58\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 20 May 2024 15:19:06 +0200 (CEST)","from [192.168.0.43]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 10332C67\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 20 May 2024 15:18:55 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"pDYJ3Dxl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1716211135;\n\tbh=/REdD17qNMySDgJJ4o4I3Vsnkg7awsmQ2LcUFSCuswI=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=pDYJ3DxlzKrXV/ABub3vddcoxAtNOqeesDOd0lHRxiA7fthKnSLP+/h9L0FFQo7Pm\n\tcSaweEDCyO+CtyWQDq6YgBdAfVASNF0V8/GE16fIsD2eqLbTkSqjc2PzRf/N1BNC+D\n\t1aBbXK6i7qUlikcGcdWhCoWpqFhcihcq9bP//304=","Message-ID":"<70df9566-fc3d-44fe-bb63-4ab7f2a60ad3@ideasonboard.com>","Date":"Mon, 20 May 2024 14:19:03 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 1/5] utils: libtuning: modules: Add skeletal AGC\n\tmodule","To":"libcamera-devel@lists.libcamera.org","References":"<20240517075751.3866269-1-paul.elder@ideasonboard.com>\n\t<20240517075751.3866269-2-paul.elder@ideasonboard.com>","Content-Language":"en-US","From":"Dan Scally <dan.scally@ideasonboard.com>","Autocrypt":"addr=dan.scally@ideasonboard.com; keydata=\n\txsFNBGLydlEBEADa5O2s0AbUguprfvXOQun/0a8y2Vk6BqkQALgeD6KnXSWwaoCULp18etYW\n\tB31bfgrdphXQ5kUQibB0ADK8DERB4wrzrUb5CMxLBFE7mQty+v5NsP0OFNK9XTaAOcmD+Ove\n\teIjYvqurAaro91jrRVrS1gBRxIFqyPgNvwwL+alMZhn3/2jU2uvBmuRrgnc/e9cHKiuT3Dtq\n\tMHGPKL2m+plk+7tjMoQFfexoQ1JKugHAjxAhJfrkXh6uS6rc01bYCyo7ybzg53m1HLFJdNGX\n\tsUKR+dQpBs3SY4s66tc1sREJqdYyTsSZf80HjIeJjU/hRunRo4NjRIJwhvnK1GyjOvvuCKVU\n\tRWpY8dNjNu5OeAfdrlvFJOxIE9M8JuYCQTMULqd1NuzbpFMjc9524U3Cngs589T7qUMPb1H1\n\tNTA81LmtJ6Y+IV5/kiTUANflpzBwhu18Ok7kGyCq2a2jsOcVmk8gZNs04gyjuj8JziYwwLbf\n\tvzABwpFVcS8aR+nHIZV1HtOzyw8CsL8OySc3K9y+Y0NRpziMRvutrppzgyMb9V+N31mK9Mxl\n\t1YkgaTl4ciNWpdfUe0yxH03OCuHi3922qhPLF4XX5LN+NaVw5Xz2o3eeWklXdouxwV7QlN33\n\tu4+u2FWzKxDqO6WLQGjxPE0mVB4Gh5Pa1Vb0ct9Ctg0qElvtGQARAQABzShEYW4gU2NhbGx5\n\tIDxkYW4uc2NhbGx5QGlkZWFzb25ib2FyZC5jb20+wsGNBBMBCAA3FiEEsdtt8OWP7+8SNfQe\n\tkiQuh/L+GMQFAmLydlIFCQWjmoACGwMECwkIBwUVCAkKCwUWAgMBAAAKCRCSJC6H8v4YxDI2\n\tEAC2Gz0iyaXJkPInyshrREEWbo0CA6v5KKf3I/HlMPqkZ48bmGoYm4mEQGFWZJAT3K4ir8bg\n\tcEfs9V54gpbrZvdwS4abXbUK4WjKwEs8HK3XJv1WXUN2bsz5oEJWZUImh9gD3naiLLI9QMMm\n\tw/aZkT+NbN5/2KvChRWhdcha7+2Te4foOY66nIM+pw2FZM6zIkInLLUik2zXOhaZtqdeJZQi\n\tHSPU9xu7TRYN4cvdZAnSpG7gQqmLm5/uGZN1/sB3kHTustQtSXKMaIcD/DMNI3JN/t+RJVS7\n\tc0Jh/ThzTmhHyhxx3DRnDIy7kwMI4CFvmhkVC2uNs9kWsj1DuX5kt8513mvfw2OcX9UnNKmZ\n\tnhNCuF6DxVrL8wjOPuIpiEj3V+K7DFF1Cxw1/yrLs8dYdYh8T8vCY2CHBMsqpESROnTazboh\n\tAiQ2xMN1cyXtX11Qwqm5U3sykpLbx2BcmUUUEAKNsM//Zn81QXKG8vOx0ZdMfnzsCaCzt8f6\n\t9dcDBBI3tJ0BI9ByiocqUoL6759LM8qm18x3FYlxvuOs4wSGPfRVaA4yh0pgI+ModVC2Pu3y\n\tejE/IxeatGqJHh6Y+iJzskdi27uFkRixl7YJZvPJAbEn7kzSi98u/5ReEA8Qhc8KO/B7wprj\n\txjNMZNYd0Eth8+WkixHYj752NT5qshKJXcyUU87BTQRi8nZSARAAx0BJayh1Fhwbf4zoY56x\n\txHEpT6DwdTAYAetd3yiKClLVJadYxOpuqyWa1bdfQWPb+h4MeXbWw/53PBgn7gI2EA7ebIRC\n\tPJJhAIkeym7hHZoxqDQTGDJjxFEL11qF+U3rhWiL2Zt0Pl+zFq0eWYYVNiXjsIS4FI2+4m16\n\ttPbDWZFJnSZ828VGtRDQdhXfx3zyVX21lVx1bX4/OZvIET7sVUufkE4hrbqrrufre7wsjD1t\n\t8MQKSapVrr1RltpzPpScdoxknOSBRwOvpp57pJJe5A0L7+WxJ+vQoQXj0j+5tmIWOAV1qBQp\n\thyoyUk9JpPfntk2EKnZHWaApFp5TcL6c5LhUvV7F6XwOjGPuGlZQCWXee9dr7zym8iR3irWT\n\t+49bIh5PMlqSLXJDYbuyFQHFxoiNdVvvf7etvGfqFYVMPVjipqfEQ38ST2nkzx+KBICz7uwj\n\tJwLBdTXzGFKHQNckGMl7F5QdO/35An/QcxBnHVMXqaSd12tkJmoRVWduwuuoFfkTY5mUV3uX\n\txGj3iVCK4V+ezOYA7c2YolfRCNMTza6vcK/P4tDjjsyBBZrCCzhBvd4VVsnnlZhVaIxoky4K\n\taL+AP+zcQrUZmXmgZjXOLryGnsaeoVrIFyrU6ly90s1y3KLoPsDaTBMtnOdwxPmo1xisH8oL\n\ta/VRgpFBfojLPxMAEQEAAcLBfAQYAQgAJhYhBLHbbfDlj+/vEjX0HpIkLofy/hjEBQJi8nZT\n\tBQkFo5qAAhsMAAoJEJIkLofy/hjEXPcQAMIPNqiWiz/HKu9W4QIf1OMUpKn3YkVIj3p3gvfM\n\tRes4fGX94Ji599uLNrPoxKyaytC4R6BTxVriTJjWK8mbo9jZIRM4vkwkZZ2bu98EweSucxbp\n\tvjESsvMXGgxniqV/RQ/3T7LABYRoIUutARYq58p5HwSP0frF0fdFHYdTa2g7MYZl1ur2JzOC\n\tFHRpGadlNzKDE3fEdoMobxHB3Lm6FDml5GyBAA8+dQYVI0oDwJ3gpZPZ0J5Vx9RbqXe8RDuR\n\tdu90hvCJkq7/tzSQ0GeD3BwXb9/R/A4dVXhaDd91Q1qQXidI+2jwhx8iqiYxbT+DoAUkQRQy\n\txBtoCM1CxH7u45URUgD//fxYr3D4B1SlonA6vdaEdHZOGwECnDpTxecENMbz/Bx7qfrmd901\n\tD+N9SjIwrbVhhSyUXYnSUb8F+9g2RDY42Sk7GcYxIeON4VzKqWM7hpkXZ47pkK0YodO+dRKM\n\tyMcoUWrTK0Uz6UzUGKoJVbxmSW/EJLEGoI5p3NWxWtScEVv8mO49gqQdrRIOheZycDmHnItt\n\t9Qjv00uFhEwv2YfiyGk6iGF2W40s2pH2t6oeuGgmiZ7g6d0MEK8Ql/4zPItvr1c1rpwpXUC1\n\tu1kQWgtnNjFHX3KiYdqjcZeRBiry1X0zY+4Y24wUU0KsEewJwjhmCKAsju1RpdlPg2kC","In-Reply-To":"<20240517075751.3866269-2-paul.elder@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]