| Message ID | 20200416145605.12399-1-dafna.hirschfeld@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,v3,1/2] v4l2: add support for colorspace conversion for video capture | expand |
On 16/04/2020 16:56, Dafna Hirschfeld wrote: > From: Philipp Zabel <p.zabel@pengutronix.de> > > For video capture it is the driver that reports the colorspace, > Y'CbCr/HSV encoding, quantization range and transfer function > used by the video, and there is no way to request something > different, even though many HDTV receivers have some sort of > colorspace conversion capabilities. > > For output video this feature already exists since the application > specifies this information for the video format it will send out, and > the transmitter will enable any available CSC if a format conversion has > to be performed in order to match the capabilities of the sink. > > For video capture we propose adding new pix_format flag: > V4L2_PIX_FMT_FLAG_HAS_CSC. The flag is set by the application, That should be V4L2_PIX_FMT_FLAG_REQUEST_CSC, since the application actively requests the use of the CSC. > the driver will interpret the ycbcr_enc/hsv_enc, and quantization fields > as the requested colorspace information and will attempt to > do the conversion it supports. > > Drivers set the flags > V4L2_FMT_FLAG_CSC_YCBCR_ENC, > V4L2_FMT_FLAG_CSC_HSV_ENC, > V4L2_FMT_FLAG_CSC_HSV_QUANTIZATION, That last define should be 'V4L2_FMT_FLAG_CSC_QUANTIZATION' without 'HSV_'. > in the flags field of the struct v4l2_fmtdesc during enumeration to > indicate that they support colorspace conversion for the respective field. > Currently the conversion of the fields colorspace, xfer_func is not > supported since there are no drivers that support it. > > The same API is added for the subdevices. With the flag V4L2_MBUS_FRAMEFMT_HAS_CSC Should also be REQUEST_CSC. > set by the application in the VIDIOC_SUBDEV_S_FMT ioctl and the flags > V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC, V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION > set by the driver in the VIDIOC_SUBDEV_ENUM_MBUS_CODE ioctl. > > For subdevices, new 'flags' fields were added to the structs > v4l2_subdev_mbus_code_enum, v4l2_mbus_framefmt which are borrowed from the 'reserved' field > > Drivers do not have to actually look at the flags: if the flags are not > set, then the colorspace, ycbcr_enc and quantization fields are set to > the default values by the core, i.e. just pass on the received format > without conversion. > > Signed-off-by: Hans Verkuil <Hans Verkuil@cisco.com> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> > Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> > --- > This is v3 of the RFC suggested originaly by Hans Verkuil: > > https://patchwork.linuxtv.org/patch/28847/ > > And then a v2 from Philipp Zabel: > > https://patchwork.kernel.org/project/linux-media/list/?series=15483 > > changes in v3: > I added the API to subdevices as well and added fixes according to > comments from Hans. > I also added a usecase for the new API for the rkisp1 driver. > > changes in v2 (reported by Philipp Zabel): > - convert to rst > - split V4L2_PIX_FMT_FLAG_REQUEST_CSC into four separate flags for > colorspace, ycbcr_enc/hsv_enc, quantization, and xfer_func > - let driver set flags to indicate supported features > > [1] https://patchwork.linuxtv.org/patch/28847/ > > .../media/v4l/pixfmt-reserved.rst | 6 +++ > .../media/v4l/pixfmt-v4l2-mplane.rst | 16 ++----- > .../userspace-api/media/v4l/pixfmt-v4l2.rst | 36 +++++++++++++--- > .../media/v4l/subdev-formats.rst | 42 +++++++++++++++++-- > .../media/v4l/vidioc-enum-fmt.rst | 18 ++++++++ > .../v4l/vidioc-subdev-enum-mbus-code.rst | 23 ++++++++++ > drivers/media/v4l2-core/v4l2-ioctl.c | 19 ++++++++- > drivers/media/v4l2-core/v4l2-subdev.c | 7 ++++ > include/uapi/linux/v4l2-mediabus.h | 5 ++- > include/uapi/linux/v4l2-subdev.h | 5 ++- > include/uapi/linux/videodev2.h | 4 ++ > 11 files changed, 158 insertions(+), 23 deletions(-) > > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst > index 59b9e7238f90..fa8dada69f8c 100644 > --- a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst > +++ b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst Why on earth did the documentation of these pixel format flags end up in pixfmt-reserved.rst? Can you make a patch that moves this to pixfmt-v4l2.rst? > @@ -280,3 +280,9 @@ please make a proposal on the linux-media mailing list. > by RGBA values (128, 192, 255, 128), the same pixel described with > premultiplied colors would be described by RGBA values (64, 96, > 128, 128) > + * - ``V4L2_PIX_FMT_FLAG_HAS_CSC`` > + - 0x00000002 > + - Set by the application. It is only used for capture and is > + ignored for output streams. If set, then request the driver to do > + colorspace conversion from the received colorspace, only conversions > + of Ycbcr encoding, HSV encoding and quantization are supported. I'd replace the part "If set..." with this: ---------- If set, then request the driver to do colorspace conversion from the received colorspace to the requested colorspace values. If a colorimetry field (``colorspace``, ``xfer_func``, ``ycncr_enc`` or ``quantization``) is set to 0, then that colorimetry setting will remain unchanged from what was received. So to just change the quantization only the ``quantization`` field shall be set to a non-zero value (``V4L2_QUANTIZATION_FULL_RANGE`` or ``V4L2_QUANTIZATION_LIM_RANGE``) and all other colorimetry fields shall be set to 0. To check which conversions are supported by the hardware for the current pixel format, see :ref:`fmtdesc-flags`. ---------- > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst > index 444b4082684c..66f3365d7b72 100644 > --- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst > +++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst > @@ -105,29 +105,21 @@ describing all planes of that format. > * - __u8 > - ``ycbcr_enc`` > - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`. > - This information supplements the ``colorspace`` and must be set by > - the driver for capture streams and by the application for output > - streams, see :ref:`colorspaces`. > + See struct :c:type:`v4l2_pix_format`. > * - __u8 > - ``hsv_enc`` > - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`. > - This information supplements the ``colorspace`` and must be set by > - the driver for capture streams and by the application for output > - streams, see :ref:`colorspaces`. > + See struct :c:type:`v4l2_pix_format`. > * - } > - > * - __u8 > - ``quantization`` > - Quantization range, from enum :c:type:`v4l2_quantization`. > - This information supplements the ``colorspace`` and must be set by > - the driver for capture streams and by the application for output > - streams, see :ref:`colorspaces`. > + See struct :c:type:`v4l2_pix_format`. > * - __u8 > - ``xfer_func`` > - Transfer function, from enum :c:type:`v4l2_xfer_func`. > - This information supplements the ``colorspace`` and must be set by > - the driver for capture streams and by the application for output > - streams, see :ref:`colorspaces`. > + See struct :c:type:`v4l2_pix_format`. > * - __u8 > - ``reserved[7]`` > - Reserved for future extensions. Should be zeroed by drivers and > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst > index 759420a872d6..ce57718cd66b 100644 > --- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst > +++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst > @@ -110,8 +110,8 @@ Single-planar format structure > - ``colorspace`` > - Image colorspace, from enum :c:type:`v4l2_colorspace`. > This information supplements the ``pixelformat`` and must be set > - by the driver for capture streams and by the application for > - output streams, see :ref:`colorspaces`. > + by the driver for capture streams and by the application for output > + streams, see :ref:`colorspace`. This doesn't appear to change anything. > * - __u32 > - ``priv`` > - This field indicates whether the remaining fields of the > @@ -148,13 +148,31 @@ Single-planar format structure > - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`. > This information supplements the ``colorspace`` and must be set by > the driver for capture streams and by the application for output > - streams, see :ref:`colorspaces`. > + streams, see :ref:`colorspaces`. If the application sets the > + flag ``V4L2_PIX_FMT_FLAG_HAS_CSC`` then the application can set > + this field for a capture stream to request a specific Y'CbCr encoding > + for the captured image data. The driver will attempt to do the > + conversion to the specified Y'CbCr encoding or return the encoding it > + will use if it can't do the conversion. This field is ignored for > + non-Y'CbCr pixelformats. The driver indicates that ycbcr_enc conversion > + is supported by setting the flag V4L2_FMT_FLAG_CSC_YCBCR_ENC on the > + on the corresponding struct :c:type:`v4l2_fmtdesc` during enumeration. on the on the -> in the > + See :ref:`fmtdesc-flags` > * - __u32 > - ``hsv_enc`` > - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`. > This information supplements the ``colorspace`` and must be set by > the driver for capture streams and by the application for output > - streams, see :ref:`colorspaces`. > + streams, see :ref:`colorspaces`. If the application sets the flag > + ``V4L2_PIX_FMT_FLAG_HAS_CSC`` then the application can set this > + field for a capture stream to request a specific HSV encoding for the > + captured image data. The driver will attempt to do the conversion to > + the specified HSV encoding or return the encoding it will use if it > + can't do the conversion. This field is ignored for non-HSV > + pixelformats. The driver indicates that hsv_enc conversion > + is supported by setting the flag V4L2_FMT_FLAG_CSC_HSV_ENC on the > + on the corresponding struct :c:type:`v4l2_fmtdesc` during enumeration. Ditto. > + See :ref:`fmtdesc-flags` > * - } > - > * - __u32 > @@ -162,7 +180,15 @@ Single-planar format structure > - Quantization range, from enum :c:type:`v4l2_quantization`. > This information supplements the ``colorspace`` and must be set by > the driver for capture streams and by the application for output > - streams, see :ref:`colorspaces`. > + streams, see :ref:`colorspaces`. If the application sets the flag > + ``V4L2_PIX_FMT_FLAG_HAS_CSC`` then the application can set > + this field for a capture stream to request a specific quantization > + range for the captured image data. The driver will attempt to do the > + conversion to the specified quantization range or return the > + quantization it will use if it can't do the conversion. The driver > + indicates that quantization conversion is supported by setting the flag > + V4L2_FMT_FLAG_CSC_QUANTIZATION on the on the corresponding struct Ditto > + :c:type:`v4l2_fmtdesc` during enumeration. See :ref:`fmtdesc-flags` > * - __u32 > - ``xfer_func`` > - Transfer function, from enum :c:type:`v4l2_xfer_func`. > diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst > index 9a4d61b0d76f..f1d4ca29a3e8 100644 > --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst > +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst > @@ -49,13 +49,33 @@ Media Bus Formats > - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`. > This information supplements the ``colorspace`` and must be set by > the driver for capture streams and by the application for output > - streams, see :ref:`colorspaces`. > + streams, see :ref:`colorspaces`. If the application sets the > + flag ``V4L2_MBUS_FRAMEFMT_HAS_CSC`` the the application can set > + this field for a capture stream to request a specific Y'CbCr encoding > + for the mbus data. The driver will attempt to do the mbus -> media bus > + conversion to the specified Y'CbCr encoding or return the encoding it > + will use if it can't do the conversion. This field is ignored for > + non-Y'CbCr pixelformats. The driver indicates that ycbcr_enc conversion This is a media bus format, not a pixelformat. > + is supported by setting the flag V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC on on -> in > + the corresponding struct c:type:`v4l2_subdev_mbus_code_enum` during > + enumeration. See :ref:`vidioc-subdev-enum-mbus-code.rst <v4l2-subdev-mbus-code-flags>` > + > * - __u16 > - ``quantization`` > - Quantization range, from enum :c:type:`v4l2_quantization`. > This information supplements the ``colorspace`` and must be set by > the driver for capture streams and by the application for output > - streams, see :ref:`colorspaces`. > + streams, see :ref:`colorspaces`. If the application sets the > + flag ``V4L2_MBUS_FRAMEFMT_HAS_CSC`` the the application can set > + this field for a capture stream to request a specific quantization > + encoding for the mbus data. The driver will attempt to do the mbus -> media bus > + conversion to the specified quantization or return the quantization it > + will use if it can't do the conversion. The driver indicates that > + quantization conversion is supported by setting the flag > + V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION on the corresponding struct on -> in > + c:type:`v4l2_subdev_mbus_code_enum` during enumeration. > + See :ref:`vidioc-subdev-enum-mbus-code.rst <v4l2-subdev-mbus-code-flags>` > + > * - __u16 > - ``xfer_func`` > - Transfer function, from enum :c:type:`v4l2_xfer_func`. > @@ -63,10 +83,26 @@ Media Bus Formats > the driver for capture streams and by the application for output > streams, see :ref:`colorspaces`. > * - __u16 > - - ``reserved``\ [11] > + - ``flags`` > + - flags see: :ref:v4l2-mbus-framefmt-flags flags see: -> See: > + * - __u16 > + - ``reserved``\ [10] > - Reserved for future extensions. Applications and drivers must set > the array to zero. > > +.. _v4l2-mbus-framefmt-flags: > + > +.. flat-table:: v4l2_mbus_framefmt Flags > + :header-rows: 0 > + :stub-columns: 0 > + :widths: 3 1 4 > + > + * - ``V4L2_MBUS_FRAMEFMT_HAS_CSC`` > + - 0x0001 > + - Set by the application. It is only used for capture and is > + ignored for output streams. If set, then request the subdevice to do > + colorspace conversion from the received colorspace, only conversions > + of Ycbcr encoding, and quantization are supported. Replace by a similar text as for V4L2_PIX_FMT_FLAG_HAS_CSC. > > > .. _v4l2-mbus-pixelcode: > diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst > index 7e3142e11d77..125f074543af 100644 > --- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst > +++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst > @@ -145,6 +145,24 @@ formats in preference order, where preferred formats are returned before > parameters are detected. This flag can only be used in combination > with the ``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to > compressed formats only. It is also only applies to stateful codecs. > + * - ``V4L2_FMT_FLAG_CSC_YCBCR_ENC`` > + - 0x0010 > + - The driver allows the application to try to change the default > + ycbcr encoding. This flag is relevant only for capture devices. > + The application can ask to configure the ycbcr_enc of the capture device > + when calling the :c:func:`VIDIOC_S_FMT` ioctl. add: ...ioctl with ``V4L2_FMT_FLAG_REQUEST_CSC`` set. (this should probably be a reference to the REQUEST_CSC flag documentation). > + * - ``V4L2_FMT_FLAG_CSC_HSV_ENC`` > + - 0x0010 > + - The driver allows the application to try to change the default > + hsv encoding. This flag is relevant only for capture devices. > + The application can ask to configure the hsv_enc of the capture device > + when calling the :c:func:`VIDIOC_S_FMT` ioctl. Ditto. Also: hsv -> HSV. > + * - ``V4L2_FMT_FLAG_CSC_QUANTIZATION`` > + - 0x0020 > + - The driver allows the application to try to change the default > + quantization. This flag is relevant only for capture devices. > + The application can ask to configure the quantization of the capture device > + when calling the :c:func:`VIDIOC_S_FMT` ioctl. Ditto. > > > Return Value > diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst > index 35b8607203a4..4ad87cb74f57 100644 > --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst > +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst > @@ -78,11 +78,34 @@ information about the try formats. > - ``which`` > - Media bus format codes to be enumerated, from enum > :ref:`v4l2_subdev_format_whence <v4l2-subdev-format-whence>`. > + * - __u32 > + - ``flags`` > + - see :ref:`v4l2-subdev-mbus-code-flags` see -> See > * - __u32 > - ``reserved``\ [8] > - Reserved for future extensions. Applications and drivers must set > the array to zero. > > +.. _v4l2-subdev-mbus-code-flags: > + > + > +.. tabularcolumns:: |p{4.4cm}|p{4.4cm}|p{7.7cm}| > + > +.. flat-table:: flags in struct v4l2_subdev_mbus_code_enum > + :header-rows: 0 > + :stub-columns: 0 > + :widths: 1 1 2 > + > + * - V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC > + - 0x00000001 > + - The driver supports setting the ycbcr encoding by the application > + when calling the VIDIOC_SUBDEV_S_FMT ioctl. see :ref:`v4l2-mbus-format` see -> See > + on how to do this. > + * - V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION > + - 0x00000002 > + - The driver supports setting the quantization by the application > + when calling the VIDIOC_SUBDEV_S_FMT ioctl. see :ref:`v4l2-mbus-format` see -> See > + on how to do this. > > Return Value > ============ > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c > index b2ef8e60ea7d..3c7ffb6b15cb 100644 > --- a/drivers/media/v4l2-core/v4l2-ioctl.c > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c > @@ -1029,6 +1029,15 @@ static void v4l_sanitize_format(struct v4l2_format *fmt) > fmt->fmt.pix_mp.num_planes = min_t(u32, fmt->fmt.pix_mp.num_planes, > VIDEO_MAX_PLANES); > > + if (fmt->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) { > + if (fmt->fmt.pix_mp.flags & V4L2_PIX_FMT_FLAG_HAS_CSC) > + return; > + fmt->fmt.pix_mp.colorspace = V4L2_COLORSPACE_DEFAULT; > + fmt->fmt.pix_mp.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; > + fmt->fmt.pix_mp.quantization = V4L2_QUANTIZATION_DEFAULT; > + fmt->fmt.pix_mp.xfer_func = V4L2_XFER_FUNC_DEFAULT; > + } > + > /* > * The v4l2_pix_format structure has been extended with fields that were > * not previously required to be set to zero by applications. The priv > @@ -1043,8 +1052,16 @@ static void v4l_sanitize_format(struct v4l2_format *fmt) > fmt->type != V4L2_BUF_TYPE_VIDEO_OUTPUT) > return; > > - if (fmt->fmt.pix.priv == V4L2_PIX_FMT_PRIV_MAGIC) > + if (fmt->fmt.pix.priv == V4L2_PIX_FMT_PRIV_MAGIC) { > + if (fmt->type != V4L2_BUF_TYPE_VIDEO_CAPTURE || > + fmt->fmt.pix.flags & V4L2_PIX_FMT_FLAG_HAS_CSC) > + return; > + fmt->fmt.pix.colorspace = V4L2_COLORSPACE_DEFAULT; > + fmt->fmt.pix.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; > + fmt->fmt.pix.quantization = V4L2_QUANTIZATION_DEFAULT; > + fmt->fmt.pix.xfer_func = V4L2_XFER_FUNC_DEFAULT; > return; > + } > > fmt->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC; > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > index a376b351135f..51e009936aad 100644 > --- a/drivers/media/v4l2-core/v4l2-subdev.c > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > @@ -477,6 +477,13 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) > case VIDIOC_SUBDEV_S_FMT: { > struct v4l2_subdev_format *format = arg; > > + if (!(format->format.flags & V4L2_MBUS_FRAMEFMT_HAS_CSC)) { > + format->format.colorspace = V4L2_COLORSPACE_DEFAULT; > + format->format.xfer_func = V4L2_XFER_FUNC_DEFAULT; > + format->format.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; > + format->format.quantization = V4L2_QUANTIZATION_DEFAULT; > + } > + > memset(format->reserved, 0, sizeof(format->reserved)); > memset(format->format.reserved, 0, sizeof(format->format.reserved)); > return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh->pad, format); > diff --git a/include/uapi/linux/v4l2-mediabus.h b/include/uapi/linux/v4l2-mediabus.h > index 123a231001a8..89ff0ad6a631 100644 > --- a/include/uapi/linux/v4l2-mediabus.h > +++ b/include/uapi/linux/v4l2-mediabus.h > @@ -16,6 +16,8 @@ > #include <linux/types.h> > #include <linux/videodev2.h> > > +#define V4L2_MBUS_FRAMEFMT_HAS_CSC 0x0001 > + > /** > * struct v4l2_mbus_framefmt - frame format on the media bus > * @width: image width > @@ -36,7 +38,8 @@ struct v4l2_mbus_framefmt { > __u16 ycbcr_enc; > __u16 quantization; > __u16 xfer_func; > - __u16 reserved[11]; > + __u16 flags; > + __u16 reserved[10]; > }; > > #ifndef __KERNEL__ > diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h > index 03970ce30741..4410b26a7158 100644 > --- a/include/uapi/linux/v4l2-subdev.h > +++ b/include/uapi/linux/v4l2-subdev.h > @@ -65,6 +65,8 @@ struct v4l2_subdev_crop { > __u32 reserved[8]; > }; > > +#define V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC 0x00000001 I think it is better to add an HSV_ENC alias here as well. It's for readability only, since talking about YCBCR_ENC when the mediabus format is a HSV format is just weird. > +#define V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION 0x00000002 > /** > * struct v4l2_subdev_mbus_code_enum - Media bus format enumeration > * @pad: pad number, as reported by the media API > @@ -77,7 +79,8 @@ struct v4l2_subdev_mbus_code_enum { > __u32 index; > __u32 code; > __u32 which; > - __u32 reserved[8]; > + __u32 flags; > + __u32 reserved[7]; > }; > > /** > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index 9817b7e2c968..adc9dd1080b8 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -772,6 +772,7 @@ struct v4l2_pix_format { > > /* Flags */ > #define V4L2_PIX_FMT_FLAG_PREMUL_ALPHA 0x00000001 > +#define V4L2_PIX_FMT_FLAG_HAS_CSC 0x00000002 > > /* > * F O R M A T E N U M E R A T I O N > @@ -789,6 +790,9 @@ struct v4l2_fmtdesc { > #define V4L2_FMT_FLAG_EMULATED 0x0002 > #define V4L2_FMT_FLAG_CONTINUOUS_BYTESTREAM 0x0004 > #define V4L2_FMT_FLAG_DYN_RESOLUTION 0x0008 > +#define V4L2_FMT_FLAG_CSC_YCBCR_ENC 0x0010 > +#define V4L2_FMT_FLAG_CSC_HSV_ENC 0x0010 > +#define V4L2_FMT_FLAG_CSC_QUANTIZATION 0x0020 > > /* Frame Size and frame rate enumeration */ > /* > I think this is quite good, it's mainly doc improvements and renaming HAS_CSC to REQUEST_CSC. That said, I do want to see this supported in at least vivid (vimc would be nice too!). And v4l2-ctl needs a patch to support this. A compliance test for v4l2-compliance would be nice too, but it is not a prerequisite. I'm not entirely sure what would be the best way to test this. Perhaps a vivid-specific test might be easiest (i.e., don't test it for all drivers, but just for vivid). Regards, Hans
Hi On 07.05.20 15:00, Hans Verkuil wrote: > On 16/04/2020 16:56, Dafna Hirschfeld wrote: >> From: Philipp Zabel <p.zabel@pengutronix.de> >> >> For video capture it is the driver that reports the colorspace, >> Y'CbCr/HSV encoding, quantization range and transfer function >> used by the video, and there is no way to request something >> different, even though many HDTV receivers have some sort of >> colorspace conversion capabilities. >> >> For output video this feature already exists since the application >> specifies this information for the video format it will send out, and >> the transmitter will enable any available CSC if a format conversion has >> to be performed in order to match the capabilities of the sink. >> >> For video capture we propose adding new pix_format flag: >> V4L2_PIX_FMT_FLAG_HAS_CSC. The flag is set by the application, > > That should be V4L2_PIX_FMT_FLAG_REQUEST_CSC, since the application actively > requests the use of the CSC. Maybe change to V4L2_PIX_FMT_FLAG_ASK_FOR_CSC so people won't think it is related to the request API ? > >> the driver will interpret the ycbcr_enc/hsv_enc, and quantization fields >> as the requested colorspace information and will attempt to >> do the conversion it supports. >> >> Drivers set the flags >> V4L2_FMT_FLAG_CSC_YCBCR_ENC, >> V4L2_FMT_FLAG_CSC_HSV_ENC, >> V4L2_FMT_FLAG_CSC_HSV_QUANTIZATION, > > That last define should be 'V4L2_FMT_FLAG_CSC_QUANTIZATION' without 'HSV_'. > >> in the flags field of the struct v4l2_fmtdesc during enumeration to >> indicate that they support colorspace conversion for the respective field. >> Currently the conversion of the fields colorspace, xfer_func is not >> supported since there are no drivers that support it. >> >> The same API is added for the subdevices. With the flag V4L2_MBUS_FRAMEFMT_HAS_CSC > > Should also be REQUEST_CSC. > >> set by the application in the VIDIOC_SUBDEV_S_FMT ioctl and the flags >> V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC, V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION >> set by the driver in the VIDIOC_SUBDEV_ENUM_MBUS_CODE ioctl. >> >> For subdevices, new 'flags' fields were added to the structs >> v4l2_subdev_mbus_code_enum, v4l2_mbus_framefmt which are borrowed from the 'reserved' field >> >> Drivers do not have to actually look at the flags: if the flags are not >> set, then the colorspace, ycbcr_enc and quantization fields are set to >> the default values by the core, i.e. just pass on the received format >> without conversion. >> >> Signed-off-by: Hans Verkuil <Hans Verkuil@cisco.com> >> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> >> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> >> --- >> This is v3 of the RFC suggested originaly by Hans Verkuil: >> >> https://patchwork.linuxtv.org/patch/28847/ >> >> And then a v2 from Philipp Zabel: >> >> https://patchwork.kernel.org/project/linux-media/list/?series=15483 >> >> changes in v3: >> I added the API to subdevices as well and added fixes according to >> comments from Hans. >> I also added a usecase for the new API for the rkisp1 driver. >> >> changes in v2 (reported by Philipp Zabel): >> - convert to rst >> - split V4L2_PIX_FMT_FLAG_REQUEST_CSC into four separate flags for >> colorspace, ycbcr_enc/hsv_enc, quantization, and xfer_func >> - let driver set flags to indicate supported features >> >> [1] https://patchwork.linuxtv.org/patch/28847/ >> >> .../media/v4l/pixfmt-reserved.rst | 6 +++ >> .../media/v4l/pixfmt-v4l2-mplane.rst | 16 ++----- >> .../userspace-api/media/v4l/pixfmt-v4l2.rst | 36 +++++++++++++--- >> .../media/v4l/subdev-formats.rst | 42 +++++++++++++++++-- >> .../media/v4l/vidioc-enum-fmt.rst | 18 ++++++++ >> .../v4l/vidioc-subdev-enum-mbus-code.rst | 23 ++++++++++ >> drivers/media/v4l2-core/v4l2-ioctl.c | 19 ++++++++- >> drivers/media/v4l2-core/v4l2-subdev.c | 7 ++++ >> include/uapi/linux/v4l2-mediabus.h | 5 ++- >> include/uapi/linux/v4l2-subdev.h | 5 ++- >> include/uapi/linux/videodev2.h | 4 ++ >> 11 files changed, 158 insertions(+), 23 deletions(-) >> >> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst >> index 59b9e7238f90..fa8dada69f8c 100644 >> --- a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst >> +++ b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst > > Why on earth did the documentation of these pixel format flags end up in > pixfmt-reserved.rst? > > Can you make a patch that moves this to pixfmt-v4l2.rst? > >> @@ -280,3 +280,9 @@ please make a proposal on the linux-media mailing list. >> by RGBA values (128, 192, 255, 128), the same pixel described with >> premultiplied colors would be described by RGBA values (64, 96, >> 128, 128) >> + * - ``V4L2_PIX_FMT_FLAG_HAS_CSC`` >> + - 0x00000002 >> + - Set by the application. It is only used for capture and is >> + ignored for output streams. If set, then request the driver to do >> + colorspace conversion from the received colorspace, only conversions >> + of Ycbcr encoding, HSV encoding and quantization are supported. > > I'd replace the part "If set..." with this: > > ---------- > If set, then request the driver to do colorspace conversion from the received > colorspace to the requested colorspace values. If a colorimetry field > (``colorspace``, ``xfer_func``, ``ycncr_enc`` or ``quantization``) is set to 0, > then that colorimetry setting will remain unchanged from what was received. > So to just change the quantization only the ``quantization`` field shall be set > to a non-zero value (``V4L2_QUANTIZATION_FULL_RANGE`` or > ``V4L2_QUANTIZATION_LIM_RANGE``) and all other colorimetry fields shall be set to 0. > > To check which conversions are supported by the hardware for the current pixel > format, see :ref:`fmtdesc-flags`. > ---------- > > >> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst >> index 444b4082684c..66f3365d7b72 100644 >> --- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst >> +++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst >> @@ -105,29 +105,21 @@ describing all planes of that format. >> * - __u8 >> - ``ycbcr_enc`` >> - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`. >> - This information supplements the ``colorspace`` and must be set by >> - the driver for capture streams and by the application for output >> - streams, see :ref:`colorspaces`. >> + See struct :c:type:`v4l2_pix_format`. >> * - __u8 >> - ``hsv_enc`` >> - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`. >> - This information supplements the ``colorspace`` and must be set by >> - the driver for capture streams and by the application for output >> - streams, see :ref:`colorspaces`. >> + See struct :c:type:`v4l2_pix_format`. >> * - } >> - >> * - __u8 >> - ``quantization`` >> - Quantization range, from enum :c:type:`v4l2_quantization`. >> - This information supplements the ``colorspace`` and must be set by >> - the driver for capture streams and by the application for output >> - streams, see :ref:`colorspaces`. >> + See struct :c:type:`v4l2_pix_format`. >> * - __u8 >> - ``xfer_func`` >> - Transfer function, from enum :c:type:`v4l2_xfer_func`. >> - This information supplements the ``colorspace`` and must be set by >> - the driver for capture streams and by the application for output >> - streams, see :ref:`colorspaces`. >> + See struct :c:type:`v4l2_pix_format`. >> * - __u8 >> - ``reserved[7]`` >> - Reserved for future extensions. Should be zeroed by drivers and >> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst >> index 759420a872d6..ce57718cd66b 100644 >> --- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst >> +++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst >> @@ -110,8 +110,8 @@ Single-planar format structure >> - ``colorspace`` >> - Image colorspace, from enum :c:type:`v4l2_colorspace`. >> This information supplements the ``pixelformat`` and must be set >> - by the driver for capture streams and by the application for >> - output streams, see :ref:`colorspaces`. >> + by the driver for capture streams and by the application for output >> + streams, see :ref:`colorspace`. > > This doesn't appear to change anything. > >> * - __u32 >> - ``priv`` >> - This field indicates whether the remaining fields of the >> @@ -148,13 +148,31 @@ Single-planar format structure >> - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`. >> This information supplements the ``colorspace`` and must be set by >> the driver for capture streams and by the application for output >> - streams, see :ref:`colorspaces`. >> + streams, see :ref:`colorspaces`. If the application sets the >> + flag ``V4L2_PIX_FMT_FLAG_HAS_CSC`` then the application can set >> + this field for a capture stream to request a specific Y'CbCr encoding >> + for the captured image data. The driver will attempt to do the >> + conversion to the specified Y'CbCr encoding or return the encoding it >> + will use if it can't do the conversion. This field is ignored for >> + non-Y'CbCr pixelformats. The driver indicates that ycbcr_enc conversion >> + is supported by setting the flag V4L2_FMT_FLAG_CSC_YCBCR_ENC on the >> + on the corresponding struct :c:type:`v4l2_fmtdesc` during enumeration. > > on the on the -> in the > >> + See :ref:`fmtdesc-flags` >> * - __u32 >> - ``hsv_enc`` >> - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`. >> This information supplements the ``colorspace`` and must be set by >> the driver for capture streams and by the application for output >> - streams, see :ref:`colorspaces`. >> + streams, see :ref:`colorspaces`. If the application sets the flag >> + ``V4L2_PIX_FMT_FLAG_HAS_CSC`` then the application can set this >> + field for a capture stream to request a specific HSV encoding for the >> + captured image data. The driver will attempt to do the conversion to >> + the specified HSV encoding or return the encoding it will use if it >> + can't do the conversion. This field is ignored for non-HSV >> + pixelformats. The driver indicates that hsv_enc conversion >> + is supported by setting the flag V4L2_FMT_FLAG_CSC_HSV_ENC on the >> + on the corresponding struct :c:type:`v4l2_fmtdesc` during enumeration. > > Ditto. > >> + See :ref:`fmtdesc-flags` >> * - } >> - >> * - __u32 >> @@ -162,7 +180,15 @@ Single-planar format structure >> - Quantization range, from enum :c:type:`v4l2_quantization`. >> This information supplements the ``colorspace`` and must be set by >> the driver for capture streams and by the application for output >> - streams, see :ref:`colorspaces`. >> + streams, see :ref:`colorspaces`. If the application sets the flag >> + ``V4L2_PIX_FMT_FLAG_HAS_CSC`` then the application can set >> + this field for a capture stream to request a specific quantization >> + range for the captured image data. The driver will attempt to do the >> + conversion to the specified quantization range or return the >> + quantization it will use if it can't do the conversion. The driver >> + indicates that quantization conversion is supported by setting the flag >> + V4L2_FMT_FLAG_CSC_QUANTIZATION on the on the corresponding struct > > Ditto > >> + :c:type:`v4l2_fmtdesc` during enumeration. See :ref:`fmtdesc-flags` >> * - __u32 >> - ``xfer_func`` >> - Transfer function, from enum :c:type:`v4l2_xfer_func`. >> diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst >> index 9a4d61b0d76f..f1d4ca29a3e8 100644 >> --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst >> +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst >> @@ -49,13 +49,33 @@ Media Bus Formats >> - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`. >> This information supplements the ``colorspace`` and must be set by >> the driver for capture streams and by the application for output >> - streams, see :ref:`colorspaces`. >> + streams, see :ref:`colorspaces`. If the application sets the >> + flag ``V4L2_MBUS_FRAMEFMT_HAS_CSC`` the the application can set >> + this field for a capture stream to request a specific Y'CbCr encoding >> + for the mbus data. The driver will attempt to do the > > mbus -> media bus > >> + conversion to the specified Y'CbCr encoding or return the encoding it >> + will use if it can't do the conversion. This field is ignored for >> + non-Y'CbCr pixelformats. The driver indicates that ycbcr_enc conversion > > This is a media bus format, not a pixelformat. > >> + is supported by setting the flag V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC on > > on -> in > >> + the corresponding struct c:type:`v4l2_subdev_mbus_code_enum` during >> + enumeration. See :ref:`vidioc-subdev-enum-mbus-code.rst <v4l2-subdev-mbus-code-flags>` >> + >> * - __u16 >> - ``quantization`` >> - Quantization range, from enum :c:type:`v4l2_quantization`. >> This information supplements the ``colorspace`` and must be set by >> the driver for capture streams and by the application for output >> - streams, see :ref:`colorspaces`. >> + streams, see :ref:`colorspaces`. If the application sets the >> + flag ``V4L2_MBUS_FRAMEFMT_HAS_CSC`` the the application can set >> + this field for a capture stream to request a specific quantization >> + encoding for the mbus data. The driver will attempt to do the > > mbus -> media bus > >> + conversion to the specified quantization or return the quantization it >> + will use if it can't do the conversion. The driver indicates that >> + quantization conversion is supported by setting the flag >> + V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION on the corresponding struct > > on -> in > >> + c:type:`v4l2_subdev_mbus_code_enum` during enumeration. >> + See :ref:`vidioc-subdev-enum-mbus-code.rst <v4l2-subdev-mbus-code-flags>` >> + >> * - __u16 >> - ``xfer_func`` >> - Transfer function, from enum :c:type:`v4l2_xfer_func`. >> @@ -63,10 +83,26 @@ Media Bus Formats >> the driver for capture streams and by the application for output >> streams, see :ref:`colorspaces`. >> * - __u16 >> - - ``reserved``\ [11] >> + - ``flags`` >> + - flags see: :ref:v4l2-mbus-framefmt-flags > > flags see: -> See: > >> + * - __u16 >> + - ``reserved``\ [10] >> - Reserved for future extensions. Applications and drivers must set >> the array to zero. >> >> +.. _v4l2-mbus-framefmt-flags: >> + >> +.. flat-table:: v4l2_mbus_framefmt Flags >> + :header-rows: 0 >> + :stub-columns: 0 >> + :widths: 3 1 4 >> + >> + * - ``V4L2_MBUS_FRAMEFMT_HAS_CSC`` >> + - 0x0001 >> + - Set by the application. It is only used for capture and is >> + ignored for output streams. If set, then request the subdevice to do >> + colorspace conversion from the received colorspace, only conversions >> + of Ycbcr encoding, and quantization are supported. > > Replace by a similar text as for V4L2_PIX_FMT_FLAG_HAS_CSC. > >> >> >> .. _v4l2-mbus-pixelcode: >> diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst >> index 7e3142e11d77..125f074543af 100644 >> --- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst >> +++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst >> @@ -145,6 +145,24 @@ formats in preference order, where preferred formats are returned before >> parameters are detected. This flag can only be used in combination >> with the ``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to >> compressed formats only. It is also only applies to stateful codecs. >> + * - ``V4L2_FMT_FLAG_CSC_YCBCR_ENC`` >> + - 0x0010 >> + - The driver allows the application to try to change the default >> + ycbcr encoding. This flag is relevant only for capture devices. >> + The application can ask to configure the ycbcr_enc of the capture device >> + when calling the :c:func:`VIDIOC_S_FMT` ioctl. > > add: ...ioctl with ``V4L2_FMT_FLAG_REQUEST_CSC`` set. (this should probably be > a reference to the REQUEST_CSC flag documentation). > >> + * - ``V4L2_FMT_FLAG_CSC_HSV_ENC`` >> + - 0x0010 >> + - The driver allows the application to try to change the default >> + hsv encoding. This flag is relevant only for capture devices. >> + The application can ask to configure the hsv_enc of the capture device >> + when calling the :c:func:`VIDIOC_S_FMT` ioctl. > > Ditto. Also: hsv -> HSV. > >> + * - ``V4L2_FMT_FLAG_CSC_QUANTIZATION`` >> + - 0x0020 >> + - The driver allows the application to try to change the default >> + quantization. This flag is relevant only for capture devices. >> + The application can ask to configure the quantization of the capture device >> + when calling the :c:func:`VIDIOC_S_FMT` ioctl. > > Ditto. > >> >> >> Return Value >> diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst >> index 35b8607203a4..4ad87cb74f57 100644 >> --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst >> +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst >> @@ -78,11 +78,34 @@ information about the try formats. >> - ``which`` >> - Media bus format codes to be enumerated, from enum >> :ref:`v4l2_subdev_format_whence <v4l2-subdev-format-whence>`. >> + * - __u32 >> + - ``flags`` >> + - see :ref:`v4l2-subdev-mbus-code-flags` > > see -> See > >> * - __u32 >> - ``reserved``\ [8] >> - Reserved for future extensions. Applications and drivers must set >> the array to zero. >> >> +.. _v4l2-subdev-mbus-code-flags: >> + >> + >> +.. tabularcolumns:: |p{4.4cm}|p{4.4cm}|p{7.7cm}| >> + >> +.. flat-table:: flags in struct v4l2_subdev_mbus_code_enum >> + :header-rows: 0 >> + :stub-columns: 0 >> + :widths: 1 1 2 >> + >> + * - V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC >> + - 0x00000001 >> + - The driver supports setting the ycbcr encoding by the application >> + when calling the VIDIOC_SUBDEV_S_FMT ioctl. see :ref:`v4l2-mbus-format` > > see -> See > >> + on how to do this. >> + * - V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION >> + - 0x00000002 >> + - The driver supports setting the quantization by the application >> + when calling the VIDIOC_SUBDEV_S_FMT ioctl. see :ref:`v4l2-mbus-format` > > see -> See > >> + on how to do this. >> >> Return Value >> ============ >> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c >> index b2ef8e60ea7d..3c7ffb6b15cb 100644 >> --- a/drivers/media/v4l2-core/v4l2-ioctl.c >> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c >> @@ -1029,6 +1029,15 @@ static void v4l_sanitize_format(struct v4l2_format *fmt) >> fmt->fmt.pix_mp.num_planes = min_t(u32, fmt->fmt.pix_mp.num_planes, >> VIDEO_MAX_PLANES); >> >> + if (fmt->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) { >> + if (fmt->fmt.pix_mp.flags & V4L2_PIX_FMT_FLAG_HAS_CSC) >> + return; >> + fmt->fmt.pix_mp.colorspace = V4L2_COLORSPACE_DEFAULT; >> + fmt->fmt.pix_mp.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; >> + fmt->fmt.pix_mp.quantization = V4L2_QUANTIZATION_DEFAULT; >> + fmt->fmt.pix_mp.xfer_func = V4L2_XFER_FUNC_DEFAULT; >> + } >> + >> /* >> * The v4l2_pix_format structure has been extended with fields that were >> * not previously required to be set to zero by applications. The priv >> @@ -1043,8 +1052,16 @@ static void v4l_sanitize_format(struct v4l2_format *fmt) >> fmt->type != V4L2_BUF_TYPE_VIDEO_OUTPUT) >> return; >> >> - if (fmt->fmt.pix.priv == V4L2_PIX_FMT_PRIV_MAGIC) >> + if (fmt->fmt.pix.priv == V4L2_PIX_FMT_PRIV_MAGIC) { >> + if (fmt->type != V4L2_BUF_TYPE_VIDEO_CAPTURE || >> + fmt->fmt.pix.flags & V4L2_PIX_FMT_FLAG_HAS_CSC) >> + return; >> + fmt->fmt.pix.colorspace = V4L2_COLORSPACE_DEFAULT; >> + fmt->fmt.pix.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; >> + fmt->fmt.pix.quantization = V4L2_QUANTIZATION_DEFAULT; >> + fmt->fmt.pix.xfer_func = V4L2_XFER_FUNC_DEFAULT; >> return; >> + } >> >> fmt->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC; >> >> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c >> index a376b351135f..51e009936aad 100644 >> --- a/drivers/media/v4l2-core/v4l2-subdev.c >> +++ b/drivers/media/v4l2-core/v4l2-subdev.c >> @@ -477,6 +477,13 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) >> case VIDIOC_SUBDEV_S_FMT: { >> struct v4l2_subdev_format *format = arg; >> >> + if (!(format->format.flags & V4L2_MBUS_FRAMEFMT_HAS_CSC)) { >> + format->format.colorspace = V4L2_COLORSPACE_DEFAULT; >> + format->format.xfer_func = V4L2_XFER_FUNC_DEFAULT; >> + format->format.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; >> + format->format.quantization = V4L2_QUANTIZATION_DEFAULT; >> + } >> + >> memset(format->reserved, 0, sizeof(format->reserved)); >> memset(format->format.reserved, 0, sizeof(format->format.reserved)); >> return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh->pad, format); >> diff --git a/include/uapi/linux/v4l2-mediabus.h b/include/uapi/linux/v4l2-mediabus.h >> index 123a231001a8..89ff0ad6a631 100644 >> --- a/include/uapi/linux/v4l2-mediabus.h >> +++ b/include/uapi/linux/v4l2-mediabus.h >> @@ -16,6 +16,8 @@ >> #include <linux/types.h> >> #include <linux/videodev2.h> >> >> +#define V4L2_MBUS_FRAMEFMT_HAS_CSC 0x0001 >> + >> /** >> * struct v4l2_mbus_framefmt - frame format on the media bus >> * @width: image width >> @@ -36,7 +38,8 @@ struct v4l2_mbus_framefmt { >> __u16 ycbcr_enc; >> __u16 quantization; >> __u16 xfer_func; >> - __u16 reserved[11]; >> + __u16 flags; >> + __u16 reserved[10]; >> }; >> >> #ifndef __KERNEL__ >> diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h >> index 03970ce30741..4410b26a7158 100644 >> --- a/include/uapi/linux/v4l2-subdev.h >> +++ b/include/uapi/linux/v4l2-subdev.h >> @@ -65,6 +65,8 @@ struct v4l2_subdev_crop { >> __u32 reserved[8]; >> }; >> >> +#define V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC 0x00000001 > > I think it is better to add an HSV_ENC alias here as well. > > It's for readability only, since talking about YCBCR_ENC when the mediabus format > is a HSV format is just weird. I see that in 'v4l2_pix_format' the {ycbcr_enc, hsv_enc} are a union but in 'v4l2_mbus_framefmt' there is only the field ycbcr_enc. I wonder why is that, since I see there is one HSV media bus format V4L2_MBUS_FROM_MEDIA_BUS_FMT(AHSV8888_1X32). So I guess this is why I didn't add a V4L2_SUBDEV_MBUS_CODE_CSC_HSV_ENC define. I also didn't add this flag to the docs. Should I? Thanks, Dafna > >> +#define V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION 0x00000002 >> /** >> * struct v4l2_subdev_mbus_code_enum - Media bus format enumeration >> * @pad: pad number, as reported by the media API >> @@ -77,7 +79,8 @@ struct v4l2_subdev_mbus_code_enum { >> __u32 index; >> __u32 code; >> __u32 which; >> - __u32 reserved[8]; >> + __u32 flags; >> + __u32 reserved[7]; >> }; >> >> /** >> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h >> index 9817b7e2c968..adc9dd1080b8 100644 >> --- a/include/uapi/linux/videodev2.h >> +++ b/include/uapi/linux/videodev2.h >> @@ -772,6 +772,7 @@ struct v4l2_pix_format { >> >> /* Flags */ >> #define V4L2_PIX_FMT_FLAG_PREMUL_ALPHA 0x00000001 >> +#define V4L2_PIX_FMT_FLAG_HAS_CSC 0x00000002 >> >> /* >> * F O R M A T E N U M E R A T I O N >> @@ -789,6 +790,9 @@ struct v4l2_fmtdesc { >> #define V4L2_FMT_FLAG_EMULATED 0x0002 >> #define V4L2_FMT_FLAG_CONTINUOUS_BYTESTREAM 0x0004 >> #define V4L2_FMT_FLAG_DYN_RESOLUTION 0x0008 >> +#define V4L2_FMT_FLAG_CSC_YCBCR_ENC 0x0010 >> +#define V4L2_FMT_FLAG_CSC_HSV_ENC 0x0010 >> +#define V4L2_FMT_FLAG_CSC_QUANTIZATION 0x0020 >> >> /* Frame Size and frame rate enumeration */ >> /* >> > > I think this is quite good, it's mainly doc improvements and renaming HAS_CSC to REQUEST_CSC. > > That said, I do want to see this supported in at least vivid (vimc would be nice > too!). And v4l2-ctl needs a patch to support this. > > A compliance test for v4l2-compliance would be nice too, but it is not a prerequisite. > I'm not entirely sure what would be the best way to test this. Perhaps a vivid-specific > test might be easiest (i.e., don't test it for all drivers, but just for vivid). > > Regards, > > Hans >
On 25/05/2020 15:56, Dafna Hirschfeld wrote: > Hi > > On 07.05.20 15:00, Hans Verkuil wrote: >> On 16/04/2020 16:56, Dafna Hirschfeld wrote: >>> From: Philipp Zabel <p.zabel@pengutronix.de> >>> >>> For video capture it is the driver that reports the colorspace, >>> Y'CbCr/HSV encoding, quantization range and transfer function >>> used by the video, and there is no way to request something >>> different, even though many HDTV receivers have some sort of >>> colorspace conversion capabilities. >>> >>> For output video this feature already exists since the application >>> specifies this information for the video format it will send out, and >>> the transmitter will enable any available CSC if a format conversion has >>> to be performed in order to match the capabilities of the sink. >>> >>> For video capture we propose adding new pix_format flag: >>> V4L2_PIX_FMT_FLAG_HAS_CSC. The flag is set by the application, >> >> That should be V4L2_PIX_FMT_FLAG_REQUEST_CSC, since the application actively >> requests the use of the CSC. > Maybe change to V4L2_PIX_FMT_FLAG_ASK_FOR_CSC so people won't think > it is related to the request API ? How about _SET_CSC? I think that's even better than REQUEST_CSC. >> >>> the driver will interpret the ycbcr_enc/hsv_enc, and quantization fields >>> as the requested colorspace information and will attempt to >>> do the conversion it supports. >>> >>> Drivers set the flags >>> V4L2_FMT_FLAG_CSC_YCBCR_ENC, >>> V4L2_FMT_FLAG_CSC_HSV_ENC, >>> V4L2_FMT_FLAG_CSC_HSV_QUANTIZATION, >> >> That last define should be 'V4L2_FMT_FLAG_CSC_QUANTIZATION' without 'HSV_'. >> >>> in the flags field of the struct v4l2_fmtdesc during enumeration to >>> indicate that they support colorspace conversion for the respective field. >>> Currently the conversion of the fields colorspace, xfer_func is not >>> supported since there are no drivers that support it. >>> >>> The same API is added for the subdevices. With the flag V4L2_MBUS_FRAMEFMT_HAS_CSC >> >> Should also be REQUEST_CSC. >> >>> set by the application in the VIDIOC_SUBDEV_S_FMT ioctl and the flags >>> V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC, V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION >>> set by the driver in the VIDIOC_SUBDEV_ENUM_MBUS_CODE ioctl. >>> >>> For subdevices, new 'flags' fields were added to the structs >>> v4l2_subdev_mbus_code_enum, v4l2_mbus_framefmt which are borrowed from the 'reserved' field >>> >>> Drivers do not have to actually look at the flags: if the flags are not >>> set, then the colorspace, ycbcr_enc and quantization fields are set to >>> the default values by the core, i.e. just pass on the received format >>> without conversion. >>> >>> Signed-off-by: Hans Verkuil <Hans Verkuil@cisco.com> >>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> >>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> >>> --- >>> This is v3 of the RFC suggested originaly by Hans Verkuil: >>> >>> https://patchwork.linuxtv.org/patch/28847/ >>> >>> And then a v2 from Philipp Zabel: >>> >>> https://patchwork.kernel.org/project/linux-media/list/?series=15483 >>> >>> changes in v3: >>> I added the API to subdevices as well and added fixes according to >>> comments from Hans. >>> I also added a usecase for the new API for the rkisp1 driver. >>> >>> changes in v2 (reported by Philipp Zabel): >>> - convert to rst >>> - split V4L2_PIX_FMT_FLAG_REQUEST_CSC into four separate flags for >>> colorspace, ycbcr_enc/hsv_enc, quantization, and xfer_func >>> - let driver set flags to indicate supported features >>> >>> [1] https://patchwork.linuxtv.org/patch/28847/ >>> >>> .../media/v4l/pixfmt-reserved.rst | 6 +++ >>> .../media/v4l/pixfmt-v4l2-mplane.rst | 16 ++----- >>> .../userspace-api/media/v4l/pixfmt-v4l2.rst | 36 +++++++++++++--- >>> .../media/v4l/subdev-formats.rst | 42 +++++++++++++++++-- >>> .../media/v4l/vidioc-enum-fmt.rst | 18 ++++++++ >>> .../v4l/vidioc-subdev-enum-mbus-code.rst | 23 ++++++++++ >>> drivers/media/v4l2-core/v4l2-ioctl.c | 19 ++++++++- >>> drivers/media/v4l2-core/v4l2-subdev.c | 7 ++++ >>> include/uapi/linux/v4l2-mediabus.h | 5 ++- >>> include/uapi/linux/v4l2-subdev.h | 5 ++- >>> include/uapi/linux/videodev2.h | 4 ++ >>> 11 files changed, 158 insertions(+), 23 deletions(-) >>> >>> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst >>> index 59b9e7238f90..fa8dada69f8c 100644 >>> --- a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst >>> +++ b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst >> >> Why on earth did the documentation of these pixel format flags end up in >> pixfmt-reserved.rst? >> >> Can you make a patch that moves this to pixfmt-v4l2.rst? >> >>> @@ -280,3 +280,9 @@ please make a proposal on the linux-media mailing list. >>> by RGBA values (128, 192, 255, 128), the same pixel described with >>> premultiplied colors would be described by RGBA values (64, 96, >>> 128, 128) >>> + * - ``V4L2_PIX_FMT_FLAG_HAS_CSC`` >>> + - 0x00000002 >>> + - Set by the application. It is only used for capture and is >>> + ignored for output streams. If set, then request the driver to do >>> + colorspace conversion from the received colorspace, only conversions >>> + of Ycbcr encoding, HSV encoding and quantization are supported. >> >> I'd replace the part "If set..." with this: >> >> ---------- >> If set, then request the driver to do colorspace conversion from the received >> colorspace to the requested colorspace values. If a colorimetry field >> (``colorspace``, ``xfer_func``, ``ycncr_enc`` or ``quantization``) is set to 0, >> then that colorimetry setting will remain unchanged from what was received. >> So to just change the quantization only the ``quantization`` field shall be set >> to a non-zero value (``V4L2_QUANTIZATION_FULL_RANGE`` or >> ``V4L2_QUANTIZATION_LIM_RANGE``) and all other colorimetry fields shall be set to 0. >> >> To check which conversions are supported by the hardware for the current pixel >> format, see :ref:`fmtdesc-flags`. >> ---------- >> >> >>> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst >>> index 444b4082684c..66f3365d7b72 100644 >>> --- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst >>> +++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst >>> @@ -105,29 +105,21 @@ describing all planes of that format. >>> * - __u8 >>> - ``ycbcr_enc`` >>> - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`. >>> - This information supplements the ``colorspace`` and must be set by >>> - the driver for capture streams and by the application for output >>> - streams, see :ref:`colorspaces`. >>> + See struct :c:type:`v4l2_pix_format`. >>> * - __u8 >>> - ``hsv_enc`` >>> - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`. >>> - This information supplements the ``colorspace`` and must be set by >>> - the driver for capture streams and by the application for output >>> - streams, see :ref:`colorspaces`. >>> + See struct :c:type:`v4l2_pix_format`. >>> * - } >>> - >>> * - __u8 >>> - ``quantization`` >>> - Quantization range, from enum :c:type:`v4l2_quantization`. >>> - This information supplements the ``colorspace`` and must be set by >>> - the driver for capture streams and by the application for output >>> - streams, see :ref:`colorspaces`. >>> + See struct :c:type:`v4l2_pix_format`. >>> * - __u8 >>> - ``xfer_func`` >>> - Transfer function, from enum :c:type:`v4l2_xfer_func`. >>> - This information supplements the ``colorspace`` and must be set by >>> - the driver for capture streams and by the application for output >>> - streams, see :ref:`colorspaces`. >>> + See struct :c:type:`v4l2_pix_format`. >>> * - __u8 >>> - ``reserved[7]`` >>> - Reserved for future extensions. Should be zeroed by drivers and >>> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst >>> index 759420a872d6..ce57718cd66b 100644 >>> --- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst >>> +++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst >>> @@ -110,8 +110,8 @@ Single-planar format structure >>> - ``colorspace`` >>> - Image colorspace, from enum :c:type:`v4l2_colorspace`. >>> This information supplements the ``pixelformat`` and must be set >>> - by the driver for capture streams and by the application for >>> - output streams, see :ref:`colorspaces`. >>> + by the driver for capture streams and by the application for output >>> + streams, see :ref:`colorspace`. >> >> This doesn't appear to change anything. >> >>> * - __u32 >>> - ``priv`` >>> - This field indicates whether the remaining fields of the >>> @@ -148,13 +148,31 @@ Single-planar format structure >>> - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`. >>> This information supplements the ``colorspace`` and must be set by >>> the driver for capture streams and by the application for output >>> - streams, see :ref:`colorspaces`. >>> + streams, see :ref:`colorspaces`. If the application sets the >>> + flag ``V4L2_PIX_FMT_FLAG_HAS_CSC`` then the application can set >>> + this field for a capture stream to request a specific Y'CbCr encoding >>> + for the captured image data. The driver will attempt to do the >>> + conversion to the specified Y'CbCr encoding or return the encoding it >>> + will use if it can't do the conversion. This field is ignored for >>> + non-Y'CbCr pixelformats. The driver indicates that ycbcr_enc conversion >>> + is supported by setting the flag V4L2_FMT_FLAG_CSC_YCBCR_ENC on the >>> + on the corresponding struct :c:type:`v4l2_fmtdesc` during enumeration. >> >> on the on the -> in the >> >>> + See :ref:`fmtdesc-flags` >>> * - __u32 >>> - ``hsv_enc`` >>> - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`. >>> This information supplements the ``colorspace`` and must be set by >>> the driver for capture streams and by the application for output >>> - streams, see :ref:`colorspaces`. >>> + streams, see :ref:`colorspaces`. If the application sets the flag >>> + ``V4L2_PIX_FMT_FLAG_HAS_CSC`` then the application can set this >>> + field for a capture stream to request a specific HSV encoding for the >>> + captured image data. The driver will attempt to do the conversion to >>> + the specified HSV encoding or return the encoding it will use if it >>> + can't do the conversion. This field is ignored for non-HSV >>> + pixelformats. The driver indicates that hsv_enc conversion >>> + is supported by setting the flag V4L2_FMT_FLAG_CSC_HSV_ENC on the >>> + on the corresponding struct :c:type:`v4l2_fmtdesc` during enumeration. >> >> Ditto. >> >>> + See :ref:`fmtdesc-flags` >>> * - } >>> - >>> * - __u32 >>> @@ -162,7 +180,15 @@ Single-planar format structure >>> - Quantization range, from enum :c:type:`v4l2_quantization`. >>> This information supplements the ``colorspace`` and must be set by >>> the driver for capture streams and by the application for output >>> - streams, see :ref:`colorspaces`. >>> + streams, see :ref:`colorspaces`. If the application sets the flag >>> + ``V4L2_PIX_FMT_FLAG_HAS_CSC`` then the application can set >>> + this field for a capture stream to request a specific quantization >>> + range for the captured image data. The driver will attempt to do the >>> + conversion to the specified quantization range or return the >>> + quantization it will use if it can't do the conversion. The driver >>> + indicates that quantization conversion is supported by setting the flag >>> + V4L2_FMT_FLAG_CSC_QUANTIZATION on the on the corresponding struct >> >> Ditto >> >>> + :c:type:`v4l2_fmtdesc` during enumeration. See :ref:`fmtdesc-flags` >>> * - __u32 >>> - ``xfer_func`` >>> - Transfer function, from enum :c:type:`v4l2_xfer_func`. >>> diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst >>> index 9a4d61b0d76f..f1d4ca29a3e8 100644 >>> --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst >>> +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst >>> @@ -49,13 +49,33 @@ Media Bus Formats >>> - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`. >>> This information supplements the ``colorspace`` and must be set by >>> the driver for capture streams and by the application for output >>> - streams, see :ref:`colorspaces`. >>> + streams, see :ref:`colorspaces`. If the application sets the >>> + flag ``V4L2_MBUS_FRAMEFMT_HAS_CSC`` the the application can set >>> + this field for a capture stream to request a specific Y'CbCr encoding >>> + for the mbus data. The driver will attempt to do the >> >> mbus -> media bus >> >>> + conversion to the specified Y'CbCr encoding or return the encoding it >>> + will use if it can't do the conversion. This field is ignored for >>> + non-Y'CbCr pixelformats. The driver indicates that ycbcr_enc conversion >> >> This is a media bus format, not a pixelformat. >> >>> + is supported by setting the flag V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC on >> >> on -> in >> >>> + the corresponding struct c:type:`v4l2_subdev_mbus_code_enum` during >>> + enumeration. See :ref:`vidioc-subdev-enum-mbus-code.rst <v4l2-subdev-mbus-code-flags>` >>> + >>> * - __u16 >>> - ``quantization`` >>> - Quantization range, from enum :c:type:`v4l2_quantization`. >>> This information supplements the ``colorspace`` and must be set by >>> the driver for capture streams and by the application for output >>> - streams, see :ref:`colorspaces`. >>> + streams, see :ref:`colorspaces`. If the application sets the >>> + flag ``V4L2_MBUS_FRAMEFMT_HAS_CSC`` the the application can set >>> + this field for a capture stream to request a specific quantization >>> + encoding for the mbus data. The driver will attempt to do the >> >> mbus -> media bus >> >>> + conversion to the specified quantization or return the quantization it >>> + will use if it can't do the conversion. The driver indicates that >>> + quantization conversion is supported by setting the flag >>> + V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION on the corresponding struct >> >> on -> in >> >>> + c:type:`v4l2_subdev_mbus_code_enum` during enumeration. >>> + See :ref:`vidioc-subdev-enum-mbus-code.rst <v4l2-subdev-mbus-code-flags>` >>> + >>> * - __u16 >>> - ``xfer_func`` >>> - Transfer function, from enum :c:type:`v4l2_xfer_func`. >>> @@ -63,10 +83,26 @@ Media Bus Formats >>> the driver for capture streams and by the application for output >>> streams, see :ref:`colorspaces`. >>> * - __u16 >>> - - ``reserved``\ [11] >>> + - ``flags`` >>> + - flags see: :ref:v4l2-mbus-framefmt-flags >> >> flags see: -> See: >> >>> + * - __u16 >>> + - ``reserved``\ [10] >>> - Reserved for future extensions. Applications and drivers must set >>> the array to zero. >>> >>> +.. _v4l2-mbus-framefmt-flags: >>> + >>> +.. flat-table:: v4l2_mbus_framefmt Flags >>> + :header-rows: 0 >>> + :stub-columns: 0 >>> + :widths: 3 1 4 >>> + >>> + * - ``V4L2_MBUS_FRAMEFMT_HAS_CSC`` >>> + - 0x0001 >>> + - Set by the application. It is only used for capture and is >>> + ignored for output streams. If set, then request the subdevice to do >>> + colorspace conversion from the received colorspace, only conversions >>> + of Ycbcr encoding, and quantization are supported. >> >> Replace by a similar text as for V4L2_PIX_FMT_FLAG_HAS_CSC. >> >>> >>> >>> .. _v4l2-mbus-pixelcode: >>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst >>> index 7e3142e11d77..125f074543af 100644 >>> --- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst >>> +++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst >>> @@ -145,6 +145,24 @@ formats in preference order, where preferred formats are returned before >>> parameters are detected. This flag can only be used in combination >>> with the ``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to >>> compressed formats only. It is also only applies to stateful codecs. >>> + * - ``V4L2_FMT_FLAG_CSC_YCBCR_ENC`` >>> + - 0x0010 >>> + - The driver allows the application to try to change the default >>> + ycbcr encoding. This flag is relevant only for capture devices. >>> + The application can ask to configure the ycbcr_enc of the capture device >>> + when calling the :c:func:`VIDIOC_S_FMT` ioctl. >> >> add: ...ioctl with ``V4L2_FMT_FLAG_REQUEST_CSC`` set. (this should probably be >> a reference to the REQUEST_CSC flag documentation). >> >>> + * - ``V4L2_FMT_FLAG_CSC_HSV_ENC`` >>> + - 0x0010 >>> + - The driver allows the application to try to change the default >>> + hsv encoding. This flag is relevant only for capture devices. >>> + The application can ask to configure the hsv_enc of the capture device >>> + when calling the :c:func:`VIDIOC_S_FMT` ioctl. >> >> Ditto. Also: hsv -> HSV. >> >>> + * - ``V4L2_FMT_FLAG_CSC_QUANTIZATION`` >>> + - 0x0020 >>> + - The driver allows the application to try to change the default >>> + quantization. This flag is relevant only for capture devices. >>> + The application can ask to configure the quantization of the capture device >>> + when calling the :c:func:`VIDIOC_S_FMT` ioctl. >> >> Ditto. >> >>> >>> >>> Return Value >>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst >>> index 35b8607203a4..4ad87cb74f57 100644 >>> --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst >>> +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst >>> @@ -78,11 +78,34 @@ information about the try formats. >>> - ``which`` >>> - Media bus format codes to be enumerated, from enum >>> :ref:`v4l2_subdev_format_whence <v4l2-subdev-format-whence>`. >>> + * - __u32 >>> + - ``flags`` >>> + - see :ref:`v4l2-subdev-mbus-code-flags` >> >> see -> See >> >>> * - __u32 >>> - ``reserved``\ [8] >>> - Reserved for future extensions. Applications and drivers must set >>> the array to zero. >>> >>> +.. _v4l2-subdev-mbus-code-flags: >>> + >>> + >>> +.. tabularcolumns:: |p{4.4cm}|p{4.4cm}|p{7.7cm}| >>> + >>> +.. flat-table:: flags in struct v4l2_subdev_mbus_code_enum >>> + :header-rows: 0 >>> + :stub-columns: 0 >>> + :widths: 1 1 2 >>> + >>> + * - V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC >>> + - 0x00000001 >>> + - The driver supports setting the ycbcr encoding by the application >>> + when calling the VIDIOC_SUBDEV_S_FMT ioctl. see :ref:`v4l2-mbus-format` >> >> see -> See >> >>> + on how to do this. >>> + * - V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION >>> + - 0x00000002 >>> + - The driver supports setting the quantization by the application >>> + when calling the VIDIOC_SUBDEV_S_FMT ioctl. see :ref:`v4l2-mbus-format` >> >> see -> See >> >>> + on how to do this. >>> >>> Return Value >>> ============ >>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c >>> index b2ef8e60ea7d..3c7ffb6b15cb 100644 >>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c >>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c >>> @@ -1029,6 +1029,15 @@ static void v4l_sanitize_format(struct v4l2_format *fmt) >>> fmt->fmt.pix_mp.num_planes = min_t(u32, fmt->fmt.pix_mp.num_planes, >>> VIDEO_MAX_PLANES); >>> >>> + if (fmt->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) { >>> + if (fmt->fmt.pix_mp.flags & V4L2_PIX_FMT_FLAG_HAS_CSC) >>> + return; >>> + fmt->fmt.pix_mp.colorspace = V4L2_COLORSPACE_DEFAULT; >>> + fmt->fmt.pix_mp.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; >>> + fmt->fmt.pix_mp.quantization = V4L2_QUANTIZATION_DEFAULT; >>> + fmt->fmt.pix_mp.xfer_func = V4L2_XFER_FUNC_DEFAULT; >>> + } >>> + >>> /* >>> * The v4l2_pix_format structure has been extended with fields that were >>> * not previously required to be set to zero by applications. The priv >>> @@ -1043,8 +1052,16 @@ static void v4l_sanitize_format(struct v4l2_format *fmt) >>> fmt->type != V4L2_BUF_TYPE_VIDEO_OUTPUT) >>> return; >>> >>> - if (fmt->fmt.pix.priv == V4L2_PIX_FMT_PRIV_MAGIC) >>> + if (fmt->fmt.pix.priv == V4L2_PIX_FMT_PRIV_MAGIC) { >>> + if (fmt->type != V4L2_BUF_TYPE_VIDEO_CAPTURE || >>> + fmt->fmt.pix.flags & V4L2_PIX_FMT_FLAG_HAS_CSC) >>> + return; >>> + fmt->fmt.pix.colorspace = V4L2_COLORSPACE_DEFAULT; >>> + fmt->fmt.pix.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; >>> + fmt->fmt.pix.quantization = V4L2_QUANTIZATION_DEFAULT; >>> + fmt->fmt.pix.xfer_func = V4L2_XFER_FUNC_DEFAULT; >>> return; >>> + } >>> >>> fmt->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC; >>> >>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c >>> index a376b351135f..51e009936aad 100644 >>> --- a/drivers/media/v4l2-core/v4l2-subdev.c >>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c >>> @@ -477,6 +477,13 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) >>> case VIDIOC_SUBDEV_S_FMT: { >>> struct v4l2_subdev_format *format = arg; >>> >>> + if (!(format->format.flags & V4L2_MBUS_FRAMEFMT_HAS_CSC)) { >>> + format->format.colorspace = V4L2_COLORSPACE_DEFAULT; >>> + format->format.xfer_func = V4L2_XFER_FUNC_DEFAULT; >>> + format->format.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; >>> + format->format.quantization = V4L2_QUANTIZATION_DEFAULT; >>> + } >>> + >>> memset(format->reserved, 0, sizeof(format->reserved)); >>> memset(format->format.reserved, 0, sizeof(format->format.reserved)); >>> return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh->pad, format); >>> diff --git a/include/uapi/linux/v4l2-mediabus.h b/include/uapi/linux/v4l2-mediabus.h >>> index 123a231001a8..89ff0ad6a631 100644 >>> --- a/include/uapi/linux/v4l2-mediabus.h >>> +++ b/include/uapi/linux/v4l2-mediabus.h >>> @@ -16,6 +16,8 @@ >>> #include <linux/types.h> >>> #include <linux/videodev2.h> >>> >>> +#define V4L2_MBUS_FRAMEFMT_HAS_CSC 0x0001 >>> + >>> /** >>> * struct v4l2_mbus_framefmt - frame format on the media bus >>> * @width: image width >>> @@ -36,7 +38,8 @@ struct v4l2_mbus_framefmt { >>> __u16 ycbcr_enc; >>> __u16 quantization; >>> __u16 xfer_func; >>> - __u16 reserved[11]; >>> + __u16 flags; >>> + __u16 reserved[10]; >>> }; >>> >>> #ifndef __KERNEL__ >>> diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h >>> index 03970ce30741..4410b26a7158 100644 >>> --- a/include/uapi/linux/v4l2-subdev.h >>> +++ b/include/uapi/linux/v4l2-subdev.h >>> @@ -65,6 +65,8 @@ struct v4l2_subdev_crop { >>> __u32 reserved[8]; >>> }; >>> >>> +#define V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC 0x00000001 >> >> I think it is better to add an HSV_ENC alias here as well. >> >> It's for readability only, since talking about YCBCR_ENC when the mediabus format >> is a HSV format is just weird. > > I see that in 'v4l2_pix_format' the {ycbcr_enc, hsv_enc} are a union > but in 'v4l2_mbus_framefmt' there is only the field ycbcr_enc. > I wonder why is that, since I see there is one HSV media bus format > V4L2_MBUS_FROM_MEDIA_BUS_FMT(AHSV8888_1X32). > > So I guess this is why I didn't add a V4L2_SUBDEV_MBUS_CODE_CSC_HSV_ENC define. > I also didn't add this flag to the docs. Should I? I think this can be done in a separate (final) patch where a union is added for v4l2_mbus_framefmt together with a V4L2_SUBDEV_MBUS_CODE_CSC_HSV_ENC define. These are effectively aliases to make code that works with HSV easier to read, so they do not change functionality. Regards, Hans > > Thanks, > Dafna > >> >>> +#define V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION 0x00000002 >>> /** >>> * struct v4l2_subdev_mbus_code_enum - Media bus format enumeration >>> * @pad: pad number, as reported by the media API >>> @@ -77,7 +79,8 @@ struct v4l2_subdev_mbus_code_enum { >>> __u32 index; >>> __u32 code; >>> __u32 which; >>> - __u32 reserved[8]; >>> + __u32 flags; >>> + __u32 reserved[7]; >>> }; >>> >>> /** >>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h >>> index 9817b7e2c968..adc9dd1080b8 100644 >>> --- a/include/uapi/linux/videodev2.h >>> +++ b/include/uapi/linux/videodev2.h >>> @@ -772,6 +772,7 @@ struct v4l2_pix_format { >>> >>> /* Flags */ >>> #define V4L2_PIX_FMT_FLAG_PREMUL_ALPHA 0x00000001 >>> +#define V4L2_PIX_FMT_FLAG_HAS_CSC 0x00000002 >>> >>> /* >>> * F O R M A T E N U M E R A T I O N >>> @@ -789,6 +790,9 @@ struct v4l2_fmtdesc { >>> #define V4L2_FMT_FLAG_EMULATED 0x0002 >>> #define V4L2_FMT_FLAG_CONTINUOUS_BYTESTREAM 0x0004 >>> #define V4L2_FMT_FLAG_DYN_RESOLUTION 0x0008 >>> +#define V4L2_FMT_FLAG_CSC_YCBCR_ENC 0x0010 >>> +#define V4L2_FMT_FLAG_CSC_HSV_ENC 0x0010 >>> +#define V4L2_FMT_FLAG_CSC_QUANTIZATION 0x0020 >>> >>> /* Frame Size and frame rate enumeration */ >>> /* >>> >> >> I think this is quite good, it's mainly doc improvements and renaming HAS_CSC to REQUEST_CSC. >> >> That said, I do want to see this supported in at least vivid (vimc would be nice >> too!). And v4l2-ctl needs a patch to support this. >> >> A compliance test for v4l2-compliance would be nice too, but it is not a prerequisite. >> I'm not entirely sure what would be the best way to test this. Perhaps a vivid-specific >> test might be easiest (i.e., don't test it for all drivers, but just for vivid). >> >> Regards, >> >> Hans >>
On 25.05.20 16:25, Hans Verkuil wrote: > On 25/05/2020 15:56, Dafna Hirschfeld wrote: >> Hi >> >> On 07.05.20 15:00, Hans Verkuil wrote: >>> On 16/04/2020 16:56, Dafna Hirschfeld wrote: >>>> From: Philipp Zabel <p.zabel@pengutronix.de> >>>> >>>> For video capture it is the driver that reports the colorspace, >>>> Y'CbCr/HSV encoding, quantization range and transfer function >>>> used by the video, and there is no way to request something >>>> different, even though many HDTV receivers have some sort of >>>> colorspace conversion capabilities. >>>> >>>> For output video this feature already exists since the application >>>> specifies this information for the video format it will send out, and >>>> the transmitter will enable any available CSC if a format conversion has >>>> to be performed in order to match the capabilities of the sink. >>>> >>>> For video capture we propose adding new pix_format flag: >>>> V4L2_PIX_FMT_FLAG_HAS_CSC. The flag is set by the application, >>> >>> That should be V4L2_PIX_FMT_FLAG_REQUEST_CSC, since the application actively >>> requests the use of the CSC. >> Maybe change to V4L2_PIX_FMT_FLAG_ASK_FOR_CSC so people won't think >> it is related to the request API ? > > How about _SET_CSC? I think that's even better than REQUEST_CSC. > >>> >>>> the driver will interpret the ycbcr_enc/hsv_enc, and quantization fields >>>> as the requested colorspace information and will attempt to >>>> do the conversion it supports. >>>> >>>> Drivers set the flags >>>> V4L2_FMT_FLAG_CSC_YCBCR_ENC, >>>> V4L2_FMT_FLAG_CSC_HSV_ENC, >>>> V4L2_FMT_FLAG_CSC_HSV_QUANTIZATION, >>> >>> That last define should be 'V4L2_FMT_FLAG_CSC_QUANTIZATION' without 'HSV_'. >>> >>>> in the flags field of the struct v4l2_fmtdesc during enumeration to >>>> indicate that they support colorspace conversion for the respective field. >>>> Currently the conversion of the fields colorspace, xfer_func is not >>>> supported since there are no drivers that support it. >>>> >>>> The same API is added for the subdevices. With the flag V4L2_MBUS_FRAMEFMT_HAS_CSC >>> >>> Should also be REQUEST_CSC. >>> >>>> set by the application in the VIDIOC_SUBDEV_S_FMT ioctl and the flags >>>> V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC, V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION >>>> set by the driver in the VIDIOC_SUBDEV_ENUM_MBUS_CODE ioctl. >>>> >>>> For subdevices, new 'flags' fields were added to the structs >>>> v4l2_subdev_mbus_code_enum, v4l2_mbus_framefmt which are borrowed from the 'reserved' field >>>> >>>> Drivers do not have to actually look at the flags: if the flags are not >>>> set, then the colorspace, ycbcr_enc and quantization fields are set to >>>> the default values by the core, i.e. just pass on the received format >>>> without conversion. >>>> >>>> Signed-off-by: Hans Verkuil <Hans Verkuil@cisco.com> >>>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> >>>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> >>>> --- >>>> This is v3 of the RFC suggested originaly by Hans Verkuil: >>>> >>>> https://patchwork.linuxtv.org/patch/28847/ >>>> >>>> And then a v2 from Philipp Zabel: >>>> >>>> https://patchwork.kernel.org/project/linux-media/list/?series=15483 >>>> >>>> changes in v3: >>>> I added the API to subdevices as well and added fixes according to >>>> comments from Hans. >>>> I also added a usecase for the new API for the rkisp1 driver. >>>> >>>> changes in v2 (reported by Philipp Zabel): >>>> - convert to rst >>>> - split V4L2_PIX_FMT_FLAG_REQUEST_CSC into four separate flags for >>>> colorspace, ycbcr_enc/hsv_enc, quantization, and xfer_func >>>> - let driver set flags to indicate supported features >>>> >>>> [1] https://patchwork.linuxtv.org/patch/28847/ >>>> >>>> .../media/v4l/pixfmt-reserved.rst | 6 +++ >>>> .../media/v4l/pixfmt-v4l2-mplane.rst | 16 ++----- >>>> .../userspace-api/media/v4l/pixfmt-v4l2.rst | 36 +++++++++++++--- >>>> .../media/v4l/subdev-formats.rst | 42 +++++++++++++++++-- >>>> .../media/v4l/vidioc-enum-fmt.rst | 18 ++++++++ >>>> .../v4l/vidioc-subdev-enum-mbus-code.rst | 23 ++++++++++ >>>> drivers/media/v4l2-core/v4l2-ioctl.c | 19 ++++++++- >>>> drivers/media/v4l2-core/v4l2-subdev.c | 7 ++++ >>>> include/uapi/linux/v4l2-mediabus.h | 5 ++- >>>> include/uapi/linux/v4l2-subdev.h | 5 ++- >>>> include/uapi/linux/videodev2.h | 4 ++ >>>> 11 files changed, 158 insertions(+), 23 deletions(-) >>>> >>>> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst >>>> index 59b9e7238f90..fa8dada69f8c 100644 >>>> --- a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst >>>> +++ b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst >>> >>> Why on earth did the documentation of these pixel format flags end up in >>> pixfmt-reserved.rst? >>> >>> Can you make a patch that moves this to pixfmt-v4l2.rst? >>> >>>> @@ -280,3 +280,9 @@ please make a proposal on the linux-media mailing list. >>>> by RGBA values (128, 192, 255, 128), the same pixel described with >>>> premultiplied colors would be described by RGBA values (64, 96, >>>> 128, 128) >>>> + * - ``V4L2_PIX_FMT_FLAG_HAS_CSC`` >>>> + - 0x00000002 >>>> + - Set by the application. It is only used for capture and is >>>> + ignored for output streams. If set, then request the driver to do >>>> + colorspace conversion from the received colorspace, only conversions >>>> + of Ycbcr encoding, HSV encoding and quantization are supported. >>> >>> I'd replace the part "If set..." with this: >>> >>> ---------- >>> If set, then request the driver to do colorspace conversion from the received >>> colorspace to the requested colorspace values. If a colorimetry field >>> (``colorspace``, ``xfer_func``, ``ycncr_enc`` or ``quantization``) is set to 0, >>> then that colorimetry setting will remain unchanged from what was received. >>> So to just change the quantization only the ``quantization`` field shall be set >>> to a non-zero value (``V4L2_QUANTIZATION_FULL_RANGE`` or >>> ``V4L2_QUANTIZATION_LIM_RANGE``) and all other colorimetry fields shall be set to 0. >>> >>> To check which conversions are supported by the hardware for the current pixel >>> format, see :ref:`fmtdesc-flags`. >>> ---------- >>> >>> >>>> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst >>>> index 444b4082684c..66f3365d7b72 100644 >>>> --- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst >>>> +++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst >>>> @@ -105,29 +105,21 @@ describing all planes of that format. >>>> * - __u8 >>>> - ``ycbcr_enc`` >>>> - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`. >>>> - This information supplements the ``colorspace`` and must be set by >>>> - the driver for capture streams and by the application for output >>>> - streams, see :ref:`colorspaces`. >>>> + See struct :c:type:`v4l2_pix_format`. >>>> * - __u8 >>>> - ``hsv_enc`` >>>> - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`. >>>> - This information supplements the ``colorspace`` and must be set by >>>> - the driver for capture streams and by the application for output >>>> - streams, see :ref:`colorspaces`. >>>> + See struct :c:type:`v4l2_pix_format`. >>>> * - } >>>> - >>>> * - __u8 >>>> - ``quantization`` >>>> - Quantization range, from enum :c:type:`v4l2_quantization`. >>>> - This information supplements the ``colorspace`` and must be set by >>>> - the driver for capture streams and by the application for output >>>> - streams, see :ref:`colorspaces`. >>>> + See struct :c:type:`v4l2_pix_format`. >>>> * - __u8 >>>> - ``xfer_func`` >>>> - Transfer function, from enum :c:type:`v4l2_xfer_func`. >>>> - This information supplements the ``colorspace`` and must be set by >>>> - the driver for capture streams and by the application for output >>>> - streams, see :ref:`colorspaces`. >>>> + See struct :c:type:`v4l2_pix_format`. >>>> * - __u8 >>>> - ``reserved[7]`` >>>> - Reserved for future extensions. Should be zeroed by drivers and >>>> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst >>>> index 759420a872d6..ce57718cd66b 100644 >>>> --- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst >>>> +++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst >>>> @@ -110,8 +110,8 @@ Single-planar format structure >>>> - ``colorspace`` >>>> - Image colorspace, from enum :c:type:`v4l2_colorspace`. >>>> This information supplements the ``pixelformat`` and must be set >>>> - by the driver for capture streams and by the application for >>>> - output streams, see :ref:`colorspaces`. >>>> + by the driver for capture streams and by the application for output >>>> + streams, see :ref:`colorspace`. >>> >>> This doesn't appear to change anything. >>> >>>> * - __u32 >>>> - ``priv`` >>>> - This field indicates whether the remaining fields of the >>>> @@ -148,13 +148,31 @@ Single-planar format structure >>>> - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`. >>>> This information supplements the ``colorspace`` and must be set by >>>> the driver for capture streams and by the application for output >>>> - streams, see :ref:`colorspaces`. >>>> + streams, see :ref:`colorspaces`. If the application sets the >>>> + flag ``V4L2_PIX_FMT_FLAG_HAS_CSC`` then the application can set >>>> + this field for a capture stream to request a specific Y'CbCr encoding >>>> + for the captured image data. The driver will attempt to do the >>>> + conversion to the specified Y'CbCr encoding or return the encoding it >>>> + will use if it can't do the conversion. This field is ignored for >>>> + non-Y'CbCr pixelformats. The driver indicates that ycbcr_enc conversion >>>> + is supported by setting the flag V4L2_FMT_FLAG_CSC_YCBCR_ENC on the >>>> + on the corresponding struct :c:type:`v4l2_fmtdesc` during enumeration. >>> >>> on the on the -> in the >>> >>>> + See :ref:`fmtdesc-flags` >>>> * - __u32 >>>> - ``hsv_enc`` >>>> - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`. >>>> This information supplements the ``colorspace`` and must be set by >>>> the driver for capture streams and by the application for output >>>> - streams, see :ref:`colorspaces`. >>>> + streams, see :ref:`colorspaces`. If the application sets the flag >>>> + ``V4L2_PIX_FMT_FLAG_HAS_CSC`` then the application can set this >>>> + field for a capture stream to request a specific HSV encoding for the >>>> + captured image data. The driver will attempt to do the conversion to >>>> + the specified HSV encoding or return the encoding it will use if it >>>> + can't do the conversion. This field is ignored for non-HSV >>>> + pixelformats. The driver indicates that hsv_enc conversion >>>> + is supported by setting the flag V4L2_FMT_FLAG_CSC_HSV_ENC on the >>>> + on the corresponding struct :c:type:`v4l2_fmtdesc` during enumeration. >>> >>> Ditto. >>> >>>> + See :ref:`fmtdesc-flags` >>>> * - } >>>> - >>>> * - __u32 >>>> @@ -162,7 +180,15 @@ Single-planar format structure >>>> - Quantization range, from enum :c:type:`v4l2_quantization`. >>>> This information supplements the ``colorspace`` and must be set by >>>> the driver for capture streams and by the application for output >>>> - streams, see :ref:`colorspaces`. >>>> + streams, see :ref:`colorspaces`. If the application sets the flag >>>> + ``V4L2_PIX_FMT_FLAG_HAS_CSC`` then the application can set >>>> + this field for a capture stream to request a specific quantization >>>> + range for the captured image data. The driver will attempt to do the >>>> + conversion to the specified quantization range or return the >>>> + quantization it will use if it can't do the conversion. The driver >>>> + indicates that quantization conversion is supported by setting the flag >>>> + V4L2_FMT_FLAG_CSC_QUANTIZATION on the on the corresponding struct >>> >>> Ditto >>> >>>> + :c:type:`v4l2_fmtdesc` during enumeration. See :ref:`fmtdesc-flags` >>>> * - __u32 >>>> - ``xfer_func`` >>>> - Transfer function, from enum :c:type:`v4l2_xfer_func`. >>>> diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst >>>> index 9a4d61b0d76f..f1d4ca29a3e8 100644 >>>> --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst >>>> +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst >>>> @@ -49,13 +49,33 @@ Media Bus Formats >>>> - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`. >>>> This information supplements the ``colorspace`` and must be set by >>>> the driver for capture streams and by the application for output >>>> - streams, see :ref:`colorspaces`. >>>> + streams, see :ref:`colorspaces`. If the application sets the >>>> + flag ``V4L2_MBUS_FRAMEFMT_HAS_CSC`` the the application can set >>>> + this field for a capture stream to request a specific Y'CbCr encoding >>>> + for the mbus data. The driver will attempt to do the >>> >>> mbus -> media bus >>> >>>> + conversion to the specified Y'CbCr encoding or return the encoding it >>>> + will use if it can't do the conversion. This field is ignored for >>>> + non-Y'CbCr pixelformats. The driver indicates that ycbcr_enc conversion >>> >>> This is a media bus format, not a pixelformat. >>> >>>> + is supported by setting the flag V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC on >>> >>> on -> in >>> >>>> + the corresponding struct c:type:`v4l2_subdev_mbus_code_enum` during >>>> + enumeration. See :ref:`vidioc-subdev-enum-mbus-code.rst <v4l2-subdev-mbus-code-flags>` >>>> + >>>> * - __u16 >>>> - ``quantization`` >>>> - Quantization range, from enum :c:type:`v4l2_quantization`. >>>> This information supplements the ``colorspace`` and must be set by >>>> the driver for capture streams and by the application for output >>>> - streams, see :ref:`colorspaces`. >>>> + streams, see :ref:`colorspaces`. If the application sets the >>>> + flag ``V4L2_MBUS_FRAMEFMT_HAS_CSC`` the the application can set >>>> + this field for a capture stream to request a specific quantization >>>> + encoding for the mbus data. The driver will attempt to do the >>> >>> mbus -> media bus >>> >>>> + conversion to the specified quantization or return the quantization it >>>> + will use if it can't do the conversion. The driver indicates that >>>> + quantization conversion is supported by setting the flag >>>> + V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION on the corresponding struct >>> >>> on -> in >>> >>>> + c:type:`v4l2_subdev_mbus_code_enum` during enumeration. >>>> + See :ref:`vidioc-subdev-enum-mbus-code.rst <v4l2-subdev-mbus-code-flags>` >>>> + >>>> * - __u16 >>>> - ``xfer_func`` >>>> - Transfer function, from enum :c:type:`v4l2_xfer_func`. >>>> @@ -63,10 +83,26 @@ Media Bus Formats >>>> the driver for capture streams and by the application for output >>>> streams, see :ref:`colorspaces`. >>>> * - __u16 >>>> - - ``reserved``\ [11] >>>> + - ``flags`` >>>> + - flags see: :ref:v4l2-mbus-framefmt-flags >>> >>> flags see: -> See: >>> >>>> + * - __u16 >>>> + - ``reserved``\ [10] >>>> - Reserved for future extensions. Applications and drivers must set >>>> the array to zero. >>>> >>>> +.. _v4l2-mbus-framefmt-flags: >>>> + >>>> +.. flat-table:: v4l2_mbus_framefmt Flags >>>> + :header-rows: 0 >>>> + :stub-columns: 0 >>>> + :widths: 3 1 4 >>>> + >>>> + * - ``V4L2_MBUS_FRAMEFMT_HAS_CSC`` >>>> + - 0x0001 >>>> + - Set by the application. It is only used for capture and is >>>> + ignored for output streams. If set, then request the subdevice to do >>>> + colorspace conversion from the received colorspace, only conversions >>>> + of Ycbcr encoding, and quantization are supported. >>> >>> Replace by a similar text as for V4L2_PIX_FMT_FLAG_HAS_CSC. >>> >>>> >>>> >>>> .. _v4l2-mbus-pixelcode: >>>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst >>>> index 7e3142e11d77..125f074543af 100644 >>>> --- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst >>>> +++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst >>>> @@ -145,6 +145,24 @@ formats in preference order, where preferred formats are returned before >>>> parameters are detected. This flag can only be used in combination >>>> with the ``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to >>>> compressed formats only. It is also only applies to stateful codecs. >>>> + * - ``V4L2_FMT_FLAG_CSC_YCBCR_ENC`` >>>> + - 0x0010 >>>> + - The driver allows the application to try to change the default >>>> + ycbcr encoding. This flag is relevant only for capture devices. >>>> + The application can ask to configure the ycbcr_enc of the capture device >>>> + when calling the :c:func:`VIDIOC_S_FMT` ioctl. >>> >>> add: ...ioctl with ``V4L2_FMT_FLAG_REQUEST_CSC`` set. (this should probably be >>> a reference to the REQUEST_CSC flag documentation). >>> >>>> + * - ``V4L2_FMT_FLAG_CSC_HSV_ENC`` >>>> + - 0x0010 >>>> + - The driver allows the application to try to change the default >>>> + hsv encoding. This flag is relevant only for capture devices. >>>> + The application can ask to configure the hsv_enc of the capture device >>>> + when calling the :c:func:`VIDIOC_S_FMT` ioctl. >>> >>> Ditto. Also: hsv -> HSV. >>> >>>> + * - ``V4L2_FMT_FLAG_CSC_QUANTIZATION`` >>>> + - 0x0020 >>>> + - The driver allows the application to try to change the default >>>> + quantization. This flag is relevant only for capture devices. >>>> + The application can ask to configure the quantization of the capture device >>>> + when calling the :c:func:`VIDIOC_S_FMT` ioctl. >>> >>> Ditto. >>> >>>> >>>> >>>> Return Value >>>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst >>>> index 35b8607203a4..4ad87cb74f57 100644 >>>> --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst >>>> +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst >>>> @@ -78,11 +78,34 @@ information about the try formats. >>>> - ``which`` >>>> - Media bus format codes to be enumerated, from enum >>>> :ref:`v4l2_subdev_format_whence <v4l2-subdev-format-whence>`. >>>> + * - __u32 >>>> + - ``flags`` >>>> + - see :ref:`v4l2-subdev-mbus-code-flags` >>> >>> see -> See >>> >>>> * - __u32 >>>> - ``reserved``\ [8] >>>> - Reserved for future extensions. Applications and drivers must set >>>> the array to zero. >>>> >>>> +.. _v4l2-subdev-mbus-code-flags: >>>> + >>>> + >>>> +.. tabularcolumns:: |p{4.4cm}|p{4.4cm}|p{7.7cm}| >>>> + >>>> +.. flat-table:: flags in struct v4l2_subdev_mbus_code_enum >>>> + :header-rows: 0 >>>> + :stub-columns: 0 >>>> + :widths: 1 1 2 >>>> + >>>> + * - V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC >>>> + - 0x00000001 >>>> + - The driver supports setting the ycbcr encoding by the application >>>> + when calling the VIDIOC_SUBDEV_S_FMT ioctl. see :ref:`v4l2-mbus-format` >>> >>> see -> See >>> >>>> + on how to do this. >>>> + * - V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION >>>> + - 0x00000002 >>>> + - The driver supports setting the quantization by the application >>>> + when calling the VIDIOC_SUBDEV_S_FMT ioctl. see :ref:`v4l2-mbus-format` >>> >>> see -> See >>> >>>> + on how to do this. >>>> >>>> Return Value >>>> ============ >>>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c >>>> index b2ef8e60ea7d..3c7ffb6b15cb 100644 >>>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c >>>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c >>>> @@ -1029,6 +1029,15 @@ static void v4l_sanitize_format(struct v4l2_format *fmt) >>>> fmt->fmt.pix_mp.num_planes = min_t(u32, fmt->fmt.pix_mp.num_planes, >>>> VIDEO_MAX_PLANES); >>>> >>>> + if (fmt->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) { >>>> + if (fmt->fmt.pix_mp.flags & V4L2_PIX_FMT_FLAG_HAS_CSC) >>>> + return; >>>> + fmt->fmt.pix_mp.colorspace = V4L2_COLORSPACE_DEFAULT; >>>> + fmt->fmt.pix_mp.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; >>>> + fmt->fmt.pix_mp.quantization = V4L2_QUANTIZATION_DEFAULT; >>>> + fmt->fmt.pix_mp.xfer_func = V4L2_XFER_FUNC_DEFAULT; >>>> + } >>>> + >>>> /* >>>> * The v4l2_pix_format structure has been extended with fields that were >>>> * not previously required to be set to zero by applications. The priv >>>> @@ -1043,8 +1052,16 @@ static void v4l_sanitize_format(struct v4l2_format *fmt) >>>> fmt->type != V4L2_BUF_TYPE_VIDEO_OUTPUT) >>>> return; >>>> >>>> - if (fmt->fmt.pix.priv == V4L2_PIX_FMT_PRIV_MAGIC) >>>> + if (fmt->fmt.pix.priv == V4L2_PIX_FMT_PRIV_MAGIC) { >>>> + if (fmt->type != V4L2_BUF_TYPE_VIDEO_CAPTURE || >>>> + fmt->fmt.pix.flags & V4L2_PIX_FMT_FLAG_HAS_CSC) >>>> + return; >>>> + fmt->fmt.pix.colorspace = V4L2_COLORSPACE_DEFAULT; >>>> + fmt->fmt.pix.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; >>>> + fmt->fmt.pix.quantization = V4L2_QUANTIZATION_DEFAULT; >>>> + fmt->fmt.pix.xfer_func = V4L2_XFER_FUNC_DEFAULT; >>>> return; >>>> + } >>>> >>>> fmt->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC; >>>> >>>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c >>>> index a376b351135f..51e009936aad 100644 >>>> --- a/drivers/media/v4l2-core/v4l2-subdev.c >>>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c >>>> @@ -477,6 +477,13 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) >>>> case VIDIOC_SUBDEV_S_FMT: { >>>> struct v4l2_subdev_format *format = arg; >>>> >>>> + if (!(format->format.flags & V4L2_MBUS_FRAMEFMT_HAS_CSC)) { >>>> + format->format.colorspace = V4L2_COLORSPACE_DEFAULT; >>>> + format->format.xfer_func = V4L2_XFER_FUNC_DEFAULT; >>>> + format->format.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; >>>> + format->format.quantization = V4L2_QUANTIZATION_DEFAULT; >>>> + } I wonder if those 3 code chunks added to v4l2-ioctl.c and v4l2-subdev.c are actually needed. They set the colorspace fields on capture devices to 0 if the HAS_CSC flag is not set. But I don't see the reason to do it, if the capture driver do not support CSC it will just ignore the values of those fields that came from userspace which is what is already done. Also, it could be that a driver does not support CSC and userspace set the SET_CSC flag anyway, in this case those fields will not be set to defalut although they should. What do you think? Thanks, Dafna >>>> + >>>> memset(format->reserved, 0, sizeof(format->reserved)); >>>> memset(format->format.reserved, 0, sizeof(format->format.reserved)); >>>> return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh->pad, format); >>>> diff --git a/include/uapi/linux/v4l2-mediabus.h b/include/uapi/linux/v4l2-mediabus.h >>>> index 123a231001a8..89ff0ad6a631 100644 >>>> --- a/include/uapi/linux/v4l2-mediabus.h >>>> +++ b/include/uapi/linux/v4l2-mediabus.h >>>> @@ -16,6 +16,8 @@ >>>> #include <linux/types.h> >>>> #include <linux/videodev2.h> >>>> >>>> +#define V4L2_MBUS_FRAMEFMT_HAS_CSC 0x0001 >>>> + >>>> /** >>>> * struct v4l2_mbus_framefmt - frame format on the media bus >>>> * @width: image width >>>> @@ -36,7 +38,8 @@ struct v4l2_mbus_framefmt { >>>> __u16 ycbcr_enc; >>>> __u16 quantization; >>>> __u16 xfer_func; >>>> - __u16 reserved[11]; >>>> + __u16 flags; >>>> + __u16 reserved[10]; >>>> }; >>>> >>>> #ifndef __KERNEL__ >>>> diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h >>>> index 03970ce30741..4410b26a7158 100644 >>>> --- a/include/uapi/linux/v4l2-subdev.h >>>> +++ b/include/uapi/linux/v4l2-subdev.h >>>> @@ -65,6 +65,8 @@ struct v4l2_subdev_crop { >>>> __u32 reserved[8]; >>>> }; >>>> >>>> +#define V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC 0x00000001 >>> >>> I think it is better to add an HSV_ENC alias here as well. >>> >>> It's for readability only, since talking about YCBCR_ENC when the mediabus format >>> is a HSV format is just weird. >> >> I see that in 'v4l2_pix_format' the {ycbcr_enc, hsv_enc} are a union >> but in 'v4l2_mbus_framefmt' there is only the field ycbcr_enc. >> I wonder why is that, since I see there is one HSV media bus format >> V4L2_MBUS_FROM_MEDIA_BUS_FMT(AHSV8888_1X32). >> >> So I guess this is why I didn't add a V4L2_SUBDEV_MBUS_CODE_CSC_HSV_ENC define. >> I also didn't add this flag to the docs. Should I? > > I think this can be done in a separate (final) patch where a union is added for > v4l2_mbus_framefmt together with a V4L2_SUBDEV_MBUS_CODE_CSC_HSV_ENC define. > > These are effectively aliases to make code that works with HSV easier to read, > so they do not change functionality. > > Regards, > > Hans > >> >> Thanks, >> Dafna >> >>> >>>> +#define V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION 0x00000002 >>>> /** >>>> * struct v4l2_subdev_mbus_code_enum - Media bus format enumeration >>>> * @pad: pad number, as reported by the media API >>>> @@ -77,7 +79,8 @@ struct v4l2_subdev_mbus_code_enum { >>>> __u32 index; >>>> __u32 code; >>>> __u32 which; >>>> - __u32 reserved[8]; >>>> + __u32 flags; >>>> + __u32 reserved[7]; >>>> }; >>>> >>>> /** >>>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h >>>> index 9817b7e2c968..adc9dd1080b8 100644 >>>> --- a/include/uapi/linux/videodev2.h >>>> +++ b/include/uapi/linux/videodev2.h >>>> @@ -772,6 +772,7 @@ struct v4l2_pix_format { >>>> >>>> /* Flags */ >>>> #define V4L2_PIX_FMT_FLAG_PREMUL_ALPHA 0x00000001 >>>> +#define V4L2_PIX_FMT_FLAG_HAS_CSC 0x00000002 >>>> >>>> /* >>>> * F O R M A T E N U M E R A T I O N >>>> @@ -789,6 +790,9 @@ struct v4l2_fmtdesc { >>>> #define V4L2_FMT_FLAG_EMULATED 0x0002 >>>> #define V4L2_FMT_FLAG_CONTINUOUS_BYTESTREAM 0x0004 >>>> #define V4L2_FMT_FLAG_DYN_RESOLUTION 0x0008 >>>> +#define V4L2_FMT_FLAG_CSC_YCBCR_ENC 0x0010 >>>> +#define V4L2_FMT_FLAG_CSC_HSV_ENC 0x0010 >>>> +#define V4L2_FMT_FLAG_CSC_QUANTIZATION 0x0020 >>>> >>>> /* Frame Size and frame rate enumeration */ >>>> /* >>>> >>> >>> I think this is quite good, it's mainly doc improvements and renaming HAS_CSC to REQUEST_CSC. >>> >>> That said, I do want to see this supported in at least vivid (vimc would be nice >>> too!). And v4l2-ctl needs a patch to support this. >>> >>> A compliance test for v4l2-compliance would be nice too, but it is not a prerequisite. >>> I'm not entirely sure what would be the best way to test this. Perhaps a vivid-specific >>> test might be easiest (i.e., don't test it for all drivers, but just for vivid). >>> >>> Regards, >>> >>> Hans >>> >
On 26/05/2020 14:36, Dafna Hirschfeld wrote: > > > On 25.05.20 16:25, Hans Verkuil wrote: >> On 25/05/2020 15:56, Dafna Hirschfeld wrote: >>> Hi >>> >>> On 07.05.20 15:00, Hans Verkuil wrote: >>>> On 16/04/2020 16:56, Dafna Hirschfeld wrote: >>>>> From: Philipp Zabel <p.zabel@pengutronix.de> >>>>> >>>>> For video capture it is the driver that reports the colorspace, >>>>> Y'CbCr/HSV encoding, quantization range and transfer function >>>>> used by the video, and there is no way to request something >>>>> different, even though many HDTV receivers have some sort of >>>>> colorspace conversion capabilities. >>>>> >>>>> For output video this feature already exists since the application >>>>> specifies this information for the video format it will send out, and >>>>> the transmitter will enable any available CSC if a format conversion has >>>>> to be performed in order to match the capabilities of the sink. >>>>> >>>>> For video capture we propose adding new pix_format flag: >>>>> V4L2_PIX_FMT_FLAG_HAS_CSC. The flag is set by the application, >>>> >>>> That should be V4L2_PIX_FMT_FLAG_REQUEST_CSC, since the application actively >>>> requests the use of the CSC. >>> Maybe change to V4L2_PIX_FMT_FLAG_ASK_FOR_CSC so people won't think >>> it is related to the request API ? >> >> How about _SET_CSC? I think that's even better than REQUEST_CSC. >> >>>> >>>>> the driver will interpret the ycbcr_enc/hsv_enc, and quantization fields >>>>> as the requested colorspace information and will attempt to >>>>> do the conversion it supports. >>>>> >>>>> Drivers set the flags >>>>> V4L2_FMT_FLAG_CSC_YCBCR_ENC, >>>>> V4L2_FMT_FLAG_CSC_HSV_ENC, >>>>> V4L2_FMT_FLAG_CSC_HSV_QUANTIZATION, >>>> >>>> That last define should be 'V4L2_FMT_FLAG_CSC_QUANTIZATION' without 'HSV_'. >>>> >>>>> in the flags field of the struct v4l2_fmtdesc during enumeration to >>>>> indicate that they support colorspace conversion for the respective field. >>>>> Currently the conversion of the fields colorspace, xfer_func is not >>>>> supported since there are no drivers that support it. >>>>> >>>>> The same API is added for the subdevices. With the flag V4L2_MBUS_FRAMEFMT_HAS_CSC >>>> >>>> Should also be REQUEST_CSC. >>>> >>>>> set by the application in the VIDIOC_SUBDEV_S_FMT ioctl and the flags >>>>> V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC, V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION >>>>> set by the driver in the VIDIOC_SUBDEV_ENUM_MBUS_CODE ioctl. >>>>> >>>>> For subdevices, new 'flags' fields were added to the structs >>>>> v4l2_subdev_mbus_code_enum, v4l2_mbus_framefmt which are borrowed from the 'reserved' field >>>>> >>>>> Drivers do not have to actually look at the flags: if the flags are not >>>>> set, then the colorspace, ycbcr_enc and quantization fields are set to >>>>> the default values by the core, i.e. just pass on the received format >>>>> without conversion. >>>>> >>>>> Signed-off-by: Hans Verkuil <Hans Verkuil@cisco.com> >>>>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> >>>>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> >>>>> --- >>>>> This is v3 of the RFC suggested originaly by Hans Verkuil: >>>>> >>>>> https://patchwork.linuxtv.org/patch/28847/ >>>>> >>>>> And then a v2 from Philipp Zabel: >>>>> >>>>> https://patchwork.kernel.org/project/linux-media/list/?series=15483 >>>>> >>>>> changes in v3: >>>>> I added the API to subdevices as well and added fixes according to >>>>> comments from Hans. >>>>> I also added a usecase for the new API for the rkisp1 driver. >>>>> >>>>> changes in v2 (reported by Philipp Zabel): >>>>> - convert to rst >>>>> - split V4L2_PIX_FMT_FLAG_REQUEST_CSC into four separate flags for >>>>> colorspace, ycbcr_enc/hsv_enc, quantization, and xfer_func >>>>> - let driver set flags to indicate supported features >>>>> >>>>> [1] https://patchwork.linuxtv.org/patch/28847/ >>>>> >>>>> .../media/v4l/pixfmt-reserved.rst | 6 +++ >>>>> .../media/v4l/pixfmt-v4l2-mplane.rst | 16 ++----- >>>>> .../userspace-api/media/v4l/pixfmt-v4l2.rst | 36 +++++++++++++--- >>>>> .../media/v4l/subdev-formats.rst | 42 +++++++++++++++++-- >>>>> .../media/v4l/vidioc-enum-fmt.rst | 18 ++++++++ >>>>> .../v4l/vidioc-subdev-enum-mbus-code.rst | 23 ++++++++++ >>>>> drivers/media/v4l2-core/v4l2-ioctl.c | 19 ++++++++- >>>>> drivers/media/v4l2-core/v4l2-subdev.c | 7 ++++ >>>>> include/uapi/linux/v4l2-mediabus.h | 5 ++- >>>>> include/uapi/linux/v4l2-subdev.h | 5 ++- >>>>> include/uapi/linux/videodev2.h | 4 ++ >>>>> 11 files changed, 158 insertions(+), 23 deletions(-) >>>>> >>>>> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst >>>>> index 59b9e7238f90..fa8dada69f8c 100644 >>>>> --- a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst >>>>> +++ b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst >>>> >>>> Why on earth did the documentation of these pixel format flags end up in >>>> pixfmt-reserved.rst? >>>> >>>> Can you make a patch that moves this to pixfmt-v4l2.rst? >>>> >>>>> @@ -280,3 +280,9 @@ please make a proposal on the linux-media mailing list. >>>>> by RGBA values (128, 192, 255, 128), the same pixel described with >>>>> premultiplied colors would be described by RGBA values (64, 96, >>>>> 128, 128) >>>>> + * - ``V4L2_PIX_FMT_FLAG_HAS_CSC`` >>>>> + - 0x00000002 >>>>> + - Set by the application. It is only used for capture and is >>>>> + ignored for output streams. If set, then request the driver to do >>>>> + colorspace conversion from the received colorspace, only conversions >>>>> + of Ycbcr encoding, HSV encoding and quantization are supported. >>>> >>>> I'd replace the part "If set..." with this: >>>> >>>> ---------- >>>> If set, then request the driver to do colorspace conversion from the received >>>> colorspace to the requested colorspace values. If a colorimetry field >>>> (``colorspace``, ``xfer_func``, ``ycncr_enc`` or ``quantization``) is set to 0, >>>> then that colorimetry setting will remain unchanged from what was received. >>>> So to just change the quantization only the ``quantization`` field shall be set >>>> to a non-zero value (``V4L2_QUANTIZATION_FULL_RANGE`` or >>>> ``V4L2_QUANTIZATION_LIM_RANGE``) and all other colorimetry fields shall be set to 0. >>>> >>>> To check which conversions are supported by the hardware for the current pixel >>>> format, see :ref:`fmtdesc-flags`. >>>> ---------- >>>> >>>> >>>>> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst >>>>> index 444b4082684c..66f3365d7b72 100644 >>>>> --- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst >>>>> +++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst >>>>> @@ -105,29 +105,21 @@ describing all planes of that format. >>>>> * - __u8 >>>>> - ``ycbcr_enc`` >>>>> - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`. >>>>> - This information supplements the ``colorspace`` and must be set by >>>>> - the driver for capture streams and by the application for output >>>>> - streams, see :ref:`colorspaces`. >>>>> + See struct :c:type:`v4l2_pix_format`. >>>>> * - __u8 >>>>> - ``hsv_enc`` >>>>> - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`. >>>>> - This information supplements the ``colorspace`` and must be set by >>>>> - the driver for capture streams and by the application for output >>>>> - streams, see :ref:`colorspaces`. >>>>> + See struct :c:type:`v4l2_pix_format`. >>>>> * - } >>>>> - >>>>> * - __u8 >>>>> - ``quantization`` >>>>> - Quantization range, from enum :c:type:`v4l2_quantization`. >>>>> - This information supplements the ``colorspace`` and must be set by >>>>> - the driver for capture streams and by the application for output >>>>> - streams, see :ref:`colorspaces`. >>>>> + See struct :c:type:`v4l2_pix_format`. >>>>> * - __u8 >>>>> - ``xfer_func`` >>>>> - Transfer function, from enum :c:type:`v4l2_xfer_func`. >>>>> - This information supplements the ``colorspace`` and must be set by >>>>> - the driver for capture streams and by the application for output >>>>> - streams, see :ref:`colorspaces`. >>>>> + See struct :c:type:`v4l2_pix_format`. >>>>> * - __u8 >>>>> - ``reserved[7]`` >>>>> - Reserved for future extensions. Should be zeroed by drivers and >>>>> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst >>>>> index 759420a872d6..ce57718cd66b 100644 >>>>> --- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst >>>>> +++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst >>>>> @@ -110,8 +110,8 @@ Single-planar format structure >>>>> - ``colorspace`` >>>>> - Image colorspace, from enum :c:type:`v4l2_colorspace`. >>>>> This information supplements the ``pixelformat`` and must be set >>>>> - by the driver for capture streams and by the application for >>>>> - output streams, see :ref:`colorspaces`. >>>>> + by the driver for capture streams and by the application for output >>>>> + streams, see :ref:`colorspace`. >>>> >>>> This doesn't appear to change anything. >>>> >>>>> * - __u32 >>>>> - ``priv`` >>>>> - This field indicates whether the remaining fields of the >>>>> @@ -148,13 +148,31 @@ Single-planar format structure >>>>> - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`. >>>>> This information supplements the ``colorspace`` and must be set by >>>>> the driver for capture streams and by the application for output >>>>> - streams, see :ref:`colorspaces`. >>>>> + streams, see :ref:`colorspaces`. If the application sets the >>>>> + flag ``V4L2_PIX_FMT_FLAG_HAS_CSC`` then the application can set >>>>> + this field for a capture stream to request a specific Y'CbCr encoding >>>>> + for the captured image data. The driver will attempt to do the >>>>> + conversion to the specified Y'CbCr encoding or return the encoding it >>>>> + will use if it can't do the conversion. This field is ignored for >>>>> + non-Y'CbCr pixelformats. The driver indicates that ycbcr_enc conversion >>>>> + is supported by setting the flag V4L2_FMT_FLAG_CSC_YCBCR_ENC on the >>>>> + on the corresponding struct :c:type:`v4l2_fmtdesc` during enumeration. >>>> >>>> on the on the -> in the >>>> >>>>> + See :ref:`fmtdesc-flags` >>>>> * - __u32 >>>>> - ``hsv_enc`` >>>>> - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`. >>>>> This information supplements the ``colorspace`` and must be set by >>>>> the driver for capture streams and by the application for output >>>>> - streams, see :ref:`colorspaces`. >>>>> + streams, see :ref:`colorspaces`. If the application sets the flag >>>>> + ``V4L2_PIX_FMT_FLAG_HAS_CSC`` then the application can set this >>>>> + field for a capture stream to request a specific HSV encoding for the >>>>> + captured image data. The driver will attempt to do the conversion to >>>>> + the specified HSV encoding or return the encoding it will use if it >>>>> + can't do the conversion. This field is ignored for non-HSV >>>>> + pixelformats. The driver indicates that hsv_enc conversion >>>>> + is supported by setting the flag V4L2_FMT_FLAG_CSC_HSV_ENC on the >>>>> + on the corresponding struct :c:type:`v4l2_fmtdesc` during enumeration. >>>> >>>> Ditto. >>>> >>>>> + See :ref:`fmtdesc-flags` >>>>> * - } >>>>> - >>>>> * - __u32 >>>>> @@ -162,7 +180,15 @@ Single-planar format structure >>>>> - Quantization range, from enum :c:type:`v4l2_quantization`. >>>>> This information supplements the ``colorspace`` and must be set by >>>>> the driver for capture streams and by the application for output >>>>> - streams, see :ref:`colorspaces`. >>>>> + streams, see :ref:`colorspaces`. If the application sets the flag >>>>> + ``V4L2_PIX_FMT_FLAG_HAS_CSC`` then the application can set >>>>> + this field for a capture stream to request a specific quantization >>>>> + range for the captured image data. The driver will attempt to do the >>>>> + conversion to the specified quantization range or return the >>>>> + quantization it will use if it can't do the conversion. The driver >>>>> + indicates that quantization conversion is supported by setting the flag >>>>> + V4L2_FMT_FLAG_CSC_QUANTIZATION on the on the corresponding struct >>>> >>>> Ditto >>>> >>>>> + :c:type:`v4l2_fmtdesc` during enumeration. See :ref:`fmtdesc-flags` >>>>> * - __u32 >>>>> - ``xfer_func`` >>>>> - Transfer function, from enum :c:type:`v4l2_xfer_func`. >>>>> diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst >>>>> index 9a4d61b0d76f..f1d4ca29a3e8 100644 >>>>> --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst >>>>> +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst >>>>> @@ -49,13 +49,33 @@ Media Bus Formats >>>>> - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`. >>>>> This information supplements the ``colorspace`` and must be set by >>>>> the driver for capture streams and by the application for output >>>>> - streams, see :ref:`colorspaces`. >>>>> + streams, see :ref:`colorspaces`. If the application sets the >>>>> + flag ``V4L2_MBUS_FRAMEFMT_HAS_CSC`` the the application can set >>>>> + this field for a capture stream to request a specific Y'CbCr encoding >>>>> + for the mbus data. The driver will attempt to do the >>>> >>>> mbus -> media bus >>>> >>>>> + conversion to the specified Y'CbCr encoding or return the encoding it >>>>> + will use if it can't do the conversion. This field is ignored for >>>>> + non-Y'CbCr pixelformats. The driver indicates that ycbcr_enc conversion >>>> >>>> This is a media bus format, not a pixelformat. >>>> >>>>> + is supported by setting the flag V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC on >>>> >>>> on -> in >>>> >>>>> + the corresponding struct c:type:`v4l2_subdev_mbus_code_enum` during >>>>> + enumeration. See :ref:`vidioc-subdev-enum-mbus-code.rst <v4l2-subdev-mbus-code-flags>` >>>>> + >>>>> * - __u16 >>>>> - ``quantization`` >>>>> - Quantization range, from enum :c:type:`v4l2_quantization`. >>>>> This information supplements the ``colorspace`` and must be set by >>>>> the driver for capture streams and by the application for output >>>>> - streams, see :ref:`colorspaces`. >>>>> + streams, see :ref:`colorspaces`. If the application sets the >>>>> + flag ``V4L2_MBUS_FRAMEFMT_HAS_CSC`` the the application can set >>>>> + this field for a capture stream to request a specific quantization >>>>> + encoding for the mbus data. The driver will attempt to do the >>>> >>>> mbus -> media bus >>>> >>>>> + conversion to the specified quantization or return the quantization it >>>>> + will use if it can't do the conversion. The driver indicates that >>>>> + quantization conversion is supported by setting the flag >>>>> + V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION on the corresponding struct >>>> >>>> on -> in >>>> >>>>> + c:type:`v4l2_subdev_mbus_code_enum` during enumeration. >>>>> + See :ref:`vidioc-subdev-enum-mbus-code.rst <v4l2-subdev-mbus-code-flags>` >>>>> + >>>>> * - __u16 >>>>> - ``xfer_func`` >>>>> - Transfer function, from enum :c:type:`v4l2_xfer_func`. >>>>> @@ -63,10 +83,26 @@ Media Bus Formats >>>>> the driver for capture streams and by the application for output >>>>> streams, see :ref:`colorspaces`. >>>>> * - __u16 >>>>> - - ``reserved``\ [11] >>>>> + - ``flags`` >>>>> + - flags see: :ref:v4l2-mbus-framefmt-flags >>>> >>>> flags see: -> See: >>>> >>>>> + * - __u16 >>>>> + - ``reserved``\ [10] >>>>> - Reserved for future extensions. Applications and drivers must set >>>>> the array to zero. >>>>> >>>>> +.. _v4l2-mbus-framefmt-flags: >>>>> + >>>>> +.. flat-table:: v4l2_mbus_framefmt Flags >>>>> + :header-rows: 0 >>>>> + :stub-columns: 0 >>>>> + :widths: 3 1 4 >>>>> + >>>>> + * - ``V4L2_MBUS_FRAMEFMT_HAS_CSC`` >>>>> + - 0x0001 >>>>> + - Set by the application. It is only used for capture and is >>>>> + ignored for output streams. If set, then request the subdevice to do >>>>> + colorspace conversion from the received colorspace, only conversions >>>>> + of Ycbcr encoding, and quantization are supported. >>>> >>>> Replace by a similar text as for V4L2_PIX_FMT_FLAG_HAS_CSC. >>>> >>>>> >>>>> >>>>> .. _v4l2-mbus-pixelcode: >>>>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst >>>>> index 7e3142e11d77..125f074543af 100644 >>>>> --- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst >>>>> +++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst >>>>> @@ -145,6 +145,24 @@ formats in preference order, where preferred formats are returned before >>>>> parameters are detected. This flag can only be used in combination >>>>> with the ``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to >>>>> compressed formats only. It is also only applies to stateful codecs. >>>>> + * - ``V4L2_FMT_FLAG_CSC_YCBCR_ENC`` >>>>> + - 0x0010 >>>>> + - The driver allows the application to try to change the default >>>>> + ycbcr encoding. This flag is relevant only for capture devices. >>>>> + The application can ask to configure the ycbcr_enc of the capture device >>>>> + when calling the :c:func:`VIDIOC_S_FMT` ioctl. >>>> >>>> add: ...ioctl with ``V4L2_FMT_FLAG_REQUEST_CSC`` set. (this should probably be >>>> a reference to the REQUEST_CSC flag documentation). >>>> >>>>> + * - ``V4L2_FMT_FLAG_CSC_HSV_ENC`` >>>>> + - 0x0010 >>>>> + - The driver allows the application to try to change the default >>>>> + hsv encoding. This flag is relevant only for capture devices. >>>>> + The application can ask to configure the hsv_enc of the capture device >>>>> + when calling the :c:func:`VIDIOC_S_FMT` ioctl. >>>> >>>> Ditto. Also: hsv -> HSV. >>>> >>>>> + * - ``V4L2_FMT_FLAG_CSC_QUANTIZATION`` >>>>> + - 0x0020 >>>>> + - The driver allows the application to try to change the default >>>>> + quantization. This flag is relevant only for capture devices. >>>>> + The application can ask to configure the quantization of the capture device >>>>> + when calling the :c:func:`VIDIOC_S_FMT` ioctl. >>>> >>>> Ditto. >>>> >>>>> >>>>> >>>>> Return Value >>>>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst >>>>> index 35b8607203a4..4ad87cb74f57 100644 >>>>> --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst >>>>> +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst >>>>> @@ -78,11 +78,34 @@ information about the try formats. >>>>> - ``which`` >>>>> - Media bus format codes to be enumerated, from enum >>>>> :ref:`v4l2_subdev_format_whence <v4l2-subdev-format-whence>`. >>>>> + * - __u32 >>>>> + - ``flags`` >>>>> + - see :ref:`v4l2-subdev-mbus-code-flags` >>>> >>>> see -> See >>>> >>>>> * - __u32 >>>>> - ``reserved``\ [8] >>>>> - Reserved for future extensions. Applications and drivers must set >>>>> the array to zero. >>>>> >>>>> +.. _v4l2-subdev-mbus-code-flags: >>>>> + >>>>> + >>>>> +.. tabularcolumns:: |p{4.4cm}|p{4.4cm}|p{7.7cm}| >>>>> + >>>>> +.. flat-table:: flags in struct v4l2_subdev_mbus_code_enum >>>>> + :header-rows: 0 >>>>> + :stub-columns: 0 >>>>> + :widths: 1 1 2 >>>>> + >>>>> + * - V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC >>>>> + - 0x00000001 >>>>> + - The driver supports setting the ycbcr encoding by the application >>>>> + when calling the VIDIOC_SUBDEV_S_FMT ioctl. see :ref:`v4l2-mbus-format` >>>> >>>> see -> See >>>> >>>>> + on how to do this. >>>>> + * - V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION >>>>> + - 0x00000002 >>>>> + - The driver supports setting the quantization by the application >>>>> + when calling the VIDIOC_SUBDEV_S_FMT ioctl. see :ref:`v4l2-mbus-format` >>>> >>>> see -> See >>>> >>>>> + on how to do this. >>>>> >>>>> Return Value >>>>> ============ >>>>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c >>>>> index b2ef8e60ea7d..3c7ffb6b15cb 100644 >>>>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c >>>>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c >>>>> @@ -1029,6 +1029,15 @@ static void v4l_sanitize_format(struct v4l2_format *fmt) >>>>> fmt->fmt.pix_mp.num_planes = min_t(u32, fmt->fmt.pix_mp.num_planes, >>>>> VIDEO_MAX_PLANES); >>>>> >>>>> + if (fmt->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) { >>>>> + if (fmt->fmt.pix_mp.flags & V4L2_PIX_FMT_FLAG_HAS_CSC) >>>>> + return; >>>>> + fmt->fmt.pix_mp.colorspace = V4L2_COLORSPACE_DEFAULT; >>>>> + fmt->fmt.pix_mp.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; >>>>> + fmt->fmt.pix_mp.quantization = V4L2_QUANTIZATION_DEFAULT; >>>>> + fmt->fmt.pix_mp.xfer_func = V4L2_XFER_FUNC_DEFAULT; >>>>> + } >>>>> + >>>>> /* >>>>> * The v4l2_pix_format structure has been extended with fields that were >>>>> * not previously required to be set to zero by applications. The priv >>>>> @@ -1043,8 +1052,16 @@ static void v4l_sanitize_format(struct v4l2_format *fmt) >>>>> fmt->type != V4L2_BUF_TYPE_VIDEO_OUTPUT) >>>>> return; >>>>> >>>>> - if (fmt->fmt.pix.priv == V4L2_PIX_FMT_PRIV_MAGIC) >>>>> + if (fmt->fmt.pix.priv == V4L2_PIX_FMT_PRIV_MAGIC) { >>>>> + if (fmt->type != V4L2_BUF_TYPE_VIDEO_CAPTURE || >>>>> + fmt->fmt.pix.flags & V4L2_PIX_FMT_FLAG_HAS_CSC) >>>>> + return; >>>>> + fmt->fmt.pix.colorspace = V4L2_COLORSPACE_DEFAULT; >>>>> + fmt->fmt.pix.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; >>>>> + fmt->fmt.pix.quantization = V4L2_QUANTIZATION_DEFAULT; >>>>> + fmt->fmt.pix.xfer_func = V4L2_XFER_FUNC_DEFAULT; >>>>> return; >>>>> + } >>>>> >>>>> fmt->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC; >>>>> >>>>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c >>>>> index a376b351135f..51e009936aad 100644 >>>>> --- a/drivers/media/v4l2-core/v4l2-subdev.c >>>>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c >>>>> @@ -477,6 +477,13 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) >>>>> case VIDIOC_SUBDEV_S_FMT: { >>>>> struct v4l2_subdev_format *format = arg; >>>>> >>>>> + if (!(format->format.flags & V4L2_MBUS_FRAMEFMT_HAS_CSC)) { >>>>> + format->format.colorspace = V4L2_COLORSPACE_DEFAULT; >>>>> + format->format.xfer_func = V4L2_XFER_FUNC_DEFAULT; >>>>> + format->format.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; >>>>> + format->format.quantization = V4L2_QUANTIZATION_DEFAULT; >>>>> + } > I wonder if those 3 code chunks added to v4l2-ioctl.c and v4l2-subdev.c are actually > needed. They set the colorspace fields on capture devices to 0 if the HAS_CSC flag is not set. > But I don't see the reason to do it, if the capture driver do not support CSC it will just ignore > the values of those fields that came from userspace which is what is already done. > Also, it could be that a driver does not support CSC and userspace set the SET_CSC flag anyway, in this > case those fields will not be set to defalut although they should. > What do you think? I agree. That code doesn't make sense. I think it is safe to drop this. Regards, Hans > > Thanks, > Dafna > >>>>> + >>>>> memset(format->reserved, 0, sizeof(format->reserved)); >>>>> memset(format->format.reserved, 0, sizeof(format->format.reserved)); >>>>> return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh->pad, format); >>>>> diff --git a/include/uapi/linux/v4l2-mediabus.h b/include/uapi/linux/v4l2-mediabus.h >>>>> index 123a231001a8..89ff0ad6a631 100644 >>>>> --- a/include/uapi/linux/v4l2-mediabus.h >>>>> +++ b/include/uapi/linux/v4l2-mediabus.h >>>>> @@ -16,6 +16,8 @@ >>>>> #include <linux/types.h> >>>>> #include <linux/videodev2.h> >>>>> >>>>> +#define V4L2_MBUS_FRAMEFMT_HAS_CSC 0x0001 >>>>> + >>>>> /** >>>>> * struct v4l2_mbus_framefmt - frame format on the media bus >>>>> * @width: image width >>>>> @@ -36,7 +38,8 @@ struct v4l2_mbus_framefmt { >>>>> __u16 ycbcr_enc; >>>>> __u16 quantization; >>>>> __u16 xfer_func; >>>>> - __u16 reserved[11]; >>>>> + __u16 flags; >>>>> + __u16 reserved[10]; >>>>> }; >>>>> >>>>> #ifndef __KERNEL__ >>>>> diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h >>>>> index 03970ce30741..4410b26a7158 100644 >>>>> --- a/include/uapi/linux/v4l2-subdev.h >>>>> +++ b/include/uapi/linux/v4l2-subdev.h >>>>> @@ -65,6 +65,8 @@ struct v4l2_subdev_crop { >>>>> __u32 reserved[8]; >>>>> }; >>>>> >>>>> +#define V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC 0x00000001 >>>> >>>> I think it is better to add an HSV_ENC alias here as well. >>>> >>>> It's for readability only, since talking about YCBCR_ENC when the mediabus format >>>> is a HSV format is just weird. >>> >>> I see that in 'v4l2_pix_format' the {ycbcr_enc, hsv_enc} are a union >>> but in 'v4l2_mbus_framefmt' there is only the field ycbcr_enc. >>> I wonder why is that, since I see there is one HSV media bus format >>> V4L2_MBUS_FROM_MEDIA_BUS_FMT(AHSV8888_1X32). >>> >>> So I guess this is why I didn't add a V4L2_SUBDEV_MBUS_CODE_CSC_HSV_ENC define. >>> I also didn't add this flag to the docs. Should I? >> >> I think this can be done in a separate (final) patch where a union is added for >> v4l2_mbus_framefmt together with a V4L2_SUBDEV_MBUS_CODE_CSC_HSV_ENC define. >> >> These are effectively aliases to make code that works with HSV easier to read, >> so they do not change functionality. >> >> Regards, >> >> Hans >> >>> >>> Thanks, >>> Dafna >>> >>>> >>>>> +#define V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION 0x00000002 >>>>> /** >>>>> * struct v4l2_subdev_mbus_code_enum - Media bus format enumeration >>>>> * @pad: pad number, as reported by the media API >>>>> @@ -77,7 +79,8 @@ struct v4l2_subdev_mbus_code_enum { >>>>> __u32 index; >>>>> __u32 code; >>>>> __u32 which; >>>>> - __u32 reserved[8]; >>>>> + __u32 flags; >>>>> + __u32 reserved[7]; >>>>> }; >>>>> >>>>> /** >>>>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h >>>>> index 9817b7e2c968..adc9dd1080b8 100644 >>>>> --- a/include/uapi/linux/videodev2.h >>>>> +++ b/include/uapi/linux/videodev2.h >>>>> @@ -772,6 +772,7 @@ struct v4l2_pix_format { >>>>> >>>>> /* Flags */ >>>>> #define V4L2_PIX_FMT_FLAG_PREMUL_ALPHA 0x00000001 >>>>> +#define V4L2_PIX_FMT_FLAG_HAS_CSC 0x00000002 >>>>> >>>>> /* >>>>> * F O R M A T E N U M E R A T I O N >>>>> @@ -789,6 +790,9 @@ struct v4l2_fmtdesc { >>>>> #define V4L2_FMT_FLAG_EMULATED 0x0002 >>>>> #define V4L2_FMT_FLAG_CONTINUOUS_BYTESTREAM 0x0004 >>>>> #define V4L2_FMT_FLAG_DYN_RESOLUTION 0x0008 >>>>> +#define V4L2_FMT_FLAG_CSC_YCBCR_ENC 0x0010 >>>>> +#define V4L2_FMT_FLAG_CSC_HSV_ENC 0x0010 >>>>> +#define V4L2_FMT_FLAG_CSC_QUANTIZATION 0x0020 >>>>> >>>>> /* Frame Size and frame rate enumeration */ >>>>> /* >>>>> >>>> >>>> I think this is quite good, it's mainly doc improvements and renaming HAS_CSC to REQUEST_CSC. >>>> >>>> That said, I do want to see this supported in at least vivid (vimc would be nice >>>> too!). And v4l2-ctl needs a patch to support this. >>>> >>>> A compliance test for v4l2-compliance would be nice too, but it is not a prerequisite. >>>> I'm not entirely sure what would be the best way to test this. Perhaps a vivid-specific >>>> test might be easiest (i.e., don't test it for all drivers, but just for vivid). >>>> >>>> Regards, >>>> >>>> Hans >>>> >>
Hi Dafna, On Thu, Apr 16, 2020 at 04:56:04PM +0200, Dafna Hirschfeld wrote: > From: Philipp Zabel <p.zabel@pengutronix.de> > > For video capture it is the driver that reports the colorspace, > Y'CbCr/HSV encoding, quantization range and transfer function > used by the video, and there is no way to request something > different, even though many HDTV receivers have some sort of > colorspace conversion capabilities. > Thanks for working on this! Please see my comments inline. > For output video this feature already exists since the application > specifies this information for the video format it will send out, and > the transmitter will enable any available CSC if a format conversion has > to be performed in order to match the capabilities of the sink. > > For video capture we propose adding new pix_format flag: > V4L2_PIX_FMT_FLAG_HAS_CSC. The flag is set by the application, > the driver will interpret the ycbcr_enc/hsv_enc, and quantization fields > as the requested colorspace information and will attempt to > do the conversion it supports. > > Drivers set the flags > V4L2_FMT_FLAG_CSC_YCBCR_ENC, > V4L2_FMT_FLAG_CSC_HSV_ENC, > V4L2_FMT_FLAG_CSC_HSV_QUANTIZATION, Do we need this level of granularity? The drivers would ignore unsupported encoding/quantization settings and reset them to supported ones anyway, so if one doesn't support changing given parameter, it would just return back the original stream parameter. > in the flags field of the struct v4l2_fmtdesc during enumeration to > indicate that they support colorspace conversion for the respective field. > Currently the conversion of the fields colorspace, xfer_func is not > supported since there are no drivers that support it. > > The same API is added for the subdevices. With the flag V4L2_MBUS_FRAMEFMT_HAS_CSC > set by the application in the VIDIOC_SUBDEV_S_FMT ioctl and the flags > V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC, V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION > set by the driver in the VIDIOC_SUBDEV_ENUM_MBUS_CODE ioctl. > > For subdevices, new 'flags' fields were added to the structs > v4l2_subdev_mbus_code_enum, v4l2_mbus_framefmt which are borrowed from the 'reserved' field > > Drivers do not have to actually look at the flags: if the flags are not > set, then the colorspace, ycbcr_enc and quantization fields are set to > the default values by the core, i.e. just pass on the received format > without conversion. > > Signed-off-by: Hans Verkuil <Hans Verkuil@cisco.com> Something wrong with the email address here. > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> > Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> > --- > This is v3 of the RFC suggested originaly by Hans Verkuil: > > https://patchwork.linuxtv.org/patch/28847/ > > And then a v2 from Philipp Zabel: > > https://patchwork.kernel.org/project/linux-media/list/?series=15483 > > changes in v3: > I added the API to subdevices as well and added fixes according to > comments from Hans. > I also added a usecase for the new API for the rkisp1 driver. > > changes in v2 (reported by Philipp Zabel): > - convert to rst > - split V4L2_PIX_FMT_FLAG_REQUEST_CSC into four separate flags for > colorspace, ycbcr_enc/hsv_enc, quantization, and xfer_func > - let driver set flags to indicate supported features > > [1] https://patchwork.linuxtv.org/patch/28847/ > > .../media/v4l/pixfmt-reserved.rst | 6 +++ > .../media/v4l/pixfmt-v4l2-mplane.rst | 16 ++----- > .../userspace-api/media/v4l/pixfmt-v4l2.rst | 36 +++++++++++++--- > .../media/v4l/subdev-formats.rst | 42 +++++++++++++++++-- > .../media/v4l/vidioc-enum-fmt.rst | 18 ++++++++ > .../v4l/vidioc-subdev-enum-mbus-code.rst | 23 ++++++++++ > drivers/media/v4l2-core/v4l2-ioctl.c | 19 ++++++++- > drivers/media/v4l2-core/v4l2-subdev.c | 7 ++++ > include/uapi/linux/v4l2-mediabus.h | 5 ++- > include/uapi/linux/v4l2-subdev.h | 5 ++- > include/uapi/linux/videodev2.h | 4 ++ > 11 files changed, 158 insertions(+), 23 deletions(-) > > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst > index 59b9e7238f90..fa8dada69f8c 100644 > --- a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst > +++ b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst > @@ -280,3 +280,9 @@ please make a proposal on the linux-media mailing list. > by RGBA values (128, 192, 255, 128), the same pixel described with > premultiplied colors would be described by RGBA values (64, 96, > 128, 128) > + * - ``V4L2_PIX_FMT_FLAG_HAS_CSC`` > + - 0x00000002 > + - Set by the application. It is only used for capture and is > + ignored for output streams. If set, then request the driver to do > + colorspace conversion from the received colorspace, only conversions Should this be: colorspace. Only conversions ? > + of Ycbcr encoding, HSV encoding and quantization are supported. YCbCr? > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst > index 444b4082684c..66f3365d7b72 100644 > --- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst > +++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst > @@ -105,29 +105,21 @@ describing all planes of that format. > * - __u8 > - ``ycbcr_enc`` > - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`. > - This information supplements the ``colorspace`` and must be set by > - the driver for capture streams and by the application for output > - streams, see :ref:`colorspaces`. > + See struct :c:type:`v4l2_pix_format`. > * - __u8 > - ``hsv_enc`` > - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`. > - This information supplements the ``colorspace`` and must be set by > - the driver for capture streams and by the application for output > - streams, see :ref:`colorspaces`. > + See struct :c:type:`v4l2_pix_format`. > * - } > - > * - __u8 > - ``quantization`` > - Quantization range, from enum :c:type:`v4l2_quantization`. > - This information supplements the ``colorspace`` and must be set by > - the driver for capture streams and by the application for output > - streams, see :ref:`colorspaces`. > + See struct :c:type:`v4l2_pix_format`. > * - __u8 > - ``xfer_func`` > - Transfer function, from enum :c:type:`v4l2_xfer_func`. > - This information supplements the ``colorspace`` and must be set by > - the driver for capture streams and by the application for output > - streams, see :ref:`colorspaces`. > + See struct :c:type:`v4l2_pix_format`. > * - __u8 > - ``reserved[7]`` > - Reserved for future extensions. Should be zeroed by drivers and > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst > index 759420a872d6..ce57718cd66b 100644 > --- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst > +++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst > @@ -110,8 +110,8 @@ Single-planar format structure > - ``colorspace`` > - Image colorspace, from enum :c:type:`v4l2_colorspace`. > This information supplements the ``pixelformat`` and must be set > - by the driver for capture streams and by the application for > - output streams, see :ref:`colorspaces`. > + by the driver for capture streams and by the application for output > + streams, see :ref:`colorspace`. Is it expected that the colorspace itself can't be converted? Also is the change of the reference name expected? > * - __u32 > - ``priv`` > - This field indicates whether the remaining fields of the > @@ -148,13 +148,31 @@ Single-planar format structure > - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`. > This information supplements the ``colorspace`` and must be set by > the driver for capture streams and by the application for output > - streams, see :ref:`colorspaces`. > + streams, see :ref:`colorspaces`. If the application sets the > + flag ``V4L2_PIX_FMT_FLAG_HAS_CSC`` then the application can set > + this field for a capture stream to request a specific Y'CbCr encoding > + for the captured image data. The driver will attempt to do the > + conversion to the specified Y'CbCr encoding or return the encoding it > + will use if it can't do the conversion. This field is ignored for nit: The driver will attempt to do the conversion when the streaming starts, so that's not an entirely correct description. How about "[...] captured image data. If the driver cannot handle requested conversion, it will return another, supported encoding." ? > + non-Y'CbCr pixelformats. The driver indicates that ycbcr_enc conversion > + is supported by setting the flag V4L2_FMT_FLAG_CSC_YCBCR_ENC on the > + on the corresponding struct :c:type:`v4l2_fmtdesc` during enumeration. > + See :ref:`fmtdesc-flags` > * - __u32 > - ``hsv_enc`` > - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`. > This information supplements the ``colorspace`` and must be set by > the driver for capture streams and by the application for output > - streams, see :ref:`colorspaces`. > + streams, see :ref:`colorspaces`. If the application sets the flag > + ``V4L2_PIX_FMT_FLAG_HAS_CSC`` then the application can set this > + field for a capture stream to request a specific HSV encoding for the > + captured image data. The driver will attempt to do the conversion to > + the specified HSV encoding or return the encoding it will use if it > + can't do the conversion. This field is ignored for non-HSV Ditto. > + pixelformats. The driver indicates that hsv_enc conversion > + is supported by setting the flag V4L2_FMT_FLAG_CSC_HSV_ENC on the > + on the corresponding struct :c:type:`v4l2_fmtdesc` during enumeration. > + See :ref:`fmtdesc-flags` > * - } > - > * - __u32 > @@ -162,7 +180,15 @@ Single-planar format structure > - Quantization range, from enum :c:type:`v4l2_quantization`. > This information supplements the ``colorspace`` and must be set by > the driver for capture streams and by the application for output > - streams, see :ref:`colorspaces`. > + streams, see :ref:`colorspaces`. If the application sets the flag > + ``V4L2_PIX_FMT_FLAG_HAS_CSC`` then the application can set > + this field for a capture stream to request a specific quantization > + range for the captured image data. The driver will attempt to do the > + conversion to the specified quantization range or return the > + quantization it will use if it can't do the conversion. The driver Ditto. > + indicates that quantization conversion is supported by setting the flag > + V4L2_FMT_FLAG_CSC_QUANTIZATION on the on the corresponding struct > + :c:type:`v4l2_fmtdesc` during enumeration. See :ref:`fmtdesc-flags` > * - __u32 > - ``xfer_func`` > - Transfer function, from enum :c:type:`v4l2_xfer_func`. > diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst > index 9a4d61b0d76f..f1d4ca29a3e8 100644 > --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst > +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst > @@ -49,13 +49,33 @@ Media Bus Formats > - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`. > This information supplements the ``colorspace`` and must be set by > the driver for capture streams and by the application for output > - streams, see :ref:`colorspaces`. > + streams, see :ref:`colorspaces`. If the application sets the > + flag ``V4L2_MBUS_FRAMEFMT_HAS_CSC`` the the application can set > + this field for a capture stream to request a specific Y'CbCr encoding > + for the mbus data. The driver will attempt to do the > + conversion to the specified Y'CbCr encoding or return the encoding it > + will use if it can't do the conversion. This field is ignored for Ditto. > + non-Y'CbCr pixelformats. The driver indicates that ycbcr_enc conversion > + is supported by setting the flag V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC on > + the corresponding struct c:type:`v4l2_subdev_mbus_code_enum` during > + enumeration. See :ref:`vidioc-subdev-enum-mbus-code.rst <v4l2-subdev-mbus-code-flags>` > + > * - __u16 > - ``quantization`` > - Quantization range, from enum :c:type:`v4l2_quantization`. > This information supplements the ``colorspace`` and must be set by > the driver for capture streams and by the application for output > - streams, see :ref:`colorspaces`. > + streams, see :ref:`colorspaces`. If the application sets the > + flag ``V4L2_MBUS_FRAMEFMT_HAS_CSC`` the the application can set > + this field for a capture stream to request a specific quantization > + encoding for the mbus data. The driver will attempt to do the > + conversion to the specified quantization or return the quantization it > + will use if it can't do the conversion. The driver indicates that Ditto. > + quantization conversion is supported by setting the flag > + V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION on the corresponding struct > + c:type:`v4l2_subdev_mbus_code_enum` during enumeration. > + See :ref:`vidioc-subdev-enum-mbus-code.rst <v4l2-subdev-mbus-code-flags>` > + > * - __u16 > - ``xfer_func`` > - Transfer function, from enum :c:type:`v4l2_xfer_func`. > @@ -63,10 +83,26 @@ Media Bus Formats > the driver for capture streams and by the application for output > streams, see :ref:`colorspaces`. > * - __u16 > - - ``reserved``\ [11] > + - ``flags`` > + - flags see: :ref:v4l2-mbus-framefmt-flags > + * - __u16 > + - ``reserved``\ [10] > - Reserved for future extensions. Applications and drivers must set > the array to zero. > > +.. _v4l2-mbus-framefmt-flags: > + > +.. flat-table:: v4l2_mbus_framefmt Flags > + :header-rows: 0 > + :stub-columns: 0 > + :widths: 3 1 4 > + > + * - ``V4L2_MBUS_FRAMEFMT_HAS_CSC`` > + - 0x0001 > + - Set by the application. It is only used for capture and is > + ignored for output streams. If set, then request the subdevice to do > + colorspace conversion from the received colorspace, only conversions > + of Ycbcr encoding, and quantization are supported. YCbCr? > > > .. _v4l2-mbus-pixelcode: > diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst > index 7e3142e11d77..125f074543af 100644 > --- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst > +++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst > @@ -145,6 +145,24 @@ formats in preference order, where preferred formats are returned before > parameters are detected. This flag can only be used in combination > with the ``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to > compressed formats only. It is also only applies to stateful codecs. > + * - ``V4L2_FMT_FLAG_CSC_YCBCR_ENC`` > + - 0x0010 > + - The driver allows the application to try to change the default > + ycbcr encoding. This flag is relevant only for capture devices. YCbCr > + The application can ask to configure the ycbcr_enc of the capture device > + when calling the :c:func:`VIDIOC_S_FMT` ioctl. > + * - ``V4L2_FMT_FLAG_CSC_HSV_ENC`` > + - 0x0010 > + - The driver allows the application to try to change the default > + hsv encoding. This flag is relevant only for capture devices. > + The application can ask to configure the hsv_enc of the capture device > + when calling the :c:func:`VIDIOC_S_FMT` ioctl. > + * - ``V4L2_FMT_FLAG_CSC_QUANTIZATION`` > + - 0x0020 > + - The driver allows the application to try to change the default > + quantization. This flag is relevant only for capture devices. > + The application can ask to configure the quantization of the capture device > + when calling the :c:func:`VIDIOC_S_FMT` ioctl. > > > Return Value > diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst > index 35b8607203a4..4ad87cb74f57 100644 > --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst > +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst > @@ -78,11 +78,34 @@ information about the try formats. > - ``which`` > - Media bus format codes to be enumerated, from enum > :ref:`v4l2_subdev_format_whence <v4l2-subdev-format-whence>`. > + * - __u32 > + - ``flags`` > + - see :ref:`v4l2-subdev-mbus-code-flags` > * - __u32 > - ``reserved``\ [8] > - Reserved for future extensions. Applications and drivers must set > the array to zero. > > +.. _v4l2-subdev-mbus-code-flags: > + > + > +.. tabularcolumns:: |p{4.4cm}|p{4.4cm}|p{7.7cm}| > + > +.. flat-table:: flags in struct v4l2_subdev_mbus_code_enum > + :header-rows: 0 > + :stub-columns: 0 > + :widths: 1 1 2 > + > + * - V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC > + - 0x00000001 > + - The driver supports setting the ycbcr encoding by the application > + when calling the VIDIOC_SUBDEV_S_FMT ioctl. see :ref:`v4l2-mbus-format` > + on how to do this. > + * - V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION > + - 0x00000002 > + - The driver supports setting the quantization by the application > + when calling the VIDIOC_SUBDEV_S_FMT ioctl. see :ref:`v4l2-mbus-format` > + on how to do this. > > Return Value > ============ > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c > index b2ef8e60ea7d..3c7ffb6b15cb 100644 > --- a/drivers/media/v4l2-core/v4l2-ioctl.c > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c > @@ -1029,6 +1029,15 @@ static void v4l_sanitize_format(struct v4l2_format *fmt) > fmt->fmt.pix_mp.num_planes = min_t(u32, fmt->fmt.pix_mp.num_planes, > VIDEO_MAX_PLANES); > > + if (fmt->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) { > + if (fmt->fmt.pix_mp.flags & V4L2_PIX_FMT_FLAG_HAS_CSC) > + return; This return statement might end up being confusing in the future, because one might add further fixups below. Perhaps instead, the inner condition negated could be added to the outer one? > + fmt->fmt.pix_mp.colorspace = V4L2_COLORSPACE_DEFAULT; > + fmt->fmt.pix_mp.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; > + fmt->fmt.pix_mp.quantization = V4L2_QUANTIZATION_DEFAULT; > + fmt->fmt.pix_mp.xfer_func = V4L2_XFER_FUNC_DEFAULT; > + } > + > /* > * The v4l2_pix_format structure has been extended with fields that were > * not previously required to be set to zero by applications. The priv > @@ -1043,8 +1052,16 @@ static void v4l_sanitize_format(struct v4l2_format *fmt) > fmt->type != V4L2_BUF_TYPE_VIDEO_OUTPUT) > return; > > - if (fmt->fmt.pix.priv == V4L2_PIX_FMT_PRIV_MAGIC) > + if (fmt->fmt.pix.priv == V4L2_PIX_FMT_PRIV_MAGIC) { > + if (fmt->type != V4L2_BUF_TYPE_VIDEO_CAPTURE || > + fmt->fmt.pix.flags & V4L2_PIX_FMT_FLAG_HAS_CSC) > + return; > + fmt->fmt.pix.colorspace = V4L2_COLORSPACE_DEFAULT; > + fmt->fmt.pix.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; > + fmt->fmt.pix.quantization = V4L2_QUANTIZATION_DEFAULT; > + fmt->fmt.pix.xfer_func = V4L2_XFER_FUNC_DEFAULT; > return; > + } > > fmt->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC; > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > index a376b351135f..51e009936aad 100644 > --- a/drivers/media/v4l2-core/v4l2-subdev.c > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > @@ -477,6 +477,13 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) > case VIDIOC_SUBDEV_S_FMT: { > struct v4l2_subdev_format *format = arg; > > + if (!(format->format.flags & V4L2_MBUS_FRAMEFMT_HAS_CSC)) { > + format->format.colorspace = V4L2_COLORSPACE_DEFAULT; > + format->format.xfer_func = V4L2_XFER_FUNC_DEFAULT; > + format->format.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; > + format->format.quantization = V4L2_QUANTIZATION_DEFAULT; > + } Wouldn't this break setting the colorspaces on the sink pads, for which the flag isn't required? Actually there is some unfortunate statement in the documentation related to this: "This information supplements the colorspace and must be set by the driver for capture streams and by the application for output streams, see Colorspaces." However, I don't think there is any notion of "capture" and "output" for subdevices, right? Instead, the pad direction would have to be checked, but AFAICT there is no access to this kind of information from this wrapper. > + > memset(format->reserved, 0, sizeof(format->reserved)); > memset(format->format.reserved, 0, sizeof(format->format.reserved)); > return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh->pad, format); > diff --git a/include/uapi/linux/v4l2-mediabus.h b/include/uapi/linux/v4l2-mediabus.h > index 123a231001a8..89ff0ad6a631 100644 > --- a/include/uapi/linux/v4l2-mediabus.h > +++ b/include/uapi/linux/v4l2-mediabus.h > @@ -16,6 +16,8 @@ > #include <linux/types.h> > #include <linux/videodev2.h> > > +#define V4L2_MBUS_FRAMEFMT_HAS_CSC 0x0001 > + > /** > * struct v4l2_mbus_framefmt - frame format on the media bus > * @width: image width > @@ -36,7 +38,8 @@ struct v4l2_mbus_framefmt { > __u16 ycbcr_enc; > __u16 quantization; > __u16 xfer_func; > - __u16 reserved[11]; > + __u16 flags; Are we okay with a u16 for flags? Best regards, Tomasz
Hi, On 04.06.20 19:39, Tomasz Figa wrote: > Hi Dafna, > > On Thu, Apr 16, 2020 at 04:56:04PM +0200, Dafna Hirschfeld wrote: >> From: Philipp Zabel <p.zabel@pengutronix.de> >> >> For video capture it is the driver that reports the colorspace, >> Y'CbCr/HSV encoding, quantization range and transfer function >> used by the video, and there is no way to request something >> different, even though many HDTV receivers have some sort of >> colorspace conversion capabilities. >> > > Thanks for working on this! Please see my comments inline. > >> For output video this feature already exists since the application >> specifies this information for the video format it will send out, and >> the transmitter will enable any available CSC if a format conversion has >> to be performed in order to match the capabilities of the sink. >> >> For video capture we propose adding new pix_format flag: >> V4L2_PIX_FMT_FLAG_HAS_CSC. The flag is set by the application, >> the driver will interpret the ycbcr_enc/hsv_enc, and quantization fields >> as the requested colorspace information and will attempt to >> do the conversion it supports. >> >> Drivers set the flags >> V4L2_FMT_FLAG_CSC_YCBCR_ENC, >> V4L2_FMT_FLAG_CSC_HSV_ENC, >> V4L2_FMT_FLAG_CSC_HSV_QUANTIZATION, > > Do we need this level of granularity? The drivers would ignore > unsupported encoding/quantization settings and reset them to supported > ones anyway, so if one doesn't support changing given parameter, it > would just return back the original stream parameter. I think this granularity allows userspace to know ahead what is supported and what is not so it doesn't have to guess. > >> in the flags field of the struct v4l2_fmtdesc during enumeration to >> indicate that they support colorspace conversion for the respective field. >> Currently the conversion of the fields colorspace, xfer_func is not >> supported since there are no drivers that support it. >> >> The same API is added for the subdevices. With the flag V4L2_MBUS_FRAMEFMT_HAS_CSC >> set by the application in the VIDIOC_SUBDEV_S_FMT ioctl and the flags >> V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC, V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION >> set by the driver in the VIDIOC_SUBDEV_ENUM_MBUS_CODE ioctl. >> >> For subdevices, new 'flags' fields were added to the structs >> v4l2_subdev_mbus_code_enum, v4l2_mbus_framefmt which are borrowed from the 'reserved' field >> >> Drivers do not have to actually look at the flags: if the flags are not >> set, then the colorspace, ycbcr_enc and quantization fields are set to >> the default values by the core, i.e. just pass on the received format >> without conversion. >> >> Signed-off-by: Hans Verkuil <Hans Verkuil@cisco.com> > > Something wrong with the email address here. > >> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> >> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> >> --- >> This is v3 of the RFC suggested originaly by Hans Verkuil: >> >> https://patchwork.linuxtv.org/patch/28847/ >> >> And then a v2 from Philipp Zabel: >> >> https://patchwork.kernel.org/project/linux-media/list/?series=15483 >> >> changes in v3: >> I added the API to subdevices as well and added fixes according to >> comments from Hans. >> I also added a usecase for the new API for the rkisp1 driver. >> >> changes in v2 (reported by Philipp Zabel): >> - convert to rst >> - split V4L2_PIX_FMT_FLAG_REQUEST_CSC into four separate flags for >> colorspace, ycbcr_enc/hsv_enc, quantization, and xfer_func >> - let driver set flags to indicate supported features >> >> [1] https://patchwork.linuxtv.org/patch/28847/ >> >> .../media/v4l/pixfmt-reserved.rst | 6 +++ >> .../media/v4l/pixfmt-v4l2-mplane.rst | 16 ++----- >> .../userspace-api/media/v4l/pixfmt-v4l2.rst | 36 +++++++++++++--- >> .../media/v4l/subdev-formats.rst | 42 +++++++++++++++++-- >> .../media/v4l/vidioc-enum-fmt.rst | 18 ++++++++ >> .../v4l/vidioc-subdev-enum-mbus-code.rst | 23 ++++++++++ >> drivers/media/v4l2-core/v4l2-ioctl.c | 19 ++++++++- >> drivers/media/v4l2-core/v4l2-subdev.c | 7 ++++ >> include/uapi/linux/v4l2-mediabus.h | 5 ++- >> include/uapi/linux/v4l2-subdev.h | 5 ++- >> include/uapi/linux/videodev2.h | 4 ++ >> 11 files changed, 158 insertions(+), 23 deletions(-) >> >> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst >> index 59b9e7238f90..fa8dada69f8c 100644 >> --- a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst >> +++ b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst >> @@ -280,3 +280,9 @@ please make a proposal on the linux-media mailing list. >> by RGBA values (128, 192, 255, 128), the same pixel described with >> premultiplied colors would be described by RGBA values (64, 96, >> 128, 128) >> + * - ``V4L2_PIX_FMT_FLAG_HAS_CSC`` >> + - 0x00000002 >> + - Set by the application. It is only used for capture and is >> + ignored for output streams. If set, then request the driver to do >> + colorspace conversion from the received colorspace, only conversions > > Should this be: > > colorspace. Only conversions > > ? > >> + of Ycbcr encoding, HSV encoding and quantization are supported. > > YCbCr? I raformulate in coming v4 this doc according to Verkuil's comments. > >> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst >> index 444b4082684c..66f3365d7b72 100644 >> --- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst >> +++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst >> @@ -105,29 +105,21 @@ describing all planes of that format. >> * - __u8 >> - ``ycbcr_enc`` >> - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`. >> - This information supplements the ``colorspace`` and must be set by >> - the driver for capture streams and by the application for output >> - streams, see :ref:`colorspaces`. >> + See struct :c:type:`v4l2_pix_format`. >> * - __u8 >> - ``hsv_enc`` >> - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`. >> - This information supplements the ``colorspace`` and must be set by >> - the driver for capture streams and by the application for output >> - streams, see :ref:`colorspaces`. >> + See struct :c:type:`v4l2_pix_format`. >> * - } >> - >> * - __u8 >> - ``quantization`` >> - Quantization range, from enum :c:type:`v4l2_quantization`. >> - This information supplements the ``colorspace`` and must be set by >> - the driver for capture streams and by the application for output >> - streams, see :ref:`colorspaces`. >> + See struct :c:type:`v4l2_pix_format`. >> * - __u8 >> - ``xfer_func`` >> - Transfer function, from enum :c:type:`v4l2_xfer_func`. >> - This information supplements the ``colorspace`` and must be set by >> - the driver for capture streams and by the application for output >> - streams, see :ref:`colorspaces`. >> + See struct :c:type:`v4l2_pix_format`. >> * - __u8 >> - ``reserved[7]`` >> - Reserved for future extensions. Should be zeroed by drivers and >> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst >> index 759420a872d6..ce57718cd66b 100644 >> --- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst >> +++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst >> @@ -110,8 +110,8 @@ Single-planar format structure >> - ``colorspace`` >> - Image colorspace, from enum :c:type:`v4l2_colorspace`. >> This information supplements the ``pixelformat`` and must be set >> - by the driver for capture streams and by the application for >> - output streams, see :ref:`colorspaces`. >> + by the driver for capture streams and by the application for output >> + streams, see :ref:`colorspace`. > > Is it expected that the colorspace itself can't be converted? Also is > the change of the reference name expected? This chunk will be removed in v4 > >> * - __u32 >> - ``priv`` >> - This field indicates whether the remaining fields of the >> @@ -148,13 +148,31 @@ Single-planar format structure >> - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`. >> This information supplements the ``colorspace`` and must be set by >> the driver for capture streams and by the application for output >> - streams, see :ref:`colorspaces`. >> + streams, see :ref:`colorspaces`. If the application sets the >> + flag ``V4L2_PIX_FMT_FLAG_HAS_CSC`` then the application can set >> + this field for a capture stream to request a specific Y'CbCr encoding >> + for the captured image data. The driver will attempt to do the >> + conversion to the specified Y'CbCr encoding or return the encoding it >> + will use if it can't do the conversion. This field is ignored for > > nit: The driver will attempt to do the conversion when the streaming > starts, so that's not an entirely correct description. How about > > "[...] captured image data. If the driver cannot handle requested > conversion, it will return another, supported encoding." > > ? I'll change that > >> + non-Y'CbCr pixelformats. The driver indicates that ycbcr_enc conversion >> + is supported by setting the flag V4L2_FMT_FLAG_CSC_YCBCR_ENC on the >> + on the corresponding struct :c:type:`v4l2_fmtdesc` during enumeration. >> + See :ref:`fmtdesc-flags` >> * - __u32 >> - ``hsv_enc`` >> - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`. >> This information supplements the ``colorspace`` and must be set by >> the driver for capture streams and by the application for output >> - streams, see :ref:`colorspaces`. >> + streams, see :ref:`colorspaces`. If the application sets the flag >> + ``V4L2_PIX_FMT_FLAG_HAS_CSC`` then the application can set this >> + field for a capture stream to request a specific HSV encoding for the >> + captured image data. The driver will attempt to do the conversion to >> + the specified HSV encoding or return the encoding it will use if it >> + can't do the conversion. This field is ignored for non-HSV > > Ditto. > >> + pixelformats. The driver indicates that hsv_enc conversion >> + is supported by setting the flag V4L2_FMT_FLAG_CSC_HSV_ENC on the >> + on the corresponding struct :c:type:`v4l2_fmtdesc` during enumeration. >> + See :ref:`fmtdesc-flags` >> * - } >> - >> * - __u32 >> @@ -162,7 +180,15 @@ Single-planar format structure >> - Quantization range, from enum :c:type:`v4l2_quantization`. >> This information supplements the ``colorspace`` and must be set by >> the driver for capture streams and by the application for output >> - streams, see :ref:`colorspaces`. >> + streams, see :ref:`colorspaces`. If the application sets the flag >> + ``V4L2_PIX_FMT_FLAG_HAS_CSC`` then the application can set >> + this field for a capture stream to request a specific quantization >> + range for the captured image data. The driver will attempt to do the >> + conversion to the specified quantization range or return the >> + quantization it will use if it can't do the conversion. The driver > > Ditto. > >> + indicates that quantization conversion is supported by setting the flag >> + V4L2_FMT_FLAG_CSC_QUANTIZATION on the on the corresponding struct >> + :c:type:`v4l2_fmtdesc` during enumeration. See :ref:`fmtdesc-flags` >> * - __u32 >> - ``xfer_func`` >> - Transfer function, from enum :c:type:`v4l2_xfer_func`. >> diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst >> index 9a4d61b0d76f..f1d4ca29a3e8 100644 >> --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst >> +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst >> @@ -49,13 +49,33 @@ Media Bus Formats >> - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`. >> This information supplements the ``colorspace`` and must be set by >> the driver for capture streams and by the application for output >> - streams, see :ref:`colorspaces`. >> + streams, see :ref:`colorspaces`. If the application sets the >> + flag ``V4L2_MBUS_FRAMEFMT_HAS_CSC`` the the application can set >> + this field for a capture stream to request a specific Y'CbCr encoding >> + for the mbus data. The driver will attempt to do the >> + conversion to the specified Y'CbCr encoding or return the encoding it >> + will use if it can't do the conversion. This field is ignored for > > Ditto. > >> + non-Y'CbCr pixelformats. The driver indicates that ycbcr_enc conversion >> + is supported by setting the flag V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC on >> + the corresponding struct c:type:`v4l2_subdev_mbus_code_enum` during >> + enumeration. See :ref:`vidioc-subdev-enum-mbus-code.rst <v4l2-subdev-mbus-code-flags>` >> + >> * - __u16 >> - ``quantization`` >> - Quantization range, from enum :c:type:`v4l2_quantization`. >> This information supplements the ``colorspace`` and must be set by >> the driver for capture streams and by the application for output >> - streams, see :ref:`colorspaces`. >> + streams, see :ref:`colorspaces`. If the application sets the >> + flag ``V4L2_MBUS_FRAMEFMT_HAS_CSC`` the the application can set >> + this field for a capture stream to request a specific quantization >> + encoding for the mbus data. The driver will attempt to do the >> + conversion to the specified quantization or return the quantization it >> + will use if it can't do the conversion. The driver indicates that > > Ditto. > >> + quantization conversion is supported by setting the flag >> + V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION on the corresponding struct >> + c:type:`v4l2_subdev_mbus_code_enum` during enumeration. >> + See :ref:`vidioc-subdev-enum-mbus-code.rst <v4l2-subdev-mbus-code-flags>` >> + >> * - __u16 >> - ``xfer_func`` >> - Transfer function, from enum :c:type:`v4l2_xfer_func`. >> @@ -63,10 +83,26 @@ Media Bus Formats >> the driver for capture streams and by the application for output >> streams, see :ref:`colorspaces`. >> * - __u16 >> - - ``reserved``\ [11] >> + - ``flags`` >> + - flags see: :ref:v4l2-mbus-framefmt-flags >> + * - __u16 >> + - ``reserved``\ [10] >> - Reserved for future extensions. Applications and drivers must set >> the array to zero. >> >> +.. _v4l2-mbus-framefmt-flags: >> + >> +.. flat-table:: v4l2_mbus_framefmt Flags >> + :header-rows: 0 >> + :stub-columns: 0 >> + :widths: 3 1 4 >> + >> + * - ``V4L2_MBUS_FRAMEFMT_HAS_CSC`` >> + - 0x0001 >> + - Set by the application. It is only used for capture and is >> + ignored for output streams. If set, then request the subdevice to do >> + colorspace conversion from the received colorspace, only conversions >> + of Ycbcr encoding, and quantization are supported. > > YCbCr? > >> >> >> .. _v4l2-mbus-pixelcode: >> diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst >> index 7e3142e11d77..125f074543af 100644 >> --- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst >> +++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst >> @@ -145,6 +145,24 @@ formats in preference order, where preferred formats are returned before >> parameters are detected. This flag can only be used in combination >> with the ``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to >> compressed formats only. It is also only applies to stateful codecs. >> + * - ``V4L2_FMT_FLAG_CSC_YCBCR_ENC`` >> + - 0x0010 >> + - The driver allows the application to try to change the default >> + ycbcr encoding. This flag is relevant only for capture devices. > > YCbCr > >> + The application can ask to configure the ycbcr_enc of the capture device >> + when calling the :c:func:`VIDIOC_S_FMT` ioctl. >> + * - ``V4L2_FMT_FLAG_CSC_HSV_ENC`` >> + - 0x0010 >> + - The driver allows the application to try to change the default >> + hsv encoding. This flag is relevant only for capture devices. >> + The application can ask to configure the hsv_enc of the capture device >> + when calling the :c:func:`VIDIOC_S_FMT` ioctl. >> + * - ``V4L2_FMT_FLAG_CSC_QUANTIZATION`` >> + - 0x0020 >> + - The driver allows the application to try to change the default >> + quantization. This flag is relevant only for capture devices. >> + The application can ask to configure the quantization of the capture device >> + when calling the :c:func:`VIDIOC_S_FMT` ioctl. >> >> >> Return Value >> diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst >> index 35b8607203a4..4ad87cb74f57 100644 >> --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst >> +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst >> @@ -78,11 +78,34 @@ information about the try formats. >> - ``which`` >> - Media bus format codes to be enumerated, from enum >> :ref:`v4l2_subdev_format_whence <v4l2-subdev-format-whence>`. >> + * - __u32 >> + - ``flags`` >> + - see :ref:`v4l2-subdev-mbus-code-flags` >> * - __u32 >> - ``reserved``\ [8] >> - Reserved for future extensions. Applications and drivers must set >> the array to zero. >> >> +.. _v4l2-subdev-mbus-code-flags: >> + >> + >> +.. tabularcolumns:: |p{4.4cm}|p{4.4cm}|p{7.7cm}| >> + >> +.. flat-table:: flags in struct v4l2_subdev_mbus_code_enum >> + :header-rows: 0 >> + :stub-columns: 0 >> + :widths: 1 1 2 >> + >> + * - V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC >> + - 0x00000001 >> + - The driver supports setting the ycbcr encoding by the application >> + when calling the VIDIOC_SUBDEV_S_FMT ioctl. see :ref:`v4l2-mbus-format` >> + on how to do this. >> + * - V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION >> + - 0x00000002 >> + - The driver supports setting the quantization by the application >> + when calling the VIDIOC_SUBDEV_S_FMT ioctl. see :ref:`v4l2-mbus-format` >> + on how to do this. >> >> Return Value >> ============ >> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c >> index b2ef8e60ea7d..3c7ffb6b15cb 100644 >> --- a/drivers/media/v4l2-core/v4l2-ioctl.c >> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c >> @@ -1029,6 +1029,15 @@ static void v4l_sanitize_format(struct v4l2_format *fmt) >> fmt->fmt.pix_mp.num_planes = min_t(u32, fmt->fmt.pix_mp.num_planes, >> VIDEO_MAX_PLANES); >> >> + if (fmt->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) { >> + if (fmt->fmt.pix_mp.flags & V4L2_PIX_FMT_FLAG_HAS_CSC) >> + return; > > This return statement might end up being confusing in the future, > because one might add further fixups below. Perhaps instead, the inner > condition negated could be added to the outer one? > >> + fmt->fmt.pix_mp.colorspace = V4L2_COLORSPACE_DEFAULT; >> + fmt->fmt.pix_mp.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; >> + fmt->fmt.pix_mp.quantization = V4L2_QUANTIZATION_DEFAULT; >> + fmt->fmt.pix_mp.xfer_func = V4L2_XFER_FUNC_DEFAULT; >> + } >> + >> /* >> * The v4l2_pix_format structure has been extended with fields that were >> * not previously required to be set to zero by applications. The priv >> @@ -1043,8 +1052,16 @@ static void v4l_sanitize_format(struct v4l2_format *fmt) >> fmt->type != V4L2_BUF_TYPE_VIDEO_OUTPUT) >> return; >> >> - if (fmt->fmt.pix.priv == V4L2_PIX_FMT_PRIV_MAGIC) >> + if (fmt->fmt.pix.priv == V4L2_PIX_FMT_PRIV_MAGIC) { >> + if (fmt->type != V4L2_BUF_TYPE_VIDEO_CAPTURE || >> + fmt->fmt.pix.flags & V4L2_PIX_FMT_FLAG_HAS_CSC) >> + return; >> + fmt->fmt.pix.colorspace = V4L2_COLORSPACE_DEFAULT; >> + fmt->fmt.pix.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; >> + fmt->fmt.pix.quantization = V4L2_QUANTIZATION_DEFAULT; >> + fmt->fmt.pix.xfer_func = V4L2_XFER_FUNC_DEFAULT; >> return; >> + } >> >> fmt->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC; >> >> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c >> index a376b351135f..51e009936aad 100644 >> --- a/drivers/media/v4l2-core/v4l2-subdev.c >> +++ b/drivers/media/v4l2-core/v4l2-subdev.c >> @@ -477,6 +477,13 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) >> case VIDIOC_SUBDEV_S_FMT: { >> struct v4l2_subdev_format *format = arg; >> >> + if (!(format->format.flags & V4L2_MBUS_FRAMEFMT_HAS_CSC)) { >> + format->format.colorspace = V4L2_COLORSPACE_DEFAULT; >> + format->format.xfer_func = V4L2_XFER_FUNC_DEFAULT; >> + format->format.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; >> + format->format.quantization = V4L2_QUANTIZATION_DEFAULT; >> + } > > Wouldn't this break setting the colorspaces on the sink pads, for which > the flag isn't required? Actually there is some unfortunate statement in > the documentation related to this: > > "This information supplements the colorspace and must be set by the > driver for capture streams and by the application for output streams, > see Colorspaces." > > However, I don't think there is any notion of "capture" and "output" for > subdevices, right? Instead, the pad direction would have to be checked, > but AFAICT there is no access to this kind of information from this > wrapper. Actually in coming v4 there is no new code added accept of the new fields and macros of the API. I think there is no need to change any code. > >> + >> memset(format->reserved, 0, sizeof(format->reserved)); >> memset(format->format.reserved, 0, sizeof(format->format.reserved)); >> return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh->pad, format); >> diff --git a/include/uapi/linux/v4l2-mediabus.h b/include/uapi/linux/v4l2-mediabus.h >> index 123a231001a8..89ff0ad6a631 100644 >> --- a/include/uapi/linux/v4l2-mediabus.h >> +++ b/include/uapi/linux/v4l2-mediabus.h >> @@ -16,6 +16,8 @@ >> #include <linux/types.h> >> #include <linux/videodev2.h> >> >> +#define V4L2_MBUS_FRAMEFMT_HAS_CSC 0x0001 >> + >> /** >> * struct v4l2_mbus_framefmt - frame format on the media bus >> * @width: image width >> @@ -36,7 +38,8 @@ struct v4l2_mbus_framefmt { >> __u16 ycbcr_enc; >> __u16 quantization; >> __u16 xfer_func; >> - __u16 reserved[11]; >> + __u16 flags; > > Are we okay with a u16 for flags? I am fine with it, though don't mind changing it if there are objections. Thanks, Dafna > > Best regards, > Tomasz >
On Fri, Jun 05, 2020 at 12:11:33PM +0200, Dafna Hirschfeld wrote: > Hi, > > On 04.06.20 19:39, Tomasz Figa wrote: > > Hi Dafna, > > > > On Thu, Apr 16, 2020 at 04:56:04PM +0200, Dafna Hirschfeld wrote: > > > From: Philipp Zabel <p.zabel@pengutronix.de> > > > > > > For video capture it is the driver that reports the colorspace, > > > Y'CbCr/HSV encoding, quantization range and transfer function > > > used by the video, and there is no way to request something > > > different, even though many HDTV receivers have some sort of > > > colorspace conversion capabilities. > > > > > > > Thanks for working on this! Please see my comments inline. > > > > > For output video this feature already exists since the application > > > specifies this information for the video format it will send out, and > > > the transmitter will enable any available CSC if a format conversion has > > > to be performed in order to match the capabilities of the sink. > > > > > > For video capture we propose adding new pix_format flag: > > > V4L2_PIX_FMT_FLAG_HAS_CSC. The flag is set by the application, > > > the driver will interpret the ycbcr_enc/hsv_enc, and quantization fields > > > as the requested colorspace information and will attempt to > > > do the conversion it supports. > > > > > > Drivers set the flags > > > V4L2_FMT_FLAG_CSC_YCBCR_ENC, > > > V4L2_FMT_FLAG_CSC_HSV_ENC, > > > V4L2_FMT_FLAG_CSC_HSV_QUANTIZATION, > > > > Do we need this level of granularity? The drivers would ignore > > unsupported encoding/quantization settings and reset them to supported > > ones anyway, so if one doesn't support changing given parameter, it > > would just return back the original stream parameter. > > I think this granularity allows userspace to know ahead what is supported > and what is not so it doesn't have to guess. > The userspace needs to guess anyway, because there is no way to enumerate the supported target parameters. For example, even if the CSC_YCBCR_ENC bit is set, only only DEFAULT, 601 and BT2020 could be supported. If the userspace wants to get the 709 encoding, it needs to explicitly try setting it and see if the TRY_FMT/S_FMT operation accepts the setting. [snip] > > > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > > > index a376b351135f..51e009936aad 100644 > > > --- a/drivers/media/v4l2-core/v4l2-subdev.c > > > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > > > @@ -477,6 +477,13 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) > > > case VIDIOC_SUBDEV_S_FMT: { > > > struct v4l2_subdev_format *format = arg; > > > + if (!(format->format.flags & V4L2_MBUS_FRAMEFMT_HAS_CSC)) { > > > + format->format.colorspace = V4L2_COLORSPACE_DEFAULT; > > > + format->format.xfer_func = V4L2_XFER_FUNC_DEFAULT; > > > + format->format.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; > > > + format->format.quantization = V4L2_QUANTIZATION_DEFAULT; > > > + } > > > > Wouldn't this break setting the colorspaces on the sink pads, for which > > the flag isn't required? Actually there is some unfortunate statement in > > the documentation related to this: > > > > "This information supplements the colorspace and must be set by the > > driver for capture streams and by the application for output streams, > > see Colorspaces." > > > > However, I don't think there is any notion of "capture" and "output" for > > subdevices, right? Instead, the pad direction would have to be checked, > > but AFAICT there is no access to this kind of information from this > > wrapper. > > Actually in coming v4 there is no new code added accept of the new fields and > macros of the API. I think there is no need to change any code. > > Agreed. > > > > > + > > > memset(format->reserved, 0, sizeof(format->reserved)); > > > memset(format->format.reserved, 0, sizeof(format->format.reserved)); > > > return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh->pad, format); > > > diff --git a/include/uapi/linux/v4l2-mediabus.h b/include/uapi/linux/v4l2-mediabus.h > > > index 123a231001a8..89ff0ad6a631 100644 > > > --- a/include/uapi/linux/v4l2-mediabus.h > > > +++ b/include/uapi/linux/v4l2-mediabus.h > > > @@ -16,6 +16,8 @@ > > > #include <linux/types.h> > > > #include <linux/videodev2.h> > > > +#define V4L2_MBUS_FRAMEFMT_HAS_CSC 0x0001 > > > + > > > /** > > > * struct v4l2_mbus_framefmt - frame format on the media bus > > > * @width: image width > > > @@ -36,7 +38,8 @@ struct v4l2_mbus_framefmt { > > > __u16 ycbcr_enc; > > > __u16 quantization; > > > __u16 xfer_func; > > > - __u16 reserved[11]; > > > + __u16 flags; > > > > Are we okay with a u16 for flags? > > I am fine with it, though don't mind changing it if there are objections. > I'd suggest making it a u32 to be a bit more future-proof. Best regards, Tomasz
On 10.06.20 15:34, Tomasz Figa wrote: > On Fri, Jun 05, 2020 at 12:11:33PM +0200, Dafna Hirschfeld wrote: >> Hi, >> >> On 04.06.20 19:39, Tomasz Figa wrote: >>> Hi Dafna, >>> >>> On Thu, Apr 16, 2020 at 04:56:04PM +0200, Dafna Hirschfeld wrote: >>>> From: Philipp Zabel <p.zabel@pengutronix.de> >>>> >>>> For video capture it is the driver that reports the colorspace, >>>> Y'CbCr/HSV encoding, quantization range and transfer function >>>> used by the video, and there is no way to request something >>>> different, even though many HDTV receivers have some sort of >>>> colorspace conversion capabilities. >>>> >>> >>> Thanks for working on this! Please see my comments inline. >>> >>>> For output video this feature already exists since the application >>>> specifies this information for the video format it will send out, and >>>> the transmitter will enable any available CSC if a format conversion has >>>> to be performed in order to match the capabilities of the sink. >>>> >>>> For video capture we propose adding new pix_format flag: >>>> V4L2_PIX_FMT_FLAG_HAS_CSC. The flag is set by the application, >>>> the driver will interpret the ycbcr_enc/hsv_enc, and quantization fields >>>> as the requested colorspace information and will attempt to >>>> do the conversion it supports. >>>> >>>> Drivers set the flags >>>> V4L2_FMT_FLAG_CSC_YCBCR_ENC, >>>> V4L2_FMT_FLAG_CSC_HSV_ENC, >>>> V4L2_FMT_FLAG_CSC_HSV_QUANTIZATION, >>> >>> Do we need this level of granularity? The drivers would ignore >>> unsupported encoding/quantization settings and reset them to supported >>> ones anyway, so if one doesn't support changing given parameter, it >>> would just return back the original stream parameter. >> >> I think this granularity allows userspace to know ahead what is supported >> and what is not so it doesn't have to guess. >> > > The userspace needs to guess anyway, because there is no way to > enumerate the supported target parameters. For example, even if the > CSC_YCBCR_ENC bit is set, only only DEFAULT, 601 and BT2020 could be > supported. If the userspace wants to get the 709 encoding, it needs to > explicitly try setting it and see if the TRY_FMT/S_FMT operation accepts > the setting. yes, indeed, Hans Verkuil suggested those flags. Maybe it is indeed enough to have one flag. > > [snip] >>>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c >>>> index a376b351135f..51e009936aad 100644 >>>> --- a/drivers/media/v4l2-core/v4l2-subdev.c >>>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c >>>> @@ -477,6 +477,13 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) >>>> case VIDIOC_SUBDEV_S_FMT: { >>>> struct v4l2_subdev_format *format = arg; >>>> + if (!(format->format.flags & V4L2_MBUS_FRAMEFMT_HAS_CSC)) { >>>> + format->format.colorspace = V4L2_COLORSPACE_DEFAULT; >>>> + format->format.xfer_func = V4L2_XFER_FUNC_DEFAULT; >>>> + format->format.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; >>>> + format->format.quantization = V4L2_QUANTIZATION_DEFAULT; >>>> + } >>> >>> Wouldn't this break setting the colorspaces on the sink pads, for which >>> the flag isn't required? Actually there is some unfortunate statement in >>> the documentation related to this: >>> >>> "This information supplements the colorspace and must be set by the >>> driver for capture streams and by the application for output streams, >>> see Colorspaces." >>> >>> However, I don't think there is any notion of "capture" and "output" for >>> subdevices, right? Instead, the pad direction would have to be checked, >>> but AFAICT there is no access to this kind of information from this >>> wrapper. >> >> Actually in coming v4 there is no new code added accept of the new fields and >> macros of the API. I think there is no need to change any code. >> >> > > Agreed. > >>> >>>> + >>>> memset(format->reserved, 0, sizeof(format->reserved)); >>>> memset(format->format.reserved, 0, sizeof(format->format.reserved)); >>>> return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh->pad, format); >>>> diff --git a/include/uapi/linux/v4l2-mediabus.h b/include/uapi/linux/v4l2-mediabus.h >>>> index 123a231001a8..89ff0ad6a631 100644 >>>> --- a/include/uapi/linux/v4l2-mediabus.h >>>> +++ b/include/uapi/linux/v4l2-mediabus.h >>>> @@ -16,6 +16,8 @@ >>>> #include <linux/types.h> >>>> #include <linux/videodev2.h> >>>> +#define V4L2_MBUS_FRAMEFMT_HAS_CSC 0x0001 >>>> + >>>> /** >>>> * struct v4l2_mbus_framefmt - frame format on the media bus >>>> * @width: image width >>>> @@ -36,7 +38,8 @@ struct v4l2_mbus_framefmt { >>>> __u16 ycbcr_enc; >>>> __u16 quantization; >>>> __u16 xfer_func; >>>> - __u16 reserved[11]; >>>> + __u16 flags; >>> >>> Are we okay with a u16 for flags? >> >> I am fine with it, though don't mind changing it if there are objections. >> > > I'd suggest making it a u32 to be a bit more future-proof. ok, I see that just changing the type to __u32 and the reserved array to 'reserved[9]' increases the struct size from 48 to 52 because of padding. There are two ways to solve it, - move the flags field to be just above 'ycbcr_enc' - change reserve to 'reserve[8]' Is moving fields order in a struct ok? If so it save us 2 bytes. Thanks, Dafna > > Best regards, > Tomasz >
On Tue, Jun 30, 2020 at 6:11 PM Dafna Hirschfeld <dafna.hirschfeld@collabora.com> wrote: > > > > On 10.06.20 15:34, Tomasz Figa wrote: > > On Fri, Jun 05, 2020 at 12:11:33PM +0200, Dafna Hirschfeld wrote: > >> Hi, > >> > >> On 04.06.20 19:39, Tomasz Figa wrote: > >>> Hi Dafna, > >>> > >>> On Thu, Apr 16, 2020 at 04:56:04PM +0200, Dafna Hirschfeld wrote: > >>>> From: Philipp Zabel <p.zabel@pengutronix.de> > >>>> > >>>> For video capture it is the driver that reports the colorspace, > >>>> Y'CbCr/HSV encoding, quantization range and transfer function > >>>> used by the video, and there is no way to request something > >>>> different, even though many HDTV receivers have some sort of > >>>> colorspace conversion capabilities. > >>>> > >>> > >>> Thanks for working on this! Please see my comments inline. > >>> > >>>> For output video this feature already exists since the application > >>>> specifies this information for the video format it will send out, and > >>>> the transmitter will enable any available CSC if a format conversion has > >>>> to be performed in order to match the capabilities of the sink. > >>>> > >>>> For video capture we propose adding new pix_format flag: > >>>> V4L2_PIX_FMT_FLAG_HAS_CSC. The flag is set by the application, > >>>> the driver will interpret the ycbcr_enc/hsv_enc, and quantization fields > >>>> as the requested colorspace information and will attempt to > >>>> do the conversion it supports. > >>>> > >>>> Drivers set the flags > >>>> V4L2_FMT_FLAG_CSC_YCBCR_ENC, > >>>> V4L2_FMT_FLAG_CSC_HSV_ENC, > >>>> V4L2_FMT_FLAG_CSC_HSV_QUANTIZATION, > >>> > >>> Do we need this level of granularity? The drivers would ignore > >>> unsupported encoding/quantization settings and reset them to supported > >>> ones anyway, so if one doesn't support changing given parameter, it > >>> would just return back the original stream parameter. > >> > >> I think this granularity allows userspace to know ahead what is supported > >> and what is not so it doesn't have to guess. > >> > > > > The userspace needs to guess anyway, because there is no way to > > enumerate the supported target parameters. For example, even if the > > CSC_YCBCR_ENC bit is set, only only DEFAULT, 601 and BT2020 could be > > supported. If the userspace wants to get the 709 encoding, it needs to > > explicitly try setting it and see if the TRY_FMT/S_FMT operation accepts > > the setting. > > yes, indeed, Hans Verkuil suggested those flags. Maybe it is indeed enough > to have one flag. > Hans, what's your thought on this? > > > > [snip] > >>>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > >>>> index a376b351135f..51e009936aad 100644 > >>>> --- a/drivers/media/v4l2-core/v4l2-subdev.c > >>>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c > >>>> @@ -477,6 +477,13 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) > >>>> case VIDIOC_SUBDEV_S_FMT: { > >>>> struct v4l2_subdev_format *format = arg; > >>>> + if (!(format->format.flags & V4L2_MBUS_FRAMEFMT_HAS_CSC)) { > >>>> + format->format.colorspace = V4L2_COLORSPACE_DEFAULT; > >>>> + format->format.xfer_func = V4L2_XFER_FUNC_DEFAULT; > >>>> + format->format.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; > >>>> + format->format.quantization = V4L2_QUANTIZATION_DEFAULT; > >>>> + } > >>> > >>> Wouldn't this break setting the colorspaces on the sink pads, for which > >>> the flag isn't required? Actually there is some unfortunate statement in > >>> the documentation related to this: > >>> > >>> "This information supplements the colorspace and must be set by the > >>> driver for capture streams and by the application for output streams, > >>> see Colorspaces." > >>> > >>> However, I don't think there is any notion of "capture" and "output" for > >>> subdevices, right? Instead, the pad direction would have to be checked, > >>> but AFAICT there is no access to this kind of information from this > >>> wrapper. > >> > >> Actually in coming v4 there is no new code added accept of the new fields and > >> macros of the API. I think there is no need to change any code. > >> > >> > > > > Agreed. > > > >>> > >>>> + > >>>> memset(format->reserved, 0, sizeof(format->reserved)); > >>>> memset(format->format.reserved, 0, sizeof(format->format.reserved)); > >>>> return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh->pad, format); > >>>> diff --git a/include/uapi/linux/v4l2-mediabus.h b/include/uapi/linux/v4l2-mediabus.h > >>>> index 123a231001a8..89ff0ad6a631 100644 > >>>> --- a/include/uapi/linux/v4l2-mediabus.h > >>>> +++ b/include/uapi/linux/v4l2-mediabus.h > >>>> @@ -16,6 +16,8 @@ > >>>> #include <linux/types.h> > >>>> #include <linux/videodev2.h> > >>>> +#define V4L2_MBUS_FRAMEFMT_HAS_CSC 0x0001 > >>>> + > >>>> /** > >>>> * struct v4l2_mbus_framefmt - frame format on the media bus > >>>> * @width: image width > >>>> @@ -36,7 +38,8 @@ struct v4l2_mbus_framefmt { > >>>> __u16 ycbcr_enc; > >>>> __u16 quantization; > >>>> __u16 xfer_func; > >>>> - __u16 reserved[11]; > >>>> + __u16 flags; > >>> > >>> Are we okay with a u16 for flags? > >> > >> I am fine with it, though don't mind changing it if there are objections. > >> > > > > I'd suggest making it a u32 to be a bit more future-proof. > > ok, I see that just changing the type to __u32 and the reserved array > to 'reserved[9]' increases the struct size from 48 to 52 because of padding. > There are two ways to solve it, > - move the flags field to be just above 'ycbcr_enc' > - change reserve to 'reserve[8]' > > Is moving fields order in a struct ok? If so it save us 2 bytes. Since the structure is a part of the stable UAPI, we can't reorder the fields. Similarly, we can't change the struct size, because it's embedded in the ioctl code. (Although there are ways around it, not currently implemented by V4L2.) That leaves us only the second option - changing reserved to [8]. Best regards, Tomasz
Hi Tomasz, On Tue, Jun 30, 2020 at 06:31:19PM +0200, Tomasz Figa wrote: > On Tue, Jun 30, 2020 at 6:11 PM Dafna Hirschfeld wrote: > > On 10.06.20 15:34, Tomasz Figa wrote: > >> On Fri, Jun 05, 2020 at 12:11:33PM +0200, Dafna Hirschfeld wrote: > >>> On 04.06.20 19:39, Tomasz Figa wrote: > >>>> On Thu, Apr 16, 2020 at 04:56:04PM +0200, Dafna Hirschfeld wrote: > >>>>> From: Philipp Zabel <p.zabel@pengutronix.de> > >>>>> > >>>>> For video capture it is the driver that reports the colorspace, > >>>>> Y'CbCr/HSV encoding, quantization range and transfer function > >>>>> used by the video, and there is no way to request something > >>>>> different, even though many HDTV receivers have some sort of > >>>>> colorspace conversion capabilities. > >>>> > >>>> Thanks for working on this! Please see my comments inline. > >>>> > >>>>> For output video this feature already exists since the application > >>>>> specifies this information for the video format it will send out, and > >>>>> the transmitter will enable any available CSC if a format conversion has > >>>>> to be performed in order to match the capabilities of the sink. > >>>>> > >>>>> For video capture we propose adding new pix_format flag: > >>>>> V4L2_PIX_FMT_FLAG_HAS_CSC. The flag is set by the application, > >>>>> the driver will interpret the ycbcr_enc/hsv_enc, and quantization fields > >>>>> as the requested colorspace information and will attempt to > >>>>> do the conversion it supports. > >>>>> > >>>>> Drivers set the flags > >>>>> V4L2_FMT_FLAG_CSC_YCBCR_ENC, > >>>>> V4L2_FMT_FLAG_CSC_HSV_ENC, > >>>>> V4L2_FMT_FLAG_CSC_HSV_QUANTIZATION, > >>>> > >>>> Do we need this level of granularity? The drivers would ignore > >>>> unsupported encoding/quantization settings and reset them to supported > >>>> ones anyway, so if one doesn't support changing given parameter, it > >>>> would just return back the original stream parameter. > >>> > >>> I think this granularity allows userspace to know ahead what is supported > >>> and what is not so it doesn't have to guess. > >>> > >> > >> The userspace needs to guess anyway, because there is no way to > >> enumerate the supported target parameters. For example, even if the > >> CSC_YCBCR_ENC bit is set, only only DEFAULT, 601 and BT2020 could be > >> supported. If the userspace wants to get the 709 encoding, it needs to > >> explicitly try setting it and see if the TRY_FMT/S_FMT operation accepts > >> the setting. > > > > yes, indeed, Hans Verkuil suggested those flags. Maybe it is indeed enough > > to have one flag. > > > > Hans, what's your thought on this? > > >> > >> [snip] > >>>>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > >>>>> index a376b351135f..51e009936aad 100644 > >>>>> --- a/drivers/media/v4l2-core/v4l2-subdev.c > >>>>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c > >>>>> @@ -477,6 +477,13 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) > >>>>> case VIDIOC_SUBDEV_S_FMT: { > >>>>> struct v4l2_subdev_format *format = arg; > >>>>> + if (!(format->format.flags & V4L2_MBUS_FRAMEFMT_HAS_CSC)) { > >>>>> + format->format.colorspace = V4L2_COLORSPACE_DEFAULT; > >>>>> + format->format.xfer_func = V4L2_XFER_FUNC_DEFAULT; > >>>>> + format->format.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; > >>>>> + format->format.quantization = V4L2_QUANTIZATION_DEFAULT; > >>>>> + } > >>>> > >>>> Wouldn't this break setting the colorspaces on the sink pads, for which > >>>> the flag isn't required? Actually there is some unfortunate statement in > >>>> the documentation related to this: > >>>> > >>>> "This information supplements the colorspace and must be set by the > >>>> driver for capture streams and by the application for output streams, > >>>> see Colorspaces." > >>>> > >>>> However, I don't think there is any notion of "capture" and "output" for > >>>> subdevices, right? Instead, the pad direction would have to be checked, > >>>> but AFAICT there is no access to this kind of information from this > >>>> wrapper. > >>> > >>> Actually in coming v4 there is no new code added accept of the new fields and > >>> macros of the API. I think there is no need to change any code. > >> > >> Agreed. > >> > >>>>> + > >>>>> memset(format->reserved, 0, sizeof(format->reserved)); > >>>>> memset(format->format.reserved, 0, sizeof(format->format.reserved)); > >>>>> return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh->pad, format); > >>>>> diff --git a/include/uapi/linux/v4l2-mediabus.h b/include/uapi/linux/v4l2-mediabus.h > >>>>> index 123a231001a8..89ff0ad6a631 100644 > >>>>> --- a/include/uapi/linux/v4l2-mediabus.h > >>>>> +++ b/include/uapi/linux/v4l2-mediabus.h > >>>>> @@ -16,6 +16,8 @@ > >>>>> #include <linux/types.h> > >>>>> #include <linux/videodev2.h> > >>>>> +#define V4L2_MBUS_FRAMEFMT_HAS_CSC 0x0001 > >>>>> + > >>>>> /** > >>>>> * struct v4l2_mbus_framefmt - frame format on the media bus > >>>>> * @width: image width > >>>>> @@ -36,7 +38,8 @@ struct v4l2_mbus_framefmt { > >>>>> __u16 ycbcr_enc; > >>>>> __u16 quantization; > >>>>> __u16 xfer_func; > >>>>> - __u16 reserved[11]; > >>>>> + __u16 flags; > >>>> > >>>> Are we okay with a u16 for flags? > >>> > >>> I am fine with it, though don't mind changing it if there are objections. > >>> > >> > >> I'd suggest making it a u32 to be a bit more future-proof. > > > > ok, I see that just changing the type to __u32 and the reserved array > > to 'reserved[9]' increases the struct size from 48 to 52 because of padding. > > There are two ways to solve it, > > - move the flags field to be just above 'ycbcr_enc' > > - change reserve to 'reserve[8]' > > > > Is moving fields order in a struct ok? If so it save us 2 bytes. > > Since the structure is a part of the stable UAPI, we can't reorder the > fields. Similarly, we can't change the struct size, because it's > embedded in the ioctl code. (Although there are ways around it, not > currently implemented by V4L2.) That leaves us only the second option > - changing reserved to [8]. You can also possibly do __u16 ycbcr_enc; __u16 quantization; __u16 xfer_func; __u16 reserved2; __u32 flags; __u16 reserved[8]; to explicitly show there's a hole.
On Tue, Jun 30, 2020 at 7:43 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Tomasz, > > On Tue, Jun 30, 2020 at 06:31:19PM +0200, Tomasz Figa wrote: > > On Tue, Jun 30, 2020 at 6:11 PM Dafna Hirschfeld wrote: > > > On 10.06.20 15:34, Tomasz Figa wrote: > > >> On Fri, Jun 05, 2020 at 12:11:33PM +0200, Dafna Hirschfeld wrote: > > >>> On 04.06.20 19:39, Tomasz Figa wrote: > > >>>> On Thu, Apr 16, 2020 at 04:56:04PM +0200, Dafna Hirschfeld wrote: > > >>>>> From: Philipp Zabel <p.zabel@pengutronix.de> > > >>>>> > > >>>>> For video capture it is the driver that reports the colorspace, > > >>>>> Y'CbCr/HSV encoding, quantization range and transfer function > > >>>>> used by the video, and there is no way to request something > > >>>>> different, even though many HDTV receivers have some sort of > > >>>>> colorspace conversion capabilities. > > >>>> > > >>>> Thanks for working on this! Please see my comments inline. > > >>>> > > >>>>> For output video this feature already exists since the application > > >>>>> specifies this information for the video format it will send out, and > > >>>>> the transmitter will enable any available CSC if a format conversion has > > >>>>> to be performed in order to match the capabilities of the sink. > > >>>>> > > >>>>> For video capture we propose adding new pix_format flag: > > >>>>> V4L2_PIX_FMT_FLAG_HAS_CSC. The flag is set by the application, > > >>>>> the driver will interpret the ycbcr_enc/hsv_enc, and quantization fields > > >>>>> as the requested colorspace information and will attempt to > > >>>>> do the conversion it supports. > > >>>>> > > >>>>> Drivers set the flags > > >>>>> V4L2_FMT_FLAG_CSC_YCBCR_ENC, > > >>>>> V4L2_FMT_FLAG_CSC_HSV_ENC, > > >>>>> V4L2_FMT_FLAG_CSC_HSV_QUANTIZATION, > > >>>> > > >>>> Do we need this level of granularity? The drivers would ignore > > >>>> unsupported encoding/quantization settings and reset them to supported > > >>>> ones anyway, so if one doesn't support changing given parameter, it > > >>>> would just return back the original stream parameter. > > >>> > > >>> I think this granularity allows userspace to know ahead what is supported > > >>> and what is not so it doesn't have to guess. > > >>> > > >> > > >> The userspace needs to guess anyway, because there is no way to > > >> enumerate the supported target parameters. For example, even if the > > >> CSC_YCBCR_ENC bit is set, only only DEFAULT, 601 and BT2020 could be > > >> supported. If the userspace wants to get the 709 encoding, it needs to > > >> explicitly try setting it and see if the TRY_FMT/S_FMT operation accepts > > >> the setting. > > > > > > yes, indeed, Hans Verkuil suggested those flags. Maybe it is indeed enough > > > to have one flag. > > > > > > > Hans, what's your thought on this? > > > > >> > > >> [snip] > > >>>>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > > >>>>> index a376b351135f..51e009936aad 100644 > > >>>>> --- a/drivers/media/v4l2-core/v4l2-subdev.c > > >>>>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c > > >>>>> @@ -477,6 +477,13 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) > > >>>>> case VIDIOC_SUBDEV_S_FMT: { > > >>>>> struct v4l2_subdev_format *format = arg; > > >>>>> + if (!(format->format.flags & V4L2_MBUS_FRAMEFMT_HAS_CSC)) { > > >>>>> + format->format.colorspace = V4L2_COLORSPACE_DEFAULT; > > >>>>> + format->format.xfer_func = V4L2_XFER_FUNC_DEFAULT; > > >>>>> + format->format.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; > > >>>>> + format->format.quantization = V4L2_QUANTIZATION_DEFAULT; > > >>>>> + } > > >>>> > > >>>> Wouldn't this break setting the colorspaces on the sink pads, for which > > >>>> the flag isn't required? Actually there is some unfortunate statement in > > >>>> the documentation related to this: > > >>>> > > >>>> "This information supplements the colorspace and must be set by the > > >>>> driver for capture streams and by the application for output streams, > > >>>> see Colorspaces." > > >>>> > > >>>> However, I don't think there is any notion of "capture" and "output" for > > >>>> subdevices, right? Instead, the pad direction would have to be checked, > > >>>> but AFAICT there is no access to this kind of information from this > > >>>> wrapper. > > >>> > > >>> Actually in coming v4 there is no new code added accept of the new fields and > > >>> macros of the API. I think there is no need to change any code. > > >> > > >> Agreed. > > >> > > >>>>> + > > >>>>> memset(format->reserved, 0, sizeof(format->reserved)); > > >>>>> memset(format->format.reserved, 0, sizeof(format->format.reserved)); > > >>>>> return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh->pad, format); > > >>>>> diff --git a/include/uapi/linux/v4l2-mediabus.h b/include/uapi/linux/v4l2-mediabus.h > > >>>>> index 123a231001a8..89ff0ad6a631 100644 > > >>>>> --- a/include/uapi/linux/v4l2-mediabus.h > > >>>>> +++ b/include/uapi/linux/v4l2-mediabus.h > > >>>>> @@ -16,6 +16,8 @@ > > >>>>> #include <linux/types.h> > > >>>>> #include <linux/videodev2.h> > > >>>>> +#define V4L2_MBUS_FRAMEFMT_HAS_CSC 0x0001 > > >>>>> + > > >>>>> /** > > >>>>> * struct v4l2_mbus_framefmt - frame format on the media bus > > >>>>> * @width: image width > > >>>>> @@ -36,7 +38,8 @@ struct v4l2_mbus_framefmt { > > >>>>> __u16 ycbcr_enc; > > >>>>> __u16 quantization; > > >>>>> __u16 xfer_func; > > >>>>> - __u16 reserved[11]; > > >>>>> + __u16 flags; > > >>>> > > >>>> Are we okay with a u16 for flags? > > >>> > > >>> I am fine with it, though don't mind changing it if there are objections. > > >>> > > >> > > >> I'd suggest making it a u32 to be a bit more future-proof. > > > > > > ok, I see that just changing the type to __u32 and the reserved array > > > to 'reserved[9]' increases the struct size from 48 to 52 because of padding. > > > There are two ways to solve it, > > > - move the flags field to be just above 'ycbcr_enc' > > > - change reserve to 'reserve[8]' > > > > > > Is moving fields order in a struct ok? If so it save us 2 bytes. > > > > Since the structure is a part of the stable UAPI, we can't reorder the > > fields. Similarly, we can't change the struct size, because it's > > embedded in the ioctl code. (Although there are ways around it, not > > currently implemented by V4L2.) That leaves us only the second option > > - changing reserved to [8]. > > You can also possibly do > > __u16 ycbcr_enc; > __u16 quantization; > __u16 xfer_func; > __u16 reserved2; > __u32 flags; > __u16 reserved[8]; > > to explicitly show there's a hole. Good point. I didn't realize that there actually was a hole. Thought that xfer_func ended at a 32-bit boundary. Perhaps when changing this, we could make it __u32 reserved[4]? Or would that have some compatibility concerns? Best regards, Tomasz
diff --git a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst index 59b9e7238f90..fa8dada69f8c 100644 --- a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst +++ b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst @@ -280,3 +280,9 @@ please make a proposal on the linux-media mailing list. by RGBA values (128, 192, 255, 128), the same pixel described with premultiplied colors would be described by RGBA values (64, 96, 128, 128) + * - ``V4L2_PIX_FMT_FLAG_HAS_CSC`` + - 0x00000002 + - Set by the application. It is only used for capture and is + ignored for output streams. If set, then request the driver to do + colorspace conversion from the received colorspace, only conversions + of Ycbcr encoding, HSV encoding and quantization are supported. diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst index 444b4082684c..66f3365d7b72 100644 --- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst +++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2-mplane.rst @@ -105,29 +105,21 @@ describing all planes of that format. * - __u8 - ``ycbcr_enc`` - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`. - This information supplements the ``colorspace`` and must be set by - the driver for capture streams and by the application for output - streams, see :ref:`colorspaces`. + See struct :c:type:`v4l2_pix_format`. * - __u8 - ``hsv_enc`` - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`. - This information supplements the ``colorspace`` and must be set by - the driver for capture streams and by the application for output - streams, see :ref:`colorspaces`. + See struct :c:type:`v4l2_pix_format`. * - } - * - __u8 - ``quantization`` - Quantization range, from enum :c:type:`v4l2_quantization`. - This information supplements the ``colorspace`` and must be set by - the driver for capture streams and by the application for output - streams, see :ref:`colorspaces`. + See struct :c:type:`v4l2_pix_format`. * - __u8 - ``xfer_func`` - Transfer function, from enum :c:type:`v4l2_xfer_func`. - This information supplements the ``colorspace`` and must be set by - the driver for capture streams and by the application for output - streams, see :ref:`colorspaces`. + See struct :c:type:`v4l2_pix_format`. * - __u8 - ``reserved[7]`` - Reserved for future extensions. Should be zeroed by drivers and diff --git a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst index 759420a872d6..ce57718cd66b 100644 --- a/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst +++ b/Documentation/userspace-api/media/v4l/pixfmt-v4l2.rst @@ -110,8 +110,8 @@ Single-planar format structure - ``colorspace`` - Image colorspace, from enum :c:type:`v4l2_colorspace`. This information supplements the ``pixelformat`` and must be set - by the driver for capture streams and by the application for - output streams, see :ref:`colorspaces`. + by the driver for capture streams and by the application for output + streams, see :ref:`colorspace`. * - __u32 - ``priv`` - This field indicates whether the remaining fields of the @@ -148,13 +148,31 @@ Single-planar format structure - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`. This information supplements the ``colorspace`` and must be set by the driver for capture streams and by the application for output - streams, see :ref:`colorspaces`. + streams, see :ref:`colorspaces`. If the application sets the + flag ``V4L2_PIX_FMT_FLAG_HAS_CSC`` then the application can set + this field for a capture stream to request a specific Y'CbCr encoding + for the captured image data. The driver will attempt to do the + conversion to the specified Y'CbCr encoding or return the encoding it + will use if it can't do the conversion. This field is ignored for + non-Y'CbCr pixelformats. The driver indicates that ycbcr_enc conversion + is supported by setting the flag V4L2_FMT_FLAG_CSC_YCBCR_ENC on the + on the corresponding struct :c:type:`v4l2_fmtdesc` during enumeration. + See :ref:`fmtdesc-flags` * - __u32 - ``hsv_enc`` - HSV encoding, from enum :c:type:`v4l2_hsv_encoding`. This information supplements the ``colorspace`` and must be set by the driver for capture streams and by the application for output - streams, see :ref:`colorspaces`. + streams, see :ref:`colorspaces`. If the application sets the flag + ``V4L2_PIX_FMT_FLAG_HAS_CSC`` then the application can set this + field for a capture stream to request a specific HSV encoding for the + captured image data. The driver will attempt to do the conversion to + the specified HSV encoding or return the encoding it will use if it + can't do the conversion. This field is ignored for non-HSV + pixelformats. The driver indicates that hsv_enc conversion + is supported by setting the flag V4L2_FMT_FLAG_CSC_HSV_ENC on the + on the corresponding struct :c:type:`v4l2_fmtdesc` during enumeration. + See :ref:`fmtdesc-flags` * - } - * - __u32 @@ -162,7 +180,15 @@ Single-planar format structure - Quantization range, from enum :c:type:`v4l2_quantization`. This information supplements the ``colorspace`` and must be set by the driver for capture streams and by the application for output - streams, see :ref:`colorspaces`. + streams, see :ref:`colorspaces`. If the application sets the flag + ``V4L2_PIX_FMT_FLAG_HAS_CSC`` then the application can set + this field for a capture stream to request a specific quantization + range for the captured image data. The driver will attempt to do the + conversion to the specified quantization range or return the + quantization it will use if it can't do the conversion. The driver + indicates that quantization conversion is supported by setting the flag + V4L2_FMT_FLAG_CSC_QUANTIZATION on the on the corresponding struct + :c:type:`v4l2_fmtdesc` during enumeration. See :ref:`fmtdesc-flags` * - __u32 - ``xfer_func`` - Transfer function, from enum :c:type:`v4l2_xfer_func`. diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst index 9a4d61b0d76f..f1d4ca29a3e8 100644 --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst @@ -49,13 +49,33 @@ Media Bus Formats - Y'CbCr encoding, from enum :c:type:`v4l2_ycbcr_encoding`. This information supplements the ``colorspace`` and must be set by the driver for capture streams and by the application for output - streams, see :ref:`colorspaces`. + streams, see :ref:`colorspaces`. If the application sets the + flag ``V4L2_MBUS_FRAMEFMT_HAS_CSC`` the the application can set + this field for a capture stream to request a specific Y'CbCr encoding + for the mbus data. The driver will attempt to do the + conversion to the specified Y'CbCr encoding or return the encoding it + will use if it can't do the conversion. This field is ignored for + non-Y'CbCr pixelformats. The driver indicates that ycbcr_enc conversion + is supported by setting the flag V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC on + the corresponding struct c:type:`v4l2_subdev_mbus_code_enum` during + enumeration. See :ref:`vidioc-subdev-enum-mbus-code.rst <v4l2-subdev-mbus-code-flags>` + * - __u16 - ``quantization`` - Quantization range, from enum :c:type:`v4l2_quantization`. This information supplements the ``colorspace`` and must be set by the driver for capture streams and by the application for output - streams, see :ref:`colorspaces`. + streams, see :ref:`colorspaces`. If the application sets the + flag ``V4L2_MBUS_FRAMEFMT_HAS_CSC`` the the application can set + this field for a capture stream to request a specific quantization + encoding for the mbus data. The driver will attempt to do the + conversion to the specified quantization or return the quantization it + will use if it can't do the conversion. The driver indicates that + quantization conversion is supported by setting the flag + V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION on the corresponding struct + c:type:`v4l2_subdev_mbus_code_enum` during enumeration. + See :ref:`vidioc-subdev-enum-mbus-code.rst <v4l2-subdev-mbus-code-flags>` + * - __u16 - ``xfer_func`` - Transfer function, from enum :c:type:`v4l2_xfer_func`. @@ -63,10 +83,26 @@ Media Bus Formats the driver for capture streams and by the application for output streams, see :ref:`colorspaces`. * - __u16 - - ``reserved``\ [11] + - ``flags`` + - flags see: :ref:v4l2-mbus-framefmt-flags + * - __u16 + - ``reserved``\ [10] - Reserved for future extensions. Applications and drivers must set the array to zero. +.. _v4l2-mbus-framefmt-flags: + +.. flat-table:: v4l2_mbus_framefmt Flags + :header-rows: 0 + :stub-columns: 0 + :widths: 3 1 4 + + * - ``V4L2_MBUS_FRAMEFMT_HAS_CSC`` + - 0x0001 + - Set by the application. It is only used for capture and is + ignored for output streams. If set, then request the subdevice to do + colorspace conversion from the received colorspace, only conversions + of Ycbcr encoding, and quantization are supported. .. _v4l2-mbus-pixelcode: diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst index 7e3142e11d77..125f074543af 100644 --- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst +++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst @@ -145,6 +145,24 @@ formats in preference order, where preferred formats are returned before parameters are detected. This flag can only be used in combination with the ``V4L2_FMT_FLAG_COMPRESSED`` flag, since this applies to compressed formats only. It is also only applies to stateful codecs. + * - ``V4L2_FMT_FLAG_CSC_YCBCR_ENC`` + - 0x0010 + - The driver allows the application to try to change the default + ycbcr encoding. This flag is relevant only for capture devices. + The application can ask to configure the ycbcr_enc of the capture device + when calling the :c:func:`VIDIOC_S_FMT` ioctl. + * - ``V4L2_FMT_FLAG_CSC_HSV_ENC`` + - 0x0010 + - The driver allows the application to try to change the default + hsv encoding. This flag is relevant only for capture devices. + The application can ask to configure the hsv_enc of the capture device + when calling the :c:func:`VIDIOC_S_FMT` ioctl. + * - ``V4L2_FMT_FLAG_CSC_QUANTIZATION`` + - 0x0020 + - The driver allows the application to try to change the default + quantization. This flag is relevant only for capture devices. + The application can ask to configure the quantization of the capture device + when calling the :c:func:`VIDIOC_S_FMT` ioctl. Return Value diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst index 35b8607203a4..4ad87cb74f57 100644 --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-enum-mbus-code.rst @@ -78,11 +78,34 @@ information about the try formats. - ``which`` - Media bus format codes to be enumerated, from enum :ref:`v4l2_subdev_format_whence <v4l2-subdev-format-whence>`. + * - __u32 + - ``flags`` + - see :ref:`v4l2-subdev-mbus-code-flags` * - __u32 - ``reserved``\ [8] - Reserved for future extensions. Applications and drivers must set the array to zero. +.. _v4l2-subdev-mbus-code-flags: + + +.. tabularcolumns:: |p{4.4cm}|p{4.4cm}|p{7.7cm}| + +.. flat-table:: flags in struct v4l2_subdev_mbus_code_enum + :header-rows: 0 + :stub-columns: 0 + :widths: 1 1 2 + + * - V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC + - 0x00000001 + - The driver supports setting the ycbcr encoding by the application + when calling the VIDIOC_SUBDEV_S_FMT ioctl. see :ref:`v4l2-mbus-format` + on how to do this. + * - V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION + - 0x00000002 + - The driver supports setting the quantization by the application + when calling the VIDIOC_SUBDEV_S_FMT ioctl. see :ref:`v4l2-mbus-format` + on how to do this. Return Value ============ diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index b2ef8e60ea7d..3c7ffb6b15cb 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -1029,6 +1029,15 @@ static void v4l_sanitize_format(struct v4l2_format *fmt) fmt->fmt.pix_mp.num_planes = min_t(u32, fmt->fmt.pix_mp.num_planes, VIDEO_MAX_PLANES); + if (fmt->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) { + if (fmt->fmt.pix_mp.flags & V4L2_PIX_FMT_FLAG_HAS_CSC) + return; + fmt->fmt.pix_mp.colorspace = V4L2_COLORSPACE_DEFAULT; + fmt->fmt.pix_mp.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; + fmt->fmt.pix_mp.quantization = V4L2_QUANTIZATION_DEFAULT; + fmt->fmt.pix_mp.xfer_func = V4L2_XFER_FUNC_DEFAULT; + } + /* * The v4l2_pix_format structure has been extended with fields that were * not previously required to be set to zero by applications. The priv @@ -1043,8 +1052,16 @@ static void v4l_sanitize_format(struct v4l2_format *fmt) fmt->type != V4L2_BUF_TYPE_VIDEO_OUTPUT) return; - if (fmt->fmt.pix.priv == V4L2_PIX_FMT_PRIV_MAGIC) + if (fmt->fmt.pix.priv == V4L2_PIX_FMT_PRIV_MAGIC) { + if (fmt->type != V4L2_BUF_TYPE_VIDEO_CAPTURE || + fmt->fmt.pix.flags & V4L2_PIX_FMT_FLAG_HAS_CSC) + return; + fmt->fmt.pix.colorspace = V4L2_COLORSPACE_DEFAULT; + fmt->fmt.pix.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; + fmt->fmt.pix.quantization = V4L2_QUANTIZATION_DEFAULT; + fmt->fmt.pix.xfer_func = V4L2_XFER_FUNC_DEFAULT; return; + } fmt->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC; diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index a376b351135f..51e009936aad 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -477,6 +477,13 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg) case VIDIOC_SUBDEV_S_FMT: { struct v4l2_subdev_format *format = arg; + if (!(format->format.flags & V4L2_MBUS_FRAMEFMT_HAS_CSC)) { + format->format.colorspace = V4L2_COLORSPACE_DEFAULT; + format->format.xfer_func = V4L2_XFER_FUNC_DEFAULT; + format->format.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT; + format->format.quantization = V4L2_QUANTIZATION_DEFAULT; + } + memset(format->reserved, 0, sizeof(format->reserved)); memset(format->format.reserved, 0, sizeof(format->format.reserved)); return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh->pad, format); diff --git a/include/uapi/linux/v4l2-mediabus.h b/include/uapi/linux/v4l2-mediabus.h index 123a231001a8..89ff0ad6a631 100644 --- a/include/uapi/linux/v4l2-mediabus.h +++ b/include/uapi/linux/v4l2-mediabus.h @@ -16,6 +16,8 @@ #include <linux/types.h> #include <linux/videodev2.h> +#define V4L2_MBUS_FRAMEFMT_HAS_CSC 0x0001 + /** * struct v4l2_mbus_framefmt - frame format on the media bus * @width: image width @@ -36,7 +38,8 @@ struct v4l2_mbus_framefmt { __u16 ycbcr_enc; __u16 quantization; __u16 xfer_func; - __u16 reserved[11]; + __u16 flags; + __u16 reserved[10]; }; #ifndef __KERNEL__ diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h index 03970ce30741..4410b26a7158 100644 --- a/include/uapi/linux/v4l2-subdev.h +++ b/include/uapi/linux/v4l2-subdev.h @@ -65,6 +65,8 @@ struct v4l2_subdev_crop { __u32 reserved[8]; }; +#define V4L2_SUBDEV_MBUS_CODE_CSC_YCBCR_ENC 0x00000001 +#define V4L2_SUBDEV_MBUS_CODE_CSC_QUANTIZATION 0x00000002 /** * struct v4l2_subdev_mbus_code_enum - Media bus format enumeration * @pad: pad number, as reported by the media API @@ -77,7 +79,8 @@ struct v4l2_subdev_mbus_code_enum { __u32 index; __u32 code; __u32 which; - __u32 reserved[8]; + __u32 flags; + __u32 reserved[7]; }; /** diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index 9817b7e2c968..adc9dd1080b8 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -772,6 +772,7 @@ struct v4l2_pix_format { /* Flags */ #define V4L2_PIX_FMT_FLAG_PREMUL_ALPHA 0x00000001 +#define V4L2_PIX_FMT_FLAG_HAS_CSC 0x00000002 /* * F O R M A T E N U M E R A T I O N @@ -789,6 +790,9 @@ struct v4l2_fmtdesc { #define V4L2_FMT_FLAG_EMULATED 0x0002 #define V4L2_FMT_FLAG_CONTINUOUS_BYTESTREAM 0x0004 #define V4L2_FMT_FLAG_DYN_RESOLUTION 0x0008 +#define V4L2_FMT_FLAG_CSC_YCBCR_ENC 0x0010 +#define V4L2_FMT_FLAG_CSC_HSV_ENC 0x0010 +#define V4L2_FMT_FLAG_CSC_QUANTIZATION 0x0020 /* Frame Size and frame rate enumeration */ /*