From d13871b709d7d7042b17ef6e2051198772a67dba Mon Sep 17 00:00:00 2001 From: mglocker Date: Tue, 16 Feb 2021 13:50:46 +0000 Subject: [PATCH] Enable multiple opens of a video(4) device as described in the V4L2 specification: https://www.kernel.org/doc/html/v5.10/userspace-api/media/v4l/open.html#f1 The discussion has been started by jca@, who has implemented this behavior recently, but limited to the same process. This diff extends this behavior to any process. The first process which opens a stream will become the device owner. Other processes are still allowed to call certain ioctls, but none which are related to the start/stop of a stream, or manipulation of the streaming buffers. At the moment only VIDIOC_G_CTRL and VIDIOC_S_CTRL are supported to be called by non- device owner processes, which should be extended further in the future. There is no additional kernel locking implemented at the moment, since video(4) already runs under the KERNEL_LOCK(), which we expect to be sufficient for now (as discussed with claudio@). A lot of improvement input received from anton@. ok anton@ --- sys/dev/video.c | 139 +++++++++++++++++++++++++++++++++++++----------- 1 file changed, 109 insertions(+), 30 deletions(-) diff --git a/sys/dev/video.c b/sys/dev/video.c index 49ac5b9ad4c..d1fde67d132 100644 --- a/sys/dev/video.c +++ b/sys/dev/video.c @@ -1,4 +1,4 @@ -/* $OpenBSD: video.c,v 1.48 2021/01/31 19:32:01 mglocker Exp $ */ +/* $OpenBSD: video.c,v 1.49 2021/02/16 13:50:46 mglocker Exp $ */ /* * Copyright (c) 2008 Robert Nagy @@ -47,7 +47,8 @@ struct video_softc { struct device *sc_dev; /* hardware device struct */ struct video_hw_if *hw_if; /* hardware interface */ char sc_dying; /* device detached */ - struct process *sc_owner; /* owner process */ + struct process *sc_owner; /* owner process */ + uint8_t sc_open; /* device opened */ int sc_fsize; uint8_t *sc_fbuffer; @@ -69,6 +70,8 @@ int videoactivate(struct device *, int); int videoprint(void *, const char *); void video_intr(void *); +int video_stop(struct video_softc *); +int video_claim(struct video_softc *, struct process *); struct cfattach video_ca = { sizeof(struct video_softc), videoprobe, videoattach, @@ -122,6 +125,9 @@ videoopen(dev_t dev, int flags, int fmt, struct proc *p) { int unit; struct video_softc *sc; + int error = 0; + + KERNEL_ASSERT_LOCKED(); unit = VIDEOUNIT(dev); if (unit >= video_cd.cd_ndevs || @@ -129,38 +135,40 @@ videoopen(dev_t dev, int flags, int fmt, struct proc *p) sc->hw_if == NULL) return (ENXIO); - if (sc->sc_owner != NULL) { - if (sc->sc_owner == p->p_p) - return (0); - else - return (EBUSY); - } else - sc->sc_owner = p->p_p; + if (sc->sc_open) { + DPRINTF(("%s: device already open\n", __func__)); + return (0); + } else { + sc->sc_open = 1; + DPRINTF(("%s: set device to open\n", __func__)); + } sc->sc_vidmode = VIDMODE_NONE; sc->sc_frames_ready = 0; if (sc->hw_if->open != NULL) - return (sc->hw_if->open(sc->hw_hdl, flags, &sc->sc_fsize, - sc->sc_fbuffer, video_intr, sc)); - else - return (0); + error = sc->hw_if->open(sc->hw_hdl, flags, &sc->sc_fsize, + sc->sc_fbuffer, video_intr, sc); + + return (error); } int videoclose(dev_t dev, int flags, int fmt, struct proc *p) { struct video_softc *sc; - int r = 0; + int error = 0; - sc = video_cd.cd_devs[VIDEOUNIT(dev)]; + KERNEL_ASSERT_LOCKED(); - if (sc->hw_if->close != NULL) - r = sc->hw_if->close(sc->hw_hdl); + DPRINTF(("%s: last close\n", __func__)); - sc->sc_owner = NULL; + sc = video_cd.cd_devs[VIDEOUNIT(dev)]; + + error = video_stop(sc); + sc->sc_open = 0; - return (r); + return (error); } int @@ -170,6 +178,8 @@ videoread(dev_t dev, struct uio *uio, int ioflag) int unit, error; size_t size; + KERNEL_ASSERT_LOCKED(); + unit = VIDEOUNIT(dev); if (unit >= video_cd.cd_ndevs || (sc = video_cd.cd_devs[unit]) == NULL) @@ -181,6 +191,9 @@ videoread(dev_t dev, struct uio *uio, int ioflag) if (sc->sc_vidmode == VIDMODE_MMAP) return (EBUSY); + if ((error = video_claim(sc, curproc->p_p))) + return (error); + /* start the stream if not already started */ if (sc->sc_vidmode == VIDMODE_NONE && sc->hw_if->start_read) { error = sc->hw_if->start_read(sc->hw_hdl); @@ -221,6 +234,8 @@ videoioctl(dev_t dev, u_long cmd, caddr_t data, int flags, struct proc *p) struct v4l2_buffer *vb = (struct v4l2_buffer *)data; int unit, error; + KERNEL_ASSERT_LOCKED(); + unit = VIDEOUNIT(dev); if (unit >= video_cd.cd_ndevs || (sc = video_cd.cd_devs[unit]) == NULL || sc->hw_if == NULL) @@ -231,6 +246,31 @@ videoioctl(dev_t dev, u_long cmd, caddr_t data, int flags, struct proc *p) error = EOPNOTSUPP; switch (cmd) { + case VIDIOC_G_CTRL: + if (sc->hw_if->g_ctrl) + error = (sc->hw_if->g_ctrl)(sc->hw_hdl, + (struct v4l2_control *)data); + break; + case VIDIOC_S_CTRL: + if (sc->hw_if->s_ctrl) + error = (sc->hw_if->s_ctrl)(sc->hw_hdl, + (struct v4l2_control *)data); + break; + default: + error = (ENOTTY); + } + if (error != ENOTTY) + return (error); + + if ((error = video_claim(sc, p->p_p))) + return (error); + + /* + * The following IOCTLs can only be called by the device owner. + * For further shared IOCTLs please move it up. + */ + error = EOPNOTSUPP; + switch (cmd) { case VIDIOC_QUERYCAP: if (sc->hw_if->querycap) error = (sc->hw_if->querycap)(sc->hw_hdl, @@ -326,6 +366,10 @@ videoioctl(dev_t dev, u_long cmd, caddr_t data, int flags, struct proc *p) if (sc->hw_if->streamoff) error = (sc->hw_if->streamoff)(sc->hw_hdl, (int)*data); + if (!error) { + /* Release device ownership and streaming buffers. */ + video_stop(sc); + } break; case VIDIOC_TRY_FMT: if (sc->hw_if->try_fmt) @@ -337,16 +381,6 @@ videoioctl(dev_t dev, u_long cmd, caddr_t data, int flags, struct proc *p) error = (sc->hw_if->queryctrl)(sc->hw_hdl, (struct v4l2_queryctrl *)data); break; - case VIDIOC_G_CTRL: - if (sc->hw_if->g_ctrl) - error = (sc->hw_if->g_ctrl)(sc->hw_hdl, - (struct v4l2_control *)data); - break; - case VIDIOC_S_CTRL: - if (sc->hw_if->s_ctrl) - error = (sc->hw_if->s_ctrl)(sc->hw_hdl, - (struct v4l2_control *)data); - break; default: error = (ENOTTY); } @@ -361,6 +395,8 @@ videopoll(dev_t dev, int events, struct proc *p) struct video_softc *sc; int error, revents = 0; + KERNEL_ASSERT_LOCKED(); + if (unit >= video_cd.cd_ndevs || (sc = video_cd.cd_devs[unit]) == NULL) return (POLLERR); @@ -368,6 +404,9 @@ videopoll(dev_t dev, int events, struct proc *p) if (sc->sc_dying) return (POLLERR); + if ((error = video_claim(sc, p->p_p))) + return (error); + DPRINTF(("%s: events=0x%x\n", __func__, events)); if (events & (POLLIN | POLLRDNORM)) { @@ -405,6 +444,8 @@ videommap(dev_t dev, off_t off, int prot) caddr_t p; paddr_t pa; + KERNEL_ASSERT_LOCKED(); + DPRINTF(("%s: off=%lld, prot=%d\n", __func__, off, prot)); unit = VIDEOUNIT(dev); @@ -466,7 +507,9 @@ videokqfilter(dev_t dev, struct knote *kn) { int unit = VIDEOUNIT(dev); struct video_softc *sc; - int s; + int s, error; + + KERNEL_ASSERT_LOCKED(); if (unit >= video_cd.cd_ndevs || (sc = video_cd.cd_devs[unit]) == NULL) @@ -484,6 +527,9 @@ videokqfilter(dev_t dev, struct knote *kn) return (EINVAL); } + if ((error = video_claim(sc, curproc->p_p))) + return (error); + /* * Start the stream in read() mode if not already started. If * the user wanted mmap() mode, he should have called mmap() @@ -539,6 +585,39 @@ video_intr(void *addr) selwakeup(&sc->sc_rsel); } +int +video_stop(struct video_softc *sc) +{ + int error = 0; + + DPRINTF(("%s: stream close\n", __func__)); + + if (sc->hw_if->close != NULL) + error = sc->hw_if->close(sc->hw_hdl); + + sc->sc_vidmode = VIDMODE_NONE; + sc->sc_frames_ready = 0; + sc->sc_owner = NULL; + + return (error); +} + +int +video_claim(struct video_softc *sc, struct process *p) +{ + if (sc->sc_owner != NULL && sc->sc_owner != p) { + DPRINTF(("%s: already owned=%p\n", __func__, sc->sc_owner)); + return (EBUSY); + } + + if (sc->sc_owner == NULL) { + sc->sc_owner = p; + DPRINTF(("%s: new owner=%p\n", __func__, sc->sc_owner)); + } + + return (0); +} + int videoprint(void *aux, const char *pnp) { -- 2.20.1