Re: [PATCH 4/6 v5] uvcvideo: add a metadata device node

[Guennadi Liakhovetski <g.liakhovetski@gmx.de>]

Hi Hans,

On Fri, 28 Jul 2017, Hans Verkuil wrote:

> On 07/28/2017 02:33 PM, Guennadi Liakhovetski wrote:

[snip]

> > +/**
> > + * 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.

It worked for me so far on an x86-64 system, I was able to access buffer 
data correctly, but yes, it's probably safer to use __packed.

> BTW: __u8 for length? The payload can never be longer in UVC?

No, it cannot. That's exactly how UVC defines it.

Thanks
Guennadi

> 
> Regards,
> 
> 	Hans
> 
> > +	__u8 buf[];
> > +};
> > +
> >  #endif
> > 
>