1224696 - [FlameKK] Large pictures will appear as black in the gallery

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Gallery, defect, P3)

Description

[Adam Alldredge [:AdamA][Inactive]]

Description:
If the user receives a large gallery or puts it on the phone using usb it will appear as a black thumbnail. if the picture is opened it will be displayed as black until the user tries to zoom then it will start to appear correctly. The images that I have seen this with are above 3500x3500, and they are jpg's.

Repro Steps:
1) Update a Aries to 20151113121953
2) Transfer a large photo to the phone via bluetooth or USB
3) Open gallery
4) Observe images

Actual:
Large files will appear as black

Expected:
It is expected that large files appear correctly in the gallery

Environmental Variables:
Device: Aries 2.6 [Full Flash]
Build ID: 20151113121953
Gaia: e8c15ae4e5324a210000ee0a869a962aa542009f
Gecko: faf815a0fa9b052a38bce00c0c2aa1e2c9610936
Gonk: a19052e4389c3ae2d8fc3e7a74a475401baacc56
Version: 45.0a1 (2.6)
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:45.0) Gecko/45.0 Firefox/45.0

Repro frequency: 10/10
See attached: video clip, logcat, example file.

Comment 1

[Adam Alldredge [:AdamA][Inactive]]

Attachment: logcat

Comment 2

[Adam Alldredge [:AdamA][Inactive]]

Attachment: Example large image

Comment 3

[Adam Alldredge [:AdamA][Inactive]]

This issue DOES occur on Flame 2.5.

Environmental Variables:
Device: Flame 2.5 [Full Flash][512mb]
Build ID: 20151109004552
Gaia: cf646c52bb947af28329b0a100df91d1b1f2a907
Gecko: 4eafef5b80f8985c94c4a067f130d37513e1a581
Gonk: 205ac4204bbbb2098a8046444acba551ba5dc75a
Version: 44.0a2 (2.5)
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:44.0) Gecko/44.0 Firefox/44.0

Result:
Large files will appear black
----------------------------------------------
This issue DOES NOT occur on Aries 2.6 or Flame 2.2.

Environmental Variables:
Device: Aries 2.6 [Full Flash]
BuildID: 20151113123209
OTABuildID: 20151113121953
Gaia: e8c15ae4e5324a210000ee0a869a962aa542009f
Gecko: faf815a0fa9b052a38bce00c0c2aa1e2c9610936
Gonk: a19052e4389c3ae2d8fc3e7a74a475401baacc56
Version: 45.0a1 (2.6) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:45.0) Gecko/45.0 Firefox/45.0

Device: Flame 2.2 [Full Flash][512mb]
BuildID: 20151111032502
Gaia: 885647d92208fb67574ced44004ab2f29d23cb45
Gecko: ac5fce5a78e5
Gonk: bd9cb3af2a0354577a6903917bc826489050b40d
Version: 37.0 (2.2) 
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Result:
Large files appear in the gallery.
-----------------------------------
NI'ing djf per Conversation with Nhirata.

Comment 4

[David Flanagan [:djf]]

Comment #0 says this does occur on Aries, but comment #3 says it doesn't.

The attached image looks like it is from an Aries camera, so if this was affecting Aries, it would presumably affect all images from the camera. So I'm guessing that this really is Flame only. That's what the youtube video shows.

Comment #0 says there is a repro frequency of 10/10.  But the youtube videos shows two black thumbnails and two non-black thumbnails of the example image, so it appears to me that the thumbnail is correctly created at least some of the time.

Comment 5

[David Flanagan [:djf]]

I fear that this is bug 1163225 come back to haunt us, though I suppose I need to investigate and reproduce this myself.

It would probably be better for the app to ignore large images than to display blank thumbnails. So one solution would be to go back to the old days where we just rejected images that were too big for the device memory.  That would suck.

Another solution is to pre-initialize the canvas with some random pixels before the drawImage so that we can test whether the drawImage succeeded or not. And if it failed, then we reject the image and try again some other time, I suppose.

Seth: what's the latest with your size prediction patches? Can I remove the #-moz-samplesize now, and is there a chance that will help with memory usage?

Comment 6

[David Flanagan [:djf]]

Adam,

Could you clarify whether you've ever seen this bug on Aries, please?  Or is it really Flame only?

Also: what is the repro rate you see on Flame?

And if you configure your flame with 1024mb, does the bug go away?

Comment 7

[Seth Fowler [:seth] [:s2h]]

