Keep the ICC profile of a stripped Imagick image in PNG output

[nlemoine]

StripMetaModifier reads the icc profile, calls Imagick::stripImage(), then puts the profile back. That works for JPEG, WebP and the other formats that reach the modifier. Not for PNG: the profile is on the wand afterwards, but the encoded PNG carries none.

stripImage() sets a png:exclude-chunk artifact and that artifact stays on the wand, so it also hits any later PNG encode of the same image:

$image = $manager->decodePath('test.jpg');
$image->encode(new JpegEncoder(strip: true));   // icc kept
$image->encode(new PngEncoder());               // icc gone

Encoding the PNG first keeps the profile, so the result depends on the order. Same with strip: true in the config.

The cause lives in coders/png.c the loop that writes profiles is guarded by

if ((mng_info->exclude_tEXt == MagickFalse ||
     mng_info->exclude_zTXt == MagickFalse) &&
   (ping_exclude_iCCP == MagickFalse || ping_exclude_zCCP == MagickFalse))

The list stripImage() sets excludes both tEXt and zTXt, so no profile is written at all. Removing only iCCP from that list does not help, I checked, and png:include-chunk=iCCP does not either. The list also excludes pHYs, so a resolution set after the strip is lost in the PNG too.

Fix: delete the artifact and clear the property cache instead. The sweep is needed because the JPEG reader materialises exif:* into the property cache and stripImage() leaves them there, so once the artifact is gone the PNG writer would serialise them back into text chunks. png: and jpeg: properties are kept, the writer skips those prefixes anyway.

One caveat on the sweep, and the reason I am flagging it rather than deciding it myself.

Imagick::getImageProperties() does not report properties whose name starts with [, and a PNG tEXt keyword can produce one (0x5B is a legal keyword byte). The PNG writer has no such restriction, so a sweep based only on getImageProperties() lets those through, which is worse than today where the artifact suppresses every text chunk. I complete the list with the names from identifyFormat('%[*]'), which walks the same property iterator as the writer.

That call builds a string of every property with its value. On normal images it is nothing, on a crafted PNG it is not:

input	strip cost	php peak
exif.jpg	0.1 ms	27 KB
crafted 125 KB PNG, 2000 zTXt chunks x 20 KB	1.7 ms	46 KB
same file, keywords prefixed with [	620 ms	81 MB

The sweep runs %[*] last, once everything it can enumerate is already deleted, which is why the middle row is cheap. The last row I could not avoid: to delete those properties I have to read them, and reading them is the cost. Timings grow faster than the payload, my guess is the repeated realloc in AppendKeyValue2Text. For reference that same file takes about 100 ms to decode, and 72 ms to decode and re-encode as PNG without stripping.

I tried identifyImage(true) (9.5 s on a 4000x3000 image), identifyImage(false) (no properties), %[property:*] (returns nothing) and a glob restricted to [ names to keep the output small (could not express one, the %[...] parser counts bracket depth). None of them work.

So if the last row is not acceptable, the alternative is to drop the identifyFormat('%[*]') part and accept that [ prefixed text chunks survive a strip in PNG output. Happy to do that, just tell me which you prefer. Both underlying problems look like ImageMagick ones anyway, the profile loop guarded by the text chunk flags and getImageProperties() hiding names the writer will write. Not reported upstream yet.

Side effect: a stripped PNG now carries the cHRM, bKGD and pHYs chunks the artifact used to suppress, so it is slightly larger. test.jpg stripped and encoded as PNG goes from 2356 to 3034 bytes, 536 of which are the icc profile that was missing.

[olivervogel]

Thanks.

[nlemoine]

Thanks @olivervogel