Hello Laurie, Sorry for the delay - I'm back to business mode and need to take care about naive project managers with crazy requirements the most of the day ...
See inline some (nitpicking) feedback and adapted diff. Ok for you? Thanks, Marcus On Sun, 9 Aug 2020 13:09:34 +0100 Laurence Tratt <[email protected]> wrote: > On Sat, Aug 08, 2020 at 11:29:41PM +0200, Marcus Glocker wrote: > > Hello Marcus, > > >>>> One other thing has occurred to me -- but can be done in a > >>>> future patch -- is that we probably want to be able to do: > >>>> > >>>> $ video white_balance_temperature=auto > > Please find attached a patch which does this. It also makes setting > auto controls more generic for if/when we gain more. If you set (say) > "saturation=auto" we warn, but don't error. So for example: > > $ video -d > $ video white_balance_temperature=3000 > white_balance_temperature: auto -> 3000 > $ video white_balance_temperature=auto > white_balance_temperature: 3000 -> auto > $ video white_balance_temperature=auto > white_balance_temperature: auto -> auto > $ video white_balance_temperature=3000 saturation=auto > white_balance_temperature: auto -> 3000 > saturation: no automatic control found > > > Laurie > > > Index: video.c > =================================================================== > RCS file: /cvs/xenocara/app/video/video.c,v > retrieving revision 1.35 > diff -u -r1.35 video.c > --- video.c 9 Aug 2020 06:51:04 -0000 1.35 > +++ video.c 9 Aug 2020 11:58:35 -0000 > @@ -219,8 +219,8 @@ > int dev_init(struct video *); > 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); > int dev_get_ctrl_auto(struct video *, int); > +void dev_set_ctrl_auto(struct video *, int, int, int); > void dev_reset_ctrls(struct video *); > > int parse_ctrl(struct video *, int, char **); > @@ -1037,7 +1037,7 @@ > * 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, 0); > + dev_set_ctrl_auto(vid, ctrl, 0, 0); > } > if (val > ctrls[ctrl].max) > val = ctrls[ctrl].max; > @@ -1082,12 +1082,15 @@ > } > > void > -dev_set_ctrl_auto_white_balance(struct video *vid, int value, int > reset) +dev_set_ctrl_auto(struct video *vid, int ctrl, int value, int > reset) { > struct dev *d = &vid->dev; > struct v4l2_control control; > > - control.id = V4L2_CID_AUTO_WHITE_BALANCE; > + if (!ctrls[ctrl].id_auto) > + return; > + > + control.id = ctrls[ctrl].id_auto; > if (ioctl(d->fd, VIDIOC_G_CTRL, &control) != 0) { > warn("VIDIOC_G_CTRL"); > return; > @@ -1134,8 +1137,7 @@ > if (!ctrls[i].supported) > continue; > dev_set_ctrl_abs(vid, i, ctrls[i].def); > - if (i == CTRL_WHITE_BALANCE_TEMPERATURE) > - dev_set_ctrl_auto_white_balance(vid, 1, 0); > + dev_set_ctrl_auto(vid, i, 1, 0); > } > } > > @@ -1303,7 +1305,7 @@ > } > } > if (i == CTRL_LAST) > - warnx("%s: unknown control", > *argv); > + warnx("%s: unknown control", *argv); > continue; > } Lets commit this separately since it's not related to the main change. > @@ -1315,21 +1317,34 @@ > 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; > auto_old = dev_get_ctrl_auto(vid, i); > - if (dev_set_ctrl_abs(vid, i, val_new) == 0) { > - if (auto_old) { > - fprintf(stderr, "%s: auto -> > %d\n", > - ctrls[i].name, > ctrls[i].cur); > + val_old = ctrls[i].cur; > + if (strcmp(p, "auto") == 0) { > + if (ctrls[i].id_auto == 0) { > + fprintf(stderr, "%s: no > automatic control found\n", > + ctrls[i].name); > + } else if (!auto_old) { > + fprintf(stderr, "%s: %d -> > auto\n", > + ctrls[i].name, > val_old); > + dev_set_ctrl_auto(vid, i, 1, > 0); } else { > - fprintf(stderr, "%s: %d -> > %d\n", > - ctrls[i].name, val_old, > - ctrls[i].cur); > + fprintf(stderr, "%s: auto -> > auto\n", ctrls[i].name); > + } > + } else { > + val_new = strtonum(p, -32768, 32768, > &errstr); > + if (errstr != NULL) { > + warnx("%s: %s", *argv, > errstr); > + return 0; > + } > + if (dev_set_ctrl_abs(vid, i, > val_new) == 0) { > + if (auto_old) { > + fprintf(stderr, "%s: > auto -> %d\n", > + > ctrls[i].name, ctrls[i].cur); > + } else { > + fprintf(stderr, "%s: > %d -> %d\n", > + > ctrls[i].name, val_old, > + > ctrls[i].cur); > + } > } > } > break; I did some line break exercise here to fit to 80 chars/line. > @@ -1601,6 +1616,8 @@ > int > setup(struct video *vid) > { > + int i; > + > if (vid->mode & M_IN_FILE) { > if (!strcmp(vid->iofile, "-")) > vid->iofile_fd = STDIN_FILENO; > @@ -1689,7 +1706,11 @@ > * 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); > + for (i = 0; i < CTRL_LAST; i++) { > + if (ctrls[i].id_auto) { > + dev_set_ctrl_auto(vid, i, 0, 1); > + } > + } I would leave that to auto white balance for now, until we know that other auto controls also require the same. Also we would need to change the comment then. > if (vid->mode & M_OUT_XV) > net_wm_supported(vid); > Index: video.c =================================================================== RCS file: /cvs/xenocara/app/video/video.c,v retrieving revision 1.36 diff -u -p -u -p -r1.36 video.c --- video.c 11 Aug 2020 05:35:17 -0000 1.36 +++ video.c 23 Aug 2020 06:56:54 -0000 @@ -219,8 +219,8 @@ void dev_dump_query_ctrls(struct video * int dev_init(struct video *); 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); int dev_get_ctrl_auto(struct video *, int); +void dev_set_ctrl_auto(struct video *, int, int, int); void dev_reset_ctrls(struct video *); int parse_ctrl(struct video *, int, char **); @@ -1037,7 +1037,7 @@ dev_set_ctrl_abs(struct video *vid, int * 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, 0); + dev_set_ctrl_auto(vid, ctrl, 0, 0); } if (val > ctrls[ctrl].max) val = ctrls[ctrl].max; @@ -1082,12 +1082,15 @@ dev_set_ctrl_rel(struct video *vid, int } void -dev_set_ctrl_auto_white_balance(struct video *vid, int value, int reset) +dev_set_ctrl_auto(struct video *vid, int ctrl, int value, int reset) { struct dev *d = &vid->dev; struct v4l2_control control; - control.id = V4L2_CID_AUTO_WHITE_BALANCE; + if (!ctrls[ctrl].id_auto) + return; + + control.id = ctrls[ctrl].id_auto; if (ioctl(d->fd, VIDIOC_G_CTRL, &control) != 0) { warn("VIDIOC_G_CTRL"); return; @@ -1134,8 +1137,7 @@ dev_reset_ctrls(struct video *vid) if (!ctrls[i].supported) continue; dev_set_ctrl_abs(vid, i, ctrls[i].def); - if (i == CTRL_WHITE_BALANCE_TEMPERATURE) - dev_set_ctrl_auto_white_balance(vid, 1, 0); + dev_set_ctrl_auto(vid, i, 1, 0); } } @@ -1310,21 +1312,40 @@ parse_ctrl(struct video *vid, int argc, 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; auto_old = dev_get_ctrl_auto(vid, i); - if (dev_set_ctrl_abs(vid, i, val_new) == 0) { - if (auto_old) { - fprintf(stderr, "%s: auto -> %d\n", - ctrls[i].name, ctrls[i].cur); + val_old = ctrls[i].cur; + if (strcmp(p, "auto") == 0) { + if (ctrls[i].id_auto == 0) { + fprintf(stderr, + "%s: no automatic control found\n", + ctrls[i].name); + } else if (!auto_old) { + fprintf(stderr, "%s: %d -> auto\n", + ctrls[i].name, val_old); + dev_set_ctrl_auto(vid, i, 1, 0); } else { - fprintf(stderr, "%s: %d -> %d\n", - ctrls[i].name, val_old, - ctrls[i].cur); + fprintf(stderr, + "%s: auto -> auto\n", + ctrls[i].name); + } + } else { + val_new = strtonum(p, -32768, 32768, &errstr); + if (errstr != NULL) { + warnx("%s: %s", *argv, errstr); + return 0; + } + if (dev_set_ctrl_abs(vid, i, val_new) == 0) { + if (auto_old) { + fprintf(stderr, + "%s: auto -> %d\n", + ctrls[i].name, + ctrls[i].cur); + } else { + fprintf(stderr, + "%s: %d -> %d\n", + ctrls[i].name, val_old, + ctrls[i].cur); + } } } break; @@ -1684,7 +1705,7 @@ setup(struct video *vid) * 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); + dev_set_ctrl_auto(vid, V4L2_CID_AUTO_WHITE_BALANCE, 0, 1); if (vid->mode & M_OUT_XV) net_wm_supported(vid);
