[{"id":33514,"web_url":"https://patchwork.libcamera.org/comment/33514/","msgid":"<963b9637-05e8-45c0-b67d-fca6dba72ea8@ideasonboard.com>","date":"2025-02-28T13:00:09","subject":"Re: [PATCH 2/3] ipa: libipa: agc_mean_luminance: Error out when\n\teffectiveExposureValue is zero","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Stefan\n\nOn 28/02/2025 12:55, Stefan Klug wrote:\n> In a proper system it never happens that the effectiveExposureValue\n> drops to zero. If that still happens due to a bug outside of\n> agc_mean_luminance, the calculated gain goes towards infinity but the\n> newExposureValue is still 0 because it is the result of multiplying the\n> effectiveExposureTime with the gain, leading to wild oscillations.\n>\n> Catch that condition, print an error message and set the new effective\n> exposure value to an arbitrary 10ms.\n>\n> Note that in any case the underlying problem must be fixed. The\n> important change is the added error message to be able to detect such a\n> situation.\n>\n> Signed-off-by: Stefan Klug \nReviewed-by: Daniel Scally \n> ---\n> src/ipa/libipa/agc_mean_luminance.cpp | 7 +++++++\n> 1 file changed, 7 insertions(+)\n>\n> diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp\n> index 02555a44d271..a7343c18f5aa 100644\n> --- a/src/ipa/libipa/agc_mean_luminance.cpp\n> +++ b/src/ipa/libipa/agc_mean_luminance.cpp\n> @@ -541,6 +541,13 @@ AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex,\n> \tstd::shared_ptr exposureModeHelper =\n> \t\texposureModeHelpers_.at(exposureModeIndex);\n> \n> +\tif (effectiveExposureValue == 0s) {\n> +\t\tLOG(AgcMeanLuminance, Error) << \"Effective exposure value is 0.\"\n> +\t\t\t\t\t << \" Resetting exposure time and\"\n> +\t\t\t\t\t << \" gain to arbitrary defaults.\";\n> +\t\treturn exposureModeHelper->splitExposure(10ms);\n> +\t}\n> +\n> \tdouble gain = estimateInitialGain();\n> \tgain = constraintClampGain(constraintModeIndex, yHist, gain);\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 D2FB5BE080\n\tfor ;\n\tFri, 28 Feb 2025 13:00:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C34B468778;\n\tFri, 28 Feb 2025 14:00:13 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A1ACE61853\n\tfor ;\n\tFri, 28 Feb 2025 14:00:12 +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 30ED2606\n\tfor ;\n\tFri, 28 Feb 2025 13:58:43 +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=\"sW7R3DVh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1740747523;\n\tbh=Y/rx0ZHqxgV1JNXNH5ip5n9Sl1b0lBiCOs6NjgX/6eQ=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=sW7R3DVhkbj6zp0Xi3Zs+12fXm/q/max8RjN2RRE7x2wnpN9exLLX9BhpGBdKKZdn\n\t4g6JgC65xSSss2MRRoORsIiAFjvhhYi+qRTvw9oqVeH/vXo2R/NsGOxFyd/BOQRiET\n\tzKlBXOYRZto4pOdl4qQYDAmqo3Kv+FcSVPrO5hmE=","Message-ID":"<963b9637-05e8-45c0-b67d-fca6dba72ea8@ideasonboard.com>","Date":"Fri, 28 Feb 2025 13:00:09 +0000","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 2/3] ipa: libipa: agc_mean_luminance: Error out when\n\teffectiveExposureValue is zero","To":"libcamera-devel@lists.libcamera.org","References":"<20250228125600.3241397-1-stefan.klug@ideasonboard.com>\n\t<20250228125600.3241397-3-stefan.klug@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":"<20250228125600.3241397-3-stefan.klug@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":33515,"web_url":"https://patchwork.libcamera.org/comment/33515/","msgid":"<199b7a407d49d0d4a6de9d4e1c35c75893c185a1.camel@ideasonboard.com>","date":"2025-02-28T14:28:19","subject":"Re: [PATCH 2/3] ipa: libipa: agc_mean_luminance: Error out when\n\teffectiveExposureValue is zero","submitter":{"id":215,"url":"https://patchwork.libcamera.org/api/people/215/","name":"Isaac Scott","email":"isaac.scott@ideasonboard.com"},"content":"Hi Stefan,\n\nOn Fri, 2025-02-28 at 13:55 +0100, Stefan Klug wrote:\n> In a proper system it never happens that the effectiveExposureValue\n> drops to zero. If that still happens due to a bug outside of\n> agc_mean_luminance, the calculated gain goes towards infinity but the\n> newExposureValue is still 0 because it is the it never happens that\n> the effectiveExposureValue drops to zero. If that still happens due\n> to a bug outside of\n\nWhat conditions lead to this being zero when it shouldn't?\n\nIf they can be zero, this looks good to me!\n\n> +\n>  \tdouble gain = estimateInitialGain();\n>  \tgain = constraintClampGain(constraintModeIndex, yHist,\n> gain);\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 F2F37C3259\n\tfor ;\n\tFri, 28 Feb 2025 15:28:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DB0A668778;\n\tFri, 28 Feb 2025 16:28:35 +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 3307861853\n\tfor ;\n\tFri, 28 Feb 2025 16:28:34 +0100 (CET)","from [172.20.10.10] (unknown [85.255.232.248])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0CB6D8BE;\n\tFri, 28 Feb 2025 16:27:03 +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=\"nqQtSVZP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1740756424;\n\tbh=8e72tzeFnzTBxrKJoQIa7MHLc08XXFXOIjzYR6sMmEs=;\n\th=Subject:From:To:Date:In-Reply-To:References:From;\n\tb=nqQtSVZPvbCcs9hPlnqC2xoGk6UuK9yGzfWEVl4K+0JE3o+yOp6ejfWKi1Tbp+QJW\n\t7rjohq3Apvtxv8/MDQVzQHWN5py7YSi+pjaZQml+8AJKW098i6JtZJ0wbgU0btk+pG\n\tzq9hUTLmDw5KsqGZbtj5woWipTGq56lDA9bflO8s=","Message-ID":"<199b7a407d49d0d4a6de9d4e1c35c75893c185a1.camel@ideasonboard.com>","Subject":"Re: [PATCH 2/3] ipa: libipa: agc_mean_luminance: Error out when\n\teffectiveExposureValue is zero","From":"Isaac Scott ","To":"Stefan Klug , \n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 28 Feb 2025 14:28:19 +0000","In-Reply-To":"<20250228125600.3241397-3-stefan.klug@ideasonboard.com>","References":"<20250228125600.3241397-1-stefan.klug@ideasonboard.com>\n\t<20250228125600.3241397-3-stefan.klug@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","User-Agent":"Evolution 3.54.3 (by Flathub.org) ","MIME-Version":"1.0","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":33585,"web_url":"https://patchwork.libcamera.org/comment/33585/","msgid":"","date":"2025-03-05T09:57:27","subject":"Re: [PATCH 2/3] ipa: libipa: agc_mean_luminance: Error out when\n\teffectiveExposureValue is zero","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Isaac,\n\nThank you for the review. \n\nOn Fri, Feb 28, 2025 at 02:28:19PM +0000, Isaac Scott wrote:\n> Hi Stefan,\n> \n> On Fri, 2025-02-28 at 13:55 +0100, Stefan Klug wrote:\n> > In a proper system it never happens that the effectiveExposureValue\n> > drops to zero. If that still happens due to a bug outside of\n> > agc_mean_luminance, the calculated gain goes towards infinity but the\n> > newExposureValue is still 0 because it is the it never happens that\n> > the effectiveExposureValue drops to zero. If that still happens due\n> > to a bug outside of\n> \n> What conditions lead to this being zero when it shouldn't?\n\nOne condition of this kind gets fixed in the next patch. In that case\nthe sensor was set to an exposure time of 0 (the root cause). Now a bit\nlater statistics for that frame come in and as the frame context\ncontains an exposure time of 0 the effectiveExposureValue is also 0,\nbreaking all calculations.\n\n> \n> If they can be zero, this looks good to me!\n\nGreat :-). Would you mind giving it a reviewed-by?\n\nBest regards,\nStefan\n\n> \n> > +\n> >  \tdouble gain = estimateInitialGain();\n> >  \tgain = constraintClampGain(constraintModeIndex, yHist,\n> > gain);\n> >  \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 BA4ABC3257\n\tfor ;\n\tWed, 5 Mar 2025 09:57:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DB7D768822;\n\tWed, 5 Mar 2025 10:57:31 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D30CC68771\n\tfor ;\n\tWed, 5 Mar 2025 10:57:30 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:e13d:9941:bf88:d66d])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E334363F;\n\tWed, 5 Mar 2025 10:55:57 +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=\"qD6tABE7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1741168558;\n\tbh=fliI8J3JjKVFXYmgIJ+VZyc4+nKLvU7upcRSJgFRMRY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qD6tABE7IbzTAl7jX4uaCINJwigvk1YIb1OLkBtI01lYWewQkxgIAe/16JwemqkdS\n\txx4GA87qoCs50irS5KQFl23JlgT8hb6u6rRfz9CQXx+sH4YV8sWou3DnrJeY03CGGL\n\t6dPW2NlloCTX2ewZL2m/wXExwWGu699WzjQX+P+A=","Date":"Wed, 5 Mar 2025 10:57:27 +0100","From":"Stefan Klug ","To":"Isaac Scott ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 2/3] ipa: libipa: agc_mean_luminance: Error out when\n\teffectiveExposureValue is zero","Message-ID":"","References":"<20250228125600.3241397-1-stefan.klug@ideasonboard.com>\n\t<20250228125600.3241397-3-stefan.klug@ideasonboard.com>\n\t<199b7a407d49d0d4a6de9d4e1c35c75893c185a1.camel@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<199b7a407d49d0d4a6de9d4e1c35c75893c185a1.camel@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":33715,"web_url":"https://patchwork.libcamera.org/comment/33715/","msgid":"<20250326122439.GC29473@pendragon.ideasonboard.com>","date":"2025-03-26T12:24:39","subject":"Re: [PATCH 2/3] ipa: libipa: agc_mean_luminance: Error out when\n\teffectiveExposureValue is zero","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Stefan,\n\nThank you for the patch.\n\nOn Fri, Feb 28, 2025 at 01:55:54PM +0100, Stefan Klug wrote:\n> In a proper system it never happens that the effectiveExposureValue\n> drops to zero. If that still happens due to a bug outside of\n> agc_mean_luminance, the calculated gain goes towards infinity but the\n> newExposureValue is still 0 because it is the result of multiplying the\n> effectiveExposureTime with the gain, leading to wild oscillations.\n> \n> Catch that condition, print an error message and set the new effective\n> exposure value to an arbitrary 10ms.\n> \n> Note that in any case the underlying problem must be fixed. The\n> important change is the added error message to be able to detect such a\n> situation.\n\nI'm tempted to then return all 0's. If this indicates a bug somewhere\nelse, the bug should be fixed, is there a value in working around it ?\n\n> Signed-off-by: Stefan Klug \n> ---\n> src/ipa/libipa/agc_mean_luminance.cpp | 7 +++++++\n> 1 file changed, 7 insertions(+)\n> \n> diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp\n> index 02555a44d271..a7343c18f5aa 100644\n> --- a/src/ipa/libipa/agc_mean_luminance.cpp\n> +++ b/src/ipa/libipa/agc_mean_luminance.cpp\n> @@ -541,6 +541,13 @@ AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex,\n> \tstd::shared_ptr exposureModeHelper =\n> \t\texposureModeHelpers_.at(exposureModeIndex);\n> \n> +\tif (effectiveExposureValue == 0s) {\n> +\t\tLOG(AgcMeanLuminance, Error) << \"Effective exposure value is 0.\"\n> +\t\t\t\t\t << \" Resetting exposure time and\"\n> +\t\t\t\t\t << \" gain to arbitrary defaults.\";\n\nYou can write\n\n\t\tLOG(AgcMeanLuminance, Error)\n\t\t\t<< \"Effective exposure value is 0. Resetting exposure \"\n\t\t\t \"time and gain to arbitrary defaults.\";\n\nwhich will split the string on the lines, but won't call the << operator\nmultiple times.\n\nI'd focus the error message on reporting the underlying problem. As it\nis, I don't think it would be clear for users what to do.\n\n\t\tLOG(AgcMeanLuminance, Error)\n\t\t\t<< \"Effective exposure value is 0. This is a bug in AGC \"\n\t\t\t \"and must be fixed for proper operation.\"\n\nor something like that ?\n\n> +\t\treturn exposureModeHelper->splitExposure(10ms);\n> +\t}\n> +\n> \tdouble gain = estimateInitialGain();\n> \tgain = constraintClampGain(constraintModeIndex, yHist, gain);\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 8FAE2C3213\n\tfor ;\n\tWed, 26 Mar 2025 12:25:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6770E6896A;\n\tWed, 26 Mar 2025 13:25:04 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F28C868950\n\tfor ;\n\tWed, 26 Mar 2025 13:25:02 +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 B83AC3A4;\n\tWed, 26 Mar 2025 13:23:14 +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=\"J1FUt2jo\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1742991795;\n\tbh=JuxoUHm39tNxNvR+SLNbn0vhNST8eHb/oPQ4F+cnz/8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=J1FUt2jo5MOKY+L3f359ZZcvAbTbDHn9BDG+t+n/FM7fF9uMfP8yhupGgnb/uQShG\n\teM/zukWmu4XzPPRm/XynwHzmE/12RFJ5IGSQXHR2RwIeAw3ToyM+iXIuhi42yZK2Kf\n\ts221q0JtIN7gwuYbKZkfG/avxnSv22RDIwchhJq8=","Date":"Wed, 26 Mar 2025 14:24:39 +0200","From":"Laurent Pinchart ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 2/3] ipa: libipa: agc_mean_luminance: Error out when\n\teffectiveExposureValue is zero","Message-ID":"<20250326122439.GC29473@pendragon.ideasonboard.com>","References":"<20250228125600.3241397-1-stefan.klug@ideasonboard.com>\n\t<20250228125600.3241397-3-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250228125600.3241397-3-stefan.klug@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":33718,"web_url":"https://patchwork.libcamera.org/comment/33718/","msgid":"","date":"2025-03-26T12:50:18","subject":"Re: [PATCH 2/3] ipa: libipa: agc_mean_luminance: Error out when\n\teffectiveExposureValue is zero","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Laurent,\n\nThank you for the review. \n\nOn Wed, Mar 26, 2025 at 02:24:39PM +0200, Laurent Pinchart wrote:\n> Hi Stefan,\n> \n> Thank you for the patch.\n> \n> On Fri, Feb 28, 2025 at 01:55:54PM +0100, Stefan Klug wrote:\n> > In a proper system it never happens that the effectiveExposureValue\n> > drops to zero. If that still happens due to a bug outside of\n> > agc_mean_luminance, the calculated gain goes towards infinity but the\n> > newExposureValue is still 0 because it is the result of multiplying the\n> > effectiveExposureTime with the gain, leading to wild oscillations.\n> > \n> > Catch that condition, print an error message and set the new effective\n> > exposure value to an arbitrary 10ms.\n> > \n> > Note that in any case the underlying problem must be fixed. The\n> > important change is the added error message to be able to detect such a\n> > situation.\n> \n> I'm tempted to then return all 0's. If this indicates a bug somewhere\n> else, the bug should be fixed, is there a value in working around it ?\n\nThe idea was that an exposure time of 0 is in all cases a bad thing and\nthe risk is higher to end up stuck somewhere in the regulation.\n\n> \n> > Signed-off-by: Stefan Klug \n> > ---\n> > src/ipa/libipa/agc_mean_luminance.cpp | 7 +++++++\n> > 1 file changed, 7 insertions(+)\n> > \n> > diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp\n> > index 02555a44d271..a7343c18f5aa 100644\n> > --- a/src/ipa/libipa/agc_mean_luminance.cpp\n> > +++ b/src/ipa/libipa/agc_mean_luminance.cpp\n> > @@ -541,6 +541,13 @@ AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex,\n> > \tstd::shared_ptr exposureModeHelper =\n> > \t\texposureModeHelpers_.at(exposureModeIndex);\n> > \n> > +\tif (effectiveExposureValue == 0s) {\n> > +\t\tLOG(AgcMeanLuminance, Error) << \"Effective exposure value is 0.\"\n> > +\t\t\t\t\t << \" Resetting exposure time and\"\n> > +\t\t\t\t\t << \" gain to arbitrary defaults.\";\n> \n> You can write\n> \n> \t\tLOG(AgcMeanLuminance, Error)\n> \t\t\t<< \"Effective exposure value is 0. Resetting exposure \"\n> \t\t\t \"time and gain to arbitrary defaults.\";\n> \n> which will split the string on the lines, but won't call the << operator\n> multiple times.\n> \n> I'd focus the error message on reporting the underlying problem. As it\n> is, I don't think it would be clear for users what to do.\n> \n> \t\tLOG(AgcMeanLuminance, Error)\n> \t\t\t<< \"Effective exposure value is 0. This is a bug in AGC \"\n> \t\t\t \"and must be fixed for proper operation.\"\n> \n> or something like that ?\n\nThat makes things clearer. I'll change that.\n\nThanks,\nStefan\n\n> \n> > +\t\treturn exposureModeHelper->splitExposure(10ms);\n> > +\t}\n> > +\n> > \tdouble gain = estimateInitialGain();\n> > \tgain = constraintClampGain(constraintModeIndex, yHist, gain);\n> > \n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 41526C3213\n\tfor ;\n\tWed, 26 Mar 2025 12:50:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0545A6896D;\n\tWed, 26 Mar 2025 13:50:22 +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 A7FB568950\n\tfor ;\n\tWed, 26 Mar 2025 13:50:20 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:b21a:42f4:9f3:1df6])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B02A53A4;\n\tWed, 26 Mar 2025 13:48:32 +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=\"f028DQeD\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1742993312;\n\tbh=sxja6Hwauh/TcqYBZjuXy7DAtO0dW2JLOVR33sPN414=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=f028DQeDYB+Leml/V1oz2xY+labgx0g8pi61gfFDS24QdrBo3cCR3YUqJFA6gjrNq\n\tRDBd/gVXdncFoH/MDsVxPghW/1K4vQO2I1PbXhUlL/sQrVJXOuXb56IXzSCKF6tOEN\n\tUxHxjICeAzywVzOFaa397wigvRE1d0U3bkPKHdUo=","Date":"Wed, 26 Mar 2025 13:50:18 +0100","From":"Stefan Klug ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 2/3] ipa: libipa: agc_mean_luminance: Error out when\n\teffectiveExposureValue is zero","Message-ID":"","References":"<20250228125600.3241397-1-stefan.klug@ideasonboard.com>\n\t<20250228125600.3241397-3-stefan.klug@ideasonboard.com>\n\t<20250326122439.GC29473@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250326122439.GC29473@pendragon.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":33720,"web_url":"https://patchwork.libcamera.org/comment/33720/","msgid":"<20250326125820.GB32291@pendragon.ideasonboard.com>","date":"2025-03-26T12:58:20","subject":"Re: [PATCH 2/3] ipa: libipa: agc_mean_luminance: Error out when\n\teffectiveExposureValue is zero","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Mar 26, 2025 at 01:50:18PM +0100, Stefan Klug wrote:\n> On Wed, Mar 26, 2025 at 02:24:39PM +0200, Laurent Pinchart wrote:\n> > On Fri, Feb 28, 2025 at 01:55:54PM +0100, Stefan Klug wrote:\n> > > In a proper system it never happens that the effectiveExposureValue\n> > > drops to zero. If that still happens due to a bug outside of\n> > > agc_mean_luminance, the calculated gain goes towards infinity but the\n> > > newExposureValue is still 0 because it is the result of multiplying the\n> > > effectiveExposureTime with the gain, leading to wild oscillations.\n> > > \n> > > Catch that condition, print an error message and set the new effective\n> > > exposure value to an arbitrary 10ms.\n> > > \n> > > Note that in any case the underlying problem must be fixed. The\n> > > important change is the added error message to be able to detect such a\n> > > situation.\n> > \n> > I'm tempted to then return all 0's. If this indicates a bug somewhere\n> > else, the bug should be fixed, is there a value in working around it ?\n> \n> The idea was that an exposure time of 0 is in all cases a bad thing and\n> the risk is higher to end up stuck somewhere in the regulation.\n\nHmmm... so your point is that this patch will help recovering from an\neffectiveExposureValue occasionally being 0 ? I was thinking of a case\nwhere it would be stuck at 0 constantly. I agree recovering from\ntransient bugs in the caller is a good idea. A comment to explain this\nin the code would be useful.\n\n> > > Signed-off-by: Stefan Klug \n> > > ---\n> > > src/ipa/libipa/agc_mean_luminance.cpp | 7 +++++++\n> > > 1 file changed, 7 insertions(+)\n> > > \n> > > diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp\n> > > index 02555a44d271..a7343c18f5aa 100644\n> > > --- a/src/ipa/libipa/agc_mean_luminance.cpp\n> > > +++ b/src/ipa/libipa/agc_mean_luminance.cpp\n> > > @@ -541,6 +541,13 @@ AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex,\n> > > \tstd::shared_ptr exposureModeHelper =\n> > > \t\texposureModeHelpers_.at(exposureModeIndex);\n> > > \n> > > +\tif (effectiveExposureValue == 0s) {\n> > > +\t\tLOG(AgcMeanLuminance, Error) << \"Effective exposure value is 0.\"\n> > > +\t\t\t\t\t << \" Resetting exposure time and\"\n> > > +\t\t\t\t\t << \" gain to arbitrary defaults.\";\n> > \n> > You can write\n> > \n> > \t\tLOG(AgcMeanLuminance, Error)\n> > \t\t\t<< \"Effective exposure value is 0. Resetting exposure \"\n> > \t\t\t \"time and gain to arbitrary defaults.\";\n> > \n> > which will split the string on the lines, but won't call the << operator\n> > multiple times.\n> > \n> > I'd focus the error message on reporting the underlying problem. As it\n> > is, I don't think it would be clear for users what to do.\n> > \n> > \t\tLOG(AgcMeanLuminance, Error)\n> > \t\t\t<< \"Effective exposure value is 0. This is a bug in AGC \"\n> > \t\t\t \"and must be fixed for proper operation.\"\n> > \n> > or something like that ?\n> \n> That makes things clearer. I'll change that.\n> \n> > > +\t\treturn exposureModeHelper->splitExposure(10ms);\n> > > +\t}\n> > > +\n> > > \tdouble gain = estimateInitialGain();\n> > > \tgain = constraintClampGain(constraintModeIndex, yHist, gain);\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 581E0C3213\n\tfor ;\n\tWed, 26 Mar 2025 12:58:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7F4176896A;\n\tWed, 26 Mar 2025 13:58:45 +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 7450F68950\n\tfor ;\n\tWed, 26 Mar 2025 13:58:43 +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 535383A4;\n\tWed, 26 Mar 2025 13:56:55 +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=\"r9Nj6BpK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1742993815;\n\tbh=kijnfNiWYovo/7/nmGcVn+j+qFO6hqfyZUfdvS0irwU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=r9Nj6BpKhkN9Jj5g+YfY3yIbUnjE1oSGoZQlJ955Ews+IAfpxuZqYe+FNpd2mh7Nq\n\tl9HEEfqkH2eihpDViaMhjZKSo+QAZWhekAx2iEwqTs3OMqYQ3fe+ePjHsYuP2CVAfo\n\tdQ2Dc6DeklsCKTwTsf0f11FP9v7X8RKw1Q83QUJs=","Date":"Wed, 26 Mar 2025 14:58:20 +0200","From":"Laurent Pinchart ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 2/3] ipa: libipa: agc_mean_luminance: Error out when\n\teffectiveExposureValue is zero","Message-ID":"<20250326125820.GB32291@pendragon.ideasonboard.com>","References":"<20250228125600.3241397-1-stefan.klug@ideasonboard.com>\n\t<20250228125600.3241397-3-stefan.klug@ideasonboard.com>\n\t<20250326122439.GC29473@pendragon.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\" "}}]