1212226 - [Camera] The thumbnail is very blurry on Aries

Status: VERIFIED FIXED

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

Description

[Julien Wajsberg [:julienw]]

Attachment: debug.patch

STR:
1. Take a picture with a Sony device.
=> Notice the bottom left thumbnail is very blurry

This is not happening on Flame.

I've been told that this is because the thumbnail given by the camera is too small (see bug 1131294) but I don't believe this is the reason. For example we (Vivien and I) tried to just not do any "crop resize rotate" (by returning the input blob from the function in shared/js/media/crop_resize_rotate.js, see attachment) and it makes the thumbnail crisp.

Then I definitely think the issue is in the Gaia code and we should fix it for 2.5, because we can't even recognize whatever was taken with this pixel porridge.
(Sorry I don't have a screenshot because it seems I can't take one from this screen... It just takes another picture ;) )

Comment 1

[vnicolas]

Attachment: fix.preview.patch

This fixes the issue for me.

The issue is that the width/height of the metadata are not updated. As a result we end up calculating the sampling based on the width of the original image instead of the width of the thumbnail.
On Aries, it ends up having a sample-size of 8 for the thumbnail, before making them bigger again for displaying...

Comment 2

[vnicolas]

Attachment: blurry.preview.patch

Altering metadata directly has some side effects. Let's clone the metadata before.

Comment 3

[GitHub Autolander Bot]

Attachment: [gaia] davidflanagan:bug1212226 > mozilla-b2g:master

Comment 4

[David Flanagan [:djf]]

Comment on attachment 8671028 [details] [review]
[gaia] davidflanagan:bug1212226 > mozilla-b2g:master

Andrew,

This is basically just Vivien's patch but changed around a bit.

Vivien: thanks for figuring this out.

Comment 5

[David Flanagan [:djf]]

I agree with Julien that this is a blocker. And I'm surprised that no one has reported it before now!

Comment 6

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

Comment on attachment 8671028 [details] [review]
[gaia] davidflanagan:bug1212226 > mozilla-b2g:master

LGTM. So if I understand this correctly, our EXIF thumbnails have always been messed up but before it was not as bad because the difference between the thumbnail size and the image size was not as great. At least the gigantic aries image sizes have been useful for once :).

Comment 7

[David Flanagan [:djf]]

(In reply to Andrew Osmond [:aosmond] from comment #6)
> Comment on attachment 8671028 [details] [review]
> [gaia] davidflanagan:bug1212226 > mozilla-b2g:master
> 
> LGTM. So if I understand this correctly, our EXIF thumbnails have always
> been messed up but before it was not as bad because the difference between
> the thumbnail size and the image size was not as great. At least the
> gigantic aries image sizes have been useful for once :).

Yes, I think this code has probably been broken for a long time, but we got away with it because there are only a few downsampling choices with #-moz-samplesize.  I think that this patch does make the thumbnails slighly sharper on Flame to.  But I guess it was only on aries where the contrast is sizes was so big to make the blurriness unmistakeable.

My patch broke a unit test, so I'll have to fix that and resubmit.

Comment 8

[David Flanagan [:djf]]

Landed: https://github.com/mozilla-b2g/gaia/commit/d9125fb0df5bdd19e1f47d2e36be10c8fce8b039

Comment 9

[Pi Wei Cheng [:piwei] (inactive)]

(In reply to David Flanagan [:djf] from comment #5)
> I agree with Julien that this is a blocker. And I'm surprised that no one
> has reported it before now!

This has been reported since a long time ago in bug 1131294 and for some odd reason they decided it's not a blocker. I was going to reply in 1131294 today but found that a fix has been submitted in this bug.

Comment 10

[Julien Wajsberg [:julienw]]

This feels so good having a fix less than 24h after filing the bug. Thanks all !

Comment 11

[vnicolas]

(In reply to David Flanagan [:djf] from comment #4)
> Vivien: thanks for figuring this out.

Thanks for making it a real fix :)

Comment 13

[Michael Bryant (uber)]

Tested successfully and marking as verified