Closed andrewcking closed 6 years ago
Hi @andrewcking the PR looks very nice! Thanks! I have 2 minor comments only which would be nice to solve before merging.
Do we also need to use getExposureCompensationStep
in the Camera.Parameters
? Like, what happens with the in between of the step exposure values? They get rounded?
Updated, I agree defaultExposure
is more intuitive.
Regarding getExposureCompensationStep
, it is a little odd. You set the exposure compensation with an int
since setExposureCompensation
only accepts int
values, but the int
value you supply to has to be scaled by getExposureCompensationStep
if you want to know the actual underlying exposure compensation value (EV). So for instance, my Galaxy S8 accepts any int from -20 to 20 for exposure compensation, but the actual allowed EV range is (-2,2), so you have to scale the int
value you supplied by .1 (the exposure compensation step) if you want to know the underlying EV value. Setting it up this way allows manufactures to have any desired granularity of exposure compensation they want within their actual EV range. So a device could allow for the standard EV range of (-2,2) but accept int
values between (-100,100) giving a lot of granularity. It isn't necessary to know that underlying EV value, but one may wish to display that value to a user and in that case you would need to know what the step value is.
So if my understanding is correct then we don't need getExposureCompensationStep
to set or modify the exposure compensation. And we currently have the full range of values available to users.
We could however let the step value be queried via getCapabilities
if you would like (I am happy to add that). It might make sense if someone wants to display the EV and if there are manufactures that allow for atypical EV ranges of (-3,3) for instance (to my knowledge everyone uses (-2,2) but there are seldom hard truths in the android camera world as I am sure you know).
Thanks for the explanation @andrewcking. Let's merge it as is.
The plan is to make a beta release before a stable one so some of us can test that it's totally ok to ignore steps. If it's proven that we need EV steps, we can add a step
in the IntRange
which supports it already.
Hopefully this is more helpful than it is trouble. I am not entirely sure about my unit tests.
Since different devices will return a different IntRange for exposure compensation, I figured the best way to implement it would be with the typical highest, lowest and manual selectors.
In addition to those selectors I added an autoExposure selector which returns the exposure compensation back to the default zero. It isn’t strictly necessary since one could set it manually to zero themselves. I can remove it if you see fit.