Browse Source

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.
pull/3101/head
Andreas Eriksson 2 months ago
parent
commit
ac1905328d
  1. 30
      src/ImageSharp/Formats/Png/PngDecoderCore.cs
  2. 12
      tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs
  3. 1
      tests/ImageSharp.Tests/TestImages.cs
  4. 3
      tests/Images/Input/Png/animated/issue-animated-frame-count.png

30
src/ImageSharp/Formats/Png/PngDecoderCore.cs

@ -428,9 +428,10 @@ internal sealed class PngDecoderCore : ImageDecoderCore
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 @@ internal sealed class PngDecoderCore : ImageDecoderCore
return 0;
}
/// <summary>
/// Skips any remaining <see cref="PngChunkType.FrameData"/> chunks belonging to the current frame.
/// This mirrors how <see cref="ReadNextFrameDataChunk"/> is used during decoding:
/// consecutive fdAT chunks are consumed until a non-fdAT chunk is encountered,
/// which is stored in <see cref="nextChunk"/> for the next iteration.
/// </summary>
/// <param name="buffer">Temporary buffer.</param>
private void SkipRemainingFrameDataChunks(Span<byte> 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;
}
}
}
/// <summary>
/// Reads a chunk from the stream.
/// </summary>

12
tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs

@ -411,6 +411,18 @@ public partial class PngDecoderTests
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<TPixel>(TestImageProvider<TPixel> provider)

1
tests/ImageSharp.Tests/TestImages.cs

@ -76,6 +76,7 @@ public static class TestImages
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";

3
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
Loading…
Cancel
Save