From: Kirill A. Korinsky <kirill@korins.ky>
Subject: sys/uvideo: don't skip frame immediately when buffer unavailable (regression from v1.256)
To: OpenBSD tech <tech@openbsd.org>
Cc: Marcus Glocker <mglocker@openbsd.org>, Ian Darwin <ian@darwinsys.com>
Date: Fri, 04 Apr 2025 09:24:51 +0200

tech@,

this diff fixes a regression that was introduced by v1.256 and reported
by ian@.

The original change (v1.256), which avoided one bcopy, was essential to
improve performance sufficiently for supporting 4K 60fps devices.

However, some applications (e.g., multimedia/motion) release a buffer
back to the driver just before the read. If this release happens after a
frame packet has arrived, the logic introduced in v1.256 drops the
frame. This dropped frame can be critical, especially for devices with
low framerates, causing the stream to glitch.

Therefore, instead of dropping the frame immediately, this fix parses
the incoming frame into sc->sc_frame_buffer.buf. The data is then bcopy
into shared memory after parsing is complete if buffers was released,
restoring the behavior from before v1.256.

Notably, this issue is application-dependent; for example, ffplay works
correctly, while motion does not.

ian@ has confirmed that this fix the issue on his affected device.

Ok?

Index: sys/dev/usb/uvideo.c
===================================================================
RCS file: /home/cvs/src/sys/dev/usb/uvideo.c,v
diff -u -p -r1.256 uvideo.c
--- sys/dev/usb/uvideo.c	24 Mar 2025 18:56:34 -0000	1.256
+++ sys/dev/usb/uvideo.c	4 Apr 2025 07:10:32 -0000
@@ -2249,7 +2249,7 @@ uvideo_vs_start_bulk_thread(void *arg)
 
 		if (sc->sc_mmap_flag) {
 			if ((buf = uvideo_mmap_getbuf(sc)) == NULL)
-				continue;
+				buf = sc->sc_frame_buffer.buf;
 		} else
 			buf = sc->sc_frame_buffer.buf;
 
@@ -2330,7 +2330,7 @@ uvideo_vs_cb(struct usbd_xfer *xfer, voi
 
 		if (sc->sc_mmap_flag) {
 			if ((buf = uvideo_mmap_getbuf(sc)) == NULL)
-				goto skip;
+				buf = sc->sc_frame_buffer.buf;
 		} else
 			buf = sc->sc_frame_buffer.buf;
 
@@ -2536,8 +2536,14 @@ uvideo_mmap_getbuf(struct uvideo_softc *
 void
 uvideo_mmap_queue(struct uvideo_softc *sc, int len, int err)
 {
-	if (sc->sc_mmap_cur == NULL)
-		panic("uvideo_mmap_queue: NULL pointer!");
+	uint8_t *buf;
+
+	if (sc->sc_mmap_cur == NULL) {
+		if ((buf = uvideo_mmap_getbuf(sc)) == NULL)
+			return;
+
+		bcopy(sc->sc_frame_buffer.buf, buf, len);
+	}
 
 	/* report frame length */
 	sc->sc_mmap_cur->v4l2_buf.bytesused = len;