Closed Bug 1020497 Opened 5 years ago Closed 5 years ago

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

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
2.0 S4 (20june)

People

(Reporter: mikeh, Assigned: mikeh)

References

Details

(Keywords: dev-doc-needed)

Attachments

(1 file, 1 obsolete file)

CameraControl currently implements a setter and read-only attribute. This should be cleaned up to a single attribute.
Simple change.
Assignee: nobody → mhabicher
Status: NEW → ASSIGNED
Attachment #8434348 - Flags: review?(bzbarsky)
Update test with changes from bug 1018820.
Attachment #8434348 - Attachment is obsolete: true
Attachment #8434348 - Flags: review?(bzbarsky)
Attachment #8434406 - Flags: review?(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...
Attachment #8434406 - Flags: review?(bzbarsky) → review-
(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 on attachment 8434406 [details] [diff] [review]
Collapse exposureCompensation to a single attribute, v1.1

Ah, I see.  Looks good then!

r=me
Attachment #8434406 - Flags: review- → review+
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
Flags: needinfo?(mhabicher)
(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.
Flags: needinfo?(mhabicher)
https://hg.mozilla.org/mozilla-central/rev/6ae8548ce144
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.0 S4 (20june)
You need to log in before you can comment on or make changes to this bug.