Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Option to retain gif loop count when resizing #124

Closed
iamcarbon opened this issue Oct 6, 2023 · 6 comments
Closed

Option to retain gif loop count when resizing #124

iamcarbon opened this issue Oct 6, 2023 · 6 comments

Comments

@iamcarbon
Copy link
Contributor

The library currently overrides the loop count when resizing. It would be useful to be able to maintain the original loop count, or override it in GifEncoderOptions.

@iamcarbon iamcarbon changed the title Option to retain gif loop count when resizing GIFs Option to retain gif loop count when resizing Oct 6, 2023
@saucecontrol
Copy link
Owner

saucecontrol commented Oct 6, 2023

The loop count should be preserved by the current implementation, using both of the common application extensions (NETSCAPE2.0 and ANIMEXTS1.0)

It's read here:

int loopCount = 0;
var sbuff = (Span<byte>)stackalloc byte[16];
var appext = meta.Get()->GetValueOrDefault(Wic.Metadata.Gif.AppExtension, sbuff);
if (appext.Length == 11 && (appext.SequenceEqual(Netscape2_0) || appext.SequenceEqual(Animexts1_0)))
{
var appdata = meta.Get()->GetValueOrDefault(Wic.Metadata.Gif.AppExtensionData, sbuff);
if (appdata.Length >= 4 && appdata[0] >= 3 && appdata[1] == 1)
loopCount = BinaryPrimitives.ReadUInt16LittleEndian(appdata[2..]);
}
int par = meta.Get()->GetValueOrDefault<byte>(Wic.Metadata.Gif.PixelAspectRatio);
float pixelAspect = par == default ? 1f : ((par + 15) / 64f);
AnimationMetadata = new(screenWidth, screenHeight, (int)fcount, loopCount, bgColor, pixelAspect, true);

And written here:

var appext = WicGifContainer.Netscape2_0;
var pvae = new PROPVARIANT { vt = (ushort)(VARENUM.VT_UI1 | VARENUM.VT_VECTOR) };
pvae.Anonymous.blob.cbSize = (uint)appext.Length;
pvae.Anonymous.blob.pBlobData = appext.GetAddressOf();
HRESULT.Check(encmeta.Get()->SetMetadataByName(Wic.Metadata.Gif.AppExtension, &pvae));
byte* pvdd = stackalloc byte[] { 3, 1, 0, 0 };
BinaryPrimitives.WriteUInt16LittleEndian(new Span<byte>(pvdd + 2, 2), (ushort)anicnt.LoopCount);
var pvad = new PROPVARIANT { vt = (ushort)(VARENUM.VT_UI1 | VARENUM.VT_VECTOR) };
pvad.Anonymous.blob.cbSize = 4;
pvad.Anonymous.blob.pBlobData = pvdd;
HRESULT.Check(encmeta.Get()->SetMetadataByName(Wic.Metadata.Gif.AppExtensionData, &pvad));

It's possible there's a WIC bug causing the write to fail in some cases, but my test for this is still passing.

Do you have a sample that fails you can share?

@iamcarbon
Copy link
Contributor Author

iamcarbon commented Oct 6, 2023

This is the first time I've seen this behavior and it may be unique to this specific case.

Here's a simple test case that shows the loop count change after transformation. The original image plays once in the browser and ImageSharp. The transformed image gets the loop extension to repeat indefinitely.

NOTE: imageSharp's repeatCount's is 1 based, and does not map directly to the extension value.

 [Fact]
 public async Task RetainsLoopCount()
 {
     using var httpClient = new HttpClient();

     // no NETSCAPE2.0 extension
     // plays once in the browser
     var sourceUrl = "https://carbon-media.accelerator.net/0000000mdrD/jWYe1Diz17Kgj1Vt6FE4QW";
     var input = await httpClient.GetByteArrayAsync(sourceUrl);

     var sourceMetadata = SixLabors.ImageSharp.Image.Load(input).Metadata.GetGifMetadata();

     Assert.Equal(1, sourceMetadata.RepeatCount); // ImageSharp count is set as repeat n - 1 times.

     using var output = new MemoryStream();

     // The image is set to repeat indefinitely after transformation
     var transformed =  MagicImageProcessor.ProcessImage(input, output, new ProcessImageSettings { Width = 400 });

     output.Position = 0;

     var transformedMetadata = SixLabors.ImageSharp.Image.Load(output).Metadata.GetGifMetadata();

     Assert.Equal(0, transformedMetadata.RepeatCount); // 0 = repeat indefinitely 
 }

@saucecontrol
Copy link
Owner

Ah, thanks. So that gif doesn't have an application extension block at all, and I'm attempting to normalize the metadata on output. I thought the default behavior in browsers was to infinite loop in the absence of a valid extension with count, so I was normalizing to a default of loop count of 0. It could be that behavior is only applied when the extension is present but invalid and that a single play is default when the extension is missing entirely.

I'll see if I can find any more authoritative reference on that, but it seems changing the default is a safe fix.

@iamcarbon
Copy link
Contributor Author

iamcarbon commented Oct 6, 2023

I'll let you know if I find a more authoritative reference too.

FYI, the loop count here was originally modified via an FFMPEG command, which removed the extension all together.

./ffmpeg.exe -i https://carbon-media.accelerator.net/0000000mdrD/exXZzUXQSd0fPkyLbBkNFU -loop -1  new-logo.gif

@saucecontrol
Copy link
Owner

There's really no documented standard for the animation stuff other than what Netscape implemented and others copied. I usually refer to https://web.archive.org/web/20010813104712/http://member.aol.com/royalef/gifabout.htm for the edge case behavior. Seems Netscape used to always loop infinitely, which is probably where I got my default. On a quick test of modern browsers:

Chrome plays once with either missing or invalid application extension.
Firefox plays once with missing extension and errors on an invalid extension.

So that seems pretty definitive.

@saucecontrol
Copy link
Owner

Fixed in 11e96cc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants