| Message ID | 20230505215257.60704-8-sakari.ailus@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | Generic line based metadata support, internal pads | expand |
Hi Sakari, Thank you for the patch. On Sat, May 06, 2023 at 12:52:57AM +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 +++++++ > include/uapi/linux/videodev2.h | 10 ++++++++++ > 3 files changed, 32 insertions(+) > > diff --git a/Documentation/userspace-api/media/v4l/dev-meta.rst b/Documentation/userspace-api/media/v4l/dev-meta.rst > index 0e7e1ee1471a..7d3a64514db0 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 bytes. Valid when :c:type`v4l2_fmtdesc` This departs from pixel formats, where the width is defined in pixels. I wonder what the implications will be for userspace. Seeing one implementation, both in a kernel driver and in libcamera, will help validating the API. > + flag ``V4L2_FMT_FLAG_META_LINE_BASED`` is set, otherwise zero. See > + :c:func:`VIDIOC_ENUM_FMT`. > + * - __u32 > + - ``height`` > + - Height of a line of metadata in bytes. Valid when :c:type`v4l2_fmtdesc` The "height of a line" seems like a weird concept, especially if the height is expressed in bytes. I assume this is a bad copy&paste. > + flag ``V4L2_FMT_FLAG_META_LINE_BASED`` is set, otherwise zero. See > + :c:func:`VIDIOC_ENUM_FMT`. > + * - __u32 > + - ``bytesperlines`` > + - 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``. If we add width and height for metadata formats, does it mean that drivers have to (or can) implement VIDIOC_ENUM_FRAMESIZES ? This should be documented. > > Return Value > ============ > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index adcbdc15dcdb..3681b2c15901 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -873,6 +873,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 */ > /* > @@ -2407,10 +2408,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)); > > /**
Hi Hans, On Fri, Jun 02, 2023 at 01:50:54PM +0300, Laurent Pinchart wrote: > Hi Sakari, > > Thank you for the patch. > > On Sat, May 06, 2023 at 12:52:57AM +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 +++++++ > > include/uapi/linux/videodev2.h | 10 ++++++++++ > > 3 files changed, 32 insertions(+) > > > > diff --git a/Documentation/userspace-api/media/v4l/dev-meta.rst b/Documentation/userspace-api/media/v4l/dev-meta.rst > > index 0e7e1ee1471a..7d3a64514db0 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 bytes. Valid when :c:type`v4l2_fmtdesc` > > This departs from pixel formats, where the width is defined in pixels. I > wonder what the implications will be for userspace. Seeing one > implementation, both in a kernel driver and in libcamera, will help > validating the API. I thought of bytes when writing this but pixels (or samples) are probably a better term for this. > > > + flag ``V4L2_FMT_FLAG_META_LINE_BASED`` is set, otherwise zero. See > > + :c:func:`VIDIOC_ENUM_FMT`. > > + * - __u32 > > + - ``height`` > > + - Height of a line of metadata in bytes. Valid when :c:type`v4l2_fmtdesc` > > The "height of a line" seems like a weird concept, especially if the > height is expressed in bytes. I assume this is a bad copy&paste. Yes. I'll address these for v2. > > > + flag ``V4L2_FMT_FLAG_META_LINE_BASED`` is set, otherwise zero. See > > + :c:func:`VIDIOC_ENUM_FMT`. > > + * - __u32 > > + - ``bytesperlines`` > > + - 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``. > > If we add width and height for metadata formats, does it mean that > drivers have to (or can) implement VIDIOC_ENUM_FRAMESIZES ? This should > be documented. Good point. I don't think it's meaningful to implement that as the pipeline configuration determines this in any case. But I wonder whether this would better be documented for V4L2_CAP_IO_MC rather than for metadata formats. There's currently only a reference to this in ENUM_FMT documentation. > > > > > Return Value > > ============ > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > > index adcbdc15dcdb..3681b2c15901 100644 > > --- a/include/uapi/linux/videodev2.h > > +++ b/include/uapi/linux/videodev2.h > > @@ -873,6 +873,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 */ > > /* > > @@ -2407,10 +2408,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)); > > > > /**
diff --git a/Documentation/userspace-api/media/v4l/dev-meta.rst b/Documentation/userspace-api/media/v4l/dev-meta.rst index 0e7e1ee1471a..7d3a64514db0 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 bytes. 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`` + - Height of a line of metadata in bytes. Valid when :c:type`v4l2_fmtdesc` + flag ``V4L2_FMT_FLAG_META_LINE_BASED`` is set, otherwise zero. See + :c:func:`VIDIOC_ENUM_FMT`. + * - __u32 + - ``bytesperlines`` + - 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/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index adcbdc15dcdb..3681b2c15901 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -873,6 +873,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 */ /* @@ -2407,10 +2408,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)); /**
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 +++++++ include/uapi/linux/videodev2.h | 10 ++++++++++ 3 files changed, 32 insertions(+)