[{"id":36034,"web_url":"https://patchwork.libcamera.org/comment/36034/","msgid":"<8eb7cc12-b7f5-43bf-856a-5c9d8e773ed9@kernel.org>","date":"2025-09-29T11:35:57","subject":"Re: [PATCH v4 6/7] ipa: simple: blc: Prevent division by zero in BLC","submitter":{"id":239,"url":"https://patchwork.libcamera.org/api/people/239/","name":"Hans de Goede","email":"hansg@kernel.org"},"content":"Hi,\n\nOn 25-Sep-25 21:28, Milan Zamazal wrote:\n> When there are no values in the histogram, BLC simple IPA can crash on\n> division by zero. We cannot get anything meaningful in such a case\n> anyway, let's simply return from `process()' then.\n> \n> Tested-by: Barnabás Pőcze \n> Reviewed-by: Barnabás Pőcze \n> Reviewed-by: Maciej S. Szmigiero \n> Signed-off-by: Milan Zamazal \n> ---\n> src/ipa/simple/algorithms/blc.cpp | 5 +++++\n> 1 file changed, 5 insertions(+)\n> \n> diff --git a/src/ipa/simple/algorithms/blc.cpp b/src/ipa/simple/algorithms/blc.cpp\n> index 8c1e9ed08..ea5356443 100644\n> --- a/src/ipa/simple/algorithms/blc.cpp\n> +++ b/src/ipa/simple/algorithms/blc.cpp\n> @@ -77,6 +77,11 @@ void BlackLevel::process(IPAContext &context,\n> \tconstexpr float ignoredPercentage = 0.02;\n> \tconst unsigned int total =\n> \t\tstd::accumulate(begin(histogram), end(histogram), 0);\n> +\tif (total == 0) {\n> +\t\tLOG(IPASoftBL, Debug) << \"Not guessing black level, histogram is empty\";\n> +\t\treturn;\n> +\t}\n> +\nt \nI don't think this can ever happen ?\n\nWhat I've seen happening which triggers a divide by 0 in the AGC code is\nall samples being in the highest bin of the histogram, because the first\nframe of the sensor sometimes seems to have all 0x[3]ff as output, at least\nfor the top 3/4th of the frame or something like that.\n\nWhich causes a blacklevel of >= 244 which in turn causes a divide by 0\nin the AGC code.\n\nRegards,\n\nHans\n\n\n\n> \tconst unsigned int pixelThreshold = ignoredPercentage * total;\n> \tconst unsigned int histogramRatio = 256 / SwIspStats::kYHistogramSize;\n> \tconst unsigned int currentBlackIdx =","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 7068DC328C\n\tfor ;\n\tMon, 29 Sep 2025 11:36:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1DA366B5F8;\n\tMon, 29 Sep 2025 13:36:05 +0200 (CEST)","from tor.source.kernel.org (tor.source.kernel.org [172.105.4.254])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 90AA76B599\n\tfor ;\n\tMon, 29 Sep 2025 13:36:02 +0200 (CEST)","from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58])\n\tby tor.source.kernel.org (Postfix) with ESMTP id 988F162466;\n\tMon, 29 Sep 2025 11:36:01 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id 07C90C116D0;\n\tMon, 29 Sep 2025 11:35:59 +0000 (UTC)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=kernel.org header.i=@kernel.org\n\theader.b=\"NB8sZIQk\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org;\n\ts=k20201202; t=1759145761;\n\tbh=RB2J7/Z1JZUZB2Mj1Zizdo4iDID2s/Kbaj7lDZ8tyUQ=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=NB8sZIQksjgKWpp7//+XUibCBNNkMVTouN9gzf/ljh3dwd/xt4wmgYuKa9q+6j9KM\n\tP4ndhNs4Uf+FI+yFbwhPFPbsr0qlEKi1o1dtcY9iJ4m0ILbzAyI2wfMbfpI2LAypKs\n\tl+lTvmZgYf8yYjxELMuUDi6mQU8+ZSVCmMXu4Z5AOM4kqArjzTtiGLyw0dax84RUOA\n\tuziP858f5OqOGb2gS9GSC1dpOi0b40Cv4EXVGCXKyw5ZKArMc9gzGUcsYwroleuD7g\n\tFrk39M6IrMa2F8V7fG869hN/z6UulGtBkDjXnodOb5aS/zKfYmFSR+OwvIt5y9kLVI\n\tJNtErAYrRM8tw==","Message-ID":"<8eb7cc12-b7f5-43bf-856a-5c9d8e773ed9@kernel.org>","Date":"Mon, 29 Sep 2025 13:35:57 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v4 6/7] ipa: simple: blc: Prevent division by zero in BLC","To":"Milan Zamazal , libcamera-devel@lists.libcamera.org","Cc":"pobrn@protonmail.com, mail@maciej.szmigiero.name, =?utf-8?q?Barnab?=\n\t=?utf-8?b?w6FzIFDFkWN6ZQ==?= ","References":"<20250925192856.77881-1-mzamazal@redhat.com>\n\t<20250925192856.77881-7-mzamazal@redhat.com>","From":"Hans de Goede ","Content-Language":"en-US, nl","In-Reply-To":"<20250925192856.77881-7-mzamazal@redhat.com>","Content-Type":"text/plain; charset=UTF-8","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\" "}},{"id":36035,"web_url":"https://patchwork.libcamera.org/comment/36035/","msgid":"<91dbe0f0-3a72-4b56-a95a-c1927ea9002c@kernel.org>","date":"2025-09-29T11:36:35","subject":"Re: [PATCH v4 6/7] ipa: simple: blc: Prevent division by zero in BLC","submitter":{"id":239,"url":"https://patchwork.libcamera.org/api/people/239/","name":"Hans de Goede","email":"hansg@kernel.org"},"content":"Hi,\n\nOn 25-Sep-25 21:28, Milan Zamazal wrote:\n> When there are no values in the histogram, BLC simple IPA can crash on\n> division by zero. We cannot get anything meaningful in such a case\n> anyway, let's simply return from `process()' then.\n> \n> Tested-by: Barnabás Pőcze \n> Reviewed-by: Barnabás Pőcze \n> Reviewed-by: Maciej S. Szmigiero \n> Signed-off-by: Milan Zamazal \n\nForgot to mention that despite that this should never happen it still\nis a good fix to have:\n\nThanks, patch looks good to me:\n\nReviewed-by: Hans de Goede \n\nRegards,\n\nHans\n\n\n> ---\n> src/ipa/simple/algorithms/blc.cpp | 5 +++++\n> 1 file changed, 5 insertions(+)\n> \n> diff --git a/src/ipa/simple/algorithms/blc.cpp b/src/ipa/simple/algorithms/blc.cpp\n> index 8c1e9ed08..ea5356443 100644\n> --- a/src/ipa/simple/algorithms/blc.cpp\n> +++ b/src/ipa/simple/algorithms/blc.cpp\n> @@ -77,6 +77,11 @@ void BlackLevel::process(IPAContext &context,\n> \tconstexpr float ignoredPercentage = 0.02;\n> \tconst unsigned int total =\n> \t\tstd::accumulate(begin(histogram), end(histogram), 0);\n> +\tif (total == 0) {\n> +\t\tLOG(IPASoftBL, Debug) << \"Not guessing black level, histogram is empty\";\n> +\t\treturn;\n> +\t}\n> +\n> \tconst unsigned int pixelThreshold = ignoredPercentage * total;\n> \tconst unsigned int histogramRatio = 256 / SwIspStats::kYHistogramSize;\n> \tconst unsigned int currentBlackIdx =","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 B0889C328C\n\tfor ;\n\tMon, 29 Sep 2025 11:36:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 52D566B5FE;\n\tMon, 29 Sep 2025 13:36:42 +0200 (CEST)","from sea.source.kernel.org (sea.source.kernel.org [172.234.252.31])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 357576B5F3\n\tfor ;\n\tMon, 29 Sep 2025 13:36:40 +0200 (CEST)","from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58])\n\tby sea.source.kernel.org (Postfix) with ESMTP id 1D10C449F4;\n\tMon, 29 Sep 2025 11:36:39 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id C1EA2C4CEF4;\n\tMon, 29 Sep 2025 11:36:37 +0000 (UTC)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=kernel.org header.i=@kernel.org\n\theader.b=\"pRgFK1vH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org;\n\ts=k20201202; t=1759145799;\n\tbh=8gTObHidECa/b9yM+cOFDSpX+Cy7M6MNDgvXH9RQEBQ=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=pRgFK1vHvXkqFCcWV5wUuWzh4WAI1MxdSsN1j/urGBqtcxcjcmQ2EhjG56YOXIZut\n\twpSikJqMozrQukdeYJS9fvuDOvRQNVffN1rnyRuno0NLFsZDGCZbMLNj6gt6GbyLAx\n\t6X5nYByfenQWKQ4uSF5qeuBSJDq7uLdBlgps7DCt2k/qBjhTlkjAmI63GTuGsDhw9G\n\tkHHXQ/UaIW6AzaaA4BGfegV397wiwNeXyroXwlJdwrGa9QFLQsdSr/gkKJrHChXbtJ\n\tHsbKHa0lY9KT73+AcpLgyHB9aL8a55DqTMPJ9I8XnkvPoUAR2GFLutxqQRDFxIr+13\n\ttqkEiAyIAvL3g==","Message-ID":"<91dbe0f0-3a72-4b56-a95a-c1927ea9002c@kernel.org>","Date":"Mon, 29 Sep 2025 13:36:35 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v4 6/7] ipa: simple: blc: Prevent division by zero in BLC","To":"Milan Zamazal , libcamera-devel@lists.libcamera.org","Cc":"pobrn@protonmail.com, mail@maciej.szmigiero.name, =?utf-8?q?Barnab?=\n\t=?utf-8?b?w6FzIFDFkWN6ZQ==?= ","References":"<20250925192856.77881-1-mzamazal@redhat.com>\n\t<20250925192856.77881-7-mzamazal@redhat.com>","From":"Hans de Goede ","Content-Language":"en-US, nl","In-Reply-To":"<20250925192856.77881-7-mzamazal@redhat.com>","Content-Type":"text/plain; charset=UTF-8","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\" "}},{"id":36037,"web_url":"https://patchwork.libcamera.org/comment/36037/","msgid":"","date":"2025-09-29T11:40:53","subject":"Re: [PATCH v4 6/7] ipa: simple: blc: Prevent division by zero in BLC","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 09. 29. 13:35 keltezéssel, Hans de Goede írta:\n> Hi,\n> \n> On 25-Sep-25 21:28, Milan Zamazal wrote:\n>> When there are no values in the histogram, BLC simple IPA can crash on\n>> division by zero. We cannot get anything meaningful in such a case\n>> anyway, let's simply return from `process()' then.\n>>\n>> Tested-by: Barnabás Pőcze \n>> Reviewed-by: Barnabás Pőcze \n>> Reviewed-by: Maciej S. Szmigiero \n>> Signed-off-by: Milan Zamazal \n>> ---\n>> src/ipa/simple/algorithms/blc.cpp | 5 +++++\n>> 1 file changed, 5 insertions(+)\n>>\n>> diff --git a/src/ipa/simple/algorithms/blc.cpp b/src/ipa/simple/algorithms/blc.cpp\n>> index 8c1e9ed08..ea5356443 100644\n>> --- a/src/ipa/simple/algorithms/blc.cpp\n>> +++ b/src/ipa/simple/algorithms/blc.cpp\n>> @@ -77,6 +77,11 @@ void BlackLevel::process(IPAContext &context,\n>> \tconstexpr float ignoredPercentage = 0.02;\n>> \tconst unsigned int total =\n>> \t\tstd::accumulate(begin(histogram), end(histogram), 0);\n>> +\tif (total == 0) {\n>> +\t\tLOG(IPASoftBL, Debug) << \"Not guessing black level, histogram is empty\";\n>> +\t\treturn;\n>> +\t}\n>> +\n> t\n> I don't think this can ever happen ?\n\nIt can, if something else is buggy. At the moment if the output size is too small,\nthen the histogram will stay empty because the `y` coordinate passed to processLine*()\nis out of range. This should be easily reproducible with just `cam` and selecting the\nsmallest resolution.\n\n$ cam -c1 -s width=160,height=120 -C32\n[3:56:45.273196182] [8608] INFO IPAManager ipa_manager.cpp:147 libcamera is not installed. Adding '/libcamera/build/src/ipa' to the IPA search path\n[3:56:45.276523870] [8608] INFO Camera camera_manager.cpp:340 libcamera v0.5.2+102-24905025\n[...]\nUsing camera \\_SB_.PC00.LNK1 as cam0\n[3:56:45.335555691] [8608] INFO Camera camera.cpp:1215 configuring streams: (0) 160x120-ABGR8888/Unset\n[3:56:45.336489404] [8609] INFO IPASoft soft_simple.cpp:264 IPASoft: Exposure 4-1102, gain 1-15.4922 (0.144922)\ncam0: Capture 32 frames\n../src/ipa/simple/algorithms/blc.cpp:96:44: runtime error: division by zero\nAddressSanitizer:DEADLYSIGNAL\n=================================================================\n==8608==ERROR: AddressSanitizer: FPE on unknown address 0x7b3560b92e35 (pc 0x7b3560b92e35 bp 0x7b35607fd570 sp 0x7b35607fd240 T2)\n #0 0x7b3560b92e35 in libcamera::ipa::soft::algorithms::BlackLevel::process(libcamera::ipa::soft::IPAContext&, unsigned int, libcamera::ipa::soft::IPAFrameContext&, libcamera::SwIspStats const*, libcamera::ControlList&) ../src/ipa/simple/algorithms/blc.cpp:96\n [...]\n\n\nRegards,\nBarnabás Pőcze\n\n\n> \n> What I've seen happening which triggers a divide by 0 in the AGC code is\n> all samples being in the highest bin of the histogram, because the first\n> frame of the sensor sometimes seems to have all 0x[3]ff as output, at least\n> for the top 3/4th of the frame or something like that.\n> \n> Which causes a blacklevel of >= 244 which in turn causes a divide by 0\n> in the AGC code.\n> \n> Regards,\n> \n> Hans\n> \n> \n> \n>> \tconst unsigned int pixelThreshold = ignoredPercentage * total;\n>> \tconst unsigned int histogramRatio = 256 / SwIspStats::kYHistogramSize;\n>> \tconst unsigned int currentBlackIdx =\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 1B081C328C\n\tfor ;\n\tMon, 29 Sep 2025 11:40:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CBC536B5FB;\n\tMon, 29 Sep 2025 13:40:57 +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 4B73C6B599\n\tfor ;\n\tMon, 29 Sep 2025 13:40:56 +0200 (CEST)","from [192.168.33.13] (185.221.142.146.nat.pool.zt.hu\n\t[185.221.142.146])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B442BF6;\n\tMon, 29 Sep 2025 13:39:28 +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=\"iI+KdAIT\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1759145968;\n\tbh=tcHzyx2kmrmnmW0+BcGsBW0f9NOsFV2TzhbRodWUqvE=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=iI+KdAITG0FvOiCQMq2VNLYPAblukJUM6yjwI4KsRmZf2vxFTlr+HIATb0LjgxkxQ\n\tcsgzAJnF3lN2q/yuDjQVfRTcScapijMKHXx1osFF/xExoeww525ZH9/tY2A5wPnvsU\n\tkZQlMKLSVd2Ulxq76AttxyInV5a8EG2YRjqPnYvk=","Message-ID":"","Date":"Mon, 29 Sep 2025 13:40:53 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v4 6/7] ipa: simple: blc: Prevent division by zero in BLC","To":"Hans de Goede ","Cc":"mail@maciej.szmigiero.name, Milan Zamazal ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20250925192856.77881-1-mzamazal@redhat.com>\n\t<20250925192856.77881-7-mzamazal@redhat.com>\n\t<8eb7cc12-b7f5-43bf-856a-5c9d8e773ed9@kernel.org>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<8eb7cc12-b7f5-43bf-856a-5c9d8e773ed9@kernel.org>","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\" "}}]