Hi Laurie, Thanks for testing and feedback!
On Thu, 30 Jul 2020 08:07:56 +0100 Laurence Tratt <[email protected]> wrote: > On Wed, Jul 29, 2020 at 10:52:31PM +0200, Marcus Glocker wrote: > > Hello Marcus, > > > Slightly adapted diff; Negative numbers can happen on controls. > > This looks really good to me, and a significant improvement on my > original. I've tested everything I can think of and it all looks > good, with one mild exception. > > UVC has separate controls for white_balance_temperature [int] and > white_balance_temperature_auto [bool]. The former is only active if > the latter is false. This probably makes devices about 30 seconds > easier to develop, but it's not a nice UI for end users, so we've > conflated them into one control. However, I now think we might need > to display this conflated control slightly differently to others. For > example consider this: > > $ video -d > $ video -c > brightness=128 > contrast=128 > saturation=128 > gain=0 > sharpness=128 > white_balance_temperature=4000 > > That last line, I think, should probably be: > > white_balance_temperature=auto > > because although the device's white_balance_temperature is set to > 4000K, we've turned auto white balance back on, so the device will > choose whatever white_balance_temperature it feels like. If/when we > get controls like zoom/exposure, they'll also probably want to be > treated in the same way, so if there's a generic way of handling > these in the code, it might be a nice bit of future proofing? Yes, I was shortly thinking about the right way of how to handle the white balance temperature control as well. And I agree, probably we should display 'auto' when auto white balance is turned on, but the current value when it's turned off. > However, this is not a huge deal: IMHO your diff is already good > enough to go in tree! Right, I also would prefer to get this diff in as is, and then implement the white balance change on top of that. As you already mentioned, that should be no big deal. > Laurie Marcus
