On Thu, 09 Jan 2025 19:10:50 +0100,
Stuart Henderson <stu@spacehopper.org> wrote:
> 
> this generally seems reasonable to me.
> 
> a couple of nits,
> 
> > +const struct uvideo_map_fmts {
> > +	uint8_t		guidFormat[16];
> > +	uint32_t	pixelformat;
> > +} uvideo_map_fmts[] = {
> > +	{ UVIDEO_FORMAT_GUID_YUY2, V4L2_PIX_FMT_YUYV },
> > +
> > +	{ UVIDEO_FORMAT_GUID_YV12, V4L2_PIX_FMT_YVU420 },
> > +
> > +	{ UVIDEO_FORMAT_GUID_I420, V4L2_PIX_FMT_YUV420 },
> > +
> > +	{ UVIDEO_FORMAT_GUID_Y800, V4L2_PIX_FMT_GREY },
> > +
> > +	{ UVIDEO_FORMAT_GUID_Y8, V4L2_PIX_FMT_GREY },
> > +
> > +	{ UVIDEO_FORMAT_GUID_KSMEDIA_L8_IR, V4L2_PIX_FMT_GREY },
> 
> similar tables, e.g. in usb_quirks.c, usually don't have so much
> whitespace - I think it might be better to drop the blank lines
>

Thanks.

> > @@ -295,17 +295,25 @@ struct usb_video_probe_commit {
> >  /*
> >   * USB Video Payload Uncompressed
> >   */
> > -/* Table 2-1: Compression Formats */
> 
> that comment refers to a table in the "USB Video Payload Uncompressed"
> specification (USB_Video_Payload_Uncompressed_1.5.pdf); it would
> probably be better to keep the comment
>

It reffers indeed, but it is missleading I think. The table in pdf contains
4 formats, and only two of them is here.

> > +
> >  #define	UVIDEO_FORMAT_GUID_YUY2	{			\
> > -    0x59, 0x55, 0x59, 0x32, 0x00, 0x00, 0x10, 0x00,	\
> > +    'Y',  'U',  'Y',  '2',  '\0', 0x00, 0x10, 0x00,	\
> > +    0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71 }
> > +
> > +#define	UVIDEO_FORMAT_GUID_YV12	{			\
> > +    'Y',  'V',  '1',  '2',  '\0', 0x00, 0x10, 0x00,	\
> > +    0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71 }
> > +
> > +#define	UVIDEO_FORMAT_GUID_I420	{			\
> > +    'I',  '4',  '2',  '0',  '\0', 0x00, 0x10, 0x00,	\
> >      0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71 }
> > 
> > -#define	UVIDEO_FORMAT_GUID_NV12	{			\
> > -    0x4e, 0x56, 0x31, 0x32, 0x00, 0x00, 0x10, 0x00,	\
> > +#define	UVIDEO_FORMAT_GUID_Y800	{			\
> > +    'Y',  '8',  '0',  '0',  '\0', 0x00, 0x10, 0x00,	\
> >      0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71 }
> > 
> > -#define	UVIDEO_FORMAT_GUID_UYVY	{			\
> > -    0x55, 0x59, 0x56, 0x59, 0x00, 0x00, 0x10, 0x00,	\
> > +#define	UVIDEO_FORMAT_GUID_Y8	{			\
> > +    'Y',  '8',  ' ',  ' ',  '\0', 0x00, 0x10, 0x00,	\
> >      0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71 }
> > 
> >  #define	UVIDEO_FORMAT_GUID_KSMEDIA_L8_IR	{	\
> > 
> > 
> > --
> > wbr, Kirill
> > 
> 

-- 
wbr, Kirill