From ac1905328d37213c92fcf57d32417f9322294fe1 Mon Sep 17 00:00:00 2001 From: Andreas Eriksson <4438107+andreas-eriksson@users.noreply.github.com> Date: Wed, 1 Apr 2026 09:51:36 +0200 Subject: [PATCH 1/4] Fix Identify returning incorrect frame count for animated PNGs The Identify method had two bugs when processing fdAT (FrameData) chunks: 1. A spurious Skip(4) before SkipChunkDataAndCrc caused the stream to be misaligned by 4 bytes, since chunk.Length already includes the 4-byte sequence number. 2. Unlike Decode, which consumes all fdAT chunks for a frame in one shot via ReadScanlines + ReadNextFrameDataChunk, Identify processed them individually, calling InitializeFrameMetadata for each chunk and inflating the frame count. The fix removes the extra Skip(4) and adds SkipRemainingFrameDataChunks to consume all continuation fdAT chunks for a frame, mirroring how ReadNextFrameDataChunk works during decoding. --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 30 +++++++++++++++++-- .../Formats/Png/PngDecoderTests.cs | 12 ++++++++ tests/ImageSharp.Tests/TestImages.cs | 1 + .../animated/issue-animated-frame-count.png | 3 ++ 4 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 tests/Images/Input/Png/animated/issue-animated-frame-count.png diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 8962182679..52858ec129 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -428,9 +428,10 @@ protected override ImageInfo Identify(BufferedReadStream stream, CancellationTok InitializeFrameMetadata(framesMetadata, currentFrameControl.Value); - // Skip sequence number - this.currentStream.Skip(4); + // Skip data for this and all remaining FrameData chunks belonging to the same frame + // (comparable to how Decode consumes them via ReadScanlines + ReadNextFrameDataChunk). this.SkipChunkDataAndCrc(chunk); + this.SkipRemainingFrameDataChunks(buffer); break; case PngChunkType.Data: @@ -2093,6 +2094,31 @@ private int ReadNextFrameDataChunk() return 0; } + /// + /// Skips any remaining chunks belonging to the current frame. + /// This mirrors how is used during decoding: + /// consecutive fdAT chunks are consumed until a non-fdAT chunk is encountered, + /// which is stored in for the next iteration. + /// + /// Temporary buffer. + private void SkipRemainingFrameDataChunks(Span buffer) + { + while (this.TryReadChunk(buffer, out PngChunk chunk)) + { + if (chunk.Type is PngChunkType.FrameData) + { + chunk.Data?.Dispose(); + this.SkipChunkDataAndCrc(chunk); + } + else + { + // Not a FrameData chunk; store it so the next TryReadChunk call returns it. + this.nextChunk = chunk; + return; + } + } + } + /// /// Reads a chunk from the stream. /// diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index a58101a6bd..69e656849b 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -411,6 +411,18 @@ public void Identify_IgnoreCrcErrors(string imagePath, int expectedPixelSize) Assert.Equal(expectedPixelSize, imageInfo.PixelType.BitsPerPixel); } + [Fact] + public void Identify_AnimatedPng_ReadsFrameCountCorrectly() + { + TestFile testFile = TestFile.Create(TestImages.Png.AnimatedFrameCount); + + using MemoryStream stream = new(testFile.Bytes, false); + ImageInfo imageInfo = Image.Identify(stream); + + Assert.NotNull(imageInfo); + Assert.Equal(50, imageInfo.FrameMetadataCollection.Count); + } + [Theory] [WithFile(TestImages.Png.Bad.MissingDataChunk, PixelTypes.Rgba32)] public void Decode_MissingDataChunk_ThrowsException(TestImageProvider provider) diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index fab1b2891c..730e62d824 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -76,6 +76,7 @@ public static class Png public const string BlendOverMultiple = "Png/animated/21-blend-over-multiple.png"; public const string FrameOffset = "Png/animated/frame-offset.png"; public const string DefaultNotAnimated = "Png/animated/default-not-animated.png"; + public const string AnimatedFrameCount = "Png/animated/issue-animated-frame-count.png"; public const string Issue2666 = "Png/issues/Issue_2666.png"; public const string Issue2882 = "Png/issues/Issue_2882.png"; diff --git a/tests/Images/Input/Png/animated/issue-animated-frame-count.png b/tests/Images/Input/Png/animated/issue-animated-frame-count.png new file mode 100644 index 0000000000..db8ff47b9b --- /dev/null +++ b/tests/Images/Input/Png/animated/issue-animated-frame-count.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:62d51679bcb096ae45ae0f5bf874916ad929014f68ae43b487253d5050c8b68b +size 13561079 From 7b13e1df1b909510ba1303e4b7b25d9d1d8e9df4 Mon Sep 17 00:00:00 2001 From: Andreas Eriksson <4438107+andreas-eriksson@users.noreply.github.com> Date: Wed, 1 Apr 2026 12:58:13 +0200 Subject: [PATCH 2/4] Add generated animated PNG tests for Identify and Decode frame counts --- .../Formats/Png/PngDecoderTests.cs | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 69e656849b..0ba8866127 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -423,6 +423,55 @@ public void Identify_AnimatedPng_ReadsFrameCountCorrectly() Assert.Equal(50, imageInfo.FrameMetadataCollection.Count); } + [Theory] + [InlineData(1)] + [InlineData(2)] + [InlineData(5)] + [InlineData(10)] + [InlineData(100)] + public void Identify_AnimatedPng_FrameCount_MatchesDecode(int frameCount) + { + using Image image = new(10, 10, Color.Red.ToPixel()); + for (int i = 1; i < frameCount; i++) + { + using ImageFrame frame = new(Configuration.Default, 10, 10); + image.Frames.AddFrame(frame); + } + + using MemoryStream stream = new(); + image.Save(stream, new PngEncoder()); + stream.Position = 0; + + ImageInfo imageInfo = Image.Identify(stream); + + Assert.NotNull(imageInfo); + Assert.Equal(frameCount, imageInfo.FrameMetadataCollection.Count); + } + + [Theory] + [InlineData(1)] + [InlineData(2)] + [InlineData(5)] + [InlineData(10)] + [InlineData(100)] + public void Decode_AnimatedPng_FrameCount(int frameCount) + { + using Image image = new(10, 10, Color.Red.ToPixel()); + for (int i = 1; i < frameCount; i++) + { + using ImageFrame frame = new(Configuration.Default, 10, 10); + image.Frames.AddFrame(frame); + } + + using MemoryStream stream = new(); + image.Save(stream, new PngEncoder()); + stream.Position = 0; + + using Image decoded = Image.Load(stream); + + Assert.Equal(frameCount, decoded.Frames.Count); + } + [Theory] [WithFile(TestImages.Png.Bad.MissingDataChunk, PixelTypes.Rgba32)] public void Decode_MissingDataChunk_ThrowsException(TestImageProvider provider) From a76c02f7c095df9b578a8c0336712b148abd92d1 Mon Sep 17 00:00:00 2001 From: Andreas <4438107+andreas-eriksson@users.noreply.github.com> Date: Tue, 7 Apr 2026 07:46:22 +0200 Subject: [PATCH 3/4] Replace test image with a smaller one. Adjusted Identify_AnimatedPng_ReadsFrameCountCorrectly to expect 48 frames instead of 50. --- tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs | 2 +- .../Images/Input/Png/animated/issue-animated-frame-count.png | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 0ba8866127..802f2aba39 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -420,7 +420,7 @@ public void Identify_AnimatedPng_ReadsFrameCountCorrectly() ImageInfo imageInfo = Image.Identify(stream); Assert.NotNull(imageInfo); - Assert.Equal(50, imageInfo.FrameMetadataCollection.Count); + Assert.Equal(48, imageInfo.FrameMetadataCollection.Count); } [Theory] diff --git a/tests/Images/Input/Png/animated/issue-animated-frame-count.png b/tests/Images/Input/Png/animated/issue-animated-frame-count.png index db8ff47b9b..88427f4873 100644 --- a/tests/Images/Input/Png/animated/issue-animated-frame-count.png +++ b/tests/Images/Input/Png/animated/issue-animated-frame-count.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:62d51679bcb096ae45ae0f5bf874916ad929014f68ae43b487253d5050c8b68b -size 13561079 +oid sha256:af4e320f586ab26c55612a7ccfc98a8c99cd6a0efe8a70d379503751d06fe8bd +size 51542 From 9569449ac46e844c33c7b4073c8773d9f3d87134 Mon Sep 17 00:00:00 2001 From: Andreas <4438107+andreas-eriksson@users.noreply.github.com> Date: Tue, 7 Apr 2026 08:19:40 +0200 Subject: [PATCH 4/4] Fix MaxFrames handling in PNG decoder - Change >= to > for correct MaxFrames boundary - Skip fdAT chunk data when hitting maxFrames in Identify to maintain stream alignment - Add tests for Identify and Load with MaxFrames --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 12 ++++++---- .../Formats/Png/PngDecoderTests.cs | 24 +++++++++++++++++++ 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 52858ec129..d794c66e27 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -214,7 +214,7 @@ protected override Image Decode(BufferedReadStream stream, Cance break; case PngChunkType.FrameData: { - if (frameCount >= this.maxFrames) + if (frameCount > this.maxFrames) { goto EOF; } @@ -275,7 +275,7 @@ protected override Image Decode(BufferedReadStream stream, Cance previousFrameControl = currentFrameControl; } - if (frameCount >= this.maxFrames) + if (frameCount > this.maxFrames) { goto EOF; } @@ -402,7 +402,7 @@ protected override ImageInfo Identify(BufferedReadStream stream, CancellationTok break; case PngChunkType.FrameControl: ++frameCount; - if (frameCount >= this.maxFrames) + if (frameCount > this.maxFrames) { break; } @@ -411,8 +411,12 @@ protected override ImageInfo Identify(BufferedReadStream stream, CancellationTok break; case PngChunkType.FrameData: - if (frameCount >= this.maxFrames) + if (frameCount > this.maxFrames) { + // Must skip the chunk data even when we've hit maxFrames, because TryReadChunk + // restores the stream position to the start of the fdAT data after CRC validation. + this.SkipChunkDataAndCrc(chunk); + this.SkipRemainingFrameDataChunks(buffer); break; } diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 802f2aba39..4712fc0dd5 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -423,6 +423,30 @@ public void Identify_AnimatedPng_ReadsFrameCountCorrectly() Assert.Equal(48, imageInfo.FrameMetadataCollection.Count); } + [Fact] + public void Identify_AnimatedPngWithMaxFrames_ReadsFrameCountCorrectly() + { + TestFile testFile = TestFile.Create(TestImages.Png.AnimatedFrameCount); + + using MemoryStream stream = new(testFile.Bytes, false); + ImageInfo imageInfo = Image.Identify(new DecoderOptions { MaxFrames = 40 }, stream); + + Assert.NotNull(imageInfo); + Assert.Equal(40, imageInfo.FrameMetadataCollection.Count); + } + + [Fact] + public void Load_AnimatedPngWithMaxFrames_ReadsFrameCountCorrectly() + { + TestFile testFile = TestFile.Create(TestImages.Png.AnimatedFrameCount); + + using MemoryStream stream = new(testFile.Bytes, false); + using Image image = Image.Load(new DecoderOptions { MaxFrames = 40 }, stream); + + Assert.NotNull(image); + Assert.Equal(40, image.Frames.Count); + } + [Theory] [InlineData(1)] [InlineData(2)]