Bug 93764 - Skipping frames in animated GIFs (or other multiframe images) can cause inifinite loop in WebCore::BitmapImage::startAnimation()
Summary: Skipping frames in animated GIFs (or other multiframe images) can cause inifi...
Status: RESOLVED DUPLICATE of bug 116041
Alias: None
Product: WebKit
Classification: Unclassified
Component: Images (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-08-11 03:10 PDT by Tobias Netzel
Modified: 2013-06-25 13:39 PDT (History)
1 user (show)

See Also:


Attachments
don't force decoding of skipped frames (1.30 KB, patch)
2012-08-11 03:10 PDT, Tobias Netzel
no flags Details | Formatted Diff | Diff
prepared for review (2.16 KB, patch)
2012-08-13 12:32 PDT, Tobias Netzel
no flags Details | Formatted Diff | Diff
patch format as produced by the scripts (1.93 KB, patch)
2013-01-17 02:02 PST, Tobias Netzel
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tobias Netzel 2012-08-11 03:10:52 PDT
Created attachment 157864 [details]
don't force decoding of skipped frames

The current code for frame skipping in WebCore::BitmapImage::startAnimation() forces decoding of skipped frames by calling frameIsCompleteAtIndex() and frameDurationAtIndex(). On slow or VERY busy machines and animated images larger than 5MB ( ->cLargeAnimationCutoff) that will cause the for loop to be started over and over again without displaying any frames in case decoding the frame takes more time than the frame's duration. Effectively the application will seem to hang.

In the patch I made that frame skipping loop do it the same way as the loop in BitmapImage::dataChanged(); the comment in dataChanged() is also valid for the frame skipping loop.
Comment 1 Alexey Proskuryakov 2012-08-13 09:55:01 PDT
This patch is for a branch. Would you be willing to post a patch against truck for review, following <http://www.webkit.org/coding/contributing.html>? Notably, the patch misses a ChangeLog.
Comment 2 Tobias Netzel 2012-08-13 12:32:47 PDT
Created attachment 158074 [details]
prepared for review

Added a ChangeLog entry.

Should be covered by existing tests. I didn't run the tests and I don't have the resources to do so.
I found this website which should be enough stress testing for animated GIFs:
http://twistedsifter.com/2012/06/high-quality-animated-gifs-by-micael-reynaud/
Comment 3 Tobias Netzel 2012-11-23 05:42:09 PST
May I kindly ask for review? Thank you!
Comment 4 Tobias Netzel 2013-01-17 01:55:36 PST
Is there anything I can do to get someone providing some feedback?
Comment 5 Tobias Netzel 2013-01-17 02:02:56 PST
Created attachment 183150 [details]
patch format as produced by the scripts
Comment 6 Tobias Netzel 2013-06-25 12:48:02 PDT
Finally someone else fixed this - but why was it impossible to get this patch committed?

*** This bug has been marked as a duplicate of bug 116041 ***