diff mbox series

[v3,08/10] media: v4l: Support line-based metadata capture

Message ID 20230808075538.3043934-9-sakari.ailus@linux.intel.com
State Superseded
Headers show
Series Generic line based metadata support, internal pads | expand

Commit Message

Sakari Ailus Aug. 8, 2023, 7:55 a.m. UTC
many camera sensors, among other devices, transmit embedded data and image
data for each CSI-2 frame. This embedded data typically contains register
configuration of the sensor that has been used to capture the image data
of the same frame.

The embedded data is received by the CSI-2 receiver and has the same
properties as the image data, including that it is line based: it has
width, height and bytesperline (stride).

Add these fields to struct v4l2_meta_format and document them.

Also add V4L2_FMT_FLAG_META_LINE_BASED to tell a given format is
line-based i.e. these fields of struct v4l2_meta_format are valid for it.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../userspace-api/media/v4l/dev-meta.rst          | 15 +++++++++++++++
 .../userspace-api/media/v4l/vidioc-enum-fmt.rst   |  7 +++++++
 .../media/videodev2.h.rst.exceptions              |  1 +
 drivers/media/v4l2-core/v4l2-ioctl.c              |  5 +++--
 include/uapi/linux/videodev2.h                    | 10 ++++++++++
 5 files changed, 36 insertions(+), 2 deletions(-)

Comments

Jacopo Mondi Aug. 10, 2023, 3:24 p.m. UTC | #1
Hi Sakari

On Tue, Aug 08, 2023 at 10:55:36AM +0300, Sakari Ailus wrote:
> many camera sensors, among other devices, transmit embedded data and image
> data for each CSI-2 frame. This embedded data typically contains register
> configuration of the sensor that has been used to capture the image data
> of the same frame.
>
> The embedded data is received by the CSI-2 receiver and has the same
> properties as the image data, including that it is line based: it has
> width, height and bytesperline (stride).
>
> Add these fields to struct v4l2_meta_format and document them.
>
> Also add V4L2_FMT_FLAG_META_LINE_BASED to tell a given format is
> line-based i.e. these fields of struct v4l2_meta_format are valid for it.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  .../userspace-api/media/v4l/dev-meta.rst          | 15 +++++++++++++++
>  .../userspace-api/media/v4l/vidioc-enum-fmt.rst   |  7 +++++++
>  .../media/videodev2.h.rst.exceptions              |  1 +
>  drivers/media/v4l2-core/v4l2-ioctl.c              |  5 +++--
>  include/uapi/linux/videodev2.h                    | 10 ++++++++++
>  5 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/userspace-api/media/v4l/dev-meta.rst b/Documentation/userspace-api/media/v4l/dev-meta.rst
> index 0e7e1ee1471a..4b24bae6e171 100644
> --- a/Documentation/userspace-api/media/v4l/dev-meta.rst
> +++ b/Documentation/userspace-api/media/v4l/dev-meta.rst
> @@ -65,3 +65,18 @@ to 0.
>        - ``buffersize``
>        - Maximum buffer size in bytes required for data. The value is set by the
>          driver.
> +    * - __u32
> +      - ``width``
> +      - Width of a line of metadata in samples. Valid when :c:type`v4l2_fmtdesc`
> +	flag ``V4L2_FMT_FLAG_META_LINE_BASED`` is set, otherwise zero. See
> +	:c:func:`VIDIOC_ENUM_FMT`.
> +    * - __u32
> +      - ``height``
> +      - Number of rows of metadata. Valid when :c:type`v4l2_fmtdesc` flag
> +	``V4L2_FMT_FLAG_META_LINE_BASED`` is set, otherwise zero. See
> +	:c:func:`VIDIOC_ENUM_FMT`.
> +    * - __u32
> +      - ``bytesperline``
> +      - Offset in bytes between the beginning of two consecutive lines. Valid
> +	when :c:type`v4l2_fmtdesc` flag ``V4L2_FMT_FLAG_META_LINE_BASED`` is
> +	set, otherwise zero. See :c:func:`VIDIOC_ENUM_FMT`.
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> index 000c154b0f98..6d7664345a4e 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> @@ -227,6 +227,13 @@ the ``mbus_code`` field is handled differently:
>  	The application can ask to configure the quantization of the capture
>  	device when calling the :ref:`VIDIOC_S_FMT <VIDIOC_G_FMT>` ioctl with
>  	:ref:`V4L2_PIX_FMT_FLAG_SET_CSC <v4l2-pix-fmt-flag-set-csc>` set.
> +    * - ``V4L2_FMT_FLAG_META_LINE_BASED``
> +      - 0x0200
> +      - The metadata format is line-based. In this case the ``width``,
> +	``height`` and ``bytesperline`` fields of :c:type:`v4l2_meta_format` are
> +	valid. The buffer consists of ``height`` lines, each having ``width``
> +	bytes of data and offset between the beginning of each two consecutive

Isn't ``width`` in samples ?

