GNOME Bugzilla – Bug 640409
png-1.5 compatibility fixes
Last modified: 2011-05-15 04:08:43 UTC
Created attachment 179155 [details] Patch fixing the problem. png-1.5 hides structure members from public view. For this reason, some code doesn't compile any more. Found in gimp-2.6.11, but still needed in git master.
Created attachment 179188 [details] [review] Bug fix for previous. Due to a png documentation bug, the filter and interlacing arguments were swapped. Fixed in this version.
Created attachment 179191 [details] [review] 0001-Bug-640409-png-1.5-compatibility-fixes.patch
Cool, thanks, it applies to git master too. I've attached a git-format-patch formated patch with adjusted formating, namely <= 80 char wide lines (apply with git-am). Ideally (thinking loud): * We would have written regression tests for all PNG functionality we support * We would automatically build and make distcheck GIMP with your patch applied as soon as you attached your patch here * We would have a script to check the patch for code formating issues That would make a patch like this effort-less and as good as risk-free to apply with almost no manual work from core developers. Will take a while before we get there though... Two questions: 1. Have you performed manual regression testing yourself or is this untested code so to speak? 2. Does it build with libpng 1.2.37 which is our current minimum required libpng version?
Created attachment 179239 [details] First example file.
Created attachment 179240 [details] Second example file.
I created and loaded two small PNG files; one transparent, one interlaced; they load fine and I can look at them in geeqie (see attachments). There is one new warning on stdout when creating png files: libpng warning: zero length keyword libpng warning: Empty language field in iTXt chunk I don't know yet if it's caused by the patch or because png-1.5 is complaining more (I think the latter). png_get_bit_depth, png_get_color_type, png_get_valid, png_get_image_width, png_get_image_height, png_get_PLTE, png_set_IHDR, and png_jmpbuf are documented in 1.2.44 (oldest I found quickly), so I'm pretty sure it's safe. The only part where I'm not sure is @@ -1072,7 +1080,8 @@ load_image (const gchar *filename, { png_uint_32 proflen; - png_charp profname, profile; + png_charp profname; + png_bytep profile; int profcomp; if (png_get_iCCP (pp, info, &profname, &profcomp, &profile, &proflen)) since the prototype for png_get_iCCP changed between 1.4 and 1.5. Perhaps you prefer #if (PNG_LIBPNG_VER < 10500) png_charp profname, profile; #else png_charp profname; png_bytep profile; #endif.
Here is my plan for getting this in: * Write automatic regression tests for non-trivial changes the code does * Double-check it builds with 1.2.37, and if not, what new minimum version we should require * Sort out the part where you're not sure Any help with this would be appreciated.
According to our policy, we can require 1.2.44 which is in debian testing. Can somebody enlighten me about the various libpng versions? http://www.libpng.org/pub/png/libpng.html seems to say 1.5.0 is current and 1.2.44 is the previous stable version. And 1.4 is beta code? So if I'm not mistaken, anything that works with both 1.2.44 and 1.5 is fine, and we can safely ignore any 1.4 version, right?
Michael: png-1.4 was the successor to png-1.2 until png-1.5 came along. png-1.4 deprecated some symbols and warned about them, while png-1.5 hid the symbols which breaks the build now. There might be some distributions that still use 1.4, but IIRC, gimp didn't need any fixes for that version.
There seems to be a similar issue in file-mng.c: gcc -DHAVE_CONFIG_H -I. -I../.. -I../.. -pthread -DQT_SHARED -I/usr/include/gtk-2.0 -I/usr/lib64/gtk-2.0/include -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/pango-1.0 -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/freetype2 -I/usr/include/libpng15 -I/usr/include/qt4 -I/usr/include/qt4/QtGui -I/usr/include/libdrm -I/usr/include/qt4/QtCore -I/usr/local/include -DGIMP_DISABLE_DEPRECATED -DGDK_MULTIHEAD_SAFE -DGTK_MULTIHEAD_SAFE -g -O2 -Wall -Wdeclaration-after-statement -Wmissing-prototypes -Wmissing-declarations -Winit-self -Wpointer-arith -Wold-style-definition -MT file-mng.o -MD -MP -MF .deps/file-mng.Tpo -c -o file-mng.o file-mng.c file-mng.c: In function 'mng_save_image': file-mng.c:972:11: error: dereferencing pointer to incomplete type file-mng.c:984:19: error: dereferencing pointer to incomplete type file-mng.c:985:19: error: dereferencing pointer to incomplete type file-mng.c:986:19: error: dereferencing pointer to incomplete type file-mng.c:987:19: error: dereferencing pointer to incomplete type file-mng.c:992:23: error: dereferencing pointer to incomplete type file-mng.c:995:23: error: dereferencing pointer to incomplete type file-mng.c:998:23: error: dereferencing pointer to incomplete type file-mng.c:1001:23: error: dereferencing pointer to incomplete type file-mng.c:1004:23: error: dereferencing pointer to incomplete type file-mng.c:1005:23: error: dereferencing pointer to incomplete type file-mng.c:1006:23: error: dereferencing pointer to incomplete type file-mng.c:1008:23: error: dereferencing pointer to incomplete type file-mng.c:1011:23: error: dereferencing pointer to incomplete type file-mng.c:1024:24: error: dereferencing pointer to incomplete type file-mng.c:1026:27: error: dereferencing pointer to incomplete type file-mng.c:1027:25: error: dereferencing pointer to incomplete type file-mng.c:1028:32: error: dereferencing pointer to incomplete type file-mng.c:1029:25: error: dereferencing pointer to incomplete type file-mng.c:1030:32: error: dereferencing pointer to incomplete type file-mng.c:1031:25: error: dereferencing pointer to incomplete type file-mng.c:1041:24: error: dereferencing pointer to incomplete type file-mng.c:1042:24: error: dereferencing pointer to incomplete type file-mng.c:1068:32: error: dereferencing pointer to incomplete type file-mng.c:1080:35: error: dereferencing pointer to incomplete type file-mng.c:1234:17: warning: ignoring return value of 'fread', declared with attribute warn_unused_result make: *** [file-mng.o] Error 1
I have mng-1.0.9 installed and don't see any errors or warnings compiling the MNG plug-in. Hanno, what version of MNG library is installed on your machine?
Created attachment 180967 [details] [review] Patch fixing the mng problem. (In reply to comment #10) > There seems to be a similar issue in file-mng.c: Sorry. The first patch included a fix for that, but in the second version I forgot that part. I'll attach it separately.
We desperately need to get 2.8 out, let's look at this for 2.10 instead
Um, this is a trivial patch, and a true bugfix... can't somebody on the CC list simply look at it, test it, and give their thumbs up?
Created attachment 185548 [details] [review] Libpng 1.4 compatibility patch 2.7 still needs this patch, first applied here: https://bugzilla.gnome.org/show_bug.cgi?id=607242
There's a catch about patch from comment 2 - see http://bugs.gentoo.org/show_bug.cgi?id=363493. Code needs far more restructuring than this patch does, as till png header is written, stuff like png_{s,g}et_PLTE simply won't work. That makes GIMP_INDEXED_IMAGE and GIMP_INDEXEDA_IMAGE into non-working cases (as respin_cmap accesses those structure too).
Created attachment 186265 [details] [review] Patch fixing the issues reported by Rafał Mużyło. This patch fixes the problem reported by Rafał Mużyło. The file-png part was tested by the Gentoo bug reporter (see Gentoo's bugtracking); the file-mng part is analog.
De-deprecation of libpng API usage was committed in a series of patches in the master tree. Resolving this bug as FIXED, milestone 2.8.
Btw, I do want to add an appreciation to Thomas Klausner here. I wish I had seen this patch before. It would have avoided duplication of work. The deprecation warnings were noticed as a part of the buildbot reported warnings and were all fixed recently.
*** Bug 650192 has been marked as a duplicate of this bug. ***