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