On Wed, 29 Jul 2020 07:45:52 +0200
Marcus Glocker <mar...@nazgul.ch> wrote:
> On Sat, 25 Jul 2020 18:27:45 +0100
> Laurence Tratt <lau...@tratt.net> wrote:
>
> > On Sat, Jul 25, 2020 at 10:27:15AM -0600, Theo de Raadt wrote:
> >
> > Hello Theo,
> >
> > > My primary concern is about a user changing settings which then
> > > persist past close.
> > >
> > > Upon re-open, how do they know what mode they are in?
> > >
> > > I understand the default mode for a camera might not be nice. But
> > > at least it is a default. If the previous use of the camera put
> > > it into a nasty mode, does this mean all new users must first
> > > force default?
> > >
> > > Now you don't know what mode it is in. As a result, you must
> > > *always* demand change into a specific mode. Rather than making
> > > things simpler, doesn't use of a camera become potentially more
> > > complicated?
> >
> > From what I can tell, there are two ways to control a uvideo device:
> >
> > 1) The "semi-persistent" state changes that video(1) can make that
> > affects subsequent apps which access the device. My patch simply
> > makes those state changes possible from the command-line instead of
> > forcing the user to open a video and hold down keys until they reach
> > the desired state. In otReset all supported controls to their
> > default value and quit.her words, it doesn't change how you control
> > the device: "-c reset" is equivalent to running video(1) and
> > pressing "r", for example.
> >
> > 2) Control via a loopback device. For example, on Windows,
> > Logitech allow you to change controls in their app where you can
> > see video; they then expose a second internal device which other
> > apps can use; I think controls are reset when the Logitech app is
> > closed.
> >
> > On Linux I believe v4l2-ctl works as video(1) does (semi-persistent
> > state changes) but Linux also has video loopback devices. Presumably
> > they could do something similar to the Logitech Windows app, but I
> > don't know if they do so or not.
> >
> > Unless we develop a loopback facility (or, perhaps, some sort of
> > uvideo daemon roughly equivalent to sndiod), I don't think we have
> > much choice but to continue with the semi-persistent state changes
> > that video(1) has always been capable of. It is a bit icky, but it's
> > the only way, for example, to change a webcam's brightness before
> > taking a video call in a web browser.
>
> OK - Let me try to pick this thread up once more :-)
>
> Lets assume we leave the current behaviour of video(1) as is, which
> doesn't reset back the default control values on device close.
>
> This adapted patch adds two new options and the ability to set
> multiple control values on the CLI with a similar interface as
> sysctl(8) has:
>
> * c: List all current supported control values and quit
> (-q is already occupied).
>
> * d: Reset all supported controls to their default value and quit
> (-r is already occupied).
> -> This does the same thing as when pressing 'r' in the GUI.
>
> Some examples:
>
> $ doas video -c
> brightness=128
> contrast=32
> saturation=64
> hue=0
> gamma=120
> sharpness=2
> white_balance_temperature=4000
> $
>
> $ doas video brightness=200 sharpness=4
> brightness: 128 -> 200
> sharpness: 2 -> 4
> $
>
> $ doas video brightness
> brightness: 200
> $
>
> $ doas video -c
> brightness=200
> contrast=32
> saturation=64
> hue=0
> gamma=120
> sharpness=4
> white_balance_temperature=4000
> $
>
> $ doas video -d
> $
>
> $ doas video -c
> brightness=128
> contrast=32
> saturation=64
> hue=0
> gamma=120
> sharpness=2
> white_balance_temperature=4000
> $
>
> $ doas video -f /dev/video1 gain=1
> gain: 0 -> 1
> $
>
> $ doas video -f /dev/video1 -c
> brightness=128
> contrast=128
> saturation=128
> gain=1
> sharpness=128
> white_balance_temperature=4000
> $
>
> What we could think about optionally is to reset to the default
> control values on device close when we did use the GUI mode, but
> leave the controls sticky when we set them through the CLI, and
> explicitly document that in the video(1) man page for the
> [control[=value]] arguments.
>
> But entirely removing the sticky controls I think is odd since as
> already mentioned before, it will remove the ability to preset any
> controls before we enter an application where we can't change them,
> like in an browser. And web conference calls are pretty common
> nowadays ...
Slightly adapted diff; Negative numbers can happen on controls.
Index: video.1
===================================================================
RCS file: /cvs/xenocara/app/video/video.1,v
retrieving revision 1.15
diff -u -p -u -p -r1.15 video.1
--- video.1 17 Jul 2020 07:51:23 -0000 1.15
+++ video.1 29 Jul 2020 20:47:28 -0000
@@ -25,7 +25,7 @@
.Sh SYNOPSIS
.Nm
.Bk -words
-.Op Fl \&gqRv
+.Op Fl \&cdgqRv
.Op Fl a Ar adaptor
.Op Fl e Ar encoding
.Op Fl f Ar file
@@ -34,6 +34,7 @@
.Op Fl o Ar output
.Op Fl r Ar rate
.Op Fl s Ar size
+.Op Ar control Ns Op = Ns Ar value
.Ek
.Sh DESCRIPTION
.Nm
@@ -81,6 +82,10 @@ Index of
adaptor to use.
The default is 0, the first adaptor reported by
.Xr X 7 .
+.It Fl c
+List all current supported control values and quit.
+.It Fl d
+Reset all supported controls to their default value and quit.
.It Fl e Ar encoding
Lowercase FOURCC name of video encoding to use.
Valid arguments are
@@ -219,6 +224,14 @@ Verbose mode.
Multiple instances of this option are allowed.
Each instance increases the level of informational output printed to
.Ar stderr .
+.It Ar control Ns Op = Ns Ar value
+Retrieve the specified
+.Ar control ,
+or attempt to set it to
+.Ar value .
+Multiple
+.Ar control Ns Op = Ns Ar value
+arguments may be given.
.El
.Pp
.Nm
Index: video.c
===================================================================
RCS file: /cvs/xenocara/app/video/video.c,v
retrieving revision 1.31
diff -u -p -u -p -r1.31 video.c
--- video.c 17 Jul 2020 07:51:23 -0000 1.31
+++ video.c 29 Jul 2020 20:47:28 -0000
@@ -192,6 +192,8 @@ struct video {
#define M_IN_FILE 0x4
#define M_OUT_FILE 0x8
#define M_QUERY 0x10
+#define M_QUERY_CTRLS 0x20
+#define M_RESET 0x40
int mode;
int verbose;
};
@@ -211,11 +213,14 @@ int dev_get_rates(struct video *);
int dev_get_ctrls(struct video *);
void dev_dump_info(struct video *);
void dev_dump_query(struct video *);
+void dev_dump_query_ctrls(struct video *);
int dev_init(struct video *);
-void dev_set_ctrl(struct video *, int, int);
-void dev_set_ctrl_auto_white_balance(struct video *, int);
+int dev_set_ctrl_abs(struct video *vid, int, int);
+void dev_set_ctrl_rel(struct video *, int, int);
+void dev_set_ctrl_auto_white_balance(struct video *, int, int);
void dev_reset_ctrls(struct video *);
+int parse_ctrl(struct video *, int, char **);
int parse_size(struct video *);
int choose_size(struct video *);
int choose_enc(struct video *);
@@ -240,10 +245,10 @@ extern char *__progname;
void
usage(void)
{
- fprintf(stderr, "usage: %s [-gqRv] "
+ fprintf(stderr, "usage: %s [-cdgqRv] "
"[-a adaptor] [-e encoding] [-f file] [-i input] [-O output]\n"
- " %*s [-o output] [-r rate] [-s size]\n", __progname,
- (int)strlen(__progname), "");
+ " %*s [-o output] [-r rate] [-s size] [control[=value]]\n",
+ __progname, (int)strlen(__progname), "");
}
int
@@ -657,46 +662,46 @@ display_event(struct video *vid)
switch (str) {
case 'A':
if (vid->mode & M_IN_DEV)
- dev_set_ctrl(vid, CTRL_SHARPNESS, 1);
+ dev_set_ctrl_rel(vid, CTRL_SHARPNESS,
1);
break;
case 'a':
if (vid->mode & M_IN_DEV)
- dev_set_ctrl(vid, CTRL_SHARPNESS, -1);
+ dev_set_ctrl_rel(vid, CTRL_SHARPNESS,
-1);
break;
case 'B':
if (vid->mode & M_IN_DEV)
- dev_set_ctrl(vid, CTRL_BRIGHTNESS, 1);
+ dev_set_ctrl_rel(vid, CTRL_BRIGHTNESS,
1);
break;
case 'b':
if (vid->mode & M_IN_DEV)
- dev_set_ctrl(vid, CTRL_BRIGHTNESS, -1);
+ dev_set_ctrl_rel(vid, CTRL_BRIGHTNESS,
-1);
break;
case 'C':
if (vid->mode & M_IN_DEV)
- dev_set_ctrl(vid, CTRL_CONTRAST, 1);
+ dev_set_ctrl_rel(vid, CTRL_CONTRAST, 1);
break;
case 'c':
if (vid->mode & M_IN_DEV)
- dev_set_ctrl(vid, CTRL_CONTRAST, -1);
+ dev_set_ctrl_rel(vid, CTRL_CONTRAST,
-1);
break;
case 'f':
resize_window(vid, 1);
break;
case 'G':
if (vid->mode & M_IN_DEV)
- dev_set_ctrl(vid, CTRL_GAIN, 1);
+ dev_set_ctrl_rel(vid, CTRL_GAIN, 1);
break;
case 'g':
if (vid->mode & M_IN_DEV)
- dev_set_ctrl(vid, CTRL_GAIN, -1);
+ dev_set_ctrl_rel(vid, CTRL_GAIN, -1);
break;
case 'H':
if (vid->mode & M_IN_DEV)
- dev_set_ctrl(vid, CTRL_HUE, 1);
+ dev_set_ctrl_rel(vid, CTRL_HUE, 1);
break;
case 'h':
if (vid->mode & M_IN_DEV)
- dev_set_ctrl(vid, CTRL_HUE, -1);
+ dev_set_ctrl_rel(vid, CTRL_HUE, -1);
break;
case 'O':
if (!wout && vid->verbose > 0)
@@ -710,11 +715,11 @@ display_event(struct video *vid)
break;
case 'M':
if (vid->mode & M_IN_DEV)
- dev_set_ctrl(vid, CTRL_GAMMA, 1);
+ dev_set_ctrl_rel(vid, CTRL_GAMMA, 1);
break;
case 'm':
if (vid->mode & M_IN_DEV)
- dev_set_ctrl(vid, CTRL_GAMMA, -1);
+ dev_set_ctrl_rel(vid, CTRL_GAMMA, -1);
break;
case 'p':
hold = !hold;
@@ -728,20 +733,20 @@ display_event(struct video *vid)
break;
case 'S':
if (vid->mode & M_IN_DEV)
- dev_set_ctrl(vid, CTRL_SATURATION, 1);
+ dev_set_ctrl_rel(vid, CTRL_SATURATION,
1);
break;
case 's':
if (vid->mode & M_IN_DEV)
- dev_set_ctrl(vid, CTRL_SATURATION, -1);
+ dev_set_ctrl_rel(vid, CTRL_SATURATION,
-1);
break;
case 'W':
if (vid->mode & M_IN_DEV)
- dev_set_ctrl(vid,
+ dev_set_ctrl_rel(vid,
CTRL_WHITE_BALANCE_TEMPERATURE, 10);
break;
case 'w':
if (vid->mode & M_IN_DEV)
- dev_set_ctrl(vid,
+ dev_set_ctrl_rel(vid,
CTRL_WHITE_BALANCE_TEMPERATURE,
-10);
break;
default:
@@ -1009,30 +1014,28 @@ dev_get_ctrls(struct video *vid)
return 1;
}
-void
-dev_set_ctrl(struct video *vid, int ctrl, int change)
+int
+dev_set_ctrl_abs(struct video *vid, int ctrl, int val)
{
struct dev *d = &vid->dev;
struct v4l2_control control;
- int val;
if (ctrl < 0 || ctrl >= CTRL_LAST) {
warnx("invalid control");
- return;
+ return -1;
}
if (!ctrls[ctrl].supported) {
warnx("control %s not supported by %s",
ctrls[ctrl].name, d->path);
- return;
+ return -1;
}
if (ctrl == CTRL_WHITE_BALANCE_TEMPERATURE) {
/*
* The spec requires auto-white balance to be off before
* we can set the white balance temperature.
*/
- dev_set_ctrl_auto_white_balance(vid, 0);
+ dev_set_ctrl_auto_white_balance(vid, 0, 0);
}
- val = ctrls[ctrl].cur + ctrls[ctrl].step * change;
if (val > ctrls[ctrl].max)
val = ctrls[ctrl].max;
else if (val < ctrls[ctrl].min)
@@ -1041,34 +1044,62 @@ dev_set_ctrl(struct video *vid, int ctrl
control.value = val;
if (ioctl(d->fd, VIDIOC_S_CTRL, &control) != 0) {
warn("VIDIOC_S_CTRL");
- return;
+ return -1;
}
control.id = ctrls[ctrl].id;
if (ioctl(d->fd, VIDIOC_G_CTRL, &control) != 0) {
warn("VIDIOC_G_CTRL");
- return;
+ return -1;
}
ctrls[ctrl].cur = control.value;
if (vid->verbose > 0)
fprintf(stderr, "%s now %d\n", ctrls[ctrl].name,
ctrls[ctrl].cur);
+
+ return 0;
}
void
-dev_set_ctrl_auto_white_balance(struct video *vid, int toggle)
+dev_set_ctrl_rel(struct video *vid, int ctrl, int change)
+{
+ struct dev *d = &vid->dev;
+ int val;
+
+ if (ctrl < 0 || ctrl >= CTRL_LAST) {
+ warnx("invalid control");
+ return;
+ }
+ if (!ctrls[ctrl].supported) {
+ warnx("control %s not supported by %s",
+ ctrls[ctrl].name, d->path);
+ return;
+ }
+ val = ctrls[ctrl].cur + ctrls[ctrl].step * change;
+ dev_set_ctrl_abs(vid, ctrl, val);
+}
+
+void
+dev_set_ctrl_auto_white_balance(struct video *vid, int value, int reset)
{
struct dev *d = &vid->dev;
struct v4l2_control control;
control.id = V4L2_CID_AUTO_WHITE_BALANCE;
- if (ioctl(d->fd, VIDIOC_G_CTRL, &control) != 0)
+ if (ioctl(d->fd, VIDIOC_G_CTRL, &control) != 0) {
warn("VIDIOC_G_CTRL");
- if (control.value == toggle)
return;
+ }
- control.value = toggle;
- if (ioctl(d->fd, VIDIOC_S_CTRL, &control) != 0)
- warn("VIDIOC_S_CTRL");
+ if (reset) {
+ if (ioctl(d->fd, VIDIOC_S_CTRL, &control) != 0)
+ warn("VIDIOC_S_CTRL");
+ } else {
+ if (control.value == value)
+ return;
+ control.value = value;
+ if (ioctl(d->fd, VIDIOC_S_CTRL, &control) != 0)
+ warn("VIDIOC_S_CTRL");
+ }
}
void
@@ -1081,27 +1112,9 @@ dev_reset_ctrls(struct video *vid)
for (i = 0; i < CTRL_LAST; i++) {
if (!ctrls[i].supported)
continue;
+ dev_set_ctrl_abs(vid, i, ctrls[i].def);
if (i == CTRL_WHITE_BALANCE_TEMPERATURE) {
- /*
- * We might be asked to reset before the white balance
- * temperature has been adjusted, so we need to make
- * sure that auto-white balance really is off.
- */
- dev_set_ctrl_auto_white_balance(vid, 0);
- }
- control.id = ctrls[i].id;
- control.value = ctrls[i].def;
- if (ioctl(d->fd, VIDIOC_S_CTRL, &control) != 0)
- warn("VIDIOC_S_CTRL(%s)", ctrls[i].name);
- control.id = ctrls[i].id;
- if (ioctl(d->fd, VIDIOC_G_CTRL, &control) != 0)
- warn("VIDIOC_G_CTRL(%s)", ctrls[i].name);
- ctrls[i].cur = control.value;
- if (vid->verbose > 0)
- fprintf(stderr, "%s now %d\n", ctrls[i].name,
- ctrls[i].cur);
- if (i == CTRL_WHITE_BALANCE_TEMPERATURE) {
- dev_set_ctrl_auto_white_balance(vid, 1);
+ dev_set_ctrl_auto_white_balance(vid, 1, 0);
}
}
}
@@ -1171,6 +1184,22 @@ dev_dump_query(struct video *vid)
dev_dump_info(vid);
}
+void
+dev_dump_query_ctrls(struct video *vid)
+{
+ int i;
+
+ if (!dev_check_caps(vid))
+ return;
+ if (!dev_get_ctrls(vid))
+ return;
+
+ for (i = 0; i < CTRL_LAST; i++) {
+ if (ctrls[i].supported)
+ fprintf(stderr, "%s=%d\n", ctrls[i].name, ctrls[i].cur);
+ }
+}
+
int
dev_init(struct video *vid)
{
@@ -1222,6 +1251,64 @@ dev_init(struct video *vid)
}
int
+parse_ctrl(struct video *vid, int argc, char **argv)
+{
+ int i, val_old, val_new;
+ char *p;
+ const char *errstr;
+
+ if (*argv == NULL)
+ return 1; /* No control arguments found. */
+
+ if (!dev_check_caps(vid))
+ return 0;
+ if (!dev_get_ctrls(vid))
+ return 0;
+
+ for (; argc > 0; argc--, argv++) {
+ p = strchr(*argv, '=');
+
+ /* Display control value. */
+ if (p == NULL) {
+ for (i = 0; i < CTRL_LAST; i++) {
+ if (!strcmp(*argv, ctrls[i].name)) {
+ fprintf(stderr, "%s: %d\n",
+ ctrls[i].name, ctrls[i].cur);
+ break;
+ }
+ }
+ if (i == CTRL_LAST)
+ warnx("%s: unknown control", *argv);
+ continue;
+ }
+
+ /* Set control value. */
+ for (i = 0, *p++ = '\0'; i < CTRL_LAST; i++) {
+ if (strcmp(*argv, ctrls[i].name) != 0)
+ continue;
+ if (*p == '\0') {
+ warnx("%s: no value", *argv);
+ break;
+ }
+ val_new = strtonum(p, -32768, 32768, &errstr);
+ if (errstr != NULL) {
+ warnx("%s: %s", *argv, errstr);
+ return 0;
+ }
+ val_old = ctrls[i].cur;
+ if (dev_set_ctrl_abs(vid, i, val_new) == 0)
+ fprintf(stderr, "%s: %d -> %d\n",
+ ctrls[i].name, val_old, ctrls[i].cur);
+ break;
+ }
+ if (i == CTRL_LAST)
+ warnx("%s: unknown control", *argv);
+ }
+
+ return 0;
+}
+
+int
parse_size(struct video *vid)
{
struct xdsp *x = &vid->xdsp;
@@ -1564,6 +1651,13 @@ setup(struct video *vid)
return 0;
}
+ /*
+ * Reset the current White Balance Temperature Auto Control value
+ * after the video stream has been started since some cams only
+ * process this control while the video stream is on.
+ */
+ dev_set_ctrl_auto_white_balance(vid, 0, 1);
+
if (vid->mode & M_OUT_XV)
net_wm_supported(vid);
@@ -1949,7 +2043,7 @@ main(int argc, char *argv[])
vid.mmap_on = 1; /* mmap method is default */
wout = 1;
- while ((ch = getopt(argc, argv, "gqRva:e:f:i:O:o:r:s:")) != -1) {
+ while ((ch = getopt(argc, argv, "cdgqRva:e:f:i:O:o:r:s:")) != -1) {
switch (ch) {
case 'a':
x->cur_adap = strtonum(optarg, 0, 4, &errstr);
@@ -1958,6 +2052,14 @@ main(int argc, char *argv[])
errs++;
}
break;
+ case 'c':
+ vid.mode |= M_QUERY_CTRLS;
+ vid.mode &= ~M_OUT_XV;
+ break;
+ case 'd':
+ vid.mode |= M_RESET;
+ vid.mode &= ~M_OUT_XV;
+ break;
case 'e':
vid.enc = find_enc(optarg);
if (vid.enc >= ENC_LAST) {
@@ -2043,6 +2145,14 @@ main(int argc, char *argv[])
argc -= optind;
argv += optind;
+ if (!parse_ctrl(&vid, argc, argv))
+ cleanup(&vid, 0);
+
+ if (vid.mode & M_QUERY_CTRLS) {
+ dev_dump_query_ctrls(&vid);
+ cleanup(&vid, 0);
+ }
+
if (vid.mode & M_QUERY) {
if (pledge("stdio rpath wpath video", NULL) == -1)
err(1, "pledge");
@@ -2055,6 +2165,11 @@ main(int argc, char *argv[])
if (!setup(&vid))
cleanup(&vid, 1);
+
+ if (vid.mode & M_RESET) {
+ dev_reset_ctrls(&vid);
+ cleanup(&vid, 0);
+ }
if (vid.mode & M_IN_FILE) {
if (pledge("stdio rpath", NULL) == -1)
