Derek Harris [:DerekH] - Dont need info. Last day 6/5/15 Reporter
	Description • 9 years ago

Description:
If the user attaches an image from either camera or the gallery the image will be shown sideways once it is sent or in the message right before being sent. The thumbnail will always be roated to the left.


Repro Steps:
1) Update a Xperia Z3 Compact (B2G) to 20150526220312
2) Open messages app
3) Enter a thread, or start a new thread
4) Tap on the paperclip icon> Select camera
5) Take a picture> Tap select
6) Send the message> Observe sent thumbnail


Actual:
The Image that was attached is rotated 90 degrees to the left


Expected:
The image keeps the same rotation it had when it was originally attatched

Environmental Variables:
Device: Xperia Z3 Compact (B2G) 3.0
Build ID: 20150526220312
Gaia: b168b4e2b04ecc2c0a6dcd591679d7694128fb32
Gecko: cafc8056bea143c379643eb9d7c502cc9a2cac25
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


Repro frequency: 10/10
See attached: Logcat, Video - https://youtu.be/hReLA5WSoJA

	Derek Harris [:DerekH] - Dont need info. Last day 6/5/15 Reporter
	Comment 1 • 9 years ago

This issue does NOT occur on Flame 3.0

The image keeps the same rotation it had when it was originally attatched

Device: Flame 3.0 (319mb)(Kitkat)(Full Flash)
BuildID: 20150527010201
Gaia: 8ca93673869a64e09ed6153c5402896822dfb253
Gecko: ff2e07228041
Gonk: 040bb1e9ac8a5b6dd756fdd696aa37a8868b5c67
Version: 41.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0

	Julien Wajsberg [:julienw]
	Comment 2 • 9 years ago

Hey David, I remember we had such an issue on a previous device. Do you remember by chance how we fixed it?

	David Flanagan [:djf] Assignee
	Comment 3 • 9 years ago

[Blocking Requested - why for this release]: pretty bad user experience, and texting photos is a very common thing to do on a dogfooding phone