(In reply to David Flanagan [:djf] from comment #5)
> Seth: what's the latest with your size prediction patches? Can I remove the
> #-moz-samplesize now, and is there a chance that will help with memory usage?

Bug 1218990 is green on try but waiting on its dependencies, which are undergoing review. (The code is all written, but as usual there are tweaks being requested during review and such.) That will allow us to track the visibility of CSS background images and discard them when they are no longer visible, which should greatly improve the memory situation in the Gallery app. Once that lands, we'll be in a position to painfully rebase and land bug 1151373, which will give us size prediction for CSS background images. I'm expecting all of this to land during this release, and we should also get DDD/size prediction for layerized images.

That basically covers everything except for canvas, but due to some complications (the canvas graphics backend can be different than the graphics backend we use for other content, which makes some things trickier), we probably won't get that until the next Gecko release.

David, I'm trying to remember how the Gallery app works. Is it the case that when you tap on an image (so that you transition to viewing a single image rather than the list of images in the Gallery), we're still displaying a thumbnailed version of the image made using canvas.drawImage()? And then once the user zooms, we start using a non-thumbnailed version of the image? Is that why the image is black until the user starts zooming?

Comment 8

[David Flanagan [:djf]]

(In reply to Seth Fowler [:seth] [:s2h] from comment #7)
> David, I'm trying to remember how the Gallery app works. Is it the case that
> when you tap on an image (so that you transition to viewing a single image
> rather than the list of images in the Gallery), we're still displaying a
> thumbnailed version of the image made using canvas.drawImage()? And then
> once the user zooms, we start using a non-thumbnailed version of the image?
> Is that why the image is black until the user starts zooming?

For images (like those from the Aries camera) that don't have a big enough EXIF preview, we create a preview image when we create the thumbnail. So in this case, we're apparently getting a black thumbnail and a black preview at the same time.

Note that Gallery doesn't use CSS images anymore, so I wouldn't expect much memory improvement when your CSS size prediction stuff lands.  Until canvas supports downsample-during-decode, I'm going to have to keep using #-moz-samplesize, I guess.

Comment 9

[Seth Fowler [:seth] [:s2h]]

(In reply to David Flanagan [:djf] from comment #8)
> For images (like those from the Aries camera) that don't have a big enough
> EXIF preview, we create a preview image when we create the thumbnail. So in
> this case, we're apparently getting a black thumbnail and a black preview at
> the same time.

Thanks. So canvas.drawImage() failure fully explains this bug.

> Note that Gallery doesn't use CSS images anymore, so I wouldn't expect much
> memory improvement when your CSS size prediction stuff lands.  Until canvas
> supports downsample-during-decode, I'm going to have to keep using
> #-moz-samplesize, I guess.

Unless you removed all usage of CSS images, it should make a difference under memory pressure. Though how much of a difference it will make depends on how many CSS images remain. I thought we were still using background-image for some things, though I may misremember. (For example, I thought we were still using them to display a thumbnail version of the image to avoid flashing while the larger version in the <img> element loads.)

Comment 10

[Seth Fowler [:seth] [:s2h]]

(In reply to Seth Fowler [:seth] [:s2h] from comment #9)
> Thanks. So canvas.drawImage() failure fully explains this bug.

This actually begs the question: why is canvas.drawImage() failing? It sounds like it's not failing on 2.2 and not failing on 2.6, but it is failing on 2.5. If that's correct, my guess is that the reason is that bug 1207355 is not present on 2.5. In 2.5 DDD got enabled for all image types, but without bug 1207355, DDD actually worsens memory usage, because we decode at the intrinsic size as well.

Comment 11

[Adam Alldredge [:AdamA][Inactive]]

Sorry about the confusion, the wrong variables are in Comment 0. The correct variables are:

Environmental Variables:
Device: Flame KK 2.6 [Full Flash][512MB]
BuildID: 20151113030210
Gaia: e8c15ae4e5324a210000ee0a869a962aa542009f
Gecko: 0eaf345983b3afc2b426e25a3be93ebf0d93e6c1
Gonk: 205ac4204bbbb2098a8046444acba551ba5dc75a
Version: 45.0a1 (2.6)
Firmware Version: v18D v4
User Agent: Mozilla/5.0 (Mobile; rv:45.0) Gecko/45.0 Firefox/45.0

This issue is Flame only.

It depends on the file, but I see it 10/10(100%) with the same files. the example large image attachment is a photo that is occurring 100% with.

This issue still occurs with the phone memory set at 1024.

Comment 12

[David Flanagan [:djf]]

It looks like this is a regression caused by bug 1181290. It is causing us to decode the 21mp image at full size, without #-moz-samplesize, as if it was a PNG image.

So this is not a deep gecko issue. It is just a screw up. We should fix this and get the fix into 2.5

[Blocking Requested - why for this release]: 

This is a serious bug where thumbnail creation can fail for large images, leaving the user with a permanently black square in their gallery rather than a real thumbnail. It is straightforwardly fixable from Gaia, and we should fix it.

Comment 13

[KTucker [:KTucker][Inactive 3/4/2016]]

Aries is mistakenly in Comment 0. This is a Flame only issue.

Comment 14

[David Flanagan [:djf]]

Actually, this is also a huge performance issue on Aries.  With 100 copies of the example image on my 2.6 Ares device, it takes 99 seconds to scan them all. Since this is just a normal Aries image, it should be faster than that.

With the patch I'm working on, it takes 61 seconds.

So this bug is making Gallery scanning take 1.5x longer than it should on Aries.

Comment 15

[GitHub Autolander Bot]

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

Comment 16

[David Flanagan [:djf]]

Comment on attachment 8688671 [details] [review]
[gaia] davidflanagan:bug1224696 > mozilla-b2g:master

Punam,

We goofed when we landed bug 1181290. That patch used the createThumbnailAndPreview() codepath that predated #-moz-samplesize, so it was decoding jpegs at full size in order to create a preview for them. For large images (like Aries photos) this was slow on Aries and broken on Flame.

This patch adds a new createJPEGThumbnailAndPreview() function that uses cropResizeRotate() to enable downsampling.  It leaves the old non-jpeg function alone to minimize changes for non-jpeg images. The code for saving the preview to the sd card is refactored into a separate function so that both the old and new preview creation code can use it.

I've tested this with the images in apps/gallery/test/images/ but it probably needs for manual testing as well.

Comment 17

[David Flanagan [:djf]]

With this patch, I can scan 100 copies of the attached sample image on a 319mb flame and get thumbnails and previews. It takes 160 seconds, which is not good, but it works at least.  Images this large should be rare on flame, so this is okay. And presumably faster iwth 512 mb.

Comment 18

[David Flanagan [:djf]]

I've noticed that I can reproduce the originally reported black thumbnail issue on Aries when using the test images in apps/gallery/test/images/  The largest jpegs: the 64, 48 and 32 megapixel images have black thumbnails and previews because of this bug. So it is not Flame only, just easier to reproduce on Flame.

Comment 19

[Hema Koka [:hema]]

Regression from previous releases...bad user experience with black screen

Comment 20

[David Flanagan [:djf]]

rebased the PR to re-run tests now that the gij test runner has been fixed

Comment 21

[Punam Dahiya [:pdahiya]]

Here's my observations  while trying to replicate this issue, I copied attached 21 mp image (Example.jpg) on flame-kk and gallery app shows the thumbnail in list view. (Tried by increasing number of copied Example.jpg files to 10, gallery app scans and shows thumbnails). 

I am able to replicate this issue with 64, 48 and 32 megapixel images from test_images folder. So I agree with djf that createThumbnailAndPreview has started failing for really big image jpg files after patch in Bug 1181290 but it appears to be not consistently replicable for ~20 MP images. 

Thanks David for patch, I will review and test patch today and update bug. Thanks!

Comment 22

[David Flanagan [:djf]]

Punam: how much memory is your Flame configured with? With 319mb, I was consistently seeing Example.jpg fail. The bug was reported for a 512mb Flame. In any case, we shouldn't be decoding these images at full size if we don't have to!

Comment 23

[Punam Dahiya [:pdahiya]]

(In reply to David Flanagan [:djf] from comment #22)
> Punam: how much memory is your Flame configured with? With 319mb, I was
> consistently seeing Example.jpg fail. The bug was reported for a 512mb
> Flame. In any case, we shouldn't be decoding these images at full size if we
> don't have to!

I was testing with Flame 1024mb, configuring my device to 512mb replicates the issue for attached image. Thanks!

Comment 24

[Punam Dahiya [:pdahiya]]

Comment on attachment 8688671 [details] [review]
[gaia] davidflanagan:bug1224696 > mozilla-b2g:master

Hi David
The patch looks good and resolves the issue for large jpeg images. I have noted few comments in github for your input. Thanks for the patch!

Comment 25

[David Flanagan [:djf]]

Landed to master: https://github.com/mozilla-b2g/gaia/commit/e9b48b4f9061f20b798267834ece6e7a365df281

Comment 26

[Adam Alldredge [:AdamA][Inactive]]

This issue is verified fixed in Flame 2.6

Environmental Variables:
Device: Flame 2.6 [Full Flash][512mb]
BuildID: 20151123142444
Gaia: bae13c9ac6a91beecd7c94384e2aef25ed1a3214
Gecko: d3d286102ba7f8801e9dfe12d534f49554ba50c0
Gonk: 205ac4204bbbb2098a8046444acba551ba5dc75a
Version: 45.0a1 (2.6) 
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:45.0) Gecko/45.0 Firefox/45.0

Result:
All pictures are appearing correctly in gallery.