386268 - Animated GIF doesn't loop properly.

Status: VERIFIED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Justin Dolske [:Dolske]]

Attachment: Wheeeeeeeeeee! (loops over just a couple frames)

I noticed today that this little animation only seems to play a few frames before repeating. Looks like a regression from 196295... From IRC:

<ispiked> I confirmed it regressed between 2007-06-25-04 and 2007-06-26-04, if you want to add that to the bug.

Comment 1

[Justin Dolske [:Dolske]]

Hmm, I looked at some other animated gifs created with Adobe ImageReady, and none of them play properly.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Might be fixed by the patch in bug 386269 or the new patch in bug 196295?

Comment 3

[Alfred Kayser]

Attachment: Patch to fix the broken 'broken gif' handling

Found it, the problem is in handling of broken gif's.

The code before:
        /* Make sure there are still rows left. If the GIF data */
        /* is corrupt, we may not get an explicit terminator.   */
        if (gs->rows_remaining == 0) {
          /* This is an illegal GIF, but we remain tolerant. */
#ifdef DONT_TOLERATE_BROKEN_GIFS
          gs->state = gif_error;
          break;
#else
          GETN(1, gif_sub_block);
#endif
        }

was with my patch changed to:
        /* Make sure there are still rows left. If the GIF data */
        /* is corrupt, we may not get an explicit terminator.   */
        if (gs->rows_remaining == 0) {
          /* This is an illegal GIF, but we remain tolerant. */
#ifdef DONT_TOLERATE_BROKEN_GIFS
          gs->state = gif_error;
#else
          GETN(1, gif_sub_block);
#endif
        }
        break;

This resulted in aborting of decoding of 'broken' gif images.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 270312 [details] [diff] [review]
Patch to fix the broken 'broken gif' handling

You'll have to ask a peer for this module for review.

Comment 5

[Alfred Kayser]

Comment on attachment 270312 [details] [diff] [review]
Patch to fix the broken 'broken gif' handling

Christian, can you review this?

Comment 6

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

May want to ask andrew (asmith, iirc) or stuart for review if biesi doesn't get to it.

Comment 8

[Bill Gianopoulos [:WG9s]]

OK -> ALL as this occurs in Windows and Linux as well.

Comment 9

[Alfred Kayser]

Who can do the checkin for me?

Comment 10

[Dão Gottwald [:dao]]

Looks like there aren't many people around who can do checkins and look for / care about "checkin-needed" bugs. If it takes too long (e.g. blocks your work in other bugs), you might want to ask directly in #developers.

Comment 11

[Michael Wu [:mwu]]

Checking in modules/libpr0n/decoders/gif/nsGIFDecoder2.cpp;
/cvsroot/mozilla/modules/libpr0n/decoders/gif/nsGIFDecoder2.cpp,v  <--  nsGIFDecoder2.cpp
new revision: 1.79; previous revision: 1.78
done