fix: Write order of colourspace chunks should conform to PNG v3

cICP was written after PLTE, not before. The other chunks were output in an order which does not match the new PNG-v3 "priority" order. This change outputs all chunks in the "priority" order; highest precedence first. This means that the PNGs so written conform to PNG v3 (cICP), and allow a streaming app to handle chunks in order, without buffering data which may later be overridden. Note that PNG-v3 establishes the idea of dropping ancillary chunks which are inconveniently ordered in the definition of how APNG chunks are handled. Reviewed-by: Cosmin Truta <[email protected]> Signed-off-by: John Bowler <[email protected]> Signed-off-by: Cosmin Truta <[email protected]>
pnggroup · Jan 3, 2025 · 945f260 · 945f260
1 parent 27c2ac7
commit 945f260
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 37 deletions.
diff --git a/contrib/testpngs/png-3/cicp-display-p3_reencoded.png b/contrib/testpngs/png-3/cicp-display-p3_reencoded.png
diff --git a/pngtest.png b/pngtest.png
diff --git a/pngwrite.c b/pngwrite.c
@@ -1,6 +1,6 @@
 /* pngwrite.c - general routines to write a PNG file
  *
- * Copyright (c) 2018-2024 Cosmin Truta
+ * Copyright (c) 2018-2025 Cosmin Truta
  * Copyright (c) 1998-2002,2004,2006-2018 Glenn Randers-Pehrson
  * Copyright (c) 1996-1997 Andreas Dilger
  * Copyright (c) 1995-1996 Guy Eric Schalnat, Group 42, Inc.
@@ -127,29 +127,61 @@ png_write_info_before_PLTE(png_structrp png_ptr, png_const_inforp info_ptr)
        * the application continues writing the PNG.  So check the 'invalid'
        * flag here too.
        */
-#ifdef PNG_GAMMA_SUPPORTED
-#  ifdef PNG_WRITE_gAMA_SUPPORTED
-      if ((info_ptr->colorspace.flags & PNG_COLORSPACE_INVALID) == 0 &&
-          (info_ptr->colorspace.flags & PNG_COLORSPACE_FROM_gAMA) != 0 &&
-          (info_ptr->valid & PNG_INFO_gAMA) != 0)
-         png_write_gAMA_fixed(png_ptr, info_ptr->colorspace.gamma);
-#  endif
+#ifdef PNG_WRITE_UNKNOWN_CHUNKS_SUPPORTED
+         /* Write unknown chunks first; PNG v3 establishes a precedence order
+          * for colourspace chunks.  It is certain therefore that new
+          * colourspace chunks will have a precedence and very likely it will be
+          * higher than all known so far.  Writing the unknown chunks here is
+          * most likely to present the chunks in the most convenient order.
+          *
+          * FUTURE: maybe write chunks in the order the app calls png_set_chnk
+          * to give the app control.
+          */
+         write_unknown_chunks(png_ptr, info_ptr, PNG_HAVE_IHDR);
+#endif
+
+#ifdef PNG_WRITE_sBIT_SUPPORTED
+         /* PNG v3: a streaming app will need to see this before cICP because
+          * the information is helpful in handling HLG encoding (which is
+          * natively 10 bits but gets expanded to 16 in PNG.)
+          *
+          * The app shouldn't care about the order ideally, but it might have
+          * no choice.  In PNG v3, apps are allowed to reject PNGs where the
+          * APNG chunks are out of order so it behooves libpng to be nice here.
+          */
+         if ((info_ptr->valid & PNG_INFO_sBIT) != 0)
+            png_write_sBIT(png_ptr, &(info_ptr->sig_bit), info_ptr->color_type);
 #endif
 
+   /* PNG v3: the July 2004 version of the TR introduced the concept of colour
+    * space priority.  As above it therefore behooves libpng to write the colour
+    * space chunks in the priority order so that a streaming app need not buffer
+    * them.
+    */
 #ifdef PNG_COLORSPACE_SUPPORTED
-      /* Write only one of sRGB or an ICC profile.  If a profile was supplied
-       * and it matches one of the known sRGB ones issue a warning.
+#  ifdef PNG_WRITE_cICP_SUPPORTED /* Priority 4 */
+   if ((info_ptr->valid & PNG_INFO_cICP) != 0)
+      {
+         png_write_cICP(png_ptr,
+                        info_ptr->cicp_colour_primaries,
+                        info_ptr->cicp_transfer_function,
+                        info_ptr->cicp_matrix_coefficients,
+                        info_ptr->cicp_video_full_range_flag);
+      }
+#  endif
+
+      /* PNG v3 change: it is now permitted to write both sRGB and ICC profiles,
+       * however because the libpng code auto-generates an sRGB for the
+       * corresponding ICC profiles and because PNG v2 disallowed this we need
+       * to only write one.
+       *
+       * Remove the PNG v2 warning about writing an sRGB ICC profile as well
+       * because it's invalid with PNG v3.
        */
