saucecontrol / PhotoSauce

MagicScaler high-performance, high-quality image processing pipeline for .NET
http://photosauce.net/
MIT License
589 stars 49 forks source link

Option to retain gif loop count when resizing #124

Closed iamcarbon closed 12 months ago

iamcarbon commented 1 year ago

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.

saucecontrol commented 1 year ago

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: https://github.com/saucecontrol/PhotoSauce/blob/f244b741cc3d9dbf6c11c31f184a58d0a907a1a6/src/MagicScaler/WIC/WicImageContainer.cs#L121-L134

And written here: https://github.com/saucecontrol/PhotoSauce/blob/f244b741cc3d9dbf6c11c31f184a58d0a907a1a6/src/MagicScaler/WIC/WicCodec.cs#L132-L145

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 12 months ago

Fixed in 11e96cc