After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 319580 - PNG plug-in ignores iCCP chunk.
PNG plug-in ignores iCCP chunk.
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: Plugins
2.3.x
Other All
: Normal enhancement
: 2.4
Assigned To: GIMP Bugs
GIMP Bugs
Depends on: 78265
Blocks:
 
 
Reported: 2005-10-24 09:08 UTC by Ture Pålsson
Modified: 2008-01-15 14:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Implementation sketch for rudimentary ICC profile support in PNG files (2.04 KB, patch)
2005-10-24 09:52 UTC, Ture Pålsson
accepted-commit_now Details | Review
improved patch (3.55 KB, patch)
2005-10-24 15:04 UTC, Sven Neumann
committed Details | Review

Description Ture Pålsson 2005-10-24 09:08:19 UTC
As far as I can tell, the PNG plug-in ignores the iCCP (ICC profile) chunk. It
would be nice if ICC profiles for PNG files were handled like they are for TIFF
and JPEG files.

For a while, I have been running Gimp with the following patch applied. I
haven't noticed any ill effects, but I'm not really a Gimp hacker so there may
be lots of nasty bugs hiding in there...

--- png.c.~1~   2005-09-20 18:19:36.000000000 +0200
+++ png.c       2005-10-19 14:05:19.000000000 +0200
@@ -683,6 +683,32 @@ load_image (const gchar *filename,
   png_textp  text;
   gint       num_texts;

+#if defined(PNG_iCCP_SUPPORTED)
+  /*
+   * Get the iCCP (colour profile) chunk, if any, and attach it as
+   * a parasite
+   */
+
+  if (1) {
+    png_uint_32 proflen;
+    int profcomp;
+    png_charp profname, profile;
+
+    if (png_get_iCCP(pp, info, &profname, &profcomp, &profile, &proflen)) {
+      GimpParasite *parasite;
+      parasite = gimp_parasite_new("icc-profile", 0, proflen,
+                                   profile);
+      gimp_image_parasite_attach(image, parasite);
+      gimp_parasite_free(parasite);
+      parasite  = gimp_parasite_new("icc-profile-name", 0,
+                                    /* +1: include trailing NUL in length */
+                                    strlen(profname) + 1, profname);
+      gimp_image_parasite_attach(image, parasite);
+      gimp_parasite_free(parasite);
+    }
+  }
+#endif
+
   /*
    * PNG 0.89 and newer have a sane, forwards compatible constructor.
    * Some SGI IRIX users will not have a new enough version though
@@ -1387,6 +1413,28 @@ save_image (const gchar *filename,

 #endif /* PNG_LIBPNG_VER > 99 */

+#if defined(PNG_iCCP_SUPPORTED)
+  if (1) {
+    GimpParasite *profile_parasite;
+    guint32 profile_size;
+    guchar *profile, *profname;
+    profile_parasite = gimp_image_parasite_find(orig_image_ID, "icc-profile");
+    if (profile_parasite) {
+      GimpParasite *profile_name_parasite;
+      profile_name_parasite
+        = gimp_image_parasite_find(orig_image_ID, "icc-profile-name");
+      if (profile_name_parasite) {
+        profname = gimp_parasite_data(profile_name_parasite);
+      } else {
+        profname = "profile";
+      }
+      profile_size = gimp_parasite_data_size(profile_parasite);
+      profile = gimp_parasite_data(profile_parasite);
+      png_set_iCCP(pp, info, profname, 0, profile, profile_size);
+    }
+  }
+#endif
+
   png_write_info (pp, info);

   /*
Comment 1 Michael Schumacher 2005-10-24 09:48:08 UTC
Please attach the diff as a file, so that it can be reviewed more easily.
Comment 2 Ture Pålsson 2005-10-24 09:52:07 UTC
Created attachment 53822 [details] [review]
Implementation sketch for rudimentary ICC profile support in PNG files

As I wrote earlier, this is written by a non-gimp-hacker and probably has all
sorts of bugs...
Comment 3 Michael Natterer 2005-10-24 10:07:01 UTC
Actually, that patch looks fine and should go into HEAD.
Comment 4 Sven Neumann 2005-10-24 14:32:36 UTC
Yes, we definitely need this in order to finish color management support for
GIMP 2.4.
Comment 5 Sven Neumann 2005-10-24 15:03:22 UTC
I had another look at the PNG spec and it isn't all that easy. Especially
handling of the profile name needed some changes. For the reference I will
attach the patch that I am going to commit.
Comment 6 Sven Neumann 2005-10-24 15:04:07 UTC
Created attachment 53837 [details] [review]
improved patch
Comment 7 Sven Neumann 2005-10-24 15:12:46 UTC
2005-10-24  Sven Neumann  <sven@gimp.org>

	* plug-ins/common/png.c: load and save embedded ICC profiles, based
	on a patch from Ture Pålsson (bug #319580).

	* devel-docs/parasites.txt: document the "icc-profile-name" parasite.


Ideally we would also follow the advice in the spec that says:

A PNG encoder that writes the iCCP chunk is encouraged to also write gAMA and
cHRM chunks that approximate the ICC profile, to provide compatibility with
applications that do not use the iCCP chunk. When the iCCP chunk is present, PNG
decoders that recognize it and are capable of colour management [ICC] shall
ignore the gAMA and cHRM chunks and use the iCCP chunk instead and interpret it
according to [ICC-1] and [ICC-1A]. PNG decoders that are used in an environment
that is incapable of full-fledged colour management should use the gAMA and cHRM
chunks if present.

A PNG datastream should contain at most one embedded profile, whether specified
explicitly with an iCCP chunk or implicitly with an sRGB chunk.
Comment 8 weskaggs 2006-05-21 19:13:58 UTC
Is there anything in this bug report that needs to be completed for 2.4?  If not, let's either resolve it as FIXED or bump the target.
Comment 9 Sven Neumann 2006-05-29 16:17:50 UTC
We should perhaps keep it opened until the color management support for 2.4 is completed and check if there is anything in comment #7 that should be implemented.
Comment 10 Sven Neumann 2007-05-05 20:53:11 UTC
I've opened a new report for the remaining issues (bug #436198). Closing this one as FIXED.