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

[Hans Verkuil]

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?

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