(In reply to Julien Wajsberg [:julienw] from comment #2)
> Hey David, I remember we had such an issue on a previous device. Do you
> remember by chance how we fixed it?

I don't have an Aries yet, but it should be arriving soon. From the description, it sounds like the device is using EXIF orientation in the images rather than actually producing properly oriented jpegs. The Tarako device did that. If Aries is like Tarako, then one of the landscape modes should produce photos that do not have this problem of appearing rotated in the SMS app.

Needinfo Derek: would you attach one of the original images from the Aries camera that causes this bug? Actually it would be great to see one portrait photo and two landscape photos from the two possible (90 degrees and 270 degrees) landscape orientations. Can you confirm that this bug does not occur if you take photos with the phone rotated 90 degrees clockwise or 90 degrees counterclockwise?

Needinfo Andrew: can we make the Aries camera driver not use EXIF orientation and instead actually rotate the pixels? If that is not an option for the giant 20mp images we take now, is it an option if we take smaller photos?

Julien: When we dealt with this for the Tarako our decision was that we would make the Gallery and Camera apps EXIF-aware and code them so that they can honor the EXIF flag. But we decided that the larger web does not (and cannot) support EXIF orientation properly, so that we needed to ensure that unrotated photos with EXIF orientation were never exposed to apps other than Camera and Gallery. As far as I know, that decision is still the right thing to do.

I suspect that when we lifted the image size constraints in gallery and camera to support the gigantic image sizes on Aries, we broke the rotation code that was supposed to run for any image that is shared by or picked from the Camera or Gallery apps.  There should be code in there that calls crop_resize_rotate() and that ought be handling the rotation.

It would be really great if we could fix this at the driver level so we just don't have to deal with EXIF orientation on this device. If we have to rotate and re-encode all photos we share or pick from Gallery and Camera, then we're automatically losing image quality because of the extra re-encode.

But if Andrew can't fix it in the driver, then I can look into what is going on in camera and gallery. I'm swamped with 2.2+ work now, but if we can get this marked as spark+, I can look into it as my next-highest priority, probably next week.

Andrew: if you can't fix this, I guess you should assign it to me.

	Julien Wajsberg [:julienw]
	Comment 4 • 9 years ago

(In reply to David Flanagan [:djf] from comment #3)

> It would be really great if we could fix this at the driver level so we just
> don't have to deal with EXIF orientation on this device. If we have to
> rotate and re-encode all photos we share or pick from Gallery and Camera,
> then we're automatically losing image quality because of the extra re-encode.

Just answering to this part (because the rest of your answer is great).
As far as I know we can losslessly rotate jpegs as long as it's by increments of 90°. There are command line programs that do this (I know "exiftran"). I think this is also faster than decoding/reencoding because this likely works at the encoded level.
However I guess we'd have to code it directly in JS so this is quite some work.

Some other links:
http://www.betterjpeg.com/lossless-rotation.htm
http://www.impulseadventure.com/photo/rotation-partial-mcu.html

I don't know exiftran deals with the issues outlined in these articles though :/

	Naoki Hirata :nhirata (please use needinfo instead of cc)
	Comment 5 • 9 years ago

QA Wanted and a NI to annswer question in comment 3.

	Naoki Hirata :nhirata (please use needinfo instead of cc)
	Comment 6 • 9 years ago

To note, if you add a picture to the contact, this bug also occurs.  I think djf is right.  We saw a very similar issue happen on the Tarako.

	David Flanagan [:djf] Assignee
	Comment 8 • 9 years ago

Bug 1169500 was the same bug, but manifesting in the Contacts app instead of Messages.

	David Flanagan [:djf] Assignee
	Comment 9 • 9 years ago

When my Aries device arrives in the mail, I'll try to remember to try out the Android camera on it before I flash it to FirefoxOS because I want to see if the camera also produces photos with EXIF orientation in Android, and if it does so at all image sizes.

	David Flanagan [:djf] Assignee
	Comment 10 • 9 years ago

In bug 1169467, Marcia points out this error message while attaching a photo to an MMS message:

E/Gallery ( 5687): Content JS ERROR: while resizing image: Failed to decode image in cropResizeRotate; image may be corrupt or too large: IndexSizeError: Index or size is negative or greater than the allowed amount 

I suspect that is what is happening in this bug, too. We have some kind of size limit in crop_resize_rotate() that needs to be adjusted for giant Aries photos.

We'll need to fix that, regardless, so I'll take this bug and investigate next week after my 2.2+ bugs.

But I still want to explore whether we can avoid EXIF orientation completely on Aries at the gonk or gecko layer because having to rotate these images by hand will reduce responsiveness and will also reduce photo quality.

	Derek Harris [:DerekH] - Dont need info. Last day 6/5/15 Reporter
	Comment 11 • 9 years ago

> Needinfo Derek: would you attach one of the original images from the Aries
> camera that causes this bug? Actually it would be great to see one portrait
> photo and two landscape photos from the two possible (90 degrees and 270
> degrees) landscape orientations. Can you confirm that this bug does not
> occur if you take photos with the phone rotated 90 degrees clockwise or 90
> degrees counterclockwise?

Attaching a creenshot showing the image rotations after being sent via the messages app. one picute is taken in portrait, and 2 images in landscape with 90 degree rotations clockwise and counter clockwise.

	Pi Wei Cheng [:piwei] (inactive)
	Comment 12 • 9 years ago

See comment 11. To make it clear, the only instance that this issue does not occur is when taking pictures 90 degrees counterclockwise.

	David Flanagan [:djf] Assignee
	Comment 13 • 9 years ago

Thanks Derek.  My aries device was supposed to arrive on Wednesday, but it just shipped today for delivery Tuesday. So it will probably be Wednesday at the earliest that I can look at this.

	Julien Wajsberg [:julienw]
	Comment 14 • 9 years ago

Can you please attach an _actual_ image reproducing the issue ?
(given David has no aries yet)

	Derek Harris [:DerekH] - Dont need info. Last day 6/5/15 Reporter
	Comment 15 • 9 years ago

All images taken with the phone will exhibit the behavior of this bug. An image has been attached per comment 14

	Julien Wajsberg [:julienw]
	Comment 16 • 9 years ago

(In reply to Derek Harris [:DerekH] from comment #15)
> Created attachment 8613621 [details]
> Affected Image
> 
> All images taken with the phone will exhibit the behavior of this bug. 

Yes but because we don't have the phone yet we can try to reproduce the bug with Flame with this image.

> An image has been attached per comment 14

Thanks !

	Andrew Osmond [:aosmond] (he/him)
	Comment 17 • 9 years ago

Confirmed this reproduces on shinano (still waiting on that aries). Investigating what we can do on the gecko side (e.g. maybe configuring the sensor offset will change matters...). We can certainly take smaller pictures, it would just need to be configured that way similar to how the camera uses the 'low' recording profile for MMS.

	Andrew Osmond [:aosmond] (he/him)
	Comment 18 • 9 years ago

David: According to the documentation, the driver has the choice of updating the EXIF data or actually rotating the picture based on the configured settings. Shinano (and presumably aries) opts to update the EXIF data so anything I tweak at the Gonk layer will not help.

With respect to taking a smaller picture, yes we can take smaller photos. The messages app appears to be using the camera activity incorrectly as it only provides a maximum file size:

https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/contacts/js/views/form.js#L1404

In the camera activity implementation, we require the max pixels if one wishes to control the picture size, not the max file size:

https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/controllers/settings.js#L245

I tweaked the algorithm to use half the maximum file size as the pixel size if the latter was not provided. That seems to work well enough as an estimate, at least for the purposes of resizing the photo and generating the thumbnail properly.

	Julien Wajsberg [:julienw]
	Comment 19 • 9 years ago

From bug 1169680, we know there is currently an issue with moz-sample-size, that we also use in the SMS app. So I'd suggest to wait until this is fixed first.

	David Flanagan [:djf] Assignee
	Comment 20 • 9 years ago

I'm guessing that this is actually the same bug as 1169680. Seth has a gecko patch for that one under review. Maybe someone could try that out and see if it fixes this one?  Or, just try backing out bug 1154974 and see if that fixes this one.

	David Flanagan [:djf] Assignee
	Comment 21 • 9 years ago

Looks like this bug is not specific to Aries or specific to the large image size of the Aries camera.  I can reproduce it on Flame with the test image gaia/apps/gallery/test/images/55.jpg which is a 1200x1600 photo from a Tarako.

	David Flanagan [:djf] Assignee
	Comment 22 • 9 years ago

I've confirmed that what's happening here is that when we display an image to be cropped after it is picked, we use #-moz-samplesize to show it smaller. Then, when we try to decode it full-size to actually crop or rotate before returning it as the result of a pick activity, it is being returned from the cache at small size (because of bug 115494).  We then try to use canvas.drawImage() on this small image, assuming that the size is much larger. drawImage() fails because the arguments we pass are out of bounds.  And we end up using the original (unrotated) image instead of the cropped image.

I can see the same thing happen on flame when I try to crop an image that is big enough to use #-moz-samplesize on.

So this bug is really just a dupe of 1169680.

	Pi Wei Cheng [:piwei] (inactive)
	Comment 23 • 9 years ago

This is not a dupe of bug 1169680, see https://bugzilla.mozilla.org/show_bug.cgi?id=1169680#c43

	David Flanagan [:djf] Assignee
	Comment 24 • 9 years ago

I'm really quite confident that the patch from bug 1169680 did fix something here.  I'd guess that with that landed, this bug no longer reproduces for small images like the one in comment #21.

But there are probably other bugs if you test on a flame with really big images from an Aries phone. That's unlikely to ever work reliably on a 319mb flame.

	Martin Shuman [:Marty]
	Comment 25 • 9 years ago

Adding smoketest keyword

	Naoki Hirata :nhirata (please use needinfo instead of cc)
	Comment 26 • 9 years ago

Should we have better display/UX in regards to images that can't be seen?
Like some web pages has "Image can't be displayed" images in lieu of the image that can't be displayed.

	Stephany Wilkes
	Comment 27 • 9 years ago

Please file a new bug about better messaging. This bug is about image rotation, but the UX need is about changes to messaging for images that can't be see (new work). Assign that bug to Tif, as I will no longer be at Mozilla, and Rob will be manning the UX triage account (so you can skip him).

	David Flanagan [:djf] Assignee
	Comment 28 • 9 years ago

Naoki: I'm not sure I understand the question, actually, but I agree with Stephany that it sounds like a separate bug.

Do you happen to know whether this bug still affects Aries? Should it still be spark+? Or is it just a low-memory Flame bug now?

	David Flanagan [:djf] Assignee
	Comment 29 • 9 years ago

I'm pretty sure that this is no longer a spark+ bug.  Let's repurpose this bug to be about the same symptoms that occur when cropping or rotating large images during a pick activity. So before closing this bug, I'll want to resolve the EXIF rotation issue and the image cropping issue described in bug 1169680 so that they do not affect low-memory Flames.

	Naoki Hirata :nhirata (please use needinfo instead of cc)
	Comment 30 • 9 years ago

This still occurs on aries when using the webmaker app.
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 sidweays

	David Flanagan [:djf] Assignee
	Comment 31 • 9 years ago

Naoki: please file a Gaia::Camera bug for that. I think I actually saw the same thing when picking a photo from Camera to attach to an MMS message.  You should probably needinfo both Justin and I on the new bug.

	David Flanagan [:djf] Assignee
	Comment 32 • 9 years ago

I just noticed that even though this bug is in the Gallery component and has "Gallery" in the summary, the STR are actually using the Camera app, not Gallery.  But Naoki just filed bug 1173974 for the camera bug he described in comment #30.

I'm going to treat this one as a gallery bug, since that's what I've been doing all along. I have confirmed that it no longers occurs on Spark, so clearing the blocking flag.  But I'm keeping it open because the same symptoms occur (for different reasons) on low-memory Flame devices, and I do need to get that fixed.

	Naoki Hirata :nhirata (please use needinfo instead of cc)
	Comment 33 • 9 years ago

Filed new bug 1173974; guess you already saw and are on it.

	Martin Shuman [:Marty]
	Comment 34 • 9 years ago

This issue has not been seen recently by Smoke team.  Resolving this issue as WorksForMe.

Environmental Variables:
Device: Aries 2.5
Build ID: 20150715181511
Gaia: c6ef08964711f461a8e6326eae911789d1ec220c
Gecko: 16e30af059b7
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 42.0a1 (2.5)
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0