Re: [PATCH 0/6] media: v4l2-isp: Add support for extensible statistics

[Jacopo Mondi <jacopo.mondi@ideasonboard.com>]

Hi Hans

On Wed, Jun 03, 2026 at 10:41:09AM +0200, Hans Verkuil wrote:
> On 6/3/26 10:17, Jacopo Mondi wrote:
> > Hi Antoine
> >
> > On Mon, Jun 01, 2026 at 03:29:54PM +0200, Antoine Bouyer wrote:
> >> On 5/27/26 2:09 PM, Jacopo Mondi wrote:
> >>>
> >>>
> >>> Hi Sakari,
> >>>
> >>> On Fri, May 15, 2026 at 03:11:18PM +0200, Jacopo Mondi wrote:
> >>>> Hi Antoine, Keke
> >>>>
> >>>> On Wed, May 13, 2026 at 09:04:27AM +0800, Keke Li wrote:
> >>>>>
> >>>>> On 5/12/26 17:26, Antoine Bouyer wrote:
> >>>>>> [ EXTERNAL EMAIL ]
> >>>>>>
> >>>>>> Le 05/05/2026 à 18:49, Antoine Bouyer a écrit :
> >>>>>>> On 5/5/26 4:12 PM, Jacopo Mondi wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> This series breaks out from Antonie's
> >>>>>>>> https://eur01.safelinks.protection.outlook.com/?
> >>>>>>>> url=https%3A%2F%2Fpatchwork.linuxtv.org%2Fproject%2Flinux-
> >>>>>>>> media%2Flist%2F%3Fseries%3D24043&data=05%7C02%7Cantoine.bouyer%40nxp.com%7Cd0e9f403856c4146996308deaab05cd1%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C639135871605732002%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=FXaz7QYQvS3s%2B4l9AFMrGgE7kmXlEil%2FKD6DibB0%2FJY%3D&reserved=0
> >>>>>>>>
> >>>>>>>> the extensible stats support and adds a few more patches on top to:
> >>>>>>>>
> >>>>>>>> - add support for per-block validation as suggested during the
> >>>>>>>> review of
> >>>>>>>>     Ricardo's
> >>>>>>>>     https://eur01.safelinks.protection.outlook.com/?
> >>>>>>>> url=https%3A%2F%2Fpatchwork.linuxtv.org%2Fproject%2Flinux-
> >>>>>>>> media%2Fpatch%2F20260504-smatch-7-1-v3-6-
> >>>>>>>> fda125c30058%40chromium.org%2F&data=05%7C02%7Cantoine.bouyer%40nxp.com%7Cd0e9f403856c4146996308deaab05cd1%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C639135871605751612%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=AiRH8MhIbXt3dr%2B2r3I6STE6TJAChylnH%2Fz3tLdS36k%3D&reserved=0
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> - add two helper functions to v4l2-isp to ease handling of extensible
> >>>>>>>>     statistics for drivers. An early user, based on a preliminary
> >>>>>>>> version
> >>>>>>>>     of the patches is available here as a reference:
> >>>>>>>>     https://eur01.safelinks.protection.outlook.com/?
> >>>>>>>> url=https%3A%2F%2Fpatchwork.linuxtv.org%2Fproject%2Flinux-
> >>>>>>>> media%2Flist%2F%3Fseries%3D24703&data=05%7C02%7Cantoine.bouyer%40nxp.com%7Cd0e9f403856c4146996308deaab05cd1%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C639135871605763086%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=%2Fpsb7Z3lb8ingDILxc3LoEWKTojl5BGMbk6FiR%2FcO9I%3D&reserved=0
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Antonie: I took the liberty to fold in your patches changes to address
> >>>>>>>> my comments on your v1. I pushed an un-squased version of the patches
> >>>>>>>> here:
> >>>>>>>> https://eur01.safelinks.protection.outlook.com/?
> >>>>>>>> url=https%3A%2F%2Fgitlab.freedesktop.org%2Flinux-
> >>>>>>>> media%2Fusers%2Fjmondi%2F-%2Ftree%2Fb4%2Fextensible-stats-
> >>>>>>>> unsquashed&data=05%7C02%7Cantoine.bouyer%40nxp.com%7Cd0e9f403856c4146996308deaab05cd1%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C639135871605775020%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=iF2BE0GZ8HcVPmOJDhbmLsXQDOXV9JvqfMK6DRPPvrg%3D&reserved=0
> >>>>>>>>
> >>>>>>>> so you can easily get the diff from this and your version. Please feel
> >>>>>>>> free to comment on these as you're the original author.
> >>>>>>>
> >>>>>>> Hi Jacopo
> >>>>>>>
> >>>>>>> Thanks for the rework and the links. I'm fine with your [SQUASH]
> >>>>>>> commits.
> >>>>>>>
> >>>>>>> If there are no other comments, I assume I can reuse the common patches
> >>>>>>> in my v2 then, and also apply the new helpers to neoisp driver changes.
> >>>>>>> They look very useful (especially to prevent out-of-bounds crashes I
> >>>>>>> observed when data_size was not set before filling stats :( ). I'll
> >>>>>>> check in your user example.
> >>>>>>>
> >>>>>>> BR
> >>>>>>> Antoine
> >>>>>>
> >>>>>> Hi Jacopo
> >>>>>>
> >>>>>> Do you think it would make sense to create a new generic V4L2_META_FMT
> >>>>>> too ? which can be used by all user of v4l2-isp extensible params a/o
> >>>>>> stats. To avoid each driver creating its own meta fmt with same purpose.
> >>>>>>
> >>>>>> Or do you think it could have side effects ?
> >>>>>>
> >>>>>> BR
> >>>>>> Antoine
> >>>>>>
> >>>>>>>
> >>>>> I think this proposal is excellent.
> >>>>> 🙂
> >>>>
> >>>> To me, platform-specific formats mostly serve for documenting the ISP blocks.
> >>>> In example
> >>>> https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/metafmt-rkisp1.html
> >>>>
> >>>> There might be ways to handle it without defining a dedicated format
> >>>> indeed.
> >>>>
> >>>> Sakari Laurent and Hans are in cc, what do they think ?
> >>>>
> >>>
> >>> We briefly discussed it on irc and a few days ago again.
> >>>
> >>> Am I correct you think this is a good idea ?
> >>>
> >>> Antonie, do you plan to include the two new generic formts in your new
> >>> version ? Should we have a single format for STATS and PARAMS too ? I
> >>> see merit in both ways, to be hones two formats sound better to me as
> >>> they apply to two different queue types (output for params and capture
> >>> for stats)
> >>
> >> Hi Jacopo
> >>
> >> I would personally prefer using a single format for both params and stats.
> >>
> >> In my view, the format describes how the meta buffer is structured (header,
> >> size, version, flags, etc.), and it should not depend on the queue type.
> >> Since both stats and params will use the exact same structure, then it makes
> >> sense to me to share a single format. Similar to how a pixel format applies
> >> to both source and sink queues.
> >
> > You certainly have a point here!
>
> Even though how the content is structured in the buffer is the same for both
> parameters and statistics, the actual content is quite different. So the format
> description in v4l2_fmtdesc is also different. When you list available formats
> for a video device it is nice if you can see what the format actually contains.
>
> A V4L2 format has always specified both the structure of the data and what the
> content is (with the odd exception like V4L2_META_FMT_GENERIC_8), and I think
> that should be done here as well.
>
> Interpreting the content of the buffers is quite different for parameters and
> statistics, so just because the high-level structure is the same does IMHO not
> make this a good enough reason to use a single pixelformat. So I recommend
> sticking with two formats.
>
> Unless I'm missing something?

To summarize the day-long discussions on irc (quoting here your
messages)

-------------------------------------------------------------------------------
 1) I think we should have separate pixelformats for parameters and
 statistics (and whatever else we will need in the future). I think
 that makes sense. That way you can also give a sane pixelformat
 description.

 2) If we go for a generic pixelformat rather than a per-platform one,
 then it has to be documented how the application discovers which
 driver it is. It is one of those things that I don't think is
 explicitly stated anywhere. Taking it from the media controller makes
 sense, but it needs to be documented. It also means that the media
 device driver name is part of the ABI. Changing it would break lots
 of things. That too must be documented.
