Index | Thread | Search

From:
Kirill A. Korinsky <kirill@korins.ky>
Subject:
sys/uvideo: forward error bit to consumer
To:
OpenBSD tech <tech@openbsd.org>
Cc:
sdk@openbsd.org, mglocker@openbsd.org
Date:
Thu, 19 Dec 2024 23:49:34 +0100

Download raw body.

Thread
tech@,

Some invalid frames or frames with an error flag from the camera might be 
essential for a stream consumer for synchronization and other purposes. 
Without this, the consumer may be unable to synchronize the stream or play 
it at all.

Instead of skipping frames that the driver considers incorrect, this change 
forwards them to the V4L consumer with the V4L2_BUF_FLAG_ERROR flag set, 
allowing the consumer to decide whether to skip the frame or use parts of it.

This change fixes the embedded webcam on the ThinkPad T14 Gen 5, which was 
tested by sdk@.

The behavior for non-mmap consumers remains unchanged, and error frames 
are still skipped by the driver.

I had tested this diff with the following webcams and no regressions
were noticed:
 - Azurewave, USB camera
 - LG UltraFine Display Camera
 - Jabra PanaCast 20

Feedback? Tests? Ok?

Index: sys/dev/usb/uvideo.c
===================================================================
RCS file: /home/cvs/src/sys/dev/usb/uvideo.c,v
diff -u -p -r1.228 uvideo.c
--- sys/dev/usb/uvideo.c	14 Dec 2024 09:58:04 -0000	1.228
+++ sys/dev/usb/uvideo.c	18 Dec 2024 09:49:15 -0000
@@ -168,7 +168,7 @@ usbd_status	uvideo_vs_decode_stream_head
 		    uint8_t *, int); 
 usbd_status	uvideo_vs_decode_stream_header_isight(struct uvideo_softc *,
 		    uint8_t *, int);
-int		uvideo_mmap_queue(struct uvideo_softc *, uint8_t *, int);
+int		uvideo_mmap_queue(struct uvideo_softc *, uint8_t *, int, int);
 void		uvideo_read(struct uvideo_softc *, uint8_t *, int);
 usbd_status	uvideo_usb_control(struct uvideo_softc *, uint8_t, uint8_t,
 		    uint16_t, uint8_t *, size_t);
