1173974 - Camera pick activity does not rotate images with EXIF orientation before returning them

Status: VERIFIED FIXED

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

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

1. launch webmaker
2. select start from scratch
3. tap + button
4. tap image
5. tap on pencil icon 
6. tap on bigger pencil icon
7. select camera
8. take a picture
9. select select.

Expected: picture is in correct view
Actual: picture is sideways

Note:
1. from https://bugzilla.mozilla.org/show_bug.cgi?id=1169055#c30; 

Build ID               20150611131651
Gaia Revision          68269e7b6510930eb2f644f69d27d456c1bdec75
Gaia Date              2015-06-10 23:36:01
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/9ebd530c5843
Gecko Version          41.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20150611.130230
Firmware Date          Thu Jun 11 13:02:38 UTC 2015
Bootloader             s1
ro.semc.version.sw_revision 23.0.1.A.5.77

Comment 1

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

[Blocking Requested - why for this release]: similar to 1169055

Note: wasn't sure where webmaker falls under.

Comment 2

[David Flanagan [:djf]]

Thanks for filing this, Naoki. Updating the summary and component to indicate that this is a camera bug and that any app (like SMS) that picks a photo from the camera has the same bug.   share activity is fine, however, which may be an important clue in figuring out what is going wrong here.

Adding the regression flag because this used to work for the Tarako device, which is the only other one we've had to deal with the depends on EXIF orientation.

I agree with Naoki that this should probably be spark+

Comment 3

[David Flanagan [:djf]]

I just cleared the spark+ flag on bug 1169055 because that is a gallery bug that no longer reproduces on Aries. So I'm going to go ahead and set spark+ here since bug 1169055 did originally include this camera bug in its scope, and it is clear that we really do need to fix this for spark.

I'll take this and take a crack at it. If I can't make any progress, I'll try to pass it back to Justin or Andrew.

Comment 4

[David Flanagan [:djf]]

Attachment: link to patch on github

Justin,

The fix here was simple. controllers/activity.js was only calling resizeAndSave() if the image actually needed to be resized. That function calls cropResizeRotate() and it is the rotate part that automatically deals with EXIF orientation.

On Tarako we always had to resize images, so we always got the rotation for free. But for Aries, we were not calling resizeAndSave so the photos were not being rotated.

This is spark+, so I'm hoping for a quick review. Its a small patch.

Andrew: I think Justin is most familiar with the camera activity code, but I'm flagging you for feedback because I thought you might be interested in how we handle the EXIF orientation issue for pick activities.

Comment 5

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8621280 [details] [review]
link to patch on github

LGTM, thanks for the quick turnaround! Would be nice if we could put this behind a build-time flag or something for devices that rely on EXIF rotation. But, it seems like more often than not, the camera hardware we're seeing does use it, so this is fine.

Comment 6

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 8621280 [details] [review]
link to patch on github

(In reply to Justin D'Arcangelo [:justindarc] from comment #5)
> Comment on attachment 8621280 [details] [review]
> link to patch on github
> 
> LGTM, thanks for the quick turnaround! Would be nice if we could put this
> behind a build-time flag or something for devices that rely on EXIF
> rotation. But, it seems like more often than not, the camera hardware we're
> seeing does use it, so this is fine.

Since it doesn't actually do the resize/rotate if it is unnecessary, is checking the metadata to verify this so costly that we would want to avoid it on platforms that don't need it?

Comment 7

[Justin D'Arcangelo [:justindarc]]

(In reply to Andrew Osmond [:aosmond] from comment #6)
> Comment on attachment 8621280 [details] [review]
> link to patch on github
> 
> (In reply to Justin D'Arcangelo [:justindarc] from comment #5)
> > Comment on attachment 8621280 [details] [review]
> > link to patch on github
> > 
> > LGTM, thanks for the quick turnaround! Would be nice if we could put this
> > behind a build-time flag or something for devices that rely on EXIF
> > rotation. But, it seems like more often than not, the camera hardware we're
> > seeing does use it, so this is fine.
> 
> Since it doesn't actually do the resize/rotate if it is unnecessary, is
> checking the metadata to verify this so costly that we would want to avoid
> it on platforms that don't need it?

Checking the metadata is a little bit costly (you have to parse the file in JS), but its not too bad I suppose if we're only doing this for the pick activity.

Comment 8

[David Flanagan [:djf]]

Landed on master: https://github.com/mozilla-b2g/gaia/commit/eaddf6e1e2341bee948ff63206c8dbd83ed38b0e

Comment 9

[Nicholas Nelson [:NicholasN]]

Issue has been VERIFIED FIXED on aries devices.

Result: Pictures always appear in correct orientation.

Environmental Variables:
Device: Aries 3.0
Build ID: 20150614233012
Gaia: 1bf2da102560481748ff3f6202fbed5c4daa5832
Gecko: 3c26bef95d54
Gonk: 75c7e6ca80d0c7a53f346ecfcde2343be95808c9
Version: 41.0a1 (3.0)
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0