[{"id":26539,"web_url":"https://patchwork.libcamera.org/comment/26539/","msgid":"<167778007830.93391.1683014294955790958@Monstersaurus>","date":"2023-03-02T18:01:18","subject":"Re: [libcamera-devel] [PATCH] ipa: rkisp1: lsc: Fix integer\n\tdivision error","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jacopo Mondi via libcamera-devel (2023-03-02 15:38:52)\n> The RkISP1 implementation of the LensShadinCorrection algorithm has been\n> made adaptive to the scene color temperature in commit 14c869c00fdd (\"ipa:\n> rkisp1: Take into account color temperature during LSC algorithm\").\n> \n> The LSC algorithm interpolates the correction factors using the\n> table's reference color temperatures. When calculating the interpolation\n> coefficients, an unintended integer division makes both coefficient\n> zeros resulting in a completely black image.\n> \n> Fix this by type casting to double one of the division operands.\n> \n\nThis seems reasonable to me.\n\n\nReviewed-by: Kieran Bingham \n\n> Fixes: 14c869c00fdd (\"ipa: rkisp1: Take into account color temperature during LSC algorithm\")\n> Signed-off-by: Jacopo Mondi \n> ---\n> src/ipa/rkisp1/algorithms/lsc.cpp | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> index 3a443e776a53..a7ccedb1ed3b 100644\n> --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> @@ -216,8 +216,8 @@ void LensShadingCorrection::interpolateTable(rkisp1_cif_isp_lsc_config &config,\n> const Components &set1,\n> const uint32_t ct)\n> {\n> - double coeff0 = (set1.ct - ct) / (set1.ct - set0.ct);\n> - double coeff1 = (ct - set0.ct) / (set1.ct - set0.ct);\n> + double coeff0 = (set1.ct - ct) / static_cast(set1.ct - set0.ct);\n> + double coeff1 = (ct - set0.ct) / static_cast(set1.ct - set0.ct);\n> \n> for (unsigned int i = 0; i < RKISP1_CIF_ISP_LSC_SAMPLES_MAX; ++i) {\n> for (unsigned int j = 0; j < RKISP1_CIF_ISP_LSC_SAMPLES_MAX; ++j) {\n> --\n> 2.39.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 CDB87BE080\n\tfor ;\n\tThu, 2 Mar 2023 18:01:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 510B56269C;\n\tThu, 2 Mar 2023 19:01:23 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BE3D86267E\n\tfor ;\n\tThu, 2 Mar 2023 19:01:21 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3756F735;\n\tThu, 2 Mar 2023 19:01:21 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1677780083;\n\tbh=06R1UzTZcSKx/uYYBeK4TeTQbHIYst6DbfPb2dnmvOA=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=IrYlZiY2k+QWS7ixk3Agkx/tgZ6aGw0JAxPCqDJCy0DfVm8QXSw6tN0jACy7gYRcr\n\tZpk8Ds/wQ+8rIijTLviM0rXKkiZ2GiY2PbWe6u6gYmc9XEh3uEio7xGtJvDQFKz5hN\n\tx7s+FkBafAshGFYc/JgZlEPwCSM3vp/YcSbMzgifpVDc8YoEnWd1mtJH9i36UJM4Gx\n\tCbQBagwjApO3vjyp0QzHdAQk7/l4aQCONSvgwteg0zmiZJ7nAFpwWF0pXBR+7Vca1v\n\tyqYAU8d+fJik100Zb8Zy49UhiO4hW3cOexbPyt3MYJvQROf+HOeaRSUs4MVaUXfkMk\n\tRRomnrtAKsE7A==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1677780081;\n\tbh=06R1UzTZcSKx/uYYBeK4TeTQbHIYst6DbfPb2dnmvOA=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=FVT9Q3ejqwthlFja+HlrGVPTHS2W79f3E7dim9VGQc8IF7HShzwazUl3pxDX0BhbN\n\treYVhLFTVxwNBHXitJ3Gbr1YM5RRXMFcRHl0oYbXP+vrvcZdcqwy1aRBeCTji8pjL2\n\t349o5bmpJr2O8saU90igG6SnEeJWI9AH+IDeIinM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"FVT9Q3ej\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20230302153852.63619-1-jacopo.mondi@ideasonboard.com>","References":"<20230302153852.63619-1-jacopo.mondi@ideasonboard.com>","To":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 02 Mar 2023 18:01:18 +0000","Message-ID":"<167778007830.93391.1683014294955790958@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH] ipa: rkisp1: lsc: Fix integer\n\tdivision error","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"Jacopo Mondi ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26540,"web_url":"https://patchwork.libcamera.org/comment/26540/","msgid":"","date":"2023-03-03T05:57:55","subject":"Re: [libcamera-devel] [PATCH] ipa: rkisp1: lsc: Fix integer\n\tdivision error","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Thu, Mar 02, 2023 at 04:38:52PM +0100, Jacopo Mondi via libcamera-devel wrote:\n> The RkISP1 implementation of the LensShadinCorrection algorithm has been\n\ns/Shadin/Shading/\n\n> made adaptive to the scene color temperature in commit 14c869c00fdd (\"ipa:\n> rkisp1: Take into account color temperature during LSC algorithm\").\n> \n> The LSC algorithm interpolates the correction factors using the\n> table's reference color temperatures. When calculating the interpolation\n> coefficients, an unintended integer division makes both coefficient\n> zeros resulting in a completely black image.\n> \n> Fix this by type casting to double one of the division operands.\n> \n> Fixes: 14c869c00fdd (\"ipa: rkisp1: Take into account color temperature during LSC algorithm\")\n> Signed-off-by: Jacopo Mondi \n\nReviewed-by: Paul Elder \n\n> ---\n> src/ipa/rkisp1/algorithms/lsc.cpp | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> index 3a443e776a53..a7ccedb1ed3b 100644\n> --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> @@ -216,8 +216,8 @@ void LensShadingCorrection::interpolateTable(rkisp1_cif_isp_lsc_config &config,\n> \t\t\t\t\t const Components &set1,\n> \t\t\t\t\t const uint32_t ct)\n> {\n> -\tdouble coeff0 = (set1.ct - ct) / (set1.ct - set0.ct);\n> -\tdouble coeff1 = (ct - set0.ct) / (set1.ct - set0.ct);\n> +\tdouble coeff0 = (set1.ct - ct) / static_cast(set1.ct - set0.ct);\n> +\tdouble coeff1 = (ct - set0.ct) / static_cast(set1.ct - set0.ct);\n> \n> \tfor (unsigned int i = 0; i < RKISP1_CIF_ISP_LSC_SAMPLES_MAX; ++i) {\n> \t\tfor (unsigned int j = 0; j < RKISP1_CIF_ISP_LSC_SAMPLES_MAX; ++j) {\n> --\n> 2.39.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 B576BBF415\n\tfor ;\n\tFri, 3 Mar 2023 05:58:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D13CF626E6;\n\tFri, 3 Mar 2023 06:58:05 +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 8FA7162664\n\tfor ;\n\tFri, 3 Mar 2023 06:58:03 +0100 (CET)","from pyrite.rasen.tech (h175-177-042-159.catv02.itscom.jp\n\t[175.177.42.159])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E3F33739;\n\tFri, 3 Mar 2023 06:58:01 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1677823085;\n\tbh=b9pgzQ5espZKTtG9Pm7e7RG9ze2+IvqsoOHapZDHGS0=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=mrKoQ9wEUbjNNE5RQzIv+k3I7m/8DGFjN+NfRfKEmjAjEWQRjFgsonkSwVtT0WbjT\n\t6kM8sVjP3XrPrtLCMEdtmE5fUb9CYD5WqGv8nZnneRovc44o7DNRkPrq5Ez0lytyN0\n\t8P+oUv37vcIDgP2cQEtqlDXHSCX2rj6TZd9iQjz5ouyPJEjZywPYd3KyCa7UbowN9Y\n\t8fKIPhS/+XxDhVRmdUej8jKgN/cYFqEf3jfvPptyoRErS3OGrR9w+InJy8WiGr6Id3\n\toDnT9mbFGPTu3bzPqDPYnTk0VxSfSJPUvKVBelfHdczEXJHWLwJJOwwvRSBlzFL6Lb\n\t5crUFeN1AAb9A==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1677823083;\n\tbh=b9pgzQ5espZKTtG9Pm7e7RG9ze2+IvqsoOHapZDHGS0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=RTpRjzETeYw2QBFzZ7DpkfR6gmwHqHybE4nesNXiFm/2toFV6Vm5nhTtCg6P6/vBv\n\tL24HypWGGvWnLGObOJJg3UDU0wJ0Jr6zHf0Igb6jgcPcRd2/zJ3HOGQcZq0vS8EtXJ\n\tYrIz7oK84kqHi0d+yWq3deLni/EC50454Ty5/AJA="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"RTpRjzET\"; dkim-atps=neutral","Date":"Fri, 3 Mar 2023 14:57:55 +0900","To":"Jacopo Mondi ","Message-ID":"","References":"<20230302153852.63619-1-jacopo.mondi@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20230302153852.63619-1-jacopo.mondi@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] ipa: rkisp1: lsc: Fix integer\n\tdivision error","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","From":"Paul Elder via libcamera-devel ","Reply-To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]