From b2bee3374c6f22e9052b104fa1636584a4e27c53 Mon Sep 17 00:00:00 2001 From: John Bowler Date: Fri, 10 Jun 2011 23:24:58 -0500 Subject: [PATCH] [devel] Make the 16-to-8 scaling accurate. Dividing by 256 with no rounding is wrong (high by one) 25% of the time. Dividing by 257 with rounding is wrong in 128 out of 65536 cases. Getting the right answer all the time without division is easy. --- ANNOUNCE | 4 ++ CHANGES | 4 ++ libpng-manual.txt | 6 +++ libpng.3 | 6 +++ png.h | 4 +- pngrtran.c | 85 ++++++++++++++++++----------------- pngstruct.h | 2 +- pngtest.c | 2 +- pngwrite.c | 4 +- pngwutil.c | 4 +- scripts/pnglibconf.dfa | 11 +++-- scripts/pnglibconf.h.prebuilt | 4 +- 12 files changed, 81 insertions(+), 55 deletions(-) diff --git a/ANNOUNCE b/ANNOUNCE index 99aa75588..c914c53d6 100644 --- a/ANNOUNCE +++ b/ANNOUNCE @@ -159,6 +159,10 @@ Version 1.5.3beta11 [June 11, 2011] Revised documentation about png_set_user_limits() to say that it can only be used to reduce the defined limit, and that it also affects png writing. + Make the 16-to-8 scaling accurate. Dividing by 256 with no rounding is + wrong (high by one) 25% of the time. Dividing by 257 with rounding is + wrong in 128 out of 65536 cases. Getting the right answer all the time + without division is easy. Send comments/corrections/commendations to png-mng-implement at lists.sf.net: (subscription required; visit diff --git a/CHANGES b/CHANGES index 5d09122bc..96a35a21b 100644 --- a/CHANGES +++ b/CHANGES @@ -3422,6 +3422,10 @@ Version 1.5.3beta11 [June 11, 2011] Revised documentation about png_set_user_limits() to say that it can only be used to reduce the defined limit, and that it also affects png writing. + Make the 16-to-8 scaling accurate. Dividing by 256 with no rounding is + wrong (high by one) 25% of the time. Dividing by 257 with rounding is + wrong in 128 out of 65536 cases. Getting the right answer all the time + without division is easy. Send comments/corrections/commendations to png-mng-implement at lists.sf.net (subscription required; visit diff --git a/libpng-manual.txt b/libpng-manual.txt index c8fa524f0..96878fd35 100644 --- a/libpng-manual.txt +++ b/libpng-manual.txt @@ -4096,6 +4096,12 @@ In libpng-1.5.3 we reinitialized the zlib stream for each type of data. We added five png_set_text_*() functions for setting the parameters to use with textual data. +Prior to libpng-1.5.3, the PNG_READ_16_TO_8_ACCURATE_SCALE_SUPPORTED +option was off by default, and slightly inaccurate scaling occurred. +This option can no longer be turned off, and 16-to-8 scaling is always +accurate. This change will result in some different results while +reading 16-bit images, with some of the pixels no longer being off-by-one. + B. Changes to the build and configuration of libpng Details of internal changes to the library code can be found in the CHANGES diff --git a/libpng.3 b/libpng.3 index 40b2e6c0d..ea935b27d 100644 --- a/libpng.3 +++ b/libpng.3 @@ -5047,6 +5047,12 @@ In libpng-1.5.3 we reinitialized the zlib stream for each type of data. We added five png_set_text_*() functions for setting the parameters to use with textual data. +Prior to libpng-1.5.3, the PNG_READ_16_TO_8_ACCURATE_SCALE_SUPPORTED +option was off by default, and slightly inaccurate scaling occurred. +This option can no longer be turned off, and 16-to-8 scaling is always +accurate. This change will result in some different results while +reading 16-bit images, with some of the pixels no longer being off-by-one. + B. Changes to the build and configuration of libpng Details of internal changes to the library code can be found in the CHANGES diff --git a/png.h b/png.h index c62c0258b..da6b9261d 100644 --- a/png.h +++ b/png.h @@ -1659,7 +1659,7 @@ PNG_EXPORT(73, void, png_set_compression_method, (png_structp png_ptr, int method)); #endif -#ifdef PNG_WRITE_CUSTOMIZE_ZTXT_COMPRESSION +#ifdef PNG_WRITE_CUSTOMIZE_ZTXT_COMPRESSION_SUPPORTED /* Also set zlib parameters for compressing non-IDAT chunks */ PNG_EXPORT(222, void, png_set_text_compression_level, (png_structp png_ptr, int level)); @@ -1678,7 +1678,7 @@ PNG_EXPORT(225, void, png_set_text_compression_window_bits, (png_structp PNG_EXPORT(226, void, png_set_text_compression_method, (png_structp png_ptr, int method)); -#endif /* PNG_WRITE_CUSTOMIZE_ZTXT_COMPRESSION */ +#endif /* PNG_WRITE_CUSTOMIZE_ZTXT_COMPRESSION_SUPPORTED */ /* These next functions are called for input/output, memory, and error * handling. They are in the file pngrio.c, pngwio.c, and pngerror.c, diff --git a/pngrtran.c b/pngrtran.c index 4dec1f91c..86c0558a5 100644 --- a/pngrtran.c +++ b/pngrtran.c @@ -1439,11 +1439,11 @@ png_init_read_transformations(png_structp png_ptr) * The PNG_BACKGROUND_EXPAND code above does not expand to 16 bits at * present, so that case is ok (until do_expand_16 is moved.) */ -# define CHOP(x) ((png_uint_16)((2*(png_uint_32)(x) + 257)/514)) - png_ptr->background.red = CHOP(png_ptr->background.red); - png_ptr->background.green = CHOP(png_ptr->background.green); - png_ptr->background.blue = CHOP(png_ptr->background.blue); - png_ptr->background.gray = CHOP(png_ptr->background.gray); +# define CHOP(x) (x)=((png_uint_16)(((png_uint_32)(x)*255+32895) >> 16)) + CHOP(png_ptr->background.red); + CHOP(png_ptr->background.green); + CHOP(png_ptr->background.blue); + CHOP(png_ptr->background.gray); # undef CHOP } #endif @@ -2457,45 +2457,48 @@ png_do_chop(png_row_infop row_info, png_bytep row) if (row_info->bit_depth == 16) { - png_bytep sp = row; - png_bytep dp = row; - png_uint_32 i; - png_uint_32 istop = row_info->width * row_info->channels; + png_bytep sp = row; /* source */ + png_bytep dp = row; /* destinaton */ + png_bytep ep = sp + row_info->rowbytes; /* end+1 */ - for (i = 0; i> 8)) >> 8; - * - * Approximate calculation with shift/add instead of multiply/divide: - * *dp = ((((png_uint_32)(*sp) << 8) | - * (png_uint_32)((int)(*(sp + 1)) - *sp)) + 128) >> 8; - * - * What we actually do to avoid extra shifting and conversion: - */ - - *dp = *sp + ((((int)(*(sp + 1)) - *sp) > 128) ? 1 : 0); -#else - /* Simply discard the low order byte */ - *dp = *sp; -#endif + /* The input is an array of 16 bit components, these must be scaled to + * 8 bits each. For a 16 bit value V the required value (from the PNG + * specification) is: + * + * (V * 255) / 65535 + * + * This reduces to round(V / 257), or floor((V + 128.5)/257) + * + * Represent V as the two byte value vhi.vlo. Make a guess that the + * result is the top byte of V, vhi, then the correction to this value + * is: + * + * error = floor(((V-vhi.vhi) + 128.5) / 257) + * = floor(((vlo-vhi) + 128.5) / 257) + * + * This can be approximated using integer arithmetic (and a signed + * shift): + * + * error = (vlo-vhi+128) >> 8; + * + * The approximate differs from the exact answer only when (vlo-vhi) is + * 128; it then gives a correction of +1 when the exact correction is + * 0. This gives 128 errors. The exact answer (correct for all 16 bit + * input values) is: + * + * error = (vlo-vhi+128)*65535 >> 24; + * + * An alternative arithmetic calculation which also gives no errors is: + * + * (V * 255 + 32895) >> 16 + */ + png_int_32 tmp = *sp++; /* must be signed! */ + tmp += (((int)*sp++ - tmp + 128) * 65535) >> 24; + *dp++ = (png_byte)tmp; } + row_info->bit_depth = 8; row_info->pixel_depth = (png_byte)(8 * row_info->channels); row_info->rowbytes = row_info->width * row_info->channels; diff --git a/pngstruct.h b/pngstruct.h index 8ee8e5054..60aaae2eb 100644 --- a/pngstruct.h +++ b/pngstruct.h @@ -88,7 +88,7 @@ struct png_struct_def #endif /* Added at libpng 1.5.3 */ #if defined(PNG_WRITE_COMPRESSED_TEXT_SUPPORTED) || \ - defined(PNG_WRITE_CUSTOMIZE_ZTXT_COMPRESSION) + defined(PNG_WRITE_CUSTOMIZE_ZTXT_COMPRESSION_SUPPORTED) int zlib_text_level; /* holds zlib compression level */ int zlib_text_method; /* holds zlib compression method */ int zlib_text_window_bits; /* holds zlib compression window bits */ diff --git a/pngtest.c b/pngtest.c index dfdf9e2e0..478d04fb6 100644 --- a/pngtest.c +++ b/pngtest.c @@ -915,7 +915,7 @@ test_one_file(PNG_CONST char *inname, PNG_CONST char *outname) # endif #endif -#ifdef PNG_WRITE_CUSTOMIZE_ZTXT_COMPRESSION +#ifdef PNG_WRITE_CUSTOMIZE_ZTXT_COMPRESSION_SUPPORTED /* Normally one would use Z_DEFAULT_STRATEGY for text compression. * This is here just to make pngtest replicate the results from libpng * versions prior to 1.5.3, and to test this new API. diff --git a/pngwrite.c b/pngwrite.c index c36b0767f..30805eeeb 100644 --- a/pngwrite.c +++ b/pngwrite.c @@ -1459,7 +1459,7 @@ png_set_compression_method(png_structp png_ptr, int method) } /* The following were added to libpng-1.5.3 */ -#ifdef PNG_WRITE_CUSTOMIZE_ZTXT_COMPRESSION +#ifdef PNG_WRITE_CUSTOMIZE_ZTXT_COMPRESSION_SUPPORTED void PNGAPI png_set_text_compression_level(png_structp png_ptr, int level) { @@ -1538,7 +1538,7 @@ png_set_text_compression_method(png_structp png_ptr, int method) png_ptr->flags |= PNG_FLAG_ZTXT_CUSTOM_METHOD; png_ptr->zlib_text_method = method; } -#endif /* PNG_WRITE_CUSTOMIZE_ZTXT_COMPRESSION */ +#endif /* PNG_WRITE_CUSTOMIZE_ZTXT_COMPRESSION_SUPPORTED */ /* end of API added to libpng-1.5.3 */ void PNGAPI diff --git a/pngwutil.c b/pngwutil.c index ffa879238..adca5fcd7 100644 --- a/pngwutil.c +++ b/pngwutil.c @@ -834,7 +834,7 @@ png_write_IHDR(png_structp png_ptr, png_uint_32 width, png_uint_32 height, png_ptr->zlib_method = 8; #ifdef PNG_WRITE_COMPRESSED_TEXT_SUPPORTED -#ifdef PNG_WRITE_CUSTOMIZE_ZTXT_COMPRESSION +#ifdef PNG_WRITE_CUSTOMIZE_ZTXT_COMPRESSION_SUPPORTED if (!(png_ptr->flags & PNG_FLAG_ZTXT_CUSTOM_STRATEGY)) png_ptr->zlib_text_strategy = Z_DEFAULT_STRATEGY; @@ -855,7 +855,7 @@ png_write_IHDR(png_structp png_ptr, png_uint_32 width, png_uint_32 height, png_ptr->zlib_text_mem_level = png_ptr->zlib_mem_level; png_ptr->zlib_text_window_bits = png_ptr->zlib_window_bits; png_ptr->zlib_text_method = png_ptr->zlib_method; -#endif /* PNG_WRITE_CUSTOMIZE_ZTXT_COMPRESSION */ +#endif /* PNG_WRITE_CUSTOMIZE_ZTXT_COMPRESSION_SUPPORTED */ #endif /* PNG_WRITE_COMPRESSED_TEXT_SUPPORTED */ /* Record that the compressor has not yet been initialized. */ diff --git a/scripts/pnglibconf.dfa b/scripts/pnglibconf.dfa index 79377e3ee..0e95ec779 100644 --- a/scripts/pnglibconf.dfa +++ b/scripts/pnglibconf.dfa @@ -346,12 +346,15 @@ option INCH_CONVERSIONS option BUILD_GRAYSCALE_PALETTE +# This is no longer used - the scaling is always accurate - but the definition +# is left in for older applications (including pngvalid) that check it. Do not +# turn it off! +option READ_16_TO_8_ACCURATE_SCALE requires READ + # IN DEVELOPMENT -# These are currently experimental features, define them if you want +# These are currently experimental features; define them if you want -# Very little testing, not enabled by default. - -option READ_16_TO_8_ACCURATE_SCALE requires READ disabled +# NOTHING HERE # WRITE options diff --git a/scripts/pnglibconf.h.prebuilt b/scripts/pnglibconf.h.prebuilt index e1046687b..dd470dd9f 100644 --- a/scripts/pnglibconf.h.prebuilt +++ b/scripts/pnglibconf.h.prebuilt @@ -68,6 +68,7 @@ #define PNG_POINTER_INDEXING_SUPPORTED #define PNG_PROGRESSIVE_READ_SUPPORTED #define PNG_READ_16BIT_SUPPORTED +#define PNG_READ_16_TO_8_ACCURATE_SCALE_SUPPORTED #define PNG_READ_16_TO_8_SUPPORTED #define PNG_READ_ALPHA_MODE_SUPPORTED #define PNG_READ_ANCILLARY_CHUNKS_SUPPORTED @@ -154,7 +155,7 @@ #define PNG_WRITE_INT_FUNCTIONS_SUPPORTED #define PNG_WRITE_INVERT_ALPHA_SUPPORTED #define PNG_WRITE_INVERT_SUPPORTED -#define PNG_WRITE_CUSTOMIZE_ZTXT_COMPRESSION +#define PNG_WRITE_CUSTOMIZE_ZTXT_COMPRESSION_SUPPORTED #define PNG_WRITE_iTXt_SUPPORTED #define PNG_WRITE_oFFs_SUPPORTED #define PNG_WRITE_OPTIMIZE_CMF_SUPPORTED @@ -182,6 +183,5 @@ #define PNG_zTXt_SUPPORTED #define PNG_WRITE_COMPRESSED_TEXT_SUPPORTED /*#undef PNG_ERROR_NUMBERS_SUPPORTED*/ -/*#undef PNG_READ_16_TO_8_ACCURATE_SCALE_SUPPORTED*/ /* end of options */ #endif /* PNGLCONF_H */