Bug 146205 – Decoding of animated PNG (APNG) is not optimized for memory usage.

• Status: RESOLVED
• Resolution: FIXED
• Component: Images

[Carlos Alberto Lopez Perez]

The current code for decoding animated PNGs (see bug 17022.) decodes all frames at once
and loads the raw pixels on memory for each frame.

This is a problem when we have an animated PNG with many frames.

Take the following example:

http://people.igalia.com/clopez/apngmem

The image loaded (clouds.png) has 375 frames at 700 x 394 (8-bit/color RGBA),
so when each frame of this image is loaded into memory it uses:

700*394*4 ~ 1.05 MB

That should be ok if the image had only a few frames, however it has 375 frames,
so when we load all the 375 frames into memory we end using around ~400MB,
which is too much.


The current PNGImageDecoder decoder implements PNGImageDecoder::clearFrameBufferCache(),
which helps to clear the already displayed frames. So if you check the memory
usage of WebKitWebProcess when the example test page is playing the animation,
you will see that the memory used by WebKitWebProcess lowers as it displays
the animation, however when the animation ends and starts again, the memory usage
peaks again because it decodes all the frames into memory again.


So, instead of loading all the frames on memory at once, I think it should be better
if it loads the frames on demand as the animation shows on the screen.


The GIF decoder has a parameter named haltAtFrame that seems designed to 
avoid this very same isue. See:
http://trac.webkit.org/browser/trunk/Source/WebCore/platform/image-decoders/gif/GIFImageDecoder.cpp?rev=185822#L299
http://trac.webkit.org/browser/trunk/Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp?rev=185822#L354

For example, check loading this gif (1116 frames at 800 x 800) <http://people.igalia.com/clopez/apngmem/giant-redacted.gif>
on the WebKitGTK+ MiniBrowser and on some image viewer (eog, ristretto, ...).

You will see that on the WebKitGTK+ MiniBrowser it only uses a few megabytes
while on the image viewer it would use so much ram that it will end being killed by the OOM.

I have converted this gif to png with gif2apng. The result is here <http://people.igalia.com/clopez/apngmem/giant-redacted.png>
If you download it and open with WebKitGTK+ you will see that it uses a lot of ram. While with the gif version that don't happens.

So, I think that the png decoder should gain support to only decode the frames
up to a haltAtFrame parameter like the GIFImageDecoder does.

[Carlos Alberto Lopez Perez]

(In reply to comment #0)
> The current code for decoding animated PNGs (see bug 17022.) decodes all
> frames at once
> and loads the raw pixels on memory for each frame.
> 
> This is a problem when we have an animated PNG with many frames.
> 
> Take the following example:
> 
> http://people.igalia.com/clopez/apngmem
> 
> The image loaded (clouds.png) has 375 frames at 700 x 394 (8-bit/color RGBA),
> so when each frame of this image is loaded into memory it uses:
> 
> 700*394*4 ~ 1.05 MB
> 
> That should be ok if the image had only a few frames, however it has 375
> frames,
> so when we load all the 375 frames into memory we end using around ~400MB,
> which is too much.
> 
>

I have replaced the previous examples with one that includes the same test with gif and apng because is much clearer the issue.

Now if you load http://people.igalia.com/clopez/apngmem you see there two test pages, the one with gif and the another one with apng.

On the one with gif WebKitWebProcess only uses ~110MB in total, on the one with apng it peaks at ~ 3GB of RAM, and as the animation is playing the memory usage starts to lower (because PNGImageDecoder::clearFrameBufferCache() is starting to clear already played frames), but when the animation finish and it starts again the memory usage peaks again at 3GB of RAM because we load all frames into memory.

[Max Stepin]

Created attachment 261090 [details]
v01.patch

Only decode the frames up to haltAtFrame

[Carlos Alberto Lopez Perez]

(In reply to comment #2)
> Created attachment 261090 [details]
> v01.patch
> 
> Only decode the frames up to haltAtFrame

Thanks for the patch Max.

I have tested it, and it fixes the problem with the memory consumption completely.


The problem now is that I can see some artifacts on the imagen.

I have taken two screen-shoots to show you what I'm talking about:

 * http://people.igalia.com/clopez/apngmem/artifacts/while_playing.png This how the image looks on the MiniBrowser after playing it for some seconds. The numbers on the nodes showing the network traffic are blurry. Looks like if the numbers from the previous frames were not cleared somehow.

http://people.igalia.com/clopez/apngmem/artifacts/after_resize_window.png This is what happens if I resize the window of the MiniBrowser while it plays the animation. The image gets completely messed up.

[Max Stepin]

If segmentLength is big enough we might overshoot a little, and decode not just haltAtFrame but a few more frames after haltAtFrame.

It might potentially lead to a problem with clearFrameBufferCache().

I guess we need to find a way to stop precisely at haltAtFrame. 

There is something similar: decodingSizeOnly() case stops precisely at the header, I'll see if we can use this method.

[Max Stepin]

Created attachment 262881 [details]
v02.patch

Only decode the frames up to haltAtFrame, also set the frame attributes as soon as possible, instead of waiting until frameComplete(). 

That was the reason for the artifacts - 'dispose' attribute was used by clearFrameBufferCache() a bit earlier, before it was actually set.

[Carlos Alberto Lopez Perez]

This last patch fixes the artifact issue. Now the image is displayed correctly and the memory usage problem is fixed. (70MB now vs 2.5GB before).

Thanks!

[Carlos Alberto Lopez Perez]

I think he also needs cq+?

[Michael Catanzaro]

Comment on attachment 262881 [details]
v02.patch

OK, I see no reason not to commit this.

[WebKit Commit Bot]

Comment on attachment 262881 [details]
v02.patch

Clearing flags on attachment: 262881

Committed r194503: <http://trac.webkit.org/changeset/194503>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.