Thanks for testing Laurie. Otherwise I got not much feedback -- No interest in this?
What I like about the approach are mainly two things: 1. We're aligning to audio(4), which has the kern.audio.record toggle already in place. If we have a toggle for audio recording, why not having one for video recording? 2. If we would decide to get kern.video.record in, as a next step I would like to propose changing the /dev/video* permissions so we don't require root access to execute video programs, by adding a new group 'video' and apply it together with 660 permissions to /dev/video*. By that we require initial root access to enable video recording, but can then permit non-root accounts for video access. On Wed, 16 Dec 2020 15:57:20 +0000 Laurence Tratt <[email protected]> wrote: > On Wed, Dec 16, 2020 at 03:50:54PM +0100, Marcus Glocker wrote: > > Hello Marcus, > > > I reviewed this again, and I think this implementation should be > > consistent on a video(4) level now. > > I've just tried this, and it works, and works better than my original > (and uvideo-only) patch. I've been flipping it on and off and > video(1), ffplay, Firefox, and Chrome all work fine with it even when > they're midway through using the stream. [Of mild interest, it seems > that you turn kern.video.record to 0, ffplay and Firefox keep > displaying the last frame while video(1) and Chrome display a solid > green picture.] > > This certainly supersedes my patch and has my OK, not that it needs > it :) > > > Laurie Index: sys/dev/video.c =================================================================== RCS file: /cvs/src/sys/dev/video.c,v retrieving revision 1.44 diff -u -p -u -p -r1.44 video.c --- sys/dev/video.c 16 May 2020 10:47:22 -0000 1.44 +++ sys/dev/video.c 16 Dec 2020 14:05:11 -0000 @@ -51,6 +51,7 @@ struct video_softc { int sc_fsize; uint8_t *sc_fbuffer; + caddr_t sc_fbuffer_mmap; size_t sc_fbufferlen; int sc_vidmode; /* access mode */ #define VIDMODE_NONE 0 @@ -78,6 +79,11 @@ struct cfdriver video_cd = { NULL, "video", DV_DULL }; +/* + * Global flag to control if video recording is enabled by kern.video.record. + */ +int video_record_enable = 0; + int videoprobe(struct device *parent, void *match, void *aux) { @@ -191,6 +197,8 @@ videoread(dev_t dev, struct uio *uio, in /* move no more than 1 frame to userland, as per specification */ size = ulmin(uio->uio_resid, sc->sc_fsize); + if (!video_record_enable) + bzero(sc->sc_fbuffer, size); error = uiomove(sc->sc_fbuffer, size, uio); sc->sc_frames_ready--; if (error) @@ -205,6 +213,7 @@ int videoioctl(dev_t dev, u_long cmd, caddr_t data, int flags, struct proc *p) { struct video_softc *sc; + struct v4l2_buffer *vb = (struct v4l2_buffer *)data; int unit, error; unit = VIDEOUNIT(dev); @@ -299,6 +308,8 @@ videoioctl(dev_t dev, u_long cmd, caddr_ } error = (sc->hw_if->dqbuf)(sc->hw_hdl, (struct v4l2_buffer *)data); + if (!video_record_enable) + bzero(sc->sc_fbuffer_mmap + vb->m.offset, vb->length); sc->sc_frames_ready--; break; case VIDIOC_STREAMON: @@ -408,6 +419,10 @@ videommap(dev_t dev, off_t off, int prot if (pmap_extract(pmap_kernel(), (vaddr_t)p, &pa) == FALSE) panic("videommap: invalid page"); sc->sc_vidmode = VIDMODE_MMAP; + + /* store frame buffer base address for later blanking */ + if (off == 0) + sc->sc_fbuffer_mmap = p; return (pa); } Index: sys/kern/kern_sysctl.c =================================================================== RCS file: /cvs/src/sys/kern/kern_sysctl.c,v retrieving revision 1.384 diff -u -p -u -p -r1.384 kern_sysctl.c --- sys/kern/kern_sysctl.c 7 Dec 2020 16:55:29 -0000 1.384 +++ sys/kern/kern_sysctl.c 16 Dec 2020 14:05:12 -0000 @@ -114,6 +114,7 @@ #endif #include "audio.h" +#include "video.h" #include "pf.h" extern struct forkstat forkstat; @@ -125,6 +126,9 @@ extern long numvnodes; #if NAUDIO > 0 extern int audio_record_enable; #endif +#if NVIDEO > 0 +extern int video_record_enable; +#endif int allowkmem; int allowdt; @@ -141,6 +145,9 @@ int sysctl_cptime2(int *, u_int, void *, #if NAUDIO > 0 int sysctl_audio(int *, u_int, void *, size_t *, void *, size_t); #endif +#if NVIDEO > 0 +int sysctl_video(int *, u_int, void *, size_t *, void *, size_t); +#endif int sysctl_cpustats(int *, u_int, void *, size_t *, void *, size_t); int sysctl_utc_offset(void *, size_t *, void *, size_t); @@ -379,6 +386,7 @@ kern_sysctl(int *name, u_int namelen, vo case KERN_FILE: case KERN_WITNESS: case KERN_AUDIO: + case KERN_VIDEO: case KERN_CPUSTATS: break; default: @@ -642,6 +650,11 @@ kern_sysctl(int *name, u_int namelen, vo return (sysctl_audio(name + 1, namelen - 1, oldp, oldlenp, newp, newlen)); #endif +#if NVIDEO > 0 + case KERN_VIDEO: + return (sysctl_video(name + 1, namelen - 1, oldp, oldlenp, + newp, newlen)); +#endif case KERN_CPUSTATS: return (sysctl_cpustats(name + 1, namelen - 1, oldp, oldlenp, newp, newlen)); @@ -2440,6 +2453,21 @@ sysctl_audio(int *name, u_int namelen, v return (ENOENT); return (sysctl_int(oldp, oldlenp, newp, newlen, &audio_record_enable)); +} +#endif + +#if NVIDEO > 0 +int +sysctl_video(int *name, u_int namelen, void *oldp, size_t *oldlenp, + void *newp, size_t newlen) +{ + if (namelen != 1) + return (ENOTDIR); + + if (name[0] != KERN_VIDEO_RECORD) + return (ENOENT); + + return (sysctl_int(oldp, oldlenp, newp, newlen, &video_record_enable)); } #endif Index: sys/sys/sysctl.h =================================================================== RCS file: /cvs/src/sys/sys/sysctl.h,v retrieving revision 1.212 diff -u -p -u -p -r1.212 sysctl.h --- sys/sys/sysctl.h 7 Nov 2020 05:24:20 -0000 1.212 +++ sys/sys/sysctl.h 16 Dec 2020 14:05:13 -0000 @@ -189,7 +189,8 @@ struct ctlname { #define KERN_PFSTATUS 86 /* struct: pf status and stats */ #define KERN_TIMEOUT_STATS 87 /* struct: timeout status and stats */ #define KERN_UTC_OFFSET 88 /* int: adjust RTC time to UTC */ -#define KERN_MAXID 89 /* number of valid kern ids */ +#define KERN_VIDEO 89 /* struct: video properties */ +#define KERN_MAXID 90 /* number of valid kern ids */ #define CTL_KERN_NAMES { \ { 0, 0 }, \ @@ -281,6 +282,7 @@ struct ctlname { { "pfstatus", CTLTYPE_STRUCT }, \ { "timeout_stats", CTLTYPE_STRUCT }, \ { "utc_offset", CTLTYPE_INT }, \ + { "video", CTLTYPE_STRUCT }, \ } /* @@ -318,6 +320,17 @@ struct ctlname { #define KERN_AUDIO_MAXID 2 #define CTL_KERN_AUDIO_NAMES { \ + { 0, 0 }, \ + { "record", CTLTYPE_INT }, \ +} + +/* + * KERN_VIDEO + */ +#define KERN_VIDEO_RECORD 1 +#define KERN_VIDEO_MAXID 2 + +#define CTL_KERN_VIDEO_NAMES { \ { 0, 0 }, \ { "record", CTLTYPE_INT }, \ } Index: sbin/sysctl/sysctl.c =================================================================== RCS file: /cvs/src/sbin/sysctl/sysctl.c,v retrieving revision 1.253 diff -u -p -u -p -r1.253 sysctl.c --- sbin/sysctl/sysctl.c 17 Nov 2020 12:11:04 -0000 1.253 +++ sbin/sysctl/sysctl.c 16 Dec 2020 12:54:40 -0000 @@ -130,6 +130,7 @@ struct ctlname machdepname[] = CTL_MACHD #endif struct ctlname ddbname[] = CTL_DDB_NAMES; struct ctlname audioname[] = CTL_KERN_AUDIO_NAMES; +struct ctlname videoname[] = CTL_KERN_VIDEO_NAMES; struct ctlname witnessname[] = CTL_KERN_WITNESS_NAMES; char names[BUFSIZ]; int lastused; @@ -219,6 +220,7 @@ void print_sensor(struct sensor *); int sysctl_chipset(char *, char **, int *, int, int *); #endif int sysctl_audio(char *, char **, int *, int, int *); +int sysctl_video(char *, char **, int *, int, int *); int sysctl_witness(char *, char **, int *, int, int *); void vfsinit(void); @@ -517,6 +519,11 @@ parse(char *string, int flags) if (len < 0) return; break; + case KERN_VIDEO: + len = sysctl_video(string, &bufp, mib, flags, &type); + if (len < 0) + return; + break; case KERN_WITNESS: len = sysctl_witness(string, &bufp, mib, flags, &type); if (len < 0) @@ -1766,6 +1773,7 @@ struct list shmlist = { shmname, KERN_SH struct list watchdoglist = { watchdogname, KERN_WATCHDOG_MAXID }; struct list tclist = { tcname, KERN_TIMECOUNTER_MAXID }; struct list audiolist = { audioname, KERN_AUDIO_MAXID }; +struct list videolist = { videoname, KERN_VIDEO_MAXID }; struct list witnesslist = { witnessname, KERN_WITNESS_MAXID }; /* @@ -2813,6 +2821,25 @@ sysctl_audio(char *string, char **bufpp, return (-1); mib[2] = indx; *typep = audiolist.list[indx].ctl_type; + return (3); +} + +/* + * Handle video support + */ +int +sysctl_video(char *string, char **bufpp, int mib[], int flags, int *typep) +{ + int indx; + + if (*bufpp == NULL) { + listall(string, &videolist); + return (-1); + } + if ((indx = findname(string, "third", bufpp, &videolist)) == -1) + return (-1); + mib[2] = indx; + *typep = videolist.list[indx].ctl_type; return (3); }
