File Tk-804.030-libpng16.diff of Package perl-Tk.11483

Move 'sRGB block' before png_read_update_info()

My email to Slaven, ccing Glenn:
================================
Hi Slaven,

when building perl-Tk against libpng16, t/photo.t test is failing
because:

#   Failed test 'No error loading t/test.png as png'
#   at t/photo.t line 45.
#          got: 'invalid after png_start_read_image or
#          png_read_update_info at
#          /home/abuild/rpmbuild/BUILD/Tk-804.030/blib/lib/Tk/Image.pm
#          line 21.
# '
#     expected: ''

This is related to png_rtran_ok() check running at beginning of
libpng-1.6.0/pngrtran.c. Relevant code from perl-Tk is:

-----------------------------8<-----------------------------
    if (png_set_strip_16 != NULL) {
        png_set_strip_16(png_ptr);
    } else if (bit_depth == 16) {
        block.offset[1] = 2;
        block.offset[2] = 4;
    }

    if (png_set_expand != NULL) {
        png_set_expand(png_ptr);
    }

    png_read_update_info(png_ptr,info_ptr);
    block.pixelSize = png_get_channels(png_ptr, info_ptr);
    block.pitch = png_get_rowbytes(png_ptr, info_ptr);

    if ((color_type & PNG_COLOR_MASK_COLOR) == 0) {
        /* grayscale image */
        block.offset[1] = 0;
        block.offset[2] = 0;
    }
    block.width = width;
    block.height = height;

    if ((color_type & PNG_COLOR_MASK_ALPHA)
            || png_get_valid(png_ptr, info_ptr, PNG_INFO_tRNS)) {
        /* with alpha channel */
        block.offset[3] = block.pixelSize - 1;
    } else {
        /* without alpha channel */
        block.offset[3] = 0;
    }

#if defined(PNG_sRGB_SUPPORTED) || defined(PNG_gAMA_SUPPORTED)
#if defined(PNG_sRGB_SUPPORTED) 
    if (png_get_sRGB(png_ptr, info_ptr, &intent)) {
        png_set_sRGB(png_ptr, info_ptr, intent);
    } else {
#endif
#if defined(PNG_gAMA_SUPPORTED)
        double gamma;
        if (!png_get_gAMA(png_ptr, info_ptr, &gamma)) {
            gamma = 0.45455;
        }
        png_set_gamma(png_ptr, 1.0, gamma);
#endif
#if defined(PNG_sRGB_SUPPORTED)
    }
#endif
#endif

-------------------------------->8------------------------

First, png_set_strip_16() and png_set_expand() are called. They
contain the check as well, nevertheless png_read_update_info() is
called after them. But, later on, there is png_set_gamma() is
invoked and it triggers png_app_error() with 'invalid after
png_start_read_image or png_read_update_info'.

CCing Glenn because my knowledge about libpng is tiny.

Thanks!
Petr

Glenn's reply:
==============
Date: Tue, 26 Feb 2013 08:07:23 -0500
From: Glenn Randers-Pehrson <glennrp@gmail.com>
To: pgajdos@suse.cz
Cc: srezic@cpan.org, John Bowler <jbowler@acm.org>
Subject: Re: perl-Tk and libpng16

[-- Autoview using w3m -dump '/tmp/mutt.html' --]
cc'ing John Bowler because he wrote this part of libpng.

I believe the problem is that png_set_gamma() can no longer
(using libpng-1.6.0 and later)
appear after png_read_update_info().  In the past it was
allowed but transformations involving background and
gamma may have been incorrect.

So in your code you need to move the sRGB block
to a position earlier in the code, between png_set_expand()
and png_read_update_info().

Glenn

John's follow up:
=================
That's correct; the png_get calls made *before* png_read_update_info
return the original image information, the png_get calls made *after*
are intended to return information about the transformed image
(intended - there are bugs).  Therefore all transformations which
change the information must be set before png_read_update_info is
called.

The effect of not doing so was undefined before 1.6 - some things
worked, some things didn't.  1.6 should consistently signal the
problem with png_app_error.  You can turn it off with
png_set_benign_errors(png_ptr, 1/*allowed*/), then you will just get a
warning and the old undefined behavior from 1.5 (which may or may not
have been the same as the undefined behavior in 1.4).  If you do this,
however, the behavior may change quite radically in 1.7 - it's better
to fix the code.

Index: PNG/imgPNG.c
===================================================================
--- PNG/imgPNG.c
+++ PNG/imgPNG.c
@@ -490,6 +490,24 @@ static int CommonReadPNG(png_ptr, format
 	png_set_expand(png_ptr);
     }
 
+#if defined(PNG_sRGB_SUPPORTED) || defined(PNG_gAMA_SUPPORTED)
+#if defined(PNG_sRGB_SUPPORTED)
+    if (png_get_sRGB(png_ptr, info_ptr, &intent)) {
+	png_set_sRGB(png_ptr, info_ptr, intent);
+    } else {
+#endif
+#if defined(PNG_gAMA_SUPPORTED)
+	double gamma;
+	if (!png_get_gAMA(png_ptr, info_ptr, &gamma)) {
+	    gamma = 0.45455;
+	}
+	png_set_gamma(png_ptr, 1.0, gamma);
+#endif
+#if defined(PNG_sRGB_SUPPORTED)
+    }
+#endif
+#endif
+
     png_read_update_info(png_ptr,info_ptr);
     block.pixelSize = png_get_channels(png_ptr, info_ptr);
     block.pitch = png_get_rowbytes(png_ptr, info_ptr);
@@ -511,24 +529,6 @@ static int CommonReadPNG(png_ptr, format
 	block.offset[3] = 0;
     }
 
-#if defined(PNG_sRGB_SUPPORTED) || defined(PNG_gAMA_SUPPORTED)
-#if defined(PNG_sRGB_SUPPORTED)
-    if (png_get_sRGB(png_ptr, info_ptr, &intent)) {
-	png_set_sRGB(png_ptr, info_ptr, intent);
-    } else {
-#endif
-#if defined(PNG_gAMA_SUPPORTED)
-	double gamma;
-	if (!png_get_gAMA(png_ptr, info_ptr, &gamma)) {
-	    gamma = 0.45455;
-	}
-	png_set_gamma(png_ptr, 1.0, gamma);
-#endif
-#if defined(PNG_sRGB_SUPPORTED)
-    }
-#endif
-#endif
-
     png_data= (char **) ckalloc(sizeof(char *) * info_height +
 	    info_height * block.pitch);