| Message ID | 20260505-extensible-stats-v1-2-0b56c7b1bbd6@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
2026. 05. 05. 18:11 keltezéssel, Jacopo Mondi írta: > Add a V4L2Stats class similar in spirit to the existing V4L2Params > class to allow IPA modules to easily sub-class it to access ISP > statistics blocks serialized into a v4l2_isp_buffer. > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > --- > src/ipa/libipa/meson.build | 2 + > src/ipa/libipa/v4l2_stats.cpp | 226 ++++++++++++++++++++++++++++++++++++++++++ > src/ipa/libipa/v4l2_stats.h | 124 +++++++++++++++++++++++ > 3 files changed, 352 insertions(+) > > diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build > index 963c5ee73063..16f4b095f220 100644 > --- a/src/ipa/libipa/meson.build > +++ b/src/ipa/libipa/meson.build > @@ -19,6 +19,7 @@ libipa_headers = files([ > 'pwl.h', > 'quantized.h', > 'v4l2_params.h', > + 'v4l2_stats.h', > ]) > > libipa_sources = files([ > @@ -40,6 +41,7 @@ libipa_sources = files([ > 'pwl.cpp', > 'quantized.cpp', > 'v4l2_params.cpp', > + 'v4l2_stats.cpp', > ]) > > libipa_includes = include_directories('..') > diff --git a/src/ipa/libipa/v4l2_stats.cpp b/src/ipa/libipa/v4l2_stats.cpp > new file mode 100644 > index 000000000000..050e2329dfa9 > --- /dev/null > +++ b/src/ipa/libipa/v4l2_stats.cpp > @@ -0,0 +1,226 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2026, Ideas On Board > + * > + * V4L2 Statistics > + */ > + > +#include "v4l2_stats.h" > + > +namespace libcamera { > + > +LOG_DEFINE_CATEGORY(V4L2Stats) The `V4L2Params` category is declared in the `ipa` namespace, so I'd do the same with this as well. > + > +namespace ipa { > + > [...] > + > +} /* namespace ipa */ > + > +} /* namespace libcamera */ > diff --git a/src/ipa/libipa/v4l2_stats.h b/src/ipa/libipa/v4l2_stats.h > new file mode 100644 > index 000000000000..404cb606e135 > --- /dev/null > +++ b/src/ipa/libipa/v4l2_stats.h > @@ -0,0 +1,124 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2026, Ideas On Board > + * > + * V4L2 Stats > + */ > + > +#pragma once > + > +#include <stdint.h> > + > +#include <linux/media/v4l2-isp.h> > + > +#include <libcamera/base/log.h> > +#include <libcamera/base/span.h> > + > +namespace libcamera { > + > +LOG_DECLARE_CATEGORY(V4L2Stats) > + > +namespace ipa { > + > +template<typename T> > +class V4L2StatsBlock > +{ > +public: > + V4L2StatsBlock(const Span<const uint8_t> data) > + : data_(data) > + { > + } > + > + virtual ~V4L2StatsBlock() {} In the `V4L2Params` case, `virtual` was used to handle the differences in the rkisp1 case as far as I can recall. Is it necessary here? > + > + virtual const T *operator->() const > + { > + return reinterpret_cast<const T *>(data_.data()); > + } > + > + virtual const T &operator*() const > + { > + return *reinterpret_cast<const T *>(data_.data()); > + } > + > + size_t size() const > + { > + return data_.size(); > + } > + > +protected: > + Span<const uint8_t> data_; > +}; > + > +template<typename Traits> > +class V4L2Stats > +{ > +public: > + V4L2Stats(Span<uint8_t> data, unsigned int version) > + : data_(data) > + { > + struct v4l2_isp_buffer *stats = > + reinterpret_cast<struct v4l2_isp_buffer *>(data_.data()); > + used_ = stats->data_size; > + > + if (version != stats->version) > + LOG(V4L2Stats, Error) > + << "Unsupported v4l2-isp version: " << stats->version; > + } > + > + size_t bytesused() const { return used_; } > + > + template<typename Traits::id_type Id> > + auto block() const > + { > + using Details = typename Traits::template id_to_details<Id>; > + > + using Type = typename Details::type; > + constexpr auto kernelId = Details::blockType; > + > + auto data = block(kernelId, sizeof(Type)); > + return V4L2StatsBlock<Type>(data); Maybe we could use an `std::optional`? Or does the kernel have some guarantees about when it will most definitely provide a block? > + } > + > +protected: > + const Span<const uint8_t> block(unsigned int blockType, size_t blockSize) const > + { > + struct v4l2_isp_buffer *stats = > + reinterpret_cast<struct v4l2_isp_buffer *>(data_.data()); > + > + __u8 *data = stats->data; > + while (data < stats->data + stats->data_size) { > + struct v4l2_isp_block_header *header = > + reinterpret_cast<struct v4l2_isp_block_header *>(data); > + > + if (header->type != blockType) { > + data += header->size; > + continue; > + } > + > + if (header->size != blockSize) { > + LOG(V4L2Stats, Error) > + << "Block type " << blockType > + << " size mistmatch: expected " > + << blockSize << " got:" > + << header->size; > + return {}; > + } > + > + Span<const uint8_t> block(data, header->size); > + return block; > + } > + > + LOG(V4L2Stats, Error) << "Unsupported stats block type: " > + << blockType; > + > + return {}; > + } > + > + Span<uint8_t> data_; > + size_t used_; > +}; > + > +} /* namespace ipa */ > + > +} /* namespace libcamera */ >
Quoting Jacopo Mondi (2026-05-05 17:11:10) > Add a V4L2Stats class similar in spirit to the existing V4L2Params > class to allow IPA modules to easily sub-class it to access ISP > statistics blocks serialized into a v4l2_isp_buffer. > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > --- > src/ipa/libipa/meson.build | 2 + > src/ipa/libipa/v4l2_stats.cpp | 226 ++++++++++++++++++++++++++++++++++++++++++ > src/ipa/libipa/v4l2_stats.h | 124 +++++++++++++++++++++++ > 3 files changed, 352 insertions(+) > > diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build > index 963c5ee73063..16f4b095f220 100644 > --- a/src/ipa/libipa/meson.build > +++ b/src/ipa/libipa/meson.build > @@ -19,6 +19,7 @@ libipa_headers = files([ > 'pwl.h', > 'quantized.h', > 'v4l2_params.h', > + 'v4l2_stats.h', > ]) > > libipa_sources = files([ > @@ -40,6 +41,7 @@ libipa_sources = files([ > 'pwl.cpp', > 'quantized.cpp', > 'v4l2_params.cpp', > + 'v4l2_stats.cpp', > ]) > > libipa_includes = include_directories('..') > diff --git a/src/ipa/libipa/v4l2_stats.cpp b/src/ipa/libipa/v4l2_stats.cpp > new file mode 100644 > index 000000000000..050e2329dfa9 > --- /dev/null > +++ b/src/ipa/libipa/v4l2_stats.cpp > @@ -0,0 +1,226 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2026, Ideas On Board > + * > + * V4L2 Statistics > + */ > + > +#include "v4l2_stats.h" > + > +namespace libcamera { > + > +LOG_DEFINE_CATEGORY(V4L2Stats) > + > +namespace ipa { > + > +/** > + * \file v4l2_stats.cpp > + * \brief Helper class to handle an ISP statistics buffer compatible with > + * the generic V4L2 ISP format > + * > + * The Linux kernel defines a generic buffer format for ISP statistics. > + * The format describes a serialisation method that allows userspace to > + * access statistics data from a binary buffer. > + * > + * The V4L2Stats class implements support the V4L2 ISP statistics buffer format support for the > + * and allows users to retrieve an ISP statistics blocks, represented as as a V4L2StatsBlock > + * V4L2StatsBlock class instances. > + * > + * IPA implementations using this helpers should define an enumeration of ISP using this helper or using these helpers > + * blocks supported by the IPA module and use a set of common abstraction to abstractions > + * help their derived implementation of V4L2Stats translate the enumerated ISP > + * block identifiers to the actual type of the statistics data as defined by > + * the kernel interface. > + */ > + > +/** > + * \class V4L2StatsBlock > + * \brief Helper class that represents an ISP statistics block > + * > + * Each ISP measurement block produces a set of statistics data whose memory > + * layout is defined by the kernel interface. > + * > + * This class represents an ISP statistics block entry. It is constructed > + * with a reference to the memory area where the statistics block is > + * stored in the statistics buffer. The template parameter represents > + * the underlying kernel-defined ISP statistics block type and allows its > + * user to easily cast it to said type to read the statistics data. > + * > + * \sa V4L2Stats::block() > + */ > + > +/** > + * \fn V4L2StatsBlock::V4L2StatsBlock() > + * \brief Construct a V4L2StatsBlock with memory represented by \a data > + * \param[in] data The memory area where the statistics block is located > + */ > + > +/** > + * \fn V4L2StatsBlock::operator->() const > + * \brief Access the statistics block casting it to the kernel-defined > + * ISP block type > + * > + * The V4L2StatsBlock is templated with the kernel defined ISP block type. This > + * function allows users to easily cast a V4L2StatsBlock to the underlying > + * kernel-defined type in order to easily access the statistics data. > + * > + * \code{.cpp} > + * > + * // The kernel header defines the ISP statistics types, in example for example: > + * // struct my_isp_awb_stats_data { > + * // u16 mean_r; > + * // u16 mean_g; > + * // u16 mean_b; > + * // } > + * > + * template<> V4L2StatsBlock<struct my_isp_awb_stats_data> awbStats = ... > + * > + * u16 mean_r = awbStats->mean_r; > + * u16 mean_g = awbStats->mean_g; > + * u16 mean_b = awbStats->mean_b; > + * > + * \endcode > + * > + * Users of this class shall not create a V4L2StatsBlock manually but should > + * use V4L2Stats::block(). > + */ > + > +/** > + * \fn V4L2StatsBlock::operator*() const > + * \copydoc V4L2StatsBlock::operator->() const > + */ > + > +/** > + * \fn V4L2StatsBlock::size() const > + * \brief Retrieve the size (in bytes) of the ISP statistics block, including > + * the block's header > + * \return The size of the stats block > + */ > + > +/** > + * \var V4L2StatsBlock::data_ > + * \brief Memory area where ISP statistics have been serialized > + */ > + > +/** > + * \class V4L2Stats > + * \brief Helper class that represent an ISP statistics buffer > + * > + * This class represents an ISP statistics buffer. It is constructed with a > + * reference to the memory mapped buffer that has been dequeued from the ISP > + * driver. > + * > + * This class is templated with the type of the enumeration of ISP blocks that > + * each IPA module is expected to support. IPA modules are expected to derive > + * this class by providing a 'stats_traits' type that helps the class associate > + * a block type with the actual memory area that represents the ISP statistics > + * block. > + * > + * \code{.cpp} > + * > + * // Define the supported ISP statistics blocks > + * enum class myISPStats { > + * Agc, > + * Awb, > + * ... > + * }; > + * > + * // Maps the C++ enum type to the kernel enum type and concrete parameter type > + * template<myISPStats B> > + * struct block_type { > + * }; > + * > + * template<> > + * struct block_type<myISPStats::Agc> { > + * using type = struct my_isp_kernel_stats_type_agc; > + * static constexpr kernel_enum_type blockType = MY_ISP_STATS_TYPE_AGC; > + * }; > + * > + * template<> > + * struct block_type<myISPStats::Awb> { > + * using type = struct my_isp_kernel_stats_type_awb; > + * static constexpr kernel_enum_type blockType = MY_ISP_STATS_TYPE_AWB; > + * }; > + * > + * > + * // Convenience type to associate a block id to the 'block_type' overload > + * struct stats_traits { > + * using id_type = myISPStats; > + * template<id_type Id> using id_to_details = block_type<Id>; > + * }; > + * > + * ... > + * > + * // Derive the V4L2Stats class by providing stats_traits > + * class MyISPStats : public V4L2Stats<stats_traits> > + * { > + * public: > + * MyISPStats::MyISPStats(Span<uint8_t> data) > + * : V4L2Stats(data) > + * { > + * } > + * }; > + * > + * \endcode > + * > + * Users of this class can then easily access an ISP statistics block as a > + * V4L2StatsBlock instance. > + * > + * \code{.cpp} > + * > + * MyISPStats stats(data); > + * > + * auto awb = stats.block<myISPStats::AWB>(); > + * auto mean_r = awb->mean_r; > + * auto mean_g = awb->mean_g; > + * auto mean_b = awb->mean_b; > + * \endcode > + */ > + > +/** > + * \fn V4L2Stats::V4L2Stats() > + * \brief Construct an instance of V4L2Stats > + * \param[in] data Reference to the v4l2-buffer memory mapped area > + * \param[in] version The expected V4L2 ISP serialization format version > + */ > + > +/** > + * \fn V4L2Stats::bytesused() > + * \brief Retrieve the size of the statistics buffer (in bytes) > + * \return The number of bytes occupied by the ISP statistics > + */ > + > +/** > + * \fn V4L2Stats::block() const > + * \brief Retrieve the location of an ISP statistics block a return it and return it ? Or just simply "Retrieve the location of an ISP statistics block" > + * \return A V4L2StatsBlock instance that points to the ISP statistics block > + */ > + > +/** > + * \fn V4L2Stats::block(unsigned int blockType, size_t blockSize) const > + * \brief Retrieve an ISP statistics block a returns a reference to it > + * \param[in] blockType The kernel-defined ISP block identifier, used to > + * identify the block header > + * \param[in] blockSize The ISP statistics block size, for validation > + * > + * Parse the buffer of serialized statistics data and retrieve the location > + * of the block with type \a blockType of size \a blockSize. > + * > + * IPA modules that derive the V4L2Stats class shall use this function to > + * retrieve the memory area that will be used to construct a V4L2StatsBlock<T> > + * before returning it to the caller. > + */ > + > +/** > + * \var V4L2Stats::data_ > + * \brief The ISP statistics buffer memory > + */ > + > +/** > + * \var V4L2Stats::used_ > + * \brief The number of bytes used by the statistics buffer > + */ > + > +} /* namespace ipa */ > + > +} /* namespace libcamera */ > diff --git a/src/ipa/libipa/v4l2_stats.h b/src/ipa/libipa/v4l2_stats.h > new file mode 100644 > index 000000000000..404cb606e135 > --- /dev/null > +++ b/src/ipa/libipa/v4l2_stats.h > @@ -0,0 +1,124 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2026, Ideas On Board > + * > + * V4L2 Stats > + */ > + > +#pragma once > + > +#include <stdint.h> > + > +#include <linux/media/v4l2-isp.h> > + > +#include <libcamera/base/log.h> > +#include <libcamera/base/span.h> > + > +namespace libcamera { > + > +LOG_DECLARE_CATEGORY(V4L2Stats) > + > +namespace ipa { > + > +template<typename T> > +class V4L2StatsBlock > +{ > +public: > + V4L2StatsBlock(const Span<const uint8_t> data) > + : data_(data) > + { > + } > + > + virtual ~V4L2StatsBlock() {} > + > + virtual const T *operator->() const Are these operators really virtual? Will they be overridden by other implementations? It seems like they just access data_ and perform the cast ... so I don't think they need to go through the vtable ? > + { > + return reinterpret_cast<const T *>(data_.data()); > + } > + > + virtual const T &operator*() const > + { > + return *reinterpret_cast<const T *>(data_.data()); > + } > + > + size_t size() const > + { > + return data_.size(); > + } > + > +protected: > + Span<const uint8_t> data_; > +}; > + > +template<typename Traits> > +class V4L2Stats > +{ > +public: > + V4L2Stats(Span<uint8_t> data, unsigned int version) > + : data_(data) > + { > + struct v4l2_isp_buffer *stats = > + reinterpret_cast<struct v4l2_isp_buffer *>(data_.data()); > + used_ = stats->data_size; > + > + if (version != stats->version) > + LOG(V4L2Stats, Error) > + << "Unsupported v4l2-isp version: " << stats->version; > + } > + > + size_t bytesused() const { return used_; } > + > + template<typename Traits::id_type Id> > + auto block() const > + { > + using Details = typename Traits::template id_to_details<Id>; > + > + using Type = typename Details::type; > + constexpr auto kernelId = Details::blockType; > + > + auto data = block(kernelId, sizeof(Type)); > + return V4L2StatsBlock<Type>(data); > + } > + > +protected: > + const Span<const uint8_t> block(unsigned int blockType, size_t blockSize) const > + { > + struct v4l2_isp_buffer *stats = > + reinterpret_cast<struct v4l2_isp_buffer *>(data_.data()); > + > + __u8 *data = stats->data; > + while (data < stats->data + stats->data_size) { > + struct v4l2_isp_block_header *header = > + reinterpret_cast<struct v4l2_isp_block_header *>(data); > + > + if (header->type != blockType) { > + data += header->size; > + continue; > + } > + > + if (header->size != blockSize) { > + LOG(V4L2Stats, Error) > + << "Block type " << blockType > + << " size mistmatch: expected " > + << blockSize << " got:" > + << header->size; > + return {}; > + } > + > + Span<const uint8_t> block(data, header->size); > + return block; Does this work? return {data, header->size}; or this still fits on one line? return Span<const uint8_t>(data, header->size); Otherwise, nothing specifically major stands out in any of that for me, so if this gets us a starting point to use the new kernel interface: Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > + } > + > + LOG(V4L2Stats, Error) << "Unsupported stats block type: " > + << blockType; > + > + return {}; > + } > + > + Span<uint8_t> data_; > + size_t used_; > +}; > + > +} /* namespace ipa */ > + > +} /* namespace libcamera */ > > -- > 2.53.0 >
Hi Barnabás On Wed, May 06, 2026 at 09:48:44AM +0200, Barnabás Pőcze wrote: > 2026. 05. 05. 18:11 keltezéssel, Jacopo Mondi írta: > > Add a V4L2Stats class similar in spirit to the existing V4L2Params > > class to allow IPA modules to easily sub-class it to access ISP > > statistics blocks serialized into a v4l2_isp_buffer. > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > --- > > src/ipa/libipa/meson.build | 2 + > > src/ipa/libipa/v4l2_stats.cpp | 226 ++++++++++++++++++++++++++++++++++++++++++ > > src/ipa/libipa/v4l2_stats.h | 124 +++++++++++++++++++++++ > > 3 files changed, 352 insertions(+) > > > > diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build > > index 963c5ee73063..16f4b095f220 100644 > > --- a/src/ipa/libipa/meson.build > > +++ b/src/ipa/libipa/meson.build > > @@ -19,6 +19,7 @@ libipa_headers = files([ > > 'pwl.h', > > 'quantized.h', > > 'v4l2_params.h', > > + 'v4l2_stats.h', > > ]) > > libipa_sources = files([ > > @@ -40,6 +41,7 @@ libipa_sources = files([ > > 'pwl.cpp', > > 'quantized.cpp', > > 'v4l2_params.cpp', > > + 'v4l2_stats.cpp', > > ]) > > libipa_includes = include_directories('..') > > diff --git a/src/ipa/libipa/v4l2_stats.cpp b/src/ipa/libipa/v4l2_stats.cpp > > new file mode 100644 > > index 000000000000..050e2329dfa9 > > --- /dev/null > > +++ b/src/ipa/libipa/v4l2_stats.cpp > > @@ -0,0 +1,226 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2026, Ideas On Board > > + * > > + * V4L2 Statistics > > + */ > > + > > +#include "v4l2_stats.h" > > + > > +namespace libcamera { > > + > > +LOG_DEFINE_CATEGORY(V4L2Stats) > > The `V4L2Params` category is declared in the `ipa` namespace, > so I'd do the same with this as well. > ack > > > + > > +namespace ipa { > > + > > [...] > > + > > +} /* namespace ipa */ > > + > > +} /* namespace libcamera */ > > diff --git a/src/ipa/libipa/v4l2_stats.h b/src/ipa/libipa/v4l2_stats.h > > new file mode 100644 > > index 000000000000..404cb606e135 > > --- /dev/null > > +++ b/src/ipa/libipa/v4l2_stats.h > > @@ -0,0 +1,124 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2026, Ideas On Board > > + * > > + * V4L2 Stats > > + */ > > + > > +#pragma once > > + > > +#include <stdint.h> > > + > > +#include <linux/media/v4l2-isp.h> > > + > > +#include <libcamera/base/log.h> > > +#include <libcamera/base/span.h> > > + > > +namespace libcamera { > > + > > +LOG_DECLARE_CATEGORY(V4L2Stats) > > + > > +namespace ipa { > > + > > +template<typename T> > > +class V4L2StatsBlock > > +{ > > +public: > > + V4L2StatsBlock(const Span<const uint8_t> data) > > + : data_(data) > > + { > > + } > > + > > + virtual ~V4L2StatsBlock() {} > > In the `V4L2Params` case, `virtual` was used to handle the differences > in the rkisp1 case as far as I can recall. Is it necessary here? > Probably not > > > + > > + virtual const T *operator->() const > > + { > > + return reinterpret_cast<const T *>(data_.data()); > > + } > > + > > + virtual const T &operator*() const > > + { > > + return *reinterpret_cast<const T *>(data_.data()); > > + } and has Kieran has pointed out, these shouldn't probably be virtual as well > > + > > + size_t size() const > > + { > > + return data_.size(); > > + } > > + > > +protected: > > + Span<const uint8_t> data_; > > +}; > > + > > +template<typename Traits> > > +class V4L2Stats > > +{ > > +public: > > + V4L2Stats(Span<uint8_t> data, unsigned int version) > > + : data_(data) > > + { > > + struct v4l2_isp_buffer *stats = > > + reinterpret_cast<struct v4l2_isp_buffer *>(data_.data()); > > + used_ = stats->data_size; > > + > > + if (version != stats->version) > > + LOG(V4L2Stats, Error) > > + << "Unsupported v4l2-isp version: " << stats->version; > > + } > > + > > + size_t bytesused() const { return used_; } > > + > > + template<typename Traits::id_type Id> > > + auto block() const > > + { > > + using Details = typename Traits::template id_to_details<Id>; > > + > > + using Type = typename Details::type; > > + constexpr auto kernelId = Details::blockType; > > + > > + auto data = block(kernelId, sizeof(Type)); > > + return V4L2StatsBlock<Type>(data); > > Maybe we could use an `std::optional`? Or does the kernel have some guarantees > about when it will most definitely provide a block? let's say that if you configure the ISP to produce statistics for a block, then the stats should be there. But I guess it makes sense to assume a user of this class wants to check if there actually are stats for the block it desires in the buffer. However, using std::optional<> really gets in the way of the using V4L2StatsBlock::operator-> and V4L2StatsBlock::operator* as a user would have to do something like auto opt = stats->block<Type>(); if (opt) { auto actualStats = *opt; int32_t s = actualStats->s; } Isn't it enough for users of this class to check the size of the returned V4L2StatsBlock to discern if there are stats available for that block or not ? auto stat = stats->block<Type>(); if (stat.size() > 0) { int32_t s = stat->s; } ? (that's actually what happens already in the out-of-tree pipeline that uses V4L2Stats I have) const auto postHist = stats->block<StatsType::PostHist>(); if (postHist.size() == 0) { fillMetadata(context, frameContext, metadata); LOG(RppX1Agc, Error) << "HIST data is missing in statistics"; return; } const auto exm = stats->block<StatsType::Pre1Exm>(); if (exm.size() == 0) { fillMetadata(context, frameContext, metadata); LOG(RppX1Agc, Error) << "EXM data is missing in statistics"; return; } > > > > + } > > + > > +protected: > > + const Span<const uint8_t> block(unsigned int blockType, size_t blockSize) const > > + { > > + struct v4l2_isp_buffer *stats = > > + reinterpret_cast<struct v4l2_isp_buffer *>(data_.data()); > > + > > + __u8 *data = stats->data; > > + while (data < stats->data + stats->data_size) { > > + struct v4l2_isp_block_header *header = > > + reinterpret_cast<struct v4l2_isp_block_header *>(data); > > + > > + if (header->type != blockType) { > > + data += header->size; > > + continue; > > + } > > + > > + if (header->size != blockSize) { > > + LOG(V4L2Stats, Error) > > + << "Block type " << blockType > > + << " size mistmatch: expected " > > + << blockSize << " got:" > > + << header->size; > > + return {}; > > + } > > + > > + Span<const uint8_t> block(data, header->size); > > + return block; > > + } > > + > > + LOG(V4L2Stats, Error) << "Unsupported stats block type: " > > + << blockType; > > + > > + return {}; > > + } > > + > > + Span<uint8_t> data_; > > + size_t used_; > > +}; > > + > > +} /* namespace ipa */ > > + > > +} /* namespace libcamera */ > > >
2026. 05. 07. 10:56 keltezéssel, Jacopo Mondi írta: > Hi Barnabás > > On Wed, May 06, 2026 at 09:48:44AM +0200, Barnabás Pőcze wrote: >> 2026. 05. 05. 18:11 keltezéssel, Jacopo Mondi írta: >>> Add a V4L2Stats class similar in spirit to the existing V4L2Params >>> class to allow IPA modules to easily sub-class it to access ISP >>> statistics blocks serialized into a v4l2_isp_buffer. >>> >>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> >>> --- >>> src/ipa/libipa/meson.build | 2 + >>> src/ipa/libipa/v4l2_stats.cpp | 226 ++++++++++++++++++++++++++++++++++++++++++ >>> src/ipa/libipa/v4l2_stats.h | 124 +++++++++++++++++++++++ >>> 3 files changed, 352 insertions(+) >>> >>> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build >>> index 963c5ee73063..16f4b095f220 100644 >>> --- a/src/ipa/libipa/meson.build >>> +++ b/src/ipa/libipa/meson.build >>> @@ -19,6 +19,7 @@ libipa_headers = files([ >>> 'pwl.h', >>> 'quantized.h', >>> 'v4l2_params.h', >>> + 'v4l2_stats.h', >>> ]) >>> libipa_sources = files([ >>> @@ -40,6 +41,7 @@ libipa_sources = files([ >>> 'pwl.cpp', >>> 'quantized.cpp', >>> 'v4l2_params.cpp', >>> + 'v4l2_stats.cpp', >>> ]) >>> libipa_includes = include_directories('..') >>> diff --git a/src/ipa/libipa/v4l2_stats.cpp b/src/ipa/libipa/v4l2_stats.cpp >>> new file mode 100644 >>> index 000000000000..050e2329dfa9 >>> --- /dev/null >>> +++ b/src/ipa/libipa/v4l2_stats.cpp >>> @@ -0,0 +1,226 @@ >>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */ >>> +/* >>> + * Copyright (C) 2026, Ideas On Board >>> + * >>> + * V4L2 Statistics >>> + */ >>> + >>> +#include "v4l2_stats.h" >>> + >>> +namespace libcamera { >>> + >>> +LOG_DEFINE_CATEGORY(V4L2Stats) >> >> The `V4L2Params` category is declared in the `ipa` namespace, >> so I'd do the same with this as well. >> > > ack > >> >>> + >>> +namespace ipa { >>> + >>> [...] >>> + >>> +} /* namespace ipa */ >>> + >>> +} /* namespace libcamera */ >>> diff --git a/src/ipa/libipa/v4l2_stats.h b/src/ipa/libipa/v4l2_stats.h >>> new file mode 100644 >>> index 000000000000..404cb606e135 >>> --- /dev/null >>> +++ b/src/ipa/libipa/v4l2_stats.h >>> @@ -0,0 +1,124 @@ >>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */ >>> +/* >>> + * Copyright (C) 2026, Ideas On Board >>> + * >>> + * V4L2 Stats >>> + */ >>> + >>> +#pragma once >>> + >>> +#include <stdint.h> >>> + >>> +#include <linux/media/v4l2-isp.h> >>> + >>> +#include <libcamera/base/log.h> >>> +#include <libcamera/base/span.h> >>> + >>> +namespace libcamera { >>> + >>> +LOG_DECLARE_CATEGORY(V4L2Stats) >>> + >>> +namespace ipa { >>> + >>> +template<typename T> >>> +class V4L2StatsBlock >>> +{ >>> +public: >>> + V4L2StatsBlock(const Span<const uint8_t> data) >>> + : data_(data) >>> + { >>> + } >>> + >>> + virtual ~V4L2StatsBlock() {} >> >> In the `V4L2Params` case, `virtual` was used to handle the differences >> in the rkisp1 case as far as I can recall. Is it necessary here? >> > > Probably not > >> >>> + >>> + virtual const T *operator->() const >>> + { >>> + return reinterpret_cast<const T *>(data_.data()); >>> + } >>> + >>> + virtual const T &operator*() const >>> + { >>> + return *reinterpret_cast<const T *>(data_.data()); >>> + } > > and has Kieran has pointed out, these shouldn't probably be virtual as > well > >>> + >>> + size_t size() const >>> + { >>> + return data_.size(); >>> + } >>> + >>> +protected: >>> + Span<const uint8_t> data_; >>> +}; >>> + >>> +template<typename Traits> >>> +class V4L2Stats >>> +{ >>> +public: >>> + V4L2Stats(Span<uint8_t> data, unsigned int version) >>> + : data_(data) >>> + { >>> + struct v4l2_isp_buffer *stats = >>> + reinterpret_cast<struct v4l2_isp_buffer *>(data_.data()); >>> + used_ = stats->data_size; >>> + >>> + if (version != stats->version) >>> + LOG(V4L2Stats, Error) >>> + << "Unsupported v4l2-isp version: " << stats->version; >>> + } >>> + >>> + size_t bytesused() const { return used_; } >>> + >>> + template<typename Traits::id_type Id> >>> + auto block() const >>> + { >>> + using Details = typename Traits::template id_to_details<Id>; >>> + >>> + using Type = typename Details::type; >>> + constexpr auto kernelId = Details::blockType; >>> + >>> + auto data = block(kernelId, sizeof(Type)); >>> + return V4L2StatsBlock<Type>(data); >> >> Maybe we could use an `std::optional`? Or does the kernel have some guarantees >> about when it will most definitely provide a block? > > let's say that if you configure the ISP to produce statistics for a block, > then the stats should be there. But I guess it makes sense to assume a > user of this class wants to check if there actually are stats for the > block it desires in the buffer. > > However, using std::optional<> really gets in the way of the using > V4L2StatsBlock::operator-> and V4L2StatsBlock::operator* as a user > would have to do something like > > auto opt = stats->block<Type>(); > if (opt) { > auto actualStats = *opt; > > int32_t s = actualStats->s; > } > > Isn't it enough for users of this class to check the size of the > returned V4L2StatsBlock to discern if there are stats available for > that block or not ? That's true... I suppose `explicit operator bool()` can be added that checks the size, and then auto stat = ...; if (stat) { ... } And now I'm wondering, would it not be sufficient to just return a simple pointer from `block()` (and nullptr if not found)? Is there a need for the separate `V4L2StatsBlock` type? > > auto stat = stats->block<Type>(); > if (stat.size() > 0) { > int32_t s = stat->s; > } > > ? > > (that's actually what happens already in the out-of-tree pipeline that > uses V4L2Stats I have) > > const auto postHist = stats->block<StatsType::PostHist>(); > if (postHist.size() == 0) { > fillMetadata(context, frameContext, metadata); > LOG(RppX1Agc, Error) << "HIST data is missing in statistics"; > return; > } > > const auto exm = stats->block<StatsType::Pre1Exm>(); > if (exm.size() == 0) { > fillMetadata(context, frameContext, metadata); > LOG(RppX1Agc, Error) << "EXM data is missing in statistics"; > return; > } > >> >> >>> + } >>> + >>> +protected: >>> + const Span<const uint8_t> block(unsigned int blockType, size_t blockSize) const >>> + { >>> + struct v4l2_isp_buffer *stats = >>> + reinterpret_cast<struct v4l2_isp_buffer *>(data_.data()); >>> + >>> + __u8 *data = stats->data; >>> + while (data < stats->data + stats->data_size) { >>> + struct v4l2_isp_block_header *header = >>> + reinterpret_cast<struct v4l2_isp_block_header *>(data); >>> + >>> + if (header->type != blockType) { >>> + data += header->size; >>> + continue; >>> + } >>> + >>> + if (header->size != blockSize) { >>> + LOG(V4L2Stats, Error) >>> + << "Block type " << blockType >>> + << " size mistmatch: expected " >>> + << blockSize << " got:" >>> + << header->size; >>> + return {}; >>> + } >>> + >>> + Span<const uint8_t> block(data, header->size); >>> + return block; >>> + } >>> + >>> + LOG(V4L2Stats, Error) << "Unsupported stats block type: " >>> + << blockType; >>> + >>> + return {}; >>> + } >>> + >>> + Span<uint8_t> data_; >>> + size_t used_; >>> +}; >>> + >>> +} /* namespace ipa */ >>> + >>> +} /* namespace libcamera */ >>> >>
On Thu, May 07, 2026 at 12:01:29PM +0200, Barnabás Pőcze wrote: > 2026. 05. 07. 10:56 keltezéssel, Jacopo Mondi írta: > > Hi Barnabás > > > > On Wed, May 06, 2026 at 09:48:44AM +0200, Barnabás Pőcze wrote: > > > 2026. 05. 05. 18:11 keltezéssel, Jacopo Mondi írta: > > > > Add a V4L2Stats class similar in spirit to the existing V4L2Params > > > > class to allow IPA modules to easily sub-class it to access ISP > > > > statistics blocks serialized into a v4l2_isp_buffer. > > > > > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > > --- > > > > src/ipa/libipa/meson.build | 2 + > > > > src/ipa/libipa/v4l2_stats.cpp | 226 ++++++++++++++++++++++++++++++++++++++++++ > > > > src/ipa/libipa/v4l2_stats.h | 124 +++++++++++++++++++++++ > > > > 3 files changed, 352 insertions(+) > > > > > > > > diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build > > > > index 963c5ee73063..16f4b095f220 100644 > > > > --- a/src/ipa/libipa/meson.build > > > > +++ b/src/ipa/libipa/meson.build > > > > @@ -19,6 +19,7 @@ libipa_headers = files([ > > > > 'pwl.h', > > > > 'quantized.h', > > > > 'v4l2_params.h', > > > > + 'v4l2_stats.h', > > > > ]) > > > > libipa_sources = files([ > > > > @@ -40,6 +41,7 @@ libipa_sources = files([ > > > > 'pwl.cpp', > > > > 'quantized.cpp', > > > > 'v4l2_params.cpp', > > > > + 'v4l2_stats.cpp', > > > > ]) > > > > libipa_includes = include_directories('..') > > > > diff --git a/src/ipa/libipa/v4l2_stats.cpp b/src/ipa/libipa/v4l2_stats.cpp > > > > new file mode 100644 > > > > index 000000000000..050e2329dfa9 > > > > --- /dev/null > > > > +++ b/src/ipa/libipa/v4l2_stats.cpp > > > > @@ -0,0 +1,226 @@ > > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > > +/* > > > > + * Copyright (C) 2026, Ideas On Board > > > > + * > > > > + * V4L2 Statistics > > > > + */ > > > > + > > > > +#include "v4l2_stats.h" > > > > + > > > > +namespace libcamera { > > > > + > > > > +LOG_DEFINE_CATEGORY(V4L2Stats) > > > > > > The `V4L2Params` category is declared in the `ipa` namespace, > > > so I'd do the same with this as well. > > > > > > > ack > > > > > > > > > + > > > > +namespace ipa { > > > > + > > > > [...] > > > > + > > > > +} /* namespace ipa */ > > > > + > > > > +} /* namespace libcamera */ > > > > diff --git a/src/ipa/libipa/v4l2_stats.h b/src/ipa/libipa/v4l2_stats.h > > > > new file mode 100644 > > > > index 000000000000..404cb606e135 > > > > --- /dev/null > > > > +++ b/src/ipa/libipa/v4l2_stats.h > > > > @@ -0,0 +1,124 @@ > > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > > +/* > > > > + * Copyright (C) 2026, Ideas On Board > > > > + * > > > > + * V4L2 Stats > > > > + */ > > > > + > > > > +#pragma once > > > > + > > > > +#include <stdint.h> > > > > + > > > > +#include <linux/media/v4l2-isp.h> > > > > + > > > > +#include <libcamera/base/log.h> > > > > +#include <libcamera/base/span.h> > > > > + > > > > +namespace libcamera { > > > > + > > > > +LOG_DECLARE_CATEGORY(V4L2Stats) > > > > + > > > > +namespace ipa { > > > > + > > > > +template<typename T> > > > > +class V4L2StatsBlock > > > > +{ > > > > +public: > > > > + V4L2StatsBlock(const Span<const uint8_t> data) > > > > + : data_(data) > > > > + { > > > > + } > > > > + > > > > + virtual ~V4L2StatsBlock() {} > > > > > > In the `V4L2Params` case, `virtual` was used to handle the differences > > > in the rkisp1 case as far as I can recall. Is it necessary here? > > > > > > > Probably not > > > > > > > > > + > > > > + virtual const T *operator->() const > > > > + { > > > > + return reinterpret_cast<const T *>(data_.data()); > > > > + } > > > > + > > > > + virtual const T &operator*() const > > > > + { > > > > + return *reinterpret_cast<const T *>(data_.data()); > > > > + } > > > > and has Kieran has pointed out, these shouldn't probably be virtual as > > well > > > > > > + > > > > + size_t size() const > > > > + { > > > > + return data_.size(); > > > > + } > > > > + > > > > +protected: > > > > + Span<const uint8_t> data_; > > > > +}; > > > > + > > > > +template<typename Traits> > > > > +class V4L2Stats > > > > +{ > > > > +public: > > > > + V4L2Stats(Span<uint8_t> data, unsigned int version) > > > > + : data_(data) > > > > + { > > > > + struct v4l2_isp_buffer *stats = > > > > + reinterpret_cast<struct v4l2_isp_buffer *>(data_.data()); > > > > + used_ = stats->data_size; > > > > + > > > > + if (version != stats->version) > > > > + LOG(V4L2Stats, Error) > > > > + << "Unsupported v4l2-isp version: " << stats->version; > > > > + } > > > > + > > > > + size_t bytesused() const { return used_; } > > > > + > > > > + template<typename Traits::id_type Id> > > > > + auto block() const > > > > + { > > > > + using Details = typename Traits::template id_to_details<Id>; > > > > + > > > > + using Type = typename Details::type; > > > > + constexpr auto kernelId = Details::blockType; > > > > + > > > > + auto data = block(kernelId, sizeof(Type)); > > > > + return V4L2StatsBlock<Type>(data); > > > > > > Maybe we could use an `std::optional`? Or does the kernel have some guarantees > > > about when it will most definitely provide a block? > > > > let's say that if you configure the ISP to produce statistics for a block, > > then the stats should be there. But I guess it makes sense to assume a > > user of this class wants to check if there actually are stats for the > > block it desires in the buffer. > > > > However, using std::optional<> really gets in the way of the using > > V4L2StatsBlock::operator-> and V4L2StatsBlock::operator* as a user > > would have to do something like > > > > auto opt = stats->block<Type>(); > > if (opt) { > > auto actualStats = *opt; > > > > int32_t s = actualStats->s; > > } > > > > Isn't it enough for users of this class to check the size of the > > returned V4L2StatsBlock to discern if there are stats available for > > that block or not ? > > That's true... I suppose `explicit operator bool()` can be added that checks the size, > and then > > auto stat = ...; > if (stat) { > ... > } > > And now I'm wondering, would it not be sufficient to just return a simple > pointer from `block()` (and nullptr if not found)? Is there a need for the > separate `V4L2StatsBlock` type? Isn't V4L2StatsBlock there to provide the magic in operator-> to cast the returned memory location to the kernel uAPI type ? > > > > > > auto stat = stats->block<Type>(); > > if (stat.size() > 0) { > > int32_t s = stat->s; > > } > > > > ? > > > > (that's actually what happens already in the out-of-tree pipeline that > > uses V4L2Stats I have) > > > > const auto postHist = stats->block<StatsType::PostHist>(); > > if (postHist.size() == 0) { > > fillMetadata(context, frameContext, metadata); > > LOG(RppX1Agc, Error) << "HIST data is missing in statistics"; > > return; > > } > > > > const auto exm = stats->block<StatsType::Pre1Exm>(); > > if (exm.size() == 0) { > > fillMetadata(context, frameContext, metadata); > > LOG(RppX1Agc, Error) << "EXM data is missing in statistics"; > > return; > > } > > > > > > > > > > > > + } > > > > + > > > > +protected: > > > > + const Span<const uint8_t> block(unsigned int blockType, size_t blockSize) const > > > > + { > > > > + struct v4l2_isp_buffer *stats = > > > > + reinterpret_cast<struct v4l2_isp_buffer *>(data_.data()); > > > > + > > > > + __u8 *data = stats->data; > > > > + while (data < stats->data + stats->data_size) { > > > > + struct v4l2_isp_block_header *header = > > > > + reinterpret_cast<struct v4l2_isp_block_header *>(data); > > > > + > > > > + if (header->type != blockType) { > > > > + data += header->size; > > > > + continue; > > > > + } > > > > + > > > > + if (header->size != blockSize) { > > > > + LOG(V4L2Stats, Error) > > > > + << "Block type " << blockType > > > > + << " size mistmatch: expected " > > > > + << blockSize << " got:" > > > > + << header->size; > > > > + return {}; > > > > + } > > > > + > > > > + Span<const uint8_t> block(data, header->size); > > > > + return block; > > > > + } > > > > + > > > > + LOG(V4L2Stats, Error) << "Unsupported stats block type: " > > > > + << blockType; > > > > + > > > > + return {}; > > > > + } > > > > + > > > > + Span<uint8_t> data_; > > > > + size_t used_; > > > > +}; > > > > + > > > > +} /* namespace ipa */ > > > > + > > > > +} /* namespace libcamera */ > > > > > > > >
2026. 05. 07. 12:13 keltezéssel, Jacopo Mondi írta: > > On Thu, May 07, 2026 at 12:01:29PM +0200, Barnabás Pőcze wrote: >> 2026. 05. 07. 10:56 keltezéssel, Jacopo Mondi írta: >>> Hi Barnabás >>> >>> On Wed, May 06, 2026 at 09:48:44AM +0200, Barnabás Pőcze wrote: >>>> 2026. 05. 05. 18:11 keltezéssel, Jacopo Mondi írta: >>>>> Add a V4L2Stats class similar in spirit to the existing V4L2Params >>>>> class to allow IPA modules to easily sub-class it to access ISP >>>>> statistics blocks serialized into a v4l2_isp_buffer. >>>>> >>>>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> >>>>> --- >>>>> src/ipa/libipa/meson.build | 2 + >>>>> src/ipa/libipa/v4l2_stats.cpp | 226 ++++++++++++++++++++++++++++++++++++++++++ >>>>> src/ipa/libipa/v4l2_stats.h | 124 +++++++++++++++++++++++ >>>>> 3 files changed, 352 insertions(+) >>>>> >>>>> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build >>>>> index 963c5ee73063..16f4b095f220 100644 >>>>> --- a/src/ipa/libipa/meson.build >>>>> +++ b/src/ipa/libipa/meson.build >>>>> @@ -19,6 +19,7 @@ libipa_headers = files([ >>>>> 'pwl.h', >>>>> 'quantized.h', >>>>> 'v4l2_params.h', >>>>> + 'v4l2_stats.h', >>>>> ]) >>>>> libipa_sources = files([ >>>>> @@ -40,6 +41,7 @@ libipa_sources = files([ >>>>> 'pwl.cpp', >>>>> 'quantized.cpp', >>>>> 'v4l2_params.cpp', >>>>> + 'v4l2_stats.cpp', >>>>> ]) >>>>> libipa_includes = include_directories('..') >>>>> diff --git a/src/ipa/libipa/v4l2_stats.cpp b/src/ipa/libipa/v4l2_stats.cpp >>>>> new file mode 100644 >>>>> index 000000000000..050e2329dfa9 >>>>> --- /dev/null >>>>> +++ b/src/ipa/libipa/v4l2_stats.cpp >>>>> @@ -0,0 +1,226 @@ >>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */ >>>>> +/* >>>>> + * Copyright (C) 2026, Ideas On Board >>>>> + * >>>>> + * V4L2 Statistics >>>>> + */ >>>>> + >>>>> +#include "v4l2_stats.h" >>>>> + >>>>> +namespace libcamera { >>>>> + >>>>> +LOG_DEFINE_CATEGORY(V4L2Stats) >>>> >>>> The `V4L2Params` category is declared in the `ipa` namespace, >>>> so I'd do the same with this as well. >>>> >>> >>> ack >>> >>>> >>>>> + >>>>> +namespace ipa { >>>>> + >>>>> [...] >>>>> + >>>>> +} /* namespace ipa */ >>>>> + >>>>> +} /* namespace libcamera */ >>>>> diff --git a/src/ipa/libipa/v4l2_stats.h b/src/ipa/libipa/v4l2_stats.h >>>>> new file mode 100644 >>>>> index 000000000000..404cb606e135 >>>>> --- /dev/null >>>>> +++ b/src/ipa/libipa/v4l2_stats.h >>>>> @@ -0,0 +1,124 @@ >>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */ >>>>> +/* >>>>> + * Copyright (C) 2026, Ideas On Board >>>>> + * >>>>> + * V4L2 Stats >>>>> + */ >>>>> + >>>>> +#pragma once >>>>> + >>>>> +#include <stdint.h> >>>>> + >>>>> +#include <linux/media/v4l2-isp.h> >>>>> + >>>>> +#include <libcamera/base/log.h> >>>>> +#include <libcamera/base/span.h> >>>>> + >>>>> +namespace libcamera { >>>>> + >>>>> +LOG_DECLARE_CATEGORY(V4L2Stats) >>>>> + >>>>> +namespace ipa { >>>>> + >>>>> +template<typename T> >>>>> +class V4L2StatsBlock >>>>> +{ >>>>> +public: >>>>> + V4L2StatsBlock(const Span<const uint8_t> data) >>>>> + : data_(data) >>>>> + { >>>>> + } >>>>> + >>>>> + virtual ~V4L2StatsBlock() {} >>>> >>>> In the `V4L2Params` case, `virtual` was used to handle the differences >>>> in the rkisp1 case as far as I can recall. Is it necessary here? >>>> >>> >>> Probably not >>> >>>> >>>>> + >>>>> + virtual const T *operator->() const >>>>> + { >>>>> + return reinterpret_cast<const T *>(data_.data()); >>>>> + } >>>>> + >>>>> + virtual const T &operator*() const >>>>> + { >>>>> + return *reinterpret_cast<const T *>(data_.data()); >>>>> + } >>> >>> and has Kieran has pointed out, these shouldn't probably be virtual as >>> well >>> >>>>> + >>>>> + size_t size() const >>>>> + { >>>>> + return data_.size(); >>>>> + } >>>>> + >>>>> +protected: >>>>> + Span<const uint8_t> data_; >>>>> +}; >>>>> + >>>>> +template<typename Traits> >>>>> +class V4L2Stats >>>>> +{ >>>>> +public: >>>>> + V4L2Stats(Span<uint8_t> data, unsigned int version) >>>>> + : data_(data) >>>>> + { >>>>> + struct v4l2_isp_buffer *stats = >>>>> + reinterpret_cast<struct v4l2_isp_buffer *>(data_.data()); >>>>> + used_ = stats->data_size; >>>>> + >>>>> + if (version != stats->version) >>>>> + LOG(V4L2Stats, Error) >>>>> + << "Unsupported v4l2-isp version: " << stats->version; >>>>> + } >>>>> + >>>>> + size_t bytesused() const { return used_; } >>>>> + >>>>> + template<typename Traits::id_type Id> >>>>> + auto block() const >>>>> + { >>>>> + using Details = typename Traits::template id_to_details<Id>; >>>>> + >>>>> + using Type = typename Details::type; >>>>> + constexpr auto kernelId = Details::blockType; >>>>> + >>>>> + auto data = block(kernelId, sizeof(Type)); >>>>> + return V4L2StatsBlock<Type>(data); >>>> >>>> Maybe we could use an `std::optional`? Or does the kernel have some guarantees >>>> about when it will most definitely provide a block? >>> >>> let's say that if you configure the ISP to produce statistics for a block, >>> then the stats should be there. But I guess it makes sense to assume a >>> user of this class wants to check if there actually are stats for the >>> block it desires in the buffer. >>> >>> However, using std::optional<> really gets in the way of the using >>> V4L2StatsBlock::operator-> and V4L2StatsBlock::operator* as a user >>> would have to do something like >>> >>> auto opt = stats->block<Type>(); >>> if (opt) { >>> auto actualStats = *opt; >>> >>> int32_t s = actualStats->s; >>> } >>> >>> Isn't it enough for users of this class to check the size of the >>> returned V4L2StatsBlock to discern if there are stats available for >>> that block or not ? >> >> That's true... I suppose `explicit operator bool()` can be added that checks the size, >> and then >> >> auto stat = ...; >> if (stat) { >> ... >> } >> >> And now I'm wondering, would it not be sufficient to just return a simple >> pointer from `block()` (and nullptr if not found)? Is there a need for the >> separate `V4L2StatsBlock` type? > > Isn't V4L2StatsBlock there to provide the magic in operator-> to cast > the returned memory location to the kernel uAPI type ? Yes, but returning a pointer to the concrete kernel uapi type would have the same operators. The only difference is the lack of `size()`, but given that the size is guaranteed to be `sizeof(T)`, and given that a nullptr return value could be used to denote missing blocks, it seems to me that switching to a simple pointer would simplify things. (Although I haven't looked at the linked repository in detail, so I might have missed some use cases.) > >> >> >>> >>> auto stat = stats->block<Type>(); >>> if (stat.size() > 0) { >>> int32_t s = stat->s; >>> } >>> >>> ? >>> >>> (that's actually what happens already in the out-of-tree pipeline that >>> uses V4L2Stats I have) >>> >>> const auto postHist = stats->block<StatsType::PostHist>(); >>> if (postHist.size() == 0) { >>> fillMetadata(context, frameContext, metadata); >>> LOG(RppX1Agc, Error) << "HIST data is missing in statistics"; >>> return; >>> } >>> >>> const auto exm = stats->block<StatsType::Pre1Exm>(); >>> if (exm.size() == 0) { >>> fillMetadata(context, frameContext, metadata); >>> LOG(RppX1Agc, Error) << "EXM data is missing in statistics"; >>> return; >>> } >>> >>>> >>>> >>>>> + } >>>>> + >>>>> +protected: >>>>> + const Span<const uint8_t> block(unsigned int blockType, size_t blockSize) const >>>>> + { >>>>> + struct v4l2_isp_buffer *stats = >>>>> + reinterpret_cast<struct v4l2_isp_buffer *>(data_.data()); >>>>> + >>>>> + __u8 *data = stats->data; >>>>> + while (data < stats->data + stats->data_size) { >>>>> + struct v4l2_isp_block_header *header = >>>>> + reinterpret_cast<struct v4l2_isp_block_header *>(data); >>>>> + >>>>> + if (header->type != blockType) { >>>>> + data += header->size; >>>>> + continue; >>>>> + } >>>>> + >>>>> + if (header->size != blockSize) { >>>>> + LOG(V4L2Stats, Error) >>>>> + << "Block type " << blockType >>>>> + << " size mistmatch: expected " >>>>> + << blockSize << " got:" >>>>> + << header->size; >>>>> + return {}; >>>>> + } >>>>> + >>>>> + Span<const uint8_t> block(data, header->size); >>>>> + return block; >>>>> + } >>>>> + >>>>> + LOG(V4L2Stats, Error) << "Unsupported stats block type: " >>>>> + << blockType; >>>>> + >>>>> + return {}; >>>>> + } >>>>> + >>>>> + Span<uint8_t> data_; >>>>> + size_t used_; >>>>> +}; >>>>> + >>>>> +} /* namespace ipa */ >>>>> + >>>>> +} /* namespace libcamera */ >>>>> >>>> >>
Hi Barnabás On Thu, May 07, 2026 at 12:17:27PM +0200, Barnabás Pőcze wrote: > 2026. 05. 07. 12:13 keltezéssel, Jacopo Mondi írta: > > > > On Thu, May 07, 2026 at 12:01:29PM +0200, Barnabás Pőcze wrote: > > > 2026. 05. 07. 10:56 keltezéssel, Jacopo Mondi írta: > > > > Hi Barnabás > > > > > > > > On Wed, May 06, 2026 at 09:48:44AM +0200, Barnabás Pőcze wrote: > > > > > 2026. 05. 05. 18:11 keltezéssel, Jacopo Mondi írta: > > > > > > Add a V4L2Stats class similar in spirit to the existing V4L2Params > > > > > > class to allow IPA modules to easily sub-class it to access ISP > > > > > > statistics blocks serialized into a v4l2_isp_buffer. > > > > > > > > > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > > > > --- > > > > > > src/ipa/libipa/meson.build | 2 + > > > > > > src/ipa/libipa/v4l2_stats.cpp | 226 ++++++++++++++++++++++++++++++++++++++++++ > > > > > > src/ipa/libipa/v4l2_stats.h | 124 +++++++++++++++++++++++ > > > > > > 3 files changed, 352 insertions(+) > > > > > > > > > > > > diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build > > > > > > index 963c5ee73063..16f4b095f220 100644 > > > > > > --- a/src/ipa/libipa/meson.build > > > > > > +++ b/src/ipa/libipa/meson.build > > > > > > @@ -19,6 +19,7 @@ libipa_headers = files([ > > > > > > 'pwl.h', > > > > > > 'quantized.h', > > > > > > 'v4l2_params.h', > > > > > > + 'v4l2_stats.h', > > > > > > ]) > > > > > > libipa_sources = files([ > > > > > > @@ -40,6 +41,7 @@ libipa_sources = files([ > > > > > > 'pwl.cpp', > > > > > > 'quantized.cpp', > > > > > > 'v4l2_params.cpp', > > > > > > + 'v4l2_stats.cpp', > > > > > > ]) > > > > > > libipa_includes = include_directories('..') > > > > > > diff --git a/src/ipa/libipa/v4l2_stats.cpp b/src/ipa/libipa/v4l2_stats.cpp > > > > > > new file mode 100644 > > > > > > index 000000000000..050e2329dfa9 > > > > > > --- /dev/null > > > > > > +++ b/src/ipa/libipa/v4l2_stats.cpp > > > > > > @@ -0,0 +1,226 @@ > > > > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > > > > +/* > > > > > > + * Copyright (C) 2026, Ideas On Board > > > > > > + * > > > > > > + * V4L2 Statistics > > > > > > + */ > > > > > > + > > > > > > +#include "v4l2_stats.h" > > > > > > + > > > > > > +namespace libcamera { > > > > > > + > > > > > > +LOG_DEFINE_CATEGORY(V4L2Stats) > > > > > > > > > > The `V4L2Params` category is declared in the `ipa` namespace, > > > > > so I'd do the same with this as well. > > > > > > > > > > > > > ack > > > > > > > > > > > > > > > + > > > > > > +namespace ipa { > > > > > > + > > > > > > [...] > > > > > > + > > > > > > +} /* namespace ipa */ > > > > > > + > > > > > > +} /* namespace libcamera */ > > > > > > diff --git a/src/ipa/libipa/v4l2_stats.h b/src/ipa/libipa/v4l2_stats.h > > > > > > new file mode 100644 > > > > > > index 000000000000..404cb606e135 > > > > > > --- /dev/null > > > > > > +++ b/src/ipa/libipa/v4l2_stats.h > > > > > > @@ -0,0 +1,124 @@ > > > > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > > > > +/* > > > > > > + * Copyright (C) 2026, Ideas On Board > > > > > > + * > > > > > > + * V4L2 Stats > > > > > > + */ > > > > > > + > > > > > > +#pragma once > > > > > > + > > > > > > +#include <stdint.h> > > > > > > + > > > > > > +#include <linux/media/v4l2-isp.h> > > > > > > + > > > > > > +#include <libcamera/base/log.h> > > > > > > +#include <libcamera/base/span.h> > > > > > > + > > > > > > +namespace libcamera { > > > > > > + > > > > > > +LOG_DECLARE_CATEGORY(V4L2Stats) > > > > > > + > > > > > > +namespace ipa { > > > > > > + > > > > > > +template<typename T> > > > > > > +class V4L2StatsBlock > > > > > > +{ > > > > > > +public: > > > > > > + V4L2StatsBlock(const Span<const uint8_t> data) > > > > > > + : data_(data) > > > > > > + { > > > > > > + } > > > > > > + > > > > > > + virtual ~V4L2StatsBlock() {} > > > > > > > > > > In the `V4L2Params` case, `virtual` was used to handle the differences > > > > > in the rkisp1 case as far as I can recall. Is it necessary here? > > > > > > > > > > > > > Probably not > > > > > > > > > > > > > > > + > > > > > > + virtual const T *operator->() const > > > > > > + { > > > > > > + return reinterpret_cast<const T *>(data_.data()); > > > > > > + } > > > > > > + > > > > > > + virtual const T &operator*() const > > > > > > + { > > > > > > + return *reinterpret_cast<const T *>(data_.data()); > > > > > > + } > > > > > > > > and has Kieran has pointed out, these shouldn't probably be virtual as > > > > well > > > > > > > > > > + > > > > > > + size_t size() const > > > > > > + { > > > > > > + return data_.size(); > > > > > > + } > > > > > > + > > > > > > +protected: > > > > > > + Span<const uint8_t> data_; > > > > > > +}; > > > > > > + > > > > > > +template<typename Traits> > > > > > > +class V4L2Stats > > > > > > +{ > > > > > > +public: > > > > > > + V4L2Stats(Span<uint8_t> data, unsigned int version) > > > > > > + : data_(data) > > > > > > + { > > > > > > + struct v4l2_isp_buffer *stats = > > > > > > + reinterpret_cast<struct v4l2_isp_buffer *>(data_.data()); > > > > > > + used_ = stats->data_size; > > > > > > + > > > > > > + if (version != stats->version) > > > > > > + LOG(V4L2Stats, Error) > > > > > > + << "Unsupported v4l2-isp version: " << stats->version; > > > > > > + } > > > > > > + > > > > > > + size_t bytesused() const { return used_; } > > > > > > + > > > > > > + template<typename Traits::id_type Id> > > > > > > + auto block() const > > > > > > + { > > > > > > + using Details = typename Traits::template id_to_details<Id>; > > > > > > + > > > > > > + using Type = typename Details::type; > > > > > > + constexpr auto kernelId = Details::blockType; > > > > > > + > > > > > > + auto data = block(kernelId, sizeof(Type)); > > > > > > + return V4L2StatsBlock<Type>(data); > > > > > > > > > > Maybe we could use an `std::optional`? Or does the kernel have some guarantees > > > > > about when it will most definitely provide a block? > > > > > > > > let's say that if you configure the ISP to produce statistics for a block, > > > > then the stats should be there. But I guess it makes sense to assume a > > > > user of this class wants to check if there actually are stats for the > > > > block it desires in the buffer. > > > > > > > > However, using std::optional<> really gets in the way of the using > > > > V4L2StatsBlock::operator-> and V4L2StatsBlock::operator* as a user > > > > would have to do something like > > > > > > > > auto opt = stats->block<Type>(); > > > > if (opt) { > > > > auto actualStats = *opt; > > > > > > > > int32_t s = actualStats->s; > > > > } > > > > > > > > Isn't it enough for users of this class to check the size of the > > > > returned V4L2StatsBlock to discern if there are stats available for > > > > that block or not ? > > > > > > That's true... I suppose `explicit operator bool()` can be added that checks the size, > > > and then > > > > > > auto stat = ...; > > > if (stat) { > > > ... > > > } > > > > > > And now I'm wondering, would it not be sufficient to just return a simple > > > pointer from `block()` (and nullptr if not found)? Is there a need for the > > > separate `V4L2StatsBlock` type? > > > > Isn't V4L2StatsBlock there to provide the magic in operator-> to cast > > the returned memory location to the kernel uAPI type ? > > Yes, but returning a pointer to the concrete kernel uapi type would have > the same operators. The only difference is the lack of `size()`, but given The abstraction cost is so cheap I would rather keep it there. None of the currently upstream driver uses extensible stats, but it might be the case some of them will have to be ported and we will need a place where to handle both formats like we do for rkisp1 params ? > that the size is guaranteed to be `sizeof(T)`, and given that a nullptr > return value could be used to denote missing blocks, it seems to me that > switching to a simple pointer would simplify things. (Although I haven't > looked at the linked repository in detail, so I might have missed some > use cases.) > > > > > > > > > > > > > > > > > > auto stat = stats->block<Type>(); > > > > if (stat.size() > 0) { > > > > int32_t s = stat->s; > > > > } > > > > > > > > ? > > > > > > > > (that's actually what happens already in the out-of-tree pipeline that > > > > uses V4L2Stats I have) > > > > > > > > const auto postHist = stats->block<StatsType::PostHist>(); > > > > if (postHist.size() == 0) { > > > > fillMetadata(context, frameContext, metadata); > > > > LOG(RppX1Agc, Error) << "HIST data is missing in statistics"; > > > > return; > > > > } > > > > > > > > const auto exm = stats->block<StatsType::Pre1Exm>(); > > > > if (exm.size() == 0) { > > > > fillMetadata(context, frameContext, metadata); > > > > LOG(RppX1Agc, Error) << "EXM data is missing in statistics"; > > > > return; > > > > } > > > > > > > > > > > > > > > > > > > > + } > > > > > > + > > > > > > +protected: > > > > > > + const Span<const uint8_t> block(unsigned int blockType, size_t blockSize) const > > > > > > + { > > > > > > + struct v4l2_isp_buffer *stats = > > > > > > + reinterpret_cast<struct v4l2_isp_buffer *>(data_.data()); > > > > > > + > > > > > > + __u8 *data = stats->data; > > > > > > + while (data < stats->data + stats->data_size) { > > > > > > + struct v4l2_isp_block_header *header = > > > > > > + reinterpret_cast<struct v4l2_isp_block_header *>(data); > > > > > > + > > > > > > + if (header->type != blockType) { > > > > > > + data += header->size; > > > > > > + continue; > > > > > > + } > > > > > > + > > > > > > + if (header->size != blockSize) { > > > > > > + LOG(V4L2Stats, Error) > > > > > > + << "Block type " << blockType > > > > > > + << " size mistmatch: expected " > > > > > > + << blockSize << " got:" > > > > > > + << header->size; > > > > > > + return {}; > > > > > > + } > > > > > > + > > > > > > + Span<const uint8_t> block(data, header->size); > > > > > > + return block; > > > > > > + } > > > > > > + > > > > > > + LOG(V4L2Stats, Error) << "Unsupported stats block type: " > > > > > > + << blockType; > > > > > > + > > > > > > + return {}; > > > > > > + } > > > > > > + > > > > > > + Span<uint8_t> data_; > > > > > > + size_t used_; > > > > > > +}; > > > > > > + > > > > > > +} /* namespace ipa */ > > > > > > + > > > > > > +} /* namespace libcamera */ > > > > > > > > > > > > > > >
diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build index 963c5ee73063..16f4b095f220 100644 --- a/src/ipa/libipa/meson.build +++ b/src/ipa/libipa/meson.build @@ -19,6 +19,7 @@ libipa_headers = files([ 'pwl.h', 'quantized.h', 'v4l2_params.h', + 'v4l2_stats.h', ]) libipa_sources = files([ @@ -40,6 +41,7 @@ libipa_sources = files([ 'pwl.cpp', 'quantized.cpp', 'v4l2_params.cpp', + 'v4l2_stats.cpp', ]) libipa_includes = include_directories('..') diff --git a/src/ipa/libipa/v4l2_stats.cpp b/src/ipa/libipa/v4l2_stats.cpp new file mode 100644 index 000000000000..050e2329dfa9 --- /dev/null +++ b/src/ipa/libipa/v4l2_stats.cpp @@ -0,0 +1,226 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2026, Ideas On Board + * + * V4L2 Statistics + */ + +#include "v4l2_stats.h" + +namespace libcamera { + +LOG_DEFINE_CATEGORY(V4L2Stats) + +namespace ipa { + +/** + * \file v4l2_stats.cpp + * \brief Helper class to handle an ISP statistics buffer compatible with + * the generic V4L2 ISP format + * + * The Linux kernel defines a generic buffer format for ISP statistics. + * The format describes a serialisation method that allows userspace to + * access statistics data from a binary buffer. + * + * The V4L2Stats class implements support the V4L2 ISP statistics buffer format + * and allows users to retrieve an ISP statistics blocks, represented as + * V4L2StatsBlock class instances. + * + * IPA implementations using this helpers should define an enumeration of ISP + * blocks supported by the IPA module and use a set of common abstraction to + * help their derived implementation of V4L2Stats translate the enumerated ISP + * block identifiers to the actual type of the statistics data as defined by + * the kernel interface. + */ + +/** + * \class V4L2StatsBlock + * \brief Helper class that represents an ISP statistics block + * + * Each ISP measurement block produces a set of statistics data whose memory + * layout is defined by the kernel interface. + * + * This class represents an ISP statistics block entry. It is constructed + * with a reference to the memory area where the statistics block is + * stored in the statistics buffer. The template parameter represents + * the underlying kernel-defined ISP statistics block type and allows its + * user to easily cast it to said type to read the statistics data. + * + * \sa V4L2Stats::block() + */ + +/** + * \fn V4L2StatsBlock::V4L2StatsBlock() + * \brief Construct a V4L2StatsBlock with memory represented by \a data + * \param[in] data The memory area where the statistics block is located + */ + +/** + * \fn V4L2StatsBlock::operator->() const + * \brief Access the statistics block casting it to the kernel-defined + * ISP block type + * + * The V4L2StatsBlock is templated with the kernel defined ISP block type. This + * function allows users to easily cast a V4L2StatsBlock to the underlying + * kernel-defined type in order to easily access the statistics data. + * + * \code{.cpp} + * + * // The kernel header defines the ISP statistics types, in example + * // struct my_isp_awb_stats_data { + * // u16 mean_r; + * // u16 mean_g; + * // u16 mean_b; + * // } + * + * template<> V4L2StatsBlock<struct my_isp_awb_stats_data> awbStats = ... + * + * u16 mean_r = awbStats->mean_r; + * u16 mean_g = awbStats->mean_g; + * u16 mean_b = awbStats->mean_b; + * + * \endcode + * + * Users of this class shall not create a V4L2StatsBlock manually but should + * use V4L2Stats::block(). + */ + +/** + * \fn V4L2StatsBlock::operator*() const + * \copydoc V4L2StatsBlock::operator->() const + */ + +/** + * \fn V4L2StatsBlock::size() const + * \brief Retrieve the size (in bytes) of the ISP statistics block, including + * the block's header + * \return The size of the stats block + */ + +/** + * \var V4L2StatsBlock::data_ + * \brief Memory area where ISP statistics have been serialized + */ + +/** + * \class V4L2Stats + * \brief Helper class that represent an ISP statistics buffer + * + * This class represents an ISP statistics buffer. It is constructed with a + * reference to the memory mapped buffer that has been dequeued from the ISP + * driver. + * + * This class is templated with the type of the enumeration of ISP blocks that + * each IPA module is expected to support. IPA modules are expected to derive + * this class by providing a 'stats_traits' type that helps the class associate + * a block type with the actual memory area that represents the ISP statistics + * block. + * + * \code{.cpp} + * + * // Define the supported ISP statistics blocks + * enum class myISPStats { + * Agc, + * Awb, + * ... + * }; + * + * // Maps the C++ enum type to the kernel enum type and concrete parameter type + * template<myISPStats B> + * struct block_type { + * }; + * + * template<> + * struct block_type<myISPStats::Agc> { + * using type = struct my_isp_kernel_stats_type_agc; + * static constexpr kernel_enum_type blockType = MY_ISP_STATS_TYPE_AGC; + * }; + * + * template<> + * struct block_type<myISPStats::Awb> { + * using type = struct my_isp_kernel_stats_type_awb; + * static constexpr kernel_enum_type blockType = MY_ISP_STATS_TYPE_AWB; + * }; + * + * + * // Convenience type to associate a block id to the 'block_type' overload + * struct stats_traits { + * using id_type = myISPStats; + * template<id_type Id> using id_to_details = block_type<Id>; + * }; + * + * ... + * + * // Derive the V4L2Stats class by providing stats_traits + * class MyISPStats : public V4L2Stats<stats_traits> + * { + * public: + * MyISPStats::MyISPStats(Span<uint8_t> data) + * : V4L2Stats(data) + * { + * } + * }; + * + * \endcode + * + * Users of this class can then easily access an ISP statistics block as a + * V4L2StatsBlock instance. + * + * \code{.cpp} + * + * MyISPStats stats(data); + * + * auto awb = stats.block<myISPStats::AWB>(); + * auto mean_r = awb->mean_r; + * auto mean_g = awb->mean_g; + * auto mean_b = awb->mean_b; + * \endcode + */ + +/** + * \fn V4L2Stats::V4L2Stats() + * \brief Construct an instance of V4L2Stats + * \param[in] data Reference to the v4l2-buffer memory mapped area + * \param[in] version The expected V4L2 ISP serialization format version + */ + +/** + * \fn V4L2Stats::bytesused() + * \brief Retrieve the size of the statistics buffer (in bytes) + * \return The number of bytes occupied by the ISP statistics + */ + +/** + * \fn V4L2Stats::block() const + * \brief Retrieve the location of an ISP statistics block a return it + * \return A V4L2StatsBlock instance that points to the ISP statistics block + */ + +/** + * \fn V4L2Stats::block(unsigned int blockType, size_t blockSize) const + * \brief Retrieve an ISP statistics block a returns a reference to it + * \param[in] blockType The kernel-defined ISP block identifier, used to + * identify the block header + * \param[in] blockSize The ISP statistics block size, for validation + * + * Parse the buffer of serialized statistics data and retrieve the location + * of the block with type \a blockType of size \a blockSize. + * + * IPA modules that derive the V4L2Stats class shall use this function to + * retrieve the memory area that will be used to construct a V4L2StatsBlock<T> + * before returning it to the caller. + */ + +/** + * \var V4L2Stats::data_ + * \brief The ISP statistics buffer memory + */ + +/** + * \var V4L2Stats::used_ + * \brief The number of bytes used by the statistics buffer + */ + +} /* namespace ipa */ + +} /* namespace libcamera */ diff --git a/src/ipa/libipa/v4l2_stats.h b/src/ipa/libipa/v4l2_stats.h new file mode 100644 index 000000000000..404cb606e135 --- /dev/null +++ b/src/ipa/libipa/v4l2_stats.h @@ -0,0 +1,124 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2026, Ideas On Board + * + * V4L2 Stats + */ + +#pragma once + +#include <stdint.h> + +#include <linux/media/v4l2-isp.h> + +#include <libcamera/base/log.h> +#include <libcamera/base/span.h> + +namespace libcamera { + +LOG_DECLARE_CATEGORY(V4L2Stats) + +namespace ipa { + +template<typename T> +class V4L2StatsBlock +{ +public: + V4L2StatsBlock(const Span<const uint8_t> data) + : data_(data) + { + } + + virtual ~V4L2StatsBlock() {} + + virtual const T *operator->() const + { + return reinterpret_cast<const T *>(data_.data()); + } + + virtual const T &operator*() const + { + return *reinterpret_cast<const T *>(data_.data()); + } + + size_t size() const + { + return data_.size(); + } + +protected: + Span<const uint8_t> data_; +}; + +template<typename Traits> +class V4L2Stats +{ +public: + V4L2Stats(Span<uint8_t> data, unsigned int version) + : data_(data) + { + struct v4l2_isp_buffer *stats = + reinterpret_cast<struct v4l2_isp_buffer *>(data_.data()); + used_ = stats->data_size; + + if (version != stats->version) + LOG(V4L2Stats, Error) + << "Unsupported v4l2-isp version: " << stats->version; + } + + size_t bytesused() const { return used_; } + + template<typename Traits::id_type Id> + auto block() const + { + using Details = typename Traits::template id_to_details<Id>; + + using Type = typename Details::type; + constexpr auto kernelId = Details::blockType; + + auto data = block(kernelId, sizeof(Type)); + return V4L2StatsBlock<Type>(data); + } + +protected: + const Span<const uint8_t> block(unsigned int blockType, size_t blockSize) const + { + struct v4l2_isp_buffer *stats = + reinterpret_cast<struct v4l2_isp_buffer *>(data_.data()); + + __u8 *data = stats->data; + while (data < stats->data + stats->data_size) { + struct v4l2_isp_block_header *header = + reinterpret_cast<struct v4l2_isp_block_header *>(data); + + if (header->type != blockType) { + data += header->size; + continue; + } + + if (header->size != blockSize) { + LOG(V4L2Stats, Error) + << "Block type " << blockType + << " size mistmatch: expected " + << blockSize << " got:" + << header->size; + return {}; + } + + Span<const uint8_t> block(data, header->size); + return block; + } + + LOG(V4L2Stats, Error) << "Unsupported stats block type: " + << blockType; + + return {}; + } + + Span<uint8_t> data_; + size_t used_; +}; + +} /* namespace ipa */ + +} /* namespace libcamera */
Add a V4L2Stats class similar in spirit to the existing V4L2Params class to allow IPA modules to easily sub-class it to access ISP statistics blocks serialized into a v4l2_isp_buffer. Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> --- src/ipa/libipa/meson.build | 2 + src/ipa/libipa/v4l2_stats.cpp | 226 ++++++++++++++++++++++++++++++++++++++++++ src/ipa/libipa/v4l2_stats.h | 124 +++++++++++++++++++++++ 3 files changed, 352 insertions(+)