Message-ID: <2eec786d-f2b6-4445-87f4-4b6d162a2d9a@collabora.com>
Date: Fri, 19 Jul 2024 15:47:00 +0200
From: Benjamin Gaignard <benjamin.gaignard@...labora.com>
To: Hans Verkuil <hverkuil-cisco@...all.nl>, mchehab@...nel.org,
 ezequiel@...guardiasur.com.ar
Cc: linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-rockchip@...ts.infradead.org, kernel@...labora.com
Subject: Re: [PATCH v4 0/2] Enumerate all pixels formats


Le 19/07/2024 à 15:37, Hans Verkuil a écrit :
> On 19/07/2024 15:15, Benjamin Gaignard wrote:
>> Le 19/07/2024 à 14:57, Hans Verkuil a écrit :
>>> On 17/07/2024 15:14, Benjamin Gaignard wrote:
>>>> The goal of this series is to let userland applications enumerate
>>>> all the supported pixels formats of a stateless decoder without
>>>> setting all the possible codec-dependent control.
>>>> That offer a simplest solution for applications to discover
>>>> supported pixels formats and possibly let them doing smarter
>>>> choice between stateless decoders.
>>>>
>>>> An example of how it can be used in GStreamer to discover the
>>>> supported pixels formats for stateless decoder is available here:
>>>> https://gitlab.freedesktop.org/benjamin.gaignard1/gstreamer/-/commits/v4l2codecs_enum_all_supported_formats?ref_type=heads
>>> So effectively specifying this flag makes ENUM_FMT also return
>>> formats that do not match the bit depth.
>>>
>>> So the AV1 (for example) compressed video uses e.g. 8 bit depth, but instead of just
>>> listing only 8 bit uncompressed pixelformats, you want to list them for any
>>> bit depth.
>>>
>>> But what is the point of that if the decoder can't decode 8 bit compressed to,
>>> say, 10 bit uncompressed video?
>> No decoder will do 8 bits to 10 bits (as far I knows).
>> The point is to be able to say that decoder could produce 10 bit frames without
>> setting a full sps/pps for each case (and for each supported codec).
>>
>>> I actually thought that this flag would just list all formats, independent
>>> of the output format (e.g. AV1, H264, etc.), but that does not appear to be
>>> the case? I.e., if capture pixelformat X is only available with AV1, will that still
>>> be listed if the output pixel is set to H264?
>>>
>>> I think you need to describe a real use-case here, and I am not convinced about
>>> the name of the flag either.
>> I may have miss something but yes the goal is to list all formats independently
>> of the output format.
>> When a SoC have multiple decoders for the same codec, knowing the supported formats
>> is key to select the better one.
>> Since I will have to do more iteration, feel free to provide a better name for the
>> flag(s). I'm always bad for naming this kind of thing.
> That really needs to be clarified, since in patch 1/2 it says:
>
> +   * If the ``V4L2_FMT_FLAG_ENUM_ALL_FORMATS`` flag is set the driver must enumerate
> +     all the supported formats without taking care of codec-dependent controls
> +     set on the ``OUTPUT`` queue. To indicate that the driver has take care of this
> +     flag it must set ``V4L2_FMT_FLAG_ALL_FORMATS`` flag for each format while
> +     enumerating.
>
> Here it just talks about 'codec-dependent controls set on the ``OUTPUT`` queue', it
> doesn't say anything about the compressed pixelformat set for the OUTPUT queue.
>
> And patch 2/2 sets the ignore_depth_match boolean, suggesting also that it is
> not listing all formats, but just ignoring a specific check.
>
> But if you list all pixelformats without taking the OUTPUT pixelformat into
> account, how do you know which pixelformat is valid for which codec?
>
> Say that only MPEG support NV12 (just for the sake of argument), and that's
> what you want to use, you have no way of knowing that NV12 is specific to MPEG,
> you would have to try each codec and see if NV12 is supported for that codec.
>
> I just don't see how this can be used in practice.
>
> What exactly is the problem you want to solve? A real-life problem, not a theoretical
> one :-)

On real-life: on a board with 2 different stateless decoders being able to detect the
one which can decode 10 bits bitstreams without testing all codec-dependent controls.

Regards,
Benjamin

>
> Regards,
>
> 	Hans
>
>>
>>>> changes in version 4:
>>>> - Explicitly document that the new flags are targeting mem2mem devices.
>>>>
>>>> changes in version 3:
>>>> - Add a flag to inform userspace application that driver
>>>>     as take care of the flag.
>>>>
>>>> changes in version 2:
>>>> - Clarify documentation.
>>>> - Only keep V4L2_FMT_FLAG_ALL_FORMATS flag in ioctl.
>>>>
>>>> Benjamin
>>>>
>>>> Benjamin Gaignard (2):
>>>>     media: videodev2: Add flags to unconditionnaly enumerate pixels
>>>>       formats
>>> I.e.: it is not unconditionally, it still depends on the chosen codec.
>>>
>>> Regards,
>>>
>>>      Hans
>>>
>>>>     media: verisilicon: Use V4L2_FMT_FLAG_ENUM_ALL_FORMATS flag
>>>>
>>>>    .../media/v4l/dev-stateless-decoder.rst          |  6 ++++++
>>>>    .../userspace-api/media/v4l/vidioc-enum-fmt.rst  | 11 +++++++++++
>>>>    .../media/videodev2.h.rst.exceptions             |  2 ++
>>>>    drivers/media/platform/verisilicon/hantro_v4l2.c | 16 +++++++++++++---
>>>>    drivers/media/v4l2-core/v4l2-ioctl.c             |  3 +++
>>>>    include/uapi/linux/videodev2.h                   |  2 ++
>>>>    6 files changed, 37 insertions(+), 3 deletions(-)
>>>>
>

Powered by blists - more mailing lists