1020497 - [Camera][Gecko] Change exposure compensation to a single attribute

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Camera, defect)

Description

[Mike Habicher [:mikeh] (high bugzilla latency)]

CameraControl currently implements a setter and read-only attribute. This should be cleaned up to a single attribute.

Comment 1

[Mike Habicher [:mikeh] (high bugzilla latency)]

Attachment: Collapse exposureCompensation to a single attribute, v1

Simple change.

Comment 2

[Mike Habicher [:mikeh] (high bugzilla latency)]

Attachment: Collapse exposureCompensation to a single attribute, v1.1

Update test with changes from bug 1018820.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8434406 [details] [diff] [review]
Collapse exposureCompensation to a single attribute, v1.1

This changes behavior pretty significantly, no? In particular, the old setup had a way to set to auto, but the new one does not.

Shouldn't this be an unrestricted double instead, with NaN meaning default?  That's what it seems to be right now...

Comment 4

[Mike Habicher [:mikeh] (high bugzilla latency)]

(In reply to Boris Zbarsky [:bz] from comment #3)
> 
> This changes behavior pretty significantly, no? In particular, the old setup
> had a way to set to auto, but the new one does not.
>
> Shouldn't this be an unrestricted double instead, with NaN meaning default? 
> That's what it seems to be right now...

That was my understanding too, but after resolving bug 1004567 with a partner, we have a better understanding of how exposure compensation actually works on the underlying platform. Specifically, there is no 'auto': setting "0" just means "no compensation," which means no adjustment from whatever the camera thinks is the optimal exposure setting, with negative and positive compensation values setting under- and over-exposure, respectively.

As a result, the magic NaN value is no longer required.

This API isn't currently used in our Camera app, so the change doesn't affect us; but the partner-filed bug 1004567 and its underlying fix prompted me to want to clean up this attribute as well.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8434406 [details] [diff] [review]
Collapse exposureCompensation to a single attribute, v1.1

Ah, I see.  Looks good then!

r=me

Comment 6

[Mike Habicher [:mikeh] (high bugzilla latency)]

try-server push: https://tbpl.mozilla.org/?tree=Try&rev=789b54c4feb2

Comment 7

[Mike Habicher [:mikeh] (high bugzilla latency)]

https://hg.mozilla.org/integration/b2g-inbound/rev/f4f3b1e4a0e5

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out in https://hg.mozilla.org/integration/b2g-inbound/rev/5e2ba7b18016 for apparently breaking debug emulator mochitest-1: https://tbpl.mozilla.org/php/getParsedLog.php?id=41239098&tree=B2g-Inbound

Comment 9

[Mike Habicher [:mikeh] (high bugzilla latency)]

(In reply to Wes Kocher (:KWierso) from comment #8)
>
> Backed out in
> https://hg.mozilla.org/integration/b2g-inbound/rev/5e2ba7b18016 for
> apparently breaking debug emulator mochitest-1:
> https://tbpl.mozilla.org/php/getParsedLog.php?id=41239098&tree=B2g-Inbound

Not sure what's causing the assertion failure here (yet), but it wasn't my change.

Comment 10

[Mike Habicher [:mikeh] (high bugzilla latency)]

Broken test in comment 8 is fixed in bug 1022705. Relanding:
https://hg.mozilla.org/integration/b2g-inbound/rev/6ae8548ce144

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6ae8548ce144