On Sat, Aug 08, 2020 at 09:30:18PM +0200, Marcus Glocker wrote:
Hello Marcus,
>> I like your patch, which is better than my original! My only very minor
>> comment is whether we might want to document that it's safe to put "0" in
>> a "V4L2_CID_AUTO" settings, since the lowest value one of those can be is
>> 0x900 (judging by videoio.h)? At least, I had to look to make sure it was
>> safe :)
> Sorry, I lost you. Which control value exactly can be lowest 0x900?
In this bit of your diff:
#define CTRL_SHARPNESS 6
- { "sharpness", 0, V4L2_CID_SHARPNESS, 0, 0, 0, 0, 0 },
+ { "sharpness", 0, V4L2_CID_SHARPNESS, 0, 0, 0, 0, 0, 0 },
#define CTRL_WHITE_BALANCE_TEMPERATURE 7
{ "white_balance_temperature",
- 0, V4L2_CID_WHITE_BALANCE_TEMPERATURE, 0, 0, 0, 0, 0 },
+ 0, V4L2_CID_WHITE_BALANCE_TEMPERATURE,
+ V4L2_CID_AUTO_WHITE_BALANCE, 0, 0, 0, 0, 0 },
it wasn't immediately obvious to me that the first "0" after
V4L2_CID_SHARPNESS wouldn't conflict with some other V4L2 setting. But it
turns out that all the V4L2_CID values are defined relative to this:
#define V4L2_CID_BASE (V4L2_CTRL_CLASS_USER | 0x900)
so "0" can't accidentally clash with anything. It's not a big deal :)
>> 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
> I was thinking about that as well but just not decided yet if we should
> introduce this or for now be compliant with the GUI where you would
> require to reset all settings to get back to auto.
In a sense the command-line is already more expressive (you can, for
example, express arbitrary white balance temperatures when in the GUI you
have to use increments of 10), so I'm inclined to think that allowing them
to be more separate is no bad thing.
[I personally think the GUI controls are very hard to use in practise: they
change so slowly that I find it almost impossible to A/B anything. So I'm a
bit biased against them!]
Laurie
