* [PATCH v6 1/3] media: videodev2: Add flag to unconditionally enumerate pixel formats
2024-07-31 9:34 [PATCH v6 0/3] Enumerate all pixels formats Benjamin Gaignard
@ 2024-07-31 9:34 ` Benjamin Gaignard
2024-08-07 8:39 ` Hans Verkuil
2024-07-31 9:34 ` [PATCH v6 2/3] media: test-drivers: Use V4L2_FMTDESC_FLAG_ENUM_ALL flag Benjamin Gaignard
2024-07-31 9:34 ` [PATCH v6 3/3] media: verisilicon: " Benjamin Gaignard
2 siblings, 1 reply; 6+ messages in thread
From: Benjamin Gaignard @ 2024-07-31 9:34 UTC (permalink / raw)
To: mchehab, ezequiel, hverkuil-cisco
Cc: linux-media, linux-kernel, linux-rockchip, kernel,
Benjamin Gaignard
When the index is ORed with V4L2_FMTDESC_FLAG_ENUM_ALL the
driver clears the flag and enumerate all the possible formats,
ignoring any limitations from the current configuration.
Drivers which do not support this flag yet always return an EINVAL.
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
---
change in version 6:
- Change flag name.
- Improve documentation.
.../userspace-api/media/v4l/vidioc-enum-fmt.rst | 16 +++++++++++++++-
.../media/videodev2.h.rst.exceptions | 1 +
include/uapi/linux/videodev2.h | 3 +++
3 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
index 3adb3d205531..1112dc9044b2 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
@@ -85,7 +85,15 @@ the ``mbus_code`` field is handled differently:
* - __u32
- ``index``
- Number of the format in the enumeration, set by the application.
- This is in no way related to the ``pixelformat`` field.
+ This is in no way related to the ``pixelformat`` field.
+ When the index is ORed with ``V4L2_FMTDESC_FLAG_ENUM_ALL`` the
+ driver clears the flag and enumerate all the possible formats,
+ ignoring any limitations from the current configuration. Drivers
+ which do not support this flag yet always return an ``EINVAL``
+ error code.
+ Formats enumerated when using ``V4L2_FMTDESC_FLAG_ENUM_ALL`` flag
+ shouldn't be used when calling :c:func:`VIDIOC_ENUM_FRAMESIZES`
+ or :c:func:`VIDIOC_ENUM_FRAMEINTERVALS`.
* - __u32
- ``type``
- Type of the data stream, set by the application. Only these types
@@ -234,6 +242,12 @@ the ``mbus_code`` field is handled differently:
valid. The buffer consists of ``height`` lines, each having ``width``
Data Units of data and the offset (in bytes) between the beginning of
each two consecutive lines is ``bytesperline``.
+ * - ``V4L2_FMTDESC_FLAG_ENUM_ALL``
+ - 0x80000000
+ - When the applications ORs ``index`` with ``V4L2_FMTDESC_FLAG_ENUM_ALL`` flag
+ the driver enumerates all the possible pixel formats without taking care
+ of any already set configuration. Drivers which do not support this flag
+ yet, always return ``EINVAL``.
Return Value
============
diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
index bdc628e8c1d6..0a9ea9686c24 100644
--- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
+++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
@@ -216,6 +216,7 @@ 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_META_LINE_BASED fmtdesc-flags
+replace define V4L2_FMTDESC_FLAG_ENUM_ALL fmtdesc-flags
# V4L2 timecode types
replace define V4L2_TC_TYPE_24FPS timecode-type
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 4e91362da6da..421a30cb0c51 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -904,6 +904,9 @@ struct v4l2_fmtdesc {
#define V4L2_FMT_FLAG_CSC_QUANTIZATION 0x0100
#define V4L2_FMT_FLAG_META_LINE_BASED 0x0200
+/* Format description flag, to be ORed with the index */
+#define V4L2_FMTDESC_FLAG_ENUM_ALL 0x80000000
+
/* Frame Size and frame rate enumeration */
/*
* F R A M E S I Z E E N U M E R A T I O N
--
2.43.0
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v6 1/3] media: videodev2: Add flag to unconditionally enumerate pixel formats
2024-07-31 9:34 ` [PATCH v6 1/3] media: videodev2: Add flag to unconditionally enumerate pixel formats Benjamin Gaignard
@ 2024-08-07 8:39 ` Hans Verkuil
2024-08-07 8:46 ` Hans Verkuil
0 siblings, 1 reply; 6+ messages in thread
From: Hans Verkuil @ 2024-08-07 8:39 UTC (permalink / raw)
To: Benjamin Gaignard, mchehab, ezequiel
Cc: linux-media, linux-kernel, linux-rockchip, kernel
On 31/07/2024 11:34, Benjamin Gaignard wrote:
> When the index is ORed with V4L2_FMTDESC_FLAG_ENUM_ALL the
> driver clears the flag and enumerate all the possible formats,
> ignoring any limitations from the current configuration.
> Drivers which do not support this flag yet always return an EINVAL.
>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> ---
> change in version 6:
> - Change flag name.
> - Improve documentation.
>
> .../userspace-api/media/v4l/vidioc-enum-fmt.rst | 16 +++++++++++++++-
> .../media/videodev2.h.rst.exceptions | 1 +
> include/uapi/linux/videodev2.h | 3 +++
> 3 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> index 3adb3d205531..1112dc9044b2 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> @@ -85,7 +85,15 @@ the ``mbus_code`` field is handled differently:
> * - __u32
> - ``index``
> - Number of the format in the enumeration, set by the application.
> - This is in no way related to the ``pixelformat`` field.
> + This is in no way related to the ``pixelformat`` field.
> + When the index is ORed with ``V4L2_FMTDESC_FLAG_ENUM_ALL`` the
> + driver clears the flag and enumerate all the possible formats,
enumerate -> enumerates
> + ignoring any limitations from the current configuration. Drivers
> + which do not support this flag yet always return an ``EINVAL``
Drop the 'yet'.
But this raises a question: should this flag only be supported by drivers
that can actually return different format lists depending on this flag?
Or can it be supported as well by a driver where this makes no difference?
I'm inclined to limit it to drivers that actually can return different
results. If nothing else, that will indicate to the application that this
is actually possible.
If we agree on that, then that should be documented as well.
> + error code.
> + Formats enumerated when using ``V4L2_FMTDESC_FLAG_ENUM_ALL`` flag
> + shouldn't be used when calling :c:func:`VIDIOC_ENUM_FRAMESIZES`
> + or :c:func:`VIDIOC_ENUM_FRAMEINTERVALS`.
> * - __u32
> - ``type``
> - Type of the data stream, set by the application. Only these types
> @@ -234,6 +242,12 @@ the ``mbus_code`` field is handled differently:
> valid. The buffer consists of ``height`` lines, each having ``width``
> Data Units of data and the offset (in bytes) between the beginning of
> each two consecutive lines is ``bytesperline``.
> + * - ``V4L2_FMTDESC_FLAG_ENUM_ALL``
> + - 0x80000000
> + - When the applications ORs ``index`` with ``V4L2_FMTDESC_FLAG_ENUM_ALL`` flag
> + the driver enumerates all the possible pixel formats without taking care
> + of any already set configuration. Drivers which do not support this flag
> + yet, always return ``EINVAL``.
Drop 'yet'
>
> Return Value
> ============
> diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> index bdc628e8c1d6..0a9ea9686c24 100644
> --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> @@ -216,6 +216,7 @@ 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_META_LINE_BASED fmtdesc-flags
> +replace define V4L2_FMTDESC_FLAG_ENUM_ALL fmtdesc-flags
>
> # V4L2 timecode types
> replace define V4L2_TC_TYPE_24FPS timecode-type
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 4e91362da6da..421a30cb0c51 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -904,6 +904,9 @@ struct v4l2_fmtdesc {
> #define V4L2_FMT_FLAG_CSC_QUANTIZATION 0x0100
> #define V4L2_FMT_FLAG_META_LINE_BASED 0x0200
>
> +/* Format description flag, to be ORed with the index */
> +#define V4L2_FMTDESC_FLAG_ENUM_ALL 0x80000000
> +
> /* Frame Size and frame rate enumeration */
> /*
> * F R A M E S I Z E E N U M E R A T I O N
Regards,
Hans
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v6 1/3] media: videodev2: Add flag to unconditionally enumerate pixel formats
2024-08-07 8:39 ` Hans Verkuil
@ 2024-08-07 8:46 ` Hans Verkuil
0 siblings, 0 replies; 6+ messages in thread
From: Hans Verkuil @ 2024-08-07 8:46 UTC (permalink / raw)
To: Benjamin Gaignard, mchehab, ezequiel
Cc: linux-media, linux-kernel, linux-rockchip, kernel
On 07/08/2024 10:39, Hans Verkuil wrote:
> On 31/07/2024 11:34, Benjamin Gaignard wrote:
>> When the index is ORed with V4L2_FMTDESC_FLAG_ENUM_ALL the
>> driver clears the flag and enumerate all the possible formats,
>> ignoring any limitations from the current configuration.
>> Drivers which do not support this flag yet always return an EINVAL.
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
>> ---
>> change in version 6:
>> - Change flag name.
>> - Improve documentation.
>>
>> .../userspace-api/media/v4l/vidioc-enum-fmt.rst | 16 +++++++++++++++-
>> .../media/videodev2.h.rst.exceptions | 1 +
>> include/uapi/linux/videodev2.h | 3 +++
>> 3 files changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
>> index 3adb3d205531..1112dc9044b2 100644
>> --- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
>> +++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
>> @@ -85,7 +85,15 @@ the ``mbus_code`` field is handled differently:
>> * - __u32
>> - ``index``
>> - Number of the format in the enumeration, set by the application.
>> - This is in no way related to the ``pixelformat`` field.
>> + This is in no way related to the ``pixelformat`` field.
>> + When the index is ORed with ``V4L2_FMTDESC_FLAG_ENUM_ALL`` the
>> + driver clears the flag and enumerate all the possible formats,
>
> enumerate -> enumerates
>
>> + ignoring any limitations from the current configuration. Drivers
>> + which do not support this flag yet always return an ``EINVAL``
>
> Drop the 'yet'.
>
> But this raises a question: should this flag only be supported by drivers
> that can actually return different format lists depending on this flag?
>
> Or can it be supported as well by a driver where this makes no difference?
>
> I'm inclined to limit it to drivers that actually can return different
> results. If nothing else, that will indicate to the application that this
> is actually possible.
>
> If we agree on that, then that should be documented as well.
I also think that this flag makes no sense for drivers that have
V4L2_CAP_IO_MC set, since in that case the list of returned formats shall
not depend on the active configuration (see
https://hverkuil.home.xs4all.nl/spec/userspace-api/v4l/vidioc-enum-fmt.html ).
I think this should be mentioned in the documentation and tested for in the
compliance test.
Regards,
Hans
>
>> + error code.
>> + Formats enumerated when using ``V4L2_FMTDESC_FLAG_ENUM_ALL`` flag
>> + shouldn't be used when calling :c:func:`VIDIOC_ENUM_FRAMESIZES`
>> + or :c:func:`VIDIOC_ENUM_FRAMEINTERVALS`.
>> * - __u32
>> - ``type``
>> - Type of the data stream, set by the application. Only these types
>> @@ -234,6 +242,12 @@ the ``mbus_code`` field is handled differently:
>> valid. The buffer consists of ``height`` lines, each having ``width``
>> Data Units of data and the offset (in bytes) between the beginning of
>> each two consecutive lines is ``bytesperline``.
>> + * - ``V4L2_FMTDESC_FLAG_ENUM_ALL``
>> + - 0x80000000
>> + - When the applications ORs ``index`` with ``V4L2_FMTDESC_FLAG_ENUM_ALL`` flag
>> + the driver enumerates all the possible pixel formats without taking care
>> + of any already set configuration. Drivers which do not support this flag
>> + yet, always return ``EINVAL``.
>
> Drop 'yet'
>
>>
>> Return Value
>> ============
>> diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
>> index bdc628e8c1d6..0a9ea9686c24 100644
>> --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
>> +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
>> @@ -216,6 +216,7 @@ 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_META_LINE_BASED fmtdesc-flags
>> +replace define V4L2_FMTDESC_FLAG_ENUM_ALL fmtdesc-flags
>>
>> # V4L2 timecode types
>> replace define V4L2_TC_TYPE_24FPS timecode-type
>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>> index 4e91362da6da..421a30cb0c51 100644
>> --- a/include/uapi/linux/videodev2.h
>> +++ b/include/uapi/linux/videodev2.h
>> @@ -904,6 +904,9 @@ struct v4l2_fmtdesc {
>> #define V4L2_FMT_FLAG_CSC_QUANTIZATION 0x0100
>> #define V4L2_FMT_FLAG_META_LINE_BASED 0x0200
>>
>> +/* Format description flag, to be ORed with the index */
>> +#define V4L2_FMTDESC_FLAG_ENUM_ALL 0x80000000
>> +
>> /* Frame Size and frame rate enumeration */
>> /*
>> * F R A M E S I Z E E N U M E R A T I O N
>
> Regards,
>
> Hans
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v6 2/3] media: test-drivers: Use V4L2_FMTDESC_FLAG_ENUM_ALL flag
2024-07-31 9:34 [PATCH v6 0/3] Enumerate all pixels formats Benjamin Gaignard
2024-07-31 9:34 ` [PATCH v6 1/3] media: videodev2: Add flag to unconditionally enumerate pixel formats Benjamin Gaignard
@ 2024-07-31 9:34 ` Benjamin Gaignard
2024-07-31 9:34 ` [PATCH v6 3/3] media: verisilicon: " Benjamin Gaignard
2 siblings, 0 replies; 6+ messages in thread
From: Benjamin Gaignard @ 2024-07-31 9:34 UTC (permalink / raw)
To: mchehab, ezequiel, hverkuil-cisco
Cc: linux-media, linux-kernel, linux-rockchip, kernel,
Benjamin Gaignard
Since the V4L2_FMTDESC_FLAG_ENUM_ALL flag mostly targets stateless
decoder pixel-format enumeration, update visl test driver to use it.
When V4L2_FMTDESC_FLAG_ENUM_ALL flag is set let the driver returns
one more pixel format.
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
---
change in version 6:
- Improve visl driver to enumerate one more pixel format when the flag
is used.
drivers/media/test-drivers/visl/visl-video.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/media/test-drivers/visl/visl-video.c b/drivers/media/test-drivers/visl/visl-video.c
index f8d970319764..677a8564de03 100644
--- a/drivers/media/test-drivers/visl/visl-video.c
+++ b/drivers/media/test-drivers/visl/visl-video.c
@@ -136,6 +136,12 @@ static const u32 visl_decoded_fmts[] = {
V4L2_PIX_FMT_YUV420,
};
+static const u32 visl_extended_decoded_fmts[] = {
+ V4L2_PIX_FMT_NV12,
+ V4L2_PIX_FMT_YUV420,
+ V4L2_PIX_FMT_P010,
+};
+
const struct visl_coded_format_desc visl_coded_fmts[] = {
{
.pixelformat = V4L2_PIX_FMT_FWHT_STATELESS,
@@ -341,11 +347,21 @@ static int visl_enum_fmt_vid_cap(struct file *file, void *priv,
struct v4l2_fmtdesc *f)
{
struct visl_ctx *ctx = visl_file_to_ctx(file);
+ u32 index = f->index & ~V4L2_FMTDESC_FLAG_ENUM_ALL;
+ int max_fmts = ctx->coded_format_desc->num_decoded_fmts;
+ const u32 *decoded_fmts = ctx->coded_format_desc->decoded_fmts;
+
+ if (f->index & V4L2_FMTDESC_FLAG_ENUM_ALL) {
+ max_fmts = ARRAY_SIZE(visl_extended_decoded_fmts);
+ decoded_fmts = visl_extended_decoded_fmts;
+ }
+
+ f->index = index;
- if (f->index >= ctx->coded_format_desc->num_decoded_fmts)
+ if (index >= max_fmts)
return -EINVAL;
- f->pixelformat = ctx->coded_format_desc->decoded_fmts[f->index];
+ f->pixelformat = decoded_fmts[index];
return 0;
}
--
2.43.0
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v6 3/3] media: verisilicon: Use V4L2_FMTDESC_FLAG_ENUM_ALL flag
2024-07-31 9:34 [PATCH v6 0/3] Enumerate all pixels formats Benjamin Gaignard
2024-07-31 9:34 ` [PATCH v6 1/3] media: videodev2: Add flag to unconditionally enumerate pixel formats Benjamin Gaignard
2024-07-31 9:34 ` [PATCH v6 2/3] media: test-drivers: Use V4L2_FMTDESC_FLAG_ENUM_ALL flag Benjamin Gaignard
@ 2024-07-31 9:34 ` Benjamin Gaignard
2 siblings, 0 replies; 6+ messages in thread
From: Benjamin Gaignard @ 2024-07-31 9:34 UTC (permalink / raw)
To: mchehab, ezequiel, hverkuil-cisco
Cc: linux-media, linux-kernel, linux-rockchip, kernel,
Benjamin Gaignard
By adding support for the V4L2_FMTDESC_FLAG_ENUM_ALL flag into the driver
we allow userspace applications to discover all possible
pixel formats of the hardware block. This way userspace can decide
which decoder to use given the supported pixel formats.
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
---
.../media/platform/verisilicon/hantro_v4l2.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
index df6f2536263b..f416c5b36dd0 100644
--- a/drivers/media/platform/verisilicon/hantro_v4l2.c
+++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
@@ -201,7 +201,15 @@ 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, enum_all_formats;
+ u32 index = f->index & ~V4L2_FMTDESC_FLAG_ENUM_ALL;
+
+ /*
+ * If the V4L2_FMTDESC_FLAG_ENUM_ALL flag is set, we want to enumerate all
+ * hardware supported pixel formats
+ */
+ enum_all_formats = !!(f->index & V4L2_FMTDESC_FLAG_ENUM_ALL);
+ f->index = index;
/*
* When dealing with an encoder:
@@ -222,9 +230,9 @@ 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) && !enum_all_formats)
continue;
- if (j == f->index) {
+ if (j == index) {
f->pixelformat = fmt->fourcc;
return 0;
}
@@ -242,9 +250,9 @@ 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) && !enum_all_formats)
continue;
- if (j == f->index) {
+ if (j == index) {
f->pixelformat = fmt->fourcc;
return 0;
}
--
2.43.0
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 6+ messages in thread