[{"id":29203,"web_url":"https://patchwork.libcamera.org/comment/29203/","msgid":"<19f74c61-451e-430c-940f-9fe5d59d4510@ideasonboard.com>","date":"2024-04-12T08:24:10","subject":"Re: [PATCH 3/5] fixup: ipa: rkisp1: Remove bespoke Agc functions","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Paul\n\nThank you for the patch.\n\nOn 05/04/24 8:17 pm, Paul Elder wrote:\n> Remove some constants that should have been removed, and restore a\n> fairly informative comment that got removed.\n>\n> Although the removal of one of the constants also removes a todo\n> comment, this will be addressed imminently so it should be fine.\n>\n> Signed-off-by: Paul Elder \n> ---\n> src/ipa/rkisp1/algorithms/agc.cpp | 32 +++++++++++++++++++++++++------\n> 1 file changed, 26 insertions(+), 6 deletions(-)\n>\n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index 7220f00a..1dfc4aaa 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -39,12 +39,6 @@ namespace ipa::rkisp1::algorithms {\n> \n> LOG_DEFINE_CATEGORY(RkISP1Agc)\n> \n> -/* Minimum limit for analogue gain value */\n> -static constexpr double kMinAnalogueGain = 1.0;\n> -\n> -/* \\todo Honour the FrameDurationLimits control instead of hardcoding a limit */\n> -static constexpr utils::Duration kMaxShutterSpeed = 60ms;\n> -\n> int Agc::parseMeteringModes(IPAContext &context, const YamlObject &tuningData,\n> \t\t\t const char *prop)\n> {\n> @@ -315,6 +309,32 @@ void Agc::parseStatistics(const rkisp1_stat_buffer *stats,\n> \t\t\t\t\t context.hw->numHistogramBins), 4);\n> }\n> \n> +/**\n> + * \\brief Estimate the relative luminance of the frame with a given gain\n> + * \\param[in] expMeans The mean luminance values, from the RkISP1 statistics\n> + * \\param[in] gain The gain to apply to the frame\n\nThe documentation states two input function parameters but looking at \nthis patch, estimateLuminance() has only gain parameter ?\n\nDid you miss to update the function signature here?\n> + *\n> + * This function estimates the average relative luminance of the frame that\n> + * would be output by the sensor if an additional \\a gain was applied.\n> + *\n> + * The estimation is based on the AE statistics for the current frame. Y\n> + * averages for all cells are first multiplied by the gain, and then saturated\n> + * to approximate the sensor behaviour at high brightness values. The\n> + * approximation is quite rough, as it doesn't take into account non-linearities\n> + * when approaching saturation. In this case, saturating after the conversion to\n> + * YUV doesn't take into account the fact that the R, G and B components\n> + * contribute differently to the relative luminance.\n> + *\n> + * \\todo Have a dedicated YUV algorithm ?\n> + *\n> + * The values are normalized to the [0.0, 1.0] range, where 1.0 corresponds to a\n> + * theoretical perfect reflector of 100% reference white.\n> + *\n> + * More detailed information can be found in:\n> + * https://en.wikipedia.org/wiki/Relative_luminance\n> + *\n> + * \\return The relative luminance\n> + */\n> double Agc::estimateLuminance(double gain)\n> {\n> \tdouble ySum = 0.0;","headers":{"Return-Path":"","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 E9321C3213\n\tfor ;\n\tFri, 12 Apr 2024 08:24:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A117063352;\n\tFri, 12 Apr 2024 10:24:18 +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 26E036333B\n\tfor ;\n\tFri, 12 Apr 2024 10:24:17 +0200 (CEST)","from [192.168.1.105] (unknown [103.251.226.65])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BE48FA12;\n\tFri, 12 Apr 2024 10:23:32 +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=\"ccKmwpNQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1712910213;\n\tbh=KZOF33EA9ZinPWwKnH+nQMSr2iBdi/qYUi950T+Xm5I=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=ccKmwpNQedIfmjWh+2vqC9YhV07VVYd2YZ3H1yjwHXxwOEFPW+0tfC6nOOijieMG9\n\tgjpj+7xmYHei2KGzImA3hl8tot7QkXWh11fLxqmYnnZssHA6F19wYN/hZ4tKI8s8x6\n\t6HFGWWG7IBr+lZltRfxPr3tYgkvkLB1NkJp7qoOk=","Message-ID":"<19f74c61-451e-430c-940f-9fe5d59d4510@ideasonboard.com>","Date":"Fri, 12 Apr 2024 13:54:10 +0530","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 3/5] fixup: ipa: rkisp1: Remove bespoke Agc functions","Content-Language":"en-US","To":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20240405144729.2992219-1-paul.elder@ideasonboard.com>\n\t<20240405144729.2992219-4-paul.elder@ideasonboard.com>","From":"Umang Jain ","In-Reply-To":"<20240405144729.2992219-4-paul.elder@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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":29260,"web_url":"https://patchwork.libcamera.org/comment/29260/","msgid":"","date":"2024-04-18T11:52:19","subject":"Re: [PATCH 3/5] fixup: ipa: rkisp1: Remove bespoke Agc functions","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hello Umang, Paul\n\nOn 12/04/2024 09:24, Umang Jain wrote:\n> Hi Paul\n>\n> Thank you for the patch.\n>\n> On 05/04/24 8:17 pm, Paul Elder wrote:\n>> Remove some constants that should have been removed, and restore a\n>> fairly informative comment that got removed.\n>>\n>> Although the removal of one of the constants also removes a todo\n>> comment, this will be addressed imminently so it should be fine.\n>>\n>> Signed-off-by: Paul Elder \n>> ---\n>>   src/ipa/rkisp1/algorithms/agc.cpp | 32 +++++++++++++++++++++++++------\n>>   1 file changed, 26 insertions(+), 6 deletions(-)\n>>\n>> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n>> index 7220f00a..1dfc4aaa 100644\n>> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n>> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n>> @@ -39,12 +39,6 @@ namespace ipa::rkisp1::algorithms {\n>>     LOG_DEFINE_CATEGORY(RkISP1Agc)\n>>   -/* Minimum limit for analogue gain value */\n>> -static constexpr double kMinAnalogueGain = 1.0;\n>> -\n>> -/* \\todo Honour the FrameDurationLimits control instead of hardcoding a limit */\n>> -static constexpr utils::Duration kMaxShutterSpeed = 60ms;\n>> -\n>>   int Agc::parseMeteringModes(IPAContext &context, const YamlObject &tuningData,\n>>                   const char *prop)\n>>   {\n>> @@ -315,6 +309,32 @@ void Agc::parseStatistics(const rkisp1_stat_buffer *stats,\n>>                              context.hw->numHistogramBins), 4);\n>>   }\n>>   +/**\n>> + * \\brief Estimate the relative luminance of the frame with a given gain\n>> + * \\param[in] expMeans The mean luminance values, from the RkISP1 statistics\n>> + * \\param[in] gain The gain to apply to the frame\n>\n> The documentation states two input function parameters but looking at this patch, \n> estimateLuminance() has only gain parameter ?\n>\n> Did you miss to update the function signature here?\n\nThank you for the fixes - I squashed this into v2 of the centralised AGC series, but dropped the \nexpMeans parameter from the doxygen comment here.\n>> + *\n>> + * This function estimates the average relative luminance of the frame that\n>> + * would be output by the sensor if an additional \\a gain was applied.\n>> + *\n>> + * The estimation is based on the AE statistics for the current frame. Y\n>> + * averages for all cells are first multiplied by the gain, and then saturated\n>> + * to approximate the sensor behaviour at high brightness values. The\n>> + * approximation is quite rough, as it doesn't take into account non-linearities\n>> + * when approaching saturation. In this case, saturating after the conversion to\n>> + * YUV doesn't take into account the fact that the R, G and B components\n>> + * contribute differently to the relative luminance.\n>> + *\n>> + * \\todo Have a dedicated YUV algorithm ?\n>> + *\n>> + * The values are normalized to the [0.0, 1.0] range, where 1.0 corresponds to a\n>> + * theoretical perfect reflector of 100% reference white.\n>> + *\n>> + * More detailed information can be found in:\n>> + * https://en.wikipedia.org/wiki/Relative_luminance\n>> + *\n>> + * \\return The relative luminance\n>> + */\n>>   double Agc::estimateLuminance(double gain)\n>>   {\n>>       double ySum = 0.0;\n>","headers":{"Return-Path":"","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 97B3DC3200\n\tfor ;\n\tThu, 18 Apr 2024 11:52:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5F33C633F4;\n\tThu, 18 Apr 2024 13:52:24 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B7E2C61B4F\n\tfor ;\n\tThu, 18 Apr 2024 13:52:22 +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 090BDEF2\n\tfor ;\n\tThu, 18 Apr 2024 13:51:35 +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=\"qbM8k35k\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1713441095;\n\tbh=tXGDC+glOqIqUyyHpZRU415StU4DczeKbiKC5CyQYDU=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=qbM8k35kYVk8K0+aB8BS0RYwHjG7Ix94a6x0qzHyJ7p8pejTVy3TiJl69OLvu/xB2\n\tHXoRAKFAIlmvVQJpufluCPxNXQ8GF6u643AA3ddWlBEly0aPGD0ieiPtiDBZE6atj6\n\tFEKE9KAkSZbAE1wvHmfj0KSHPOk4M2D54vBgrNnU=","Message-ID":"","Date":"Thu, 18 Apr 2024 12:52:19 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 3/5] fixup: ipa: rkisp1: Remove bespoke Agc functions","To":"libcamera-devel@lists.libcamera.org","References":"<20240405144729.2992219-1-paul.elder@ideasonboard.com>\n\t<20240405144729.2992219-4-paul.elder@ideasonboard.com>\n\t<19f74c61-451e-430c-940f-9fe5d59d4510@ideasonboard.com>","Content-Language":"en-US","From":"Dan Scally ","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":"<19f74c61-451e-430c-940f-9fe5d59d4510@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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]