[{"id":34442,"web_url":"https://patchwork.libcamera.org/comment/34442/","msgid":"","date":"2025-06-10T16:39:46","subject":"Re: [RFC PATCH v1 06/23] libcamera: base: Add alignment utility\n\tfunctions","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Fri, Jun 06, 2025 at 06:41:39PM +0200, Barnabás Pőcze wrote:\n> Add a couple internal functions to check alignment, and to\n> align integers, pointer up to a given alignment.\n>\n> Signed-off-by: Barnabás Pőcze \n> ---\n> include/libcamera/base/details/align.h | 72 ++++++++++++++++++++++++++\n> 1 file changed, 72 insertions(+)\n> create mode 100644 include/libcamera/base/details/align.h\n>\n> diff --git a/include/libcamera/base/details/align.h b/include/libcamera/base/details/align.h\n> new file mode 100644\n> index 000000000..e1c24c6e1\n> --- /dev/null\n> +++ b/include/libcamera/base/details/align.h\n\nSame question about the additional details/ subfolder\n\n> @@ -0,0 +1,72 @@\n> +#pragma once\n> +\n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +\n> +namespace libcamera::details::align {\n> +\n> +template\n> +bool is(T *p, std::uintptr_t alignment)\n\nBikeshedding on names\n\nis\n details::align:is(p, q);\n\nbetter than a longer name such as is_aligned() ?\n\nWhy have you used uintptr_t to express an alignement ?\n\n> +{\n> +\tassert(alignment > 0);\n> +\n> +\treturn reinterpret_cast(p) % alignment == 0;\n\nThis won't work with alignement as an unsigned int, right ?\n\n> +}\n> +\n> +template\n> +constexpr T up(T x, cxx20::type_identity_t alignment)\n\nI'm a bit confused. The is() function has\n\n is(T *)\n\nwhile this one has\n\n up(T)\n\nso I presume we will have\n\n is(p, q)\n up(p, q)\n\n?\n\n> +{\n> +\tstatic_assert(std::is_unsigned_v);\n> +\tassert(alignment > 0);\n> +\n> +\tconst auto padding = (alignment - (x % alignment)) % alignment;\n> +\tassert(x + padding >= x);\n> +\n> +\treturn x + padding;\n> +}\n> +\n> +template\n> +auto *up(T *p, std::uintptr_t alignment)\n> +{\n> +\tusing U = std::conditional_t<\n> +\t\tstd::is_const_v,\n> +\t\tconst std::byte,\n> +\t\tstd::byte\n> +\t>;\n> +\n> +\treturn reinterpret_cast(up(reinterpret_cast(p), alignment));\n> +}\n> +\n> +template\n> +T *up(std::size_t size, std::size_t alignment, T *&ptr, std::size_t *avail = nullptr)\n> +{\n> +\tassert(alignment > 0);\n> +\n> +\tauto p = reinterpret_cast(ptr);\n> +\tconst auto padding = (alignment - (p % alignment)) % alignment;\n> +\n> +\tif (avail) {\n> +\t\tif (size > *avail || padding > *avail - size)\n> +\t\t\treturn nullptr;\n> +\n> +\t\t*avail -= size + padding;\n> +\t}\n> +\n> +\tp += padding;\n> +\tptr = reinterpret_cast(p + size);\n> +\n> +\treturn reinterpret_cast(p);\n> +}\n> +\n> +template\n> +U *up(T *&ptr, std::size_t *avail = nullptr)\n> +{\n> +\treturn reinterpret_cast, const U, U> *>(\n> +\t\tup(sizeof(U), alignof(U), ptr, avail)\n> +\t);\n> +}\n> +\n> +} /* namespace libcamera::details::align */\n> --\n> 2.49.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 AEB42C3237\n\tfor ;\n\tTue, 10 Jun 2025 16:39:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 82F6468DBD;\n\tTue, 10 Jun 2025 18:39:51 +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 C8E7A61869\n\tfor ;\n\tTue, 10 Jun 2025 18:39:49 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 40DA1982;\n\tTue, 10 Jun 2025 18:39:42 +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=\"M81A2X6Q\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1749573582;\n\tbh=LBD9oQYCyr+vq1vOnv9jsKcyY1udy0YkrwFdOyfH1bA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=M81A2X6QUza4Rl+KfWuo9aGNKPtGICLQ1iMvenzeckz0ZY2CkTuSlst97NJkDuniw\n\tUKQx3cyIIpOnh6suc0m0fQnZSQavL/BYrCAjZAAhvNnPNSkvbEtflXKMkNP/vpR9xN\n\t+Re/Bp6fD2IQy1KVDwYB8pfCT8S9h6nq+NwwNsjM=","Date":"Tue, 10 Jun 2025 18:39:46 +0200","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v1 06/23] libcamera: base: Add alignment utility\n\tfunctions","Message-ID":"","References":"<20250606164156.1442682-1-barnabas.pocze@ideasonboard.com>\n\t<20250606164156.1442682-7-barnabas.pocze@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20250606164156.1442682-7-barnabas.pocze@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":34447,"web_url":"https://patchwork.libcamera.org/comment/34447/","msgid":"<568ad26a-5e4a-4878-8ba4-66ced6b7e7e1@ideasonboard.com>","date":"2025-06-11T12:37:44","subject":"Re: [RFC PATCH v1 06/23] libcamera: base: Add alignment utility\n\tfunctions","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. 06. 10. 18:39 keltezéssel, Jacopo Mondi írta:\n> Hi Barnabás\n> \n> On Fri, Jun 06, 2025 at 06:41:39PM +0200, Barnabás Pőcze wrote:\n>> Add a couple internal functions to check alignment, and to\n>> align integers, pointer up to a given alignment.\n>>\n>> Signed-off-by: Barnabás Pőcze \n>> ---\n>> include/libcamera/base/details/align.h | 72 ++++++++++++++++++++++++++\n>> 1 file changed, 72 insertions(+)\n>> create mode 100644 include/libcamera/base/details/align.h\n>>\n>> diff --git a/include/libcamera/base/details/align.h b/include/libcamera/base/details/align.h\n>> new file mode 100644\n>> index 000000000..e1c24c6e1\n>> --- /dev/null\n>> +++ b/include/libcamera/base/details/align.h\n> \n> Same question about the additional details/ subfolder\n> \n>> @@ -0,0 +1,72 @@\n>> +#pragma once\n>> +\n>> +#include \n>> +#include \n>> +#include \n>> +\n>> +#include \n>> +\n>> +namespace libcamera::details::align {\n>> +\n>> +template\n>> +bool is(T *p, std::uintptr_t alignment)\n> \n> Bikeshedding on names\n> \n> is\n> details::align:is(p, q);\n> \n> better than a longer name such as is_aligned() ?\n\nI don't know, I am open to suggestions. I chose these names because\nthey are in the `align` namespace.\n\n\n> \n> Why have you used uintptr_t to express an alignement ?\n\nSince the pointer is cast to `uintptr_t`, that seemed like the obvious choice.\nI think any unsigned type should do.\n\n\n> \n>> +{\n>> +\tassert(alignment > 0);\n>> +\n>> +\treturn reinterpret_cast(p) % alignment == 0;\n> \n> This won't work with alignement as an unsigned int, right ?\n\nIt should.\n\n\n> \n>> +}\n>> +\n>> +template\n>> +constexpr T up(T x, cxx20::type_identity_t alignment)\n> \n> I'm a bit confused. The is() function has\n> \n> is(T *)\n> \n> while this one has\n> \n> up(T)\n> \n> so I presume we will have\n> \n> is(p, q)\n> up(p, q)\n\nI don't expect anyone to specify the template parameters explicitly.\nBut in any case, I have modified `is()` not to be a template but instead\naccept `const void *`.\n\n\nRegards,\nBarnabás Pőcze\n\n> \n> ?\n> \n>> +{\n>> +\tstatic_assert(std::is_unsigned_v);\n>> +\tassert(alignment > 0);\n>> +\n>> +\tconst auto padding = (alignment - (x % alignment)) % alignment;\n>> +\tassert(x + padding >= x);\n>> +\n>> +\treturn x + padding;\n>> +}\n>> +\n>> +template\n>> +auto *up(T *p, std::uintptr_t alignment)\n>> +{\n>> +\tusing U = std::conditional_t<\n>> +\t\tstd::is_const_v,\n>> +\t\tconst std::byte,\n>> +\t\tstd::byte\n>> +\t>;\n>> +\n>> +\treturn reinterpret_cast(up(reinterpret_cast(p), alignment));\n>> +}\n>> +\n>> +template\n>> +T *up(std::size_t size, std::size_t alignment, T *&ptr, std::size_t *avail = nullptr)\n>> +{\n>> +\tassert(alignment > 0);\n>> +\n>> +\tauto p = reinterpret_cast(ptr);\n>> +\tconst auto padding = (alignment - (p % alignment)) % alignment;\n>> +\n>> +\tif (avail) {\n>> +\t\tif (size > *avail || padding > *avail - size)\n>> +\t\t\treturn nullptr;\n>> +\n>> +\t\t*avail -= size + padding;\n>> +\t}\n>> +\n>> +\tp += padding;\n>> +\tptr = reinterpret_cast(p + size);\n>> +\n>> +\treturn reinterpret_cast(p);\n>> +}\n>> +\n>> +template\n>> +U *up(T *&ptr, std::size_t *avail = nullptr)\n>> +{\n>> +\treturn reinterpret_cast, const U, U> *>(\n>> +\t\tup(sizeof(U), alignof(U), ptr, avail)\n>> +\t);\n>> +}\n>> +\n>> +} /* namespace libcamera::details::align */\n>> --\n>> 2.49.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 55E24C3237\n\tfor ;\n\tWed, 11 Jun 2025 12:37:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5E0B868DC0;\n\tWed, 11 Jun 2025 14:37:49 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 57E6E61552\n\tfor ;\n\tWed, 11 Jun 2025 14:37:48 +0200 (CEST)","from [192.168.33.24] (185.221.142.30.nat.pool.zt.hu\n\t[185.221.142.30])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 34ABB352;\n\tWed, 11 Jun 2025 14:37:40 +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=\"YclRh4YW\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1749645460;\n\tbh=Fdaficlh2dRHZdt/rRldiBf5rAO3zDbLVxpsqgAnjzw=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=YclRh4YWvK76NaVu2ha230LoR3I1xN6wL+K1eczX2b/uckD7vYr8PqBTTVsFsxhhd\n\tvf437n1pNyEIXqI7pFP/Zm5kXxUwxtvCsGehFCrq6aktIxsyPG1M4PGjK3TxfTneUa\n\tvpfDsI6sEJCNY1BPXtkiixmU/J9xhn2tzrFYDhk0=","Message-ID":"<568ad26a-5e4a-4878-8ba4-66ced6b7e7e1@ideasonboard.com>","Date":"Wed, 11 Jun 2025 14:37:44 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v1 06/23] libcamera: base: Add alignment utility\n\tfunctions","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20250606164156.1442682-1-barnabas.pocze@ideasonboard.com>\n\t<20250606164156.1442682-7-barnabas.pocze@ideasonboard.com>\n\t","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"","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\" "}}]