Re: RFC: kern.video.record
On Sat, Oct 24, 2020 at 01:17:05PM -0600, Theo de Raadt wrote:
> Seems better.
>
> Toggling the behaviour while video occurs should be tested, to make
> sure it matches expectations.
Right, works already for read(2) but for mmap(2) we need to move the
check to the VIDIOC_DQBUF ioctl(2) then.
I can bake a fresh, full diff for further testing.
> > > > utvfu(4) comes in mind.
> > >
> > > And therefore, it makes no sense the diff contains change to uvideo(4)
> > > only. I know video(4) is a thin shim, but the blocking logic isn't
> > > correctly placed.
> >
> > I agree.
> >
> > Why don't we simply deny turning on the cams video stream when
> > kern.video.record=0 in video(1)? See diff.
> >
> > imac:~ [hacki] $ doas video
> > video: ioctl VIDIOC_STREAMON: Operation not permitted
> >
> > > One more comment. If the bcopy's are skipped, what data is in those
> > > buffers? Are they zero'd freshly, or allocated with a M_ZERO type
> > > allocation, or what's the story? Or if one toggles the control do
> > > the buffers continue to show old records rather than 'black'?
> >
> > The video buffers aren't initialized ever in uvideo(4).
> > But with the above suggestion, we don't need to worry about the buffers
> > anymore, they simply won't get passed (nor even filled).
Index: video.c
===
RCS file: /cvs/src/sys/dev/video.c,v
retrieving revision 1.44
diff -u -p -u -p -r1.44 video.c
--- video.c 16 May 2020 10:47:22 - 1.44
+++ video.c 24 Oct 2020 19:36:17 -
@@ -40,6 +40,12 @@
#define DPRINTF(x)
#endif
+/*
+ * Global flag to control if audio recording is enabled when the
+ * mixerctl setting is record.enable=sysctl
+ */
+int video_record_enable = 0;
+
struct video_softc {
struct devicedev;
void*hw_hdl;/* hardware driver handle */
@@ -159,6 +165,9 @@ videoread(dev_t dev, struct uio *uio, in
int unit, error;
size_t size;
+ if (!video_record_enable)
+ return (EPERM);
+
unit = VIDEOUNIT(dev);
if (unit >= video_cd.cd_ndevs ||
(sc = video_cd.cd_devs[unit]) == NULL)
@@ -290,6 +299,8 @@ videoioctl(dev_t dev, u_long cmd, caddr_
(struct v4l2_buffer *)data);
break;
case VIDIOC_DQBUF:
+ if (!video_record_enable)
+ return (EPERM);
if (!sc->hw_if->dqbuf)
break;
/* should have called mmap() before now */
Re: RFC: kern.video.record
Seems better.
Toggling the behaviour while video occurs should be tested, to make
sure it matches expectations.
Marcus Glocker wrote:
> On Sat, Oct 24, 2020 at 11:34:07AM -0600, Theo de Raadt wrote:
>
> > Marcus Glocker wrote:
> >
> > > On Sun, Sep 27, 2020 at 05:57:51PM +0100, Laurence Tratt wrote:
> > >
> > > > On Sun, Sep 13, 2020 at 09:23:36AM +0100, Laurence Tratt wrote:
> > > >
> > > > > Since I recently opened my big fat mouth and suggested that
> > > > > "kern.video.record" (analogous to kern.audio.record) might be a good
> > > > > idea, I
> > > > > decided to put together a quick prototype (heavily based on the
> > > > > kern.audio.record code). This at least roughly works for me but
> > > > > raises some
> > > > > questions such as:
> > > > >
> > > > > * Is uvideo the only driver that can capture video? [I imagine not,
> > > > > but I
> > > > > don't really know.]
> > >
> > > utvfu(4) comes in mind.
> >
> > And therefore, it makes no sense the diff contains change to uvideo(4)
> > only. I know video(4) is a thin shim, but the blocking logic isn't
> > correctly placed.
>
> I agree.
>
> Why don't we simply deny turning on the cams video stream when
> kern.video.record=0 in video(1)? See diff.
>
> imac:~ [hacki] $ doas video
> video: ioctl VIDIOC_STREAMON: Operation not permitted
>
> > One more comment. If the bcopy's are skipped, what data is in those
> > buffers? Are they zero'd freshly, or allocated with a M_ZERO type
> > allocation, or what's the story? Or if one toggles the control do
> > the buffers continue to show old records rather than 'black'?
>
> The video buffers aren't initialized ever in uvideo(4).
> But with the above suggestion, we don't need to worry about the buffers
> anymore, they simply won't get passed (nor even filled).
>
>
> Index: video.c
> ===
> RCS file: /cvs/src/sys/dev/video.c,v
> retrieving revision 1.44
> diff -u -p -u -p -r1.44 video.c
> --- video.c 16 May 2020 10:47:22 - 1.44
> +++ video.c 24 Oct 2020 19:08:28 -
> @@ -40,6 +40,12 @@
> #define DPRINTF(x)
> #endif
>
> +/*
> + * Global flag to control if audio recording is enabled when the
> + * mixerctl setting is record.enable=sysctl
> + */
> +int video_record_enable = 0;
> +
> struct video_softc {
> struct devicedev;
> void*hw_hdl;/* hardware driver handle */
> @@ -159,6 +165,9 @@ videoread(dev_t dev, struct uio *uio, in
> int unit, error;
> size_t size;
>
> + if (!video_record_enable)
> + return (EPERM);
> +
> unit = VIDEOUNIT(dev);
> if (unit >= video_cd.cd_ndevs ||
> (sc = video_cd.cd_devs[unit]) == NULL)
> @@ -302,6 +311,8 @@ videoioctl(dev_t dev, u_long cmd, caddr_
> sc->sc_frames_ready--;
> break;
> case VIDIOC_STREAMON:
> + if (!video_record_enable)
> + return (EPERM);
> if (sc->hw_if->streamon)
> error = (sc->hw_if->streamon)(sc->hw_hdl,
> (int)*data);
Re: RFC: kern.video.record
On Sat, Oct 24, 2020 at 11:34:07AM -0600, Theo de Raadt wrote:
> Marcus Glocker wrote:
>
> > On Sun, Sep 27, 2020 at 05:57:51PM +0100, Laurence Tratt wrote:
> >
> > > On Sun, Sep 13, 2020 at 09:23:36AM +0100, Laurence Tratt wrote:
> > >
> > > > Since I recently opened my big fat mouth and suggested that
> > > > "kern.video.record" (analogous to kern.audio.record) might be a good
> > > > idea, I
> > > > decided to put together a quick prototype (heavily based on the
> > > > kern.audio.record code). This at least roughly works for me but raises
> > > > some
> > > > questions such as:
> > > >
> > > > * Is uvideo the only driver that can capture video? [I imagine not,
> > > > but I
> > > > don't really know.]
> >
> > utvfu(4) comes in mind.
>
> And therefore, it makes no sense the diff contains change to uvideo(4)
> only. I know video(4) is a thin shim, but the blocking logic isn't
> correctly placed.
I agree.
Why don't we simply deny turning on the cams video stream when
kern.video.record=0 in video(1)? See diff.
imac:~ [hacki] $ doas video
video: ioctl VIDIOC_STREAMON: Operation not permitted
> One more comment. If the bcopy's are skipped, what data is in those
> buffers? Are they zero'd freshly, or allocated with a M_ZERO type
> allocation, or what's the story? Or if one toggles the control do
> the buffers continue to show old records rather than 'black'?
The video buffers aren't initialized ever in uvideo(4).
But with the above suggestion, we don't need to worry about the buffers
anymore, they simply won't get passed (nor even filled).
Index: video.c
===
RCS file: /cvs/src/sys/dev/video.c,v
retrieving revision 1.44
diff -u -p -u -p -r1.44 video.c
--- video.c 16 May 2020 10:47:22 - 1.44
+++ video.c 24 Oct 2020 19:08:28 -
@@ -40,6 +40,12 @@
#define DPRINTF(x)
#endif
+/*
+ * Global flag to control if audio recording is enabled when the
+ * mixerctl setting is record.enable=sysctl
+ */
+int video_record_enable = 0;
+
struct video_softc {
struct devicedev;
void*hw_hdl;/* hardware driver handle */
@@ -159,6 +165,9 @@ videoread(dev_t dev, struct uio *uio, in
int unit, error;
size_t size;
+ if (!video_record_enable)
+ return (EPERM);
+
unit = VIDEOUNIT(dev);
if (unit >= video_cd.cd_ndevs ||
(sc = video_cd.cd_devs[unit]) == NULL)
@@ -302,6 +311,8 @@ videoioctl(dev_t dev, u_long cmd, caddr_
sc->sc_frames_ready--;
break;
case VIDIOC_STREAMON:
+ if (!video_record_enable)
+ return (EPERM);
if (sc->hw_if->streamon)
error = (sc->hw_if->streamon)(sc->hw_hdl,
(int)*data);
Re: RFC: kern.video.record
Marcus Glocker wrote: > On Sun, Sep 27, 2020 at 05:57:51PM +0100, Laurence Tratt wrote: > > > On Sun, Sep 13, 2020 at 09:23:36AM +0100, Laurence Tratt wrote: > > > > > Since I recently opened my big fat mouth and suggested that > > > "kern.video.record" (analogous to kern.audio.record) might be a good > > > idea, I > > > decided to put together a quick prototype (heavily based on the > > > kern.audio.record code). This at least roughly works for me but raises > > > some > > > questions such as: > > > > > > * Is uvideo the only driver that can capture video? [I imagine not, but > > > I > > > don't really know.] > > utvfu(4) comes in mind. And therefore, it makes no sense the diff contains change to uvideo(4) only. I know video(4) is a thin shim, but the blocking logic isn't correctly placed. One more comment. If the bcopy's are skipped, what data is in those buffers? Are they zero'd freshly, or allocated with a M_ZERO type allocation, or what's the story? Or if one toggles the control do the buffers continue to show old records rather than 'black'?
Re: RFC: kern.video.record
On Sun, Sep 27, 2020 at 05:57:51PM +0100, Laurence Tratt wrote:
> On Sun, Sep 13, 2020 at 09:23:36AM +0100, Laurence Tratt wrote:
>
> > Since I recently opened my big fat mouth and suggested that
> > "kern.video.record" (analogous to kern.audio.record) might be a good idea, I
> > decided to put together a quick prototype (heavily based on the
> > kern.audio.record code). This at least roughly works for me but raises some
> > questions such as:
> >
> > * Is uvideo the only driver that can capture video? [I imagine not, but I
> > don't really know.]
utvfu(4) comes in mind.
Though, I'm not sure how much sense it would make to apply
kern.video.record on it as well since it requires an video input device
first which is playing the video stream.
> > * I've taken the same approach as kern.audio.record which is to keep on
> > handing out data even when kern.video.record=0 *but* it's harder to work
> > out what data we should hand out. At the moment we just pass on whatever
> > happened to be in the buffer beforehand (which is clearly not a good
> > idea in the long term, but proves the point for now). For uncompressed
> > video, handing over (say) an entirely black image is probably easy; for
> > compressed video we'd have to think about whether we also want to
> > manipulate frame headers etc. It would probably be easier to simply
> > intercept the "opening video" event but that would mean that if
> > something is already streaming video, setting kern.video.record=0 would
> > allow it to keep recording.
I also think this can be looked at in a second step.
For now acting same as audio(4) by only skipping the data makes sense
to me.
> > Comments welcome, including "this is a terrible idea full stop"!
>
> It seems that no-one yet thinks this is a terrible idea, and most people
> who've expressed an opinion prefer kern.audio.record and kern.video.record
> to be separate rather than combined.
I also would leave it separated.
> The two "big" questions above remain unanswered but, in the meantime, I'm
> attaching a minor update which fixes a small bug in the patch in sysctl.c
> that Edd Barrett found.
I'm basically OK with the diff, although we would need to add the
documentation peaces as well at one point. I like to have this option
in place parallel to kern.audio.record.
I would like to see more feedback from other developers if they think we
should bring this forward.
> Index: sbin/sysctl/sysctl.c
> ===
> RCS file: /cvs/src/sbin/sysctl/sysctl.c,v
> retrieving revision 1.252
> diff -u -p -u -r1.252 sysctl.c
> --- sbin/sysctl/sysctl.c 15 Jul 2020 07:13:56 - 1.252
> +++ sbin/sysctl/sysctl.c 27 Sep 2020 16:49:54 -
> @@ -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, , mib, flags, );
> + if (len < 0)
> + return;
> + break;
> case KERN_WITNESS:
> len = sysctl_witness(string, , mib, flags, );
> 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, );
> + return (-1);
> + }
> + if ((indx = findname(string, "third", bufpp, )) == -1)
> + return (-1);
> + mib[2] = indx;
> + *typep = videolist.list[indx].ctl_type;
> return (3);
> }
>
>
Re: RFC: kern.video.record
On Sun, Sep 13, 2020 at 09:23:36AM +0100, Laurence Tratt wrote:
> Since I recently opened my big fat mouth and suggested that
> "kern.video.record" (analogous to kern.audio.record) might be a good idea, I
> decided to put together a quick prototype (heavily based on the
> kern.audio.record code). This at least roughly works for me but raises some
> questions such as:
>
> * Is uvideo the only driver that can capture video? [I imagine not, but I
> don't really know.]
>
> * I've taken the same approach as kern.audio.record which is to keep on
> handing out data even when kern.video.record=0 *but* it's harder to work
> out what data we should hand out. At the moment we just pass on whatever
> happened to be in the buffer beforehand (which is clearly not a good
> idea in the long term, but proves the point for now). For uncompressed
> video, handing over (say) an entirely black image is probably easy; for
> compressed video we'd have to think about whether we also want to
> manipulate frame headers etc. It would probably be easier to simply
> intercept the "opening video" event but that would mean that if
> something is already streaming video, setting kern.video.record=0 would
> allow it to keep recording.
>
> Comments welcome, including "this is a terrible idea full stop"!
It seems that no-one yet thinks this is a terrible idea, and most people
who've expressed an opinion prefer kern.audio.record and kern.video.record
to be separate rather than combined.
The two "big" questions above remain unanswered but, in the meantime, I'm
attaching a minor update which fixes a small bug in the patch in sysctl.c
that Edd Barrett found.
Laurie
Index: sbin/sysctl/sysctl.c
===
RCS file: /cvs/src/sbin/sysctl/sysctl.c,v
retrieving revision 1.252
diff -u -p -u -r1.252 sysctl.c
--- sbin/sysctl/sysctl.c15 Jul 2020 07:13:56 - 1.252
+++ sbin/sysctl/sysctl.c27 Sep 2020 16:49:54 -
@@ -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, , mib, flags, );
+ if (len < 0)
+ return;
+ break;
case KERN_WITNESS:
len = sysctl_witness(string, , mib, flags, );
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, );
+ return (-1);
+ }
+ if ((indx = findname(string, "third", bufpp, )) == -1)
+ return (-1);
+ mib[2] = indx;
+ *typep = videolist.list[indx].ctl_type;
return (3);
}
Index: sys/dev/usb/uvideo.c
===
RCS file: /cvs/src/sys/dev/usb/uvideo.c,v
retrieving revision 1.209
diff -u -p -u -r1.209 uvideo.c
--- sys/dev/usb/uvideo.c31 Jul 2020 10:49:33 - 1.209
+++ sys/dev/usb/uvideo.c27 Sep 2020 16:49:55 -
@@ -386,6 +386,12 @@ struct uvideo_devs {
#define uvideo_lookup(v, p) \
((struct uvideo_devs *)usb_lookup(uvideo_devs, v, p))
+/*
+ * Global flag to control if video recording is enabled when the
+ * kern.video.record setting is set to 1.
+ */
+int video_record_enable = 0;
+
int
uvideo_enable(void *v)
{
@@ -2210,7 +2216,8 @@ uvideo_vs_decode_stream_header(struct uv
/* save sample */
sample_len = frame_size - sh->bLength;
if ((fb->offset + sample_len) <= fb->buf_size) {
-
Re: RFC: kern.video.record
On Sat, Sep 19, 2020 at 06:07:05PM +0200, Solene Rapenne wrote: > I vote for keeping both so we can enable audio only or video only > and be sure about what is enabled and able to record our environment I agree. I do lots of audio-only calls with one of my colleagues and in that scenario I'd prefer fine-grained control. -- Best Regards Edd Barrett http://www.theunixzoo.co.uk
Re: RFC: kern.video.record
On Sun, 13 Sep 2020 09:23:36 +0100 Laurence Tratt : > Since I recently opened my big fat mouth and suggested that > "kern.video.record" (analogous to kern.audio.record) might be a good idea, I > decided to put together a quick prototype (heavily based on the > kern.audio.record code). This at least roughly works for me but raises some > questions such as: I vote for keeping both so we can enable audio only or video only and be sure about what is enabled and able to record our environment
Re: RFC: kern.video.record
> On Sep 19, 2020, at 10:39 AM, Chris Bennett > wrote: > > On Sat, Sep 19, 2020 at 10:14:55AM +0100, Laurence Tratt wrote: >> >> I agree that it would simplify the code. The reason that I didn't merge them >> is because I know that sometimes people want to record audio but not video (I >> doubt that many people record video without audio). Now, admittedly, this >> isn't necessarily a super-common use case, so it might not be worth having >> two knobs for it, but it might be worth considering. Personally I'm >> completely comfortable with whatever the general consensus is for >> merging/not-merging! > > There are legal reasons for recording video only. Depends on local laws. > I forget the exact details, but Texas laws regarding informing others if > they are being recorded are more permissive than other states as far as > needing to inform. I've only been following this thread lightly, but it > seems relevant to at least throw that information in. > > Chris Bennett > > Admittedly, I have not used audio or video recording on OpenBSD; however, the minimal code duplication is avoidable in other ways. Having two switches to control two separate modalities, as you described, provides more flexibility with respect to truly preventing an application (poorly designed, malicious, etc) from “snooping” on your audio or video. Again, I’m not a user of the recording functionality, so this is me throwing my two cents out there. -Brian
Re: RFC: kern.video.record
On Sat, Sep 19, 2020 at 10:14:55AM +0100, Laurence Tratt wrote: > > I agree that it would simplify the code. The reason that I didn't merge them > is because I know that sometimes people want to record audio but not video (I > doubt that many people record video without audio). Now, admittedly, this > isn't necessarily a super-common use case, so it might not be worth having > two knobs for it, but it might be worth considering. Personally I'm > completely comfortable with whatever the general consensus is for > merging/not-merging! There are legal reasons for recording video only. Depends on local laws. I forget the exact details, but Texas laws regarding informing others if they are being recorded are more permissive than other states as far as needing to inform. I've only been following this thread lightly, but it seems relevant to at least throw that information in. Chris Bennett
Re: RFC: kern.video.record
On Fri, Sep 18, 2020 at 06:50:33AM -0600, Thomas Frohwein wrote: Hello Thomas, Thanks for your comments! > It might be worth considering combining kern.audio.record and > kern.video.record, as they serve the same purpose (just different > modalities). I doubt that granular controls with separate switches will be > very useful at this level; maybe rather leave this differentiation to the > application? > > An added benefit would be that there would be less largely duplicated code; > and probably better maintainability. I agree that it would simplify the code. The reason that I didn't merge them is because I know that sometimes people want to record audio but not video (I doubt that many people record video without audio). Now, admittedly, this isn't necessarily a super-common use case, so it might not be worth having two knobs for it, but it might be worth considering. Personally I'm completely comfortable with whatever the general consensus is for merging/not-merging! Laurie
Re: RFC: kern.video.record
On Sun, Sep 13, 2020 at 09:23:36AM +0100, Laurence Tratt wrote:
> Since I recently opened my big fat mouth and suggested that
> "kern.video.record" (analogous to kern.audio.record) might be a good idea, I
I support this suggestion. I think the idea behind it is based on the
same concerns that led to "kern.audio.record" (though admittedly I
wasn't privy to the conversations leading up to it).
> decided to put together a quick prototype (heavily based on the
> kern.audio.record code). This at least roughly works for me but raises some
> questions such as:
>
> * Is uvideo the only driver that can capture video? [I imagine not, but I
> don't really know.]
>
> * I've taken the same approach as kern.audio.record which is to keep on
> handing out data even when kern.video.record=0 *but* it's harder to work
> out what data we should hand out. At the moment we just pass on whatever
> happened to be in the buffer beforehand (which is clearly not a good
> idea in the long term, but proves the point for now). For uncompressed
> video, handing over (say) an entirely black image is probably easy; for
> compressed video we'd have to think about whether we also want to
> manipulate frame headers etc. It would probably be easier to simply
> intercept the "opening video" event but that would mean that if
> something is already streaming video, setting kern.video.record=0 would
> allow it to keep recording.
I built a kernel and sysctl with this and can confirm it works with my
Logitech C310. Of note, in its current form, running video(1) while
kern.video.record is 0 leads to a window showing mostly green and pink colors.
>
> Comments welcome, including "this is a terrible idea full stop"!
Again, I'm in favor of this idea.
It might be worth considering combining kern.audio.record and
kern.video.record, as they serve the same purpose (just different
modalities). I doubt that granular controls with separate switches will
be very useful at this level; maybe rather leave this differentiation
to the application?
An added benefit would be that there would be less largely duplicated
code; and probably better maintainability.
>
>
> Laurie
>
>
>
> Index: sbin/sysctl/sysctl.c
> ===
> RCS file: /cvs/src/sbin/sysctl/sysctl.c,v
> retrieving revision 1.252
> diff -u -p -r1.252 sysctl.c
> --- sbin/sysctl/sysctl.c 15 Jul 2020 07:13:56 - 1.252
> +++ sbin/sysctl/sysctl.c 13 Sep 2020 08:08:57 -
> @@ -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, , mib, flags, );
> + if (len < 0)
> + return;
> + break;
> case KERN_WITNESS:
> len = sysctl_witness(string, , mib, flags, );
> 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 = { audioname, 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, );
> + return (-1);
> + }
> + if ((indx = findname(string, "third", bufpp, )) == -1)
> + return (-1);
> + mib[2] = indx;
> + *typep = videolist.list[indx].ctl_type;
> return (3);
> }
>
> Index: sys/dev/usb/uvideo.c
> ===
> RCS file: /cvs/src/sys/dev/usb/uvideo.c,v
> retrieving revision 1.209
> diff -u -p -r1.209 uvideo.c
> --- sys/dev/usb/uvideo.c 31 Jul 2020
