Hi Hans,
Thanks for your comment.
On Fri, 28 Jul 2017, Hans Verkuil wrote:
> On 07/28/2017 02:33 PM, Guennadi Liakhovetski wrote:
> > Some UVC video cameras contain metadata in their payload headers. This
> > patch extracts that data, adding more clock synchronisation information,
> > on both bulk and isochronous endpoints and makes it available to the
> > user space on a separate video node, using the V4L2_CAP_META_CAPTURE
> > capability and the V4L2_BUF_TYPE_META_CAPTURE buffer queue type. Even
> > though different cameras will have different metadata formats, we use
> > the same V4L2_META_FMT_UVC pixel format for all of them. Users have to
> > parse data, based on the specific camera model information. This
> > version of the patch only creates such metadata nodes for cameras,
> > specifying a UVC_QUIRK_METADATA_NODE quirk flag.
> >
> > Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@xxxxxxxxx>
> > ---
> > drivers/media/usb/uvc/Makefile | 2 +-
> > drivers/media/usb/uvc/uvc_ctrl.c | 3 +
> > drivers/media/usb/uvc/uvc_driver.c | 12 +++
> > drivers/media/usb/uvc/uvc_isight.c | 2 +-
> > drivers/media/usb/uvc/uvc_metadata.c | 139 +++++++++++++++++++++++++++++++++++
> > drivers/media/usb/uvc/uvc_queue.c | 41 +++++++++--
> > drivers/media/usb/uvc/uvc_video.c | 119 +++++++++++++++++++++++++++---
> > drivers/media/usb/uvc/uvcvideo.h | 17 ++++-
> > drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
> > include/uapi/linux/uvcvideo.h | 26 +++++++
> > 10 files changed, 341 insertions(+), 21 deletions(-)
> > create mode 100644 drivers/media/usb/uvc/uvc_metadata.c
[snip]
> > diff --git a/include/uapi/linux/uvcvideo.h b/include/uapi/linux/uvcvideo.h
> > index 3b08186..ffe17ec 100644
> > --- a/include/uapi/linux/uvcvideo.h
> > +++ b/include/uapi/linux/uvcvideo.h
> > @@ -67,4 +67,30 @@ struct uvc_xu_control_query {
> > #define UVCIOC_CTRL_MAP _IOWR('u', 0x20, struct uvc_xu_control_mapping)
> > #define UVCIOC_CTRL_QUERY _IOWR('u', 0x21, struct uvc_xu_control_query)
> >
> > +/*
> > + * Metadata node
> > + */
> > +
> > +/**
> > + * struct uvc_meta_buf - metadata buffer building block
> > + * @ns - system timestamp of the payload in nanoseconds
> > + * @sof - USB Frame Number
> > + * @length - length of the payload header
> > + * @flags - payload header flags
> > + * @buf - optional device-specific header data
> > + *
> > + * UVC metadata nodes fill buffers with possibly multiple instances of this
> > + * struct. The first two fields are added by the driver, they can be used for
> > + * clock synchronisation. The rest is an exact copy of a UVC payload header.
> > + * Only complete objects with complete buffers are included. Therefore it's
> > + * always sizeof(meta->ts) + sizeof(meta->sof) + meta->length bytes large.
> > + */
> > +struct uvc_meta_buf {
> > + __u64 ns;
> > + __u16 sof;
> > + __u8 length;
> > + __u8 flags;
>
> I think the struct layout is architecture dependent. I could be wrong, but I think
> for x64 there is a 4-byte hole here, but not for arm32/arm64.
>
> Please double-check the struct layout.
You mean this can be a problem for 64- / 32-bit compatibility? If the
difference is just between ARM and X86 then we don't care, do we? Or do
video buffers have to be safe for cross-system transfer?
> BTW: __u8 for length? The payload can never be longer in UVC?
No, not the payload, a single payload header cannot be larger than 255
bytes, that's right.
Thanks
Guennadi
> Regards,
>
> Hans
>
> > + __u8 buf[];
> > +};
> > +
> > #endif
> >
>
