[RFC,v2,06/22] libcamera: base: Add alignment utility functions
diff mbox series

Message ID 20250721104622.1550908-7-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • libcamera: Add `MetadataList`
Related show

Commit Message

Barnabás Pőcze July 21, 2025, 10:46 a.m. UTC 
Add a couple internal functions to check alignment, and to
align integers, pointer up to a given alignment.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 include/libcamera/base/details/align.h | 78 ++++++++++++++++++++++++++
 include/libcamera/base/meson.build     |  1 +
 2 files changed, 79 insertions(+)
 create mode 100644 include/libcamera/base/details/align.h

Comments

Jacopo Mondi July 25, 2025, 1:35 p.m. UTC | #1 
Hi Barnabás

On Mon, Jul 21, 2025 at 12:46:06PM +0200, Barnabás Pőcze wrote:
> Add a couple internal functions to check alignment, and to
> align integers, pointer up to a given alignment.
>
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
>  include/libcamera/base/details/align.h | 78 ++++++++++++++++++++++++++
>  include/libcamera/base/meson.build     |  1 +

Just as a reminder, we will need documentation as well :)

>  2 files changed, 79 insertions(+)
>  create mode 100644 include/libcamera/base/details/align.h
>
> diff --git a/include/libcamera/base/details/align.h b/include/libcamera/base/details/align.h
> new file mode 100644
> index 000000000..e9569613b
> --- /dev/null
> +++ b/include/libcamera/base/details/align.h
> @@ -0,0 +1,78 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2025, Ideas on Board Oy
> + *
> + * Alignment utilities
> + */
> +
> +#pragma once
> +
> +#include <cassert>
> +#include <cstddef>
> +#include <cstdint>
> +
> +#include <libcamera/base/details/cxx20.h>
> +
> +namespace libcamera::details::align {
> +
> +inline bool is(const void *p, std::uintptr_t alignment)
> +{
> +	assert(alignment > 0);
> +
> +	return reinterpret_cast<std::uintptr_t>(p) % alignment == 0;
> +}
> +
> +template<typename T>
> +constexpr T up(T x, cxx20::type_identity_t<T> alignment)
> +{
> +	static_assert(std::is_unsigned_v<T>);
> +	assert(alignment > 0);
> +
> +	const auto padding = (alignment - (x % alignment)) % alignment;
> +	assert(x + padding >= x);
> +
> +	return x + padding;
> +}
> +
> +template<typename T>
> +auto *up(T *p, std::uintptr_t alignment)
> +{
> +	using U = std::conditional_t<
> +		std::is_const_v<T>,
> +		const std::byte,
> +		std::byte
> +	>;
> +
> +	return reinterpret_cast<U *>(up(reinterpret_cast<std::uintptr_t>(p), alignment));
> +}
> +
> +template<typename T>
> +T *up(std::size_t size, std::size_t alignment, T *&ptr, std::size_t *avail = nullptr)
> +{
> +	assert(alignment > 0);
> +
> +	auto p = reinterpret_cast<std::uintptr_t>(ptr);
> +	const auto padding = (alignment - (p % alignment)) % alignment;
> +
> +	if (avail) {
> +		if (size > *avail || padding > *avail - size)
> +			return nullptr;
> +
> +		*avail -= size + padding;
> +	}
> +
> +	p += padding;
> +	ptr = reinterpret_cast<T *>(p + size);
> +
> +	return reinterpret_cast<T *>(p);
> +}
> +
> +template<typename U, typename T>
> +U *up(T *&ptr, std::size_t *avail = nullptr)
> +{
> +	return reinterpret_cast<std::conditional_t<std::is_const_v<T>, const U, U> *>(
> +		up(sizeof(U), alignof(U), ptr, avail)
> +	);
> +}

I'm curious to see this how this will look like when used.
Do you use all these overloads in the rest of the series ?

> +
> +} /* namespace libcamera::details::align */
> diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build
> index 9836daff9..1554b996b 100644
> --- a/include/libcamera/base/meson.build
> +++ b/include/libcamera/base/meson.build
> @@ -33,6 +33,7 @@ libcamera_base_private_headers = files([
>
>  libcamera_base_details_headers = files([
>      'details/cxx20.h',
> +    'details/align.h',
>  ])
>
>  libcamera_base_headers = [
> --
> 2.50.1
>
Barnabás Pőcze July 30, 2025, 2:48 p.m. UTC | #2 
Hi

2025. 07. 25. 15:35 keltezéssel, Jacopo Mondi írta:
> Hi Barnabás
> 
> On Mon, Jul 21, 2025 at 12:46:06PM +0200, Barnabás Pőcze wrote:
>> Add a couple internal functions to check alignment, and to
>> align integers, pointer up to a given alignment.
>>
>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
>> ---
>>   include/libcamera/base/details/align.h | 78 ++++++++++++++++++++++++++
>>   include/libcamera/base/meson.build     |  1 +
> 
> Just as a reminder, we will need documentation as well :)

Things in a `details` namespace are ignored by doxygen;
I could still add some documentation in the header files.


> 
>>   2 files changed, 79 insertions(+)
>>   create mode 100644 include/libcamera/base/details/align.h
>>
>> diff --git a/include/libcamera/base/details/align.h b/include/libcamera/base/details/align.h
>> new file mode 100644
>> index 000000000..e9569613b
>> --- /dev/null
>> +++ b/include/libcamera/base/details/align.h
>> @@ -0,0 +1,78 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2025, Ideas on Board Oy
>> + *
>> + * Alignment utilities
>> + */
>> +
>> +#pragma once
>> +
>> +#include <cassert>
>> +#include <cstddef>
>> +#include <cstdint>
>> +
>> +#include <libcamera/base/details/cxx20.h>
>> +
>> +namespace libcamera::details::align {
>> +
>> +inline bool is(const void *p, std::uintptr_t alignment)
>> +{
>> +	assert(alignment > 0);
>> +
>> +	return reinterpret_cast<std::uintptr_t>(p) % alignment == 0;
>> +}
>> +
>> +template<typename T>
>> +constexpr T up(T x, cxx20::type_identity_t<T> alignment)
>> +{
>> +	static_assert(std::is_unsigned_v<T>);
>> +	assert(alignment > 0);
>> +
>> +	const auto padding = (alignment - (x % alignment)) % alignment;
>> +	assert(x + padding >= x);
>> +
>> +	return x + padding;
>> +}
>> +
>> +template<typename T>
>> +auto *up(T *p, std::uintptr_t alignment)
>> +{
>> +	using U = std::conditional_t<
>> +		std::is_const_v<T>,
>> +		const std::byte,
>> +		std::byte
>> +	>;
>> +
>> +	return reinterpret_cast<U *>(up(reinterpret_cast<std::uintptr_t>(p), alignment));
>> +}
>> +
>> +template<typename T>
>> +T *up(std::size_t size, std::size_t alignment, T *&ptr, std::size_t *avail = nullptr)
>> +{
>> +	assert(alignment > 0);
>> +
>> +	auto p = reinterpret_cast<std::uintptr_t>(ptr);
>> +	const auto padding = (alignment - (p % alignment)) % alignment;
>> +
>> +	if (avail) {
>> +		if (size > *avail || padding > *avail - size)
>> +			return nullptr;
>> +
>> +		*avail -= size + padding;
>> +	}
>> +
>> +	p += padding;
>> +	ptr = reinterpret_cast<T *>(p + size);
>> +
>> +	return reinterpret_cast<T *>(p);
>> +}
>> +
>> +template<typename U, typename T>
>> +U *up(T *&ptr, std::size_t *avail = nullptr)
>> +{
>> +	return reinterpret_cast<std::conditional_t<std::is_const_v<T>, const U, U> *>(
>> +		up(sizeof(U), alignof(U), ptr, avail)
>> +	);
>> +}
> 
> I'm curious to see this how this will look like when used.
> Do you use all these overloads in the rest of the series ?

All of these should be used somewhere.


std::byte *buf = ...;
std::size_t avail = ...;

auto *p = details::align::up<SomeType>(buf, &avail); // allocate suitable aligned and sized storage
auto *x = new (p) SomeType { ... }; // construct object in said storage


Regards,
Barnabás Pőcze

> 
>> +
>> +} /* namespace libcamera::details::align */
>> diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build
>> index 9836daff9..1554b996b 100644
>> --- a/include/libcamera/base/meson.build
>> +++ b/include/libcamera/base/meson.build
>> @@ -33,6 +33,7 @@ libcamera_base_private_headers = files([
>>
>>   libcamera_base_details_headers = files([
>>       'details/cxx20.h',
>> +    'details/align.h',
>>   ])
>>
>>   libcamera_base_headers = [
>> --
>> 2.50.1
>>

Patch
diff mbox series 

diff --git a/include/libcamera/base/details/align.h b/include/libcamera/base/details/align.h
new file mode 100644
index 000000000..e9569613b
--- /dev/null
+++ b/include/libcamera/base/details/align.h
@@ -0,0 +1,78 @@ 
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2025, Ideas on Board Oy
+ *
+ * Alignment utilities
+ */
+
+#pragma once
+
+#include <cassert>
+#include <cstddef>
+#include <cstdint>
+
+#include <libcamera/base/details/cxx20.h>
+
+namespace libcamera::details::align {
+
+inline bool is(const void *p, std::uintptr_t alignment)
+{
+	assert(alignment > 0);
+
+	return reinterpret_cast<std::uintptr_t>(p) % alignment == 0;
+}
+
+template<typename T>
+constexpr T up(T x, cxx20::type_identity_t<T> alignment)
+{
+	static_assert(std::is_unsigned_v<T>);
+	assert(alignment > 0);
+
+	const auto padding = (alignment - (x % alignment)) % alignment;
+	assert(x + padding >= x);
+
+	return x + padding;
+}
+
+template<typename T>
+auto *up(T *p, std::uintptr_t alignment)
+{
+	using U = std::conditional_t<
+		std::is_const_v<T>,
+		const std::byte,
+		std::byte
+	>;
+
+	return reinterpret_cast<U *>(up(reinterpret_cast<std::uintptr_t>(p), alignment));
+}
+
+template<typename T>
+T *up(std::size_t size, std::size_t alignment, T *&ptr, std::size_t *avail = nullptr)
+{
+	assert(alignment > 0);
+
+	auto p = reinterpret_cast<std::uintptr_t>(ptr);
+	const auto padding = (alignment - (p % alignment)) % alignment;
+
+	if (avail) {
+		if (size > *avail || padding > *avail - size)
+			return nullptr;
+
+		*avail -= size + padding;
+	}
+
+	p += padding;
+	ptr = reinterpret_cast<T *>(p + size);
+
+	return reinterpret_cast<T *>(p);
+}
+
+template<typename U, typename T>
+U *up(T *&ptr, std::size_t *avail = nullptr)
+{
+	return reinterpret_cast<std::conditional_t<std::is_const_v<T>, const U, U> *>(
+		up(sizeof(U), alignof(U), ptr, avail)
+	);
+}
+
+} /* namespace libcamera::details::align */
diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build
index 9836daff9..1554b996b 100644
--- a/include/libcamera/base/meson.build
+++ b/include/libcamera/base/meson.build
@@ -33,6 +33,7 @@  libcamera_base_private_headers = files([
 
 libcamera_base_details_headers = files([
     'details/cxx20.h',
+    'details/align.h',
 ])
 
 libcamera_base_headers = [