> +	lines is ``bytesperline``.
>
>  Return Value
>  ============
> diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> index 3e58aac4ef0b..bdc628e8c1d6 100644
> --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> @@ -215,6 +215,7 @@ replace define V4L2_FMT_FLAG_CSC_XFER_FUNC fmtdesc-flags
>  replace define V4L2_FMT_FLAG_CSC_YCBCR_ENC fmtdesc-flags
>  replace define V4L2_FMT_FLAG_CSC_HSV_ENC fmtdesc-flags
>  replace define V4L2_FMT_FLAG_CSC_QUANTIZATION fmtdesc-flags
> +replace define V4L2_FMT_FLAG_META_LINE_BASED fmtdesc-flags
>
>  # V4L2 timecode types
>  replace define V4L2_TC_TYPE_24FPS timecode-type
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index fbbddc333a30..971d784e7429 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -343,8 +343,9 @@ static void v4l_print_format(const void *arg, bool write_only)
>  	case V4L2_BUF_TYPE_META_OUTPUT:
>  		meta = &p->fmt.meta;
>  		pixelformat = meta->dataformat;
> -		pr_cont(", dataformat=%p4cc, buffersize=%u\n",
> -			&pixelformat, meta->buffersize);
> +		pr_cont(", dataformat=%p4cc, buffersize=%u, width=%u, height=%u, bytesperline=%u\n",
> +			&pixelformat, meta->buffersize, meta->width,
> +			meta->height, meta->bytesperline);
>  		break;
>  	}
>  }
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index b4284a564025..d26c0650c6a7 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -877,6 +877,7 @@ struct v4l2_fmtdesc {
>  #define V4L2_FMT_FLAG_CSC_YCBCR_ENC		0x0080
>  #define V4L2_FMT_FLAG_CSC_HSV_ENC		V4L2_FMT_FLAG_CSC_YCBCR_ENC
>  #define V4L2_FMT_FLAG_CSC_QUANTIZATION		0x0100
> +#define V4L2_FMT_FLAG_META_LINE_BASED		0x0200
>
>  	/* Frame Size and frame rate enumeration */
>  /*
> @@ -2420,10 +2421,19 @@ struct v4l2_sdr_format {
>   * struct v4l2_meta_format - metadata format definition
>   * @dataformat:		little endian four character code (fourcc)
>   * @buffersize:		maximum size in bytes required for data
> + * @width:		number of bytes of data per line (valid for line based

I'm a bit confused here as well, isn't width in samples ?

> + *			formats only, see format documentation)
> + * @height:		number of lines of data per buffer (valid for line based
> + *			formats only)
> + * @bytesperline:	offset between the beginnings of two adjacent lines in
> + *			bytes (valid for line based formats only)
>   */
>  struct v4l2_meta_format {
>  	__u32				dataformat;
>  	__u32				buffersize;
> +	__u32				width;
> +	__u32				height;
> +	__u32				bytesperline;
>  } __attribute__ ((packed));
>
>  /**
> --
> 2.39.2
>
Sakari Ailus Aug. 14, 2023, 11:02 a.m. UTC | #2
Hi Jacopo,

On Thu, Aug 10, 2023 at 05:24:14PM +0200, Jacopo Mondi wrote:
> Hi Sakari
> 
> On Tue, Aug 08, 2023 at 10:55:36AM +0300, Sakari Ailus wrote:
> > many camera sensors, among other devices, transmit embedded data and image
> > data for each CSI-2 frame. This embedded data typically contains register
> > configuration of the sensor that has been used to capture the image data
> > of the same frame.
> >
> > The embedded data is received by the CSI-2 receiver and has the same
> > properties as the image data, including that it is line based: it has
> > width, height and bytesperline (stride).
> >
> > Add these fields to struct v4l2_meta_format and document them.
> >
> > Also add V4L2_FMT_FLAG_META_LINE_BASED to tell a given format is
> > line-based i.e. these fields of struct v4l2_meta_format are valid for it.
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  .../userspace-api/media/v4l/dev-meta.rst          | 15 +++++++++++++++
> >  .../userspace-api/media/v4l/vidioc-enum-fmt.rst   |  7 +++++++
> >  .../media/videodev2.h.rst.exceptions              |  1 +
> >  drivers/media/v4l2-core/v4l2-ioctl.c              |  5 +++--
> >  include/uapi/linux/videodev2.h                    | 10 ++++++++++
> >  5 files changed, 36 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/userspace-api/media/v4l/dev-meta.rst b/Documentation/userspace-api/media/v4l/dev-meta.rst
> > index 0e7e1ee1471a..4b24bae6e171 100644
> > --- a/Documentation/userspace-api/media/v4l/dev-meta.rst
> > +++ b/Documentation/userspace-api/media/v4l/dev-meta.rst
> > @@ -65,3 +65,18 @@ to 0.
> >        - ``buffersize``
> >        - Maximum buffer size in bytes required for data. The value is set by the
> >          driver.
> > +    * - __u32
> > +      - ``width``
> > +      - Width of a line of metadata in samples. Valid when :c:type`v4l2_fmtdesc`
> > +	flag ``V4L2_FMT_FLAG_META_LINE_BASED`` is set, otherwise zero. See
> > +	:c:func:`VIDIOC_ENUM_FMT`.
> > +    * - __u32
> > +      - ``height``
> > +      - Number of rows of metadata. Valid when :c:type`v4l2_fmtdesc` flag
> > +	``V4L2_FMT_FLAG_META_LINE_BASED`` is set, otherwise zero. See
> > +	:c:func:`VIDIOC_ENUM_FMT`.
> > +    * - __u32
> > +      - ``bytesperline``
> > +      - Offset in bytes between the beginning of two consecutive lines. Valid
> > +	when :c:type`v4l2_fmtdesc` flag ``V4L2_FMT_FLAG_META_LINE_BASED`` is
> > +	set, otherwise zero. See :c:func:`VIDIOC_ENUM_FMT`.
> > diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> > index 000c154b0f98..6d7664345a4e 100644
> > --- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> > +++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> > @@ -227,6 +227,13 @@ the ``mbus_code`` field is handled differently:
> >  	The application can ask to configure the quantization of the capture
> >  	device when calling the :ref:`VIDIOC_S_FMT <VIDIOC_G_FMT>` ioctl with
> >  	:ref:`V4L2_PIX_FMT_FLAG_SET_CSC <v4l2-pix-fmt-flag-set-csc>` set.
> > +    * - ``V4L2_FMT_FLAG_META_LINE_BASED``
> > +      - 0x0200
> > +      - The metadata format is line-based. In this case the ``width``,
> > +	``height`` and ``bytesperline`` fields of :c:type:`v4l2_meta_format` are
> > +	valid. The buffer consists of ``height`` lines, each having ``width``
> > +	bytes of data and offset between the beginning of each two consecutive
> 
> Isn't ``width`` in samples ?

Indeed, it's better to refer to samples for clarity. I'll fix for v4.

I'll also add bytesperline is in bytes (and not in samples).

> 
> > +	lines is ``bytesperline``.
> >
> >  Return Value
> >  ============
> > diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> > index 3e58aac4ef0b..bdc628e8c1d6 100644
> > --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> > +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> > @@ -215,6 +215,7 @@ replace define V4L2_FMT_FLAG_CSC_XFER_FUNC fmtdesc-flags
> >  replace define V4L2_FMT_FLAG_CSC_YCBCR_ENC fmtdesc-flags
> >  replace define V4L2_FMT_FLAG_CSC_HSV_ENC fmtdesc-flags
> >  replace define V4L2_FMT_FLAG_CSC_QUANTIZATION fmtdesc-flags
> > +replace define V4L2_FMT_FLAG_META_LINE_BASED fmtdesc-flags
> >
> >  # V4L2 timecode types
> >  replace define V4L2_TC_TYPE_24FPS timecode-type
> > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> > index fbbddc333a30..971d784e7429 100644
> > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > @@ -343,8 +343,9 @@ static void v4l_print_format(const void *arg, bool write_only)
> >  	case V4L2_BUF_TYPE_META_OUTPUT:
> >  		meta = &p->fmt.meta;
> >  		pixelformat = meta->dataformat;
> > -		pr_cont(", dataformat=%p4cc, buffersize=%u\n",
> > -			&pixelformat, meta->buffersize);
> > +		pr_cont(", dataformat=%p4cc, buffersize=%u, width=%u, height=%u, bytesperline=%u\n",
> > +			&pixelformat, meta->buffersize, meta->width,
> > +			meta->height, meta->bytesperline);
> >  		break;
> >  	}
> >  }
> > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> > index b4284a564025..d26c0650c6a7 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -877,6 +877,7 @@ struct v4l2_fmtdesc {
> >  #define V4L2_FMT_FLAG_CSC_YCBCR_ENC		0x0080
> >  #define V4L2_FMT_FLAG_CSC_HSV_ENC		V4L2_FMT_FLAG_CSC_YCBCR_ENC
> >  #define V4L2_FMT_FLAG_CSC_QUANTIZATION		0x0100
> > +#define V4L2_FMT_FLAG_META_LINE_BASED		0x0200
> >
> >  	/* Frame Size and frame rate enumeration */
> >  /*
> > @@ -2420,10 +2421,19 @@ struct v4l2_sdr_format {
> >   * struct v4l2_meta_format - metadata format definition
> >   * @dataformat:		little endian four character code (fourcc)
> >   * @buffersize:		maximum size in bytes required for data
> > + * @width:		number of bytes of data per line (valid for line based
> 
> I'm a bit confused here as well, isn't width in samples ?

I'll change this one as well.

> 
> > + *			formats only, see format documentation)
> > + * @height:		number of lines of data per buffer (valid for line based
> > + *			formats only)
> > + * @bytesperline:	offset between the beginnings of two adjacent lines in
> > + *			bytes (valid for line based formats only)
> >   */
> >  struct v4l2_meta_format {
> >  	__u32				dataformat;
> >  	__u32				buffersize;
> > +	__u32				width;
> > +	__u32				height;
> > +	__u32				bytesperline;
> >  } __attribute__ ((packed));
> >
> >  /**
Laurent Pinchart Sept. 5, 2023, 5:15 p.m. UTC | #3
Hi Sakari,

On Mon, Aug 14, 2023 at 11:02:40AM +0000, Sakari Ailus wrote:
> On Thu, Aug 10, 2023 at 05:24:14PM +0200, Jacopo Mondi wrote:
> > On Tue, Aug 08, 2023 at 10:55:36AM +0300, Sakari Ailus wrote:
> > > many camera sensors, among other devices, transmit embedded data and image

s/many/Many/

> > > data for each CSI-2 frame. This embedded data typically contains register
> > > configuration of the sensor that has been used to capture the image data
> > > of the same frame.
> > >
> > > The embedded data is received by the CSI-2 receiver and has the same
> > > properties as the image data, including that it is line based: it has
> > > width, height and bytesperline (stride).
> > >
> > > Add these fields to struct v4l2_meta_format and document them.
> > >
> > > Also add V4L2_FMT_FLAG_META_LINE_BASED to tell a given format is
> > > line-based i.e. these fields of struct v4l2_meta_format are valid for it.
> > >
> > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > ---
> > >  .../userspace-api/media/v4l/dev-meta.rst          | 15 +++++++++++++++
> > >  .../userspace-api/media/v4l/vidioc-enum-fmt.rst   |  7 +++++++
> > >  .../media/videodev2.h.rst.exceptions              |  1 +
> > >  drivers/media/v4l2-core/v4l2-ioctl.c              |  5 +++--
> > >  include/uapi/linux/videodev2.h                    | 10 ++++++++++
> > >  5 files changed, 36 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/userspace-api/media/v4l/dev-meta.rst b/Documentation/userspace-api/media/v4l/dev-meta.rst
> > > index 0e7e1ee1471a..4b24bae6e171 100644
> > > --- a/Documentation/userspace-api/media/v4l/dev-meta.rst
> > > +++ b/Documentation/userspace-api/media/v4l/dev-meta.rst
> > > @@ -65,3 +65,18 @@ to 0.
> > >        - ``buffersize``
> > >        - Maximum buffer size in bytes required for data. The value is set by the
> > >          driver.
> > > +    * - __u32
> > > +      - ``width``
> > > +      - Width of a line of metadata in samples. Valid when :c:type`v4l2_fmtdesc`
> > > +	flag ``V4L2_FMT_FLAG_META_LINE_BASED`` is set, otherwise zero. See
> > > +	:c:func:`VIDIOC_ENUM_FMT`.
> > > +    * - __u32
> > > +      - ``height``
> > > +      - Number of rows of metadata. Valid when :c:type`v4l2_fmtdesc` flag
> > > +	``V4L2_FMT_FLAG_META_LINE_BASED`` is set, otherwise zero. See
> > > +	:c:func:`VIDIOC_ENUM_FMT`.
> > > +    * - __u32
> > > +      - ``bytesperline``
> > > +      - Offset in bytes between the beginning of two consecutive lines. Valid
> > > +	when :c:type`v4l2_fmtdesc` flag ``V4L2_FMT_FLAG_META_LINE_BASED`` is
> > > +	set, otherwise zero. See :c:func:`VIDIOC_ENUM_FMT`.
> > > diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> > > index 000c154b0f98..6d7664345a4e 100644
> > > --- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> > > +++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> > > @@ -227,6 +227,13 @@ the ``mbus_code`` field is handled differently:
> > >  	The application can ask to configure the quantization of the capture
> > >  	device when calling the :ref:`VIDIOC_S_FMT <VIDIOC_G_FMT>` ioctl with
> > >  	:ref:`V4L2_PIX_FMT_FLAG_SET_CSC <v4l2-pix-fmt-flag-set-csc>` set.
> > > +    * - ``V4L2_FMT_FLAG_META_LINE_BASED``
> > > +      - 0x0200
> > > +      - The metadata format is line-based. In this case the ``width``,
> > > +	``height`` and ``bytesperline`` fields of :c:type:`v4l2_meta_format` are
> > > +	valid. The buffer consists of ``height`` lines, each having ``width``
> > > +	bytes of data and offset between the beginning of each two consecutive
> > 
> > Isn't ``width`` in samples ?
> 
> Indeed, it's better to refer to samples for clarity. I'll fix for v4.

How do you define a "sample" in this case ? I wonder if it wouldn't be
simpler for both userspace and kernel drivers if the width was specified
in bytes, including the padding bytes.

We need an implementation here to test it out. The good news is that I'm
working on it, the bad news is that I'm also busy with other things.

> I'll also add bytesperline is in bytes (and not in samples).

Thanks for not messing up (more than needed) with my mental health by
not specifying bytesperline in something else than bytes :-)

> > > +	lines is ``bytesperline``.
> > >
> > >  Return Value
> > >  ============
> > > diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> > > index 3e58aac4ef0b..bdc628e8c1d6 100644
> > > --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> > > +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> > > @@ -215,6 +215,7 @@ replace define V4L2_FMT_FLAG_CSC_XFER_FUNC fmtdesc-flags
> > >  replace define V4L2_FMT_FLAG_CSC_YCBCR_ENC fmtdesc-flags
> > >  replace define V4L2_FMT_FLAG_CSC_HSV_ENC fmtdesc-flags
> > >  replace define V4L2_FMT_FLAG_CSC_QUANTIZATION fmtdesc-flags
> > > +replace define V4L2_FMT_FLAG_META_LINE_BASED fmtdesc-flags
> > >
> > >  # V4L2 timecode types
> > >  replace define V4L2_TC_TYPE_24FPS timecode-type
> > > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> > > index fbbddc333a30..971d784e7429 100644
> > > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > > @@ -343,8 +343,9 @@ static void v4l_print_format(const void *arg, bool write_only)
> > >  	case V4L2_BUF_TYPE_META_OUTPUT:
> > >  		meta = &p->fmt.meta;
> > >  		pixelformat = meta->dataformat;
> > > -		pr_cont(", dataformat=%p4cc, buffersize=%u\n",
> > > -			&pixelformat, meta->buffersize);
> > > +		pr_cont(", dataformat=%p4cc, buffersize=%u, width=%u, height=%u, bytesperline=%u\n",
> > > +			&pixelformat, meta->buffersize, meta->width,
> > > +			meta->height, meta->bytesperline);
> > >  		break;
> > >  	}
> > >  }
> > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> > > index b4284a564025..d26c0650c6a7 100644
> > > --- a/include/uapi/linux/videodev2.h
> > > +++ b/include/uapi/linux/videodev2.h
> > > @@ -877,6 +877,7 @@ struct v4l2_fmtdesc {
> > >  #define V4L2_FMT_FLAG_CSC_YCBCR_ENC		0x0080
> > >  #define V4L2_FMT_FLAG_CSC_HSV_ENC		V4L2_FMT_FLAG_CSC_YCBCR_ENC
> > >  #define V4L2_FMT_FLAG_CSC_QUANTIZATION		0x0100
> > > +#define V4L2_FMT_FLAG_META_LINE_BASED		0x0200
> > >
> > >  	/* Frame Size and frame rate enumeration */
> > >  /*
> > > @@ -2420,10 +2421,19 @@ struct v4l2_sdr_format {
> > >   * struct v4l2_meta_format - metadata format definition
> > >   * @dataformat:		little endian four character code (fourcc)
> > >   * @buffersize:		maximum size in bytes required for data
> > > + * @width:		number of bytes of data per line (valid for line based
> > 
> > I'm a bit confused here as well, isn't width in samples ?
> 
> I'll change this one as well.
> 
> > > + *			formats only, see format documentation)
> > > + * @height:		number of lines of data per buffer (valid for line based
> > > + *			formats only)
> > > + * @bytesperline:	offset between the beginnings of two adjacent lines in
> > > + *			bytes (valid for line based formats only)
> > >   */
> > >  struct v4l2_meta_format {
> > >  	__u32				dataformat;
> > >  	__u32				buffersize;
> > > +	__u32				width;
> > > +	__u32				height;
> > > +	__u32				bytesperline;
> > >  } __attribute__ ((packed));
> > >
> > >  /**
Sakari Ailus Sept. 6, 2023, 12:24 p.m. UTC | #4
Hi Laurent,

On Wed, Sep 06, 2023 at 09:21:42AM +0200, Jacopo Mondi wrote:
> Hi Laurent
> 
> On Tue, Sep 05, 2023 at 08:15:33PM +0300, Laurent Pinchart wrote:
> > Hi Sakari,
> >
> > On Mon, Aug 14, 2023 at 11:02:40AM +0000, Sakari Ailus wrote:
> > > On Thu, Aug 10, 2023 at 05:24:14PM +0200, Jacopo Mondi wrote:
> > > > On Tue, Aug 08, 2023 at 10:55:36AM +0300, Sakari Ailus wrote:
> > > > > many camera sensors, among other devices, transmit embedded data and image
> >
> > s/many/Many/
> >
> > > > > data for each CSI-2 frame. This embedded data typically contains register
> > > > > configuration of the sensor that has been used to capture the image data
> > > > > of the same frame.
> > > > >
> > > > > The embedded data is received by the CSI-2 receiver and has the same
> > > > > properties as the image data, including that it is line based: it has
> > > > > width, height and bytesperline (stride).
> > > > >
> > > > > Add these fields to struct v4l2_meta_format and document them.
> > > > >
> > > > > Also add V4L2_FMT_FLAG_META_LINE_BASED to tell a given format is
> > > > > line-based i.e. these fields of struct v4l2_meta_format are valid for it.
> > > > >
> > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > > ---
> > > > >  .../userspace-api/media/v4l/dev-meta.rst          | 15 +++++++++++++++
> > > > >  .../userspace-api/media/v4l/vidioc-enum-fmt.rst   |  7 +++++++
> > > > >  .../media/videodev2.h.rst.exceptions              |  1 +
> > > > >  drivers/media/v4l2-core/v4l2-ioctl.c              |  5 +++--
> > > > >  include/uapi/linux/videodev2.h                    | 10 ++++++++++
> > > > >  5 files changed, 36 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/Documentation/userspace-api/media/v4l/dev-meta.rst b/Documentation/userspace-api/media/v4l/dev-meta.rst
> > > > > index 0e7e1ee1471a..4b24bae6e171 100644
> > > > > --- a/Documentation/userspace-api/media/v4l/dev-meta.rst
> > > > > +++ b/Documentation/userspace-api/media/v4l/dev-meta.rst
> > > > > @@ -65,3 +65,18 @@ to 0.
> > > > >        - ``buffersize``
> > > > >        - Maximum buffer size in bytes required for data. The value is set by the
> > > > >          driver.
> > > > > +    * - __u32
> > > > > +      - ``width``
> > > > > +      - Width of a line of metadata in samples. Valid when :c:type`v4l2_fmtdesc`
> > > > > +	flag ``V4L2_FMT_FLAG_META_LINE_BASED`` is set, otherwise zero. See
> > > > > +	:c:func:`VIDIOC_ENUM_FMT`.
> > > > > +    * - __u32
> > > > > +      - ``height``
> > > > > +      - Number of rows of metadata. Valid when :c:type`v4l2_fmtdesc` flag
> > > > > +	``V4L2_FMT_FLAG_META_LINE_BASED`` is set, otherwise zero. See
> > > > > +	:c:func:`VIDIOC_ENUM_FMT`.
> > > > > +    * - __u32
> > > > > +      - ``bytesperline``
> > > > > +      - Offset in bytes between the beginning of two consecutive lines. Valid
> > > > > +	when :c:type`v4l2_fmtdesc` flag ``V4L2_FMT_FLAG_META_LINE_BASED`` is
> > > > > +	set, otherwise zero. See :c:func:`VIDIOC_ENUM_FMT`.
> > > > > diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> > > > > index 000c154b0f98..6d7664345a4e 100644
> > > > > --- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> > > > > +++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> > > > > @@ -227,6 +227,13 @@ the ``mbus_code`` field is handled differently:
> > > > >  	The application can ask to configure the quantization of the capture
> > > > >  	device when calling the :ref:`VIDIOC_S_FMT <VIDIOC_G_FMT>` ioctl with
> > > > >  	:ref:`V4L2_PIX_FMT_FLAG_SET_CSC <v4l2-pix-fmt-flag-set-csc>` set.
> > > > > +    * - ``V4L2_FMT_FLAG_META_LINE_BASED``
> > > > > +      - 0x0200
> > > > > +      - The metadata format is line-based. In this case the ``width``,
> > > > > +	``height`` and ``bytesperline`` fields of :c:type:`v4l2_meta_format` are
> > > > > +	valid. The buffer consists of ``height`` lines, each having ``width``
> > > > > +	bytes of data and offset between the beginning of each two consecutive
> > > >
> > > > Isn't ``width`` in samples ?
> > >
> > > Indeed, it's better to refer to samples for clarity. I'll fix for v4.
> >
> > How do you define a "sample" in this case ? I wonder if it wouldn't be
> > simpler for both userspace and kernel drivers if the width was specified
> > in bytes, including the padding bytes.
> 
> Wouldn't this make the image line length (expressed in 'samples')
> different than the embedded data line length ? Would this confuse
> userspace or is it fine ?

If padding is included to width, then the user needs to calculate how many
bytes of metadata there is, apart from the padding (which is redundant).
That value is provided to the user for this purpose --- just like for image
data.

> 
> >
> > We need an implementation here to test it out. The good news is that I'm
> > working on it, the bad news is that I'm also busy with other things.
> >
> > > I'll also add bytesperline is in bytes (and not in samples).
> >
> > Thanks for not messing up (more than needed) with my mental health by
> > not specifying bytesperline in something else than bytes :-)
> >
> 
> :)

I remember back in the Amiga days when memory was sometimes scarce, nibbles
were used. That works when bytes have an even number of bits.
Laurent Pinchart Sept. 6, 2023, 1:20 p.m. UTC | #5
On Wed, Sep 06, 2023 at 12:24:26PM +0000, Sakari Ailus wrote:
> On Wed, Sep 06, 2023 at 09:21:42AM +0200, Jacopo Mondi wrote:
> > On Tue, Sep 05, 2023 at 08:15:33PM +0300, Laurent Pinchart wrote:
> > > On Mon, Aug 14, 2023 at 11:02:40AM +0000, Sakari Ailus wrote:
> > > > On Thu, Aug 10, 2023 at 05:24:14PM +0200, Jacopo Mondi wrote:
> > > > > On Tue, Aug 08, 2023 at 10:55:36AM +0300, Sakari Ailus wrote:
> > > > > > many camera sensors, among other devices, transmit embedded data and image
> > >
> > > s/many/Many/
> > >
> > > > > > data for each CSI-2 frame. This embedded data typically contains register
> > > > > > configuration of the sensor that has been used to capture the image data
> > > > > > of the same frame.
> > > > > >
> > > > > > The embedded data is received by the CSI-2 receiver and has the same
> > > > > > properties as the image data, including that it is line based: it has
> > > > > > width, height and bytesperline (stride).
> > > > > >
> > > > > > Add these fields to struct v4l2_meta_format and document them.
> > > > > >
> > > > > > Also add V4L2_FMT_FLAG_META_LINE_BASED to tell a given format is
> > > > > > line-based i.e. these fields of struct v4l2_meta_format are valid for it.
> > > > > >
> > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > > > ---
> > > > > >  .../userspace-api/media/v4l/dev-meta.rst          | 15 +++++++++++++++
> > > > > >  .../userspace-api/media/v4l/vidioc-enum-fmt.rst   |  7 +++++++
> > > > > >  .../media/videodev2.h.rst.exceptions              |  1 +
> > > > > >  drivers/media/v4l2-core/v4l2-ioctl.c              |  5 +++--
> > > > > >  include/uapi/linux/videodev2.h                    | 10 ++++++++++
> > > > > >  5 files changed, 36 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/Documentation/userspace-api/media/v4l/dev-meta.rst b/Documentation/userspace-api/media/v4l/dev-meta.rst
> > > > > > index 0e7e1ee1471a..4b24bae6e171 100644
> > > > > > --- a/Documentation/userspace-api/media/v4l/dev-meta.rst
> > > > > > +++ b/Documentation/userspace-api/media/v4l/dev-meta.rst
> > > > > > @@ -65,3 +65,18 @@ to 0.
> > > > > >        - ``buffersize``
> > > > > >        - Maximum buffer size in bytes required for data. The value is set by the
> > > > > >          driver.
> > > > > > +    * - __u32
> > > > > > +      - ``width``
> > > > > > +      - Width of a line of metadata in samples. Valid when :c:type`v4l2_fmtdesc`
> > > > > > +	flag ``V4L2_FMT_FLAG_META_LINE_BASED`` is set, otherwise zero. See
> > > > > > +	:c:func:`VIDIOC_ENUM_FMT`.
> > > > > > +    * - __u32
> > > > > > +      - ``height``
> > > > > > +      - Number of rows of metadata. Valid when :c:type`v4l2_fmtdesc` flag
> > > > > > +	``V4L2_FMT_FLAG_META_LINE_BASED`` is set, otherwise zero. See
> > > > > > +	:c:func:`VIDIOC_ENUM_FMT`.
> > > > > > +    * - __u32
> > > > > > +      - ``bytesperline``
> > > > > > +      - Offset in bytes between the beginning of two consecutive lines. Valid
> > > > > > +	when :c:type`v4l2_fmtdesc` flag ``V4L2_FMT_FLAG_META_LINE_BASED`` is
> > > > > > +	set, otherwise zero. See :c:func:`VIDIOC_ENUM_FMT`.
> > > > > > diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> > > > > > index 000c154b0f98..6d7664345a4e 100644
> > > > > > --- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> > > > > > +++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> > > > > > @@ -227,6 +227,13 @@ the ``mbus_code`` field is handled differently:
> > > > > >  	The application can ask to configure the quantization of the capture
> > > > > >  	device when calling the :ref:`VIDIOC_S_FMT <VIDIOC_G_FMT>` ioctl with
> > > > > >  	:ref:`V4L2_PIX_FMT_FLAG_SET_CSC <v4l2-pix-fmt-flag-set-csc>` set.
> > > > > > +    * - ``V4L2_FMT_FLAG_META_LINE_BASED``
> > > > > > +      - 0x0200
> > > > > > +      - The metadata format is line-based. In this case the ``width``,
> > > > > > +	``height`` and ``bytesperline`` fields of :c:type:`v4l2_meta_format` are
> > > > > > +	valid. The buffer consists of ``height`` lines, each having ``width``
> > > > > > +	bytes of data and offset between the beginning of each two consecutive
> > > > >
> > > > > Isn't ``width`` in samples ?
> > > >
> > > > Indeed, it's better to refer to samples for clarity. I'll fix for v4.
> > >
> > > How do you define a "sample" in this case ? I wonder if it wouldn't be
> > > simpler for both userspace and kernel drivers if the width was specified
> > > in bytes, including the padding bytes.
> > 
> > Wouldn't this make the image line length (expressed in 'samples')
> > different than the embedded data line length ? Would this confuse
> > userspace or is it fine ?
> 
> If padding is included to width, then the user needs to calculate how many
> bytes of metadata there is, apart from the padding (which is redundant).
> That value is provided to the user for this purpose --- just like for image
> data.

The bytesperline value includes padding at the end of the line to
achieve a particular stride, so that doesn't tell how many bytes to
parse. If the width is specified in "samples", then the parser will need
to compute the number of bytes it spans, and then parse those bytes.

> > > We need an implementation here to test it out. The good news is that I'm
> > > working on it, the bad news is that I'm also busy with other things.
> > >
> > > > I'll also add bytesperline is in bytes (and not in samples).
> > >
> > > Thanks for not messing up (more than needed) with my mental health by
> > > not specifying bytesperline in something else than bytes :-)
> > 
> > :)
> 
> I remember back in the Amiga days when memory was sometimes scarce, nibbles
> were used. That works when bytes have an even number of bits.
Sakari Ailus Sept. 7, 2023, 8:48 a.m. UTC | #6
On Wed, Sep 06, 2023 at 12:24:26PM +0000, Sakari Ailus wrote:
> Hi Laurent,
> 
> On Wed, Sep 06, 2023 at 09:21:42AM +0200, Jacopo Mondi wrote:
> > Hi Laurent
> > 
> > On Tue, Sep 05, 2023 at 08:15:33PM +0300, Laurent Pinchart wrote:
> > > Hi Sakari,
> > >
> > > On Mon, Aug 14, 2023 at 11:02:40AM +0000, Sakari Ailus wrote:
> > > > On Thu, Aug 10, 2023 at 05:24:14PM +0200, Jacopo Mondi wrote:
> > > > > On Tue, Aug 08, 2023 at 10:55:36AM +0300, Sakari Ailus wrote:
> > > > > > many camera sensors, among other devices, transmit embedded data and image
> > >
> > > s/many/Many/
> > >
> > > > > > data for each CSI-2 frame. This embedded data typically contains register
> > > > > > configuration of the sensor that has been used to capture the image data
> > > > > > of the same frame.
> > > > > >
> > > > > > The embedded data is received by the CSI-2 receiver and has the same
> > > > > > properties as the image data, including that it is line based: it has
> > > > > > width, height and bytesperline (stride).
> > > > > >
> > > > > > Add these fields to struct v4l2_meta_format and document them.
> > > > > >
> > > > > > Also add V4L2_FMT_FLAG_META_LINE_BASED to tell a given format is
> > > > > > line-based i.e. these fields of struct v4l2_meta_format are valid for it.
> > > > > >
> > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > > > ---
> > > > > >  .../userspace-api/media/v4l/dev-meta.rst          | 15 +++++++++++++++
> > > > > >  .../userspace-api/media/v4l/vidioc-enum-fmt.rst   |  7 +++++++
> > > > > >  .../media/videodev2.h.rst.exceptions              |  1 +
> > > > > >  drivers/media/v4l2-core/v4l2-ioctl.c              |  5 +++--
> > > > > >  include/uapi/linux/videodev2.h                    | 10 ++++++++++
> > > > > >  5 files changed, 36 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/Documentation/userspace-api/media/v4l/dev-meta.rst b/Documentation/userspace-api/media/v4l/dev-meta.rst
> > > > > > index 0e7e1ee1471a..4b24bae6e171 100644
> > > > > > --- a/Documentation/userspace-api/media/v4l/dev-meta.rst
> > > > > > +++ b/Documentation/userspace-api/media/v4l/dev-meta.rst
> > > > > > @@ -65,3 +65,18 @@ to 0.
> > > > > >        - ``buffersize``
> > > > > >        - Maximum buffer size in bytes required for data. The value is set by the
> > > > > >          driver.
> > > > > > +    * - __u32
> > > > > > +      - ``width``
> > > > > > +      - Width of a line of metadata in samples. Valid when :c:type`v4l2_fmtdesc`
> > > > > > +	flag ``V4L2_FMT_FLAG_META_LINE_BASED`` is set, otherwise zero. See
> > > > > > +	:c:func:`VIDIOC_ENUM_FMT`.
> > > > > > +    * - __u32
> > > > > > +      - ``height``
> > > > > > +      - Number of rows of metadata. Valid when :c:type`v4l2_fmtdesc` flag
> > > > > > +	``V4L2_FMT_FLAG_META_LINE_BASED`` is set, otherwise zero. See
> > > > > > +	:c:func:`VIDIOC_ENUM_FMT`.
> > > > > > +    * - __u32
> > > > > > +      - ``bytesperline``
> > > > > > +      - Offset in bytes between the beginning of two consecutive lines. Valid
> > > > > > +	when :c:type`v4l2_fmtdesc` flag ``V4L2_FMT_FLAG_META_LINE_BASED`` is
> > > > > > +	set, otherwise zero. See :c:func:`VIDIOC_ENUM_FMT`.
> > > > > > diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> > > > > > index 000c154b0f98..6d7664345a4e 100644
> > > > > > --- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> > > > > > +++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> > > > > > @@ -227,6 +227,13 @@ the ``mbus_code`` field is handled differently:
> > > > > >  	The application can ask to configure the quantization of the capture
> > > > > >  	device when calling the :ref:`VIDIOC_S_FMT <VIDIOC_G_FMT>` ioctl with
> > > > > >  	:ref:`V4L2_PIX_FMT_FLAG_SET_CSC <v4l2-pix-fmt-flag-set-csc>` set.
> > > > > > +    * - ``V4L2_FMT_FLAG_META_LINE_BASED``
> > > > > > +      - 0x0200
> > > > > > +      - The metadata format is line-based. In this case the ``width``,
> > > > > > +	``height`` and ``bytesperline`` fields of :c:type:`v4l2_meta_format` are
> > > > > > +	valid. The buffer consists of ``height`` lines, each having ``width``
> > > > > > +	bytes of data and offset between the beginning of each two consecutive
> > > > >
> > > > > Isn't ``width`` in samples ?
> > > >
> > > > Indeed, it's better to refer to samples for clarity. I'll fix for v4.
> > >
> > > How do you define a "sample" in this case ? I wonder if it wouldn't be
> > > simpler for both userspace and kernel drivers if the width was specified
> > > in bytes, including the padding bytes.
> > 
> > Wouldn't this make the image line length (expressed in 'samples')
> > different than the embedded data line length ? Would this confuse
> > userspace or is it fine ?
> 
> If padding is included to width, then the user needs to calculate how many
> bytes of metadata there is, apart from the padding (which is redundant).
> That value is provided to the user for this purpose --- just like for image
> data.

Think of this: the embedded data parser is split into two, the access
function that accesses metadata bytes depending on the generic V4L2
metadata format and the rest, which cares about the specific mbus code. The
latter will need the information how many bytes of metadata there is,
instead it calculates the position in the buffer based on bytesperline,
width and the format itself.

Of course, the amount of padding can be calculated from the total amount,
by adding a second function that does it, but still it's somehow backwards
way to do that. (Makes me think of negabinaries.)
Sakari Ailus Sept. 22, 2023, 8:47 a.m. UTC | #7
Hi Laurent,

On Wed, Sep 06, 2023 at 04:20:37PM +0300, Laurent Pinchart wrote:
> On Wed, Sep 06, 2023 at 12:24:26PM +0000, Sakari Ailus wrote:
> > On Wed, Sep 06, 2023 at 09:21:42AM +0200, Jacopo Mondi wrote:
> > > On Tue, Sep 05, 2023 at 08:15:33PM +0300, Laurent Pinchart wrote:
> > > > On Mon, Aug 14, 2023 at 11:02:40AM +0000, Sakari Ailus wrote:
> > > > > On Thu, Aug 10, 2023 at 05:24:14PM +0200, Jacopo Mondi wrote:
> > > > > > On Tue, Aug 08, 2023 at 10:55:36AM +0300, Sakari Ailus wrote:
> > > > > > > many camera sensors, among other devices, transmit embedded data and image
> > > >
> > > > s/many/Many/
> > > >
> > > > > > > data for each CSI-2 frame. This embedded data typically contains register
> > > > > > > configuration of the sensor that has been used to capture the image data
> > > > > > > of the same frame.
> > > > > > >
> > > > > > > The embedded data is received by the CSI-2 receiver and has the same
> > > > > > > properties as the image data, including that it is line based: it has
> > > > > > > width, height and bytesperline (stride).
> > > > > > >
> > > > > > > Add these fields to struct v4l2_meta_format and document them.
> > > > > > >
> > > > > > > Also add V4L2_FMT_FLAG_META_LINE_BASED to tell a given format is
> > > > > > > line-based i.e. these fields of struct v4l2_meta_format are valid for it.
> > > > > > >
> > > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > > > > ---
> > > > > > >  .../userspace-api/media/v4l/dev-meta.rst          | 15 +++++++++++++++
> > > > > > >  .../userspace-api/media/v4l/vidioc-enum-fmt.rst   |  7 +++++++
> > > > > > >  .../media/videodev2.h.rst.exceptions              |  1 +
> > > > > > >  drivers/media/v4l2-core/v4l2-ioctl.c              |  5 +++--
> > > > > > >  include/uapi/linux/videodev2.h                    | 10 ++++++++++
> > > > > > >  5 files changed, 36 insertions(+), 2 deletions(-)
> > > > > > >
> > > > > > > diff --git a/Documentation/userspace-api/media/v4l/dev-meta.rst b/Documentation/userspace-api/media/v4l/dev-meta.rst
> > > > > > > index 0e7e1ee1471a..4b24bae6e171 100644
> > > > > > > --- a/Documentation/userspace-api/media/v4l/dev-meta.rst
> > > > > > > +++ b/Documentation/userspace-api/media/v4l/dev-meta.rst
> > > > > > > @@ -65,3 +65,18 @@ to 0.
> > > > > > >        - ``buffersize``
> > > > > > >        - Maximum buffer size in bytes required for data. The value is set by the
> > > > > > >          driver.
> > > > > > > +    * - __u32
> > > > > > > +      - ``width``
> > > > > > > +      - Width of a line of metadata in samples. Valid when :c:type`v4l2_fmtdesc`
> > > > > > > +	flag ``V4L2_FMT_FLAG_META_LINE_BASED`` is set, otherwise zero. See
> > > > > > > +	:c:func:`VIDIOC_ENUM_FMT`.
> > > > > > > +    * - __u32
> > > > > > > +      - ``height``
> > > > > > > +      - Number of rows of metadata. Valid when :c:type`v4l2_fmtdesc` flag
> > > > > > > +	``V4L2_FMT_FLAG_META_LINE_BASED`` is set, otherwise zero. See
> > > > > > > +	:c:func:`VIDIOC_ENUM_FMT`.
> > > > > > > +    * - __u32
> > > > > > > +      - ``bytesperline``
> > > > > > > +      - Offset in bytes between the beginning of two consecutive lines. Valid
> > > > > > > +	when :c:type`v4l2_fmtdesc` flag ``V4L2_FMT_FLAG_META_LINE_BASED`` is
> > > > > > > +	set, otherwise zero. See :c:func:`VIDIOC_ENUM_FMT`.
> > > > > > > diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> > > > > > > index 000c154b0f98..6d7664345a4e 100644
> > > > > > > --- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> > > > > > > +++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
> > > > > > > @@ -227,6 +227,13 @@ the ``mbus_code`` field is handled differently:
> > > > > > >  	The application can ask to configure the quantization of the capture
> > > > > > >  	device when calling the :ref:`VIDIOC_S_FMT <VIDIOC_G_FMT>` ioctl with
> > > > > > >  	:ref:`V4L2_PIX_FMT_FLAG_SET_CSC <v4l2-pix-fmt-flag-set-csc>` set.
> > > > > > > +    * - ``V4L2_FMT_FLAG_META_LINE_BASED``
> > > > > > > +      - 0x0200
> > > > > > > +      - The metadata format is line-based. In this case the ``width``,
> > > > > > > +	``height`` and ``bytesperline`` fields of :c:type:`v4l2_meta_format` are
> > > > > > > +	valid. The buffer consists of ``height`` lines, each having ``width``
> > > > > > > +	bytes of data and offset between the beginning of each two consecutive
> > > > > >
> > > > > > Isn't ``width`` in samples ?
> > > > >
> > > > > Indeed, it's better to refer to samples for clarity. I'll fix for v4.
> > > >
> > > > How do you define a "sample" in this case ? I wonder if it wouldn't be
> > > > simpler for both userspace and kernel drivers if the width was specified
> > > > in bytes, including the padding bytes.
> > > 
> > > Wouldn't this make the image line length (expressed in 'samples')
> > > different than the embedded data line length ? Would this confuse
> > > userspace or is it fine ?
> > 
> > If padding is included to width, then the user needs to calculate how many
> > bytes of metadata there is, apart from the padding (which is redundant).
> > That value is provided to the user for this purpose --- just like for image
> > data.
> 
> The bytesperline value includes padding at the end of the line to
> achieve a particular stride, so that doesn't tell how many bytes to
> parse. If the width is specified in "samples", then the parser will need
> to compute the number of bytes it spans, and then parse those bytes.

The parser is interested in metadata bytes only, not the padding. The
padding should be skipped by the data access function in order to avoid
complicating the parser with different padding options.

That's why width should be in data units (bytes of metadata without
padding).
diff mbox series

Patch

diff --git a/Documentation/userspace-api/media/v4l/dev-meta.rst b/Documentation/userspace-api/media/v4l/dev-meta.rst
index 0e7e1ee1471a..4b24bae6e171 100644
--- a/Documentation/userspace-api/media/v4l/dev-meta.rst
+++ b/Documentation/userspace-api/media/v4l/dev-meta.rst
@@ -65,3 +65,18 @@  to 0.
       - ``buffersize``
       - Maximum buffer size in bytes required for data. The value is set by the
         driver.
+    * - __u32
+      - ``width``
+      - Width of a line of metadata in samples. Valid when :c:type`v4l2_fmtdesc`
+	flag ``V4L2_FMT_FLAG_META_LINE_BASED`` is set, otherwise zero. See
+	:c:func:`VIDIOC_ENUM_FMT`.
+    * - __u32
+      - ``height``
+      - Number of rows of metadata. Valid when :c:type`v4l2_fmtdesc` flag
+	``V4L2_FMT_FLAG_META_LINE_BASED`` is set, otherwise zero. See
+	:c:func:`VIDIOC_ENUM_FMT`.
+    * - __u32
+      - ``bytesperline``
+      - Offset in bytes between the beginning of two consecutive lines. Valid
+	when :c:type`v4l2_fmtdesc` flag ``V4L2_FMT_FLAG_META_LINE_BASED`` is
+	set, otherwise zero. See :c:func:`VIDIOC_ENUM_FMT`.
diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
index 000c154b0f98..6d7664345a4e 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst
@@ -227,6 +227,13 @@  the ``mbus_code`` field is handled differently:
 	The application can ask to configure the quantization of the capture
 	device when calling the :ref:`VIDIOC_S_FMT <VIDIOC_G_FMT>` ioctl with
 	:ref:`V4L2_PIX_FMT_FLAG_SET_CSC <v4l2-pix-fmt-flag-set-csc>` set.
+    * - ``V4L2_FMT_FLAG_META_LINE_BASED``
+      - 0x0200
+      - The metadata format is line-based. In this case the ``width``,
+	``height`` and ``bytesperline`` fields of :c:type:`v4l2_meta_format` are
+	valid. The buffer consists of ``height`` lines, each having ``width``
+	bytes of data and offset between the beginning of each two consecutive
+	lines is ``bytesperline``.
 
 Return Value
 ============
diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
index 3e58aac4ef0b..bdc628e8c1d6 100644
--- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
+++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
@@ -215,6 +215,7 @@  replace define V4L2_FMT_FLAG_CSC_XFER_FUNC fmtdesc-flags
 replace define V4L2_FMT_FLAG_CSC_YCBCR_ENC fmtdesc-flags
 replace define V4L2_FMT_FLAG_CSC_HSV_ENC fmtdesc-flags
 replace define V4L2_FMT_FLAG_CSC_QUANTIZATION fmtdesc-flags
+replace define V4L2_FMT_FLAG_META_LINE_BASED fmtdesc-flags
 
 # V4L2 timecode types
 replace define V4L2_TC_TYPE_24FPS timecode-type
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index fbbddc333a30..971d784e7429 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -343,8 +343,9 @@  static void v4l_print_format(const void *arg, bool write_only)
 	case V4L2_BUF_TYPE_META_OUTPUT:
 		meta = &p->fmt.meta;
 		pixelformat = meta->dataformat;
-		pr_cont(", dataformat=%p4cc, buffersize=%u\n",
-			&pixelformat, meta->buffersize);
+		pr_cont(", dataformat=%p4cc, buffersize=%u, width=%u, height=%u, bytesperline=%u\n",
+			&pixelformat, meta->buffersize, meta->width,
+			meta->height, meta->bytesperline);
 		break;
 	}
 }
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index b4284a564025..d26c0650c6a7 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -877,6 +877,7 @@  struct v4l2_fmtdesc {
 #define V4L2_FMT_FLAG_CSC_YCBCR_ENC		0x0080
 #define V4L2_FMT_FLAG_CSC_HSV_ENC		V4L2_FMT_FLAG_CSC_YCBCR_ENC
 #define V4L2_FMT_FLAG_CSC_QUANTIZATION		0x0100
+#define V4L2_FMT_FLAG_META_LINE_BASED		0x0200
 
 	/* Frame Size and frame rate enumeration */
 /*
@@ -2420,10 +2421,19 @@  struct v4l2_sdr_format {
  * struct v4l2_meta_format - metadata format definition
  * @dataformat:		little endian four character code (fourcc)
  * @buffersize:		maximum size in bytes required for data
+ * @width:		number of bytes of data per line (valid for line based
+ *			formats only, see format documentation)
+ * @height:		number of lines of data per buffer (valid for line based
+ *			formats only)
+ * @bytesperline:	offset between the beginnings of two adjacent lines in
+ *			bytes (valid for line based formats only)
  */
 struct v4l2_meta_format {
 	__u32				dataformat;
 	__u32				buffersize;
+	__u32				width;
+	__u32				height;
+	__u32				bytesperline;
 } __attribute__ ((packed));
 
 /**