LOW_MEMORY and COMPAT bug fixes

The LOW_MEMORY PNG_COMPRESSION option should not be setting HUFFMAN_ONLY or
using a low deflate 'level'; according to the comments in zconf.h only
windowBits and memLevel affect the memory.  pngwutil.c has been changed to use
the same values as HIGH compression.

The COMPAT option turned on the old optimize_cmf code (now in fix_cinfo),
however there was a serious bug in that code; it put the wrong value in z_cmf.
The setting was also not handled correctly in pz_compression_settings.

pngtest now verifies the operation of COMPAT and, as a result, pngtest.png has
been reverted to the libpng 1.6 (etc) version.

IDAT size handling has been improved; if not explicitly set values appropriate
to png_level are now chosen (in addition to the handling for the COMPAT
setting).  HIGH and HIGH_READ_SPEED now create unlimited size IDAT chunks, which
requires buffering the whole of the IDAT data in memory but reflects what other
programs and optimizers do.

Signed-off-by: John Bowler <jbowler@acm.org>

png.h

@@ -1732,11 +1732,12 @@ PNG_REMOVED(66, void, png_set_crc_action, (png_structrp png_ptr,
       /* Minimize the memory required both when reading (4) and writing (2) the
        * PNG.  This results in a significantly larger PNG (which may itself have
        * the opposite effect of slowing down either read or write) however the
-       * memory overhead is reduced and, apart from the extra time to read or
-       * write the data, the read and write time is likely to be reduced too.
+       * memory overhead is reduced and, apart from the extra time to read the
+       * data, the read time is likely to be reduced too.
        *
        * Use this when both read and write will happen on a memory starved
-       * (really, very low memory) system.
+       * (really, very low memory) system.  Note that this sets a high deflate
+       * compression setting because that does not affect zlib memory usage.
        */
 #  define PNG_COMPRESSION_HIGH_SPEED (2)
       /* Minimize the time to both read (5) and write (3) the PNG.  This uses

pngtest.c

@@ -1386,6 +1386,11 @@ test_one_file(PNG_CONST char *inname, PNG_CONST char *outname)
    png_write_info(write_ptr, write_info_ptr);
 
    write_chunks(write_ptr, before_IDAT); /* after PLTE */
+
+#ifdef PNG_COMPRESSION_COMPAT
+   /* Test the 'compatibility' setting here, if it is available. */
+   png_set_compression(write_ptr, PNG_COMPRESSION_COMPAT);
+#endif
 #endif
 
 #ifdef SINGLE_ROWBUF_ALLOC

pngwutil.c

@@ -1036,7 +1036,7 @@ png_deflate_destroy(png_structrp png_ptr)
 
 static png_int_32
 pz_compression_setting(png_structrp png_ptr, png_uint_32 owner,
-      int min, int max, int shift, png_int_32 value, int only_get)
+      int min, int max, int shift, png_int_32 value, int only_get, int unset)
    /* This is a support function for png_write_setting below. */
 {
    png_zlib_statep ps;
@@ -1079,33 +1079,32 @@ pz_compression_setting(png_structrp png_ptr, png_uint_32 owner,
           * software engineers never made mistakes.
           */
          res = pz_compression_setting(png_ptr, png_IDAT, min, max, shift,
-               value, 0/*set*/);
+               value, 0/*set*/, 1/*iff unset*/);
 
          if (PNG_FAILED(res))
             return res;
 
          res = pz_compression_setting(png_ptr, png_iCCP, min, max, shift,
-               value, 0/*set*/);
+               value, 0/*set*/, 1/*iff unset*/);
 
          if (PNG_FAILED(res))
             return res;
 
-         /* The text settings are only changed if the system builder didn't
-          * disable the API to change them.  Perhaps this API shouldn't exist?
+         /* The text settings are changed regardless of the customize support
+          * because if WRITE_CUSTOMIZE_ZTXT_COMPRESSION is not supported the old
+          * behavior was to use the WRITE_CUSTOMIZE_COMPRESSION setting.
+          *
+          * However, when we get png_zTXt directly (from png_write_setting) and
+          * the support is not compiled in return PNG_ENOSYS.
           */
-         res = pz_compression_setting(png_ptr, png_iCCP, min, max, shift,
-               value, 0/*set*/);
-
-         if (PNG_FAILED(res))
-            return res;
-
-         return 0;
-
+         unset = 1; /* i.e. only if not already set */
 
+#     ifdef PNG_WRITE_CUSTOMIZE_ZTXT_COMPRESSION_SUPPORTED
          case png_zTXt:
          case png_iTXt:
-            if (ps != NULL) psettings = &ps->pz_text;
-            break;
+#     endif /* WRITE_CUSTOMIZE_ZTXT_COMPRESSION */
+         if (ps != NULL) psettings = &ps->pz_text;
+         break;
 
       default:
          /* Return PNG_ENOSYS, not PNG_EINVAL, to support future addition of new
@@ -1122,7 +1121,10 @@ pz_compression_setting(png_structrp png_ptr, png_uint_32 owner,
       png_uint_32 settings = *psettings;
       png_uint_32 mask = 0xFU << shift;
 
-      if (!only_get)
+      /* Do not set it if 'only_get' was passed in or if 'unset' is true and the
+       * setting is not currently set:
+       */
+      if (!only_get && ((settings & mask) == 0U || !unset))
          *psettings = (settings & ~mask) +
             ((png_uint_32)/*SAFE*/(value-min+1) << shift);
 
@@ -1138,7 +1140,7 @@ pz_compression_setting(png_structrp png_ptr, png_uint_32 owner,
 
 #define compression_setting(pp, owner, setting, value, get)\
    pz_compression_setting(pp, owner, pz_min(setting), pz_max(setting),\
-         pz_shift(setting), value, get)
+         pz_shift(setting), value, get, 0/*always*/)
 
 /* There is (as of zlib 1.2.8) a bug in the implementation of compression with a
  * window size of 256 which zlib works round by resetting windowBits from 8 to 9
@@ -1186,28 +1188,27 @@ fix_cinfo(png_zlib_statep ps, png_bytep data, png_alloc_size_t data_size)
 
    else if (data_size > 0U)
    {
-      int windowBits = 8+(data[0] >> 4);
-      unsigned int half_window_size = 1U << (windowBits-1);
+      /* Prior to 1.7.0 libpng would shrink the windowBits even if the
+       * application requested a particular value, so:
+       */
+      unsigned int z_cinfo = data[0] >> 4;
+      unsigned int half_z_window_size = 1U << (z_cinfo + 7);
 
-      debug(pz_get(ps, current, windowBits, 0) == windowBits);
-
-      if (data_size <= half_window_size /* Can shrink */ &&
-          pz_get(ps, IDAT, png_level, PNG_DEFAULT_COMPRESSION_LEVEL) ==
-                 PNG_COMPRESSION_COMPAT)
+      if (data_size <= half_z_window_size && z_cinfo > 0)
       {
-         unsigned int d1;
+         unsigned int tmp;
 
-         /* Before 1.7.0 libpng overrode a user-supplied windowBits if the data
-          * was smaller.
-          */
          do
-            --windowBits, half_window_size >>= 1;
-         while (data_size <= half_window_size);
+         {
+            half_z_window_size >>= 1;
+            --z_cinfo;
+         }
+         while (z_cinfo > 0 && data_size <= half_z_window_size);
 
-         data[0] = PNG_BYTE((windowBits << 4) + 0x8U);
-         d1 = data[1] & 0xE0U; /* top three bits */
-         d1 += 31U - ((data[0]<<8) + d1) % 31U;
-         data[1] = PNG_BYTE(d1);
+         data[0] = PNG_BYTE((z_cinfo << 4) + 0x8U);
+         tmp = data[1] & 0xE0U; /* top three bits */
+         tmp += 31U - ((data[0] << 8) + tmp) % 31U;
+         data[1] = PNG_BYTE(tmp);
       }
    }
 
