[{"id":31895,"web_url":"https://patchwork.libcamera.org/comment/31895/","msgid":"<70ac440c-176c-457e-b015-307676aafc18@ideasonboard.com>","date":"2024-10-24T03:22:26","subject":"Re: [PATCH 2/3] ipa: rkisp1: Alias lineDuration","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch\n\nOn 14/10/24 9:17 pm, Kieran Bingham wrote:\n> The configured line duration of the sensor is used frequently throughout\n> the AGC implementation.\n>\n> It's available in the IPA context through the rather long:\n> context.configuration.sensor.lineDuration\n>\n> Take a copy of the lineDuration early in the call and replace the two\n> current usages of the reference with the shorter copy to manage line\n> length and ease readibility.\n>\n> Signed-off-by: Kieran Bingham \n\nReviewed-by: Umang Jain \n> ---\n> src/ipa/rkisp1/algorithms/agc.cpp | 7 ++++---\n> 1 file changed, 4 insertions(+), 3 deletions(-)\n>\n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index 33f902862c4a..e23ab120b3e2 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -400,6 +400,8 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> \t\treturn;\n> \t}\n> \n> +\tutils::Duration lineDuration = context.configuration.sensor.lineDuration;\n> +\n> \t/*\n> \t * \\todo Verify that the exposure and gain applied by the sensor for\n> \t * this frame match what has been requested. This isn't a hard\n> @@ -429,8 +431,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> \t * The Agc algorithm needs to know the effective exposure value that was\n> \t * applied to the sensor when the statistics were collected.\n> \t */\n> -\tutils::Duration exposureTime = context.configuration.sensor.lineDuration\n> -\t\t\t\t * frameContext.sensor.exposure;\n> +\tutils::Duration exposureTime = lineDuration * frameContext.sensor.exposure;\n> \tdouble analogueGain = frameContext.sensor.gain;\n> \tutils::Duration effectiveExposureValue = exposureTime * analogueGain;\n> \n> @@ -447,7 +448,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> \n> \tIPAActiveState &activeState = context.activeState;\n> \t/* Update the estimated exposure and gain. */\n> -\tactiveState.agc.automatic.exposure = shutterTime / context.configuration.sensor.lineDuration;\n> +\tactiveState.agc.automatic.exposure = shutterTime / lineDuration;\n> \tactiveState.agc.automatic.gain = aGain;\n> \n> \tfillMetadata(context, frameContext, metadata);","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 7956BBE08B\n\tfor ;\n\tThu, 24 Oct 2024 03:22:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7E2A865393;\n\tThu, 24 Oct 2024 05:22:32 +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 CCCEB618C0\n\tfor ;\n\tThu, 24 Oct 2024 05:22:30 +0200 (CEST)","from [IPV6:2405:201:2015:f873:55d7:c02e:b2eb:ee3f] (unknown\n\t[IPv6:2405:201:2015:f873:55d7:c02e:b2eb:ee3f])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 84ED23D5;\n\tThu, 24 Oct 2024 05:20:42 +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=\"TYrQrSSJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1729740042;\n\tbh=Gdr/DrOpb9wKDWvbzZoEz1ympchGyKaAS/OjpuLPThQ=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=TYrQrSSJ5OGUfyuh+Sqjc4JIIhXbOFVkfeQ4ek3uoaXfZWohLT3gkjztt27BmWKlr\n\tW8E34xRJEj1v5iZPzGmbj+QiSsfcyNivZQB6E+xX8rgCQedga83eaaHdzqIQWb+Nna\n\tGlM7jVk7KMcIAzootcVihvP8xiixumUtybFlz1YM=","Message-ID":"<70ac440c-176c-457e-b015-307676aafc18@ideasonboard.com>","Date":"Thu, 24 Oct 2024 08:52:26 +0530","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 2/3] ipa: rkisp1: Alias lineDuration","To":"Kieran Bingham ,\n\tlibcamera devel ","References":"<20241014154747.2295253-1-kieran.bingham@ideasonboard.com>\n\t<20241014154747.2295253-3-kieran.bingham@ideasonboard.com>","Content-Language":"en-US","From":"Umang Jain ","In-Reply-To":"<20241014154747.2295253-3-kieran.bingham@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":31904,"web_url":"https://patchwork.libcamera.org/comment/31904/","msgid":"<1eba3009-4dba-4877-a844-e05d4dbc5abb@ideasonboard.com>","date":"2024-10-24T09:30:02","subject":"Re: [PATCH 2/3] ipa: rkisp1: Alias lineDuration","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"On 14/10/2024 16:47, Kieran Bingham wrote:\n> The configured line duration of the sensor is used frequently throughout\n> the AGC implementation.\n>\n> It's available in the IPA context through the rather long:\n> context.configuration.sensor.lineDuration\n>\n> Take a copy of the lineDuration early in the call and replace the two\n> current usages of the reference with the shorter copy to manage line\n> length and ease readibility.\n>\n> Signed-off-by: Kieran Bingham \n> ---\nReviewed-by: Daniel Scally \n> src/ipa/rkisp1/algorithms/agc.cpp | 7 ++++---\n> 1 file changed, 4 insertions(+), 3 deletions(-)\n>\n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index 33f902862c4a..e23ab120b3e2 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -400,6 +400,8 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> \t\treturn;\n> \t}\n> \n> +\tutils::Duration lineDuration = context.configuration.sensor.lineDuration;\n> +\n> \t/*\n> \t * \\todo Verify that the exposure and gain applied by the sensor for\n> \t * this frame match what has been requested. This isn't a hard\n> @@ -429,8 +431,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> \t * The Agc algorithm needs to know the effective exposure value that was\n> \t * applied to the sensor when the statistics were collected.\n> \t */\n> -\tutils::Duration exposureTime = context.configuration.sensor.lineDuration\n> -\t\t\t\t * frameContext.sensor.exposure;\n> +\tutils::Duration exposureTime = lineDuration * frameContext.sensor.exposure;\n> \tdouble analogueGain = frameContext.sensor.gain;\n> \tutils::Duration effectiveExposureValue = exposureTime * analogueGain;\n> \n> @@ -447,7 +448,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> \n> \tIPAActiveState &activeState = context.activeState;\n> \t/* Update the estimated exposure and gain. */\n> -\tactiveState.agc.automatic.exposure = shutterTime / context.configuration.sensor.lineDuration;\n> +\tactiveState.agc.automatic.exposure = shutterTime / lineDuration;\n> \tactiveState.agc.automatic.gain = aGain;\n> \n> \tfillMetadata(context, frameContext, metadata);","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 1DC5DBE08B\n\tfor ;\n\tThu, 24 Oct 2024 09:30:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EB51465395;\n\tThu, 24 Oct 2024 11:30:07 +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 A0E2E6538A\n\tfor ;\n\tThu, 24 Oct 2024 11:30:05 +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 8B4203D5\n\tfor ;\n\tThu, 24 Oct 2024 11:28:17 +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=\"r+LM21An\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1729762097;\n\tbh=lAaxgL/Lp/PqMs/EmgbtGtLC9mXuWaJZTaxk/F4w2y8=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=r+LM21Anj81sycoGHVH3WYPVCW4tIWSwlZ7GmVX+E9oJTxvsYKzGl0SRa16kitgWH\n\t6e36vO6ySuNu0I9FDuB6eBRMt6/PA5CJDhEvl8LQjhI6IrDwuc10IRIJx5g1c56eP0\n\tXdNG/5JfaDruW9NT4dGT9peGm/9a57DhLVOnyCkw=","Message-ID":"<1eba3009-4dba-4877-a844-e05d4dbc5abb@ideasonboard.com>","Date":"Thu, 24 Oct 2024 10:30:02 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 2/3] ipa: rkisp1: Alias lineDuration","To":"libcamera-devel@lists.libcamera.org","References":"<20241014154747.2295253-1-kieran.bingham@ideasonboard.com>\n\t<20241014154747.2295253-3-kieran.bingham@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":"<20241014154747.2295253-3-kieran.bingham@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":31909,"web_url":"https://patchwork.libcamera.org/comment/31909/","msgid":"","date":"2024-10-24T15:39:48","subject":"Re: [PATCH 2/3] ipa: rkisp1: Alias lineDuration","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Kieran\n\nOn Mon, Oct 14, 2024 at 04:47:46PM +0100, Kieran Bingham wrote:\n> The configured line duration of the sensor is used frequently throughout\n> the AGC implementation.\n>\n> It's available in the IPA context through the rather long:\n> context.configuration.sensor.lineDuration\n>\n> Take a copy of the lineDuration early in the call and replace the two\n> current usages of the reference with the shorter copy to manage line\n> length and ease readibility.\n>\n> Signed-off-by: Kieran Bingham \n> ---\n> src/ipa/rkisp1/algorithms/agc.cpp | 7 ++++---\n> 1 file changed, 4 insertions(+), 3 deletions(-)\n>\n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index 33f902862c4a..e23ab120b3e2 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -400,6 +400,8 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> \t\treturn;\n> \t}\n>\n> +\tutils::Duration lineDuration = context.configuration.sensor.lineDuration;\n> +\n\nIf you want to enforce this is read-only you can take a const & maybe\n\n> \t/*\n> \t * \\todo Verify that the exposure and gain applied by the sensor for\n> \t * this frame match what has been requested. This isn't a hard\n> @@ -429,8 +431,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> \t * The Agc algorithm needs to know the effective exposure value that was\n> \t * applied to the sensor when the statistics were collected.\n> \t */\n> -\tutils::Duration exposureTime = context.configuration.sensor.lineDuration\n> -\t\t\t\t * frameContext.sensor.exposure;\n> +\tutils::Duration exposureTime = lineDuration * frameContext.sensor.exposure;\n> \tdouble analogueGain = frameContext.sensor.gain;\n> \tutils::Duration effectiveExposureValue = exposureTime * analogueGain;\n>\n> @@ -447,7 +448,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n>\n> \tIPAActiveState &activeState = context.activeState;\n> \t/* Update the estimated exposure and gain. */\n> -\tactiveState.agc.automatic.exposure = shutterTime / context.configuration.sensor.lineDuration;\n> +\tactiveState.agc.automatic.exposure = shutterTime / lineDuration;\n> \tactiveState.agc.automatic.gain = aGain;\n>\n> \tfillMetadata(context, frameContext, metadata);\n> --\n> 2.34.1\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 A4E46C3220\n\tfor ;\n\tThu, 24 Oct 2024 15:40:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 54B1265393;\n\tThu, 24 Oct 2024 17:40:03 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A071B618C1\n\tfor ;\n\tThu, 24 Oct 2024 17:40:01 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 89CAB59D;\n\tThu, 24 Oct 2024 17:38:09 +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=\"v7TpHHcq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1729784293;\n\tbh=apudYhD+hu8d7xpnFJnKKSzRrp0zpu+ZkogqJDjvbrA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=v7TpHHcq6QBzRMHdBkXoKMdKEr7J7Z7xpRoyqieYOZSgxc2QFZyaeTCyLlf+VTS7M\n\tT9uHcDjtKkYBseVAafj1F3L4pSaTiGYBWuhRukarAQnITx18CQtQXrSLXLwtdCJ5Z5\n\te3FL5cOEZ/gZMP8rUcELWYx2K7SJcCNx57RN+vGI=","Date":"Thu, 24 Oct 2024 17:39:48 +0200","From":"Jacopo Mondi ","To":"Kieran Bingham ","Cc":"libcamera devel ","Subject":"Re: [PATCH 2/3] ipa: rkisp1: Alias lineDuration","Message-ID":"","References":"<20241014154747.2295253-1-kieran.bingham@ideasonboard.com>\n\t<20241014154747.2295253-3-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241014154747.2295253-3-kieran.bingham@ideasonboard.com>","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":31927,"web_url":"https://patchwork.libcamera.org/comment/31927/","msgid":"<20241027185555.GD24052@pendragon.ideasonboard.com>","date":"2024-10-27T18:55:55","subject":"Re: [PATCH 2/3] ipa: rkisp1: Alias lineDuration","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Oct 24, 2024 at 05:39:48PM +0200, Jacopo Mondi wrote:\n> Hi Kieran\n> \n> On Mon, Oct 14, 2024 at 04:47:46PM +0100, Kieran Bingham wrote:\n> > The configured line duration of the sensor is used frequently throughout\n> > the AGC implementation.\n> >\n> > It's available in the IPA context through the rather long:\n> > context.configuration.sensor.lineDuration\n> >\n> > Take a copy of the lineDuration early in the call and replace the two\n> > current usages of the reference with the shorter copy to manage line\n> > length and ease readibility.\n> >\n> > Signed-off-by: Kieran Bingham \n> > ---\n> > src/ipa/rkisp1/algorithms/agc.cpp | 7 ++++---\n> > 1 file changed, 4 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> > index 33f902862c4a..e23ab120b3e2 100644\n> > --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> > @@ -400,6 +400,8 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> > \t\treturn;\n> > \t}\n> >\n> > +\tutils::Duration lineDuration = context.configuration.sensor.lineDuration;\n> > +\n> \n> If you want to enforce this is read-only you can take a const & maybe\n\nconst sounds good. It doesn't have to be a reference as the type is\nsmall, it will only copy an integer.\n\nWith an added const,\n\nReviewed-by: Laurent Pinchart \n\n> > \t/*\n> > \t * \\todo Verify that the exposure and gain applied by the sensor for\n> > \t * this frame match what has been requested. This isn't a hard\n> > @@ -429,8 +431,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> > \t * The Agc algorithm needs to know the effective exposure value that was\n> > \t * applied to the sensor when the statistics were collected.\n> > \t */\n> > -\tutils::Duration exposureTime = context.configuration.sensor.lineDuration\n> > -\t\t\t\t * frameContext.sensor.exposure;\n> > +\tutils::Duration exposureTime = lineDuration * frameContext.sensor.exposure;\n> > \tdouble analogueGain = frameContext.sensor.gain;\n> > \tutils::Duration effectiveExposureValue = exposureTime * analogueGain;\n> >\n> > @@ -447,7 +448,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> >\n> > \tIPAActiveState &activeState = context.activeState;\n> > \t/* Update the estimated exposure and gain. */\n> > -\tactiveState.agc.automatic.exposure = shutterTime / context.configuration.sensor.lineDuration;\n> > +\tactiveState.agc.automatic.exposure = shutterTime / lineDuration;\n> > \tactiveState.agc.automatic.gain = aGain;\n> >\n> > \tfillMetadata(context, frameContext, metadata);","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 86974C3220\n\tfor ;\n\tSun, 27 Oct 2024 18:56:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C381F6539E;\n\tSun, 27 Oct 2024 19:56:02 +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 BA7E365398\n\tfor ;\n\tSun, 27 Oct 2024 19:56:00 +0100 (CET)","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 390182C5;\n\tSun, 27 Oct 2024 19:55:59 +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=\"cvmts9FV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1730055359;\n\tbh=2zGkqN2Jyit1eIGTZyi5eDatZSoUWXIcT90gLb1MDog=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=cvmts9FVTBMKNK4qELZot/mEtrAKq2hCIcpLBRkU0E94hXd01U3V+qlx12BWNd4a0\n\titD6B6E1rvrL2TgPeCbBZaUiw4K9AZXNz1iWy9ovz1wzTvGBpZZ0x96jYJLvCRxNX+\n\tNwyWUv77xZHtpYN/QWHS1rX6tlVqa3oWq3YHH6dE=","Date":"Sun, 27 Oct 2024 20:55:55 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"Kieran Bingham ,\n\tlibcamera devel ","Subject":"Re: [PATCH 2/3] ipa: rkisp1: Alias lineDuration","Message-ID":"<20241027185555.GD24052@pendragon.ideasonboard.com>","References":"<20241014154747.2295253-1-kieran.bingham@ideasonboard.com>\n\t<20241014154747.2295253-3-kieran.bingham@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","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":32230,"web_url":"https://patchwork.libcamera.org/comment/32230/","msgid":"","date":"2024-11-18T13:57:24","subject":"Re: [PATCH 2/3] ipa: rkisp1: Alias lineDuration","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Mon, Oct 14, 2024 at 04:47:46PM +0100, Kieran Bingham wrote:\n> The configured line duration of the sensor is used frequently throughout\n> the AGC implementation.\n> \n> It's available in the IPA context through the rather long:\n> context.configuration.sensor.lineDuration\n> \n> Take a copy of the lineDuration early in the call and replace the two\n> current usages of the reference with the shorter copy to manage line\n> length and ease readibility.\n> \n> Signed-off-by: Kieran Bingham \n\nReviewed-by: Paul Elder \n\n> ---\n> src/ipa/rkisp1/algorithms/agc.cpp | 7 ++++---\n> 1 file changed, 4 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index 33f902862c4a..e23ab120b3e2 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -400,6 +400,8 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> \t\treturn;\n> \t}\n> \n> +\tutils::Duration lineDuration = context.configuration.sensor.lineDuration;\n> +\n> \t/*\n> \t * \\todo Verify that the exposure and gain applied by the sensor for\n> \t * this frame match what has been requested. This isn't a hard\n> @@ -429,8 +431,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> \t * The Agc algorithm needs to know the effective exposure value that was\n> \t * applied to the sensor when the statistics were collected.\n> \t */\n> -\tutils::Duration exposureTime = context.configuration.sensor.lineDuration\n> -\t\t\t\t * frameContext.sensor.exposure;\n> +\tutils::Duration exposureTime = lineDuration * frameContext.sensor.exposure;\n> \tdouble analogueGain = frameContext.sensor.gain;\n> \tutils::Duration effectiveExposureValue = exposureTime * analogueGain;\n> \n> @@ -447,7 +448,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> \n> \tIPAActiveState &activeState = context.activeState;\n> \t/* Update the estimated exposure and gain. */\n> -\tactiveState.agc.automatic.exposure = shutterTime / context.configuration.sensor.lineDuration;\n> +\tactiveState.agc.automatic.exposure = shutterTime / lineDuration;\n> \tactiveState.agc.automatic.gain = aGain;\n> \n> \tfillMetadata(context, frameContext, metadata);\n> -- \n> 2.34.1\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 938ACC32EA\n\tfor ;\n\tMon, 18 Nov 2024 13:57:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3EB57658E0;\n\tMon, 18 Nov 2024 14:57:32 +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 CD7D060532\n\tfor ;\n\tMon, 18 Nov 2024 14:57:30 +0100 (CET)","from pyrite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:fb8e:b30c:9a4a:eed0])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1934E316;\n\tMon, 18 Nov 2024 14:57:12 +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=\"TBFSZq/3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1731938234;\n\tbh=bZtqh6MukG5hdPctDEzuswP2HoXX93SEHb5zGseKNS8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=TBFSZq/3jvp8aI4UB+jGf+WdgUimPxLigypDfXDN4pSiTQE5FM6zzGzw4dQx9Cnpb\n\tvrw1ztFvkwWhXkXoi7yf5NYzW5PVO8hVGRjNkYtpS2K0rxyPGAl07237WZp7Nv6FF1\n\tI+ehUEfJbD9P6XiKJJHPIF/vhwA8gvYTNeU2t8sA=","Date":"Mon, 18 Nov 2024 22:57:24 +0900","From":"Paul Elder ","To":"Kieran Bingham ","Cc":"libcamera devel ","Subject":"Re: [PATCH 2/3] ipa: rkisp1: Alias lineDuration","Message-ID":"","References":"<20241014154747.2295253-1-kieran.bingham@ideasonboard.com>\n\t<20241014154747.2295253-3-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20241014154747.2295253-3-kieran.bingham@ideasonboard.com>","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\" "}}]