[{"id":32100,"web_url":"https://patchwork.libcamera.org/comment/32100/","msgid":"","date":"2024-11-11T10:28:52","subject":"Re: [PATCH v3 01/11] ipa: helpers: Add Unsigned Q(m, n) helpers","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Dan\n\nOn Thu, Nov 07, 2024 at 11:48:09AM +0000, Daniel Scally wrote:\n> Add functions to convert a double to or from an unsigned Q(m,n)\n> format. As the format is unsigned this most resembles the UQ variant\n> of the TI implementation detailed on Wikipedia:\n>\n> https://en.wikipedia.org/wiki/Q_(number_format)\n>\n> Signed-off-by: Daniel Scally \n> ---\n> Changes in v3:\n>\n> \t- New patch\n>\n> src/ipa/libipa/helpers.cpp | 29 +++++++++++++++++++++++++++++\n> src/ipa/libipa/helpers.h | 2 ++\n> 2 files changed, 31 insertions(+)\n>\n> diff --git a/src/ipa/libipa/helpers.cpp b/src/ipa/libipa/helpers.cpp\n> index 6c038895..908c6e93 100644\n> --- a/src/ipa/libipa/helpers.cpp\n> +++ b/src/ipa/libipa/helpers.cpp\n> @@ -72,6 +72,35 @@ uint32_t estimateCCT(double red, double green, double blue)\n> \treturn 449 * n * n * n + 3525 * n * n + 6823.3 * n + 5520.33;\n> }\n>\n> +/**\n> + * \\brief Convert double to Q(m.n) format\n> + * \\param[in] value The value to convert\n\nI would mention you expect only positive values\n\n> + * \\param[in] m The number of bits used to represent the integer part\n> + * \\param[in] n The number of bits used to represent the fraction part\n> + *\n> + * This function converts a double into an unsigned fractional Q format,\n> + * clamping at the largest possible value given m and n. As these formats are\n> + * unsigned the m figure does not include a sign bit.\n> + */\n> +unsigned int toUnsignedQFormat(double value, unsigned int m, unsigned int n)\n\ndouble can be negative, and if a negative is provided it gets\nautomatically converted to 0. Should this be catch ? If this function\ndoes not support negative input values, I would rather catch this\nearlier even by using FATAL as this is programming \"error\" and\nshouldn't be triggered by user input ?\n\n> +{\n> +\tdouble maxVal = (std::pow(2, m + n) - 1) / std::pow(2, n);\n> +\n> +\treturn std::clamp(value, 0.0, maxVal) * std::pow(2, n);\n> +}\n> +\n> +/**\n> + * \\brief Convert Q(m.n) formatted number to double\n> + * \\param[in] value The value to convert\n> + * \\param[in] n The number of bits used to represent the fraction part\n> + *\n> + * This function converts an unsigned Q formatted value into a double.\n> + */\n> +double fromUnsignedQFormat(unsigned int value, unsigned int n)\n> +{\n> +\treturn value / std::pow(2, n);\n> +}\n> +\n> } /* namespace ipa */\n>\n> } /* namespace libcamera */\n> diff --git a/src/ipa/libipa/helpers.h b/src/ipa/libipa/helpers.h\n> index 51c74a36..1f26e768 100644\n> --- a/src/ipa/libipa/helpers.h\n> +++ b/src/ipa/libipa/helpers.h\n> @@ -15,6 +15,8 @@ namespace ipa {\n>\n> double rec601LuminanceFromRGB(unsigned int r, unsigned int g, unsigned int b);\n> uint32_t estimateCCT(double red, double green, double blue);\n> +unsigned int toUnsignedQFormat(double value, unsigned int m, unsigned int n);\n> +double fromUnsignedQFormat(unsigned int value, unsigned int n);\n>\n> } /* namespace ipa */\n>\n> --\n> 2.30.2\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 A44F2BF415\n\tfor ;\n\tMon, 11 Nov 2024 10:28:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A8519657C9;\n\tMon, 11 Nov 2024 11:28:58 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A335E60355\n\tfor ;\n\tMon, 11 Nov 2024 11:28:56 +0100 (CET)","from ideasonboard.com (mob-5-90-143-109.net.vodafone.it\n\t[5.90.143.109])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A8538788;\n\tMon, 11 Nov 2024 11:28:44 +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=\"ELjsZ2d5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1731320924;\n\tbh=OOC3kBzN4Y4cLuNUClltjZRqeDq8PlK/oNAQkp6yvzw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ELjsZ2d5xXpc0CXxx7kFkZoPws6UzC1JNBMmOL8h6jANAMO2xWX+sy9CVoUfrhJNI\n\tpi63qlYBmMtx0fTo0T7KzLai+Gj7TsAIv6XwjahfciDBvZTJ86VtG+Ma5URM9AKrha\n\tFjZHggh1TndtW2TzggDddSLn/ZVyMcpuGFVTn4ZM=","Date":"Mon, 11 Nov 2024 11:28:52 +0100","From":"Jacopo Mondi ","To":"Daniel Scally ","Cc":"libcamera-devel@lists.libcamera.org, Anthony.McGivern@arm.com","Subject":"Re: [PATCH v3 01/11] ipa: helpers: Add Unsigned Q(m, n) helpers","Message-ID":"","References":"<20241107114819.57599-1-dan.scally@ideasonboard.com>\n\t<20241107114819.57599-2-dan.scally@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241107114819.57599-2-dan.scally@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":32114,"web_url":"https://patchwork.libcamera.org/comment/32114/","msgid":"<20241112080056.GH17916@pendragon.ideasonboard.com>","date":"2024-11-12T08:00:56","subject":"Re: [PATCH v3 01/11] ipa: helpers: Add Unsigned Q(m, n) helpers","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Dan,\n\nThank you for the patch.\n\nOn Thu, Nov 07, 2024 at 11:48:09AM +0000, Daniel Scally wrote:\n> Add functions to convert a double to or from an unsigned Q(m,n)\n> format. As the format is unsigned this most resembles the UQ variant\n> of the TI implementation detailed on Wikipedia:\n> \n> https://en.wikipedia.org/wiki/Q_(number_format)\n\nTime to move src/ipa/rkisp1/utils.{h,cpp} to libipa ? I would then call\nit math.h, fixedpoint.h or something similar.\n\n> Signed-off-by: Daniel Scally \n> ---\n> Changes in v3:\n> \n> \t- New patch\n> \n> src/ipa/libipa/helpers.cpp | 29 +++++++++++++++++++++++++++++\n> src/ipa/libipa/helpers.h | 2 ++\n> 2 files changed, 31 insertions(+)\n> \n> diff --git a/src/ipa/libipa/helpers.cpp b/src/ipa/libipa/helpers.cpp\n> index 6c038895..908c6e93 100644\n> --- a/src/ipa/libipa/helpers.cpp\n> +++ b/src/ipa/libipa/helpers.cpp\n> @@ -72,6 +72,35 @@ uint32_t estimateCCT(double red, double green, double blue)\n> \treturn 449 * n * n * n + 3525 * n * n + 6823.3 * n + 5520.33;\n> }\n> \n> +/**\n> + * \\brief Convert double to Q(m.n) format\n> + * \\param[in] value The value to convert\n> + * \\param[in] m The number of bits used to represent the integer part\n> + * \\param[in] n The number of bits used to represent the fraction part\n> + *\n> + * This function converts a double into an unsigned fractional Q format,\n> + * clamping at the largest possible value given m and n. As these formats are\n> + * unsigned the m figure does not include a sign bit.\n> + */\n> +unsigned int toUnsignedQFormat(double value, unsigned int m, unsigned int n)\n> +{\n> +\tdouble maxVal = (std::pow(2, m + n) - 1) / std::pow(2, n);\n> +\n> +\treturn std::clamp(value, 0.0, maxVal) * std::pow(2, n);\n> +}\n> +\n> +/**\n> + * \\brief Convert Q(m.n) formatted number to double\n> + * \\param[in] value The value to convert\n> + * \\param[in] n The number of bits used to represent the fraction part\n> + *\n> + * This function converts an unsigned Q formatted value into a double.\n> + */\n> +double fromUnsignedQFormat(unsigned int value, unsigned int n)\n> +{\n> +\treturn value / std::pow(2, n);\n> +}\n> +\n> } /* namespace ipa */\n> \n> } /* namespace libcamera */\n> diff --git a/src/ipa/libipa/helpers.h b/src/ipa/libipa/helpers.h\n> index 51c74a36..1f26e768 100644\n> --- a/src/ipa/libipa/helpers.h\n> +++ b/src/ipa/libipa/helpers.h\n> @@ -15,6 +15,8 @@ namespace ipa {\n> \n> double rec601LuminanceFromRGB(unsigned int r, unsigned int g, unsigned int b);\n> uint32_t estimateCCT(double red, double green, double blue);\n> +unsigned int toUnsignedQFormat(double value, unsigned int m, unsigned int n);\n> +double fromUnsignedQFormat(unsigned int value, unsigned int n);\n> \n> } /* namespace ipa */\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 F27CEBE173\n\tfor ;\n\tTue, 12 Nov 2024 08:01:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B8455657E2;\n\tTue, 12 Nov 2024 09:01: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 A6EE265474\n\tfor ;\n\tTue, 12 Nov 2024 09:01:04 +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 D8152512;\n\tTue, 12 Nov 2024 09:00:51 +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=\"ciQxNj6j\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1731398452;\n\tbh=gUUN8+jaqp9yjrekgk6LUqwLFuqLY/PUmllnHjROwx4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ciQxNj6jBENuK6g1o5PzPD1hwaO+6W5ovw31oSGy+TVeS/Ubtr3Uo4sIv2klj4TcK\n\tmQa8plcW4tTIca6rLZGIs7uip5LpEJxF9KZz8fSpejd5/SJuiOhIzqErUFGI9NNJWW\n\thrtywKiCSHenvinA4syQiFVOx2+heRLcxFE/ttiE=","Date":"Tue, 12 Nov 2024 10:00:56 +0200","From":"Laurent Pinchart ","To":"Daniel Scally ","Cc":"libcamera-devel@lists.libcamera.org, Anthony.McGivern@arm.com","Subject":"Re: [PATCH v3 01/11] ipa: helpers: Add Unsigned Q(m, n) helpers","Message-ID":"<20241112080056.GH17916@pendragon.ideasonboard.com>","References":"<20241107114819.57599-1-dan.scally@ideasonboard.com>\n\t<20241107114819.57599-2-dan.scally@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241107114819.57599-2-dan.scally@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\" "}}]