Download raw body.
sys/uvideo: forward error bit to consumer
On Thu, Dec 19, 2024 at 11:49:34PM +0100, Kirill A. Korinsky wrote:
> 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?
Oh you rock!
The webcam on my Lenovo x13 is finally working! :-)
addr 02: 174f:1812 , Integrated Camera
high speed, power 500 mA, config 1, rev 10.22, iSerial 0001
driver: uvideo0
driver: uvideo1
driver: ugen0
uvideo0 at uhub1 port 4 configuration 1 interface 0 " Integrated Camera" rev 2.01/10.22 addr 2
video0 at uvideo0
Thanks a lot.
> 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
>
--
Antoine
sys/uvideo: forward error bit to consumer