Index | Thread | Search

From:
Kirill A. Korinsky <kirill@korins.ky>
Subject:
Re: sys/uvideo: bypass unknown pixelformat to consumer
To:
Marcus Glocker <marcus@nazgul.ch>
Cc:
tech@openbsd.org
Date:
Fri, 10 Jan 2025 23:43:40 +0100

Download raw body.

Thread
Marcus,

thanks for review.

On Fri, 10 Jan 2025 22:58:27 +0100,
Marcus Glocker <marcus@nazgul.ch> wrote:
>
> On Thu, Jan 09, 2025 at 06:38:29PM GMT, Kirill A. Korinsky wrote:
>
> > +
> > +	/* but some GUID should be mapped to other pixelformats */
>
> OK, maybe I'm a bit rusty on the pixel formats.  Where is this mapping
> table coming from?  Why exactly should we re-map them?
>

Here a Linux mapping from GUID to pixel format:
https://github.com/torvalds/linux/blob/master/drivers/media/common/uvc.c

As you may see majority is mapped as is with some exception.

Thus, current code maps YUY2 into YUYV, and KSMEDIA_L8_IR into GREY.

All other remarks are incorporated into this diff:

Index: sys/dev/usb/uvideo.c
===================================================================
RCS file: /home/cvs/src/sys/dev/usb/uvideo.c,v
diff -u -p -u -p -r1.233 uvideo.c
--- sys/dev/usb/uvideo.c	1 Jan 2025 11:42:07 -0000	1.233
+++ sys/dev/usb/uvideo.c	10 Jan 2025 22:40:47 -0000
@@ -383,6 +383,19 @@ const struct uvideo_devs {
 #define uvideo_lookup(v, p) \
 	((const struct uvideo_devs *)usb_lookup(uvideo_devs, v, p))
 
+
+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 },
+};
+
 int
 uvideo_open(void *addr, int flags, int *size, uint8_t *buffer,
     void (*intr)(void *), void *arg)
@@ -1048,8 +1061,7 @@ uvideo_vs_parse_desc_format_uncompressed
     const usb_descriptor_t *desc)
 {
 	struct usb_video_format_uncompressed_desc *d;
-	uint8_t guid_8bit_ir[16] = UVIDEO_FORMAT_GUID_KSMEDIA_L8_IR;
-	int i;
+	int i, j, nent;
 
 	d = (struct usb_video_format_uncompressed_desc *)(uint8_t *)desc;
 
@@ -1074,18 +1086,22 @@ uvideo_vs_parse_desc_format_uncompressed
 		sc->sc_fmtgrp[sc->sc_fmtgrp_idx].format_dfidx =
 		    d->bDefaultFrameIndex;
 	}
+
 	i = sc->sc_fmtgrp_idx;
-	if (!strcmp(sc->sc_fmtgrp[i].format->u.uc.guidFormat, "YUY2")) {
-		sc->sc_fmtgrp[i].pixelformat = V4L2_PIX_FMT_YUYV;
-	} else if (!strcmp(sc->sc_fmtgrp[i].format->u.uc.guidFormat, "NV12")) {
-		sc->sc_fmtgrp[i].pixelformat = V4L2_PIX_FMT_NV12;
-	} else if (!strcmp(sc->sc_fmtgrp[i].format->u.uc.guidFormat, "UYVY")) {
-		sc->sc_fmtgrp[i].pixelformat = V4L2_PIX_FMT_UYVY;
-	} else if (!memcmp(sc->sc_fmtgrp[i].format->u.uc.guidFormat,
-	    guid_8bit_ir, 16)) {
-		sc->sc_fmtgrp[i].pixelformat = V4L2_PIX_FMT_GREY;
-	} else {
-		sc->sc_fmtgrp[i].pixelformat = 0;
+
+	/* GUID is started by null-terminated pixelformat, usually */
+	memcpy(&sc->sc_fmtgrp[i].pixelformat,
+	    sc->sc_fmtgrp[i].format->u.uc.guidFormat, sizeof(uint32_t));
+
+	/* but some GUID should be mapped to other pixelformats */
+	nent = nitems(uvideo_map_fmts);
+	for (j = 0; j < nent; j++) {
+		if (!memcmp(sc->sc_fmtgrp[i].format->u.uc.guidFormat,
+			    uvideo_map_fmts[j].guidFormat, 16)) {
+			sc->sc_fmtgrp[i].pixelformat =
+				uvideo_map_fmts[j].pixelformat;
+			break;
+		}
 	}
 
 	if (sc->sc_fmtgrp_cur == NULL)
@@ -2992,26 +3008,10 @@ uvideo_enum_fmt(void *v, struct v4l2_fmt
 		break;
 	case UDESCSUB_VS_FORMAT_UNCOMPRESSED:
 		fmtdesc->flags = 0;
-		if (sc->sc_fmtgrp[idx].pixelformat ==
-		    V4L2_PIX_FMT_YUYV) {
-			(void)strlcpy(fmtdesc->description, "YUYV",
-			    sizeof(fmtdesc->description));
-			fmtdesc->pixelformat = V4L2_PIX_FMT_YUYV;
-		} else if (sc->sc_fmtgrp[idx].pixelformat ==
-		    V4L2_PIX_FMT_NV12) {
-			(void)strlcpy(fmtdesc->description, "NV12",
-			    sizeof(fmtdesc->description));
-			fmtdesc->pixelformat = V4L2_PIX_FMT_NV12;
-		} else if (sc->sc_fmtgrp[idx].pixelformat ==
-		    V4L2_PIX_FMT_UYVY) {
-			(void)strlcpy(fmtdesc->description, "UYVY",
-			    sizeof(fmtdesc->description));
-			fmtdesc->pixelformat = V4L2_PIX_FMT_UYVY;
-		} else {
-			(void)strlcpy(fmtdesc->description, "Unknown UC Format",
-			    sizeof(fmtdesc->description));
-			fmtdesc->pixelformat = 0;
-		}
+		fmtdesc->pixelformat = sc->sc_fmtgrp[idx].pixelformat;
+		(void)strlcpy(fmtdesc->description,
+		    (char *) &fmtdesc->pixelformat,
+		    sizeof(fmtdesc->description));
 		bzero(fmtdesc->reserved, sizeof(fmtdesc->reserved));
 		break;
 	default:
Index: sys/dev/usb/uvideo.h
===================================================================
RCS file: /home/cvs/src/sys/dev/usb/uvideo.h,v
diff -u -p -u -p -r1.61 uvideo.h
--- sys/dev/usb/uvideo.h	22 Dec 2024 20:30:04 -0000	1.61
+++ sys/dev/usb/uvideo.h	9 Jan 2025 16:45:22 -0000
@@ -295,17 +295,25 @@ struct usb_video_probe_commit {
 /*
  * USB Video Payload Uncompressed
  */
-/* Table 2-1: Compression Formats */
+
 #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