Closed Bug 931093 Opened 6 years ago Closed 6 years ago

[Camera] Update app to specify suitable thumbnailSize

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:hd+, b2g-v1.1hd fixed, b2g-v1.2 fixed)

RESOLVED FIXED
1.3 Sprint 4 - 11/8
blocking-b2g hd+
Tracking Status
b2g-v1.1hd --- fixed
b2g-v1.2 --- fixed

People

(Reporter: mikeh, Assigned: dmarcos)

References

Details

Attachments

(4 files)

Adding capabilities.thumbnailSizes and .thumbnailSize to CameraControl API in bug 807058; will also remove the pictureSize option from takePicture() and change it to a .pictureSize attribute.

camera.js will need to change to use these.

This bug should complete the dependency chain.
Required for HD
blocking-b2g: hd? → hd+
Diego, if you like, you can now remove [1] and just set camera.pictureSize in [2].

1. https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/camera.js#L1346
2. https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/camera.js#L904

The camera now keeps and can inform you of its configured picture size.
Attached file Pull Request
Attachment #825680 - Flags: review?(dflanagan)
Comment on attachment 825680 [details] [review]
Pull Request

See my comments on github.  But r- because you're finding the last big-enough thumbnail size rather than the smallest big-enough thumbnail.
Attachment #825680 - Flags: review?(dflanagan) → review-
Blocks: 931125
I was doing a final test of patch prior to merge it and I observed a strange behavior of the CameraControl API. My code picks one of the thumbnail sizes obtained from CameraControl.capabilities.thumbnailSizes. I set the thumbnail size using CameraControl.thumbnailSize. I check the value after setting it and it returns a different size. e.g:

I set: 

camera.thumbnailSize = {height:320, width:480}

I check the value and I get:

camera.thumbnailSize -> {height:384, width:512}

If I set:

camera.thumbnailSize = {height:720, width:1280} // The largest available size for the thumbnail

When I check the value I get:

camera.thumbnailSize -> {height:480, width:640} 

Is CameraControl selecting the proper thumbnail size? Is this a bug or something I'm not understanding?
Flags: needinfo?(mhabicher)
During testing, I observed that the camera driver was failing to take a photo if the aspect ratio of the thumbnail size does not match the aspect ratio of the picture size, so the API makes sure this matches. The selected thumbnail size will be closest in area to the size you specified, and the getter will reflect this.

Internally, the API keeps track of the thumbnail size you tried to set, so that if you then choose a picture size with an aspect ratio that matches your specified thumbnail size, the thumbnail size will be adjusted to reflect that as well. This means you can do:

  .thumbnailSize = { ... };
  .pictureSize = { ... };

...and things will work as expected.
Flags: needinfo?(mhabicher)
Target Milestone: --- → 1.3 Sprint 4 - 11/8
Attachment #825680 - Flags: review- → review+
I modified the patch to take into account the aspect ratio of the screen when selecting a thumbnail size
Attachment #825680 - Flags: review+ → review?(dflanagan)
Comment on attachment 825680 [details] [review]
Pull Request

The thumbnail aspect ratio should match the picture aspect ratio, not the screen aspect ratio.
Attachment #825680 - Flags: review?(dflanagan) → review-
Comment on attachment 825680 [details] [review]
Pull Request

Diego,

There are a few more things to fix, but feel free to land this once you've addressed my comments on github.  Or set r? again if you'd like me to take another look.
Attachment #825680 - Flags: review- → review+
Attached file Pull request for v1.2
Merged on v.1.1.0hd

https://github.com/dmarcos/gaia/commit/00c8cc45952cc963cc42dd00adbb542223367958

Merged on v1.2:

https://github.com/dmarcos/gaia/commit/09efce67dd81b198db18ff15c241f51269f2ab4d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.