Index | Thread | Search

From:
Kirill A. Korinsky <kirill@korins.ky>
Subject:
sys/uvideo: avoid one bcopy when reads into mmaped memory
To:
OpenBSD tech <tech@openbsd.org>
Cc:
Marcus Glocker <mglocker@openbsd.org>
Date:
Wed, 26 Feb 2025 23:10:01 +0100

Download raw body.

Thread
tech@,

Here a diff which removes one bcopy for each frame which is read from
webcamp into mmaped memory.

Feedback? Ok?

Index: sys/dev/usb/uvideo.c
===================================================================
RCS file: /home/cvs/src/sys/dev/usb/uvideo.c,v
diff -u -p -r1.244 uvideo.c
--- sys/dev/usb/uvideo.c	25 Feb 2025 22:13:44 -0000	1.244
+++ sys/dev/usb/uvideo.c	26 Feb 2025 15:03:13 -0000
@@ -66,6 +66,7 @@ struct uvideo_softc {
 	struct uvideo_frame_buffer		 sc_frame_buffer;
 
 	struct uvideo_mmap			 sc_mmap[UVIDEO_MAX_BUFFERS];
+	struct uvideo_mmap			*sc_mmap_cur;
 	uint8_t					*sc_mmap_buffer;
 	size_t					 sc_mmap_buffer_size;
 	q_mmap					 sc_mmap_q;
@@ -169,7 +170,8 @@ 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);
+uint8_t *	uvideo_mmap_buf(struct uvideo_softc *);
+int		uvideo_mmap_queue(struct uvideo_softc *, 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);
@@ -2244,6 +2246,7 @@ uvideo_vs_decode_stream_header(struct uv
 	struct uvideo_frame_buffer *fb = &sc->sc_frame_buffer;
 	struct usb_video_stream_header *sh;
 	int sample_len;
+	uint8_t *buf;
 	usbd_status ret = USBD_NORMAL_COMPLETION;
 
 	if (frame_size < UVIDEO_SH_MIN_LEN)
@@ -2302,8 +2305,14 @@ uvideo_vs_decode_stream_header(struct uv
 		sample_len = fb->buf_size - fb->offset;
 		fb->error = 1;
 	}
-	if (sample_len > 0) {
-		bcopy(frame + sh->bLength, fb->buf + fb->offset, sample_len);
+
+	if (sc->sc_mmap_flag)
+		buf = uvideo_mmap_buf(sc);
+	else
+		buf = fb->buf;
+
+	if (buf && sample_len > 0) {
+		bcopy(frame + sh->bLength, buf + fb->offset, sample_len);
 		fb->offset += sample_len;
 	}
 
@@ -2326,8 +2335,7 @@ uvideo_vs_decode_stream_header(struct uv
 #endif
 		if (sc->sc_mmap_flag) {
 			/* mmap */
-			if (uvideo_mmap_queue(sc, fb->buf, fb->offset,
-				    fb->error))
+			if (uvideo_mmap_queue(sc, fb->error))
 				ret = USBD_NOMEM;
 		} else if (fb->error) {
 			DPRINTF(1, "%s: %s: error frame, skipped!\n",
@@ -2367,6 +2375,7 @@ uvideo_vs_decode_stream_header_isight(st
 {
 	struct uvideo_frame_buffer *fb = &sc->sc_frame_buffer;
 	int sample_len, header = 0;
+	uint8_t *buf;
 	usbd_status ret = USBD_NORMAL_COMPLETION;
 	uint8_t magic[] = {
 	    0x11, 0x22, 0x33, 0x44,
@@ -2385,7 +2394,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, 0))
+			if (uvideo_mmap_queue(sc, 0))
 				ret = USBD_NOMEM;
 		} else {
 			/* read */
@@ -2393,10 +2402,15 @@ uvideo_vs_decode_stream_header_isight(st
 		}
 		fb->offset = 0;
 	} else {
+		if (sc->sc_mmap_flag)
+			buf = uvideo_mmap_buf(sc);
+		else
+			buf = fb->buf;
+
 		/* save sample */
 		sample_len = frame_size;
-		if ((fb->offset + sample_len) <= fb->buf_size) {
-			bcopy(frame, fb->buf + fb->offset, sample_len);
+		if (buf && (fb->offset + sample_len) <= fb->buf_size) {
+			bcopy(frame, buf + fb->offset, sample_len);
 			fb->offset += sample_len;
 		}
 	}
@@ -2404,11 +2418,14 @@ uvideo_vs_decode_stream_header_isight(st
 	return (ret);
 }
 
-int
-uvideo_mmap_queue(struct uvideo_softc *sc, uint8_t *buf, int len, int err)
+uint8_t *
+uvideo_mmap_buf(struct uvideo_softc *sc)
 {
 	int i;
 
+	if (sc->sc_mmap_cur)
+		return sc->sc_mmap_cur->buf;
+
 	if (sc->sc_mmap_count == 0 || sc->sc_mmap_buffer == NULL)
 		panic("%s: mmap buffers not allocated", __func__);
 
@@ -2420,32 +2437,47 @@ uvideo_mmap_queue(struct uvideo_softc *s
 	if (i == sc->sc_mmap_count) {
 		DPRINTF(1, "%s: %s: mmap queue is full!\n",
 		    DEVNAME(sc), __func__);
-		return ENOMEM;
+		return NULL;
 	}
 
-	/* copy frame to mmap buffer and report length */
-	bcopy(buf, sc->sc_mmap[i].buf, len);
-	sc->sc_mmap[i].v4l2_buf.bytesused = len;
+	sc->sc_mmap_cur = &sc->sc_mmap[i];
+	sc->sc_mmap_cur->v4l2_buf.flags &= ~V4L2_BUF_FLAG_QUEUED;
+
+	DPRINTF(2, "%s: %s: frame dequeued on index: %d\n",
+	    DEVNAME(sc), __func__, i);
+
+	return sc->sc_mmap_cur->buf;
+}
+
+int
+uvideo_mmap_queue(struct uvideo_softc *sc, int err)
+{
+	if (sc->sc_mmap_cur == NULL)
+		return ENOMEM;
+
+	/* report length */
+	sc->sc_mmap_cur->v4l2_buf.bytesused = sc->sc_frame_buffer.offset;
 
 	/* timestamp it */
-	getmicrouptime(&sc->sc_mmap[i].v4l2_buf.timestamp);
-	sc->sc_mmap[i].v4l2_buf.flags &= ~V4L2_BUF_FLAG_TIMESTAMP_MASK;
-	sc->sc_mmap[i].v4l2_buf.flags |= V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
-	sc->sc_mmap[i].v4l2_buf.flags &= ~V4L2_BUF_FLAG_TSTAMP_SRC_MASK;
-	sc->sc_mmap[i].v4l2_buf.flags |= V4L2_BUF_FLAG_TSTAMP_SRC_EOF;
-	sc->sc_mmap[i].v4l2_buf.flags &= ~V4L2_BUF_FLAG_TIMECODE;
+	getmicrouptime(&sc->sc_mmap_cur->v4l2_buf.timestamp);
+	sc->sc_mmap_cur->v4l2_buf.flags &= ~V4L2_BUF_FLAG_TIMESTAMP_MASK;
+	sc->sc_mmap_cur->v4l2_buf.flags |= V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
+	sc->sc_mmap_cur->v4l2_buf.flags &= ~V4L2_BUF_FLAG_TSTAMP_SRC_MASK;
+	sc->sc_mmap_cur->v4l2_buf.flags |= V4L2_BUF_FLAG_TSTAMP_SRC_EOF;
+	sc->sc_mmap_cur->v4l2_buf.flags &= ~V4L2_BUF_FLAG_TIMECODE;
 
 	/* forward error bit */
-	sc->sc_mmap[i].v4l2_buf.flags &= ~V4L2_BUF_FLAG_ERROR;
+	sc->sc_mmap_cur->v4l2_buf.flags &= ~V4L2_BUF_FLAG_ERROR;
 	if (err)
-		sc->sc_mmap[i].v4l2_buf.flags |= V4L2_BUF_FLAG_ERROR;
+		sc->sc_mmap_cur->v4l2_buf.flags |= V4L2_BUF_FLAG_ERROR;
 
 	/* queue it */
-	sc->sc_mmap[i].v4l2_buf.flags |= V4L2_BUF_FLAG_DONE;
-	sc->sc_mmap[i].v4l2_buf.flags &= ~V4L2_BUF_FLAG_QUEUED;
-	SIMPLEQ_INSERT_TAIL(&sc->sc_mmap_q, &sc->sc_mmap[i], q_frames);
-	DPRINTF(2, "%s: %s: frame queued on index %d\n",
-	    DEVNAME(sc), __func__, i);
+	sc->sc_mmap_cur->v4l2_buf.flags |= V4L2_BUF_FLAG_DONE;
+	SIMPLEQ_INSERT_TAIL(&sc->sc_mmap_q, sc->sc_mmap_cur, q_frames);
+
+	sc->sc_mmap_cur = NULL;
+
+	DPRINTF(2, "%s: %s: frame queued\n", DEVNAME(sc), __func__);
 
 	wakeup(sc);