robert-ancell / pygif

Python encoder and decoder for the GIF file format.
GNU Lesser General Public License v3.0
12 stars 2 forks source link

Does the test suite validate the number of frames in a decoded GIF? #9

Open sbridewell opened 7 months ago

sbridewell commented 7 months ago

I'm writing a GIF decoder in C#, and I'm using the files from your test-suite folder in my unit tests to validate it (thank you! very useful!)

I'm getting a test failure on the file animation-no-delays.gif. According to the corresponding .conf file this GIF should decode to 4 frames, however my decoder is decoding it to a single frame. I've compared this file in a binary editor with images-combine.gif, which according to its .conf file should (and does, according to my decoder) decode to a single frame, and I can't see any significant difference between the two to indicate that one should decode to multiple frames and the other should decode to a single frame.

Looking at the file run-test-suite (which may be dangerous because I have no experience of Python) I notice a function called run_test containing the following lines:

    frame = frames[-1]
    reference_filename = 'test-suite/%s' % frame['pixels']

    print ('  Comparing to %s' % reference_filename)
    return compare_to_reference_frame (reader, pixels, reference_filename)

If I've understood this correctly (which is probably a big if):

Is run-test-suite missing a trick here?

robert-ancell commented 7 months ago

You are correct, I never completed the test runner to check all frames.

The animation-no-delays.gif case is a tricky one, but designed to match what most existing GIF decoders do. It doesn't contain any graphic control extensions, which would normally indicate the frame boundaries. It does however contain the Netscape Extension, which implies it is animated. So most GIF decoders (e.g. Firefox, Chrome etc) treat each image inside the GIF as an animation frame and add a fixed delay between them. See the force-animation flag in test-suite/README.md.

$ PYTHONPATH=. ./examples/gif-analyse ./test-suite/animation-no-delays.gif 
Version: b'GIF89a'
Size: 2x2 pixels
Original Depth: 8 bits
Colors (2): #000000, #ffffff
Background Color: #000000 (0)
Netscape Extension:
  Loop Count: 0
Image:
  Size: 2x2
  Pixels (4, code-size=2): [1, 0, 0, 0]
Image:
  Size: 2x2
  Pixels (4, code-size=2): [0, 1, 0, 0]
Image:
  Size: 2x2
  Pixels (4, code-size=2): [0, 0, 0, 1]
Image:
  Size: 2x2
  Pixels (4, code-size=2): [0, 0, 1, 0]
sbridewell commented 7 months ago

Thank you, now that you point it out, it makes perfect sense that the presence of the Netscape extension implies that the GIF contains more than one frame (even if that's not how the GIF spec says it should be encoded). Now I just need to apply that insight to the logic in my decoder, so that it can cope with GIFs which are badly encoded in this way in the same way that most browsers do...