-------------------------------------------------------------------------------

This seems to suggest we're fine with  V4L2_META_FMT_ISP_PARAMS and
one V4L2_META_FTM_ISP_STATS formats, provided we clearly tell
userspace how to identify which platforms it is running on.

I suggested that libcamera matches on the media device driver's name
and from there we know on which platform we're on.

However you made a good point on the fact we discussed about allowing
multiple drivers to register in a single media graphs to support
complex topologies. In this case we should clarify how this would
work. If I'm not mistaken Ricardo had a similar point when talking
about media device names ?

>
> Regards,
>
> 	Hans
>
> >
> >>
> >> That said, if there is a consensus in favor of defining 2 separate formats,
> >> I am fine with following that direction.
> >
> > Let's see if we can get feedback from Sakari/Hans/Laurent.
> > I'll re-ping them!
> >
> >>
> >> For now, I don't plan to integrate the new generic format in my patch
> >> series. I would prefer to wait for converging on a solution (1 or 2 formats)
> >> first. Then, depending on timeline, yes I can integrate it.
> >
> > If you want to re-send a new version out quickly feel free to use any
> > format. I hope we can sort this out quickly so you can rebase on
> > easily.
> >
> > Thanks
> >   j
> >
> >>
> >> BR
> >> Antoine
> >>
> >>>
> >>>>
> >>>>
> >>>>>>>>
> >>>>>>>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> >>>>>>>> ---
> >>>>>>>> Antoine Bouyer (2):
> >>>>>>>>         media: uapi: v4l2-isp: Add extensible statistics
> >>>>>>>>         media: Documentation: uapi: Update V4L2 ISP for extensible stats
> >>>>>>>>
> >>>>>>>> Jacopo Mondi (4):
> >>>>>>>>         media: v4l2-isp: Rename v4l2_isp_params_buffer_size
> >>>>>>>>         media: v4l2-isp: Add per-block validation callback
> >>>>>>>>         media: amlogic-c3: Implement per-block validation
> >>>>>>>>         media: v4l2-isp: Add helpers for stats buffer
> >>>>>>>>
> >>>>>>>>    Documentation/userspace-api/media/v4l/v4l2-isp.rst |  45 ++++++--
> >>>>>>>>    .../media/platform/amlogic/c3/isp/c3-isp-params.c  |  42 ++++++-
> >>>>>>>>    .../media/platform/arm/mali-c55/mali-c55-params.c  |  12 +-
> >>>>>>>>    drivers/media/v4l2-core/v4l2-isp.c                 |  56 +++++++++
> >>>>>>>>    include/media/v4l2-isp.h                           |  94 +++++++++++
> >>>>>>>> ++---
> >>>>>>>>    include/uapi/linux/media/v4l2-isp.h                | 125 +++++++++++
> >>>>>>>> ++--------
> >>>>>>>>    6 files changed, 294 insertions(+), 80 deletions(-)
> >>>>>>>> ---
> >>>>>>>> base-commit: d9c8c4adf23d17549c0ec9c85b99d85a0ee6cf18
> >>>>>>>> change-id: 20260504-extensible-stats-f2d6befcc1ce
> >>>>>>>>
> >>>>>>>> Best regards,
> >>>>>>>> --
> >>>>>>>> Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> >>>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>
> >>
> >
>