[{"id":38736,"web_url":"https://patchwork.libcamera.org/comment/38736/","msgid":"<9dae5d30-38a7-4e80-ae1d-1417a73c1d39@ideasonboard.com>","date":"2026-05-06T07:48:44","subject":"Re: [PATCH 2/3] ipa: libipa: Introduce V4L2Stats","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 05. 05. 18:11 keltezéssel, Jacopo Mondi írta:\n> Add a V4L2Stats class similar in spirit to the existing V4L2Params\n> class to allow IPA modules to easily sub-class it to access ISP\n> statistics blocks serialized into a v4l2_isp_buffer.\n> \n> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> ---\n>   src/ipa/libipa/meson.build    |   2 +\n>   src/ipa/libipa/v4l2_stats.cpp | 226 ++++++++++++++++++++++++++++++++++++++++++\n>   src/ipa/libipa/v4l2_stats.h   | 124 +++++++++++++++++++++++\n>   3 files changed, 352 insertions(+)\n> \n> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build\n> index 963c5ee73063..16f4b095f220 100644\n> --- a/src/ipa/libipa/meson.build\n> +++ b/src/ipa/libipa/meson.build\n> @@ -19,6 +19,7 @@ libipa_headers = files([\n>       'pwl.h',\n>       'quantized.h',\n>       'v4l2_params.h',\n> +    'v4l2_stats.h',\n>   ])\n>   \n>   libipa_sources = files([\n> @@ -40,6 +41,7 @@ libipa_sources = files([\n>       'pwl.cpp',\n>       'quantized.cpp',\n>       'v4l2_params.cpp',\n> +    'v4l2_stats.cpp',\n>   ])\n>   \n>   libipa_includes = include_directories('..')\n> diff --git a/src/ipa/libipa/v4l2_stats.cpp b/src/ipa/libipa/v4l2_stats.cpp\n> new file mode 100644\n> index 000000000000..050e2329dfa9\n> --- /dev/null\n> +++ b/src/ipa/libipa/v4l2_stats.cpp\n> @@ -0,0 +1,226 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2026, Ideas On Board\n> + *\n> + * V4L2 Statistics\n> + */\n> +\n> +#include \"v4l2_stats.h\"\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(V4L2Stats)\n\nThe `V4L2Params` category is declared in the `ipa` namespace,\nso I'd do the same with this as well.\n\n\n> +\n> +namespace ipa {\n> +\n> [...]\n> +\n> +} /* namespace ipa */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/libipa/v4l2_stats.h b/src/ipa/libipa/v4l2_stats.h\n> new file mode 100644\n> index 000000000000..404cb606e135\n> --- /dev/null\n> +++ b/src/ipa/libipa/v4l2_stats.h\n> @@ -0,0 +1,124 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2026, Ideas On Board\n> + *\n> + * V4L2 Stats\n> + */\n> +\n> +#pragma once\n> +\n> +#include <stdint.h>\n> +\n> +#include <linux/media/v4l2-isp.h>\n> +\n> +#include <libcamera/base/log.h>\n> +#include <libcamera/base/span.h>\n> +\n> +namespace libcamera {\n> +\n> +LOG_DECLARE_CATEGORY(V4L2Stats)\n> +\n> +namespace ipa {\n> +\n> +template<typename T>\n> +class V4L2StatsBlock\n> +{\n> +public:\n> +\tV4L2StatsBlock(const Span<const uint8_t> data)\n> +\t\t: data_(data)\n> +\t{\n> +\t}\n> +\n> +\tvirtual ~V4L2StatsBlock() {}\n\nIn the `V4L2Params` case, `virtual` was used to handle the differences\nin the rkisp1 case as far as I can recall. Is it necessary here?\n\n\n> +\n> +\tvirtual const T *operator->() const\n> +\t{\n> +\t\treturn reinterpret_cast<const T *>(data_.data());\n> +\t}\n> +\n> +\tvirtual const T &operator*() const\n> +\t{\n> +\t\treturn *reinterpret_cast<const T *>(data_.data());\n> +\t}\n> +\n> +\tsize_t size() const\n> +\t{\n> +\t\treturn data_.size();\n> +\t}\n> +\n> +protected:\n> +\tSpan<const uint8_t> data_;\n> +};\n> +\n> +template<typename Traits>\n> +class V4L2Stats\n> +{\n> +public:\n> +\tV4L2Stats(Span<uint8_t> data, unsigned int version)\n> +\t\t: data_(data)\n> +\t{\n> +\t\tstruct v4l2_isp_buffer *stats =\n> +\t\t\treinterpret_cast<struct v4l2_isp_buffer *>(data_.data());\n> +\t\tused_ = stats->data_size;\n> +\n> +\t\tif (version != stats->version)\n> +\t\t\tLOG(V4L2Stats, Error)\n> +\t\t\t\t<< \"Unsupported v4l2-isp version: \" << stats->version;\n> +\t}\n> +\n> +\tsize_t bytesused() const { return used_; }\n> +\n> +\ttemplate<typename Traits::id_type Id>\n> +\tauto block() const\n> +\t{\n> +\t\tusing Details = typename Traits::template id_to_details<Id>;\n> +\n> +\t\tusing Type = typename Details::type;\n> +\t\tconstexpr auto kernelId = Details::blockType;\n> +\n> +\t\tauto data = block(kernelId, sizeof(Type));\n> +\t\treturn V4L2StatsBlock<Type>(data);\n\nMaybe we could use an `std::optional`? Or does the kernel have some guarantees\nabout when it will most definitely provide a block?\n\n\n> +\t}\n> +\n> +protected:\n> +\tconst Span<const uint8_t> block(unsigned int blockType, size_t blockSize) const\n> +\t{\n> +\t\tstruct v4l2_isp_buffer *stats =\n> +\t\t\treinterpret_cast<struct v4l2_isp_buffer *>(data_.data());\n> +\n> +\t\t__u8 *data = stats->data;\n> +\t\twhile (data < stats->data + stats->data_size) {\n> +\t\t\tstruct v4l2_isp_block_header *header =\n> +\t\t\t\treinterpret_cast<struct v4l2_isp_block_header *>(data);\n> +\n> +\t\t\tif (header->type != blockType) {\n> +\t\t\t\tdata += header->size;\n> +\t\t\t\tcontinue;\n> +\t\t\t}\n> +\n> +\t\t\tif (header->size != blockSize) {\n> +\t\t\t\tLOG(V4L2Stats, Error)\n> +\t\t\t\t\t<< \"Block type \" << blockType\n> +\t\t\t\t\t<< \" size mistmatch: expected \"\n> +\t\t\t\t\t<< blockSize << \" got:\"\n> +\t\t\t\t\t<< header->size;\n> +\t\t\t\treturn {};\n> +\t\t\t}\n> +\n> +\t\t\tSpan<const uint8_t> block(data, header->size);\n> +\t\t\treturn block;\n> +\t\t}\n> +\n> +\t\tLOG(V4L2Stats, Error) << \"Unsupported stats block type: \"\n> +\t\t\t\t      << blockType;\n> +\n> +\t\treturn {};\n> +\t}\n> +\n> +\tSpan<uint8_t> data_;\n> +\tsize_t used_;\n> +};\n> +\n> +} /* namespace ipa */\n> +\n> +} /* namespace libcamera */\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 8A6EDBE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  6 May 2026 07:49:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3841A63021;\n\tWed,  6 May 2026 09:49:04 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 512A362FE1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  6 May 2026 09:49:03 +0200 (CEST)","from [192.168.33.81] (185.221.140.217.nat.pool.zt.hu\n\t[185.221.140.217])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C481A78E;\n\tWed,  6 May 2026 09:48:47 +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=\"MbPkyf+Z\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1778053740;\n\tbh=QkJp9rtO+jX1BaGaB6sch/akxsNEJgqWrDQWACAZXj8=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=MbPkyf+Z7cxKNM+oHHM2qk0/qQ9Z3eiVG7qkLfu7Kx+REHOkj7KAVnfa1FJnJ+S0a\n\tz4smyLf2r5pf7erBXOY9fh5Lm8phl9MrmTfw2oPCYSahSO4FlbowVrsuGUR4dhfekV\n\tfzfg7Q+2q0zg9GrBPmmUVZVEzBKCw1ZBUHv044DM=","Message-ID":"<9dae5d30-38a7-4e80-ae1d-1417a73c1d39@ideasonboard.com>","Date":"Wed, 6 May 2026 09:48:44 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 2/3] ipa: libipa: Introduce V4L2Stats","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org,\n\tAntoine Bouyer <antoine.bouyer@nxp.com>","References":"<20260505-extensible-stats-v1-0-0b56c7b1bbd6@ideasonboard.com>\n\t<20260505-extensible-stats-v1-2-0b56c7b1bbd6@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":"<20260505-extensible-stats-v1-2-0b56c7b1bbd6@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":38738,"web_url":"https://patchwork.libcamera.org/comment/38738/","msgid":"<177805767772.3225262.11411075376872468861@ping.linuxembedded.co.uk>","date":"2026-05-06T08:54:37","subject":"Re: [PATCH 2/3] ipa: libipa: Introduce V4L2Stats","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jacopo Mondi (2026-05-05 17:11:10)\n> Add a V4L2Stats class similar in spirit to the existing V4L2Params\n> class to allow IPA modules to easily sub-class it to access ISP\n> statistics blocks serialized into a v4l2_isp_buffer.\n> \n> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> ---\n>  src/ipa/libipa/meson.build    |   2 +\n>  src/ipa/libipa/v4l2_stats.cpp | 226 ++++++++++++++++++++++++++++++++++++++++++\n>  src/ipa/libipa/v4l2_stats.h   | 124 +++++++++++++++++++++++\n>  3 files changed, 352 insertions(+)\n> \n> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build\n> index 963c5ee73063..16f4b095f220 100644\n> --- a/src/ipa/libipa/meson.build\n> +++ b/src/ipa/libipa/meson.build\n> @@ -19,6 +19,7 @@ libipa_headers = files([\n>      'pwl.h',\n>      'quantized.h',\n>      'v4l2_params.h',\n> +    'v4l2_stats.h',\n>  ])\n>  \n>  libipa_sources = files([\n> @@ -40,6 +41,7 @@ libipa_sources = files([\n>      'pwl.cpp',\n>      'quantized.cpp',\n>      'v4l2_params.cpp',\n> +    'v4l2_stats.cpp',\n>  ])\n>  \n>  libipa_includes = include_directories('..')\n> diff --git a/src/ipa/libipa/v4l2_stats.cpp b/src/ipa/libipa/v4l2_stats.cpp\n> new file mode 100644\n> index 000000000000..050e2329dfa9\n> --- /dev/null\n> +++ b/src/ipa/libipa/v4l2_stats.cpp\n> @@ -0,0 +1,226 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2026, Ideas On Board\n> + *\n> + * V4L2 Statistics\n> + */\n> +\n> +#include \"v4l2_stats.h\"\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(V4L2Stats)\n> +\n> +namespace ipa {\n> +\n> +/**\n> + * \\file v4l2_stats.cpp\n> + * \\brief Helper class to handle an ISP statistics buffer compatible with\n> + * the generic V4L2 ISP format\n> + *\n> + * The Linux kernel defines a generic buffer format for ISP statistics.\n> + * The format describes a serialisation method that allows userspace to\n> + * access statistics data from a binary buffer.\n> + *\n> + * The V4L2Stats class implements support the V4L2 ISP statistics buffer format\n\nsupport for the\n\n> + * and allows users to retrieve an ISP statistics blocks, represented as\n\nas a V4L2StatsBlock\n\n> + * V4L2StatsBlock class instances.\n> + *\n> + * IPA implementations using this helpers should define an enumeration of ISP\n\nusing this helper\n\nor\n\nusing these helpers\n\n> + * blocks supported by the IPA module and use a set of common abstraction to\n\nabstractions\n\n> + * help their derived implementation of V4L2Stats translate the enumerated ISP\n> + * block identifiers to the actual type of the statistics data as defined by\n> + * the kernel interface.\n> + */\n> +\n> +/**\n> + * \\class V4L2StatsBlock\n> + * \\brief Helper class that represents an ISP statistics block\n> + *\n> + * Each ISP measurement block produces a set of statistics data whose memory\n> + * layout is defined by the kernel interface.\n> + *\n> + * This class represents an ISP statistics block entry. It is constructed\n> + * with a reference to the memory area where the statistics block is\n> + * stored in the statistics buffer. The template parameter represents\n> + * the underlying kernel-defined ISP statistics block type and allows its\n> + * user to easily cast it to said type to read the statistics data.\n> + *\n> + * \\sa V4L2Stats::block()\n> + */\n> +\n> +/**\n> + * \\fn V4L2StatsBlock::V4L2StatsBlock()\n> + * \\brief Construct a V4L2StatsBlock with memory represented by \\a data\n> + * \\param[in] data The memory area where the statistics block is located\n> + */\n> +\n> +/**\n> + * \\fn V4L2StatsBlock::operator->() const\n> + * \\brief Access the statistics block casting it to the kernel-defined\n> + * ISP block type\n> + *\n> + * The V4L2StatsBlock is templated with the kernel defined ISP block type. This\n> + * function allows users to easily cast a V4L2StatsBlock to the underlying\n> + * kernel-defined type in order to easily access the statistics data.\n> + *\n> + * \\code{.cpp}\n> + *\n> + * // The kernel header defines the ISP statistics types, in example\n\nfor example:\n\n> + * // struct my_isp_awb_stats_data {\n> + * //          u16 mean_r;\n> + * //          u16 mean_g;\n> + * //          u16 mean_b;\n> + * //  }\n> + *\n> + * template<> V4L2StatsBlock<struct my_isp_awb_stats_data> awbStats = ...\n> + *\n> + * u16 mean_r = awbStats->mean_r;\n> + * u16 mean_g = awbStats->mean_g;\n> + * u16 mean_b = awbStats->mean_b;\n> + *\n> + * \\endcode\n> + *\n> + * Users of this class shall not create a V4L2StatsBlock manually but should\n> + * use V4L2Stats::block().\n> + */\n> +\n> +/**\n> + * \\fn V4L2StatsBlock::operator*() const\n> + * \\copydoc V4L2StatsBlock::operator->() const\n> + */\n> +\n> +/**\n> + * \\fn V4L2StatsBlock::size() const\n> + * \\brief Retrieve the size (in bytes) of the ISP statistics block, including\n> + * the block's header\n> + * \\return The size of the stats block\n> + */\n> +\n> +/**\n> + * \\var V4L2StatsBlock::data_\n> + * \\brief Memory area where ISP statistics have been serialized\n> + */\n> +\n> +/**\n> + * \\class V4L2Stats\n> + * \\brief Helper class that represent an ISP statistics buffer\n> + *\n> + * This class represents an ISP statistics buffer. It is constructed with a\n> + * reference to the memory mapped buffer that has been dequeued from the ISP\n> + * driver.\n> + *\n> + * This class is templated with the type of the enumeration of ISP blocks that\n> + * each IPA module is expected to support. IPA modules are expected to derive\n> + * this class by providing a 'stats_traits' type that helps the class associate\n> + * a block type with the actual memory area that represents the ISP statistics\n> + * block.\n> + *\n> + * \\code{.cpp}\n> + *\n> + * // Define the supported ISP statistics blocks\n> + * enum class myISPStats {\n> + *     Agc,\n> + *     Awb,\n> + *     ...\n> + * };\n> + *\n> + * // Maps the C++ enum type to the kernel enum type and concrete parameter type\n> + * template<myISPStats B>\n> + * struct block_type {\n> + * };\n> + *\n> + * template<>\n> + * struct block_type<myISPStats::Agc> {\n> + *     using type = struct my_isp_kernel_stats_type_agc;\n> + *     static constexpr kernel_enum_type blockType = MY_ISP_STATS_TYPE_AGC;\n> + * };\n> + *\n> + * template<>\n> + * struct block_type<myISPStats::Awb> {\n> + *     using type = struct my_isp_kernel_stats_type_awb;\n> + *     static constexpr kernel_enum_type blockType = MY_ISP_STATS_TYPE_AWB;\n> + * };\n> + *\n> + *\n> + * // Convenience type to associate a block id to the 'block_type' overload\n> + * struct stats_traits {\n> + *     using id_type = myISPStats;\n> + *     template<id_type Id> using id_to_details = block_type<Id>;\n> + * };\n> + *\n> + * ...\n> + *\n> + * // Derive the V4L2Stats class by providing stats_traits\n> + * class MyISPStats : public V4L2Stats<stats_traits>\n> + * {\n> + * public:\n> + *     MyISPStats::MyISPStats(Span<uint8_t> data)\n> + *             : V4L2Stats(data)\n> + *     {\n> + *     }\n> + * };\n> + *\n> + * \\endcode\n> + *\n> + * Users of this class can then easily access an ISP statistics block as a\n> + * V4L2StatsBlock instance.\n> + *\n> + * \\code{.cpp}\n> + *\n> + * MyISPStats stats(data);\n> + *\n> + * auto awb = stats.block<myISPStats::AWB>();\n> + * auto mean_r = awb->mean_r;\n> + * auto mean_g = awb->mean_g;\n> + * auto mean_b = awb->mean_b;\n> + * \\endcode\n> + */\n> +\n> +/**\n> + * \\fn V4L2Stats::V4L2Stats()\n> + * \\brief Construct an instance of V4L2Stats\n> + * \\param[in] data Reference to the v4l2-buffer memory mapped area\n> + * \\param[in] version The expected V4L2 ISP serialization format version\n> + */\n> +\n> +/**\n> + * \\fn V4L2Stats::bytesused()\n> + * \\brief Retrieve the size of the statistics buffer (in bytes)\n> + * \\return The number of bytes occupied by the ISP statistics\n> + */\n> +\n> +/**\n> + * \\fn V4L2Stats::block() const\n> + * \\brief Retrieve the location of an ISP statistics block a return it\n\nand return it ?\n\nOr just simply \"Retrieve the location of an ISP statistics block\"\n\n> + * \\return A V4L2StatsBlock instance that points to the ISP statistics block\n> + */\n> +\n> +/**\n> + * \\fn V4L2Stats::block(unsigned int blockType, size_t blockSize) const\n> + * \\brief Retrieve an ISP statistics block a returns a reference to it\n> + * \\param[in] blockType The kernel-defined ISP block identifier, used to\n> + * identify the block header\n> + * \\param[in] blockSize The ISP statistics block size, for validation\n> + *\n> + * Parse the buffer of serialized statistics data and retrieve the location\n> + * of the block with type \\a blockType of size \\a blockSize.\n> + *\n> + * IPA modules that derive the V4L2Stats class shall use this function to\n> + * retrieve the memory area that will be used to construct a V4L2StatsBlock<T>\n> + * before returning it to the caller.\n> + */\n> +\n> +/**\n> + * \\var V4L2Stats::data_\n> + * \\brief The ISP statistics buffer memory\n> + */\n> +\n> +/**\n> + * \\var V4L2Stats::used_\n> + * \\brief The number of bytes used by the statistics buffer\n> + */\n> +\n> +} /* namespace ipa */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/libipa/v4l2_stats.h b/src/ipa/libipa/v4l2_stats.h\n> new file mode 100644\n> index 000000000000..404cb606e135\n> --- /dev/null\n> +++ b/src/ipa/libipa/v4l2_stats.h\n> @@ -0,0 +1,124 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2026, Ideas On Board\n> + *\n> + * V4L2 Stats\n> + */\n> +\n> +#pragma once\n> +\n> +#include <stdint.h>\n> +\n> +#include <linux/media/v4l2-isp.h>\n> +\n> +#include <libcamera/base/log.h>\n> +#include <libcamera/base/span.h>\n> +\n> +namespace libcamera {\n> +\n> +LOG_DECLARE_CATEGORY(V4L2Stats)\n> +\n> +namespace ipa {\n> +\n> +template<typename T>\n> +class V4L2StatsBlock\n> +{\n> +public:\n> +       V4L2StatsBlock(const Span<const uint8_t> data)\n> +               : data_(data)\n> +       {\n> +       }\n> +\n> +       virtual ~V4L2StatsBlock() {}\n> +\n> +       virtual const T *operator->() const\n\nAre these operators really virtual? Will they be overridden by other\nimplementations? It seems like they just access data_ and perform the\ncast ... so I don't think they need to go through the vtable ?\n\n> +       {\n> +               return reinterpret_cast<const T *>(data_.data());\n> +       }\n> +\n> +       virtual const T &operator*() const\n> +       {\n> +               return *reinterpret_cast<const T *>(data_.data());\n> +       }\n> +\n> +       size_t size() const\n> +       {\n> +               return data_.size();\n> +       }\n> +\n> +protected:\n> +       Span<const uint8_t> data_;\n> +};\n> +\n> +template<typename Traits>\n> +class V4L2Stats\n> +{\n> +public:\n> +       V4L2Stats(Span<uint8_t> data, unsigned int version)\n> +               : data_(data)\n> +       {\n> +               struct v4l2_isp_buffer *stats =\n> +                       reinterpret_cast<struct v4l2_isp_buffer *>(data_.data());\n> +               used_ = stats->data_size;\n> +\n> +               if (version != stats->version)\n> +                       LOG(V4L2Stats, Error)\n> +                               << \"Unsupported v4l2-isp version: \" << stats->version;\n> +       }\n> +\n> +       size_t bytesused() const { return used_; }\n> +\n> +       template<typename Traits::id_type Id>\n> +       auto block() const\n> +       {\n> +               using Details = typename Traits::template id_to_details<Id>;\n> +\n> +               using Type = typename Details::type;\n> +               constexpr auto kernelId = Details::blockType;\n> +\n> +               auto data = block(kernelId, sizeof(Type));\n> +               return V4L2StatsBlock<Type>(data);\n> +       }\n> +\n> +protected:\n> +       const Span<const uint8_t> block(unsigned int blockType, size_t blockSize) const\n> +       {\n> +               struct v4l2_isp_buffer *stats =\n> +                       reinterpret_cast<struct v4l2_isp_buffer *>(data_.data());\n> +\n> +               __u8 *data = stats->data;\n> +               while (data < stats->data + stats->data_size) {\n> +                       struct v4l2_isp_block_header *header =\n> +                               reinterpret_cast<struct v4l2_isp_block_header *>(data);\n> +\n> +                       if (header->type != blockType) {\n> +                               data += header->size;\n> +                               continue;\n> +                       }\n> +\n> +                       if (header->size != blockSize) {\n> +                               LOG(V4L2Stats, Error)\n> +                                       << \"Block type \" << blockType\n> +                                       << \" size mistmatch: expected \"\n> +                                       << blockSize << \" got:\"\n> +                                       << header->size;\n> +                               return {};\n> +                       }\n> +\n> +                       Span<const uint8_t> block(data, header->size);\n> +                       return block;\n\nDoes this work?\n\t\t\treturn {data, header->size};\n\nor this still fits on one line?\n\t\t\treturn Span<const uint8_t>(data, header->size);\n\n\nOtherwise, nothing specifically major stands out in any of that for me,\nso if this gets us a starting point to use the new kernel interface:\n\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n\n> +               }\n> +\n> +               LOG(V4L2Stats, Error) << \"Unsupported stats block type: \"\n> +                                     << blockType;\n> +\n> +               return {};\n> +       }\n> +\n> +       Span<uint8_t> data_;\n> +       size_t used_;\n> +};\n> +\n> +} /* namespace ipa */\n> +\n> +} /* namespace libcamera */\n> \n> -- \n> 2.53.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 8E724BE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  6 May 2026 08:54:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4C3286301E;\n\tWed,  6 May 2026 10:54:41 +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 39D2A62FE1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  6 May 2026 10:54:40 +0200 (CEST)","from monstersaurus.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 F178D63D;\n\tWed,  6 May 2026 10:54:36 +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=\"pYiQUqfl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1778057677;\n\tbh=m3xba4nKsfYrEQs7XsIZiU22CXnNALoqHhNX4gPQosY=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=pYiQUqflCLmXKAbMly8TALe/yhbfdLRFXI1kBsOJCcLv/EQ5dCTwVe7swjvSrizuF\n\tuUa52IJsMth6oBh3pYbpWSC8BHvZgkMFBqBRP/FMb6msLzE0S970feXtLuMOMLFw7W\n\tAk5E6Ddc7jxFq8ChrWLKbeZu2ivHX44WjDhJ+i1c=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20260505-extensible-stats-v1-2-0b56c7b1bbd6@ideasonboard.com>","References":"<20260505-extensible-stats-v1-0-0b56c7b1bbd6@ideasonboard.com>\n\t<20260505-extensible-stats-v1-2-0b56c7b1bbd6@ideasonboard.com>","Subject":"Re: [PATCH 2/3] ipa: libipa: Introduce V4L2Stats","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Antoine Bouyer <antoine.bouyer@nxp.com>,\n\tJacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 06 May 2026 09:54:37 +0100","Message-ID":"<177805767772.3225262.11411075376872468861@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":38762,"web_url":"https://patchwork.libcamera.org/comment/38762/","msgid":"<afxNd0ZL-E2OvT3d@zed>","date":"2026-05-07T08:56:26","subject":"Re: [PATCH 2/3] ipa: libipa: Introduce V4L2Stats","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 Wed, May 06, 2026 at 09:48:44AM +0200, Barnabás Pőcze wrote:\n> 2026. 05. 05. 18:11 keltezéssel, Jacopo Mondi írta:\n> > Add a V4L2Stats class similar in spirit to the existing V4L2Params\n> > class to allow IPA modules to easily sub-class it to access ISP\n> > statistics blocks serialized into a v4l2_isp_buffer.\n> >\n> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > ---\n> >   src/ipa/libipa/meson.build    |   2 +\n> >   src/ipa/libipa/v4l2_stats.cpp | 226 ++++++++++++++++++++++++++++++++++++++++++\n> >   src/ipa/libipa/v4l2_stats.h   | 124 +++++++++++++++++++++++\n> >   3 files changed, 352 insertions(+)\n> >\n> > diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build\n> > index 963c5ee73063..16f4b095f220 100644\n> > --- a/src/ipa/libipa/meson.build\n> > +++ b/src/ipa/libipa/meson.build\n> > @@ -19,6 +19,7 @@ libipa_headers = files([\n> >       'pwl.h',\n> >       'quantized.h',\n> >       'v4l2_params.h',\n> > +    'v4l2_stats.h',\n> >   ])\n> >   libipa_sources = files([\n> > @@ -40,6 +41,7 @@ libipa_sources = files([\n> >       'pwl.cpp',\n> >       'quantized.cpp',\n> >       'v4l2_params.cpp',\n> > +    'v4l2_stats.cpp',\n> >   ])\n> >   libipa_includes = include_directories('..')\n> > diff --git a/src/ipa/libipa/v4l2_stats.cpp b/src/ipa/libipa/v4l2_stats.cpp\n> > new file mode 100644\n> > index 000000000000..050e2329dfa9\n> > --- /dev/null\n> > +++ b/src/ipa/libipa/v4l2_stats.cpp\n> > @@ -0,0 +1,226 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2026, Ideas On Board\n> > + *\n> > + * V4L2 Statistics\n> > + */\n> > +\n> > +#include \"v4l2_stats.h\"\n> > +\n> > +namespace libcamera {\n> > +\n> > +LOG_DEFINE_CATEGORY(V4L2Stats)\n>\n> The `V4L2Params` category is declared in the `ipa` namespace,\n> so I'd do the same with this as well.\n>\n\nack\n\n>\n> > +\n> > +namespace ipa {\n> > +\n> > [...]\n> > +\n> > +} /* namespace ipa */\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/ipa/libipa/v4l2_stats.h b/src/ipa/libipa/v4l2_stats.h\n> > new file mode 100644\n> > index 000000000000..404cb606e135\n> > --- /dev/null\n> > +++ b/src/ipa/libipa/v4l2_stats.h\n> > @@ -0,0 +1,124 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2026, Ideas On Board\n> > + *\n> > + * V4L2 Stats\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include <stdint.h>\n> > +\n> > +#include <linux/media/v4l2-isp.h>\n> > +\n> > +#include <libcamera/base/log.h>\n> > +#include <libcamera/base/span.h>\n> > +\n> > +namespace libcamera {\n> > +\n> > +LOG_DECLARE_CATEGORY(V4L2Stats)\n> > +\n> > +namespace ipa {\n> > +\n> > +template<typename T>\n> > +class V4L2StatsBlock\n> > +{\n> > +public:\n> > +\tV4L2StatsBlock(const Span<const uint8_t> data)\n> > +\t\t: data_(data)\n> > +\t{\n> > +\t}\n> > +\n> > +\tvirtual ~V4L2StatsBlock() {}\n>\n> In the `V4L2Params` case, `virtual` was used to handle the differences\n> in the rkisp1 case as far as I can recall. Is it necessary here?\n>\n\nProbably not\n\n>\n> > +\n> > +\tvirtual const T *operator->() const\n> > +\t{\n> > +\t\treturn reinterpret_cast<const T *>(data_.data());\n> > +\t}\n> > +\n> > +\tvirtual const T &operator*() const\n> > +\t{\n> > +\t\treturn *reinterpret_cast<const T *>(data_.data());\n> > +\t}\n\nand has Kieran has pointed out, these shouldn't probably be virtual as\nwell\n\n> > +\n> > +\tsize_t size() const\n> > +\t{\n> > +\t\treturn data_.size();\n> > +\t}\n> > +\n> > +protected:\n> > +\tSpan<const uint8_t> data_;\n> > +};\n> > +\n> > +template<typename Traits>\n> > +class V4L2Stats\n> > +{\n> > +public:\n> > +\tV4L2Stats(Span<uint8_t> data, unsigned int version)\n> > +\t\t: data_(data)\n> > +\t{\n> > +\t\tstruct v4l2_isp_buffer *stats =\n> > +\t\t\treinterpret_cast<struct v4l2_isp_buffer *>(data_.data());\n> > +\t\tused_ = stats->data_size;\n> > +\n> > +\t\tif (version != stats->version)\n> > +\t\t\tLOG(V4L2Stats, Error)\n> > +\t\t\t\t<< \"Unsupported v4l2-isp version: \" << stats->version;\n> > +\t}\n> > +\n> > +\tsize_t bytesused() const { return used_; }\n> > +\n> > +\ttemplate<typename Traits::id_type Id>\n> > +\tauto block() const\n> > +\t{\n> > +\t\tusing Details = typename Traits::template id_to_details<Id>;\n> > +\n> > +\t\tusing Type = typename Details::type;\n> > +\t\tconstexpr auto kernelId = Details::blockType;\n> > +\n> > +\t\tauto data = block(kernelId, sizeof(Type));\n> > +\t\treturn V4L2StatsBlock<Type>(data);\n>\n> Maybe we could use an `std::optional`? Or does the kernel have some guarantees\n> about when it will most definitely provide a block?\n\nlet's say that if you configure the ISP to produce statistics for a block,\nthen the stats should be there. But I guess it makes sense to assume a\nuser of this class wants to check if there actually are stats for the\nblock it desires in the buffer.\n\nHowever, using std::optional<> really gets in the way of the using\nV4L2StatsBlock::operator-> and V4L2StatsBlock::operator* as a user\nwould have to do something like\n\n        auto opt = stats->block<Type>();\n        if (opt) {\n                auto actualStats = *opt;\n\n                int32_t s = actualStats->s;\n        }\n\nIsn't it enough for users of this class to check the size of the\nreturned V4L2StatsBlock to discern if there are stats available for\nthat block or not ?\n\n        auto stat = stats->block<Type>();\n        if (stat.size() > 0) {\n                int32_t s = stat->s;\n        }\n\n?\n\n(that's actually what happens already in the out-of-tree pipeline that\nuses V4L2Stats I have)\n\n\tconst auto postHist = stats->block<StatsType::PostHist>();\n\tif (postHist.size() == 0) {\n\t\tfillMetadata(context, frameContext, metadata);\n\t\tLOG(RppX1Agc, Error) << \"HIST data is missing in statistics\";\n\t\treturn;\n\t}\n\n\tconst auto exm = stats->block<StatsType::Pre1Exm>();\n\tif (exm.size() == 0) {\n\t\tfillMetadata(context, frameContext, metadata);\n\t\tLOG(RppX1Agc, Error) << \"EXM data is missing in statistics\";\n\t\treturn;\n\t}\n\n>\n>\n> > +\t}\n> > +\n> > +protected:\n> > +\tconst Span<const uint8_t> block(unsigned int blockType, size_t blockSize) const\n> > +\t{\n> > +\t\tstruct v4l2_isp_buffer *stats =\n> > +\t\t\treinterpret_cast<struct v4l2_isp_buffer *>(data_.data());\n> > +\n> > +\t\t__u8 *data = stats->data;\n> > +\t\twhile (data < stats->data + stats->data_size) {\n> > +\t\t\tstruct v4l2_isp_block_header *header =\n> > +\t\t\t\treinterpret_cast<struct v4l2_isp_block_header *>(data);\n> > +\n> > +\t\t\tif (header->type != blockType) {\n> > +\t\t\t\tdata += header->size;\n> > +\t\t\t\tcontinue;\n> > +\t\t\t}\n> > +\n> > +\t\t\tif (header->size != blockSize) {\n> > +\t\t\t\tLOG(V4L2Stats, Error)\n> > +\t\t\t\t\t<< \"Block type \" << blockType\n> > +\t\t\t\t\t<< \" size mistmatch: expected \"\n> > +\t\t\t\t\t<< blockSize << \" got:\"\n> > +\t\t\t\t\t<< header->size;\n> > +\t\t\t\treturn {};\n> > +\t\t\t}\n> > +\n> > +\t\t\tSpan<const uint8_t> block(data, header->size);\n> > +\t\t\treturn block;\n> > +\t\t}\n> > +\n> > +\t\tLOG(V4L2Stats, Error) << \"Unsupported stats block type: \"\n> > +\t\t\t\t      << blockType;\n> > +\n> > +\t\treturn {};\n> > +\t}\n> > +\n> > +\tSpan<uint8_t> data_;\n> > +\tsize_t used_;\n> > +};\n> > +\n> > +} /* namespace ipa */\n> > +\n> > +} /* namespace libcamera */\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 300B0BDCB5\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  7 May 2026 08:56:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0EE5A63025;\n\tThu,  7 May 2026 10:56:32 +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 60AB462FB1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  7 May 2026 10:56:30 +0200 (CEST)","from ideasonboard.com (net-93-65-100-155.cust.vodafonedsl.it\n\t[93.65.100.155])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2F9A1664;\n\tThu,  7 May 2026 10:56:26 +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=\"hDWcRvnq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1778144186;\n\tbh=n6gxRZ+ZWagveZExwVbhM4Xwf1bTbB5+W5RjonVUoBw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=hDWcRvnqSqbfTZT/QIT7jfIPm8Mg/0NFulBkcY5U2ZrJz1jYCuoiv67Rswv+2HA4M\n\tq48g6zM92gYAQ4sgUq6Z5GflBLhU/42WV3n7P2YVg4rXiP7Rgh5YdyRe6/aBok6AP4\n\tDAawGHcqVadlJcsXcBNa4FBz6vFKiiH9CPcEp/ok=","Date":"Thu, 7 May 2026 10:56:26 +0200","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>, \n\tlibcamera-devel@lists.libcamera.org,\n\tAntoine Bouyer <antoine.bouyer@nxp.com>","Subject":"Re: [PATCH 2/3] ipa: libipa: Introduce V4L2Stats","Message-ID":"<afxNd0ZL-E2OvT3d@zed>","References":"<20260505-extensible-stats-v1-0-0b56c7b1bbd6@ideasonboard.com>\n\t<20260505-extensible-stats-v1-2-0b56c7b1bbd6@ideasonboard.com>\n\t<9dae5d30-38a7-4e80-ae1d-1417a73c1d39@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<9dae5d30-38a7-4e80-ae1d-1417a73c1d39@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>"}},{"id":38763,"web_url":"https://patchwork.libcamera.org/comment/38763/","msgid":"<87d1c61e-bd2c-4568-a27a-ff04d7790101@ideasonboard.com>","date":"2026-05-07T10:01:29","subject":"Re: [PATCH 2/3] ipa: libipa: Introduce V4L2Stats","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 05. 07. 10:56 keltezéssel, Jacopo Mondi írta:\n> Hi Barnabás\n> \n> On Wed, May 06, 2026 at 09:48:44AM +0200, Barnabás Pőcze wrote:\n>> 2026. 05. 05. 18:11 keltezéssel, Jacopo Mondi írta:\n>>> Add a V4L2Stats class similar in spirit to the existing V4L2Params\n>>> class to allow IPA modules to easily sub-class it to access ISP\n>>> statistics blocks serialized into a v4l2_isp_buffer.\n>>>\n>>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n>>> ---\n>>>    src/ipa/libipa/meson.build    |   2 +\n>>>    src/ipa/libipa/v4l2_stats.cpp | 226 ++++++++++++++++++++++++++++++++++++++++++\n>>>    src/ipa/libipa/v4l2_stats.h   | 124 +++++++++++++++++++++++\n>>>    3 files changed, 352 insertions(+)\n>>>\n>>> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build\n>>> index 963c5ee73063..16f4b095f220 100644\n>>> --- a/src/ipa/libipa/meson.build\n>>> +++ b/src/ipa/libipa/meson.build\n>>> @@ -19,6 +19,7 @@ libipa_headers = files([\n>>>        'pwl.h',\n>>>        'quantized.h',\n>>>        'v4l2_params.h',\n>>> +    'v4l2_stats.h',\n>>>    ])\n>>>    libipa_sources = files([\n>>> @@ -40,6 +41,7 @@ libipa_sources = files([\n>>>        'pwl.cpp',\n>>>        'quantized.cpp',\n>>>        'v4l2_params.cpp',\n>>> +    'v4l2_stats.cpp',\n>>>    ])\n>>>    libipa_includes = include_directories('..')\n>>> diff --git a/src/ipa/libipa/v4l2_stats.cpp b/src/ipa/libipa/v4l2_stats.cpp\n>>> new file mode 100644\n>>> index 000000000000..050e2329dfa9\n>>> --- /dev/null\n>>> +++ b/src/ipa/libipa/v4l2_stats.cpp\n>>> @@ -0,0 +1,226 @@\n>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>> +/*\n>>> + * Copyright (C) 2026, Ideas On Board\n>>> + *\n>>> + * V4L2 Statistics\n>>> + */\n>>> +\n>>> +#include \"v4l2_stats.h\"\n>>> +\n>>> +namespace libcamera {\n>>> +\n>>> +LOG_DEFINE_CATEGORY(V4L2Stats)\n>>\n>> The `V4L2Params` category is declared in the `ipa` namespace,\n>> so I'd do the same with this as well.\n>>\n> \n> ack\n> \n>>\n>>> +\n>>> +namespace ipa {\n>>> +\n>>> [...]\n>>> +\n>>> +} /* namespace ipa */\n>>> +\n>>> +} /* namespace libcamera */\n>>> diff --git a/src/ipa/libipa/v4l2_stats.h b/src/ipa/libipa/v4l2_stats.h\n>>> new file mode 100644\n>>> index 000000000000..404cb606e135\n>>> --- /dev/null\n>>> +++ b/src/ipa/libipa/v4l2_stats.h\n>>> @@ -0,0 +1,124 @@\n>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>> +/*\n>>> + * Copyright (C) 2026, Ideas On Board\n>>> + *\n>>> + * V4L2 Stats\n>>> + */\n>>> +\n>>> +#pragma once\n>>> +\n>>> +#include <stdint.h>\n>>> +\n>>> +#include <linux/media/v4l2-isp.h>\n>>> +\n>>> +#include <libcamera/base/log.h>\n>>> +#include <libcamera/base/span.h>\n>>> +\n>>> +namespace libcamera {\n>>> +\n>>> +LOG_DECLARE_CATEGORY(V4L2Stats)\n>>> +\n>>> +namespace ipa {\n>>> +\n>>> +template<typename T>\n>>> +class V4L2StatsBlock\n>>> +{\n>>> +public:\n>>> +\tV4L2StatsBlock(const Span<const uint8_t> data)\n>>> +\t\t: data_(data)\n>>> +\t{\n>>> +\t}\n>>> +\n>>> +\tvirtual ~V4L2StatsBlock() {}\n>>\n>> In the `V4L2Params` case, `virtual` was used to handle the differences\n>> in the rkisp1 case as far as I can recall. Is it necessary here?\n>>\n> \n> Probably not\n> \n>>\n>>> +\n>>> +\tvirtual const T *operator->() const\n>>> +\t{\n>>> +\t\treturn reinterpret_cast<const T *>(data_.data());\n>>> +\t}\n>>> +\n>>> +\tvirtual const T &operator*() const\n>>> +\t{\n>>> +\t\treturn *reinterpret_cast<const T *>(data_.data());\n>>> +\t}\n> \n> and has Kieran has pointed out, these shouldn't probably be virtual as\n> well\n> \n>>> +\n>>> +\tsize_t size() const\n>>> +\t{\n>>> +\t\treturn data_.size();\n>>> +\t}\n>>> +\n>>> +protected:\n>>> +\tSpan<const uint8_t> data_;\n>>> +};\n>>> +\n>>> +template<typename Traits>\n>>> +class V4L2Stats\n>>> +{\n>>> +public:\n>>> +\tV4L2Stats(Span<uint8_t> data, unsigned int version)\n>>> +\t\t: data_(data)\n>>> +\t{\n>>> +\t\tstruct v4l2_isp_buffer *stats =\n>>> +\t\t\treinterpret_cast<struct v4l2_isp_buffer *>(data_.data());\n>>> +\t\tused_ = stats->data_size;\n>>> +\n>>> +\t\tif (version != stats->version)\n>>> +\t\t\tLOG(V4L2Stats, Error)\n>>> +\t\t\t\t<< \"Unsupported v4l2-isp version: \" << stats->version;\n>>> +\t}\n>>> +\n>>> +\tsize_t bytesused() const { return used_; }\n>>> +\n>>> +\ttemplate<typename Traits::id_type Id>\n>>> +\tauto block() const\n>>> +\t{\n>>> +\t\tusing Details = typename Traits::template id_to_details<Id>;\n>>> +\n>>> +\t\tusing Type = typename Details::type;\n>>> +\t\tconstexpr auto kernelId = Details::blockType;\n>>> +\n>>> +\t\tauto data = block(kernelId, sizeof(Type));\n>>> +\t\treturn V4L2StatsBlock<Type>(data);\n>>\n>> Maybe we could use an `std::optional`? Or does the kernel have some guarantees\n>> about when it will most definitely provide a block?\n> \n> let's say that if you configure the ISP to produce statistics for a block,\n> then the stats should be there. But I guess it makes sense to assume a\n> user of this class wants to check if there actually are stats for the\n> block it desires in the buffer.\n> \n> However, using std::optional<> really gets in the way of the using\n> V4L2StatsBlock::operator-> and V4L2StatsBlock::operator* as a user\n> would have to do something like\n> \n>          auto opt = stats->block<Type>();\n>          if (opt) {\n>                  auto actualStats = *opt;\n> \n>                  int32_t s = actualStats->s;\n>          }\n> \n> Isn't it enough for users of this class to check the size of the\n> returned V4L2StatsBlock to discern if there are stats available for\n> that block or not ?\n\nThat's true... I suppose `explicit operator bool()` can be added that checks the size,\nand then\n\n   auto stat = ...;\n   if (stat) {\n     ...\n   }\n\nAnd now I'm wondering, would it not be sufficient to just return a simple\npointer from `block()` (and nullptr if not found)? Is there a need for the\nseparate `V4L2StatsBlock` type?\n\n\n> \n>          auto stat = stats->block<Type>();\n>          if (stat.size() > 0) {\n>                  int32_t s = stat->s;\n>          }\n> \n> ?\n> \n> (that's actually what happens already in the out-of-tree pipeline that\n> uses V4L2Stats I have)\n> \n> \tconst auto postHist = stats->block<StatsType::PostHist>();\n> \tif (postHist.size() == 0) {\n> \t\tfillMetadata(context, frameContext, metadata);\n> \t\tLOG(RppX1Agc, Error) << \"HIST data is missing in statistics\";\n> \t\treturn;\n> \t}\n> \n> \tconst auto exm = stats->block<StatsType::Pre1Exm>();\n> \tif (exm.size() == 0) {\n> \t\tfillMetadata(context, frameContext, metadata);\n> \t\tLOG(RppX1Agc, Error) << \"EXM data is missing in statistics\";\n> \t\treturn;\n> \t}\n> \n>>\n>>\n>>> +\t}\n>>> +\n>>> +protected:\n>>> +\tconst Span<const uint8_t> block(unsigned int blockType, size_t blockSize) const\n>>> +\t{\n>>> +\t\tstruct v4l2_isp_buffer *stats =\n>>> +\t\t\treinterpret_cast<struct v4l2_isp_buffer *>(data_.data());\n>>> +\n>>> +\t\t__u8 *data = stats->data;\n>>> +\t\twhile (data < stats->data + stats->data_size) {\n>>> +\t\t\tstruct v4l2_isp_block_header *header =\n>>> +\t\t\t\treinterpret_cast<struct v4l2_isp_block_header *>(data);\n>>> +\n>>> +\t\t\tif (header->type != blockType) {\n>>> +\t\t\t\tdata += header->size;\n>>> +\t\t\t\tcontinue;\n>>> +\t\t\t}\n>>> +\n>>> +\t\t\tif (header->size != blockSize) {\n>>> +\t\t\t\tLOG(V4L2Stats, Error)\n>>> +\t\t\t\t\t<< \"Block type \" << blockType\n>>> +\t\t\t\t\t<< \" size mistmatch: expected \"\n>>> +\t\t\t\t\t<< blockSize << \" got:\"\n>>> +\t\t\t\t\t<< header->size;\n>>> +\t\t\t\treturn {};\n>>> +\t\t\t}\n>>> +\n>>> +\t\t\tSpan<const uint8_t> block(data, header->size);\n>>> +\t\t\treturn block;\n>>> +\t\t}\n>>> +\n>>> +\t\tLOG(V4L2Stats, Error) << \"Unsupported stats block type: \"\n>>> +\t\t\t\t      << blockType;\n>>> +\n>>> +\t\treturn {};\n>>> +\t}\n>>> +\n>>> +\tSpan<uint8_t> data_;\n>>> +\tsize_t used_;\n>>> +};\n>>> +\n>>> +} /* namespace ipa */\n>>> +\n>>> +} /* namespace libcamera */\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 0BB40BE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  7 May 2026 10:01:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6DE0A62FE1;\n\tThu,  7 May 2026 12:01:34 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0CD9B62010\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  7 May 2026 12:01:33 +0200 (CEST)","from [192.168.33.83] (185.221.140.217.nat.pool.zt.hu\n\t[185.221.140.217])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D8FB5664;\n\tThu,  7 May 2026 12:01:28 +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=\"bNVSHxj6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1778148089;\n\tbh=NN0lsTPVOKag52BoYGo8hZ7leKuJw9xg3jH6lZQuvj4=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=bNVSHxj6hoRm/haJhOO0IxEXxtJeAECLb+Hwd869WWLr2Qt5nKSMzi+JVNOqo2nAA\n\tW2el9oLi/ZpiCopFTANrUe5mUj7LJfvIlv9UWyEp398kW5jBvfwjYzluyga/QUmgdF\n\tyfwLE4DvrL8rZjnmFGQwPHxrs59sG2d3so5jqEmM=","Message-ID":"<87d1c61e-bd2c-4568-a27a-ff04d7790101@ideasonboard.com>","Date":"Thu, 7 May 2026 12:01:29 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 2/3] ipa: libipa: Introduce V4L2Stats","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tAntoine Bouyer <antoine.bouyer@nxp.com>","References":"<20260505-extensible-stats-v1-0-0b56c7b1bbd6@ideasonboard.com>\n\t<20260505-extensible-stats-v1-2-0b56c7b1bbd6@ideasonboard.com>\n\t<9dae5d30-38a7-4e80-ae1d-1417a73c1d39@ideasonboard.com>\n\t<afxNd0ZL-E2OvT3d@zed>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<afxNd0ZL-E2OvT3d@zed>","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":38764,"web_url":"https://patchwork.libcamera.org/comment/38764/","msgid":"<afxlkzjxNcQ1va5M@zed>","date":"2026-05-07T10:13:23","subject":"Re: [PATCH 2/3] ipa: libipa: Introduce V4L2Stats","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"On Thu, May 07, 2026 at 12:01:29PM +0200, Barnabás Pőcze wrote:\n> 2026. 05. 07. 10:56 keltezéssel, Jacopo Mondi írta:\n> > Hi Barnabás\n> >\n> > On Wed, May 06, 2026 at 09:48:44AM +0200, Barnabás Pőcze wrote:\n> > > 2026. 05. 05. 18:11 keltezéssel, Jacopo Mondi írta:\n> > > > Add a V4L2Stats class similar in spirit to the existing V4L2Params\n> > > > class to allow IPA modules to easily sub-class it to access ISP\n> > > > statistics blocks serialized into a v4l2_isp_buffer.\n> > > >\n> > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > > > ---\n> > > >    src/ipa/libipa/meson.build    |   2 +\n> > > >    src/ipa/libipa/v4l2_stats.cpp | 226 ++++++++++++++++++++++++++++++++++++++++++\n> > > >    src/ipa/libipa/v4l2_stats.h   | 124 +++++++++++++++++++++++\n> > > >    3 files changed, 352 insertions(+)\n> > > >\n> > > > diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build\n> > > > index 963c5ee73063..16f4b095f220 100644\n> > > > --- a/src/ipa/libipa/meson.build\n> > > > +++ b/src/ipa/libipa/meson.build\n> > > > @@ -19,6 +19,7 @@ libipa_headers = files([\n> > > >        'pwl.h',\n> > > >        'quantized.h',\n> > > >        'v4l2_params.h',\n> > > > +    'v4l2_stats.h',\n> > > >    ])\n> > > >    libipa_sources = files([\n> > > > @@ -40,6 +41,7 @@ libipa_sources = files([\n> > > >        'pwl.cpp',\n> > > >        'quantized.cpp',\n> > > >        'v4l2_params.cpp',\n> > > > +    'v4l2_stats.cpp',\n> > > >    ])\n> > > >    libipa_includes = include_directories('..')\n> > > > diff --git a/src/ipa/libipa/v4l2_stats.cpp b/src/ipa/libipa/v4l2_stats.cpp\n> > > > new file mode 100644\n> > > > index 000000000000..050e2329dfa9\n> > > > --- /dev/null\n> > > > +++ b/src/ipa/libipa/v4l2_stats.cpp\n> > > > @@ -0,0 +1,226 @@\n> > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > > +/*\n> > > > + * Copyright (C) 2026, Ideas On Board\n> > > > + *\n> > > > + * V4L2 Statistics\n> > > > + */\n> > > > +\n> > > > +#include \"v4l2_stats.h\"\n> > > > +\n> > > > +namespace libcamera {\n> > > > +\n> > > > +LOG_DEFINE_CATEGORY(V4L2Stats)\n> > >\n> > > The `V4L2Params` category is declared in the `ipa` namespace,\n> > > so I'd do the same with this as well.\n> > >\n> >\n> > ack\n> >\n> > >\n> > > > +\n> > > > +namespace ipa {\n> > > > +\n> > > > [...]\n> > > > +\n> > > > +} /* namespace ipa */\n> > > > +\n> > > > +} /* namespace libcamera */\n> > > > diff --git a/src/ipa/libipa/v4l2_stats.h b/src/ipa/libipa/v4l2_stats.h\n> > > > new file mode 100644\n> > > > index 000000000000..404cb606e135\n> > > > --- /dev/null\n> > > > +++ b/src/ipa/libipa/v4l2_stats.h\n> > > > @@ -0,0 +1,124 @@\n> > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > > +/*\n> > > > + * Copyright (C) 2026, Ideas On Board\n> > > > + *\n> > > > + * V4L2 Stats\n> > > > + */\n> > > > +\n> > > > +#pragma once\n> > > > +\n> > > > +#include <stdint.h>\n> > > > +\n> > > > +#include <linux/media/v4l2-isp.h>\n> > > > +\n> > > > +#include <libcamera/base/log.h>\n> > > > +#include <libcamera/base/span.h>\n> > > > +\n> > > > +namespace libcamera {\n> > > > +\n> > > > +LOG_DECLARE_CATEGORY(V4L2Stats)\n> > > > +\n> > > > +namespace ipa {\n> > > > +\n> > > > +template<typename T>\n> > > > +class V4L2StatsBlock\n> > > > +{\n> > > > +public:\n> > > > +\tV4L2StatsBlock(const Span<const uint8_t> data)\n> > > > +\t\t: data_(data)\n> > > > +\t{\n> > > > +\t}\n> > > > +\n> > > > +\tvirtual ~V4L2StatsBlock() {}\n> > >\n> > > In the `V4L2Params` case, `virtual` was used to handle the differences\n> > > in the rkisp1 case as far as I can recall. Is it necessary here?\n> > >\n> >\n> > Probably not\n> >\n> > >\n> > > > +\n> > > > +\tvirtual const T *operator->() const\n> > > > +\t{\n> > > > +\t\treturn reinterpret_cast<const T *>(data_.data());\n> > > > +\t}\n> > > > +\n> > > > +\tvirtual const T &operator*() const\n> > > > +\t{\n> > > > +\t\treturn *reinterpret_cast<const T *>(data_.data());\n> > > > +\t}\n> >\n> > and has Kieran has pointed out, these shouldn't probably be virtual as\n> > well\n> >\n> > > > +\n> > > > +\tsize_t size() const\n> > > > +\t{\n> > > > +\t\treturn data_.size();\n> > > > +\t}\n> > > > +\n> > > > +protected:\n> > > > +\tSpan<const uint8_t> data_;\n> > > > +};\n> > > > +\n> > > > +template<typename Traits>\n> > > > +class V4L2Stats\n> > > > +{\n> > > > +public:\n> > > > +\tV4L2Stats(Span<uint8_t> data, unsigned int version)\n> > > > +\t\t: data_(data)\n> > > > +\t{\n> > > > +\t\tstruct v4l2_isp_buffer *stats =\n> > > > +\t\t\treinterpret_cast<struct v4l2_isp_buffer *>(data_.data());\n> > > > +\t\tused_ = stats->data_size;\n> > > > +\n> > > > +\t\tif (version != stats->version)\n> > > > +\t\t\tLOG(V4L2Stats, Error)\n> > > > +\t\t\t\t<< \"Unsupported v4l2-isp version: \" << stats->version;\n> > > > +\t}\n> > > > +\n> > > > +\tsize_t bytesused() const { return used_; }\n> > > > +\n> > > > +\ttemplate<typename Traits::id_type Id>\n> > > > +\tauto block() const\n> > > > +\t{\n> > > > +\t\tusing Details = typename Traits::template id_to_details<Id>;\n> > > > +\n> > > > +\t\tusing Type = typename Details::type;\n> > > > +\t\tconstexpr auto kernelId = Details::blockType;\n> > > > +\n> > > > +\t\tauto data = block(kernelId, sizeof(Type));\n> > > > +\t\treturn V4L2StatsBlock<Type>(data);\n> > >\n> > > Maybe we could use an `std::optional`? Or does the kernel have some guarantees\n> > > about when it will most definitely provide a block?\n> >\n> > let's say that if you configure the ISP to produce statistics for a block,\n> > then the stats should be there. But I guess it makes sense to assume a\n> > user of this class wants to check if there actually are stats for the\n> > block it desires in the buffer.\n> >\n> > However, using std::optional<> really gets in the way of the using\n> > V4L2StatsBlock::operator-> and V4L2StatsBlock::operator* as a user\n> > would have to do something like\n> >\n> >          auto opt = stats->block<Type>();\n> >          if (opt) {\n> >                  auto actualStats = *opt;\n> >\n> >                  int32_t s = actualStats->s;\n> >          }\n> >\n> > Isn't it enough for users of this class to check the size of the\n> > returned V4L2StatsBlock to discern if there are stats available for\n> > that block or not ?\n>\n> That's true... I suppose `explicit operator bool()` can be added that checks the size,\n> and then\n>\n>   auto stat = ...;\n>   if (stat) {\n>     ...\n>   }\n>\n> And now I'm wondering, would it not be sufficient to just return a simple\n> pointer from `block()` (and nullptr if not found)? Is there a need for the\n> separate `V4L2StatsBlock` type?\n\nIsn't V4L2StatsBlock there to provide the magic in operator-> to cast\nthe returned memory location to the kernel uAPI type ?\n\n>\n>\n> >\n> >          auto stat = stats->block<Type>();\n> >          if (stat.size() > 0) {\n> >                  int32_t s = stat->s;\n> >          }\n> >\n> > ?\n> >\n> > (that's actually what happens already in the out-of-tree pipeline that\n> > uses V4L2Stats I have)\n> >\n> > \tconst auto postHist = stats->block<StatsType::PostHist>();\n> > \tif (postHist.size() == 0) {\n> > \t\tfillMetadata(context, frameContext, metadata);\n> > \t\tLOG(RppX1Agc, Error) << \"HIST data is missing in statistics\";\n> > \t\treturn;\n> > \t}\n> >\n> > \tconst auto exm = stats->block<StatsType::Pre1Exm>();\n> > \tif (exm.size() == 0) {\n> > \t\tfillMetadata(context, frameContext, metadata);\n> > \t\tLOG(RppX1Agc, Error) << \"EXM data is missing in statistics\";\n> > \t\treturn;\n> > \t}\n> >\n> > >\n> > >\n> > > > +\t}\n> > > > +\n> > > > +protected:\n> > > > +\tconst Span<const uint8_t> block(unsigned int blockType, size_t blockSize) const\n> > > > +\t{\n> > > > +\t\tstruct v4l2_isp_buffer *stats =\n> > > > +\t\t\treinterpret_cast<struct v4l2_isp_buffer *>(data_.data());\n> > > > +\n> > > > +\t\t__u8 *data = stats->data;\n> > > > +\t\twhile (data < stats->data + stats->data_size) {\n> > > > +\t\t\tstruct v4l2_isp_block_header *header =\n> > > > +\t\t\t\treinterpret_cast<struct v4l2_isp_block_header *>(data);\n> > > > +\n> > > > +\t\t\tif (header->type != blockType) {\n> > > > +\t\t\t\tdata += header->size;\n> > > > +\t\t\t\tcontinue;\n> > > > +\t\t\t}\n> > > > +\n> > > > +\t\t\tif (header->size != blockSize) {\n> > > > +\t\t\t\tLOG(V4L2Stats, Error)\n> > > > +\t\t\t\t\t<< \"Block type \" << blockType\n> > > > +\t\t\t\t\t<< \" size mistmatch: expected \"\n> > > > +\t\t\t\t\t<< blockSize << \" got:\"\n> > > > +\t\t\t\t\t<< header->size;\n> > > > +\t\t\t\treturn {};\n> > > > +\t\t\t}\n> > > > +\n> > > > +\t\t\tSpan<const uint8_t> block(data, header->size);\n> > > > +\t\t\treturn block;\n> > > > +\t\t}\n> > > > +\n> > > > +\t\tLOG(V4L2Stats, Error) << \"Unsupported stats block type: \"\n> > > > +\t\t\t\t      << blockType;\n> > > > +\n> > > > +\t\treturn {};\n> > > > +\t}\n> > > > +\n> > > > +\tSpan<uint8_t> data_;\n> > > > +\tsize_t used_;\n> > > > +};\n> > > > +\n> > > > +} /* namespace ipa */\n> > > > +\n> > > > +} /* namespace libcamera */\n> > > >\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 54B13BE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  7 May 2026 10:13:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5069F62FEC;\n\tThu,  7 May 2026 12:13:27 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 157BA62010\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  7 May 2026 12:13:26 +0200 (CEST)","from ideasonboard.com (net-93-65-100-155.cust.vodafonedsl.it\n\t[93.65.100.155])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 105C3664;\n\tThu,  7 May 2026 12:13:22 +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=\"u5J7a4xA\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1778148802;\n\tbh=hHx99fqKpQ//bj6PAtan5nEO9tIg6k6I38HY/STEXLc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=u5J7a4xAnrgySYtvrcnhHNDxlapjNORQRWU5+piGRr8mGKdAlaggQPIrxIi4muUGT\n\tS50yL/9HbKsOXc0rtS+WTbMBpZlOBz0vkI+904+a+AolihqfRCNetrl9YZHFwgklEi\n\tMKOzmrg9/1JAwtTHQZsSAlMDT/sM1e6iZDXjkcr4=","Date":"Thu, 7 May 2026 12:13:23 +0200","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>, \n\tlibcamera-devel@lists.libcamera.org,\n\tAntoine Bouyer <antoine.bouyer@nxp.com>","Subject":"Re: [PATCH 2/3] ipa: libipa: Introduce V4L2Stats","Message-ID":"<afxlkzjxNcQ1va5M@zed>","References":"<20260505-extensible-stats-v1-0-0b56c7b1bbd6@ideasonboard.com>\n\t<20260505-extensible-stats-v1-2-0b56c7b1bbd6@ideasonboard.com>\n\t<9dae5d30-38a7-4e80-ae1d-1417a73c1d39@ideasonboard.com>\n\t<afxNd0ZL-E2OvT3d@zed>\n\t<87d1c61e-bd2c-4568-a27a-ff04d7790101@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<87d1c61e-bd2c-4568-a27a-ff04d7790101@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>"}},{"id":38765,"web_url":"https://patchwork.libcamera.org/comment/38765/","msgid":"<eb67e9f5-8267-489e-9606-8f82d4710525@ideasonboard.com>","date":"2026-05-07T10:17:27","subject":"Re: [PATCH 2/3] ipa: libipa: Introduce V4L2Stats","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 05. 07. 12:13 keltezéssel, Jacopo Mondi írta:\n> \n> On Thu, May 07, 2026 at 12:01:29PM +0200, Barnabás Pőcze wrote:\n>> 2026. 05. 07. 10:56 keltezéssel, Jacopo Mondi írta:\n>>> Hi Barnabás\n>>>\n>>> On Wed, May 06, 2026 at 09:48:44AM +0200, Barnabás Pőcze wrote:\n>>>> 2026. 05. 05. 18:11 keltezéssel, Jacopo Mondi írta:\n>>>>> Add a V4L2Stats class similar in spirit to the existing V4L2Params\n>>>>> class to allow IPA modules to easily sub-class it to access ISP\n>>>>> statistics blocks serialized into a v4l2_isp_buffer.\n>>>>>\n>>>>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n>>>>> ---\n>>>>>     src/ipa/libipa/meson.build    |   2 +\n>>>>>     src/ipa/libipa/v4l2_stats.cpp | 226 ++++++++++++++++++++++++++++++++++++++++++\n>>>>>     src/ipa/libipa/v4l2_stats.h   | 124 +++++++++++++++++++++++\n>>>>>     3 files changed, 352 insertions(+)\n>>>>>\n>>>>> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build\n>>>>> index 963c5ee73063..16f4b095f220 100644\n>>>>> --- a/src/ipa/libipa/meson.build\n>>>>> +++ b/src/ipa/libipa/meson.build\n>>>>> @@ -19,6 +19,7 @@ libipa_headers = files([\n>>>>>         'pwl.h',\n>>>>>         'quantized.h',\n>>>>>         'v4l2_params.h',\n>>>>> +    'v4l2_stats.h',\n>>>>>     ])\n>>>>>     libipa_sources = files([\n>>>>> @@ -40,6 +41,7 @@ libipa_sources = files([\n>>>>>         'pwl.cpp',\n>>>>>         'quantized.cpp',\n>>>>>         'v4l2_params.cpp',\n>>>>> +    'v4l2_stats.cpp',\n>>>>>     ])\n>>>>>     libipa_includes = include_directories('..')\n>>>>> diff --git a/src/ipa/libipa/v4l2_stats.cpp b/src/ipa/libipa/v4l2_stats.cpp\n>>>>> new file mode 100644\n>>>>> index 000000000000..050e2329dfa9\n>>>>> --- /dev/null\n>>>>> +++ b/src/ipa/libipa/v4l2_stats.cpp\n>>>>> @@ -0,0 +1,226 @@\n>>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>>>> +/*\n>>>>> + * Copyright (C) 2026, Ideas On Board\n>>>>> + *\n>>>>> + * V4L2 Statistics\n>>>>> + */\n>>>>> +\n>>>>> +#include \"v4l2_stats.h\"\n>>>>> +\n>>>>> +namespace libcamera {\n>>>>> +\n>>>>> +LOG_DEFINE_CATEGORY(V4L2Stats)\n>>>>\n>>>> The `V4L2Params` category is declared in the `ipa` namespace,\n>>>> so I'd do the same with this as well.\n>>>>\n>>>\n>>> ack\n>>>\n>>>>\n>>>>> +\n>>>>> +namespace ipa {\n>>>>> +\n>>>>> [...]\n>>>>> +\n>>>>> +} /* namespace ipa */\n>>>>> +\n>>>>> +} /* namespace libcamera */\n>>>>> diff --git a/src/ipa/libipa/v4l2_stats.h b/src/ipa/libipa/v4l2_stats.h\n>>>>> new file mode 100644\n>>>>> index 000000000000..404cb606e135\n>>>>> --- /dev/null\n>>>>> +++ b/src/ipa/libipa/v4l2_stats.h\n>>>>> @@ -0,0 +1,124 @@\n>>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>>>> +/*\n>>>>> + * Copyright (C) 2026, Ideas On Board\n>>>>> + *\n>>>>> + * V4L2 Stats\n>>>>> + */\n>>>>> +\n>>>>> +#pragma once\n>>>>> +\n>>>>> +#include <stdint.h>\n>>>>> +\n>>>>> +#include <linux/media/v4l2-isp.h>\n>>>>> +\n>>>>> +#include <libcamera/base/log.h>\n>>>>> +#include <libcamera/base/span.h>\n>>>>> +\n>>>>> +namespace libcamera {\n>>>>> +\n>>>>> +LOG_DECLARE_CATEGORY(V4L2Stats)\n>>>>> +\n>>>>> +namespace ipa {\n>>>>> +\n>>>>> +template<typename T>\n>>>>> +class V4L2StatsBlock\n>>>>> +{\n>>>>> +public:\n>>>>> +\tV4L2StatsBlock(const Span<const uint8_t> data)\n>>>>> +\t\t: data_(data)\n>>>>> +\t{\n>>>>> +\t}\n>>>>> +\n>>>>> +\tvirtual ~V4L2StatsBlock() {}\n>>>>\n>>>> In the `V4L2Params` case, `virtual` was used to handle the differences\n>>>> in the rkisp1 case as far as I can recall. Is it necessary here?\n>>>>\n>>>\n>>> Probably not\n>>>\n>>>>\n>>>>> +\n>>>>> +\tvirtual const T *operator->() const\n>>>>> +\t{\n>>>>> +\t\treturn reinterpret_cast<const T *>(data_.data());\n>>>>> +\t}\n>>>>> +\n>>>>> +\tvirtual const T &operator*() const\n>>>>> +\t{\n>>>>> +\t\treturn *reinterpret_cast<const T *>(data_.data());\n>>>>> +\t}\n>>>\n>>> and has Kieran has pointed out, these shouldn't probably be virtual as\n>>> well\n>>>\n>>>>> +\n>>>>> +\tsize_t size() const\n>>>>> +\t{\n>>>>> +\t\treturn data_.size();\n>>>>> +\t}\n>>>>> +\n>>>>> +protected:\n>>>>> +\tSpan<const uint8_t> data_;\n>>>>> +};\n>>>>> +\n>>>>> +template<typename Traits>\n>>>>> +class V4L2Stats\n>>>>> +{\n>>>>> +public:\n>>>>> +\tV4L2Stats(Span<uint8_t> data, unsigned int version)\n>>>>> +\t\t: data_(data)\n>>>>> +\t{\n>>>>> +\t\tstruct v4l2_isp_buffer *stats =\n>>>>> +\t\t\treinterpret_cast<struct v4l2_isp_buffer *>(data_.data());\n>>>>> +\t\tused_ = stats->data_size;\n>>>>> +\n>>>>> +\t\tif (version != stats->version)\n>>>>> +\t\t\tLOG(V4L2Stats, Error)\n>>>>> +\t\t\t\t<< \"Unsupported v4l2-isp version: \" << stats->version;\n>>>>> +\t}\n>>>>> +\n>>>>> +\tsize_t bytesused() const { return used_; }\n>>>>> +\n>>>>> +\ttemplate<typename Traits::id_type Id>\n>>>>> +\tauto block() const\n>>>>> +\t{\n>>>>> +\t\tusing Details = typename Traits::template id_to_details<Id>;\n>>>>> +\n>>>>> +\t\tusing Type = typename Details::type;\n>>>>> +\t\tconstexpr auto kernelId = Details::blockType;\n>>>>> +\n>>>>> +\t\tauto data = block(kernelId, sizeof(Type));\n>>>>> +\t\treturn V4L2StatsBlock<Type>(data);\n>>>>\n>>>> Maybe we could use an `std::optional`? Or does the kernel have some guarantees\n>>>> about when it will most definitely provide a block?\n>>>\n>>> let's say that if you configure the ISP to produce statistics for a block,\n>>> then the stats should be there. But I guess it makes sense to assume a\n>>> user of this class wants to check if there actually are stats for the\n>>> block it desires in the buffer.\n>>>\n>>> However, using std::optional<> really gets in the way of the using\n>>> V4L2StatsBlock::operator-> and V4L2StatsBlock::operator* as a user\n>>> would have to do something like\n>>>\n>>>           auto opt = stats->block<Type>();\n>>>           if (opt) {\n>>>                   auto actualStats = *opt;\n>>>\n>>>                   int32_t s = actualStats->s;\n>>>           }\n>>>\n>>> Isn't it enough for users of this class to check the size of the\n>>> returned V4L2StatsBlock to discern if there are stats available for\n>>> that block or not ?\n>>\n>> That's true... I suppose `explicit operator bool()` can be added that checks the size,\n>> and then\n>>\n>>    auto stat = ...;\n>>    if (stat) {\n>>      ...\n>>    }\n>>\n>> And now I'm wondering, would it not be sufficient to just return a simple\n>> pointer from `block()` (and nullptr if not found)? Is there a need for the\n>> separate `V4L2StatsBlock` type?\n> \n> Isn't V4L2StatsBlock there to provide the magic in operator-> to cast\n> the returned memory location to the kernel uAPI type ?\n\nYes, but returning a pointer to the concrete kernel uapi type would have\nthe same operators. The only difference is the lack of `size()`, but given\nthat the size is guaranteed to be `sizeof(T)`, and given that a nullptr\nreturn value could be used to denote missing blocks, it seems to me that\nswitching to a simple pointer would simplify things. (Although I haven't\nlooked at the linked repository in detail, so I might have missed some\nuse cases.)\n\n\n> \n>>\n>>\n>>>\n>>>           auto stat = stats->block<Type>();\n>>>           if (stat.size() > 0) {\n>>>                   int32_t s = stat->s;\n>>>           }\n>>>\n>>> ?\n>>>\n>>> (that's actually what happens already in the out-of-tree pipeline that\n>>> uses V4L2Stats I have)\n>>>\n>>> \tconst auto postHist = stats->block<StatsType::PostHist>();\n>>> \tif (postHist.size() == 0) {\n>>> \t\tfillMetadata(context, frameContext, metadata);\n>>> \t\tLOG(RppX1Agc, Error) << \"HIST data is missing in statistics\";\n>>> \t\treturn;\n>>> \t}\n>>>\n>>> \tconst auto exm = stats->block<StatsType::Pre1Exm>();\n>>> \tif (exm.size() == 0) {\n>>> \t\tfillMetadata(context, frameContext, metadata);\n>>> \t\tLOG(RppX1Agc, Error) << \"EXM data is missing in statistics\";\n>>> \t\treturn;\n>>> \t}\n>>>\n>>>>\n>>>>\n>>>>> +\t}\n>>>>> +\n>>>>> +protected:\n>>>>> +\tconst Span<const uint8_t> block(unsigned int blockType, size_t blockSize) const\n>>>>> +\t{\n>>>>> +\t\tstruct v4l2_isp_buffer *stats =\n>>>>> +\t\t\treinterpret_cast<struct v4l2_isp_buffer *>(data_.data());\n>>>>> +\n>>>>> +\t\t__u8 *data = stats->data;\n>>>>> +\t\twhile (data < stats->data + stats->data_size) {\n>>>>> +\t\t\tstruct v4l2_isp_block_header *header =\n>>>>> +\t\t\t\treinterpret_cast<struct v4l2_isp_block_header *>(data);\n>>>>> +\n>>>>> +\t\t\tif (header->type != blockType) {\n>>>>> +\t\t\t\tdata += header->size;\n>>>>> +\t\t\t\tcontinue;\n>>>>> +\t\t\t}\n>>>>> +\n>>>>> +\t\t\tif (header->size != blockSize) {\n>>>>> +\t\t\t\tLOG(V4L2Stats, Error)\n>>>>> +\t\t\t\t\t<< \"Block type \" << blockType\n>>>>> +\t\t\t\t\t<< \" size mistmatch: expected \"\n>>>>> +\t\t\t\t\t<< blockSize << \" got:\"\n>>>>> +\t\t\t\t\t<< header->size;\n>>>>> +\t\t\t\treturn {};\n>>>>> +\t\t\t}\n>>>>> +\n>>>>> +\t\t\tSpan<const uint8_t> block(data, header->size);\n>>>>> +\t\t\treturn block;\n>>>>> +\t\t}\n>>>>> +\n>>>>> +\t\tLOG(V4L2Stats, Error) << \"Unsupported stats block type: \"\n>>>>> +\t\t\t\t      << blockType;\n>>>>> +\n>>>>> +\t\treturn {};\n>>>>> +\t}\n>>>>> +\n>>>>> +\tSpan<uint8_t> data_;\n>>>>> +\tsize_t used_;\n>>>>> +};\n>>>>> +\n>>>>> +} /* namespace ipa */\n>>>>> +\n>>>>> +} /* namespace libcamera */\n>>>>>\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 5C435BDCB5\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  7 May 2026 10:17:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2D76C63021;\n\tThu,  7 May 2026 12:17:32 +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 2485462010\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  7 May 2026 12:17:31 +0200 (CEST)","from [192.168.33.83] (185.221.140.217.nat.pool.zt.hu\n\t[185.221.140.217])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1D29DB63;\n\tThu,  7 May 2026 12:17:27 +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=\"U6V/P1zW\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1778149047;\n\tbh=CRbXqBeyKa8/LbMsEjdwl5/iwgHC0YVW2sXg8JJkjRI=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=U6V/P1zWCkdvaTw6ZSH4zsJzyxsHMwinHWb3YfFqgxerJo+7xfwvhC7nNxxx5ths9\n\tQ1XpdUc9SDlymNgrR7d0hB+tnKwudZhh8xQ3tEfTltcbnY7JrONDjkM2Kj0BxxIsU8\n\tjm9fUzJHKlk+hXLp5HIglY4JGi4JCfS1c1sMqqb0=","Message-ID":"<eb67e9f5-8267-489e-9606-8f82d4710525@ideasonboard.com>","Date":"Thu, 7 May 2026 12:17:27 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 2/3] ipa: libipa: Introduce V4L2Stats","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tAntoine Bouyer <antoine.bouyer@nxp.com>","References":"<20260505-extensible-stats-v1-0-0b56c7b1bbd6@ideasonboard.com>\n\t<20260505-extensible-stats-v1-2-0b56c7b1bbd6@ideasonboard.com>\n\t<9dae5d30-38a7-4e80-ae1d-1417a73c1d39@ideasonboard.com>\n\t<afxNd0ZL-E2OvT3d@zed>\n\t<87d1c61e-bd2c-4568-a27a-ff04d7790101@ideasonboard.com>\n\t<afxlkzjxNcQ1va5M@zed>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<afxlkzjxNcQ1va5M@zed>","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":38791,"web_url":"https://patchwork.libcamera.org/comment/38791/","msgid":"<afymwyyW2EoAxjLB@zed>","date":"2026-05-07T14:52:23","subject":"Re: [PATCH 2/3] ipa: libipa: Introduce V4L2Stats","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 Thu, May 07, 2026 at 12:17:27PM +0200, Barnabás Pőcze wrote:\n> 2026. 05. 07. 12:13 keltezéssel, Jacopo Mondi írta:\n> >\n> > On Thu, May 07, 2026 at 12:01:29PM +0200, Barnabás Pőcze wrote:\n> > > 2026. 05. 07. 10:56 keltezéssel, Jacopo Mondi írta:\n> > > > Hi Barnabás\n> > > >\n> > > > On Wed, May 06, 2026 at 09:48:44AM +0200, Barnabás Pőcze wrote:\n> > > > > 2026. 05. 05. 18:11 keltezéssel, Jacopo Mondi írta:\n> > > > > > Add a V4L2Stats class similar in spirit to the existing V4L2Params\n> > > > > > class to allow IPA modules to easily sub-class it to access ISP\n> > > > > > statistics blocks serialized into a v4l2_isp_buffer.\n> > > > > >\n> > > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > > > > > ---\n> > > > > >     src/ipa/libipa/meson.build    |   2 +\n> > > > > >     src/ipa/libipa/v4l2_stats.cpp | 226 ++++++++++++++++++++++++++++++++++++++++++\n> > > > > >     src/ipa/libipa/v4l2_stats.h   | 124 +++++++++++++++++++++++\n> > > > > >     3 files changed, 352 insertions(+)\n> > > > > >\n> > > > > > diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build\n> > > > > > index 963c5ee73063..16f4b095f220 100644\n> > > > > > --- a/src/ipa/libipa/meson.build\n> > > > > > +++ b/src/ipa/libipa/meson.build\n> > > > > > @@ -19,6 +19,7 @@ libipa_headers = files([\n> > > > > >         'pwl.h',\n> > > > > >         'quantized.h',\n> > > > > >         'v4l2_params.h',\n> > > > > > +    'v4l2_stats.h',\n> > > > > >     ])\n> > > > > >     libipa_sources = files([\n> > > > > > @@ -40,6 +41,7 @@ libipa_sources = files([\n> > > > > >         'pwl.cpp',\n> > > > > >         'quantized.cpp',\n> > > > > >         'v4l2_params.cpp',\n> > > > > > +    'v4l2_stats.cpp',\n> > > > > >     ])\n> > > > > >     libipa_includes = include_directories('..')\n> > > > > > diff --git a/src/ipa/libipa/v4l2_stats.cpp b/src/ipa/libipa/v4l2_stats.cpp\n> > > > > > new file mode 100644\n> > > > > > index 000000000000..050e2329dfa9\n> > > > > > --- /dev/null\n> > > > > > +++ b/src/ipa/libipa/v4l2_stats.cpp\n> > > > > > @@ -0,0 +1,226 @@\n> > > > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > > > > +/*\n> > > > > > + * Copyright (C) 2026, Ideas On Board\n> > > > > > + *\n> > > > > > + * V4L2 Statistics\n> > > > > > + */\n> > > > > > +\n> > > > > > +#include \"v4l2_stats.h\"\n> > > > > > +\n> > > > > > +namespace libcamera {\n> > > > > > +\n> > > > > > +LOG_DEFINE_CATEGORY(V4L2Stats)\n> > > > >\n> > > > > The `V4L2Params` category is declared in the `ipa` namespace,\n> > > > > so I'd do the same with this as well.\n> > > > >\n> > > >\n> > > > ack\n> > > >\n> > > > >\n> > > > > > +\n> > > > > > +namespace ipa {\n> > > > > > +\n> > > > > > [...]\n> > > > > > +\n> > > > > > +} /* namespace ipa */\n> > > > > > +\n> > > > > > +} /* namespace libcamera */\n> > > > > > diff --git a/src/ipa/libipa/v4l2_stats.h b/src/ipa/libipa/v4l2_stats.h\n> > > > > > new file mode 100644\n> > > > > > index 000000000000..404cb606e135\n> > > > > > --- /dev/null\n> > > > > > +++ b/src/ipa/libipa/v4l2_stats.h\n> > > > > > @@ -0,0 +1,124 @@\n> > > > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > > > > +/*\n> > > > > > + * Copyright (C) 2026, Ideas On Board\n> > > > > > + *\n> > > > > > + * V4L2 Stats\n> > > > > > + */\n> > > > > > +\n> > > > > > +#pragma once\n> > > > > > +\n> > > > > > +#include <stdint.h>\n> > > > > > +\n> > > > > > +#include <linux/media/v4l2-isp.h>\n> > > > > > +\n> > > > > > +#include <libcamera/base/log.h>\n> > > > > > +#include <libcamera/base/span.h>\n> > > > > > +\n> > > > > > +namespace libcamera {\n> > > > > > +\n> > > > > > +LOG_DECLARE_CATEGORY(V4L2Stats)\n> > > > > > +\n> > > > > > +namespace ipa {\n> > > > > > +\n> > > > > > +template<typename T>\n> > > > > > +class V4L2StatsBlock\n> > > > > > +{\n> > > > > > +public:\n> > > > > > +\tV4L2StatsBlock(const Span<const uint8_t> data)\n> > > > > > +\t\t: data_(data)\n> > > > > > +\t{\n> > > > > > +\t}\n> > > > > > +\n> > > > > > +\tvirtual ~V4L2StatsBlock() {}\n> > > > >\n> > > > > In the `V4L2Params` case, `virtual` was used to handle the differences\n> > > > > in the rkisp1 case as far as I can recall. Is it necessary here?\n> > > > >\n> > > >\n> > > > Probably not\n> > > >\n> > > > >\n> > > > > > +\n> > > > > > +\tvirtual const T *operator->() const\n> > > > > > +\t{\n> > > > > > +\t\treturn reinterpret_cast<const T *>(data_.data());\n> > > > > > +\t}\n> > > > > > +\n> > > > > > +\tvirtual const T &operator*() const\n> > > > > > +\t{\n> > > > > > +\t\treturn *reinterpret_cast<const T *>(data_.data());\n> > > > > > +\t}\n> > > >\n> > > > and has Kieran has pointed out, these shouldn't probably be virtual as\n> > > > well\n> > > >\n> > > > > > +\n> > > > > > +\tsize_t size() const\n> > > > > > +\t{\n> > > > > > +\t\treturn data_.size();\n> > > > > > +\t}\n> > > > > > +\n> > > > > > +protected:\n> > > > > > +\tSpan<const uint8_t> data_;\n> > > > > > +};\n> > > > > > +\n> > > > > > +template<typename Traits>\n> > > > > > +class V4L2Stats\n> > > > > > +{\n> > > > > > +public:\n> > > > > > +\tV4L2Stats(Span<uint8_t> data, unsigned int version)\n> > > > > > +\t\t: data_(data)\n> > > > > > +\t{\n> > > > > > +\t\tstruct v4l2_isp_buffer *stats =\n> > > > > > +\t\t\treinterpret_cast<struct v4l2_isp_buffer *>(data_.data());\n> > > > > > +\t\tused_ = stats->data_size;\n> > > > > > +\n> > > > > > +\t\tif (version != stats->version)\n> > > > > > +\t\t\tLOG(V4L2Stats, Error)\n> > > > > > +\t\t\t\t<< \"Unsupported v4l2-isp version: \" << stats->version;\n> > > > > > +\t}\n> > > > > > +\n> > > > > > +\tsize_t bytesused() const { return used_; }\n> > > > > > +\n> > > > > > +\ttemplate<typename Traits::id_type Id>\n> > > > > > +\tauto block() const\n> > > > > > +\t{\n> > > > > > +\t\tusing Details = typename Traits::template id_to_details<Id>;\n> > > > > > +\n> > > > > > +\t\tusing Type = typename Details::type;\n> > > > > > +\t\tconstexpr auto kernelId = Details::blockType;\n> > > > > > +\n> > > > > > +\t\tauto data = block(kernelId, sizeof(Type));\n> > > > > > +\t\treturn V4L2StatsBlock<Type>(data);\n> > > > >\n> > > > > Maybe we could use an `std::optional`? Or does the kernel have some guarantees\n> > > > > about when it will most definitely provide a block?\n> > > >\n> > > > let's say that if you configure the ISP to produce statistics for a block,\n> > > > then the stats should be there. But I guess it makes sense to assume a\n> > > > user of this class wants to check if there actually are stats for the\n> > > > block it desires in the buffer.\n> > > >\n> > > > However, using std::optional<> really gets in the way of the using\n> > > > V4L2StatsBlock::operator-> and V4L2StatsBlock::operator* as a user\n> > > > would have to do something like\n> > > >\n> > > >           auto opt = stats->block<Type>();\n> > > >           if (opt) {\n> > > >                   auto actualStats = *opt;\n> > > >\n> > > >                   int32_t s = actualStats->s;\n> > > >           }\n> > > >\n> > > > Isn't it enough for users of this class to check the size of the\n> > > > returned V4L2StatsBlock to discern if there are stats available for\n> > > > that block or not ?\n> > >\n> > > That's true... I suppose `explicit operator bool()` can be added that checks the size,\n> > > and then\n> > >\n> > >    auto stat = ...;\n> > >    if (stat) {\n> > >      ...\n> > >    }\n> > >\n> > > And now I'm wondering, would it not be sufficient to just return a simple\n> > > pointer from `block()` (and nullptr if not found)? Is there a need for the\n> > > separate `V4L2StatsBlock` type?\n> >\n> > Isn't V4L2StatsBlock there to provide the magic in operator-> to cast\n> > the returned memory location to the kernel uAPI type ?\n>\n> Yes, but returning a pointer to the concrete kernel uapi type would have\n> the same operators. The only difference is the lack of `size()`, but given\n\nThe abstraction cost is so cheap I would rather keep it there. None of\nthe currently upstream driver uses extensible stats, but it might be\nthe case some of them will have to be ported and we will need a place\nwhere to handle both formats like we do for rkisp1 params ?\n\n> that the size is guaranteed to be `sizeof(T)`, and given that a nullptr\n> return value could be used to denote missing blocks, it seems to me that\n> switching to a simple pointer would simplify things. (Although I haven't\n> looked at the linked repository in detail, so I might have missed some\n> use cases.)\n>\n>\n> >\n> > >\n> > >\n> > > >\n> > > >           auto stat = stats->block<Type>();\n> > > >           if (stat.size() > 0) {\n> > > >                   int32_t s = stat->s;\n> > > >           }\n> > > >\n> > > > ?\n> > > >\n> > > > (that's actually what happens already in the out-of-tree pipeline that\n> > > > uses V4L2Stats I have)\n> > > >\n> > > > \tconst auto postHist = stats->block<StatsType::PostHist>();\n> > > > \tif (postHist.size() == 0) {\n> > > > \t\tfillMetadata(context, frameContext, metadata);\n> > > > \t\tLOG(RppX1Agc, Error) << \"HIST data is missing in statistics\";\n> > > > \t\treturn;\n> > > > \t}\n> > > >\n> > > > \tconst auto exm = stats->block<StatsType::Pre1Exm>();\n> > > > \tif (exm.size() == 0) {\n> > > > \t\tfillMetadata(context, frameContext, metadata);\n> > > > \t\tLOG(RppX1Agc, Error) << \"EXM data is missing in statistics\";\n> > > > \t\treturn;\n> > > > \t}\n> > > >\n> > > > >\n> > > > >\n> > > > > > +\t}\n> > > > > > +\n> > > > > > +protected:\n> > > > > > +\tconst Span<const uint8_t> block(unsigned int blockType, size_t blockSize) const\n> > > > > > +\t{\n> > > > > > +\t\tstruct v4l2_isp_buffer *stats =\n> > > > > > +\t\t\treinterpret_cast<struct v4l2_isp_buffer *>(data_.data());\n> > > > > > +\n> > > > > > +\t\t__u8 *data = stats->data;\n> > > > > > +\t\twhile (data < stats->data + stats->data_size) {\n> > > > > > +\t\t\tstruct v4l2_isp_block_header *header =\n> > > > > > +\t\t\t\treinterpret_cast<struct v4l2_isp_block_header *>(data);\n> > > > > > +\n> > > > > > +\t\t\tif (header->type != blockType) {\n> > > > > > +\t\t\t\tdata += header->size;\n> > > > > > +\t\t\t\tcontinue;\n> > > > > > +\t\t\t}\n> > > > > > +\n> > > > > > +\t\t\tif (header->size != blockSize) {\n> > > > > > +\t\t\t\tLOG(V4L2Stats, Error)\n> > > > > > +\t\t\t\t\t<< \"Block type \" << blockType\n> > > > > > +\t\t\t\t\t<< \" size mistmatch: expected \"\n> > > > > > +\t\t\t\t\t<< blockSize << \" got:\"\n> > > > > > +\t\t\t\t\t<< header->size;\n> > > > > > +\t\t\t\treturn {};\n> > > > > > +\t\t\t}\n> > > > > > +\n> > > > > > +\t\t\tSpan<const uint8_t> block(data, header->size);\n> > > > > > +\t\t\treturn block;\n> > > > > > +\t\t}\n> > > > > > +\n> > > > > > +\t\tLOG(V4L2Stats, Error) << \"Unsupported stats block type: \"\n> > > > > > +\t\t\t\t      << blockType;\n> > > > > > +\n> > > > > > +\t\treturn {};\n> > > > > > +\t}\n> > > > > > +\n> > > > > > +\tSpan<uint8_t> data_;\n> > > > > > +\tsize_t used_;\n> > > > > > +};\n> > > > > > +\n> > > > > > +} /* namespace ipa */\n> > > > > > +\n> > > > > > +} /* namespace libcamera */\n> > > > > >\n> > > > >\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 1DFEEBE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  7 May 2026 14:52:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 08F9A63020;\n\tThu,  7 May 2026 16:52:28 +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 50C5462FE1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  7 May 2026 16:52:27 +0200 (CEST)","from ideasonboard.com (93-46-82-201.ip106.fastwebnet.it\n\t[93.46.82.201])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0035F664;\n\tThu,  7 May 2026 16:52:22 +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=\"cI7vF34k\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1778165543;\n\tbh=OifRzrm72iX/xFN4gStQQV7gOAZHF39vhHp5JwGGm6A=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=cI7vF34kBOnIEnndjJcC6vzqsEyh4X65k9Dzb/LRVxQmmY6hNBbUtpmmZp2vch5b9\n\t3j8gmkeQQS+xj03P3HTEhwcvSFmNHrsbUpb26TXJYncDZEofezo6xkYQYBJKv3Ae/v\n\t3/b3TgGu4owUzoCVdEWTzfRvwlDcEVvKLdtXU2lY=","Date":"Thu, 7 May 2026 16:52:23 +0200","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>, \n\tlibcamera-devel@lists.libcamera.org,\n\tAntoine Bouyer <antoine.bouyer@nxp.com>","Subject":"Re: [PATCH 2/3] ipa: libipa: Introduce V4L2Stats","Message-ID":"<afymwyyW2EoAxjLB@zed>","References":"<20260505-extensible-stats-v1-0-0b56c7b1bbd6@ideasonboard.com>\n\t<20260505-extensible-stats-v1-2-0b56c7b1bbd6@ideasonboard.com>\n\t<9dae5d30-38a7-4e80-ae1d-1417a73c1d39@ideasonboard.com>\n\t<afxNd0ZL-E2OvT3d@zed>\n\t<87d1c61e-bd2c-4568-a27a-ff04d7790101@ideasonboard.com>\n\t<afxlkzjxNcQ1va5M@zed>\n\t<eb67e9f5-8267-489e-9606-8f82d4710525@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<eb67e9f5-8267-489e-9606-8f82d4710525@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>"}}]