On Wed, 22 Jul 2020 21:52:27 +0100
Laurence Tratt <lau...@tratt.net> wrote:
> On Wed, Jul 22, 2020 at 10:23:19PM +0200, Marcus Glocker wrote:
>
> Hello Marcus,
>
> > I've tested this here as well in the meantime by leaving
> > mmap_init() on its original location (doesn't get involved for '-c
> > reset') with three different cams:
> >
> > * Logitech C930e: I see the same problem like you do with your C920
> > finally.
> > * Logitech QuickCam Pro 9000: reset works fine.
> > * SunplusIT Inc Integrated Camera: reset works fine.
>
> Hmph, that's slightly annoying of Logitech :/
>
> > This seems to be a problem only with some cams when turning the
> > auto white balance temperature back on while the stream is off, the
> > setting doesn't get recognized by the cam afterwards.
> >
> > I'm basically OK with your last diff, except the mmap_init()
> > location change: I don't like to turn the cam stream on only for
> > setting this parameter because some cams can't handle the obvious.
> >
> > I tried out some things with the C930e to get the auto white balance
> > temperature back on without having the stream on, but no luck so
> > far.
> >
> > I would aim to get your diff in without the mmap_init() change.
> > Maybe we'll find a solution/workaround for this partial problem
> > later?
>
> If we can find a change that allows this, I'd be happy! When I briefly
> tested things on Windows, the Logitech app there activated the stream
> before changing settings, so it's quite possible that they never
> tested changing some settings with the stream off. v4l2-ctl might
> have some clues in it, but their model is subtly different than ours
> and forces the user to understand a lot more about their camera (e.g.
> they force the user to turn off auto white balance before they allow
> them to set manual white balance, a differentiation which IMHO only
> makes sense if you've read the UVC spec).
>
> That makes me think that if/when uvideo is extended to deal with
> terminal control requests (e.g. zoom, exposure, focus) we'll find
> that other settings with "auto" options have similar problems.
> Honestly, if the price of controlling exposure and focus is having to
> turn the camera stream on, I think I will consider that an acceptable
> trade-off, given how useful those settings are :)
How's that?
We read the current value of the white balance temperature auto control
(since this gets set correctly during the reset), and just reset it
again on the next cam start up after the video stream has been started?
By that we avoid the video stream on/off dance when using '-c'.
I've quickly tested this here and it seems to work for me ...
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 23 Jul 2020 19:44:21 -0000
@@ -27,6 +27,7 @@
.Bk -words
.Op Fl \&gqRv
.Op Fl a Ar adaptor
+.Op Fl c Ar reset | control=value
.Op Fl e Ar encoding
.Op Fl f Ar file
.Op Fl i Ar input
@@ -81,6 +82,15 @@ Index of
adaptor to use.
The default is 0, the first adaptor reported by
.Xr X 7 .
+.It Fl c Ar reset | control=value
+Set control value (e.g. brightness) and exit. The special name
+.Ql reset
+resets all values to their default. The available controls can be found
+with
+.Fl q
+and the default values are displayed during a reset with
+.Fl c Ar reset
+.Fl v .
.It Fl e Ar encoding
Lowercase FOURCC name of video encoding to use.
Valid arguments are
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 23 Jul 2020 19:44:22 -0000
@@ -192,7 +192,10 @@ struct video {
#define M_IN_FILE 0x4
#define M_OUT_FILE 0x8
#define M_QUERY 0x10
+#define M_RESET 0x20
+#define M_SET_CTRL 0x40
int mode;
+ char *set_ctrl_str;
int verbose;
};
@@ -212,10 +215,12 @@ int dev_get_ctrls(struct video *);
void dev_dump_info(struct video *);
void dev_dump_query(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);
+void 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 *, int *);
int parse_size(struct video *);
int choose_size(struct video *);
int choose_enc(struct video *);
@@ -241,9 +246,9 @@ void
usage(void)
{
fprintf(stderr, "usage: %s [-gqRv] "
- "[-a adaptor] [-e encoding] [-f file] [-i input] [-O output]\n"
- " %*s [-o output] [-r rate] [-s size]\n", __progname,
- (int)strlen(__progname), "");
+ "[-a adaptor] [-c reset|control=value] [-e encoding] [-f file]\n"
+ " %*s [-i input] [-O output] [-o output] [-r rate] [-s
size]\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:
@@ -1010,11 +1015,10 @@ dev_get_ctrls(struct video *vid)
}
void
-dev_set_ctrl(struct video *vid, int ctrl, int change)
+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");
@@ -1030,9 +1034,8 @@ dev_set_ctrl(struct video *vid, int ctrl
* 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)
@@ -1055,20 +1058,46 @@ dev_set_ctrl(struct video *vid, int ctrl
}
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 +1110,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);
}
}
}
@@ -1221,6 +1232,36 @@ dev_init(struct video *vid)
return 1;
}
+
+int
+parse_ctrl(struct video *vid, int *ctrl_id, int *ctrl_val)
+{
+ char *valp;
+ const char *errstr;
+
+ if (!vid->set_ctrl_str)
+ return 0;
+
+ valp = strsep(&vid->set_ctrl_str, "=");
+ if (*valp == '\0' || vid->set_ctrl_str == '\0')
+ return 0;
+ for (*ctrl_id = 0; *ctrl_id < CTRL_LAST; (*ctrl_id)++) {
+ if (strcmp(valp, ctrls[*ctrl_id].name) == 0)
+ break;
+ }
+ if (*ctrl_id == CTRL_LAST) {
+ warnx("Unknown control '%s'", valp);
+ return 0;
+ }
+ *ctrl_val = strtonum(vid->set_ctrl_str, ctrls[*ctrl_id].min,
+ ctrls[*ctrl_id].max, &errstr);
+ if (errstr != NULL) {
+ warnx("control value '%s' is %s", valp, errstr);
+ return 0;
+ }
+ return 1;
+}
+
int
parse_size(struct video *vid)
{
@@ -1481,6 +1522,8 @@ mmap_stop(struct video *vid)
int
setup(struct video *vid)
{
+ int ctrl_id, ctrl_val;
+
if (vid->mode & M_IN_FILE) {
if (!strcmp(vid->iofile, "-"))
vid->iofile_fd = STDIN_FILENO;
@@ -1553,6 +1596,18 @@ setup(struct video *vid)
if ((vid->mode & M_IN_DEV) && !dev_init(vid))
return 0;
+ if (vid->mode & M_SET_CTRL) {
+ if (!parse_ctrl(vid, &ctrl_id, &ctrl_val))
+ return 0;
+ dev_set_ctrl_abs(vid, ctrl_id, ctrl_val);
+ return 1;
+ }
+
+ if (vid->mode & M_RESET) {
+ dev_reset_ctrls(vid);
+ return 1;
+ }
+
if ((vid->mode & M_OUT_XV) && !xv_init(vid))
return 0;
@@ -1564,6 +1619,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 +2011,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, "gqRva:c:e:f:i:O:o:r:s:")) != -1) {
switch (ch) {
case 'a':
x->cur_adap = strtonum(optarg, 0, 4, &errstr);
@@ -1958,6 +2020,17 @@ main(int argc, char *argv[])
errs++;
}
break;
+ case 'c':
+ if ((vid.mode & M_RESET) || (vid.mode & M_SET_CTRL)) {
+ warnx("Only one '-c' option allowed.");
+ errs++;
+ } else if (strcmp(optarg, "reset") == 0) {
+ vid.mode |= M_RESET;
+ } else {
+ vid.mode |= M_SET_CTRL;
+ vid.set_ctrl_str = strdup(optarg);
+ }
+ break;
case 'e':
vid.enc = find_enc(optarg);
if (vid.enc >= ENC_LAST) {
@@ -2055,6 +2128,9 @@ main(int argc, char *argv[])
if (!setup(&vid))
cleanup(&vid, 1);
+
+ if ((vid.mode & M_RESET) || (vid.mode & M_SET_CTRL))
+ cleanup(&vid, 0);
if (vid.mode & M_IN_FILE) {
if (pledge("stdio rpath", NULL) == -1)
