From: Stuart Henderson <stu@spacehopper.org>
Subject: Re: sys/uvideo: bypass unknown pixelformat to consumer
To: tech@openbsd.org, mglocker@openbsd.org
Date: Thu, 9 Jan 2025 18:10:50 +0000

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

> @@ -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

> +
>  #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
>