[{"id":29042,"web_url":"https://patchwork.libcamera.org/comment/29042/","msgid":"<20240322143318.mjb4bdiu7z26pcdb@jasper>","date":"2024-03-22T14:33:18","subject":"Re: [PATCH 00/10] Centralise Agc into libipa","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Daniel,\n\nOn Fri, Mar 22, 2024 at 01:14:41PM +0000, Daniel Scally wrote:\n> Hello all\n> \n> The IPU3 and RkISP1 IPAs do Agc the same way (following the Rpi method closely),\n> but the implementations are wholly separate. This introduces the potential for\n> divergence and also makes both maintenance and improvement more difficult. This\n> series unifies them as derivations of a new MeanLuminanceAgc class within\n> libipa. The algorithm itself is done through functions of the base class\n> with the IPA's derived classes providing a function through which the mean\n> luminance input to the algorithm can be calculated from their specific stats\n> formats.\n> \n> The old implementations effectively hard coded values for the AeConstraintMode\n> and AeExposureMode controls; they adhered to a single lower-bound constraint of\n> 0.5 across the top 2% of a Histogram and acted as though the AeExposureMode\n> expected shutter time to be driven to its maximum before touching gain at all.\n> The base class additionally adds infrastructure to discover the supported values\n> for AeConstraintMode and AeExposureMode controls from a camera sensor tuning\n> file and uses the values defined in those files with the algorithm instead,\n> though as no tuning files are modified in this series the behaviour currently\n> remains as it was before.\n> \n> The series **does** alter the calculated shutter time and gain for both the\n> IPU3 and RkISP1 compared to their bespoke implementations, for two reasons:\n> \n> 1. A bug in the way they were implemented meant that an over-exposed image was\n>    corrected more slowly than an under-exposed one. This is fixed and will\n>    improve both IPAs' response to a too-bright image.\n> 2. The default kRelativeLuminanceTarget is centrally set to 0.16 which matches\n>    the value from the IPU3 implementation. In the RkISP1 implementation that\n>    value was set to 0.4 with a \\todo to see why they were different. My belief\n>    is that they were different because they were implemented in different\n>    lighting conditions. In my very imperfect testing setup the 0.16 target\n>    produced reasonable images on both.\n\nJust a wild guess (and the numbers fit nicely). This might be dependent on\nwhether gamma is included in the calculation or not. A value of 0.16\ncorresponds to 0.4 for gamma=2.\n\nCheers,\nStefan\n\n> \n> Without those two changes, the shutter time and gain calculated after this\n> series matches those calculated by their independent implementations.\n> \n> Thanks\n> Dan\n> \n> Daniel Scally (9):\n>   ipa: libipa: Allow creation of empty Histogram\n>   libcamera: controls: Generate enum value-name maps\n>   ipa: libipa: Add MeanLuminanceAgc base class\n>   ipa: ipu3: Parse and store Agc stats\n>   ipa: ipu3: Derive ipu3::algorithms::Agc from MeanLuminanceAgc\n>   ipa: ipu3: Remove bespoke AGC functions from IPU3\n>   ipa: rkisp1: Add parseStatistics() to Agc\n>   ipa: rkisp1: Derive rkisp1::algorithms::Agc from MeanLuminanceAgc\n>   ipa: rkisp1: Remove bespoke Agc functions\n> \n> Paul Elder (1):\n>   ipa: libipa: Add ExposureModeHelper\n> \n>  include/libcamera/control_ids.h.in      |   2 +\n>  include/libcamera/property_ids.h.in     |   2 +\n>  src/ipa/ipu3/algorithms/agc.cpp         | 306 ++++----------\n>  src/ipa/ipu3/algorithms/agc.h           |  27 +-\n>  src/ipa/ipu3/ipa_context.cpp            |   3 +\n>  src/ipa/ipu3/ipa_context.h              |   5 +\n>  src/ipa/ipu3/ipu3.cpp                   |   2 +-\n>  src/ipa/libipa/agc.cpp                  | 526 ++++++++++++++++++++++++\n>  src/ipa/libipa/agc.h                    |  82 ++++\n>  src/ipa/libipa/exposure_mode_helper.cpp | 307 ++++++++++++++\n>  src/ipa/libipa/exposure_mode_helper.h   |  61 +++\n>  src/ipa/libipa/histogram.h              |   1 +\n>  src/ipa/libipa/meson.build              |   4 +\n>  src/ipa/rkisp1/algorithms/agc.cpp       | 303 ++++----------\n>  src/ipa/rkisp1/algorithms/agc.h         |  19 +-\n>  src/ipa/rkisp1/ipa_context.h            |   5 +\n>  src/ipa/rkisp1/rkisp1.cpp               |   2 +-\n>  utils/gen-controls.py                   |  19 +\n>  18 files changed, 1219 insertions(+), 457 deletions(-)\n>  create mode 100644 src/ipa/libipa/agc.cpp\n>  create mode 100644 src/ipa/libipa/agc.h\n>  create mode 100644 src/ipa/libipa/exposure_mode_helper.cpp\n>  create mode 100644 src/ipa/libipa/exposure_mode_helper.h\n> \n> -- \n> 2.34.1\n>","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 CAD85C3272\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 22 Mar 2024 14:33:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E819963075;\n\tFri, 22 Mar 2024 15:33:21 +0100 (CET)","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 3848762826\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 22 Mar 2024 15:33:21 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:e5:ab7c:4271:bf1f])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2CE7E82A;\n\tFri, 22 Mar 2024 15:32:52 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"cUhMlwsS\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1711117972;\n\tbh=CAglwqfx1AStV8fq8NsSJ6cQWCUiPeokVg9r1ReNags=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=cUhMlwsSjwVdNPGpjl7Klqs9SGw83LxHzBvJDMdmeboOcKMbjNBjvyvrqEgK4IL/H\n\t/IDtzAHnvM44h8q0UqhFS8dtwXvav2vtES2NT7CJseQNauDCWH3mNVA2CoJ1LKvXsl\n\tn+skN/Pt6hc2YQvRC18kZKqu+2cMgRGsFGdxt16o=","Date":"Fri, 22 Mar 2024 15:33:18 +0100","From":"Stefan Klug <stefan.klug@ideasonboard.com>","To":"Daniel Scally <dan.scally@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 00/10] Centralise Agc into libipa","Message-ID":"<20240322143318.mjb4bdiu7z26pcdb@jasper>","References":"<20240322131451.3092931-1-dan.scally@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240322131451.3092931-1-dan.scally@ideasonboard.com>","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>"}},{"id":29043,"web_url":"https://patchwork.libcamera.org/comment/29043/","msgid":"<0a45d8a1-0da1-42ca-9d45-20ab7f482fe7@ideasonboard.com>","date":"2024-03-22T14:48:06","subject":"Re: [PATCH 00/10] Centralise Agc into libipa","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"On 22/03/2024 13:14, Daniel Scally wrote:\n> Hello all\n>\n> The IPU3 and RkISP1 IPAs do Agc the same way (following the Rpi method closely),\n> but the implementations are wholly separate. This introduces the potential for\n> divergence and also makes both maintenance and improvement more difficult. This\n> series unifies them as derivations of a new MeanLuminanceAgc class within\n> libipa. The algorithm itself is done through functions of the base class\n> with the IPA's derived classes providing a function through which the mean\n> luminance input to the algorithm can be calculated from their specific stats\n> formats.\n>\n> The old implementations effectively hard coded values for the AeConstraintMode\n> and AeExposureMode controls; they adhered to a single lower-bound constraint of\n> 0.5 across the top 2% of a Histogram and acted as though the AeExposureMode\n> expected shutter time to be driven to its maximum before touching gain at all.\n> The base class additionally adds infrastructure to discover the supported values\n> for AeConstraintMode and AeExposureMode controls from a camera sensor tuning\n> file and uses the values defined in those files with the algorithm instead,\n> though as no tuning files are modified in this series the behaviour currently\n> remains as it was before.\n>\n> The series **does** alter the calculated shutter time and gain for both the\n> IPU3 and RkISP1 compared to their bespoke implementations, for two reasons:\n>\n> 1. A bug in the way they were implemented meant that an over-exposed image was\n>     corrected more slowly than an under-exposed one. This is fixed and will\n>     improve both IPAs' response to a too-bright image.\n> 2. The default kRelativeLuminanceTarget is centrally set to 0.16 which matches\n>     the value from the IPU3 implementation. In the RkISP1 implementation that\n>     value was set to 0.4 with a \\todo to see why they were different. My belief\n>     is that they were different because they were implemented in different\n>     lighting conditions. In my very imperfect testing setup the 0.16 target\n>     produced reasonable images on both.\n\n\nActually the more I think about this, the more I think we'll need a value for each IPA anyway \nbecause it's a figure that depends on how they calculate things. RkISP1 takes an average of an array \nof values which are the themselves the average Luminance in a zone, where Y=(R+G+B) * 0.332. IPU3 \ntakes sums the average R/G/B value in each zone of red/blue/green pixels and adjusts them by the \nBT.601 values. Rpi I **think** gets R/G/B sums across an image and adjusts them by BT.601. All of \nthose methods seem bound to produce different estimations of Y even with completely identical \nscenes, so I'm leaning towards the view that we probably need to define the default \nkRelativeLuminanceTarget in each IPA's implementation of Agc separately.\n\n>\n> Without those two changes, the shutter time and gain calculated after this\n> series matches those calculated by their independent implementations.\n>\n> Thanks\n> Dan\n>\n> Daniel Scally (9):\n>    ipa: libipa: Allow creation of empty Histogram\n>    libcamera: controls: Generate enum value-name maps\n>    ipa: libipa: Add MeanLuminanceAgc base class\n>    ipa: ipu3: Parse and store Agc stats\n>    ipa: ipu3: Derive ipu3::algorithms::Agc from MeanLuminanceAgc\n>    ipa: ipu3: Remove bespoke AGC functions from IPU3\n>    ipa: rkisp1: Add parseStatistics() to Agc\n>    ipa: rkisp1: Derive rkisp1::algorithms::Agc from MeanLuminanceAgc\n>    ipa: rkisp1: Remove bespoke Agc functions\n>\n> Paul Elder (1):\n>    ipa: libipa: Add ExposureModeHelper\n>\n>   include/libcamera/control_ids.h.in      |   2 +\n>   include/libcamera/property_ids.h.in     |   2 +\n>   src/ipa/ipu3/algorithms/agc.cpp         | 306 ++++----------\n>   src/ipa/ipu3/algorithms/agc.h           |  27 +-\n>   src/ipa/ipu3/ipa_context.cpp            |   3 +\n>   src/ipa/ipu3/ipa_context.h              |   5 +\n>   src/ipa/ipu3/ipu3.cpp                   |   2 +-\n>   src/ipa/libipa/agc.cpp                  | 526 ++++++++++++++++++++++++\n>   src/ipa/libipa/agc.h                    |  82 ++++\n>   src/ipa/libipa/exposure_mode_helper.cpp | 307 ++++++++++++++\n>   src/ipa/libipa/exposure_mode_helper.h   |  61 +++\n>   src/ipa/libipa/histogram.h              |   1 +\n>   src/ipa/libipa/meson.build              |   4 +\n>   src/ipa/rkisp1/algorithms/agc.cpp       | 303 ++++----------\n>   src/ipa/rkisp1/algorithms/agc.h         |  19 +-\n>   src/ipa/rkisp1/ipa_context.h            |   5 +\n>   src/ipa/rkisp1/rkisp1.cpp               |   2 +-\n>   utils/gen-controls.py                   |  19 +\n>   18 files changed, 1219 insertions(+), 457 deletions(-)\n>   create mode 100644 src/ipa/libipa/agc.cpp\n>   create mode 100644 src/ipa/libipa/agc.h\n>   create mode 100644 src/ipa/libipa/exposure_mode_helper.cpp\n>   create mode 100644 src/ipa/libipa/exposure_mode_helper.h\n>","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 BBF68BD160\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 22 Mar 2024 14:48:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A559763055;\n\tFri, 22 Mar 2024 15:48:10 +0100 (CET)","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 2E87C61C45\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 22 Mar 2024 15:48:09 +0100 (CET)","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 35E5282A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 22 Mar 2024 15:47:40 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"B9Rxv9Gt\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1711118860;\n\tbh=pGe2caEbXSvA7I9cx6q+VU79BNeXCejrCD9DnZBoIkU=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=B9Rxv9Gt0iGv8EZCXWVA+trU5Y1dvzT6rI07Gb1/715ui8+W6IelJ4XNP9qKSwD89\n\tHYUXbFZgFSbpuPEjehyfOZMI8xcEjpnoXwiOqyTJWd8+JYJOrHITB3AgBey2z+xxEA\n\t2WNjIABIduhSyPAMfWNtkpZXqpkF89Tj4owp6swA=","Message-ID":"<0a45d8a1-0da1-42ca-9d45-20ab7f482fe7@ideasonboard.com>","Date":"Fri, 22 Mar 2024 14:48:06 +0000","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 00/10] Centralise Agc into libipa","Content-Language":"en-US","To":"libcamera-devel@lists.libcamera.org","References":"<20240322131451.3092931-1-dan.scally@ideasonboard.com>","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":"<20240322131451.3092931-1-dan.scally@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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>"}},{"id":29160,"web_url":"https://patchwork.libcamera.org/comment/29160/","msgid":"<20240405211935.GF12507@pendragon.ideasonboard.com>","date":"2024-04-05T21:19:35","subject":"Re: [PATCH 00/10] Centralise Agc into libipa","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Dan,\n\nOn Fri, Mar 22, 2024 at 02:48:06PM +0000, Daniel Scally wrote:\n> On 22/03/2024 13:14, Daniel Scally wrote:\n> > Hello all\n> >\n> > The IPU3 and RkISP1 IPAs do Agc the same way (following the Rpi method closely),\n> > but the implementations are wholly separate. This introduces the potential for\n> > divergence and also makes both maintenance and improvement more difficult. This\n> > series unifies them as derivations of a new MeanLuminanceAgc class within\n> > libipa. The algorithm itself is done through functions of the base class\n> > with the IPA's derived classes providing a function through which the mean\n> > luminance input to the algorithm can be calculated from their specific stats\n> > formats.\n> >\n> > The old implementations effectively hard coded values for the AeConstraintMode\n> > and AeExposureMode controls; they adhered to a single lower-bound constraint of\n> > 0.5 across the top 2% of a Histogram and acted as though the AeExposureMode\n> > expected shutter time to be driven to its maximum before touching gain at all.\n> > The base class additionally adds infrastructure to discover the supported values\n> > for AeConstraintMode and AeExposureMode controls from a camera sensor tuning\n> > file and uses the values defined in those files with the algorithm instead,\n> > though as no tuning files are modified in this series the behaviour currently\n> > remains as it was before.\n> >\n> > The series **does** alter the calculated shutter time and gain for both the\n> > IPU3 and RkISP1 compared to their bespoke implementations, for two reasons:\n> >\n> > 1. A bug in the way they were implemented meant that an over-exposed image was\n> >     corrected more slowly than an under-exposed one. This is fixed and will\n> >     improve both IPAs' response to a too-bright image.\n> > 2. The default kRelativeLuminanceTarget is centrally set to 0.16 which matches\n> >     the value from the IPU3 implementation. In the RkISP1 implementation that\n> >     value was set to 0.4 with a \\todo to see why they were different. My belief\n> >     is that they were different because they were implemented in different\n> >     lighting conditions.\n\nThen we clearly have something to fix, as the code shouldn't contain\ncompile-time constants that depend on the lighting conditions :-)\n\n> > In my very imperfect testing setup the 0.16 target\n> >     produced reasonable images on both.\n> \n> Actually the more I think about this, the more I think we'll need a\n> value for each IPA anyway because it's a figure that depends on how\n> they calculate things. RkISP1 takes an average of an array of values\n> which are the themselves the average Luminance in a zone, where\n> Y=(R+G+B) * 0.332. IPU3 takes sums the average R/G/B value in each\n> zone of red/blue/green pixels and adjusts them by the BT.601 values.\n> Rpi I **think** gets R/G/B sums across an image and adjusts them by\n> BT.601. All of those methods seem bound to produce different\n> estimations of Y even with completely identical scenes, so I'm leaning\n> towards the view that we probably need to define the default\n> kRelativeLuminanceTarget in each IPA's implementation of Agc\n> separately.\n\nWouldn't it be better to define how the algorithm expects the luminance\nto be estimated, and require each platform-specific specialization to\ncompute the luminance estimate accordingly ? Injecting ill-defined\nluminance estimates with ill-defined parameters to tweak the behaviour\nof the calculations doesn't seem very scientific.\n\n> > Without those two changes, the shutter time and gain calculated after this\n> > series matches those calculated by their independent implementations.\n> >\n> > Thanks\n> > Dan\n> >\n> > Daniel Scally (9):\n> >    ipa: libipa: Allow creation of empty Histogram\n> >    libcamera: controls: Generate enum value-name maps\n> >    ipa: libipa: Add MeanLuminanceAgc base class\n> >    ipa: ipu3: Parse and store Agc stats\n> >    ipa: ipu3: Derive ipu3::algorithms::Agc from MeanLuminanceAgc\n> >    ipa: ipu3: Remove bespoke AGC functions from IPU3\n> >    ipa: rkisp1: Add parseStatistics() to Agc\n> >    ipa: rkisp1: Derive rkisp1::algorithms::Agc from MeanLuminanceAgc\n> >    ipa: rkisp1: Remove bespoke Agc functions\n> >\n> > Paul Elder (1):\n> >    ipa: libipa: Add ExposureModeHelper\n> >\n> >   include/libcamera/control_ids.h.in      |   2 +\n> >   include/libcamera/property_ids.h.in     |   2 +\n> >   src/ipa/ipu3/algorithms/agc.cpp         | 306 ++++----------\n> >   src/ipa/ipu3/algorithms/agc.h           |  27 +-\n> >   src/ipa/ipu3/ipa_context.cpp            |   3 +\n> >   src/ipa/ipu3/ipa_context.h              |   5 +\n> >   src/ipa/ipu3/ipu3.cpp                   |   2 +-\n> >   src/ipa/libipa/agc.cpp                  | 526 ++++++++++++++++++++++++\n> >   src/ipa/libipa/agc.h                    |  82 ++++\n> >   src/ipa/libipa/exposure_mode_helper.cpp | 307 ++++++++++++++\n> >   src/ipa/libipa/exposure_mode_helper.h   |  61 +++\n> >   src/ipa/libipa/histogram.h              |   1 +\n> >   src/ipa/libipa/meson.build              |   4 +\n> >   src/ipa/rkisp1/algorithms/agc.cpp       | 303 ++++----------\n> >   src/ipa/rkisp1/algorithms/agc.h         |  19 +-\n> >   src/ipa/rkisp1/ipa_context.h            |   5 +\n> >   src/ipa/rkisp1/rkisp1.cpp               |   2 +-\n> >   utils/gen-controls.py                   |  19 +\n> >   18 files changed, 1219 insertions(+), 457 deletions(-)\n> >   create mode 100644 src/ipa/libipa/agc.cpp\n> >   create mode 100644 src/ipa/libipa/agc.h\n> >   create mode 100644 src/ipa/libipa/exposure_mode_helper.cpp\n> >   create mode 100644 src/ipa/libipa/exposure_mode_helper.h","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 5AAD4C0DA4\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  5 Apr 2024 21:19:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2C94761C33;\n\tFri,  5 Apr 2024 23:19:48 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8EE0961C29\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  5 Apr 2024 23:19:46 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 762FAACB;\n\tFri,  5 Apr 2024 23:19:07 +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=\"SsJPkaAi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1712351947;\n\tbh=1ffQ8nK8sjWtcLiZYnuswhQZxNvf9CSZJYTN5cobs/Y=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=SsJPkaAirwNXN3E1B86/IjnwYnWVInisSEuNIAmA031hqd2q3yavC8Tsu1z9q1yuN\n\tT7dWw09TNJ61Bi7mloY3Ym4iSxMb+YFxFrRy6z8QaSGJjTlV21nY8077kasqdiQs/9\n\tIlvmac0QnncH8NmO8y5Hnestcpx2yWGmMCx3dAw4=","Date":"Sat, 6 Apr 2024 00:19:35 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Dan Scally <dan.scally@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 00/10] Centralise Agc into libipa","Message-ID":"<20240405211935.GF12507@pendragon.ideasonboard.com>","References":"<20240322131451.3092931-1-dan.scally@ideasonboard.com>\n\t<0a45d8a1-0da1-42ca-9d45-20ab7f482fe7@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<0a45d8a1-0da1-42ca-9d45-20ab7f482fe7@ideasonboard.com>","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>"}},{"id":29161,"web_url":"https://patchwork.libcamera.org/comment/29161/","msgid":"<20240405212419.GG12507@pendragon.ideasonboard.com>","date":"2024-04-05T21:24:19","subject":"Re: [PATCH 00/10] Centralise Agc into libipa","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Fri, Mar 22, 2024 at 03:33:18PM +0100, Stefan Klug wrote:\n> Hi Daniel,\n> \n> On Fri, Mar 22, 2024 at 01:14:41PM +0000, Daniel Scally wrote:\n> > Hello all\n> > \n> > The IPU3 and RkISP1 IPAs do Agc the same way (following the Rpi method closely),\n> > but the implementations are wholly separate. This introduces the potential for\n> > divergence and also makes both maintenance and improvement more difficult. This\n> > series unifies them as derivations of a new MeanLuminanceAgc class within\n> > libipa. The algorithm itself is done through functions of the base class\n> > with the IPA's derived classes providing a function through which the mean\n> > luminance input to the algorithm can be calculated from their specific stats\n> > formats.\n> > \n> > The old implementations effectively hard coded values for the AeConstraintMode\n> > and AeExposureMode controls; they adhered to a single lower-bound constraint of\n> > 0.5 across the top 2% of a Histogram and acted as though the AeExposureMode\n> > expected shutter time to be driven to its maximum before touching gain at all.\n> > The base class additionally adds infrastructure to discover the supported values\n> > for AeConstraintMode and AeExposureMode controls from a camera sensor tuning\n> > file and uses the values defined in those files with the algorithm instead,\n> > though as no tuning files are modified in this series the behaviour currently\n> > remains as it was before.\n> > \n> > The series **does** alter the calculated shutter time and gain for both the\n> > IPU3 and RkISP1 compared to their bespoke implementations, for two reasons:\n> > \n> > 1. A bug in the way they were implemented meant that an over-exposed image was\n> >    corrected more slowly than an under-exposed one. This is fixed and will\n> >    improve both IPAs' response to a too-bright image.\n> > 2. The default kRelativeLuminanceTarget is centrally set to 0.16 which matches\n> >    the value from the IPU3 implementation. In the RkISP1 implementation that\n> >    value was set to 0.4 with a \\todo to see why they were different. My belief\n> >    is that they were different because they were implemented in different\n> >    lighting conditions. In my very imperfect testing setup the 0.16 target\n> >    produced reasonable images on both.\n> \n> Just a wild guess (and the numbers fit nicely). This might be dependent on\n> whether gamma is included in the calculation or not. A value of 0.16\n> corresponds to 0.4 for gamma=2.\n\nBoth the IPU3 and RkISP1 are supposed to produce stats computed on\nlinear-space data. We don't linearize the sensor output at the moment,\nbut it should be mostly linear to start with.\n\n> > Without those two changes, the shutter time and gain calculated after this\n> > series matches those calculated by their independent implementations.\n> > \n> > Thanks\n> > Dan\n> > \n> > Daniel Scally (9):\n> >   ipa: libipa: Allow creation of empty Histogram\n> >   libcamera: controls: Generate enum value-name maps\n> >   ipa: libipa: Add MeanLuminanceAgc base class\n> >   ipa: ipu3: Parse and store Agc stats\n> >   ipa: ipu3: Derive ipu3::algorithms::Agc from MeanLuminanceAgc\n> >   ipa: ipu3: Remove bespoke AGC functions from IPU3\n> >   ipa: rkisp1: Add parseStatistics() to Agc\n> >   ipa: rkisp1: Derive rkisp1::algorithms::Agc from MeanLuminanceAgc\n> >   ipa: rkisp1: Remove bespoke Agc functions\n> > \n> > Paul Elder (1):\n> >   ipa: libipa: Add ExposureModeHelper\n> > \n> >  include/libcamera/control_ids.h.in      |   2 +\n> >  include/libcamera/property_ids.h.in     |   2 +\n> >  src/ipa/ipu3/algorithms/agc.cpp         | 306 ++++----------\n> >  src/ipa/ipu3/algorithms/agc.h           |  27 +-\n> >  src/ipa/ipu3/ipa_context.cpp            |   3 +\n> >  src/ipa/ipu3/ipa_context.h              |   5 +\n> >  src/ipa/ipu3/ipu3.cpp                   |   2 +-\n> >  src/ipa/libipa/agc.cpp                  | 526 ++++++++++++++++++++++++\n> >  src/ipa/libipa/agc.h                    |  82 ++++\n> >  src/ipa/libipa/exposure_mode_helper.cpp | 307 ++++++++++++++\n> >  src/ipa/libipa/exposure_mode_helper.h   |  61 +++\n> >  src/ipa/libipa/histogram.h              |   1 +\n> >  src/ipa/libipa/meson.build              |   4 +\n> >  src/ipa/rkisp1/algorithms/agc.cpp       | 303 ++++----------\n> >  src/ipa/rkisp1/algorithms/agc.h         |  19 +-\n> >  src/ipa/rkisp1/ipa_context.h            |   5 +\n> >  src/ipa/rkisp1/rkisp1.cpp               |   2 +-\n> >  utils/gen-controls.py                   |  19 +\n> >  18 files changed, 1219 insertions(+), 457 deletions(-)\n> >  create mode 100644 src/ipa/libipa/agc.cpp\n> >  create mode 100644 src/ipa/libipa/agc.h\n> >  create mode 100644 src/ipa/libipa/exposure_mode_helper.cpp\n> >  create mode 100644 src/ipa/libipa/exposure_mode_helper.h","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 5C49ABD16B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  5 Apr 2024 21:24:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 57E4F61C33;\n\tFri,  5 Apr 2024 23:24:32 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6F9DA61C29\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  5 Apr 2024 23:24:31 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 421F0ACB;\n\tFri,  5 Apr 2024 23:23:52 +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=\"kB2dDuLb\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1712352232;\n\tbh=3Dn44m6KZKyivvsKCp4JCYDcUYZTPtYvo8OxqwQn8UI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kB2dDuLbJEtmZk6Lc31N0eFyFD82QgS47dWvctFeJFMIBxXEeLXF3WFJENCIcAkNh\n\tcYjH9gKDxKuv5yQ+4hqls8WwK+icBVSu7wq+LgOjcZNIDSkHqHltN9ZslO02lnuWxF\n\t5i7ZZ4gy/ZCc1Vhrxh4WZUwzzWoMK57nCvrETnRE=","Date":"Sat, 6 Apr 2024 00:24:19 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Stefan Klug <stefan.klug@ideasonboard.com>","Cc":"Daniel Scally <dan.scally@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 00/10] Centralise Agc into libipa","Message-ID":"<20240405212419.GG12507@pendragon.ideasonboard.com>","References":"<20240322131451.3092931-1-dan.scally@ideasonboard.com>\n\t<20240322143318.mjb4bdiu7z26pcdb@jasper>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240322143318.mjb4bdiu7z26pcdb@jasper>","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>"}},{"id":29182,"web_url":"https://patchwork.libcamera.org/comment/29182/","msgid":"<639d1a35-cc9b-4c02-b0f0-8fc9d2289198@ideasonboard.com>","date":"2024-04-08T14:32:31","subject":"Re: [PATCH 00/10] Centralise Agc into libipa","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Laurent\n\nOn 05/04/2024 22:19, Laurent Pinchart wrote:\n> Hi Dan,\n>\n> On Fri, Mar 22, 2024 at 02:48:06PM +0000, Daniel Scally wrote:\n>> On 22/03/2024 13:14, Daniel Scally wrote:\n>>> Hello all\n>>>\n>>> The IPU3 and RkISP1 IPAs do Agc the same way (following the Rpi method closely),\n>>> but the implementations are wholly separate. This introduces the potential for\n>>> divergence and also makes both maintenance and improvement more difficult. This\n>>> series unifies them as derivations of a new MeanLuminanceAgc class within\n>>> libipa. The algorithm itself is done through functions of the base class\n>>> with the IPA's derived classes providing a function through which the mean\n>>> luminance input to the algorithm can be calculated from their specific stats\n>>> formats.\n>>>\n>>> The old implementations effectively hard coded values for the AeConstraintMode\n>>> and AeExposureMode controls; they adhered to a single lower-bound constraint of\n>>> 0.5 across the top 2% of a Histogram and acted as though the AeExposureMode\n>>> expected shutter time to be driven to its maximum before touching gain at all.\n>>> The base class additionally adds infrastructure to discover the supported values\n>>> for AeConstraintMode and AeExposureMode controls from a camera sensor tuning\n>>> file and uses the values defined in those files with the algorithm instead,\n>>> though as no tuning files are modified in this series the behaviour currently\n>>> remains as it was before.\n>>>\n>>> The series **does** alter the calculated shutter time and gain for both the\n>>> IPU3 and RkISP1 compared to their bespoke implementations, for two reasons:\n>>>\n>>> 1. A bug in the way they were implemented meant that an over-exposed image was\n>>>      corrected more slowly than an under-exposed one. This is fixed and will\n>>>      improve both IPAs' response to a too-bright image.\n>>> 2. The default kRelativeLuminanceTarget is centrally set to 0.16 which matches\n>>>      the value from the IPU3 implementation. In the RkISP1 implementation that\n>>>      value was set to 0.4 with a \\todo to see why they were different. My belief\n>>>      is that they were different because they were implemented in different\n>>>      lighting conditions.\n> Then we clearly have something to fix, as the code shouldn't contain\n> compile-time constants that depend on the lighting conditions :-)\n>\n>>> In my very imperfect testing setup the 0.16 target\n>>>      produced reasonable images on both.\n>> Actually the more I think about this, the more I think we'll need a\n>> value for each IPA anyway because it's a figure that depends on how\n>> they calculate things. RkISP1 takes an average of an array of values\n>> which are the themselves the average Luminance in a zone, where\n>> Y=(R+G+B) * 0.332. IPU3 takes sums the average R/G/B value in each\n>> zone of red/blue/green pixels and adjusts them by the BT.601 values.\n>> Rpi I **think** gets R/G/B sums across an image and adjusts them by\n>> BT.601. All of those methods seem bound to produce different\n>> estimations of Y even with completely identical scenes, so I'm leaning\n>> towards the view that we probably need to define the default\n>> kRelativeLuminanceTarget in each IPA's implementation of Agc\n>> separately.\n> Wouldn't it be better to define how the algorithm expects the luminance\n> to be estimated, and require each platform-specific specialization to\n> compute the luminance estimate accordingly ? Injecting ill-defined\n> luminance estimates with ill-defined parameters to tweak the behaviour\n> of the calculations doesn't seem very scientific.\n\nThat approach also has problems though; at minimum it would force the IPU3 IPA to calculate a \nhistogram using the formula that's internally used by the RkISP1 (since we couldn't calculate a \nhistogram using the BT.601 values for RkISP1) and forcing one IPA to adhere to the hardware \nconstraints of another doesn't sound right. I **think** (but am less sure about) that the statistics \nformats are too different to make a truly equivalent calculation anyway; meaning for the same input \ndata from a sensor I don't think you'd be able to arrive at precisely the same luminance estimation \nacross each ISP. But is that a problem? I think as long as we understand that and make it clear that \nthat's what's happening it ought to be fine.\n\n\n@Stefan, @Jacopo we talked about this on the call that we had together...but I'm afraid I left it \ntoo long to revisit the series and now can't remember what we concluded about this difference in \nluminance targets between ISPs and the likely explanation for it.\n\n>\n>>> Without those two changes, the shutter time and gain calculated after this\n>>> series matches those calculated by their independent implementations.\n>>>\n>>> Thanks\n>>> Dan\n>>>\n>>> Daniel Scally (9):\n>>>     ipa: libipa: Allow creation of empty Histogram\n>>>     libcamera: controls: Generate enum value-name maps\n>>>     ipa: libipa: Add MeanLuminanceAgc base class\n>>>     ipa: ipu3: Parse and store Agc stats\n>>>     ipa: ipu3: Derive ipu3::algorithms::Agc from MeanLuminanceAgc\n>>>     ipa: ipu3: Remove bespoke AGC functions from IPU3\n>>>     ipa: rkisp1: Add parseStatistics() to Agc\n>>>     ipa: rkisp1: Derive rkisp1::algorithms::Agc from MeanLuminanceAgc\n>>>     ipa: rkisp1: Remove bespoke Agc functions\n>>>\n>>> Paul Elder (1):\n>>>     ipa: libipa: Add ExposureModeHelper\n>>>\n>>>    include/libcamera/control_ids.h.in      |   2 +\n>>>    include/libcamera/property_ids.h.in     |   2 +\n>>>    src/ipa/ipu3/algorithms/agc.cpp         | 306 ++++----------\n>>>    src/ipa/ipu3/algorithms/agc.h           |  27 +-\n>>>    src/ipa/ipu3/ipa_context.cpp            |   3 +\n>>>    src/ipa/ipu3/ipa_context.h              |   5 +\n>>>    src/ipa/ipu3/ipu3.cpp                   |   2 +-\n>>>    src/ipa/libipa/agc.cpp                  | 526 ++++++++++++++++++++++++\n>>>    src/ipa/libipa/agc.h                    |  82 ++++\n>>>    src/ipa/libipa/exposure_mode_helper.cpp | 307 ++++++++++++++\n>>>    src/ipa/libipa/exposure_mode_helper.h   |  61 +++\n>>>    src/ipa/libipa/histogram.h              |   1 +\n>>>    src/ipa/libipa/meson.build              |   4 +\n>>>    src/ipa/rkisp1/algorithms/agc.cpp       | 303 ++++----------\n>>>    src/ipa/rkisp1/algorithms/agc.h         |  19 +-\n>>>    src/ipa/rkisp1/ipa_context.h            |   5 +\n>>>    src/ipa/rkisp1/rkisp1.cpp               |   2 +-\n>>>    utils/gen-controls.py                   |  19 +\n>>>    18 files changed, 1219 insertions(+), 457 deletions(-)\n>>>    create mode 100644 src/ipa/libipa/agc.cpp\n>>>    create mode 100644 src/ipa/libipa/agc.h\n>>>    create mode 100644 src/ipa/libipa/exposure_mode_helper.cpp\n>>>    create mode 100644 src/ipa/libipa/exposure_mode_helper.h","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 62A66BE08B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  8 Apr 2024 14:32:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A1BCC63352;\n\tMon,  8 Apr 2024 16:32:35 +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 9364861C28\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  8 Apr 2024 16:32:34 +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 6A752ABE;\n\tMon,  8 Apr 2024 16:31:53 +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=\"c0/j/5hq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1712586713;\n\tbh=xZhKx9vxJGV9mkCtyAvKE83sL+Ybp4ind4YFU+bYEko=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=c0/j/5hq0ilXGVbRCxRlOYZR94K8vdbPsN0D/r+NlJzRHfU9fhcSvPHxbcACZuzmP\n\txEWGDqOtk2l7Cud49+5Ze6PqaX4EaEq4qnxDwck1SEYv3MTpxFFmM1ouU8/rEl0rPt\n\tVE6+lVINi7pVUA0k4z7NmsAmm3RpnGzWcQCDRDMQ=","Message-ID":"<639d1a35-cc9b-4c02-b0f0-8fc9d2289198@ideasonboard.com>","Date":"Mon, 8 Apr 2024 15:32:31 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 00/10] Centralise Agc into libipa","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tStefan Klug <stefan.klug@ideasonboard.com>,\n\tJacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","References":"<20240322131451.3092931-1-dan.scally@ideasonboard.com>\n\t<0a45d8a1-0da1-42ca-9d45-20ab7f482fe7@ideasonboard.com>\n\t<20240405211935.GF12507@pendragon.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":"<20240405211935.GF12507@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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>"}}]