From: Kirill A. Korinsky Subject: Re: sys/uvideo: bypass unknown pixelformat to consumer To: tech@openbsd.org, mglocker@openbsd.org Date: Thu, 09 Jan 2025 19:26:51 +0100 On Thu, 09 Jan 2025 19:10:50 +0100, Stuart Henderson 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