1206206 - Remove #-moz-samplesize from shared/js/media/media_frame.js

Status: RESOLVED WONTFIX

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

Description

[David Flanagan [:djf]]

The #-moz-samplesize media fragment has been superseded by gecko's built-in downsampling-during-decode support, and we should remove all the code that uses it.

This bug is to remove it from the MediaFrame utility that the Gallery and Camera apps use.

Comment 1

[David Flanagan [:djf]]

Attachment: wip patch, carried over from bug 1140619

This is the patch I was working on in bug 1140619 before that bug was closed.

Last time I tried it, downsample-on-decode was not working right in gecko, and applying the patch caused dramatically increased memory usage.

Hopefully it will work now.

Comment 2

[David Flanagan [:djf]]

Baseline test of what the gallery app does today:

I put a 20mp photo from Aries onto a 1024mb Flame device.

When I first view the image (without zooming) and look at an about:memory report, I see this:

│  │  ├───7.17 MB (19.99%) -- image(656x492, blob:app://gallery.gaiamobile.org/0a89a71a-ece0-42de-9e03-26bc2dc3a3fd#-moz-samplesize=8)
│  │  │   ├──5.94 MB (16.55%) ── source
│  │  │   └──1.23 MB (03.44%) -- locked/surface(656x492@0)
│  │  │      ├──1.23 MB (03.44%) ── decoded-nonheap
│  │  │      └──0.00 MB (00.00%) ── decoded-heap

We're using -moz-samplesize=8 to decode the image at 1/8th the width and height. The encoded image size of 6mb is much larger that the downsampled decoded size of 1mb. The reported size of 656x492 is exactly 1/8th of the original width and height, and the 1.23mb memory usage is exactly what would be expected for that many pixels.

Now if I double-tap to zoom in and do another about:memory report, I get:

│  │  ├──25.64 MB (44.72%) -- image(2624x1968, blob:app://gallery.gaiamobile.org/0a89a71a-ece0-42de-9e03-26bc2dc3a3fd#-moz-samplesize=2)
│  │  │  ├──19.70 MB (34.36%) -- locked/surface(2624x1968@0)
│  │  │  │  ├──19.70 MB (34.36%) ── decoded-nonheap
│  │  │  │  └───0.00 MB (00.00%) ── decoded-heap
│  │  │  └───5.94 MB (10.36%) ── source

Now, we're using -moz-samplesize=2. The image is decoded at exactly half width and height, and the 19.7mb memory usage seems right.

Comment 3

[David Flanagan [:djf]]

Baseline performance: tt takes about 1.3s (measured by stopwatch) to fully display a 20mp Aries image on my 1024mb Flame. (In part because the Aries images don't have EXIF previews, so the entire thing needs to be decoded.)

This is not a reasonable test for the Flame hardware, but I'm going to use it as a baseline to see if removing -moz-samplesize speeds things up at all. When Seth was working on this, he had some concerns that my continued use of -moz-samplesize might cause gecko to decode images twice.

Comment 4

[David Flanagan [:djf]]

With the patch applied, it takes 2.5 to 3s to display a 20mp image when I first tap on it.  And when I swipe sideways through the images, they sometimes refuse to decode at all.  I suspect that means that downsample-on-decode is not working for Gallery, but will do about:memory tests next.

Maybe there is something wrong with my patch, and I'll be able to work with Seth to sort it out. Or maybe DDD just won't work and we're going to have to stick with -moz-samplesize for now.

Comment 5

[David Flanagan [:djf]]

Okay, with the patch, when I tap on the icon for a 20mp image, it ends up decoding it at full-size, even before I zoom in:

│   ├──86.73 MB (80.04%) -- content/raster/used
│   │  ├──82.79 MB (76.41%) -- image(5248x3936, blob:app://gallery.gaiamobile.org/f3e1655f-736d-4138-a8cc-b9d1bf9b58a1)
│   │  │  ├──78.80 MB (72.72%) -- locked/surface(5248x3936@0)
│   │  │  │  ├──78.80 MB (72.72%) ── decoded-nonheap
│   │  │  │  └───0.00 MB (00.00%) ── decoded-heap
│   │  │  └───3.99 MB (03.68%) ── source

The image is being displayed at a much smaller size than this, but is still decoded at full size.

When I double-tap to zoom in and do another memory report, nothing has changed. The image still takes up 80+mb

I'll double check my patch then flag Seth to see if he has any ideas.

Comment 6

[David Flanagan [:djf]]

I don't see anything obviously wrong with my patch.  I've tried dropping the maximum decode size to 1mp, and removing any code that does transforms or transitions or will-scroll, but the image is still being decoded at full 20mp size.

Comment 7

[David Flanagan [:djf]]

So I'm not convinced that ddd is working at all. My Flame is flashed with the latest nightly. When I load http://djf.net/pen.html into the browser, it displays a 5mp image at 300x400px. When I do an about:memory report on it, I see that the image is decoded at full size and is using 20mb of memory.

│  │  ├──20.02 MB (20.19%) -- image(1944x2592, http://djf.net/pen.jpg)
│  │  │  ├──19.22 MB (19.39%) -- locked/surface(1944x2592@0)
│  │  │  │  ├──19.22 MB (19.39%) ── decoded-nonheap
│  │  │  │  └───0.00 MB (00.00%) ── decoded-heap

The same thing happens if I convert this test into a packaged web app and run it.

Seth, can you help me out here? Is DDD enabled in FirefoxOS 2.5? What do I need to do to get it to work?

Comment 8

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

(In reply to David Flanagan [:djf] from comment #7)
> Seth, can you help me out here? Is DDD enabled in FirefoxOS 2.5? What do I
> need to do to get it to work?

DDD was accidentally disabled on B2G and Android as a side effect of bug 1060609 part 1; I forgot that HQ scaling was disabled on those platforms. I unfortunately did not notice this until yesterday. 

Bug 1146663, which I'm planning to land before the branch on Monday, will fix this problem. Sorry about the trouble.

Comment 9

[David Flanagan [:djf]]

Now that bug 1146663 has landed, I've flashed with today's nightly and tried my patch again. No change. 20mp images are still being decoded at full size.

Seth: any ideas?

Comment 10

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

(In reply to David Flanagan [:djf] from comment #9)
> Now that bug 1146663 has landed, I've flashed with today's nightly and tried
> my patch again. No change. 20mp images are still being decoded at full size.
> 
> Seth: any ideas?

Not sure what's happening. It's definitely working on desktop!

I'll try it myself later today.

Comment 11

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

I can actually reproduce the issue on desktop. The problem is that we got rid of decode-only-on-draw, which was superseded by size prediction, but we never actually went all the way and removed the non-size-prediction stuff. So we end up running both the "old" code and the "new" code, and decoding at both the size we'll use for drawing and the intrinsic size.

Bug 1207355 fixes the issue on desktop by removing the "old" code everything except for CSS images. With the patches in that bug applied, we only decode content images at the size we'll use to draw. Should work for B2G too, though I haven't tested it there.

Comment 12

[David Flanagan [:djf]]

Thanks for the update, Seth. I didn't understand that decode-on-draw was gone.

Will size prediction work for the case where we load into an offscreen image and then draw it into a canvas? Will that decode at the intrinsic size of the image or at the size of the canvas?

Comment 13

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

(In reply to David Flanagan [:djf] from comment #12)
> Thanks for the update, Seth. I didn't understand that decode-on-draw was
> gone.

The constant changes are getting confusing, even to me. =) Decode-on-draw-only was always a temporary thing until size prediction was ready. (Since obviously it's better to start decoding *before* we draw.)

> Will size prediction work for the case where we load into an offscreen image
> and then draw it into a canvas? Will that decode at the intrinsic size of
> the image or at the size of the canvas?

Right now it will decode at the intrinsic size only because canvas does not use high-quality scaling, and that's the only scaling algorithm implemented for downscale-during-decode right now. If that's something you need, though, we can add support for it.

Comment 14

[David Flanagan [:djf]]

(In reply to Seth Fowler [:seth] [:s2h] from comment #13)
> Right now it will decode at the intrinsic size only because canvas does not
> use high-quality scaling, and that's the only scaling algorithm implemented
> for downscale-during-decode right now. If that's something you need, though,
> we can add support for it.

I absolutely need that, in order to create thumbnails. I can't remove #-moz-samplesize from my thumbnail creation code until ddd supports that use case.