Bug 93764

Summary:

Skipping frames in animated GIFs (or other multiframe images) can cause inifinite loop in WebCore::BitmapImage::startAnimation()

Product:

WebKit

Reporter:

Tobias Netzel <tobias.netzel>

Component:

Images

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED DUPLICATE

Severity:

Normal

CC:

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
don't force decoding of skipped frames	none
prepared for review	none
patch format as produced by the scripts	none

Tobias Netzel

Reported 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.

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
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Alexey Proskuryakov

Comment 1 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.

Tobias Netzel

Comment 2 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/

Tobias Netzel

Comment 3 2012-11-23 05:42:09 PST

May I kindly ask for review? Thank you!

Tobias Netzel

Comment 4 2013-01-17 01:55:36 PST

Is there anything I can do to get someone providing some feedback?

Tobias Netzel

Comment 5 2013-01-17 02:02:56 PST

Created attachment 183150 [details] patch format as produced by the scripts

Tobias Netzel

Comment 6 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 ***

Note You need to log in before you can comment on or make changes to this bug.