Summary: | Decoding of animated PNG (APNG) is not optimized for memory usage. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Carlos Alberto Lopez Perez <clopez> | ||||||
Component: | Images | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | cgarcia, commit-queue, maxstepin, sagar73510, superpuperbomb | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
URL: | http://people.igalia.com/clopez/apngmem | ||||||||
Attachments: |
|
Description
Carlos Alberto Lopez Perez
2015-06-22 07:10:26 PDT
(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. Created attachment 261090 [details]
v01.patch
Only decode the frames up to haltAtFrame
(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. 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. 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.
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! I think he also needs cq+? Comment on attachment 262881 [details]
v02.patch
OK, I see no reason not to commit this.
Comment on attachment 262881 [details] v02.patch Clearing flags on attachment: 262881 Committed r194503: <http://trac.webkit.org/changeset/194503> All reviewed patches have been landed. Closing bug. |