On Sat, 25 Jul 2020 18:27:45 +0100
Laurence Tratt <[email protected]> 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 ...
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 05:02:51 -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 05:02:52 -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, 0, 65535, &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)
