[RCF 1/2] media: videodev2: Add V4L2_FMT_FLAG_ALL_FORMATS flag
Benjamin Gaignard
benjamin.gaignard at collabora.com
Wed Jan 31 02:06:03 PST 2024
Le 17/01/2024 à 20:41, Nicolas Dufresne a écrit :
> Le jeudi 11 janvier 2024 à 17:07 +0100, Benjamin Gaignard a écrit :
>> Add new flag to allow enumerate all pixels formats when
>> calling VIDIOC_ENUM_FMT ioctl.
>> When this flag is set drivers must ignore the configuration
>> and return the hardware supported pixel formats for the specified queue.
>> This will permit to discover which pixels formats are supported
>> without setting codec-specific information so userland can more easily
>> knows if the driver suit well to what it needs.
>> The main target are stateless decoders so update the documentation
>> about how use this flag.
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard at collabora.com>
>> ---
>> .../userspace-api/media/v4l/dev-stateless-decoder.rst | 3 +++
>> Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst | 4 ++++
>> Documentation/userspace-api/media/videodev2.h.rst.exceptions | 1 +
>> drivers/media/v4l2-core/v4l2-ioctl.c | 2 +-
>> include/uapi/linux/videodev2.h | 1 +
>> 5 files changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/userspace-api/media/v4l/dev-stateless-decoder.rst b/Documentation/userspace-api/media/v4l/dev-stateless-decoder.rst
>> index 35ed05f2695e..b7b650f1a18f 100644
>> --- a/Documentation/userspace-api/media/v4l/dev-stateless-decoder.rst
>> +++ b/Documentation/userspace-api/media/v4l/dev-stateless-decoder.rst
>> @@ -58,6 +58,9 @@ Querying capabilities
>> default values for these controls being used, and a returned set of formats
>> that may not be usable for the media the client is trying to decode.
>>
>> + * If ``V4L2_FMT_FLAG_ALL_FORMATS`` flag is set the driver must enumerate
>> + all the supported formats without taking care of codec-dependent controls.
>> +
>> 3. The client may use :c:func:`VIDIOC_ENUM_FRAMESIZES` to detect supported
>> resolutions for a given format, passing desired pixel format in
>> :c:type:`v4l2_frmsizeenum`'s ``pixel_format``.
>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
>> index 000c154b0f98..db8bc8e29a91 100644
>> --- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
>> +++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
>> @@ -227,6 +227,10 @@ the ``mbus_code`` field is handled differently:
>> The application can ask to configure the quantization of the capture
>> device when calling the :ref:`VIDIOC_S_FMT <VIDIOC_G_FMT>` ioctl with
>> :ref:`V4L2_PIX_FMT_FLAG_SET_CSC <v4l2-pix-fmt-flag-set-csc>` set.
>> + * - ``V4L2_FMT_FLAG_ALL_FORMATS``
>> + - 0x0200
>> + - Set by userland application to enumerate all possible pixels formats
>> + without taking care of the current configuration.
> This is a bit ambiguous regarding if OUTPUT queue FMT is ignored or not. From
> our chat, it is ignored in this implementation. Such if I use MTK VCODEC as an
> example, using that feature would return:
I will reword it for next version, but yes the goal of this flag is to enumerate
all pixels formats without taking care of any queue configuration.
>
> - MM21
> - MT2T
> - MT2R
>
> At high level, the use case is to find an easy way to combine the per codec
> profile information and the pixel format, since userspace can only use e.g.
> 10bit capability if it knows the associated pixel formats. This implementation
> is already useful in my opinion, I'll try and draft a GStreamer change to use
> it, as I think it will better support the idea. But it has come ceavats.
>
> Notably, if you had a userland that implement MT2T (VP9/AV1/HEVC) but not MT2R
> (H264), it would not have an easy API to figure-out. It would still have to
> resort into enumerating formats for each possible codec and codec specific
> compound control configuration.
>
> An alternative is to make this enumerate "all" for the configure OUTPUT format.
> This increase the precision, while still allowing generic code to be used. In
> pseudo code that would be like:
>
> for output formats
> S_FMT(OUTPUT)
>
> for ALL capture formats
> store(format)
>
> Where right now we have do do:
>
>
> for output formats
> S_FMT(OUTPUT)
>
> S_CTRL(codec_specific_ctl_config_1)
> for capture formats
> store(format)
>
>
> S_CTRL(codec_specific_ctl_config_n)
> for capture format
> store(format)
>
> ...
>
> S_CTRL(codec_specific_ctl_config_n)
> for capture formats
> store(format)
>
> Where each config would demote a specific feature, like 10bit, 422, 444, film-
> grain (posprocessing affect output formats). The posprocessing remains a bit
> hard to figure-out in both cases though. But in practice, if I use Hantro AV1
> decoder as an example, I'd get something like:
>
> NV15_4L4
> P010
>
> Where NV15_4L4 is not available with filmgrain filter, but P010 is always
> available. For my GStreamer use case (template caps) this wouldn't be a problem,
> P010 is a well supported format and I strictly need a superset of the supported
> formats.
>
> What I'd really gain is that I don't have to do complicated enumeration logic
> per codec features.
>
> Nicolas
>
> p.s. It would be logical to update dev-stateless-decoder doc, to mention this
> enumeration option. Currently it says:
>
>
> To enumerate the set of supported raw formats, the client calls
> VIDIOC_ENUM_FMT() on the CAPTURE queue.
>
> * The driver must return only the formats supported for the format
> currently active on the OUTPUT queue.
>
> * Depending on the currently set OUTPUT format, the set of supported
> raw formats may depend on the value of some codec-dependent controls. The
> client is responsible for making sure that these controls are set before
> querying the CAPTURE queue. Failure to do so will result in the default
> values for these controls being used, and a returned set of formats that
> may not be usable for the media the client is trying to decode.
I have done it, look at the top of this patch.
>
>
>>
>> Return Value
>> ============
>> diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
>> index 3e58aac4ef0b..42d9075b7fc2 100644
>> --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
>> +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
>> @@ -215,6 +215,7 @@ replace define V4L2_FMT_FLAG_CSC_XFER_FUNC fmtdesc-flags
>> replace define V4L2_FMT_FLAG_CSC_YCBCR_ENC fmtdesc-flags
>> replace define V4L2_FMT_FLAG_CSC_HSV_ENC fmtdesc-flags
>> replace define V4L2_FMT_FLAG_CSC_QUANTIZATION fmtdesc-flags
>> +replace define V4L2_FMT_FLAG_ALL_FORMATS fmtdesc-flags
>>
>> # V4L2 timecode types
>> replace define V4L2_TC_TYPE_24FPS timecode-type
>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
>> index 33076af4dfdb..22a93d074a5b 100644
>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>> @@ -1544,7 +1544,7 @@ static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops,
>> p->mbus_code = 0;
>>
>> mbus_code = p->mbus_code;
>> - memset_after(p, 0, type);
>> + memset_after(p, 0, flags);
> In other similar places, we still clear the flags, and only keep the allowed
> bits. Maybe we should do this here too to avoid accidental flags going through ?
>
> That should maybe be under some capability flag, so that userland knows if the
> driver did implement that feature or not. If the driver didn't set that flag, we
> can then clear it so that userlands not checking that flag would at least get an
> enumeration response without it.
I will do that in next version.
Regards,
Benjamin
>
>> p->mbus_code = mbus_code;
>>
>> switch (p->type) {
>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>> index 68e7ac178cc2..82d8c8a7fb7f 100644
>> --- a/include/uapi/linux/videodev2.h
>> +++ b/include/uapi/linux/videodev2.h
>> @@ -869,6 +869,7 @@ struct v4l2_fmtdesc {
>> #define V4L2_FMT_FLAG_CSC_YCBCR_ENC 0x0080
>> #define V4L2_FMT_FLAG_CSC_HSV_ENC V4L2_FMT_FLAG_CSC_YCBCR_ENC
>> #define V4L2_FMT_FLAG_CSC_QUANTIZATION 0x0100
>> +#define V4L2_FMT_FLAG_ALL_FORMATS 0x0200
>>
>> /* Frame Size and frame rate enumeration */
>> /*
>
More information about the Linux-rockchip
mailing list