| Message ID | 20240111160721.50020-2-benjamin.gaignard@collabora.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Enumerate all pixels formats | expand |
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 */ > /*
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 */ >> /* >
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 */ /*
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(-)