Download raw body.
sys/uvideo: avoid using queue.h
On Mon, Mar 17, 2025 at 08:07:56PM GMT, Marcus Glocker wrote:
> > Well, I've just tried your diff and it doesn't work. Idea was to perform a
> > stress test of your and my diff against uvideo.c,v 1.252 but the system had
> > crashed before ffmpeg gets any frame from the device. OCRed stacktrace:
> >
> > panic: attempt to access user address 0x20 from EL1
> > Stopped at panic+0x140: cmp w21, #0x0
> > TID PID UID PRFLAGS PC PFLAGS CPU COMMAND
> > 236976 22771 1000 0x3 0x4000000 1 ffmpeg
> > db_enter() at panic+0x13c
> > panic() at kdata_abort+0x180
> > do_el0_sync() at handle_el1h_sync+0x68
> > handle_el1h_sync() at uvideo_vs_cb+0xc0
> > uvideo_vs_cb() at usb_transfer_complete+0x220
> > usb_transfer_complete() at xhci_event_dequeue+0x10c
> > xhci_event_dequeue() at xhci_softintr+0x34
> > https://www.openbsd.org/ddb.html describes the minimum info required in bug
> > reports. Insufficient info makes it difficult to find and fix bugs.
> > ddb{0}>
> >
> > The test was to use Elgato webcam to stream 4K 60 fps stream to null for 100
> > seconds as ffmpeg command:
> >
> > ffmpeg -input_format mjpeg -video_size 3840x2160 -framerate 60 -i /dev/video0 -t 100 -f null -
> >
> > Anyway, the same test works well on the same machine with the same device
> > with and without my diff.
>
> Ah crap, I had a left over in that diff from the first version, which
> most probably caused this panic. Is this better by any chance?
OK. I think we're getting closer. I did some more analysis of this
"detach cam during streaming and system freezes" issue and came to the
following conclusion:
- It happens with the -current code.
- It happens with your "own queue implementation" diff.
- It happens with my improved frame buffer utilization and mutex diff.
- It happens with my improved frame buffer utilization without mutex
diff (new, attached).
The only diff which helps to fix the detach-freeze issue is this diff
from you: [1].
For me this means:
- SIMPLEQ isn't the issue.
- Missing mutex locking isn't the issue.
If the following diff works for your devices, I would propose to only
get this in, to fix the frame buffer utilization in uvideo. Then as a
next step we review [1], and hopefully can bring that in next to fix
the detach-freeze issue.
[1] https://marc.info/?l=openbsd-tech&m=174180759929893&w=2
Index: sys/dev/usb/uvideo.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uvideo.c,v
diff -u -p -u -p -r1.252 uvideo.c
--- sys/dev/usb/uvideo.c 12 Mar 2025 14:08:51 -0000 1.252
+++ sys/dev/usb/uvideo.c 17 Mar 2025 20:35:40 -0000
@@ -68,6 +68,7 @@ struct uvideo_softc {
struct uvideo_mmap sc_mmap[UVIDEO_MAX_BUFFERS];
uint8_t *sc_mmap_buffer;
size_t sc_mmap_buffer_size;
+ int sc_mmap_buffer_idx;
q_mmap sc_mmap_q;
int sc_mmap_count;
int sc_mmap_flag;
@@ -172,6 +173,7 @@ void uvideo_vs_decode_stream_header(str
uint8_t *, int);
void uvideo_vs_decode_stream_header_isight(struct uvideo_softc *,
uint8_t *, int);
+int uvideo_get_buf(struct uvideo_softc *);
void 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,
@@ -2498,6 +2500,30 @@ uvideo_vs_decode_stream_header_isight(st
}
}
+int
+uvideo_get_buf(struct uvideo_softc *sc)
+{
+ int i, idx = sc->sc_mmap_buffer_idx;
+
+ /* find a buffer which is ready for queueing */
+ for (i = 0; i < sc->sc_mmap_count; i++) {
+ if (sc->sc_mmap[sc->sc_mmap_buffer_idx].v4l2_buf.flags &
+ V4L2_BUF_FLAG_QUEUED) {
+ idx = sc->sc_mmap_buffer_idx;
+ if (++sc->sc_mmap_buffer_idx == sc->sc_mmap_count)
+ sc->sc_mmap_buffer_idx = 0;
+ break;
+ }
+ if (++sc->sc_mmap_buffer_idx == sc->sc_mmap_count)
+ sc->sc_mmap_buffer_idx = 0;
+ }
+
+ if (i == sc->sc_mmap_count)
+ return -1;
+
+ return idx;
+}
+
void
uvideo_mmap_queue(struct uvideo_softc *sc, uint8_t *buf, int len, int err)
{
@@ -2507,11 +2533,8 @@ uvideo_mmap_queue(struct uvideo_softc *s
panic("%s: mmap buffers not allocated", __func__);
/* find a buffer which is ready for queueing */
- for (i = 0; i < sc->sc_mmap_count; i++) {
- if (sc->sc_mmap[i].v4l2_buf.flags & V4L2_BUF_FLAG_QUEUED)
- break;
- }
- if (i == sc->sc_mmap_count) {
+ i = uvideo_get_buf(sc);
+ if (i == -1) {
DPRINTF(1, "%s: %s: mmap queue is full!\n",
DEVNAME(sc), __func__);
return;
@@ -3552,6 +3575,8 @@ uvideo_reqbufs(void *v, struct v4l2_requ
sc->sc_mmap[i].v4l2_buf.m.offset,
sc->sc_mmap[i].v4l2_buf.length);
}
+
+ sc->sc_mmap_buffer_idx = 0;
/* tell how many buffers we have really allocated */
rb->count = sc->sc_mmap_count;
sys/uvideo: avoid using queue.h