Skip to content

Commit

Permalink
fix: improve GIF loop count detection and add test coverage
Browse files Browse the repository at this point in the history
- Enhance loop count detection to handle multiple extension blocks
- Add proper cleanup and error handling in GIF parsing
- Add test cases for duplicate loop counts and multiple extensions
  • Loading branch information
skidder committed Nov 7, 2024
1 parent dd020a2 commit 93f64f9
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 19 deletions.
61 changes: 42 additions & 19 deletions giflib.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1134,32 +1134,55 @@ int giflib_decoder_get_loop_count(const giflib_decoder d) {
return loop_count;
}

// Read extension blocks
// Read all blocks until we find NETSCAPE2.0 or hit end
GifRecordType recordType;
if (DGifGetRecordType(gif, &recordType) == GIF_OK &&
recordType == EXTENSION_RECORD_TYPE) {

GifByteType* ExtData;
int ExtFunction;

if (DGifGetExtension(gif, &ExtFunction, &ExtData) == GIF_OK && ExtData != NULL) {
// Look for NETSCAPE2.0 extension
if (ExtFunction == APPLICATION_EXT_FUNC_CODE && ExtData[0] >= 11 &&
memcmp(ExtData + 1, "NETSCAPE2.0", 11) == 0) {
// Get the next block with loop count
if (DGifGetExtensionNext(gif, &ExtData) == GIF_OK &&
ExtData != NULL &&
ExtData[0] >= 3 &&
ExtData[1] == 1) {

loop_count = ExtData[2] | (ExtData[3] << 8);
while (DGifGetRecordType(gif, &recordType) == GIF_OK) {
switch (recordType) {
case EXTENSION_RECORD_TYPE: {
GifByteType* ExtData;
int ExtFunction;

if (DGifGetExtension(gif, &ExtFunction, &ExtData) == GIF_OK && ExtData != NULL) {
// Look for NETSCAPE2.0 extension
if (ExtFunction == APPLICATION_EXT_FUNC_CODE && ExtData[0] >= 11 &&
memcmp(ExtData + 1, "NETSCAPE2.0", 11) == 0) {
// Get the next block with loop count
if (DGifGetExtensionNext(gif, &ExtData) == GIF_OK &&
ExtData != NULL &&
ExtData[0] >= 3 &&
ExtData[1] == 1) {
loop_count = ExtData[2] | (ExtData[3] << 8);
goto cleanup; // Found what we need
}
}

// Skip any remaining extension blocks
while (ExtData != NULL) {
if (DGifGetExtensionNext(gif, &ExtData) != GIF_OK) {
goto cleanup;
}
}
}
break;
}

case IMAGE_DESC_RECORD_TYPE:
// Skip image data
if (DGifGetImageDesc(gif) != GIF_OK) {
goto cleanup;
}
break;

case TERMINATE_RECORD_TYPE:
goto cleanup;

default:
break;
}
}

cleanup:
DGifCloseFile(gif, &error);
delete loopReader;

return loop_count;
}
Binary file added testdata/dispose_bgnd.gif
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added testdata/duplicate_number_of_loops.gif
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
20 changes: 20 additions & 0 deletions webp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,26 @@ func testNewWebpEncoderWithAnimatedGIFSource(t *testing.T) {
resizeMethod: ImageOpsResize,
wantLoops: 1,
},
{
name: "Animated GIF with duplicate number of loop count, use the first loop count",
inputPath: "testdata/duplicate_number_of_loops.gif",
outputPath: "testdata/out/duplicate_number_of_loops.webp",
width: 200,
height: 200,
quality: 80,
resizeMethod: ImageOpsResize,
wantLoops: 2,
},
{
name: "Animated GIF with multiple extension blocks",
inputPath: "testdata/dispose_bgnd.gif",
outputPath: "testdata/out/dispose_bgnd.webp",
width: 200,
height: 200,
quality: 80,
resizeMethod: ImageOpsResize,
wantLoops: 0,
},
}

for _, tc := range testCases {
Expand Down

0 comments on commit 93f64f9

Please sign in to comment.