@@ -1272,11 +1273,6 @@ pz_default_settings(png_uint_32 settings, const png_uint_32 owner,
                strategy = Z_FILTERED;
             break;
 
-         case PNG_COMPRESSION_LOW_MEMORY:
-            /* Reduce memory at all costs: */
-            strategy = Z_HUFFMAN_ONLY;
-            break;
-
          case PNG_COMPRESSION_HIGH_SPEED:
             /* RLE is as fast as HUFFMAN_ONLY and can reduce size a lot in a few
              * cases.
@@ -1314,10 +1310,13 @@ pz_default_settings(png_uint_32 settings, const png_uint_32 owner,
             else if (owner == png_iCCP)
                strategy = Z_DEFAULT_STRATEGY;
 
+            /* TODO: investigate this, the observed behavior is suspicious: */
             else /* text chunk */
                strategy = Z_FILTERED; /* Always better for some reason */
             break;
 
+         case PNG_COMPRESSION_LOW_MEMORY:
+            /* Reduce memory at all costs, speed doesn't matter. */
          case PNG_COMPRESSION_HIGH_READ_SPEED:
          case PNG_COMPRESSION_HIGH:
             if (owner == png_IDAT || owner == png_iCCP)
@@ -1393,7 +1392,6 @@ pz_default_settings(png_uint_32 settings, const png_uint_32 owner,
                   zlib_level = Z_DEFAULT_COMPRESSION; /* NOTE: -1 */
                   break;
 
-               case PNG_COMPRESSION_LOW_MEMORY:
                case PNG_COMPRESSION_HIGH_SPEED:
                   zlib_level = 1;
                   break;
@@ -1407,6 +1405,7 @@ pz_default_settings(png_uint_32 settings, const png_uint_32 owner,
                   zlib_level = 6; /* Old default! */
                   break;
 
+               case PNG_COMPRESSION_LOW_MEMORY:
                case PNG_COMPRESSION_HIGH_READ_SPEED:
                case PNG_COMPRESSION_HIGH:
                   zlib_level = 9;
@@ -1445,6 +1444,7 @@ pz_default_settings(png_uint_32 settings, const png_uint_32 owner,
           */
          windowBits = 15;
 
+         if (data_size <= 16384U)
          {
             unsigned int half_window_size = 1U << (windowBits-1);
 
@@ -3058,12 +3058,39 @@ png_write_IDAT(png_structrp png_ptr, int flush)
    IDAT_size = png_ptr->IDAT_size;
    if (IDAT_size == 0U)
    {
-      if (pz_get(ps, IDAT, png_level, PNG_DEFAULT_COMPRESSION_LEVEL) !=
-          PNG_COMPRESSION_COMPAT/*legacy*/)
-         IDAT_size = PNG_ZBUF_SIZE;
+      switch (pz_get(ps, IDAT, png_level, PNG_DEFAULT_COMPRESSION_LEVEL))
+      {
+         case PNG_COMPRESSION_COMPAT: /* Legacy */
+            IDAT_size = 8192U;
+            break;
 
-      else
-         IDAT_size = 8192U;
+         case PNG_COMPRESSION_LOW_MEMORY:
+         case PNG_COMPRESSION_HIGH_SPEED:
+         case PNG_COMPRESSION_LOW:
+            /* png_compress uses PNG_ROW_BUFFER_SIZE buffers for the compressed
+             * data.  Optimize to allocate only one of these:
+             */
+            IDAT_size = PNG_ROW_BUFFER_SIZE;
+            break;
+
+         default:
+         case PNG_COMPRESSION_MEDIUM:
+            IDAT_size = PNG_ZBUF_SIZE;
+
+         case PNG_COMPRESSION_HIGH_READ_SPEED:
+            /* Assume the reader reads partial IDAT chunks (pretty much a
+             * requirement given that some PNG encoders produce just one IDAT)
+             */
+         case PNG_COMPRESSION_HIGH:
+            /* This doesn't control the amount of memory allocated unless the
+             * PNG IDAT data really is this big.
+             *
+             * TODO: review handling out-of-memory from png_compress() by
+             * flushing an IDAT.
+             */
+            IDAT_size = PNG_UINT_31_MAX;
+            break;
+      }
    }
 
    /* Write IDAT chunks while either 'flush' is true or there are at
@@ -3985,7 +4012,7 @@ png_start_filter_select(png_zlib_statep ps, unsigned int bpp)
                ps->filter_select_threshold2 = 50U; /* TODO: experiment! */
                break;
 
-            case -1: /* Legacy */
+            case PNG_COMPRESSION_COMPAT: /* Legacy */
                memset(fs->sum_bias, 0U, sizeof fs->sum_bias);
                ps->filter_select_threshold = 1U; /* disabled */
                ps->filter_select_threshold2 = 1U;

scripts/pnglibconf.dfa

@@ -779,8 +779,12 @@ setting sCAL_PRECISION default 5
 # written IDAT chunks.  It can be any (zlib) uInt value, however this amount of
 # data has to be buffered on write so it is recommended that a smaller size be
 # used unless saving the 12-byte chunk overhead is necessary.
+#
+# If not set libpng uses the DEFAULT_COMPRESSION_LEVEL setting to determine
+# something appropriate.  The value below is only used for 'MEDIUM' compression
+# (which is the default: see below.)
 
-setting ZBUF_SIZE default 4096
+setting ZBUF_SIZE default 8192
 
 # This is the size of the decompression buffer used when counting or checking
 # the decompressed size of an LZ stream from a compressed ancilliary chunk; the