Download raw body.
sys/uvideo: avoid using queue.h
tech@,
macroses from queue.h doesn't thread safe and wraping it by mutex on
uvideo use case seems isn't so good idea.
It has a hardcoded number of used buffers as 8 and already has a code
which iterates against them to find the buffer which is ready for
queueing in uvideo_mmap_getbuf.
Here I'd like to replace SIMPLEQ_* by similar logic.
It fixes two bugs:
1. If isochronous webcam sends two frames in one transfer,
SIMPLEQ_INSERT_TAIL adds the same element twice and creates infinity
loop inside sc_mmap_q which will be iterating in uvideo_vs_free_frame.
2. Prevents from calling SIMPLEQ_INSERT_TAIL and SIMPLEQ_REMOVE_HEAD for
the same element in the queue which may lead to unexpected state.
This diff was suceffuly tested on:
- Elgato Facecam Pro (isochronous)
- Jabra PanaCast (bulk)
Ok?
Index: sys/dev/usb/uvideo.c
===================================================================
RCS file: /home/cvs/src/sys/dev/usb/uvideo.c,v
diff -u -p -r1.249 uvideo.c
--- sys/dev/usb/uvideo.c 4 Mar 2025 22:59:01 -0000 1.249
+++ sys/dev/usb/uvideo.c 5 Mar 2025 22:40:46 -0000
@@ -69,7 +69,6 @@ struct uvideo_softc {
struct uvideo_mmap *sc_mmap_cur;
uint8_t *sc_mmap_buffer;
size_t sc_mmap_buffer_size;
- q_mmap sc_mmap_q;
int sc_mmap_count;
int sc_mmap_flag;
@@ -671,8 +670,6 @@ uvideo_attach_hook(struct device *self)
if (error != USBD_NORMAL_COMPLETION)
return;
- /* init mmap queue */
- SIMPLEQ_INIT(&sc->sc_mmap_q);
sc->sc_mmap_count = 0;
DPRINTF(1, "uvideo_attach: doing video_attach_mi\n");
@@ -1962,9 +1959,6 @@ uvideo_vs_free_frame(struct uvideo_softc
sc->sc_mmap_buffer_size = 0;
}
- while (!SIMPLEQ_EMPTY(&sc->sc_mmap_q))
- SIMPLEQ_REMOVE_HEAD(&sc->sc_mmap_q, q_frames);
-
sc->sc_mmap_count = 0;
}
@@ -2556,7 +2551,6 @@ uvideo_mmap_queue(struct uvideo_softc *s
/* queue it */
sc->sc_mmap_cur->v4l2_buf.flags |= V4L2_BUF_FLAG_DONE;
sc->sc_mmap_cur->v4l2_buf.flags &= ~V4L2_BUF_FLAG_QUEUED;
- SIMPLEQ_INSERT_TAIL(&sc->sc_mmap_q, sc->sc_mmap_cur, q_frames);
DPRINTF(2, "%s: %s: frame queued\n", DEVNAME(sc), __func__);
wakeup(sc);
@@ -3624,32 +3618,34 @@ int
uvideo_dqbuf(void *v, struct v4l2_buffer *dqb)
{
struct uvideo_softc *sc = v;
- struct uvideo_mmap *mmap;
- int error;
+ int error, i;
if (dqb->type != V4L2_BUF_TYPE_VIDEO_CAPTURE ||
dqb->memory != V4L2_MEMORY_MMAP)
return (EINVAL);
- if (SIMPLEQ_EMPTY(&sc->sc_mmap_q)) {
+again:
+ /* find a buffer which is done */
+ for (i = 0; i < sc->sc_mmap_count; i++) {
+ if (sc->sc_mmap[i].v4l2_buf.flags & V4L2_BUF_FLAG_DONE)
+ break;
+ }
+
+ if (i == sc->sc_mmap_count) {
/* mmap queue is empty, block until first frame is queued */
error = tsleep_nsec(sc, 0, "vid_mmap", SEC_TO_NSEC(10));
if (error)
return (EINVAL);
+ goto again;
}
- mmap = SIMPLEQ_FIRST(&sc->sc_mmap_q);
- if (mmap == NULL)
- panic("uvideo_dqbuf: NULL pointer!");
-
- bcopy(&mmap->v4l2_buf, dqb, sizeof(struct v4l2_buffer));
+ bcopy(&sc->sc_mmap[i].v4l2_buf, dqb, sizeof(struct v4l2_buffer));
- mmap->v4l2_buf.flags &= ~V4L2_BUF_FLAG_DONE;
- mmap->v4l2_buf.flags &= ~V4L2_BUF_FLAG_QUEUED;
+ sc->sc_mmap[i].v4l2_buf.flags &= ~V4L2_BUF_FLAG_DONE;
+ sc->sc_mmap[i].v4l2_buf.flags &= ~V4L2_BUF_FLAG_QUEUED;
DPRINTF(2, "%s: %s: frame dequeued from index %d\n",
- DEVNAME(sc), __func__, mmap->v4l2_buf.index);
- SIMPLEQ_REMOVE_HEAD(&sc->sc_mmap_q, q_frames);
+ DEVNAME(sc), __func__, sc->sc_mmap[i].v4l2_buf.index);
return (0);
}
sys/uvideo: avoid using queue.h