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

[Antoine Bouyer <antoine.bouyer@nxp.com>]

On 6/3/26 4:53 PM, Jacopo Mondi wrote:
> 
> 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.

Hi Jacopo, Hans

Thanks for your inputs.

Then I will prepare a v3 of my series with these 2 meta formats, as 
separated patches. Actually, I feel like main update is about 
Documentation. So I created dedicated doc for these formats, but 
MAINTAINER is impacted too. Please feel free to review/comment, or 
cherry pick the patches in your own series with extended stats if you 
prefer. I will always rebase anyway.

Best regards
Antoine

> 
> 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>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>
>>>
>>