Hi Sakari,
On Wed, Dec 02, 2020 at 11:16:48PM +0200, Sakari Ailus wrote:
> Hi Jacopo,
>
> Thank you for the patchset.
>
> On Tue, Nov 10, 2020 at 06:40:32PM +0100, Jacopo Mondi wrote:
> > From: Naushir Patuck <naush@xxxxxxxxxxxxxxx>
> >
> > Add V4L2_META_FMT_CUSTOM_SENSOR_DATA format.
> >
> > This new format will be used to return camera sensor embedded data.
> >
> > Signed-off-by: Naushir Patuck <naush@xxxxxxxxxxxxxxx>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Jacopo Mondi <jacopo@xxxxxxxxxx>
> > ---
> > .../userspace-api/media/v4l/meta-formats.rst | 1 +
> > .../media/v4l/pixfmt-meta-sensor-data.rst | 24 +++++++++++++++
> > .../media/v4l/subdev-formats.rst | 29 +++++++++++++++++++
> > drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
> > include/uapi/linux/media-bus-format.h | 3 ++
> > include/uapi/linux/videodev2.h | 1 +
> > 6 files changed, 59 insertions(+)
> > create mode 100644 Documentation/userspace-api/media/v4l/pixfmt-meta-sensor-data.rst
> >
> > diff --git a/Documentation/userspace-api/media/v4l/meta-formats.rst b/Documentation/userspace-api/media/v4l/meta-formats.rst
> > index fff25357fe860..b2201d1524eb6 100644
> > --- a/Documentation/userspace-api/media/v4l/meta-formats.rst
> > +++ b/Documentation/userspace-api/media/v4l/meta-formats.rst
> > @@ -15,6 +15,7 @@ These formats are used for the :ref:`metadata` interface only.
> > pixfmt-meta-d4xx
> > pixfmt-meta-intel-ipu3
> > pixfmt-meta-rkisp1
> > + pixfmt-meta-sensor-data
> > pixfmt-meta-uvc
> > pixfmt-meta-vsp1-hgo
> > pixfmt-meta-vsp1-hgt
> > diff --git a/Documentation/userspace-api/media/v4l/pixfmt-meta-sensor-data.rst b/Documentation/userspace-api/media/v4l/pixfmt-meta-sensor-data.rst
> > new file mode 100644
> > index 0000000000000..cc3929f595389
> > --- /dev/null
> > +++ b/Documentation/userspace-api/media/v4l/pixfmt-meta-sensor-data.rst
> > @@ -0,0 +1,24 @@
> > +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later
> > +
> > +.. _v4l2-meta-fmt-sensor-data:
> > +
> > +***********************************
> > +V4L2_META_FMT_SENSOR_DATA ('SENS')
> > +***********************************
> > +
> > +Sensor Ancillary Metadata
>
> "Sensor Embedded Data"
>
> > +
> > +Description
> > +===========
> > +
> > +This format describes ancillary data generated by a camera sensor and
> > +transmitted over a stream on the camera bus. Most sensor vendors have their
> > +own custom format for this ancillary data. Some vendors follow a generic
> > +CSI-2/SMIA embedded data format as described in the `CSI-2 specification.
> > +<https://mipi.org/specifications/csi-2>`_
>
> CSI-2 or SMIA? SMIA does define how embedded data is formatted, but this
> format is specific to SMIA and may be used for other sensors that are
> somewhat aligned with SMIA (or CCS) even if not fully compliant.
>
> SMIA defines both the format of embedded data as well as its packing into
> pixels at different bit depths. I have to say I have little knowledge on
> how this works on non-compliant sensors. Do other sensors use the same
> packing? Given that this isn't standardised, I could imagine there are
> other kinds of solutions.
Even for SMIA compliant formats, the point Dave had about the format
changing when the image format change, still holds and we don't have
a model that defines how metadata streams are exposed to userspace at the
moment, so I would say we don't have enough information at this time
to really answer these questions.
>
> In order to interpret this from the user space (as well as eventually
> transferring it to system memory), I think we'd need at least the packing
> and bits per pixel information even if the data format itself remained
> opaque at that level.
I understand about the bit depth
I'm not sure what packaging information we can capture in the format
definition though.
As per my last reply
https://patchwork.linuxtv.org/project/linux-media/patch/20201102165258.408049-2-jacopo@xxxxxxxxxx/#123351
at the receiver level no information should be required buffer size
apart (and the current mechanism is not v4l2-compliant, I agree). Do
you agree ?
>
> What are the documentation requirements for embedded data that is not
> SMIA/CCS, and therefore likely undocumented in publicly available
> documentation? Driver documentation?
>
I always assumed userspace being a sensor-specific component that
knows how to interpret those data but I never questioned where that
knowledge come from. I assume from documentation and as much as I like
to have it documented somewhere, I'm not sure this can be done in any
way but best-effort (as if the documentation is public, well,
everybody can read it, if it's confidential and we require it to be
documented in the driver I don't think we'll receive it anyway).
> One alternative would be to define only SMIA/CCS compliant embedded data
> formats.
And use opaque for the others ?
Thanks
j
>
> > +
> > +The size of the embedded buffer is defined as a single line with a pixel width
> > +specified in bytes and obtained by a call to the :c:type:`VIDIOC_SUBDEV_G_FMT`
> > +ioctl on the sensor sub-device. Note that this size is fixed and cannot be
> > +modified with a call to :c:type:`VIDIOC_SUBDEV_S_FMT`.
> > +
> > diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > index 7f16cbe46e5c2..99e270bbdd223 100644
> > --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > @@ -7926,3 +7926,32 @@ The following table lists the existing metadata formats.
> > both sides of the link and the bus format is a fixed
> > metadata format that is not configurable from userspace.
> > Width and height will be set to 0 for this format.
> > +
> > +.. _v4l2-mbus-sensor-data:
> > +
> > +Sensor Ancillary Metadata Formats
> > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > +
> > +This section lists ancillary data generated by a camera sensor and
> > +transmitted over a stream on the camera bus.
> > +
> > +The following table lists the existing sensor ancillary metadata formats:
> > +
> > +.. _v4l2-mbus-pixelcode-sensor-metadata:
> > +
> > +.. tabularcolumns:: |p{8.0cm}|p{1.4cm}|p{7.7cm}|
> > +
> > +.. flat-table:: Sensor ancillary metadata formats
> > + :header-rows: 1
> > + :stub-columns: 0
> > +
> > + * - Identifier
> > + - Code
> > + - Comments
> > + * .. _MEDIA_BUS_FMT_CUSTOM_SENSOR_DATA:
> > +
> > + - MEDIA_BUS_FMT_CUSTOM_SENSOR_DATA
> > + - 0x7002
> > + - Sensor vendor specific ancillary metadata. Some vendors follow a generic
> > + CSI-2/SMIA embedded data format as described in the `CSI-2 specification.
> > + <https://mipi.org/specifications/csi-2>`_
> > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> > index f0f6906a879de..83288a00f28e4 100644
> > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > @@ -1403,6 +1403,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
> > case V4L2_META_FMT_UVC: descr = "UVC Payload Header Metadata"; break;
> > case V4L2_META_FMT_D4XX: descr = "Intel D4xx UVC Metadata"; break;
> > case V4L2_META_FMT_VIVID: descr = "Vivid Metadata"; break;
> > + case V4L2_META_FMT_SENSOR_DATA: descr = "Sensor Ancillary Metadata"; break;
> >
> > default:
> > /* Compressed formats */
> > diff --git a/include/uapi/linux/media-bus-format.h b/include/uapi/linux/media-bus-format.h
> > index 2ce3d891d3447..726557120fc03 100644
> > --- a/include/uapi/linux/media-bus-format.h
> > +++ b/include/uapi/linux/media-bus-format.h
> > @@ -164,4 +164,7 @@
> > */
> > #define MEDIA_BUS_FMT_METADATA_FIXED 0x7001
> >
> > +/* Sensor ancillary metadata formats - next is 0x7003 */
> > +#define MEDIA_BUS_FMT_CUSTOM_SENSOR_DATA 0x7002
> > +
> > #endif /* __LINUX_MEDIA_BUS_FORMAT_H */
> > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> > index 927075fa9099e..d1ef094090a3b 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -767,6 +767,7 @@ struct v4l2_pix_format {
> > #define V4L2_META_FMT_UVC v4l2_fourcc('U', 'V', 'C', 'H') /* UVC Payload Header metadata */
> > #define V4L2_META_FMT_D4XX v4l2_fourcc('D', '4', 'X', 'X') /* D4XX Payload Header metadata */
> > #define V4L2_META_FMT_VIVID v4l2_fourcc('V', 'I', 'V', 'D') /* Vivid Metadata */
> > +#define V4L2_META_FMT_SENSOR_DATA v4l2_fourcc('S', 'E', 'N', 'S') /* Sensor Ancillary metadata */
> >
> > /* priv field value to indicates that subsequent fields are valid. */
> > #define V4L2_PIX_FMT_PRIV_MAGIC 0xfeedcafe
>
> --
> Kind regards,
>
> Sakari Ailus
