From 945f2602a0ea05a7938b2f52df9ef16cf3440291 Mon Sep 17 00:00:00 2001 From: John Bowler Date: Fri, 3 Jan 2025 10:35:19 -0800 Subject: [PATCH] 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 Signed-off-by: John Bowler Signed-off-by: Cosmin Truta --- .../png-3/cicp-display-p3_reencoded.png | Bin 142 -> 142 bytes pngtest.png | Bin 8759 -> 8759 bytes pngwrite.c | 95 +++++++++++------- 3 files changed, 58 insertions(+), 37 deletions(-) diff --git a/contrib/testpngs/png-3/cicp-display-p3_reencoded.png b/contrib/testpngs/png-3/cicp-display-p3_reencoded.png index 47830bd6b358903bcb42d251a7efa8d727d30001..91d8e6bc4819b50bdc57025b8ee826620c3facba 100644 GIT binary patch delta 26 hcmeBU>|>mu!poBE=^Vhr%fOh&{P_JuMd^t?P5@iM2W$WU delta 15 WcmeBU>|>muGSNh0qKd#oA144Mkpz?g diff --git a/pngtest.png b/pngtest.png index 66df0c4e6f6d13981148bf2fbb85df2b797028a7..cf620eb32637a6e8b36fc60e83cd54c96c765ee1 100644 GIT binary patch delta 39 tcmdn)vfX8Zs${W~X9z10_%7A`#=yY9SRCZ;#CY?hPW?nh;f;aM6an*B466VD delta 39 vcmdn)vfX8Zs${yOuOkD)#(wTUiL49^42;D=?oNz1PwLc9R21GA_)HN1080%M diff --git a/pngwrite.c b/pngwrite.c index 68815cacba..a03380b9fc 100644 --- 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,20 +190,24 @@ 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) @@ -179,10 +215,6 @@ png_write_info_before_PLTE(png_structrp png_ptr, png_const_inforp info_ptr) # 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) {