On Wed, 15 Jul 2020 21:15:37 +0100
Laurence Tratt <[email protected]> wrote:
> On Wed, Jul 15, 2020 at 08:33:54PM +0200, Marcus Glocker wrote:
>
> Hello Marcus,
>
> Thanks for your comments. I agree with all but one, and attach a new
> diff.
>
> >> +#define CTRL_WHITE_BALANCE_TEMPERATURE 7
> >> + { "white_balance_temperature",
> > I think we should replace '_' with ' ' just to make the output look
> > more aligned when we run video -v.
>
> In the next patch [1], I allow people to adjust controls via a
> command-line switch to video(1). Having controls with spaces in would
> be really awkward for that and I can't see an obvious solution -- but
> I'm open to suggestions!
I initially had the impression that the dev_ctrls.name field is only
meant for output, but the intention seems to be to use it as a
identifier as well. Introducing spaces then is stupid of course, so
I'm OK with that. I wasn't looking at your other diff closer yet, but
will do.
Your updated diff looks good to me and testing worked fine as well here.
Any brave developer around who would OK this?
>
> Laurie
>
> [1] https://marc.info/?l=openbsd-tech&m=159466570510992&w=2
>
>
> Index: video.1
> ===================================================================
> RCS file: /cvs/xenocara/app/video/video.1,v
> retrieving revision 1.14
> diff -u -r1.14 video.1
> --- video.1 25 Feb 2019 12:34:35 -0000 1.14
> +++ video.1 15 Jul 2020 20:10:25 -0000
> @@ -66,8 +66,8 @@
> .Ar output
> and displayed via
> .Xr Xv 3 .
> -The acutance, brightness, contrast, gain, gamma, hue and saturation
> -controls of
> +The acutance, brightness, contrast, gain, gamma, hue, saturation,
> and white +balance temperature controls of
> .Ar file
> can also be adjusted if
> .Ar file
> @@ -293,6 +293,12 @@
> .Ar file .
> .It Ic s
> Decrease saturation control of
> +.Ar file .
> +.It Ic W
> +Increase white balance temperature control of
> +.Ar file .
> +.It Ic w
> +Decrease white balance temperature control of
> .Ar file .
> .El
> .Sh EXAMPLES
> Index: video.c
> ===================================================================
> RCS file: /cvs/xenocara/app/video/video.c,v
> retrieving revision 1.30
> diff -u -r1.30 video.c
> --- video.c 1 Jul 2020 06:45:24 -0000 1.30
> +++ video.c 15 Jul 2020 20:10:25 -0000
> @@ -114,7 +114,10 @@
> { "gamma", 0, V4L2_CID_GAMMA, 0, 0, 0, 0, 0 },
> #define CTRL_SHARPNESS 6
> { "sharpness", 0, V4L2_CID_SHARPNESS, 0, 0,
> 0, 0, 0 }, -#define CTRL_LAST 7
> +#define CTRL_WHITE_BALANCE_TEMPERATURE 7
> + { "white_balance_temperature",
> + 0, V4L2_CID_WHITE_BALANCE_TEMPERATURE,
> 0, 0, 0, 0, 0 }, +#define CTRL_LAST 8
> { NULL, 0, 0, 0, 0, 0, 0, 0 }
> };
>
> @@ -210,6 +213,7 @@
> 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_reset_ctrls(struct video *);
>
> int parse_size(struct video *);
> @@ -730,6 +734,16 @@
> if (vid->mode & M_IN_DEV)
> dev_set_ctrl(vid,
> CTRL_SATURATION, -1); break;
> + case 'W':
> + if (vid->mode & M_IN_DEV)
> + dev_set_ctrl(vid,
> +
> CTRL_WHITE_BALANCE_TEMPERATURE, 10);
> + break;
> + case 'w':
> + if (vid->mode & M_IN_DEV)
> + dev_set_ctrl(vid,
> +
> CTRL_WHITE_BALANCE_TEMPERATURE, -10);
> + break;
> default:
> break;
> }
> @@ -1011,6 +1025,13 @@
> ctrls[ctrl].name, d->path);
> return;
> }
> + 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);
> + }
> val = ctrls[ctrl].cur + ctrls[ctrl].step * change;
> if (val > ctrls[ctrl].max)
> val = ctrls[ctrl].max;
> @@ -1034,6 +1055,23 @@
> }
>
> void
> +dev_set_ctrl_auto_white_balance(struct video *vid, int toggle)
> +{
> + 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)
> + 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");
> +}
> +
> +void
> dev_reset_ctrls(struct video *vid)
> {
> struct dev *d = &vid->dev;
> @@ -1043,6 +1081,14 @@
> for (i = 0; i < CTRL_LAST; i++) {
> if (!ctrls[i].supported)
> continue;
> + 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)
> @@ -1054,6 +1100,9 @@
> 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);
> + }
> }
> }
>
>
