Repository: shssoichiro/oxipng (language: rust)

Issue:
after compress the apng file, YYimage decode can not get the frames count of the apng file.
when using this tool on an APNG file, a problem was discovered.

before compressing, the YYImage decoder can decode it perfectly as expected, after compressed, the YYImage decoder can not get the frame count of the file and can not animate the apng image.

I tried to use ffmpeg to compare two files:

**origin file**
```
  libavutil      56. 70.100 / 56. 70.100
  libavcodec     58.134.100 / 58.134.100
  libavformat    58. 76.100 / 58. 76.100
  libavdevice    58. 13.100 / 58. 13.100
  libavfilter     7.110.100 /  7.110.100
  libavresample   4.  0.  0 /  4.  0.  0
  libswscale      5.  9.100 /  5.  9.100
  libswresample   3.  9.100 /  3.  9.100
  libpostproc    55.  9.100 / 55.  9.100
Input #0, apng, from 'gt_apng_before.png':
  Duration: N/A, bitrate: N/A
  Stream #0:0: Video: apng, pal8(pc), 60x60, 10.42 fps, 24 tbr, 100k tbn, 100k tbc
```

**compressed file**
```
  libavutil      56. 70.100 / 56. 70.100
  libavcodec     58.134.100 / 58.134.100
  libavformat    58. 76.100 / 58. 76.100
  libavdevice    58. 13.100 / 58. 13.100
  libavfilter     7.110.100 /  7.110.100
  libavresample   4.  0.  0 /  4.  0.  0
  libswscale      5.  9.100 /  5.  9.100
  libswresample   3.  9.100 /  3.  9.100
  libpostproc    55.  9.100 / 55.  9.100
[apng @ 0x7fb323808200] Could not find codec parameters for stream 0 (Video: apng, none(pc), 60x60): unspecified pixel format
Consider increasing the value for the 'analyzeduration' (0) and 'probesize' (5000000) options
Input #0, apng, from 'gt_apng_after.png':
  Duration: N/A, bitrate: N/A
  Stream #0:0: Video: apng, none(pc), 60x60, 100k tbr, 100k tbn, 100k tbc
At least one output file must be specified
```

the "pal8(pc)" is gone and ffmpeg can't decode it too.


And on the Mac, the two files can preview normally.




Discussion / hints:
As a test, does running [`apngopt`](https://github.com/jindongyi011039/apngopt) against your original APNG, & then running the output through the above give the same result?
Looks like some chunks are in a slightly different order.
Before:
```
  chunk IHDR at offset 0x0000c, length 13
    320 x 320 image, 8-bit palette, non-interlaced
  chunk acTL at offset 0x00025, length 8
    unknown private, ancillary, unsafe-to-copy chunk
  chunk PLTE at offset 0x00039, length 762: 254 palette entries
  chunk tRNS at offset 0x0033f, length 1: 1 transparency entry
  chunk fcTL at offset 0x0034c, length 26
    unknown private, ancillary, unsafe-to-copy chunk
  chunk IDAT at offset 0x00372, length 12143
    zlib: deflated, 32K window, maximum compression
  ...
```
After:
```
  chunk IHDR at offset 0x0000c, length 13
    320 x 320 image, 8-bit palette, non-interlaced
  chunk acTL at offset 0x00025, length 8
    unknown private, ancillary, unsafe-to-copy chunk
  chunk fcTL at offset 0x00039, length 26
    unknown private, ancillary, unsafe-to-copy chunk
  chunk PLTE at offset 0x0005f, length 762: 254 palette entries
  chunk tRNS at offset 0x00365, length 1: 1 transparency entry
  chunk IDAT at offset 0x00372, length 11577
    zlib: deflated, 32K window, maximum compression
  ...
``` 
The apng spec makes no mention of required order of fcTL relative to PLTE or tRNS, so it's a bit unexpected that this would cause some programs to fail. I'm sure we can get a fix out though.

Fix patch:
diff --git a/src/png/mod.rs b/src/png/mod.rs
index a2135c47..df695973 100644
--- a/src/png/mod.rs
+++ b/src/png/mod.rs
@@ -186,9 +186,12 @@ impl PngData {
         // Ancillary chunks - split into those that come before IDAT and those that come after
         let mut aux_split = self.aux_chunks.split(|c| &c.name == b"IDAT");
         let aux_pre = aux_split.next().unwrap();
+        // Many chunks need to be before PLTE, so write all except those that explicitly need to be after
+        // Note: the PNG spec does not say that fcTL needs to be after PLTE, but some decoders expect
+        //       that (see issue #625)
         for chunk in aux_pre
             .iter()
-            .filter(|c| !(&c.name == b"bKGD" || &c.name == b"hIST" || &c.name == b"tRNS"))
+            .filter(|c| !matches!(&c.name, b"bKGD" | b"hIST" | b"tRNS" | b"fcTL"))
         {
             write_png_block(&chunk.name, &chunk.data, &mut output);
         }
@@ -223,7 +226,7 @@ impl PngData {
         // Special ancillary chunks that need to come after PLTE but before IDAT
         for chunk in aux_pre
             .iter()
-            .filter(|c| &c.name == b"bKGD" || &c.name == b"hIST" || &c.name == b"tRNS")
+            .filter(|c| matches!(&c.name, b"bKGD" | b"hIST" | b"tRNS" | b"fcTL"))
         {
             write_png_block(&chunk.name, &chunk.data, &mut output);
         }


Test patch: