[{"id":37700,"web_url":"https://patchwork.libcamera.org/comment/37700/","msgid":"<aWj-Kfp1WbpeTpIY@zed>","date":"2026-01-16T15:56:35","subject":"Re: [PATCH v4 07/22] libcamera: Add `MetadataList`","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 Tue, Jan 06, 2026 at 05:57:39PM +0100, Barnabás Pőcze wrote:\n> Add a dedicated `MetadataList` type, whose purpose is to store the metadata\n> reported by a camera for a given request. Previously, a `ControlList` was\n> used for this purpose. The reason for introducing a separate type is to\n> simplify the access to the returned metadata during the entire lifetime\n> of a request.\n>\n> Specifically, for early metadata completion to be easily usable it should be\n> guaranteed that any completed metadata item can be accessed and looked up\n> at least until the associated requested is reused with `Request::reuse()`.\n\ns/requested/request\n\n>\n> However, when a metadata item is completed early, the pipeline handler\n> might still work on the request in the `CameraManager`'s private thread,\n> therefore there is an inherent synchronization issue when an application\n> accesses early metadata.\n>\n> Restricting the user to only access the metadata items of a not yet completed\n> request in the early metadata availability signal handler by ways of\n> documenting or enforcing it at runtime could be an option, but it is not\n> too convenient for the user.\n>\n> The current `ControlList` implementation employs an `std::unordered_map`,\n> so pointers remain stable when the container is modified, so an application\n> could keep accessing particular metadata items outside the signal handler,\n> but this fact is far from obvious, and the user would still not be able\n> to make a copy of all metadata or do lookups based on the numeric ids or\n> the usual `libcamera::Control<>` objects, thus some type safety is lost.\n>\n> The above also requires that each metadata item is only completed once for\n> a given request, but this does not appear to be serious limitation,\n> and in fact, this restriction is enforced by `MetadataList`.\n>\n> The introduced `MetadataList` supports single writer - multiple reader\n> scenarios, and it can be set, looked-up, and copied in a wait-free fashion\n> without introducing data races or other synchronization issues. This is\n> achieved by requiring the possible set of metadata items to be known\n> (such set is stored in a `MetadataListPlan` object). Based on the this\n\ns/the this/the\n\n> plan, a single contiguous allocation is made to accommodate all potential\n> metadata items. Due to this single contiguous allocation that is not modified\n> during the lifetime of a `MetadataList` and atomic modifications, it is\n> possible to easily gaurantee thread-safe set, lookup, and copy; assuming\n\ns/copy;/copy\n\n> there is only ever a single writer.\n>\n> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n> ---\n> changes in v3:\n>   * make `merge()` transactional\n>   * move certain functions out of the header\n>   * documentation adjustments\n>\n> changes in v2:\n>   * remove multiple not strictly necessary functions\n> ---\n>  include/libcamera/meson.build            |   2 +\n>  include/libcamera/metadata_list.h        | 529 ++++++++++++++++++++\n>  include/libcamera/metadata_list_plan.h   | 110 +++++\n>  src/libcamera/meson.build                |   1 +\n>  src/libcamera/metadata_list.cpp          | 594 +++++++++++++++++++++++\n>  test/controls/meson.build                |  12 +-\n>  test/controls/metadata_list.cpp          | 205 ++++++++\n>  test/controls/metadata_list_iter_uaf.cpp |  36 ++\n>  8 files changed, 1488 insertions(+), 1 deletion(-)\n>  create mode 100644 include/libcamera/metadata_list.h\n>  create mode 100644 include/libcamera/metadata_list_plan.h\n>  create mode 100644 src/libcamera/metadata_list.cpp\n>  create mode 100644 test/controls/metadata_list.cpp\n>  create mode 100644 test/controls/metadata_list_iter_uaf.cpp\n>\n> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> index 30ea76f94..410b548dd 100644\n> --- a/include/libcamera/meson.build\n> +++ b/include/libcamera/meson.build\n> @@ -12,6 +12,8 @@ libcamera_public_headers = files([\n>      'framebuffer_allocator.h',\n>      'geometry.h',\n>      'logging.h',\n> +    'metadata_list.h',\n> +    'metadata_list_plan.h',\n>      'orientation.h',\n>      'pixel_format.h',\n>      'request.h',\n> diff --git a/include/libcamera/metadata_list.h b/include/libcamera/metadata_list.h\n> new file mode 100644\n> index 000000000..9366dcbc3\n> --- /dev/null\n> +++ b/include/libcamera/metadata_list.h\n> @@ -0,0 +1,529 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025, Ideas On Board Oy\n> + *\n> + * Metadata list\n> + */\n> +\n> +#pragma once\n> +\n> +#include <algorithm>\n> +#include <atomic>\n> +#include <cassert>\n> +#include <cstdint>\n> +#include <optional>\n> +#include <type_traits>\n> +#include <utility>\n> +\n> +#include <libcamera/base/internal/align.h>\n> +#include <libcamera/base/internal/cxx20.h>\n> +#include <libcamera/base/span.h>\n> +\n> +#include <libcamera/controls.h>\n> +\n> +namespace libcamera {\n> +\n> +class MetadataListPlan;\n> +\n> +class MetadataList\n> +{\n> +private:\n> +\t/**\n> +\t * \\brief The entry corresponding to a potential value in the list\n\nI'm fine with documenting private types and members with doxygen\nkeywords even if they're not parsed by doxygen, but we can omit the\n/** I think\n\nAlso, can we expand a little ?\n\n        /*\n         * \\brief The entry corresponding to a potential value in the list\n         *\n         * An Entry is created for each metadata item that can\n         * potentially be addded to the MetadataList as specified by\n         * the MetadataListPlan.\n         *\n         * Entry describes both the static information for a metadata\n         * item and identifies if the metadata item has been added to\n         * the list or not by initializing the headerOffset field with\n         * the byte offset of the ValueHeader that corresponds to the\n         * Entry.\n         */\n\n ?\n\n> +\t */\n> +\tstruct Entry {\n> +\t\tstatic constexpr uint32_t kInvalidOffset = -1;\n> +\n> +\t\t/**\n> +\t\t * \\brief Numeric identifier in the list\n\nThis is the numeric control id, isn't it ?\n\n> +\t\t */\n> +\t\tconst uint32_t tag;\n> +\n> +\t\t/**\n> +\t\t * \\brief Number of bytes available for the value\n\ns/value/metadata value\nI would also s/available/allocated\n\n> +\t\t */\n> +\t\tconst uint32_t capacity;\n> +\n> +\t\t/**\n> +\t\t * \\brief Alignment of the value\n\ns/value/metadata value\n\n> +\t\t */\n> +\t\tconst uint32_t alignment;\n> +\n> +\t\tconst ControlType type;\n> +\t\tconst bool isArray;\n> +\n> +\t\t/**\n> +\t\t * \\brief Offset of the ValueHeader of the value pertaining to this entry\n\nCould be easily be made 80 cols since the comment is already a\nmulti-line block\n\n\n> +\t\t *\n> +\t\t * Offset from the beginning of the allocation, and\n> +\t\t * and _not_ relative to `contentOffset_`.\n\ns/_not_/not\n\n> +\t\t */\n> +\t\tstd::atomic_uint32_t headerOffset = kInvalidOffset;\n\nIsn't this the valueOffset ? In other words, this is the location with\nthe [ValueHeader + value data] from the beginning of the allocation.\n\nLet's try to use 'value' for whatever is on the second part of the\nlist.\n\n> +\n> +\t\t[[nodiscard]]\n> +\t\tstd::optional<uint32_t> hasValue() const\n\nCan this be removed and open coded in 'acquireData()' ?\n\nThe only usage I see is\n\nMetadataList::set(const Entry &e, ControlValueView v, State &s)\n{\n\tif (e.hasValue())\n\t\treturn { SetError::AlreadySet, {} };\n\nWhich could be reimplemented as:\n\nMetadataList::set(const Entry &e, ControlValueView v, State &s)\n{\n\tif (e.acquireData())\n\t\treturn { SetError::AlreadySet, {} };\n\n> +\t\t{\n> +\t\t\tauto offset = headerOffset.load(std::memory_order_relaxed);\n> +\t\t\tif (offset == kInvalidOffset)\n> +\t\t\t\treturn {};\n> +\n> +\t\t\treturn offset;\n> +\t\t}\n> +\n> +\t\t[[nodiscard]]\n> +\t\tstd::optional<uint32_t> acquireData() const\n\nThis returns the location of the ValueHeader, and I would\ncall this valueOffset()\n\nThis function is only used in two places\n\n\t\tControlValueView get(uint32_t tag) const\n\t\t{\n\t\t\tauto o = e->acquireData();\n\t\t\tif (!o)\n\t\t\t\treturn {};\n                        return list_->dataOf(*o);\n                 }\n\n\n                ControlValueView dataOf(const Entry &e) const\n                {\n                        const auto o = e.acquireData();\n                        return o ? dataOf(*o) : ControlValueView{ };\n                }\n\nThey would both read better as\n\n\t\tControlValueView get(uint32_t tag) const\n\t\t{\n\t\t\tauto o = e->valueOffset();\n\t\t\tif (!o)\n\t\t\t\treturn {};\n                        return list_->dataOf(*o);\n                 }\n\n                ControlValueView dataOf(const Entry &e) const\n                {\n                        const auto o = e.valueOffset();\n                        return o ? dataOf(*o) : ControlValueView{ };\n                }\n\nThe usage of the word \"acquire\" suggests some form of ownership, while\nwe're simply returning an offset here\n\n\n> +\t\t{\n> +\t\t\tauto offset = hasValue();\n> +\t\t\tif (offset) {\n> +\t\t\t\t/* sync with release-store on `headerOffset` in `MetadataList::set()` */\n> +\t\t\t\tstd::atomic_thread_fence(std::memory_order_acquire);\n> +\t\t\t}\n> +\n> +\t\t\treturn offset;\n> +\t\t}\n> +\t};\n> +\n> +\t/**\n> +\t * \\brief The header describing a value in the list\n> +\t */\n\nSame comments as the above documentation for Entry.\n\n\t/*\n\t * \\brief The header describing a metadata value\n         *\n         * A ValueHeader is created once a metadata item is added to\n         * the list. It describes static information about a metadata\n         * item such as the expected size, type and number of\n         * elements.\n         *\n         * Each ValueHeader entry is followed by the actual metadata\n         * value in the serialized buffer format.\n\t */\n\n> +\tstruct ValueHeader {\n> +\t\t/**\n> +\t\t * \\brief Numeric identifier of the value in the list\n\nAgain, I think this is the control id\n\n                /* \\brief The numeric control id */\n\n> +\t\t */\n> +\t\tuint32_t tag;\n> +\n> +\t\t/**\n> +\t\t * \\brief Number of bytes used by the value\n\nby the metadata item ?\n\n> +\t\t *\n> +\t\t * This can be calculated using type and numElements, it is stored\n> +\t\t * here to facilitate easier iteration in the buffer.\n> +\t\t */\n> +\t\tuint32_t size;\n> +\n> +\t\t/**\n> +\t\t * \\brief Alignment of the value\n> +\t\t */\n\nAll of these comments fits on one line\n\n> +\t\tuint32_t alignment;\n> +\n> +\t\t/**\n> +\t\t * \\brief Type of the value\n> +\t\t */\n> +\t\tControlType type;\n> +\n> +\t\t/**\n> +\t\t * \\brief Whether the value is an array\n> +\t\t */\n> +\t\tbool isArray;\n> +\n> +\t\t/**\n> +\t\t * \\brief Number of elements in the value\n> +\t\t */\n> +\t\tuint32_t numElements;\n> +\t};\n> +\n> +\tstruct State {\n> +\t\t/**\n> +\t\t * \\brief Number of items present in the list\n> +\t\t */\n> +\t\tuint32_t count;\n> +\n> +\t\t/**\n> +\t\t * \\brief Number of bytes used in the buffer\n\nDoes this include the Entry items (first part of the list)\nor is this only for the values (second part) ?\n\n> +\t\t */\n> +\t\tuint32_t fill;\n> +\t};\n> +\n> +public:\n> +\texplicit MetadataList(const MetadataListPlan &plan);\n\nDo we want applications to be able to construct metadata lists ?\nShould the constructor be made private and only accessible to the Request\nclass ?\n\n> +\n> +\tMetadataList(const MetadataList &) = delete;\n> +\tMetadataList(MetadataList &&) = delete;\n> +\n> +\tMetadataList &operator=(const MetadataList &) = delete;\n> +\tMetadataList &operator=(MetadataList &&) = delete;\n> +\n> +\t~MetadataList();\n> +\n> +\t// \\todo want these?\n\nI guess so, at least they seems to be a reasonable part of the public\ninterface of a MetadataList.\n\nPlease also remove other todos similar to this one\n\n> +\t[[nodiscard]] std::size_t size() const { return state_.load(std::memory_order_relaxed).count; }\n> +\t[[nodiscard]] bool empty() const { return state_.load(std::memory_order_relaxed).fill == 0; }\n\nCan easily be made 80 ols\n\n> +\n> +\tenum class SetError {\n> +\t\tUnknownTag = 1,\n> +\t\tAlreadySet,\n> +\t\tSizeMismatch,\n> +\t\tTypeMismatch,\n> +\t};\n> +\n> +\t[[nodiscard]]\n> +\tSetError set(uint32_t tag, ControlValueView v)\n> +\t{\n> +\t\tauto *e = find(tag);\n> +\t\tif (!e)\n> +\t\t\treturn SetError::UnknownTag;\n> +\n> +\t\treturn set(*e, v);\n> +\t}\n> +\n> +\ttemplate<typename T>\n> +\t[[nodiscard]]\n> +\tSetError set(const Control<T> &ctrl, const internal::cxx20::type_identity_t<T> &value)\n\nAs discussed on the C++20 introduction, if you can remove\ntype_identity_t<> from the public interface, we can probably get rid\nof the cpp20 polyfill.\n\n> +\t{\n> +\t\tusing TypeInfo = libcamera::details::control_type<T>;\n> +\n> +\t\tif constexpr (TypeInfo::size > 0) {\n\nI guess this is where it becomes problematic if std::strings have a\nsize == 0 in control_type<std::string> ?\n\n> +\t\t\tstatic_assert(std::is_trivially_copyable_v<typename T::value_type>);\n> +\n> +\t\t\treturn set(ctrl.id(), {\n> +\t\t\t\tTypeInfo::value,\n> +\t\t\t\ttrue,\n> +\t\t\t\tvalue.size(),\n> +\t\t\t\treinterpret_cast<const std::byte *>(value.data()),\n> +\t\t\t});\n> +\t\t} else {\n> +\t\t\tstatic_assert(std::is_trivially_copyable_v<T>);\n> +\n> +\t\t\treturn set(ctrl.id(), {\n> +\t\t\t\tTypeInfo::value,\n> +\t\t\t\tfalse,\n> +\t\t\t\t1,\n> +\t\t\t\treinterpret_cast<const std::byte *>(&value),\n> +\t\t\t});\n> +\t\t}\n> +\t}\n> +\n> +\ttemplate<typename T>\n> +\t[[nodiscard]]\n> +\tstd::optional<T> get(const Control<T> &ctrl) const\n> +\t{\n> +\t\tControlValueView v = get(ctrl.id());\n> +\t\tif (!v)\n> +\t\t\treturn {};\n> +\n> +\t\treturn v.get<T>();\n> +\t}\n> +\n> +\t// \\todo operator ControlListView() const ?\n> +\t// \\todo explicit operator ControlList() const ?\n\nFor the future maybe :)\n\n> +\n> +\t[[nodiscard]]\n> +\tControlValueView get(uint32_t tag) const\n> +\t{\n> +\t\tconst auto *e = find(tag);\n> +\t\tif (!e)\n> +\t\t\treturn {};\n> +\n> +\t\treturn dataOf(*e);\n> +\t}\n> +\n> +\tvoid clear();\n> +\n> +\tclass iterator\n> +\t{\n> +\tpublic:\n> +\t\tusing difference_type = std::ptrdiff_t;\n> +\t\tusing value_type = std::pair<uint32_t, ControlValueView>;\n> +\t\tusing pointer = void;\n> +\t\tusing reference = value_type;\n> +\t\tusing iterator_category = std::forward_iterator_tag;\n> +\n> +\t\titerator() = default;\n> +\n> +\t\titerator& operator++()\n> +\t\t{\n> +\t\t\tconst auto &h = header();\n> +\n> +\t\t\tp_ += sizeof(h);\n> +\t\t\tp_ = internal::align::up(p_, h.alignment);\n> +\t\t\tp_ += h.size;\n> +\t\t\tp_ = internal::align::up(p_, alignof(decltype(h)));\n> +\n> +\t\t\treturn *this;\n> +\t\t}\n> +\n> +\t\titerator operator++(int)\n> +\t\t{\n> +\t\t\tauto copy = *this;\n> +\t\t\t++*this;\n> +\t\t\treturn copy;\n> +\t\t}\n> +\n> +\t\t[[nodiscard]]\n> +\t\treference operator*() const\n> +\t\t{\n> +\t\t\tconst auto &h = header();\n> +\t\t\tconst auto *data = internal::align::up(p_ + sizeof(h), h.alignment);\n> +\n> +\t\t\treturn { h.tag, { h.type, h.isArray, h.numElements, data } };\n> +\t\t}\n> +\n> +\t\t[[nodiscard]]\n> +\t\tbool operator==(const iterator &other) const\n> +\t\t{\n> +\t\t\treturn p_ == other.p_;\n> +\t\t}\n> +\n> +\t\t[[nodiscard]]\n> +\t\tbool operator!=(const iterator &other) const\n> +\t\t{\n> +\t\t\treturn !(*this == other);\n> +\t\t}\n> +\n> +\tprivate:\n> +\t\titerator(const std::byte *p)\n> +\t\t\t: p_(p)\n> +\t\t{\n> +\t\t}\n> +\n> +\t\t[[nodiscard]]\n> +\t\tconst ValueHeader &header() const\n> +\t\t{\n> +\t\t\treturn *reinterpret_cast<const ValueHeader *>(p_);\n> +\t\t}\n> +\n> +\t\tfriend MetadataList;\n> +\n> +\t\tconst std::byte *p_ = nullptr;\n> +\t};\n> +\n> +\t[[nodiscard]]\n> +\titerator begin() const\n> +\t{\n> +\t\treturn { p_ + contentOffset_ };\n> +\t}\n> +\n> +\t[[nodiscard]]\n> +\titerator end() const\n> +\t{\n> +\t\treturn { p_ + contentOffset_ + state_.load(std::memory_order_acquire).fill };\n> +\t}\n\nYey, that's nice we can easily iterate value like this\n\n> +\n> +\tclass Diff\n> +\t{\n> +\tpublic:\n> +\t\t// \\todo want these?\n\nAs commented on the previous todo item, they seem reasonable for the\npublic interface of a Diff, but if you're not sure feel free to drop\nthem. In any case, drop the \\todo item\n\n\n> +\t\t[[nodiscard]] explicit operator bool() const { return !empty(); }\n> +\t\t[[nodiscard]] bool empty() const { return start_ == stop_; }\n> +\t\t[[nodiscard]] std::size_t size() const { return changed_; }\n> +\t\t[[nodiscard]] const MetadataList &list() const { return *list_; }\n> +\n> +\t\t[[nodiscard]]\n> +\t\tControlValueView get(uint32_t tag) const\n> +\t\t{\n> +\t\t\tconst auto *e = list_->find(tag);\n> +\t\t\tif (!e)\n> +\t\t\t\treturn {};\n> +\n> +\t\t\tauto o = e->acquireData();\n> +\t\t\tif (!o)\n> +\t\t\t\treturn {};\n> +\n> +\t\t\tif (!(start_ <= *o && *o < stop_))\n> +\t\t\t\treturn {};\n> +\n> +\t\t\treturn list_->dataOf(*o);\n> +\t\t}\n> +\n> +\t\ttemplate<typename T>\n> +\t\t[[nodiscard]]\n> +\t\tstd::optional<T> get(const Control<T> &ctrl) const\n> +\t\t{\n> +\t\t\tControlValueView v = get(ctrl.id());\n> +\t\t\tif (!v)\n> +\t\t\t\treturn {};\n> +\n> +\t\t\treturn v.get<T>();\n> +\t\t}\n> +\n> +\t\t[[nodiscard]]\n> +\t\titerator begin() const\n> +\t\t{\n> +\t\t\treturn { list_->p_ + start_ };\n> +\t\t}\n> +\n> +\t\t[[nodiscard]]\n> +\t\titerator end() const\n> +\t\t{\n> +\t\t\treturn { list_->p_ + stop_ };\n> +\t\t}\n> +\n> +\tprivate:\n> +\t\tDiff(const MetadataList &list, std::size_t changed, std::size_t oldFill, std::size_t newFill)\n> +\t\t\t: list_(&list),\n> +\t\t\t  changed_(changed),\n> +\t\t\t  start_(list.contentOffset_ + oldFill),\n> +\t\t\t  stop_(list.contentOffset_ + newFill)\n> +\t\t{\n> +\t\t}\n> +\n> +\t\tfriend MetadataList;\n> +\t\tfriend struct Checkpoint;\n> +\n> +\t\t/**\n> +\t\t * \\brief Source lits of the checkpoint\n\nof the checkpoint used to create the diff\n\n> +\t\t */\n> +\t\tconst MetadataList *list_ = nullptr;\n> +\n> +\t\t/**\n> +\t\t * \\brief Number of items contained in the diff\n> +\t\t */\n> +\t\tstd::size_t changed_;\n> +\n> +\t\t/**\n> +\t\t * \\brief Offset of the ValueHeader of the first value in the diff\n> +\t\t */\n> +\t\tstd::size_t start_;\n> +\n> +\t\t/**\n> +\t\t * \\brief Offset of the \"past-the-end\" ValueHeader of the diff\n> +\t\t */\n> +\t\tstd::size_t stop_;\n> +\t};\n> +\n> +\t[[nodiscard]] std::optional<Diff> merge(const ControlList &other);\n> +\n> +\tclass Checkpoint\n> +\t{\n> +\tpublic:\n> +\t\t[[nodiscard]]\n> +\t\tDiff diffSince() const\n> +\t\t{\n> +\t\t\t/* sync with release-store on `state_` in `set()` */\n> +\t\t\tconst auto curr = list_->state_.load(std::memory_order_acquire);\n> +\n> +\t\t\tassert(state_.count <= curr.count);\n> +\t\t\tassert(state_.fill <= curr.fill);\n> +\n> +\t\t\treturn {\n> +\t\t\t\t*list_,\n> +\t\t\t\tcurr.count - state_.count,\n> +\t\t\t\tstate_.fill,\n> +\t\t\t\tcurr.fill,\n> +\t\t\t};\n> +\t\t}\n> +\n> +\tprivate:\n> +\t\tCheckpoint(const MetadataList &list)\n> +\t\t\t: list_(&list),\n> +\t\t\t  state_(list.state_.load(std::memory_order_relaxed))\n> +\t\t{\n> +\t\t}\n> +\n> +\t\tfriend MetadataList;\n> +\n> +\t\t/**\n> +\t\t * \\brief Source list of the checkpoint\n> +\t\t */\n> +\t\tconst MetadataList *list_ = nullptr;\n> +\n> +\t\t/**\n> +\t\t * \\brief State of the list when the checkpoint was created\n> +\t\t */\n> +\t\tState state_ = {};\n> +\t};\n> +\n> +\t[[nodiscard]]\n> +\tCheckpoint checkpoint() const\n> +\t{\n> +\t\treturn { *this };\n> +\t}\n> +\n> +private:\n> +\t[[nodiscard]]\n> +\tstatic constexpr std::size_t entriesOffset()\n> +\t{\n> +\t\treturn 0;\n> +\t}\n\nPlease remove this additional indirection layer. I don't think we want\nanything before the entries, and if we want we should decide it right\nnow. Once we commit to a serialization format that will be it, so I\ndon't think we need to prepare for modifying it.\n\nOr does this have a purpose I'm missing\n\n> +\n> +\t[[nodiscard]]\n> +\tstatic constexpr std::size_t contentOffset(std::size_t entries)\n> +\t{\n> +\t\treturn internal::align::up(entriesOffset() + entries * sizeof(Entry), alignof(ValueHeader));\n> +\t}\n\nCould you shorten this please ?\n\n\t\treturn internal::align::up(entriesOffset() + entries * sizeof(Entry),\n\t\t\t\t\t  alignof(ValueHeader));\n> +\n> +\t[[nodiscard]]\n> +\tSpan<Entry> entries() const\n> +\t{\n> +\t\treturn { reinterpret_cast<Entry *>(p_ + entriesOffset()), capacity_ };\n> +\t}\n> +\n> +\t[[nodiscard]]\n> +\tEntry *find(uint32_t tag) const\n> +\t{\n> +\t\tconst auto entries = this->entries();\n> +\t\tauto it = std::partition_point(entries.begin(), entries.end(), [&](const auto &e) {\n\nCan you shorten this please ?\n\n> +\t\t\treturn e.tag < tag;\n> +\t\t});\n> +\n> +\t\tif (it == entries.end() || it->tag != tag)\n> +\t\t\treturn nullptr;\n> +\n> +\t\treturn &*it;\n> +\t}\n> +\n> +\t[[nodiscard]]\n> +\tControlValueView dataOf(const Entry &e) const\n> +\t{\n> +\t\tconst auto o = e.acquireData();\n> +\t\treturn o ? dataOf(*o) : ControlValueView{ };\n> +\t}\n> +\n> +\t[[nodiscard]]\n> +\tControlValueView dataOf(std::size_t headerOffset) const\n\nThe only user if I'm not mistken is\n\n\t\tControlValueView get(uint32_t tag) const\n\t\t{\n\t\t\tconst auto *e = list_->find(tag);\n\t\t\tif (!e)\n\t\t\t\treturn {};\n\n\t\t\tauto o = e->acquireData();\n\t\t\tif (!o)\n\t\t\t\treturn {};\n\n\t\t\tif (!(start_ <= *o && *o < stop_))\n\t\t\t\treturn {};\n\n\t\t\treturn list_->dataOf(*o);\n\t\t}\n\nWhere you need o to bound-check the diff.\n\nHowever I guess we could save an overload by only exposing\n\n\tControlValueView dataOf(const Entry &e) const\n\nYou can pass (*e) in to the get() implementation and reimplement\ndataOf(const Entry &e) {} as dataOf(std::size_t headerOffset) {}\n\n> +\t{\n> +\t\tassert(headerOffset <= alloc_ - sizeof(ValueHeader));\n> +\t\tassert(internal::align::is(p_ + headerOffset, alignof(ValueHeader)));\n> +\n> +\t\tconst auto *vh = reinterpret_cast<const ValueHeader *>(p_ + headerOffset);\n> +\t\tconst auto *p = reinterpret_cast<const std::byte *>(vh) + sizeof(*vh);\n> +\t\tstd::size_t avail = p_ + alloc_ - p;\n> +\n> +\t\tconst auto *data = internal::align::up(vh->size, vh->alignment, p, &avail);\n> +\t\tassert(data);\n> +\n> +\t\treturn { vh->type, vh->isArray, vh->numElements, data };\n> +\t}\n\nI always struggle setting rules on which function has to be inlined\nand which should go to the .cpp file, but I guess this is large enough\nto qualify for being moved to the .cpp file\n\n> +\n> +\t[[nodiscard]] SetError set(Entry &e, ControlValueView v);\n> +\n> +\t[[nodiscard]]\n> +\tstd::pair<MetadataList::SetError, MetadataList::ValueHeader *>\n> +\tset(const Entry &e, ControlValueView v, State &s);\n> +\n> +\t/**\n> +\t * \\brief Number of \\ref Entry \"entries\"\n> +\t */\n> +\tstd::size_t capacity_ = 0;\n> +\n> +\t/**\n> +\t * \\brief Offset of the first ValueHeader\n> +\t */\n> +\tstd::size_t contentOffset_ = -1;\n> +\n> +\t/**\n> +\t * \\brief Pointer to the allocation\n> +\t */\n> +\tstd::byte *p_ = nullptr;\n> +\n> +\t/**\n> +\t * \\brief Size of the allocation in bytes\n> +\t */\n> +\tstd::size_t alloc_ = 0;\n> +\n> +\t/**\n> +\t * \\brief Current state of the list\n> +\t */\n> +\tstd::atomic<State> state_ = State{};\n> +\n> +\t// \\todo ControlIdMap in any way shape or form?\n\nNot if you didn't need it, so I guess you can remove this\n\n> +\n> +\t/*\n> +\t * If this is problematic on a 32-bit architecture, then\n\nAre you afraid that in the definition of\n\n\tstruct State {\n\t\tuint32_t count;\n\t\tuint32_t fill;\n\t};\n\ncount and fill can be stored in a single 64-bit word ?\n\n\n> +\t * `count` can be stored in a separate atomic variable\n> +\t * but then `Diff::changed_` must be removed since the fill\n> +\t * level and item count cannot be retrieved atomically.\n> +\t */\n> +\tstatic_assert(decltype(state_)::is_always_lock_free);\n> +};\n> +\n> +} /* namespace libcamera */\n> diff --git a/include/libcamera/metadata_list_plan.h b/include/libcamera/metadata_list_plan.h\n> new file mode 100644\n> index 000000000..8f058e5c0\n> --- /dev/null\n> +++ b/include/libcamera/metadata_list_plan.h\n> @@ -0,0 +1,110 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025, Ideas On Board Oy\n> + */\n> +\n> +#pragma once\n> +\n> +#include <cassert>\n> +#include <cstddef>\n> +#include <map>\n> +#include <stdint.h>\n> +#include <type_traits>\n> +\n> +#include <libcamera/base/internal/cxx20.h>\n> +\n> +#include <libcamera/controls.h>\n> +\n> +namespace libcamera {\n> +\n> +class MetadataListPlan\n> +{\n> +public:\n> +\tstruct Entry {\n> +\t\tuint32_t size;\n> +\t\tuint32_t alignment; // \\todo is this necessary?\n\nYou tell me..\nIt seems this is used to construct the Entry and the alignment is\npassed from here to the MetadataList::Entry.\n\n\n> +\t\tuint32_t numElements;\n> +\t\tControlType type;\n> +\t\tbool isArray;\n> +\t};\n> +\n> +\t[[nodiscard]] bool empty() const { return items_.empty(); }\n> +\t[[nodiscard]] std::size_t size() const { return items_.size(); }\n> +\t[[nodiscard]] decltype(auto) begin() const { return items_.begin(); }\n> +\t[[nodiscard]] decltype(auto) end() const { return items_.end(); }\n\nempty line maybe\n\n> +\tvoid clear() { items_.clear(); }\n> +\n> +\ttemplate<\n> +\t\ttypename T,\n> +\t\tstd::enable_if_t<libcamera::details::control_type<T>::size != libcamera::dynamic_extent> * = nullptr\n> +\t>\n> +\tMetadataListPlan &set(const Control<T> &ctrl)\n> +\t{\n> +\t\tif constexpr (libcamera::details::control_type<T>::size > 0) {\n> +\t\t\tstatic_assert(libcamera::details::control_type<T>::size != libcamera::dynamic_extent);\n\nisn't this specialization selected by enable_if<> only in case size !=\ndynamic_extent ?\n\n> +\n> +\t\t\treturn set<typename T::value_type>(\n> +\t\t\t\tctrl.id(),\n> +\t\t\t\tlibcamera::details::control_type<T>::size,\n> +\t\t\t\ttrue\n> +\t\t\t);\n> +\t\t} else {\n> +\t\t\treturn set<T>(ctrl.id(), 1, false);\n> +\t\t}\n\nYou can remove the else branch {} as you return in the previous if\n\nOr you can invert them\n\n\t\tif constexpr (libcamera::details::control_type<T>::size == 0)\n\t\t\treturn set<T>(ctrl.id(), 1, false);\n\n                static_assert(libcamera::details::control_type<T>::size != libcamera::dynamic_extent);\n\n                return set<typename T::value_type>(\n                        ctrl.id(),\n                        libcamera::details::control_type<T>::size,\n                        true\n                );\n> +\t}\n> +\n> +\ttemplate<\n> +\t\ttypename T,\n> +\t\tstd::enable_if_t<libcamera::details::control_type<T>::size == libcamera::dynamic_extent> * = nullptr\n> +\t>\n\nis SFINAE necessary here or are these simply two overloads of the set\nfunction ?\n\n> +\tMetadataListPlan &set(const Control<T> &ctrl, std::size_t numElements)\n> +\t{\n> +\t\treturn set<typename T::value_type>(ctrl.id(), numElements, true);\n> +\t}\n> +\n> +\t[[nodiscard]]\n> +\tbool set(uint32_t tag,\n> +\t\t std::size_t size, std::size_t alignment,\n> +\t\t std::size_t numElements, ControlType type, bool isArray);\n\nIs this meant to be used ? I mean, should this be part of the public\ninterface ?\n\n> +\n> +\t[[nodiscard]]\n> +\tconst Entry *get(uint32_t tag) const\n> +\t{\n> +\t\tauto it = items_.find(tag);\n> +\t\tif (it == items_.end())\n> +\t\t\treturn nullptr;\n> +\n> +\t\treturn &it->second;\n> +\t}\n> +\n> +\t[[nodiscard]]\n> +\tconst Entry *get(const ControlId &cid) const\n> +\t{\n> +\t\tconst auto *e = get(cid.id());\n> +\t\tif (!e)\n> +\t\t\treturn nullptr;\n> +\n> +\t\tif (e->type != cid.type() || e->isArray != cid.isArray())\n\nCan this happen ? Should we rather assert ?\n\n> +\t\t\treturn nullptr;\n> +\n> +\t\treturn e;\n> +\t}\n> +\n> +private:\n> +\tstd::map<uint32_t, Entry> items_;\n> +\n> +\ttemplate<typename T>\n> +\tMetadataListPlan &set(uint32_t tag, std::size_t numElements, bool isArray)\n> +\t{\n> +\t\tstatic_assert(std::is_trivially_copyable_v<T>);\n\nI might have missed why, since we're not copying T over I guess\n\n> +\n> +\t\t[[maybe_unused]] bool ok = set(tag,\n\nIsn't this checked just here below ?\n\n> +\t\t\t\t\t       sizeof(T), alignof(T),\n> +\t\t\t\t\t       numElements, details::control_type<T>::value, isArray);\n> +\t\tassert(ok);\n> +\n> +\t\treturn *this;\n> +\t}\n> +};\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 575408b2c..b569996bb 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -9,6 +9,7 @@ libcamera_public_sources = files([\n>      'framebuffer.cpp',\n>      'framebuffer_allocator.cpp',\n>      'geometry.cpp',\n> +    'metadata_list.cpp',\n>      'orientation.cpp',\n>      'pixel_format.cpp',\n>      'request.cpp',\n> diff --git a/src/libcamera/metadata_list.cpp b/src/libcamera/metadata_list.cpp\n> new file mode 100644\n> index 000000000..5a5114fc7\n> --- /dev/null\n> +++ b/src/libcamera/metadata_list.cpp\n> @@ -0,0 +1,594 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025, Ideas On Board Oy\n> + */\n> +\n> +#include <libcamera/metadata_list.h>\n> +\n> +#include <cstring>\n> +#include <limits>\n> +#include <new>\n> +\n> +#include <libcamera/base/internal/align.h>\n> +#include <libcamera/base/internal/cxx20.h>\n> +\n> +#include <libcamera/metadata_list_plan.h>\n\nDo you need a metadata_list_plan.cpp ?\n\n> +\n> +#if __has_include(<sanitizer/asan_interface.h>)\n> +#if __SANITIZE_ADDRESS__ /* gcc */\n> +#include <sanitizer/asan_interface.h>\n> +#define HAS_ASAN 1\n> +#elif defined(__has_feature)\n> +#if __has_feature(address_sanitizer) /* clang */\n> +#include <sanitizer/asan_interface.h>\n> +#define HAS_ASAN 1\n> +#endif\n> +#endif\n> +#endif\n> +\n> +namespace libcamera {\n> +\n> +/**\n> + * \\class MetadataListPlan\n> + * \\brief Class to hold the possible set of metadata items for a MetadataList\n\nWe need a bit more than that I'm afraid. Not very much, but not just a\nbrief\n\nWho should use them and how, what is the plan purpose etc\n\nOn a more general note: should MetdadataListPlan be exposed to\napplications ? I have a vague memory we discussed it but I don't\nremember it.\n\nWas it need by Pipewire maybe ?\n\n> + */\n> +\n> +/**\n> + * \\class MetadataListPlan::Entry\n> + * \\brief Details of a metadata item\n> + */\n> +\n> +/**\n> + * \\internal\n\nWhy internal ?\n\n> + * \\var MetadataListPlan::Entry::size\n> + * \\brief Number of bytes in a single element\n> + *\n> + * \\var MetadataListPlan::Entry::alignment\n> + * \\brief Required alignment of the elements\n> + * \\endinternal\n> + *\n> + * \\var MetadataListPlan::Entry::numElements\n> + * \\brief Number of elements in the value\n> + * \\sa ControlValueView::numElements()\n> + *\n> + * \\var MetadataListPlan::Entry::type\n> + * \\brief The type of the value\n> + * \\sa ControlValueView::type()\n> + *\n> + * \\var MetadataListPlan::Entry::isArray\n> + * \\brief Whether or not the value is array-like\n> + * \\sa ControlValueView::isArray()\n> + */\n> +\n> +/**\n> + * \\fn MetadataListPlan::begin() const\n>+ * \\brief Retrieve the begin iterator\n\nif this is actually parsed by Doxygen we'll need to document return\nvales as well\n\nSame for the functions here below\n\n> + */\n> +\n> +/**\n> + * \\fn MetadataListPlan::end() const\n> + * \\brief Retrieve the end iterator\n> + */\n> +\n> +/**\n> + * \\fn MetadataListPlan::size() const\n> + * \\brief Retrieve the number of entries\n> + */\n> +\n> +/**\n> + * \\fn MetadataListPlan::empty() const\n> + * \\brief Check if empty\n> + */\n> +\n> +/**\n> + * \\internal\n> + * \\fn MetadataListPlan::clear()\n> + * \\brief Remove all controls\n> + */\n> +\n> +/**\n> + * \\internal\n> + * \\fn MetadataListPlan::set(const Control<T> &ctrl)\n> + * \\brief Add an entry for the given control to the metadata list plan\n> + * \\param[in] ctrl The control\n> + */\n> +\n> +/**\n> + * \\internal\n> + * \\fn MetadataListPlan::set(const Control<T> &ctrl, std::size_t count)\n> + * \\brief Add an entry for the given dynamically-sized control to the metadata list plan\n> + * \\param[in] ctrl The control\n> + * \\param[in] count The maximum number of elements\n> + *\n> + * Add the dynamically-sized control \\a ctrl to the metadata list plan with a maximum\n> + * capacity of \\a count elements.\n> + */\n> +\n> +/**\n> + * \\internal\n> + * \\brief Add an entry to the metadata list plan\n> + * \\return \\a true if the entry has been added, or \\a false if the given parameters\n> + *         would result in an invalid entry\n> + *\n> + * This functions adds an entry with essentially arbitrary parameters, without deriving\n> + * them from a given ControlId instance. This is mainly used when deserializing.\n> + */\n> +bool MetadataListPlan::set(uint32_t tag,\n> +\t\t\t   std::size_t size, std::size_t alignment,\n> +\t\t\t   std::size_t numElements, ControlType type, bool isArray)\n> +{\n> +\tif (size == 0 || size > std::numeric_limits<uint32_t>::max())\n> +\t\treturn false;\n> +\tif (alignment > std::numeric_limits<uint32_t>::max())\n> +\t\treturn false;\n> +\tif (!internal::cxx20::has_single_bit(alignment))\n> +\t\treturn false;\n> +\tif (numElements > std::numeric_limits<uint32_t>::max() / size)\n> +\t\treturn false;\n> +\tif (!isArray && numElements != 1)\n> +\t\treturn false;\n> +\n> +\titems_[tag] = {\n> +\t\t.size = uint32_t(size),\n> +\t\t.alignment = uint32_t(alignment),\n> +\t\t.numElements = uint32_t(numElements),\n> +\t\t.type = type,\n> +\t\t.isArray = isArray,\n> +\t};\n> +\n> +\treturn true;\n> +}\n> +\n> +/**\n> + * \\fn MetadataListPlan::get(uint32_t tag) const\n> + * \\brief Find the \\ref Entry \"entry\" with the given identifier\n> + */\n> +\n> +/**\n> + * \\fn MetadataListPlan::get(const ControlId &cid) const\n> + * \\brief Find the \\ref Entry \"entry\" for the given ControlId\n> + *\n> + * The \\ref Entry \"entry\" is only returned if ControlId::type() and ControlId::isArray()\n> + * of \\a cid matches Entry::type and Entry::isArray, respectively.\n\nCould you check that documentation lines stays in 80 cols ?\n\n> + */\n> +\n> +/**\n> + * \\class MetadataList\n> + * \\brief Class to hold metadata items\n> + *\n> + * Similarly to a ControlList, a MetadataList provides a way for applications to\n> + * query and enumerate the values of controls. However, a MetadataList allows\n> + * thread-safe access to the data for applications, which is needed so that\n> + * applications can process the metadata of in-flight \\ref Request \"requests\"\n> + * (for which purposes ControlList is not suitable).\n> + *\n> + * \\internal\n> + * A MetadataList is essentially an append-only list of values. Internally, it\n> + * contains a single allocation that is divided into two parts:\n> + *\n> + *   * a list of entries sorted by their numeric identifiers\n> + *     (each corresponding to an entry in the MetadataListPlan);\n> + *   * a series of ValueHeader + data bytes that contain the actual data.\n> + *\n> + * When a value is added to the list, the corresponding Entry is updated, and the\n> + * ValueHeader and the data bytes are appended to the end of the second part.\n> + *\n> + * The reason for the redundancy is the following: the first part enables quick\n> + * lookups (binary search); the second part provides a self-contained flat buffer\n> + * of all the data.\n\nNice, and the design documentation nicely complements it.\n\nHowever, I'm missing where we explain how MetadataList,\nMetadataList::Checkpoint and MetadataList::Diff have to be used by\napplications.\n\nI presume a bit should go here and as done for the metadataAvailable\nsignal in the next commit, a part should go in the application\ndeveloper guide.\n\nSame for MetadataListPlan, it should probably be described in the\npipeline developer guide as well.\n\n> + */\n> +\n> +/**\n> + * \\internal\n> + * \\brief Construct a metadata list according to \\a plan\n> + *\n> + * Construct a metadata list according to the provided \\a plan.\n> + */\n> +MetadataList::MetadataList(const MetadataListPlan &plan)\n> +\t: capacity_(plan.size()),\n> +\t  contentOffset_(MetadataList::contentOffset(capacity_)),\n> +\t  alloc_(contentOffset_)\n> +{\n> +\tfor (const auto &[tag, e] : plan) {\n> +\t\talloc_ += sizeof(ValueHeader);\n> +\t\talloc_ += e.alignment - 1; // XXX: this is the maximum\n> +\t\talloc_ += e.size * e.numElements;\n> +\t\talloc_ += alignof(ValueHeader) - 1; // XXX: this is the maximum\n\nMake the // XXX proper comments if you wish\n\nOn alignments:\n\nWhat matters is that, using int32_t as an example, the values are\naligned at a 4 bytes boundary.\n\nWe serialize data after ValueHeader so I guess what we need to make\nsure is that the space reserved for ValueHeader is aligned to the size\nof the data type that follows.\n\n+------------+       +--------+    +--------+       +------------+\n| ValueHeader|padding| int32_t|....| int32_t|padding| ValueHeader|\n+------------+       +--------+    +--------+       +------------+\n\nWhich seems to match the code above. But what if sizeof(ValueHeader)\nis already aligned to the size of the next element to follow ?\n\nShould padding be added unconditionally ?\n\n> +\t}\n> +\n> +\tp_ = static_cast<std::byte *>(::operator new(alloc_));\n> +\n> +\tauto *entries = reinterpret_cast<Entry *>(p_ + entriesOffset());\n> +\tauto it = plan.begin();\n> +\n> +\tfor (std::size_t i = 0; i < capacity_; i++, ++it) {\n> +\t\tconst auto &[tag, e] = *it;\n> +\n> +\t\tnew (static_cast<void *>(&entries[i])) Entry{\n> +\t\t\t.tag = tag,\n> +\t\t\t.capacity = e.size * e.numElements,\n> +\t\t\t.alignment = e.alignment,\n> +\t\t\t.type = e.type,\n> +\t\t\t.isArray = e.isArray,\n> +\t\t};\n> +\t}\n> +\n> +#if HAS_ASAN\n> +\t::__sanitizer_annotate_contiguous_container(\n> +\t\tp_ + contentOffset_, p_ + alloc_,\n> +\t\tp_ + alloc_, p_ + contentOffset_\n> +\t);\n> +#endif\n> +}\n> +\n> +MetadataList::~MetadataList()\n> +{\n> +\tfor (auto &e : entries())\n> +\t\tstd::destroy_at(&e);\n> +\n> +#if HAS_ASAN\n> +\t/*\n> +\t * The documentation says the range apparently has to be\n> +\t * restored to its initial state before it is deallocated.\n> +\t */\n> +\t::__sanitizer_annotate_contiguous_container(\n> +\t\tp_ + contentOffset_, p_ + alloc_,\n> +\t\tp_ + contentOffset_ + state_.load(std::memory_order_relaxed).fill, p_ + alloc_\n> +\t);\n> +#endif\n> +\n> +\t::operator delete(p_, alloc_);\n> +}\n> +\n> +/**\n> + * \\fn MetadataList::size() const\n> + * \\brief Retrieve the number of controls\n> + * \\context This function is \\threadsafe.\n> + * \\note If the list is being modified, the return value may be out of\n> + *       date by the time the function returns\n\nIs it ? Doesn't the usage of std::atomic prevent exactly races ?\nI mean, of course you can call this function and read count a few\nuseconds before the libcamera thread adds new metadata, but this is\nnot something unusual, isn't it ?\n\n> + */\n> +\n> +/**\n> + * \\fn MetadataList::empty() const\n> + * \\brief Check if empty\n> + * \\context This function is \\threadsafe.\n> + * \\note If the list is being modified, the return value may be out of\n> + *       date by the time the function returns\n> + */\n> +\n> +/**\n> + * \\internal\n> + * \\brief Remove all items from the list\n> + * \\note This function in effect resets the list to its original state. As a consequence it invalidates - among others -\n> + *       all iterators, Checkpoint, and Diff objects that are associated with the list. No readers must exist\n> + *       when this function is called.\n\n80 cols\n\nWhy do we even want to expose this and not let the Request\nclear/destroy the list at the appropriate time ?\n\n> + */\n> +void MetadataList::clear()\n> +{\n> +\tfor (auto &e : entries())\n> +\t\te.headerOffset.store(Entry::kInvalidOffset, std::memory_order_relaxed);\n> +\n> +\t[[maybe_unused]] State s = state_.exchange({}, std::memory_order_relaxed);\n> +\n> +#if HAS_ASAN\n> +\t::__sanitizer_annotate_contiguous_container(\n> +\t\tp_ + contentOffset_, p_ + alloc_,\n> +\t\tp_ + contentOffset_ + s.fill, p_ + contentOffset_\n> +\t);\n> +#endif\n> +}\n> +\n> +\n\ndouble empy line\n\n> +/**\n> + * \\fn MetadataList::begin() const\n> + * \\brief Retrieve begin iterator\n> + * \\context This function is \\threadsafe.\n> + */\n> +\n> +/**\n> + * \\fn MetadataList::end() const\n> + * \\brief Retrieve end iterator\n> + * \\context This function is \\threadsafe.\n> + */\n> +\n> +/**\n> + * \\fn MetadataList::get(const Control<T> &ctrl) const\n> + * \\brief Get the value of control \\a ctrl\n> + * \\return A std::optional<T> containing the control value, or std::nullopt if\n> + *         the control \\a ctrl is not present in the list\n> + * \\context This function is \\threadsafe.\n> + */\n> +\n> +/**\n> + * \\fn MetadataList::get(uint32_t tag) const\n> + * \\brief Get the value of pertaining to the numeric identifier \\a tag\n> + * \\return A std::optional<T> containing the control value, or std::nullopt if\n> + *         the control is not present in the list\n> + * \\context This function is \\threadsafe.\n> + */\n> +\n> +/**\n> + * \\internal\n> + * \\fn MetadataList::set(const Control<T> &ctrl, const internal::cxx20::type_identity_t<T> &value)\n> + * \\brief Set the value of control \\a ctrl to \\a value\n> + */\n> +\n> +/**\n> + * \\internal\n> + * \\fn MetadataList::set(uint32_t tag, ControlValueView v)\n> + * \\brief Set the value of pertaining to the numeric identifier \\a tag to \\a v\n> + */\n> +\n> +/**\n> + * \\internal\n> + * \\brief Add items from \\a other\n> + *\n> + * If any of them items cannot be added, then an empty optional is returned,\n> + * and this function has no effects.\n> + */\n> +std::optional<MetadataList::Diff> MetadataList::merge(const ControlList &other)\n> +{\n> +\t// \\todo check id map of `other`?\n\nIsn't enough to make sure all controls we add have a corresponding\nentry in the metadata list ?\n\n> +\n> +\t/* Copy the data and update a temporary state (`newState`) */\n> +\n> +\tconst auto oldState = state_.load(std::memory_order_relaxed);\n> +\tauto newState = oldState;\n> +\tconst auto entries = this->entries();\n> +\n> +\tfor (const auto &[tag, value] : other) {\n> +\t\tauto *e = find(tag);\n> +\t\tif (!e)\n> +\t\t\treturn {};\n\nExactly.\n\nI would even assert, after all only pipelines are supposed to merge\nControlList in MetadataList, and if they do it wrong it's better to\ncatch it at development time ?\n\nI'm sorry but I'll stop here.\n\nThis review has been proven exhausting already. Let's go over what's\nabove here and complete the next part after.\n\nThanks\n  j\n\n> +\n> +\t\tauto [ err, header ] = set(*e, value, newState);\n> +\t\tif (err != SetError())\n> +\t\t\treturn {};\n> +\n> +\t\t/* HACK: temporarily use the `tag` member to store the entry index */\n> +\t\theader->tag = e - entries.data();\n> +\t}\n> +\n> +\t/*\n> +\t * At this point the data is already in place and every item has been validated\n> +\t * to have a known id, appropriate size and type, etc., but they are not visible\n> +\t * in any way. The next step is to make them visible by updating `headerOffset`\n> +\t * in each affected `Entry` and `state_` in `*this`.\n> +\t */\n> +\n> +\titerator it(p_ + contentOffset_ + oldState.fill);\n> +\tconst iterator end(p_ + contentOffset_ + newState.fill);\n> +\n> +\tfor (; it != end; ++it) {\n> +\t\tauto &header = const_cast<ValueHeader &>(it.header());\n> +\t\tauto &e = entries[header.tag]; /* HACK: header.tag is temporarily the Entry index */\n> +\n> +\t\theader.tag = e.tag; /* HACK: restore */\n> +\n> +\t\te.headerOffset.store(\n> +\t\t\treinterpret_cast<const std::byte *>(&header) - p_,\n> +\t\t\tstd::memory_order_release\n> +\t\t);\n> +\t}\n> +\n> +\tstate_.store(newState, std::memory_order_release);\n> +\n> +\treturn {{ *this, newState.count - oldState.count, oldState.fill, newState.fill }};\n> +}\n> +\n> +/**\n> + * \\internal\n> + * \\enum MetadataList::SetError\n> + * \\brief Error code returned by a set operation\n> + *\n> + * \\var MetadataList::SetError::UnknownTag\n> + * \\brief The tag is not supported by the metadata list\n> + * \\var MetadataList::SetError::AlreadySet\n> + * \\brief A value has already been added with the given tag\n> + * \\var MetadataList::SetError::SizeMismatch\n> + * \\brief The size of the data is not appropriate for the given tag\n> + * \\var MetadataList::SetError::TypeMismatch\n> + * \\brief The type of the value does not match the expected type\n> + */\n> +\n> +/**\n> + * \\internal\n> + * \\fn MetadataList::checkpoint() const\n> + * \\brief Create a checkpoint\n> + * \\context This function is \\threadsafe.\n> + */\n> +\n> +MetadataList::SetError MetadataList::set(Entry &e, ControlValueView v)\n> +{\n> +\tauto s = state_.load(std::memory_order_relaxed);\n> +\n> +\tauto [ err, header ] = set(e, v, s);\n> +\tif (err != SetError())\n> +\t\treturn err;\n> +\n> +\te.headerOffset.store(\n> +\t\treinterpret_cast<const std::byte *>(header) - p_,\n> +\t\tstd::memory_order_release\n> +\t);\n> +\tstate_.store(s, std::memory_order_release);\n> +\n> +\treturn {};\n> +}\n> +\n> +std::pair<MetadataList::SetError, MetadataList::ValueHeader *>\n> +MetadataList::set(const Entry &e, ControlValueView v, State &s)\n> +{\n> +\tif (e.hasValue())\n> +\t\treturn { SetError::AlreadySet, {} };\n> +\tif (e.type != v.type() || e.isArray != v.isArray())\n> +\t\treturn { SetError::TypeMismatch, {} };\n> +\n> +\tconst auto src = v.data();\n> +\tif (e.isArray) {\n> +\t\tif (src.size_bytes() > e.capacity)\n> +\t\t\treturn { SetError::SizeMismatch, {} };\n> +\t} else {\n> +\t\tif (src.size_bytes() != e.capacity)\n> +\t\t\treturn { SetError::SizeMismatch, {} };\n> +\t}\n> +\n> +\tstd::byte *oldEnd = p_ + contentOffset_ + s.fill;\n> +\tstd::byte *p = oldEnd;\n> +\n> +\tauto *headerPtr = internal::align::up<ValueHeader>(p);\n> +\tauto *dataPtr = internal::align::up(src.size_bytes(), e.alignment, p);\n> +\tinternal::align::up(0, alignof(ValueHeader), p);\n> +\n> +#if HAS_ASAN\n> +\t::__sanitizer_annotate_contiguous_container(\n> +\t\tp_ + contentOffset_, p_ + alloc_,\n> +\t\toldEnd, p\n> +\t);\n> +#endif\n> +\n> +\tauto *header = new (headerPtr) ValueHeader{\n> +\t\t.tag = e.tag,\n> +\t\t.size = uint32_t(src.size_bytes()),\n> +\t\t.alignment = e.alignment,\n> +\t\t.type = v.type(),\n> +\t\t.isArray = v.isArray(),\n> +\t\t.numElements = uint32_t(v.numElements()),\n> +\t};\n> +\tstd::memcpy(dataPtr, src.data(), src.size_bytes());\n> +\n> +\ts.fill += p - oldEnd;\n> +\ts.count += 1;\n> +\n> +\treturn { {}, header };\n> +}\n> +\n> +/**\n> + * \\class MetadataList::iterator\n> + * \\brief Iterator\n> + */\n> +\n> +/**\n> + * \\typedef MetadataList::iterator::difference_type\n> + * \\brief iterator's difference type\n> + */\n> +\n> +/**\n> + * \\typedef MetadataList::iterator::value_type\n> + * \\brief iterator's value type\n> + */\n> +\n> +/**\n> + * \\typedef MetadataList::iterator::pointer\n> + * \\brief iterator's pointer type\n> + */\n> +\n> +/**\n> + * \\typedef MetadataList::iterator::reference\n> + * \\brief iterator's reference type\n> + */\n> +\n> +/**\n> + * \\typedef MetadataList::iterator::iterator_category\n> + * \\brief iterator's category\n> + */\n> +\n> +/**\n> + * \\fn MetadataList::iterator::operator*()\n> + * \\brief Retrieve value at iterator\n> + * \\return A \\a ControlListView representing the value\n> + */\n> +\n> +/**\n> + * \\fn MetadataList::iterator::operator==(const iterator &other) const\n> + * \\brief Check if two iterators are equal\n> + */\n> +\n> +/**\n> + * \\fn MetadataList::iterator::operator!=(const iterator &other) const\n> + * \\brief Check if two iterators are not equal\n> + */\n> +\n> +/**\n> + * \\fn MetadataList::iterator::operator++(int)\n> + * \\brief Advance the iterator\n> + */\n> +\n> +/**\n> + * \\fn MetadataList::iterator::operator++()\n> + * \\brief Advance the iterator\n> + */\n> +\n> +/**\n> + * \\class MetadataList::Diff\n> + * \\brief Designates a series of consecutively added metadata items\n> + *\n> + * A Diff object provides a partial view into a MetadataList, it designates\n> + * a series of consecutively added metadata items. Its main purposes is to\n> + * enable applications to receive a list of changes made to a MetadataList.\n> + *\n> + * \\sa Camera::metadataAvailable\n> + * \\internal\n> + * \\sa MetadataList::Checkpoint::diffSince()\n> + */\n> +\n> +/**\n> + * \\fn MetadataList::Diff::list() const\n> + * \\brief Retrieve the associated MetadataList\n> + */\n> +\n> +/**\n> + * \\fn MetadataList::Diff::size() const\n> + * \\brief Retrieve the number of metadata items designated\n> + */\n> +\n> +/**\n> + * \\fn MetadataList::Diff::empty() const\n> + * \\brief Check if any metadata items are designated\n> + */\n> +\n> +/**\n> + * \\fn MetadataList::Diff::operator bool() const\n> + * \\copydoc MetadataList::Diff::empty() const\n> + */\n> +\n> +/**\n> + * \\fn MetadataList::Diff::get(const Control<T> &ctrl) const\n> + * \\copydoc MetadataList::get(const Control<T> &ctrl) const\n> + * \\note The lookup will fail if the metadata item is not designated by this Diff object,\n> + *       even if it is otherwise present in the backing MetadataList.\n> + */\n> +\n> +/**\n> + * \\fn MetadataList::Diff::get(uint32_t tag) const\n> + * \\copydoc MetadataList::get(uint32_t tag) const\n> + * \\note The lookup will fail if the metadata item is not designated by this Diff object,\n> + *       even if it is otherwise present in the backing MetadataList.\n> + */\n> +\n> +/**\n> + * \\fn MetadataList::Diff::begin() const\n> + * \\brief Retrieve the begin iterator\n> + */\n> +\n> +/**\n> + * \\fn MetadataList::Diff::end() const\n> + * \\brief Retrieve the end iterator\n> + */\n> +\n> +/**\n> + * \\internal\n> + * \\class MetadataList::Checkpoint\n> + * \\brief Designates a particular state of a MetadataList\n> + *\n> + * A Checkpoint object designates a point in the stream of metadata items in the associated\n> + * MetadataList. Its main use to be able to retrieve the set of metadata items that were\n> + * added to the list after the designated point using diffSince().\n> + */\n> +\n> +/**\n> + * \\internal\n> + * \\fn MetadataList::Checkpoint::diffSince() const\n> + * \\brief Retrieve the set of metadata items added since the checkpoint was created\n> + */\n> +\n> +} /* namespace libcamera */\n> diff --git a/test/controls/meson.build b/test/controls/meson.build\n> index 763f8905e..ff635454b 100644\n> --- a/test/controls/meson.build\n> +++ b/test/controls/meson.build\n> @@ -5,12 +5,22 @@ control_tests = [\n>      {'name': 'control_info_map', 'sources': ['control_info_map.cpp']},\n>      {'name': 'control_list', 'sources': ['control_list.cpp']},\n>      {'name': 'control_value', 'sources': ['control_value.cpp']},\n> +    {'name': 'metadata_list', 'sources': ['metadata_list.cpp']},\n>  ]\n>\n> +if asan_enabled\n> +    control_tests += {\n> +        'name': 'metadata_list_iter_uaf',\n> +        'sources': ['metadata_list_iter_uaf.cpp'],\n> +        'should_fail': true,\n> +    }\n> +endif\n> +\n>  foreach test : control_tests\n>      exe = executable(test['name'], test['sources'],\n>                       dependencies : libcamera_public,\n>                       link_with : test_libraries,\n>                       include_directories : test_includes_internal)\n> -    test(test['name'], exe, suite : 'controls', is_parallel : false)\n> +    test(test['name'], exe, suite : 'controls', is_parallel : false,\n> +         should_fail : test.get('should_fail', false))\n>  endforeach\n> diff --git a/test/controls/metadata_list.cpp b/test/controls/metadata_list.cpp\n> new file mode 100644\n> index 000000000..b0eddde43\n> --- /dev/null\n> +++ b/test/controls/metadata_list.cpp\n> @@ -0,0 +1,205 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2025, Ideas On Board Oy\n> + *\n> + * MetadataList tests\n> + */\n> +\n> +#include <future>\n> +#include <iostream>\n> +#include <thread>\n> +\n> +#include <libcamera/control_ids.h>\n> +#include <libcamera/metadata_list.h>\n> +#include <libcamera/metadata_list_plan.h>\n> +#include <libcamera/property_ids.h>\n> +\n> +#include \"test.h\"\n> +\n> +using namespace std;\n> +using namespace libcamera;\n> +\n> +#define ASSERT(x) do { \\\n> +\tif (!static_cast<bool>(x)) { \\\n> +\t\tstd::cerr << '`' << #x << \"` failed\" << std::endl; \\\n> +\t\treturn TestFail; \\\n> +\t} \\\n> +} while (false)\n> +\n> +class MetadataListTest : public Test\n> +{\n> +public:\n> +\tMetadataListTest() = default;\n> +\n> +protected:\n> +\tint run() override\n> +\t{\n> +\t\tMetadataListPlan mlp;\n> +\t\tmlp.set(controls::ExposureTime);\n> +\t\tmlp.set(controls::ExposureValue);\n> +\t\tmlp.set(controls::ColourGains);\n> +\t\tmlp.set(controls::AfWindows, 10);\n> +\t\tmlp.set(controls::AeEnable);\n> +\t\tmlp.set(controls::SensorTimestamp);\n> +\n> +\t\tMetadataList ml(mlp);\n> +\n> +\t\t/*\n> +\t\t*`properties::Location` has the same numeric id as `controls::AeEnable` (checked by the `static_assert`\n> +\t\t* below), but they have different types; check that this is detected.\n> +\t\t*/\n> +\t\tstatic_assert(static_cast<unsigned int>(properties::LOCATION) == controls::AE_ENABLE);\n> +\t\tASSERT(ml.set(properties::Location, 0xCDCD) == MetadataList::SetError::TypeMismatch);\n> +\n> +\t\tASSERT(ml.set(controls::AfWindows, std::array<Rectangle, 11>{}) == MetadataList::SetError::SizeMismatch);\n> +\t\tASSERT(ml.set(controls::ColourTemperature, 123) == MetadataList::SetError::UnknownTag);\n> +\n> +\t\tauto f1 = std::async(std::launch::async, [&] {\n> +\t\t\tusing namespace std::chrono_literals;\n> +\n> +\t\t\tstd::this_thread::sleep_for(500ms);\n> +\t\t\tASSERT(ml.set(controls::ExposureTime, 0x1111) == MetadataList::SetError());\n> +\n> +\t\t\tstd::this_thread::sleep_for(500ms);\n> +\t\t\tASSERT(ml.set(controls::ExposureValue, 1) == MetadataList::SetError());\n> +\n> +\t\t\tstd::this_thread::sleep_for(500ms);\n> +\t\t\tASSERT(ml.set(controls::ColourGains, std::array{\n> +\t\t\t\t123.f,\n> +\t\t\t\t456.f\n> +\t\t\t}) == MetadataList::SetError());\n> +\n> +\t\t\tstd::this_thread::sleep_for(500ms);\n> +\t\t\tASSERT(ml.set(controls::AfWindows, std::array{\n> +\t\t\t\tRectangle(),\n> +\t\t\t\tRectangle(1, 2, 3, 4),\n> +\t\t\t\tRectangle(0x1111, 0x2222, 0x3333, 0x4444),\n> +\t\t\t}) == MetadataList::SetError());\n> +\n> +\t\t\treturn TestPass;\n> +\t\t});\n> +\n> +\t\tauto f2 = std::async(std::launch::async, [&] {\n> +\t\t\tfor (;;) {\n> +\t\t\t\tconst auto x = ml.get(controls::ExposureTime);\n> +\t\t\t\tconst auto y = ml.get(controls::ExposureValue);\n> +\t\t\t\tconst auto z = ml.get(controls::ColourGains);\n> +\t\t\t\tconst auto w = ml.get(controls::AfWindows);\n> +\n> +\t\t\t\tif (x)\n> +\t\t\t\t\tASSERT(*x == 0x1111);\n> +\n> +\t\t\t\tif (y)\n> +\t\t\t\t\tASSERT(*y == 1.0f);\n> +\n> +\t\t\t\tif (z) {\n> +\t\t\t\t\tASSERT(z->size() == 2);\n> +\t\t\t\t\tASSERT((*z)[0] == 123.f);\n> +\t\t\t\t\tASSERT((*z)[1] == 456.f);\n> +\t\t\t\t}\n> +\n> +\t\t\t\tif (w) {\n> +\t\t\t\t\tASSERT(w->size() == 3);\n> +\t\t\t\t\tASSERT((*w)[0].isNull());\n> +\t\t\t\t\tASSERT((*w)[1] == Rectangle(1, 2, 3, 4));\n> +\t\t\t\t\tASSERT((*w)[2] == Rectangle(0x1111, 0x2222, 0x3333, 0x4444));\n> +\t\t\t\t}\n> +\n> +\t\t\t\tif (x && y && z && w)\n> +\t\t\t\t\tbreak;\n> +\t\t\t}\n> +\n> +\t\t\treturn TestPass;\n> +\t\t});\n> +\n> +\t\tASSERT(f1.get() == TestPass);\n> +\t\tASSERT(f2.get() == TestPass);\n> +\n> +\t\tASSERT(ml.set(controls::ExposureTime, 0x2222) == MetadataList::SetError::AlreadySet);\n> +\t\tASSERT(ml.set(controls::ExposureValue, 2) == MetadataList::SetError::AlreadySet);\n> +\n> +\t\tASSERT(ml.get(controls::ExposureTime) == 0x1111);\n> +\t\tASSERT(ml.get(controls::ExposureValue) == 1);\n> +\n> +\t\tfor (auto &&[tag, v] : ml)\n> +\t\t\tstd::cout << \"[\" << tag << \"] -> \" << v << '\\n';\n> +\n> +\t\tstd::cout << std::endl;\n> +\n> +\t\t/* Test MetadataList::Diff */\n> +\t\t{\n> +\t\t\tml.clear();\n> +\t\t\tASSERT(ml.empty());\n> +\t\t\tASSERT(ml.size() == 0);\n> +\n> +\t\t\tASSERT(ml.set(controls::ExposureTime, 0x2222) == MetadataList::SetError());\n> +\t\t\tASSERT(ml.get(controls::ExposureTime) == 0x2222);\n> +\n> +\t\t\tauto c = ml.checkpoint();\n> +\n> +\t\t\tASSERT(ml.set(controls::ExposureValue, 2) == MetadataList::SetError());\n> +\t\t\tASSERT(ml.set(controls::SensorTimestamp, 0x99999999) == MetadataList::SetError());\n> +\n> +\t\t\tauto d = c.diffSince();\n> +\t\t\tASSERT(&d.list() == &ml);\n> +\n> +\t\t\tASSERT(ml.set(controls::ColourGains, std::array{ 1.f, 2.f }) == MetadataList::SetError());\n> +\n> +\t\t\tASSERT(d);\n> +\t\t\tASSERT(!d.empty());\n> +\t\t\tASSERT(d.size() == 2);\n> +\t\t\tASSERT(!d.get(controls::ExposureTime));\n> +\t\t\tASSERT(!d.get(controls::ColourGains));\n> +\t\t\tASSERT(!d.get(controls::AfWindows));\n> +\t\t\tASSERT(d.get(controls::ExposureValue) == 2);\n> +\t\t\tASSERT(d.get(controls::SensorTimestamp) == 0x99999999);\n> +\n> +\t\t\tfor (auto &&[tag, v] : d)\n> +\t\t\t\tstd::cout << \"[\" << tag << \"] -> \" << v << '\\n';\n> +\n> +\t\t\t/* Test if iterators work with algorithms. */\n> +\t\t\tstd::ignore = std::find_if(d.begin(), d.end(), [](const auto &) {\n> +\t\t\t\treturn false;\n> +\t\t\t});\n> +\t\t}\n> +\n> +\t\t/* Test transactional behaviour of MetadataList::merge() */\n> +\t\t{\n> +\t\t\tml.clear();\n> +\t\t\tASSERT(ml.empty());\n> +\t\t\tASSERT(ml.size() == 0);\n> +\n> +\t\t\t{\n> +\t\t\t\tControlList cl;\n> +\t\t\t\tcl.set(controls::ExposureTime, 0xFEFE);\n> +\t\t\t\tcl.set(controls::ColourGains, std::array{ 1.1f, 2.2f });\n> +\n> +\t\t\t\tauto d = ml.merge(cl);\n> +\t\t\t\tASSERT(d);\n> +\t\t\t\tASSERT(d->size() == cl.size());\n> +\t\t\t\tASSERT(d->get(controls::ExposureTime) == 0xFEFE);\n> +\t\t\t\tASSERT(ml.size() == d->size());\n> +\t\t\t}\n> +\n> +\t\t\tASSERT(ml.get(controls::ExposureTime) == 0xFEFE);\n> +\n> +\t\t\t{\n> +\t\t\t\tControlList cl;\n> +\t\t\t\tcl.set(999, 999); /* not part of plan */\n> +\t\t\t\tcl.set(controls::ExposureTime, 0xEFEF); /* already set */\n> +\t\t\t\tcl.set(properties::Location, 0xCDCD); /* type mismatch */\n> +\t\t\t\tcl.set(controls::SensorTimestamp, 0xABAB); /* ok */\n> +\n> +\t\t\t\tauto c = ml.checkpoint();\n> +\t\t\t\tauto oldSize = ml.size();\n> +\t\t\t\tASSERT(!ml.merge(cl));\n> +\t\t\t\tASSERT(c.diffSince().empty());\n> +\t\t\t\tASSERT(ml.size() == oldSize);\n> +\t\t\t}\n> +\t\t}\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +};\n> +\n> +TEST_REGISTER(MetadataListTest)\n> diff --git a/test/controls/metadata_list_iter_uaf.cpp b/test/controls/metadata_list_iter_uaf.cpp\n> new file mode 100644\n> index 000000000..66b31136e\n> --- /dev/null\n> +++ b/test/controls/metadata_list_iter_uaf.cpp\n> @@ -0,0 +1,36 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2025, Ideas On Board Oy\n> + *\n> + * MetadataList tests\n> + */\n> +\n> +#include <libcamera/control_ids.h>\n> +#include <libcamera/metadata_list.h>\n> +#include <libcamera/metadata_list_plan.h>\n> +#include <libcamera/property_ids.h>\n> +\n> +#include \"test.h\"\n> +\n> +using namespace std;\n> +using namespace libcamera;\n> +\n> +class MetadataListIterUAFTest : public Test\n> +{\n> +public:\n> +\tMetadataListIterUAFTest() = default;\n> +\n> +protected:\n> +\tint run() override\n> +\t{\n> +\t\tMetadataListPlan mlp;\n> +\t\tmlp.set(controls::AeEnable);\n> +\n> +\t\tMetadataList ml(mlp);\n> +\t\tstd::ignore = *ml.begin(); /* Trigger ASAN. */\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +};\n> +\n> +TEST_REGISTER(MetadataListIterUAFTest)\n> --\n> 2.52.0\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 9C519C3226\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 16 Jan 2026 15:56:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D148661FBC;\n\tFri, 16 Jan 2026 16:56:40 +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 114FB615B2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 16 Jan 2026 16:56:39 +0100 (CET)","from ideasonboard.com (static.170.20.224.46.clients.your-server.de\n\t[46.224.20.170])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 88053352;\n\tFri, 16 Jan 2026 16:56:10 +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=\"NS9YYfeC\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768578970;\n\tbh=/xTCv28ZDCew8KKomFAGtM/7CIg2DiNfHXB0ug39w3Q=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=NS9YYfeC0P6LqhlBmYWtNUxAjQ7PrKndM8t0EnrureSTqKdPR+Cd1Jv6wpRXdWoZQ\n\tr4FpXjhr2Zc09af5vwWIj0VwlRSojZx2tAAeyKG39FRkymwF4lda2paM3f+RmNfoOd\n\tVtbZ15KtYZ4SiNo9B55dXXL0uCi97nTHwycZNtsM=","Date":"Fri, 16 Jan 2026 16:56:35 +0100","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v4 07/22] libcamera: Add `MetadataList`","Message-ID":"<aWj-Kfp1WbpeTpIY@zed>","References":"<20260106165754.1759831-1-barnabas.pocze@ideasonboard.com>\n\t<20260106165754.1759831-8-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":"<20260106165754.1759831-8-barnabas.pocze@ideasonboard.com>","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>"}}]