Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ICC not embedded when writing PNG files #4262

Closed
Beep6581 opened this issue Dec 27, 2017 · 7 comments
Closed

ICC not embedded when writing PNG files #4262

Beep6581 opened this issue Dec 27, 2017 · 7 comments
Assignees
Labels
scope: color management Related to handling of colour processing scope: metadata Reading / writing metadata type: bug Something is not doing what it's supposed to be doing

Comments

@Beep6581
Copy link
Owner

Beep6581 commented Dec 27, 2017

RawTherapee does not embed the ICC profile when writing PNG files!

Output profile: ACES
imgur-2017_12_27-17 58 47

If I insert the ACES profile into the PNG using ExifTool then it displays correctly when using @agriggio's patch from #4260:

exiftool -ICC_Profile'<='$HOME/programs/code-rawtherapee/rtdata/iccprofiles/output/ACES.icc /tmp/colorspace_flowers\ aces.png

imgur-2017_12_27-18 06 38

This is the full metadata output of PNG files produced by RawTherapee:

-ExifTool:ExifToolVersion=10.64
-System:FileName=colorspace_flowers rt_large_g10.png
-System:Directory=/tmp
-System:FileSize=2.7 MB
-System:FileModifyDate=2017:12:27 17:56:46+01:00
-System:FileAccessDate=2017:12:27 17:57:56+01:00
-System:FileInodeChangeDate=2017:12:27 17:56:46+01:00
-System:FilePermissions=rw-r--r--
-PNG:FileType=PNG
-PNG:FileTypeExtension=png
-PNG:MIMEType=image/png
-PNG-ImageHeader:ImageWidth=900
-PNG-ImageHeader:ImageHeight=604
-PNG-ImageHeader:BitDepth=16
-PNG-ImageHeader:ColorType=RGB
-PNG-ImageHeader:Compression=Deflate/Inflate
-PNG-ImageHeader:Filter=Adaptive
-PNG-ImageHeader:Interlace=Noninterlaced
-Composite:ImageSize=900x604
-Composite:Megapixels=0.544
@Beep6581 Beep6581 added type: bug Something is not doing what it's supposed to be doing scope: color management Related to handling of colour processing Component-Format scope: metadata Reading / writing metadata labels Dec 27, 2017
@agriggio agriggio self-assigned this Dec 27, 2017
@agriggio
Copy link
Contributor

Please ack that this works also for libpng 1.6.x:

diff --git a/rtengine/imageio.cc b/rtengine/imageio.cc
--- a/rtengine/imageio.cc
+++ b/rtengine/imageio.cc
@@ -365,10 +365,10 @@
     if (png_get_valid(png, info, PNG_INFO_iCCP)) {
         png_charp name;
         int compression_type;
-#if PNG_LIBPNG_VER_MAJOR > 1 || (PNG_LIBPNG_VER_MAJOR == 1 && PNG_LIBPNG_VER_MINOR > 4)
+#if PNG_LIBPNG_VER < 10500
+        png_charp profdata;
+#else
         png_bytep profdata;
-#else
-        png_charp profdata;
 #endif
         png_uint_32 proflen;
         png_get_iCCP(png, info, &name, &compression_type, &profdata, &proflen);
@@ -950,6 +950,11 @@
         return IMIO_HEADERERROR;
     }
 
+    // silence the warning about "invalid" sRGB profiles -- see #4260
+#if defined(PNG_SKIP_sRGB_CHECK_PROFILE) && defined(PNG_SET_OPTION_SUPPORTED)
+    png_set_option(png, PNG_SKIP_sRGB_CHECK_PROFILE, PNG_OPTION_ON);
+#endif
+    
     png_infop info = png_create_info_struct(png);
 
     if (!info) {
@@ -980,6 +985,15 @@
     png_set_IHDR(png, info, width, height, bps, PNG_COLOR_TYPE_RGB,
                  PNG_INTERLACE_NONE, PNG_COMPRESSION_TYPE_DEFAULT, PNG_FILTER_TYPE_BASE);
 
+    if (profileData) {
+#if PNG_LIBPNG_VER < 10500
+        png_charp profdata = static_cast<png_charp>(profileData);
+#else
+        png_bytep profdata = static_cast<png_bytep>(profileData);
+#endif
+        png_set_iCCP(png, info, const_cast<png_charp>("icc"), 0, profdata, profileLength);
+    }
+
 
     int rowlen = width * 3 * bps / 8;
     unsigned char *row = new unsigned char [rowlen];

@Beep6581
Copy link
Owner Author

@agriggio awesome to see a patch!
Compilation fails:

/home/morgan/programs/code-rawtherapee/rtengine/imageio.cc: In member function ‘int rtengine::ImageIO::savePNG(Glib::ustring, int)’:
/home/morgan/programs/code-rawtherapee/rtengine/imageio.cc:992:64: error: invalid static_cast from type ‘char*’ to type ‘png_bytep {aka unsigned char*}’
         png_bytep profdata = static_cast<png_bytep>(profileData);
                                                                ^

@agriggio
Copy link
Contributor

@Beep6581 replace with const_cast and it should work.
incidentally, this is one reason why c-style casts aren't always a mortal sin :-)

@Beep6581
Copy link
Owner Author

@agriggio

/home/morgan/programs/code-rawtherapee/rtengine/imageio.cc: In member function ‘int rtengine::ImageIO::savePNG(Glib::ustring, int)’:
/home/morgan/programs/code-rawtherapee/rtengine/imageio.cc:992:63: error: invalid const_cast from type ‘char*’ to type ‘png_bytep {aka unsigned char*}’
         png_bytep profdata = const_cast<png_bytep>(profileData);
                                                               ^

@agriggio
Copy link
Contributor

I should read better the error messages before replying :-)
a reinterpret_cast should do, or a c-style also, i.e. (png_bytep)profileData

(I am on my phone now so I can't test)

@Beep6581
Copy link
Owner Author

Compiled!

@Beep6581
Copy link
Owner Author

Tested, no issues found. Thank you @agriggio , +1 for commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: color management Related to handling of colour processing scope: metadata Reading / writing metadata type: bug Something is not doing what it's supposed to be doing
Projects
None yet
Development

No branches or pull requests

3 participants