linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hans Verkuil <hverkuil@xs4all.nl>
To: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>,
	linux-media@vger.kernel.org, helen.koike@collabora.com,
	ezequiel@collabora.com, kernel@collabora.com, dafna3@gmail.com,
	laurent.pinchart@ideasonboard.com,
	linux-rockchip@lists.infradead.org, sakari.ailus@linux.intel.com
Cc: Philipp Zabel <p.zabel@pengutronix.de>,
	Hans Verkuil <HansVerkuil@cisco.com>
Subject: Re: [RFC v3 1/2] v4l2: add support for colorspace conversion for video capture
Date: Thu, 7 May 2020 15:00:30 +0200	[thread overview]
Message-ID: <f8549b52-4b16-06f4-a017-99c521a822a7@xs4all.nl> (raw)
In-Reply-To: <20200416145605.12399-1-dafna.hirschfeld@collabora.com>

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

  parent reply	other threads:[~2020-05-07 13:00 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-16 14:56 [RFC v3 1/2] v4l2: add support for colorspace conversion for video capture Dafna Hirschfeld
2020-04-16 14:56 ` [RFC v3 2/2] media: staging: rkisp1: allow quantization conversion from userspace for isp source pad Dafna Hirschfeld
2020-06-04 17:54   ` Tomasz Figa
2020-06-04 19:11     ` Dafna Hirschfeld
2020-06-10 12:08       ` Tomasz Figa
2020-06-18 17:27         ` Tomasz Figa
2020-06-18 18:26           ` Dafna Hirschfeld
2020-06-18 18:49             ` Tomasz Figa
2020-06-18 18:54               ` Dafna Hirschfeld
2020-06-18 19:25                 ` Tomasz Figa
2020-06-26  0:15                   ` Helen Koike
2020-06-28 13:10                     ` Tomasz Figa
2020-06-28 13:15                       ` Laurent Pinchart
2020-06-28 14:59                         ` Tomasz Figa
2020-06-28 19:34                           ` Dafna Hirschfeld
2020-06-29 15:50                             ` Helen Koike
2020-06-29 23:08                               ` Laurent Pinchart
2020-06-30  9:36                                 ` Dafna Hirschfeld
2020-07-01 12:52                                   ` Laurent Pinchart
2020-07-01 14:41                                     ` Dafna Hirschfeld
2020-07-02 18:43                                       ` Laurent Pinchart
2020-08-10 12:33                                     ` Dafna Hirschfeld
2020-08-10 13:15                                       ` Dafna Hirschfeld
2020-08-10 13:29                                         ` Laurent Pinchart
2020-06-28 20:27                           ` Laurent Pinchart
2020-06-28 21:33                             ` Tomasz Figa
2020-06-28 21:37                               ` Laurent Pinchart
2020-06-29 12:08                                 ` Dafna Hirschfeld
2020-06-29 12:21                                   ` Laurent Pinchart
2020-06-29 12:29                                     ` Dafna Hirschfeld
2020-06-29 12:33                                       ` Laurent Pinchart
2020-06-26  0:01             ` Helen Koike
2020-05-07 13:00 ` Hans Verkuil [this message]
2020-05-25 13:56   ` [RFC v3 1/2] v4l2: add support for colorspace conversion for video capture Dafna Hirschfeld
2020-05-25 14:25     ` Hans Verkuil
2020-05-26 12:36       ` Dafna Hirschfeld
2020-05-26 13:47         ` Hans Verkuil
2020-06-04 17:39 ` Tomasz Figa
2020-06-05 10:11   ` Dafna Hirschfeld
2020-06-10 13:34     ` Tomasz Figa
2020-06-30 16:11       ` Dafna Hirschfeld
2020-06-30 16:31         ` Tomasz Figa
2020-06-30 17:43           ` Laurent Pinchart
2020-06-30 18:02             ` Tomasz Figa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f8549b52-4b16-06f4-a017-99c521a822a7@xs4all.nl \
    --to=hverkuil@xs4all.nl \
    --cc=HansVerkuil@cisco.com \
    --cc=dafna.hirschfeld@collabora.com \
    --cc=dafna3@gmail.com \
    --cc=ezequiel@collabora.com \
    --cc=helen.koike@collabora.com \
    --cc=kernel@collabora.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=p.zabel@pengutronix.de \
    --cc=sakari.ailus@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).