linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RCF 0/2] Enumerate all pixels formats
@ 2024-01-11 16:07 Benjamin Gaignard
  2024-01-11 16:07 ` [RCF 1/2] media: videodev2: Add V4L2_FMT_FLAG_ALL_FORMATS flag Benjamin Gaignard
  2024-01-11 16:07 ` [RCF 2/2] media: verisilicon: Use " Benjamin Gaignard
  0 siblings, 2 replies; 5+ messages in thread
From: Benjamin Gaignard @ 2024-01-11 16:07 UTC (permalink / raw)
  To: mchehab, p.zabel, hverkuil-cisco, nicolas.dufresne
  Cc: linux-media, linux-kernel, linux-rockchip, kernel, Benjamin Gaignard

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.

Benjamin

Benjamin Gaignard (2):
  media: videodev2: Add V4L2_FMT_FLAG_ALL_FORMATS flag
  media: verisilicon: Use V4L2_FMT_FLAG_ALL_FORMATS flag

 .../media/v4l/dev-stateless-decoder.rst              |  3 +++
 .../userspace-api/media/v4l/vidioc-enum-fmt.rst      |  4 ++++
 .../userspace-api/media/videodev2.h.rst.exceptions   |  1 +
 drivers/media/platform/verisilicon/hantro_v4l2.c     | 12 +++++++++---
 drivers/media/v4l2-core/v4l2-ioctl.c                 |  2 +-
 include/uapi/linux/videodev2.h                       |  1 +
 6 files changed, 19 insertions(+), 4 deletions(-)

-- 
2.40.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [RCF 1/2] media: videodev2: Add V4L2_FMT_FLAG_ALL_FORMATS flag
  2024-01-11 16:07 [RCF 0/2] Enumerate all pixels formats Benjamin Gaignard
@ 2024-01-11 16:07 ` Benjamin Gaignard
  2024-01-17 19:41   ` Nicolas Dufresne
  2024-01-11 16:07 ` [RCF 2/2] media: verisilicon: Use " Benjamin Gaignard
  1 sibling, 1 reply; 5+ messages in thread
From: Benjamin Gaignard @ 2024-01-11 16:07 UTC (permalink / raw)
  To: mchehab, p.zabel, hverkuil-cisco, nicolas.dufresne
  Cc: linux-media, linux-kernel, linux-rockchip, kernel, Benjamin Gaignard

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@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.
 
 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);
 	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 */
 /*
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [RCF 2/2] media: verisilicon: Use V4L2_FMT_FLAG_ALL_FORMATS flag
  2024-01-11 16:07 [RCF 0/2] Enumerate all pixels formats Benjamin Gaignard
  2024-01-11 16:07 ` [RCF 1/2] media: videodev2: Add V4L2_FMT_FLAG_ALL_FORMATS flag Benjamin Gaignard
@ 2024-01-11 16:07 ` Benjamin Gaignard
  1 sibling, 0 replies; 5+ messages in thread
From: Benjamin Gaignard @ 2024-01-11 16:07 UTC (permalink / raw)
  To: mchehab, p.zabel, hverkuil-cisco, nicolas.dufresne
  Cc: linux-media, linux-kernel, linux-rockchip, kernel, Benjamin Gaignard

If V4L2_FMT_FLAG_ALL_FORMATS flag has been set when calling
VIDIOC_ENUM_FMT ignore depth match and returns all the
hardware supported pixels formats.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
---
 drivers/media/platform/verisilicon/hantro_v4l2.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
index 941fa23c211a..6d840911c764 100644
--- a/drivers/media/platform/verisilicon/hantro_v4l2.c
+++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
@@ -201,7 +201,13 @@ static int vidioc_enum_fmt(struct file *file, void *priv,
 	struct hantro_ctx *ctx = fh_to_ctx(priv);
 	const struct hantro_fmt *fmt, *formats;
 	unsigned int num_fmts, i, j = 0;
-	bool skip_mode_none;
+	bool skip_mode_none, ignore_depth_match;
+
+	/*
+	 * If V4L2_FMT_FLAG_ALL_FORMATS flag is set, we want to enumerate all
+	 * hardware supported pixels formats
+	 */
+	ignore_depth_match = !!(f->flags & V4L2_FMT_FLAG_ALL_FORMATS);
 
 	/*
 	 * When dealing with an encoder:
@@ -222,7 +228,7 @@ static int vidioc_enum_fmt(struct file *file, void *priv,
 
 		if (skip_mode_none == mode_none)
 			continue;
-		if (!hantro_check_depth_match(fmt, ctx->bit_depth))
+		if (!hantro_check_depth_match(fmt, ctx->bit_depth) && !ignore_depth_match)
 			continue;
 		if (j == f->index) {
 			f->pixelformat = fmt->fourcc;
@@ -242,7 +248,7 @@ static int vidioc_enum_fmt(struct file *file, void *priv,
 	for (i = 0; i < num_fmts; i++) {
 		fmt = &formats[i];
 
-		if (!hantro_check_depth_match(fmt, ctx->bit_depth))
+		if (!hantro_check_depth_match(fmt, ctx->bit_depth) && !ignore_depth_match)
 			continue;
 		if (j == f->index) {
 			f->pixelformat = fmt->fourcc;
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [RCF 1/2] media: videodev2: Add V4L2_FMT_FLAG_ALL_FORMATS flag
  2024-01-11 16:07 ` [RCF 1/2] media: videodev2: Add V4L2_FMT_FLAG_ALL_FORMATS flag Benjamin Gaignard
@ 2024-01-17 19:41   ` Nicolas Dufresne
  2024-01-31 10:06     ` Benjamin Gaignard
  0 siblings, 1 reply; 5+ messages in thread
From: Nicolas Dufresne @ 2024-01-17 19:41 UTC (permalink / raw)
  To: Benjamin Gaignard, mchehab, p.zabel, hverkuil-cisco
  Cc: linux-media, linux-kernel, linux-rockchip, kernel

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

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


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

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RCF 1/2] media: videodev2: Add V4L2_FMT_FLAG_ALL_FORMATS flag
  2024-01-17 19:41   ` Nicolas Dufresne
@ 2024-01-31 10:06     ` Benjamin Gaignard
  0 siblings, 0 replies; 5+ messages in thread
From: Benjamin Gaignard @ 2024-01-31 10:06 UTC (permalink / raw)
  To: Nicolas Dufresne, mchehab, p.zabel, hverkuil-cisco
  Cc: linux-media, linux-kernel, linux-rockchip, kernel


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@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 */
>>   /*
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-01-31 10:06 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-11 16:07 [RCF 0/2] Enumerate all pixels formats Benjamin Gaignard
2024-01-11 16:07 ` [RCF 1/2] media: videodev2: Add V4L2_FMT_FLAG_ALL_FORMATS flag Benjamin Gaignard
2024-01-17 19:41   ` Nicolas Dufresne
2024-01-31 10:06     ` Benjamin Gaignard
2024-01-11 16:07 ` [RCF 2/2] media: verisilicon: Use " Benjamin Gaignard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).