Hi Guennadi,
Thank you for the patch.
Overall this looks good to me. Please see below for one small comment.
On Wednesday, 8 November 2017 18:00:12 EET Guennadi Liakhovetski wrote:
> From: Guennadi Liakhovetski <guennadi.liakhovetski@xxxxxxxxx>
>
> Add a pixel format, used by the UVC driver to stream metadata.
>
> Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@xxxxxxxxx>
> ---
>
> v7: alphabetic order, update documentation.
>
> Documentation/media/uapi/v4l/meta-formats.rst | 1 +
> Documentation/media/uapi/v4l/pixfmt-meta-uvc.rst | 50 +++++++++++++++++++++
> include/uapi/linux/videodev2.h | 1 +
> 3 files changed, 52 insertions(+)
> create mode 100644 Documentation/media/uapi/v4l/pixfmt-meta-uvc.rst
>
> diff --git a/Documentation/media/uapi/v4l/meta-formats.rst
> b/Documentation/media/uapi/v4l/meta-formats.rst index 01e24e3..0c4e1ec
> 100644
> --- a/Documentation/media/uapi/v4l/meta-formats.rst
> +++ b/Documentation/media/uapi/v4l/meta-formats.rst
> @@ -12,5 +12,6 @@ These formats are used for the :ref:`metadata` interface
> only.
> .. toctree::
> :maxdepth: 1
>
> + pixfmt-meta-uvc
> pixfmt-meta-vsp1-hgo
> pixfmt-meta-vsp1-hgt
> diff --git a/Documentation/media/uapi/v4l/pixfmt-meta-uvc.rst
> b/Documentation/media/uapi/v4l/pixfmt-meta-uvc.rst new file mode 100644
> index 0000000..06f603c
> --- /dev/null
> +++ b/Documentation/media/uapi/v4l/pixfmt-meta-uvc.rst
> @@ -0,0 +1,50 @@
> +.. -*- coding: utf-8; mode: rst -*-
> +
> +.. _v4l2-meta-fmt-uvc:
> +
> +*******************************
> +V4L2_META_FMT_UVC ('UVCH')
> +*******************************
> +
> +UVC Payload Header Data
> +
> +
> +Description
> +===========
> +
> +This format describes standard UVC metadata, extracted from UVC packet
> headers +and provided by the UVC driver through metadata video nodes. That
> data includes +exact copies of the standard part of UVC Payload Header
> contents and auxiliary +timing information, required for precise
> interpretation of timestamps, contained +in those headers. See section
> "2.4.3.3 Video and Still Image Payload Headers" of +the "UVC 1.5 Class
> specification" for details.
> +
> +Each UVC payload header can be between 2 and 12 bytes large. Buffers can
> contain +multiple headers, if multiple such headers have been transmitted
> by the camera +for the respective frame. However, headers, containing no
> useful information, +e.g. those without the SCR field or with that field
> identical to the previous +header, will be dropped by the driver.
If the driver receives too many headers with different SCR (more than the
buffer can hold for instance) it will have to drop some of them. The simplest
implementation would be to start dropping them when the buffer is full, but
I'd like to leave room for the driver to be a bit more clever and drop headers
that have a SCR too close to the previous one for instance. I propose wording
the above paragraph as follows.
"Each UVC payload header can be between 2 and 12 bytes large. Buffers can
contain multiple headers, if multiple such headers have been transmitted
by the camera for the respective frame. However, the driver may drop headers
when the buffer is full, when they contain no useful information (e.g. those
without the SCR field or with that field identical to the previous header), or
generally to perform rate limiting when the device sends a large number of
headers".
If you're fine with this there's no need to resent, I can update the
documentation when applying, and
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> +Each individual block contains the following fields:
> +
> +.. flat-table:: UVC Metadata Block
> + :widths: 1 4
> + :header-rows: 1
> + :stub-columns: 0
> +
> + * - Field
> + - Description
> + * - __u64 ts;
> + - system timestamp in host byte order, measured by the driver upon
> + reception of the payload
> + * - __u16 sof;
> + - USB Frame Number in host byte order, also obtained by the driver as
> + close as possible to the above timestamp to enable correlation
> between + them
> + * - :cspan:`1` *The rest is an exact copy of the UVC payload header:*
> + * - __u8 length;
> + - length of the rest of the block, including this field
> + * - __u8 flags;
> + - Flags, indicating presence of other standard UVC fields
> + * - __u8 buf[];
> + - The rest of the header, possibly including UVC PTS and SCR fields
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 185d6a0..0d07b2d 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -687,6 +687,7 @@ struct v4l2_pix_format {
> /* Meta-data formats */
> #define V4L2_META_FMT_VSP1_HGO v4l2_fourcc('V', 'S', 'P', 'H') /* R-Car
> VSP1 1-D Histogram */ #define V4L2_META_FMT_VSP1_HGT v4l2_fourcc('V',
> 'S', 'P', 'T') /* R-Car VSP1 2-D Histogram */ +#define V4L2_META_FMT_UVC
> v4l2_fourcc('U', 'V', 'C', 'H') /* UVC Payload Header metadata */
>
> /* priv field value to indicates that subsequent fields are valid. */
> #define V4L2_PIX_FMT_PRIV_MAGIC 0xfeedcafe
--
Regards,
Laurent Pinchart
