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@xxxxxxxxxxxxxxx>
> > > > ---
> > > > .../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 ?
>
> 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));
> > > >
> > > > /**
>
> --
> Regards,
>
> Laurent Pinchart