-#  ifdef PNG_WRITE_iCCP_SUPPORTED
+#  ifdef PNG_WRITE_iCCP_SUPPORTED /* Priority 3 */
          if ((info_ptr->colorspace.flags & PNG_COLORSPACE_INVALID) == 0 &&
              (info_ptr->valid & PNG_INFO_iCCP) != 0)
          {
-#    ifdef PNG_WRITE_sRGB_SUPPORTED
-               if ((info_ptr->valid & PNG_INFO_sRGB) != 0)
-                  png_app_warning(png_ptr,
-                      "profile matches sRGB but writing iCCP instead");
-#     endif
-
             png_write_iCCP(png_ptr, info_ptr->iccp_name,
                 info_ptr->iccp_profile);
          }
@@ -158,31 +190,31 @@ png_write_info_before_PLTE(png_structrp png_ptr, png_const_inforp info_ptr)
 #     endif
 #  endif
 
-#  ifdef PNG_WRITE_sRGB_SUPPORTED
+#  ifdef PNG_WRITE_sRGB_SUPPORTED /* Priority 2 */
          if ((info_ptr->colorspace.flags & PNG_COLORSPACE_INVALID) == 0 &&
              (info_ptr->valid & PNG_INFO_sRGB) != 0)
             png_write_sRGB(png_ptr, info_ptr->colorspace.rendering_intent);
 #  endif /* WRITE_sRGB */
 #endif /* COLORSPACE */
 
-#ifdef PNG_WRITE_sBIT_SUPPORTED
-         if ((info_ptr->valid & PNG_INFO_sBIT) != 0)
-            png_write_sBIT(png_ptr, &(info_ptr->sig_bit), info_ptr->color_type);
+#ifdef PNG_GAMMA_SUPPORTED
+#  ifdef PNG_WRITE_gAMA_SUPPORTED /* Priority 1 */
+      if ((info_ptr->colorspace.flags & PNG_COLORSPACE_INVALID) == 0 &&
+          (info_ptr->colorspace.flags & PNG_COLORSPACE_FROM_gAMA) != 0 &&
+          (info_ptr->valid & PNG_INFO_gAMA) != 0)
+         png_write_gAMA_fixed(png_ptr, info_ptr->colorspace.gamma);
+#  endif
 #endif
 
 #ifdef PNG_COLORSPACE_SUPPORTED
-#  ifdef PNG_WRITE_cHRM_SUPPORTED
+#  ifdef PNG_WRITE_cHRM_SUPPORTED /* Also priority 1 */
          if ((info_ptr->colorspace.flags & PNG_COLORSPACE_INVALID) == 0 &&
              (info_ptr->colorspace.flags & PNG_COLORSPACE_FROM_cHRM) != 0 &&
              (info_ptr->valid & PNG_INFO_cHRM) != 0)
             png_write_cHRM_fixed(png_ptr, &info_ptr->colorspace.end_points_xy);
 #  endif
 #endif
 
-#ifdef PNG_WRITE_UNKNOWN_CHUNKS_SUPPORTED
-         write_unknown_chunks(png_ptr, info_ptr, PNG_HAVE_IHDR);
-#endif
-
       png_ptr->mode |= PNG_WROTE_INFO_BEFORE_PLTE;
    }
 }
@@ -236,17 +268,6 @@ png_write_info(png_structrp png_ptr, png_const_inforp info_ptr)
       png_write_bKGD(png_ptr, &(info_ptr->background), info_ptr->color_type);
 #endif
 
-#ifdef PNG_WRITE_cICP_SUPPORTED
-   if ((info_ptr->valid & PNG_INFO_cICP) != 0)
-   {
-      png_write_cICP(png_ptr,
-                     info_ptr->cicp_colour_primaries,
-                     info_ptr->cicp_transfer_function,
-                     info_ptr->cicp_matrix_coefficients,
-                     info_ptr->cicp_video_full_range_flag);
-   }
-#endif
-
 #ifdef PNG_WRITE_eXIf_SUPPORTED
    if ((info_ptr->valid & PNG_INFO_eXIf) != 0)
    {