@@ -2148,18 +2148,9 @@ uvideo_vs_decode_stream_header(struct uv
 
 	DPRINTF(2, "%s: stream header len = %d\n", DEVNAME(sc), sh->bLength);
 
-	if (sh->bLength > UVIDEO_SH_MAX_LEN || sh->bLength < UVIDEO_SH_MIN_LEN)
+	if (sh->bLength > frame_size || sh->bLength < UVIDEO_SH_MIN_LEN)
 		/* invalid header size */
 		return (USBD_INVAL);
-	if (sh->bLength == frame_size && !(sh->bFlags & UVIDEO_SH_FLAG_EOF)) {
-		/* stream header without payload and no EOF */
-		return (USBD_INVAL);
-	}
-	if (sh->bFlags & UVIDEO_SH_FLAG_ERR) {
-		/* stream error, skip xfer */
-		DPRINTF(1, "%s: %s: stream error!\n", DEVNAME(sc), __func__);
-		return (USBD_CANCELLED);
-	}
 
 	DPRINTF(2, "%s: frame_size = %d\n", DEVNAME(sc), frame_size);
 
@@ -2178,6 +2169,7 @@ uvideo_vs_decode_stream_header(struct uv
 		fb->sample = 1;
 		fb->fid = sh->bFlags & UVIDEO_SH_FLAG_FID;
 		fb->offset = 0;
+		fb->error = 0;
 	} else {
 		/* continues sample for a frame, check consistency */
 		if (fb->fid != (sh->bFlags & UVIDEO_SH_FLAG_FID)) {
@@ -2186,12 +2178,25 @@ uvideo_vs_decode_stream_header(struct uv
 			fb->sample = 1;
 			fb->fid = sh->bFlags & UVIDEO_SH_FLAG_FID;
 			fb->offset = 0;
+			fb->error = 0;
 		}
 	}
 
+	if (sh->bFlags & UVIDEO_SH_FLAG_ERR) {
+		/* stream error, skip xfer */
+		DPRINTF(1, "%s: %s: stream error!\n", DEVNAME(sc), __func__);
+		fb->error = 1;
+	}
+
 	/* save sample */
 	sample_len = frame_size - sh->bLength;
-	if ((fb->offset + sample_len) <= fb->buf_size) {
+	if (sample_len > fb->buf_size - fb->offset) {
+		DPRINTF(1, "%s: %s: frame too large, marked as error\n",
+		    DEVNAME(sc), __func__);
+		sample_len = fb->buf_size - fb->offset;
+		fb->error = 1;
+	}
+	if (sample_len > 0) {
 		bcopy(frame + sh->bLength, fb->buf + fb->offset, sample_len);
 		fb->offset += sample_len;
 	}
@@ -2201,10 +2206,7 @@ uvideo_vs_decode_stream_header(struct uv
 		DPRINTF(2, "%s: %s: EOF (frame size = %d bytes)\n",
 		    DEVNAME(sc), __func__, fb->offset);
 
-		if (fb->offset > fb->buf_size) {
-			DPRINTF(1, "%s: %s: frame too large, skipped!\n",
-			    DEVNAME(sc), __func__);
-		} else if (fb->offset < fb->buf_size &&
+		if (fb->offset < fb->buf_size &&
 		    !(fb->fmt_flags & V4L2_FMT_FLAG_COMPRESSED)) {
 			DPRINTF(1, "%s: %s: frame too small, skipped!\n",
 			    DEVNAME(sc), __func__);
@@ -2216,8 +2218,12 @@ uvideo_vs_decode_stream_header(struct uv
 #endif
 			if (sc->sc_mmap_flag) {
 				/* mmap */
-				if (uvideo_mmap_queue(sc, fb->buf, fb->offset))
+				if (uvideo_mmap_queue(sc, fb->buf, fb->offset,
+					    fb->error))
 					return (USBD_NOMEM);
+			} else if (fb->error) {
+				DPRINTF(1, "%s: %s: error frame, skipped!\n",
+				    DEVNAME(sc), __func__);
 			} else {
 				/* read */
 				uvideo_read(sc, fb->buf, fb->offset);
@@ -2226,6 +2232,7 @@ uvideo_vs_decode_stream_header(struct uv
 
 		fb->sample = 0;
 		fb->fid = 0;
+		fb->error = 0;
 	}
 
 	return (USBD_NORMAL_COMPLETION);
@@ -2270,7 +2277,7 @@ uvideo_vs_decode_stream_header_isight(st
 	if (header) {
 		if (sc->sc_mmap_flag) {
 			/* mmap */
-			if (uvideo_mmap_queue(sc, fb->buf, fb->offset))
+			if (uvideo_mmap_queue(sc, fb->buf, fb->offset, 0))
 				return (USBD_NOMEM);
 		} else {
 			/* read */
@@ -2290,7 +2297,7 @@ uvideo_vs_decode_stream_header_isight(st
 }
 
 int
-uvideo_mmap_queue(struct uvideo_softc *sc, uint8_t *buf, int len)
+uvideo_mmap_queue(struct uvideo_softc *sc, uint8_t *buf, int len, int err)
 {
 	int i;
 
@@ -2314,6 +2321,11 @@ uvideo_mmap_queue(struct uvideo_softc *s
 
 	/* timestamp it */
 	getmicrotime(&sc->sc_mmap[i].v4l2_buf.timestamp);
+
+	/* forward error bit */
+	sc->sc_mmap[i].v4l2_buf.flags &= ~V4L2_BUF_FLAG_ERROR;
+	if (err)
+		sc->sc_mmap[i].v4l2_buf.flags |= V4L2_BUF_FLAG_ERROR;
 
 	/* queue it */
 	sc->sc_mmap[i].v4l2_buf.flags |= V4L2_BUF_FLAG_DONE;
Index: sys/dev/usb/uvideo.h
===================================================================
RCS file: /home/cvs/src/sys/dev/usb/uvideo.h,v
diff -u -p -r1.60 uvideo.h
--- sys/dev/usb/uvideo.h	8 Dec 2019 13:21:21 -0000	1.60
+++ sys/dev/usb/uvideo.h	18 Dec 2024 09:30:26 -0000
@@ -449,6 +449,7 @@ struct uvideo_vs_iface {
 struct uvideo_frame_buffer {
 	int		 sample;
 	uint8_t		 fid;
+	uint8_t		 error;
 	int		 offset;
 	int		 buf_size;
 	uint8_t		*buf;


-- 
wbr, Kirill