Hi Guennadi,
On Wednesday, 18 October 2017 10:32:10 EEST Guennadi Liakhovetski wrote:
> 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?
The structure size is 12 bytes on x86-32 and 16 bytes on x86-64, arm32 and
arm64 (using the GNU EABI).
Given that the structure is meant to describe the content of the buffer, it
would likely be better to make it packed.
> > 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.
>
> >> + __u8 buf[];
> >> +};
> >> +
> >>
> >> #endif
--
Regards,
Laurent Pinchart
