[{"id":37673,"web_url":"https://patchwork.libcamera.org/comment/37673/","msgid":"<b2879a37-e1c5-4146-8304-726d85a8e944@ideasonboard.com>","date":"2026-01-15T11:26:48","subject":"Re: [PATCH v5 03/16] ipa: libipa: fixedpoint: Fix unsigned usage","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 01. 14. 18:39 keltezéssel, Kieran Bingham írta:\n> The fixedToFloatingPoint does not support unsigned Q types, and\n> incorrectly sign-extends all values which have the top most bit set in\n> the quantized values.\n> \n> Fix this by ensuring that only signed types perform sign extension, and\n> simplify the calculation for unsigned types.\n> \n> Convert the storage of the test cases to signed types to correctly\n> represent their intended purpose, to prevent test failures.\n> \n> Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>\n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n>   src/ipa/libipa/fixedpoint.h    | 3 +++\n>   test/ipa/libipa/fixedpoint.cpp | 6 +++---\n>   2 files changed, 6 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/ipa/libipa/fixedpoint.h b/src/ipa/libipa/fixedpoint.h\n> index 709cf50f0fcd..b4a7fa5e0ecd 100644\n> --- a/src/ipa/libipa/fixedpoint.h\n> +++ b/src/ipa/libipa/fixedpoint.h\n> @@ -49,6 +49,9 @@ constexpr R fixedToFloatingPoint(T number)\n>   \tstatic_assert(sizeof(int) >= sizeof(T));\n>   \tstatic_assert(I + F <= sizeof(T) * 8);\n>   \n> +\tif constexpr (std::is_unsigned_v<T>)\n> +\t\treturn static_cast<R>(number) / static_cast<R>(1 << F);\n> +\n>   \t/*\n>   \t * Recreate the upper bits in case of a negative number by shifting the sign\n>   \t * bit from the fixed point to the first bit of the unsigned and then right shifting\n\nThe above change looks ok. The tests seems to be removed later, so I am not commenting on those.\n\nReviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n\n\n> diff --git a/test/ipa/libipa/fixedpoint.cpp b/test/ipa/libipa/fixedpoint.cpp\n> index 99eb662ddf4e..4b017e86a74f 100644\n> --- a/test/ipa/libipa/fixedpoint.cpp\n> +++ b/test/ipa/libipa/fixedpoint.cpp\n> @@ -68,7 +68,7 @@ protected:\n>   \t\t * The second 7.992 test is to test that unused bits don't\n>   \t\t * affect the result.\n>   \t\t */\n> -\t\tstd::map<double, uint16_t> testCases = {\n> +\t\tstd::map<double, int16_t> testCases = {\n>   \t\t\t{ 7.992, 0x3ff },\n>   \t\t\t{   0.2, 0x01a },\n>   \t\t\t{  -0.2, 0x7e6 },\n> @@ -81,14 +81,14 @@ protected:\n>   \n>   \t\tint ret;\n>   \t\tfor (const auto &testCase : testCases) {\n> -\t\t\tret = testSingleFixedPoint<4, 7, uint16_t>(testCase.first,\n> +\t\t\tret = testSingleFixedPoint<4, 7, int16_t>(testCase.first,\n>   \t\t\t\t\t\t\t\t   testCase.second);\n>   \t\t\tif (ret != TestPass)\n>   \t\t\t\treturn ret;\n>   \t\t}\n>   \n>   \t\t/* Special case with a superfluous one in the unused bits */\n> -\t\tret = testFixedToFloat<4, 7, uint16_t, double>(0xbff, 7.992);\n> +\t\tret = testFixedToFloat<4, 7, int16_t, double>(0xbff, 7.992);\n>   \t\tif (ret != TestPass)\n>   \t\t\treturn ret;\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 35919C3226\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 15 Jan 2026 11:27:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 65DA561FC3;\n\tThu, 15 Jan 2026 12:27:29 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D896961F84\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 15 Jan 2026 12:27:27 +0100 (CET)","from [192.168.33.20] (185.221.143.114.nat.pool.zt.hu\n\t[185.221.143.114])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 356154D3;\n\tThu, 15 Jan 2026 12:27:00 +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=\"L7xBPzV3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768476420;\n\tbh=VocNhMg7gqmNtZmnHrlhgI67eApvcBtlI+2aJPpeksg=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=L7xBPzV3jmzPF0UNAaNQqEXHMmJhrRC54HsNdbWf52pUrMPDDLtWrBjuqneQM4IeZ\n\t7yF1rgJpIoxxC1JUXIiwaI8QV99pL+K9ydYpgKSuLwSXNfodA1fr72KkgTW7Yn9WDc\n\tUgwhFMgSQsSc8cMIS66t1Zo2c8kYXRbs5Pn9qDIE=","Message-ID":"<b2879a37-e1c5-4146-8304-726d85a8e944@ideasonboard.com>","Date":"Thu, 15 Jan 2026 12:26:48 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v5 03/16] ipa: libipa: fixedpoint: Fix unsigned usage","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tlibcamera devel <libcamera-devel@lists.libcamera.org>","Cc":"Isaac Scott <isaac.scott@ideasonboard.com>","References":"<20260114173918.1744023-1-kieran.bingham@ideasonboard.com>\n\t<20260114173918.1744023-4-kieran.bingham@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<20260114173918.1744023-4-kieran.bingham@ideasonboard.com>","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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":37674,"web_url":"https://patchwork.libcamera.org/comment/37674/","msgid":"<d73af4f2-fb8d-43a4-9384-9f804aad1ff6@ideasonboard.com>","date":"2026-01-15T11:30:08","subject":"Re: [PATCH v5 03/16] ipa: libipa: fixedpoint: Fix unsigned usage","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 01. 15. 12:26 keltezéssel, Barnabás Pőcze írta:\n> 2026. 01. 14. 18:39 keltezéssel, Kieran Bingham írta:\n>> The fixedToFloatingPoint does not support unsigned Q types, and\n>> incorrectly sign-extends all values which have the top most bit set in\n>> the quantized values.\n>>\n>> Fix this by ensuring that only signed types perform sign extension, and\n>> simplify the calculation for unsigned types.\n>>\n>> Convert the storage of the test cases to signed types to correctly\n>> represent their intended purpose, to prevent test failures.\n>>\n>> Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>\n>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>> ---\n>>   src/ipa/libipa/fixedpoint.h    | 3 +++\n>>   test/ipa/libipa/fixedpoint.cpp | 6 +++---\n>>   2 files changed, 6 insertions(+), 3 deletions(-)\n>>\n>> diff --git a/src/ipa/libipa/fixedpoint.h b/src/ipa/libipa/fixedpoint.h\n>> index 709cf50f0fcd..b4a7fa5e0ecd 100644\n>> --- a/src/ipa/libipa/fixedpoint.h\n>> +++ b/src/ipa/libipa/fixedpoint.h\n>> @@ -49,6 +49,9 @@ constexpr R fixedToFloatingPoint(T number)\n>>       static_assert(sizeof(int) >= sizeof(T));\n>>       static_assert(I + F <= sizeof(T) * 8);\n>> +    if constexpr (std::is_unsigned_v<T>)\n>> +        return static_cast<R>(number) / static_cast<R>(1 << F);\n>> +\n>>       /*\n>>        * Recreate the upper bits in case of a negative number by shifting the sign\n>>        * bit from the fixed point to the first bit of the unsigned and then right shifting\n> \n> The above change looks ok. The tests seems to be removed later, so I am not commenting on those.\n\nActually, sorry. I think we want to write `static_cast<R>(T(1) << F)`,\nso that there are no problems with using `int` later (e.g. if 64-bit things\nare supported, etc).\n\n\n> \n> Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n> \n> \n>> diff --git a/test/ipa/libipa/fixedpoint.cpp b/test/ipa/libipa/fixedpoint.cpp\n>> index 99eb662ddf4e..4b017e86a74f 100644\n>> --- a/test/ipa/libipa/fixedpoint.cpp\n>> +++ b/test/ipa/libipa/fixedpoint.cpp\n>> @@ -68,7 +68,7 @@ protected:\n>>            * The second 7.992 test is to test that unused bits don't\n>>            * affect the result.\n>>            */\n>> -        std::map<double, uint16_t> testCases = {\n>> +        std::map<double, int16_t> testCases = {\n>>               { 7.992, 0x3ff },\n>>               {   0.2, 0x01a },\n>>               {  -0.2, 0x7e6 },\n>> @@ -81,14 +81,14 @@ protected:\n>>           int ret;\n>>           for (const auto &testCase : testCases) {\n>> -            ret = testSingleFixedPoint<4, 7, uint16_t>(testCase.first,\n>> +            ret = testSingleFixedPoint<4, 7, int16_t>(testCase.first,\n>>                                      testCase.second);\n>>               if (ret != TestPass)\n>>                   return ret;\n>>           }\n>>           /* Special case with a superfluous one in the unused bits */\n>> -        ret = testFixedToFloat<4, 7, uint16_t, double>(0xbff, 7.992);\n>> +        ret = testFixedToFloat<4, 7, int16_t, double>(0xbff, 7.992);\n>>           if (ret != TestPass)\n>>               return ret;\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 2737DC3226\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 15 Jan 2026 11:30:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6772C61FA3;\n\tThu, 15 Jan 2026 12:30:14 +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 470BF61F84\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 15 Jan 2026 12:30:12 +0100 (CET)","from [192.168.33.20] (185.221.143.114.nat.pool.zt.hu\n\t[185.221.143.114])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A3EB34D3;\n\tThu, 15 Jan 2026 12:29: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=\"Bn/HI0yF\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768476584;\n\tbh=xx2lMAFfXRqHXhR4TjJ55OMkg5U0teQgIx4Y/x+JaM8=;\n\th=Date:Subject:From:To:Cc:References:In-Reply-To:From;\n\tb=Bn/HI0yF9fktcDQkHR9zf6c85Lkd+4NjcTFuVtWTh4EwGIGUgM2CxmtU+Blm831qc\n\t1kBi4PVho9NPsuCpyQuWnjm75FukX5PN4Y2wEdNmnAWr0/2LViXEQ5/vE3Pi8cmVXB\n\tUAYCXMUcTsO0p4NtOPcq0V821siIlAWVU19GB41A=","Message-ID":"<d73af4f2-fb8d-43a4-9384-9f804aad1ff6@ideasonboard.com>","Date":"Thu, 15 Jan 2026 12:30:08 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v5 03/16] ipa: libipa: fixedpoint: Fix unsigned usage","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tlibcamera devel <libcamera-devel@lists.libcamera.org>","Cc":"Isaac Scott <isaac.scott@ideasonboard.com>","References":"<20260114173918.1744023-1-kieran.bingham@ideasonboard.com>\n\t<20260114173918.1744023-4-kieran.bingham@ideasonboard.com>\n\t<b2879a37-e1c5-4146-8304-726d85a8e944@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<b2879a37-e1c5-4146-8304-726d85a8e944@ideasonboard.com>","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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":37743,"web_url":"https://patchwork.libcamera.org/comment/37743/","msgid":"<176884306478.1693075.43781917090383747@ping.linuxembedded.co.uk>","date":"2026-01-19T17:17:44","subject":"Re: [PATCH v5 03/16] ipa: libipa: fixedpoint: Fix unsigned usage","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2026-01-15 11:30:08)\n> 2026. 01. 15. 12:26 keltezéssel, Barnabás Pőcze írta:\n> > 2026. 01. 14. 18:39 keltezéssel, Kieran Bingham írta:\n> >> The fixedToFloatingPoint does not support unsigned Q types, and\n> >> incorrectly sign-extends all values which have the top most bit set in\n> >> the quantized values.\n> >>\n> >> Fix this by ensuring that only signed types perform sign extension, and\n> >> simplify the calculation for unsigned types.\n> >>\n> >> Convert the storage of the test cases to signed types to correctly\n> >> represent their intended purpose, to prevent test failures.\n> >>\n> >> Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>\n> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> >> ---\n> >>   src/ipa/libipa/fixedpoint.h    | 3 +++\n> >>   test/ipa/libipa/fixedpoint.cpp | 6 +++---\n> >>   2 files changed, 6 insertions(+), 3 deletions(-)\n> >>\n> >> diff --git a/src/ipa/libipa/fixedpoint.h b/src/ipa/libipa/fixedpoint.h\n> >> index 709cf50f0fcd..b4a7fa5e0ecd 100644\n> >> --- a/src/ipa/libipa/fixedpoint.h\n> >> +++ b/src/ipa/libipa/fixedpoint.h\n> >> @@ -49,6 +49,9 @@ constexpr R fixedToFloatingPoint(T number)\n> >>       static_assert(sizeof(int) >= sizeof(T));\n> >>       static_assert(I + F <= sizeof(T) * 8);\n> >> +    if constexpr (std::is_unsigned_v<T>)\n> >> +        return static_cast<R>(number) / static_cast<R>(1 << F);\n> >> +\n> >>       /*\n> >>        * Recreate the upper bits in case of a negative number by shifting the sign\n> >>        * bit from the fixed point to the first bit of the unsigned and then right shifting\n> > \n> > The above change looks ok. The tests seems to be removed later, so I am not commenting on those.\n> \n> Actually, sorry. I think we want to write `static_cast<R>(T(1) << F)`,\n> so that there are no problems with using `int` later (e.g. if 64-bit things\n> are supported, etc).\n\nI think this should mean UT{1} right ?\n\n> > Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n\nThanks\n\n> > \n> > \n> >> diff --git a/test/ipa/libipa/fixedpoint.cpp b/test/ipa/libipa/fixedpoint.cpp\n> >> index 99eb662ddf4e..4b017e86a74f 100644\n> >> --- a/test/ipa/libipa/fixedpoint.cpp\n> >> +++ b/test/ipa/libipa/fixedpoint.cpp\n> >> @@ -68,7 +68,7 @@ protected:\n> >>            * The second 7.992 test is to test that unused bits don't\n> >>            * affect the result.\n> >>            */\n> >> -        std::map<double, uint16_t> testCases = {\n> >> +        std::map<double, int16_t> testCases = {\n> >>               { 7.992, 0x3ff },\n> >>               {   0.2, 0x01a },\n> >>               {  -0.2, 0x7e6 },\n> >> @@ -81,14 +81,14 @@ protected:\n> >>           int ret;\n> >>           for (const auto &testCase : testCases) {\n> >> -            ret = testSingleFixedPoint<4, 7, uint16_t>(testCase.first,\n> >> +            ret = testSingleFixedPoint<4, 7, int16_t>(testCase.first,\n> >>                                      testCase.second);\n> >>               if (ret != TestPass)\n> >>                   return ret;\n> >>           }\n> >>           /* Special case with a superfluous one in the unused bits */\n> >> -        ret = testFixedToFloat<4, 7, uint16_t, double>(0xbff, 7.992);\n> >> +        ret = testFixedToFloat<4, 7, int16_t, double>(0xbff, 7.992);\n> >>           if (ret != TestPass)\n> >>               return ret;\n> > \n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 1CA7FC3220\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 19 Jan 2026 17:17:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 314E861FB9;\n\tMon, 19 Jan 2026 18:17:48 +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 825B6606D5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 19 Jan 2026 18:17:47 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AA89FC1;\n\tMon, 19 Jan 2026 18:17:16 +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=\"v/8vaWr+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768843036;\n\tbh=64JzBOonN8kBoHGGipQaoyarksHgGxZc00wdxTuAP80=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=v/8vaWr+2veAN8O6XjQBjvluP1KTaqM06FTy3C59AR2JV3PdgmebeQqnm8TnvgjNH\n\thpXyBS3a5941OWaVDj72lOdDyMMTrBNWnLSTcIXdrZ7144RcHzMAvGRYoaKTPNffnW\n\tZvIPSziCby92lsxvLPOhvUjNvJGmXZfgwONvFc2A=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<d73af4f2-fb8d-43a4-9384-9f804aad1ff6@ideasonboard.com>","References":"<20260114173918.1744023-1-kieran.bingham@ideasonboard.com>\n\t<20260114173918.1744023-4-kieran.bingham@ideasonboard.com>\n\t<b2879a37-e1c5-4146-8304-726d85a8e944@ideasonboard.com>\n\t<d73af4f2-fb8d-43a4-9384-9f804aad1ff6@ideasonboard.com>","Subject":"Re: [PATCH v5 03/16] ipa: libipa: fixedpoint: Fix unsigned usage","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Isaac Scott <isaac.scott@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>,\n\tlibcamera devel <libcamera-devel@lists.libcamera.org>","Date":"Mon, 19 Jan 2026 17:17:44 +0000","Message-ID":"<176884306478.1693075.43781917090383747@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":37745,"web_url":"https://patchwork.libcamera.org/comment/37745/","msgid":"<176884431268.1693075.2345904219708220563@ping.linuxembedded.co.uk>","date":"2026-01-19T17:38:32","subject":"Re: [PATCH v5 03/16] ipa: libipa: fixedpoint: Fix unsigned usage","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Kieran Bingham (2026-01-19 17:17:44)\n> Quoting Barnabás Pőcze (2026-01-15 11:30:08)\n> > 2026. 01. 15. 12:26 keltezéssel, Barnabás Pőcze írta:\n> > > 2026. 01. 14. 18:39 keltezéssel, Kieran Bingham írta:\n> > >> The fixedToFloatingPoint does not support unsigned Q types, and\n> > >> incorrectly sign-extends all values which have the top most bit set in\n> > >> the quantized values.\n> > >>\n> > >> Fix this by ensuring that only signed types perform sign extension, and\n> > >> simplify the calculation for unsigned types.\n> > >>\n> > >> Convert the storage of the test cases to signed types to correctly\n> > >> represent their intended purpose, to prevent test failures.\n> > >>\n> > >> Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>\n> > >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > >> ---\n> > >>   src/ipa/libipa/fixedpoint.h    | 3 +++\n> > >>   test/ipa/libipa/fixedpoint.cpp | 6 +++---\n> > >>   2 files changed, 6 insertions(+), 3 deletions(-)\n> > >>\n> > >> diff --git a/src/ipa/libipa/fixedpoint.h b/src/ipa/libipa/fixedpoint.h\n> > >> index 709cf50f0fcd..b4a7fa5e0ecd 100644\n> > >> --- a/src/ipa/libipa/fixedpoint.h\n> > >> +++ b/src/ipa/libipa/fixedpoint.h\n> > >> @@ -49,6 +49,9 @@ constexpr R fixedToFloatingPoint(T number)\n> > >>       static_assert(sizeof(int) >= sizeof(T));\n> > >>       static_assert(I + F <= sizeof(T) * 8);\n> > >> +    if constexpr (std::is_unsigned_v<T>)\n> > >> +        return static_cast<R>(number) / static_cast<R>(1 << F);\n> > >> +\n> > >>       /*\n> > >>        * Recreate the upper bits in case of a negative number by shifting the sign\n> > >>        * bit from the fixed point to the first bit of the unsigned and then right shifting\n> > > \n> > > The above change looks ok. The tests seems to be removed later, so I am not commenting on those.\n> > \n> > Actually, sorry. I think we want to write `static_cast<R>(T(1) << F)`,\n> > so that there are no problems with using `int` later (e.g. if 64-bit things\n> > are supported, etc).\n> \n> I think this should mean UT{1} right ?\n\naha, this is before the code moves and before I bring in UT - so it has\nto be T{}.\n\n> \n> > > Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n> \n> Thanks\n> \n> > > \n> > > \n> > >> diff --git a/test/ipa/libipa/fixedpoint.cpp b/test/ipa/libipa/fixedpoint.cpp\n> > >> index 99eb662ddf4e..4b017e86a74f 100644\n> > >> --- a/test/ipa/libipa/fixedpoint.cpp\n> > >> +++ b/test/ipa/libipa/fixedpoint.cpp\n> > >> @@ -68,7 +68,7 @@ protected:\n> > >>            * The second 7.992 test is to test that unused bits don't\n> > >>            * affect the result.\n> > >>            */\n> > >> -        std::map<double, uint16_t> testCases = {\n> > >> +        std::map<double, int16_t> testCases = {\n> > >>               { 7.992, 0x3ff },\n> > >>               {   0.2, 0x01a },\n> > >>               {  -0.2, 0x7e6 },\n> > >> @@ -81,14 +81,14 @@ protected:\n> > >>           int ret;\n> > >>           for (const auto &testCase : testCases) {\n> > >> -            ret = testSingleFixedPoint<4, 7, uint16_t>(testCase.first,\n> > >> +            ret = testSingleFixedPoint<4, 7, int16_t>(testCase.first,\n> > >>                                      testCase.second);\n> > >>               if (ret != TestPass)\n> > >>                   return ret;\n> > >>           }\n> > >>           /* Special case with a superfluous one in the unused bits */\n> > >> -        ret = testFixedToFloat<4, 7, uint16_t, double>(0xbff, 7.992);\n> > >> +        ret = testFixedToFloat<4, 7, int16_t, double>(0xbff, 7.992);\n> > >>           if (ret != TestPass)\n> > >>               return ret;\n> > > \n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 9ABB1C3220\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 19 Jan 2026 17:38:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D898C61FBF;\n\tMon, 19 Jan 2026 18:38:36 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D5428606D5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 19 Jan 2026 18:38:35 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1DFE0C1;\n\tMon, 19 Jan 2026 18:38:05 +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=\"f3SNJSn6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768844285;\n\tbh=2hBLn8zUgiLrIOdLjzj/MH9Jqg/fDMZ7kE6c/vnrGCE=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=f3SNJSn6IAnQv8eyj0/k77kANZe6tXV0J8KIyVlXVe2GQbZdj/sHOU4t+5BV0ZAw5\n\thy54qBWCuowo/Rcteg26VcIgVLFPym7/NjbRJG0fCKVnNYORT1kqSxTt/Hi+yGD9A+\n\tPi07lnZ3cQEwhjr+CdCBoOzllLYASNjLw1+/3+b0=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<176884306478.1693075.43781917090383747@ping.linuxembedded.co.uk>","References":"<20260114173918.1744023-1-kieran.bingham@ideasonboard.com>\n\t<20260114173918.1744023-4-kieran.bingham@ideasonboard.com>\n\t<b2879a37-e1c5-4146-8304-726d85a8e944@ideasonboard.com>\n\t<d73af4f2-fb8d-43a4-9384-9f804aad1ff6@ideasonboard.com>\n\t<176884306478.1693075.43781917090383747@ping.linuxembedded.co.uk>","Subject":"Re: [PATCH v5 03/16] ipa: libipa: fixedpoint: Fix unsigned usage","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Isaac Scott <isaac.scott@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>,\n\tlibcamera devel <libcamera-devel@lists.libcamera.org>","Date":"Mon, 19 Jan 2026 17:38:32 +0000","Message-ID":"<176884431268.1693075.2345904219708220563@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]