From 564c3d122c8e5c82e01963f2ee58d640feafb958 Mon Sep 17 00:00:00 2001
From: James Jackson-South <james_south@hotmail.com>
Date: Tue, 17 Oct 2023 22:55:07 +1000
Subject: [PATCH] Fix encoding

---
 src/ImageSharp/Formats/Png/PngEncoderCore.cs      | 15 ++++++++++-----
 .../Formats/Png/PngDecoderTests.cs                |  7 ++++++-
 .../Formats/Png/PngEncoderTests.cs                |  6 ++++++
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/src/ImageSharp/Formats/Png/PngEncoderCore.cs b/src/ImageSharp/Formats/Png/PngEncoderCore.cs
index 6c86d1b106..bbf1a64534 100644
--- a/src/ImageSharp/Formats/Png/PngEncoderCore.cs
+++ b/src/ImageSharp/Formats/Png/PngEncoderCore.cs
@@ -189,6 +189,7 @@ internal sealed class PngEncoderCore : IImageEncoderInternals, IDisposable
             ReadOnlyMemory<TPixel>? previousPalette = quantized?.Palette.ToArray();
 
             // Write following frames.
+            uint increment = 0;
             for (int i = 1; i < image.Frames.Count; i++)
             {
                 currentFrame = image.Frames[i];
@@ -200,12 +201,14 @@ internal sealed class PngEncoderCore : IImageEncoderInternals, IDisposable
                     ClearTransparentPixels(currentFrame);
                 }
 
-                frameControl = this.WriteFrameControlChunk(stream, currentFrame, (uint)i);
+                // Each frame control sequence number must be incremented by the
+                // number of frame data chunks that follow.
+                frameControl = this.WriteFrameControlChunk(stream, currentFrame, (uint)i + increment);
 
                 // Dispose of previous quantized frame and reassign.
                 quantized?.Dispose();
                 quantized = this.CreateQuantizedImageAndUpdateBitDepth(pngMetadata, currentFrame, previousPalette);
-                this.WriteDataChunks(frameControl, currentFrame, quantized, stream, true);
+                increment += this.WriteDataChunks(frameControl, currentFrame, quantized, stream, true);
             }
         }
         else
@@ -1013,7 +1016,7 @@ internal sealed class PngEncoderCore : IImageEncoderInternals, IDisposable
     /// <param name="quantized">The quantized pixel data. Can be null.</param>
     /// <param name="stream">The stream.</param>
     /// <param name="isFrame">Is writing fdAT or IDAT.</param>
-    private int WriteDataChunks<TPixel>(FrameControl frameControl, ImageFrame<TPixel> pixels, IndexedImageFrame<TPixel>? quantized, Stream stream, bool isFrame)
+    private uint WriteDataChunks<TPixel>(FrameControl frameControl, ImageFrame<TPixel> pixels, IndexedImageFrame<TPixel>? quantized, Stream stream, bool isFrame)
         where TPixel : unmanaged, IPixel<TPixel>
     {
         byte[] buffer;
@@ -1070,7 +1073,9 @@ internal sealed class PngEncoderCore : IImageEncoderInternals, IDisposable
 
             if (isFrame)
             {
-                uint sequenceNumber = (uint)(frameControl.SequenceNumber + i);
+                // We increment the sequence number for each frame chunk.
+                // '1' is added to the sequence number to account for the preceding frame control chunk.
+                uint sequenceNumber = (uint)(frameControl.SequenceNumber + 1 + i);
                 this.WriteFrameDataChunk(stream, sequenceNumber, buffer, i * maxBlockSize, length);
             }
             else
@@ -1079,7 +1084,7 @@ internal sealed class PngEncoderCore : IImageEncoderInternals, IDisposable
             }
         }
 
-        return numChunks;
+        return (uint)numChunks;
     }
 
     /// <summary>
diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs
index 57d0619b99..9f11bf6507 100644
--- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs
+++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs
@@ -111,7 +111,12 @@ public partial class PngDecoderTests
     public void Decode_APng<TPixel>(TestImageProvider<TPixel> provider)
         where TPixel : unmanaged, IPixel<TPixel>
     {
-        using Image<TPixel> image = provider.GetImage(PngDecoder.Instance); // MagickReferenceDecoder cannot decode APNGs
+        using Image<TPixel> image = provider.GetImage(PngDecoder.Instance);
+
+        Assert.Equal(5, image.Frames.Count);
+
+        // TODO: Assertations.
+        // MagickReferenceDecoder cannot decode APNGs (Though ImageMagick can, we likely need to update our mapping implementation)
     }
 
     [Theory]
diff --git a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs
index 2c37dc4713..f6dfcd178f 100644
--- a/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs
+++ b/tests/ImageSharp.Tests/Formats/Png/PngEncoderTests.cs
@@ -451,8 +451,14 @@ public partial class PngEncoderTests
         using MemoryStream memStream = new();
         image.Save(memStream, PngEncoder);
         memStream.Position = 0;
+
+        image.DebugSave(provider: provider, encoder: PngEncoder, null, false);
+
         using Image<Rgba32> output = Image.Load<Rgba32>(memStream);
         ImageComparer.Exact.VerifySimilarity(output, image);
+
+        // TODO: Additional assertations regarding metadata.
+        Assert.Equal(5, image.Frames.Count);
     }
 
     [Theory]