Fix a memory leak in the riffled palette optimization on ARM; refactor

Move deallocation of riffled_palette from png_write_destroy to png_read_destroy. The reader (not the writer) is the owner of riffled_palette. Move allocation and initialization of riffled_palette from png_do_read_transformations to png_init_palette_transformations. Allow riffled_palette inside png_struct only if the ARM Neon optimizations are enabled. Rename png_riffle_palette_rgba to png_riffle_palette_rgba8, etc., to better indicate the strict applicability of these routines. Fix an unused parameter warning in the build configurations where riffled palette optimization is not enabled. Fix indentation.
clayne · Feb 4, 2019 · 70d122a · 70d122a
1 parent 0a882b5
commit 70d122a
Show file tree

Hide file tree

Showing 6 changed files with 68 additions and 66 deletions.
diff --git a/arm/palette_neon_intrinsics.c b/arm/palette_neon_intrinsics.c
@@ -1,7 +1,7 @@
 
 /* palette_neon_intrinsics.c - NEON optimised palette expansion functions
  *
- * Copyright (c) 2018 Cosmin Truta
+ * Copyright (c) 2018-2019 Cosmin Truta
  * Copyright (c) 2017-2018 Arm Holdings. All rights reserved.
  * Written by Richard Townsend <[email protected]>, February 2017.
  *
@@ -20,9 +20,9 @@
 #  include <arm_neon.h>
 #endif
 
-/* Build an RGBA palette from the RGB and separate alpha palettes. */
+/* Build an RGBA8 palette from the separate RGB and alpha palettes. */
 void
-png_riffle_palette_rgba(png_structrp png_ptr, png_row_infop row_info)
+png_riffle_palette_rgba8(png_structrp png_ptr)
 {
    png_const_colorp palette = png_ptr->palette;
    png_bytep riffled_palette = png_ptr->riffled_palette;
@@ -38,16 +38,10 @@ png_riffle_palette_rgba(png_structrp png_ptr, png_row_infop row_info)
       vdupq_n_u8(0xff),
    }};
 
-   if (row_info->bit_depth != 8)
-   {
-      png_error(png_ptr, "bit_depth must be 8 for png_riffle_palette_rgba");
-      return;
-   }
-
-   /* First, riffle the RGB colours into a RGBA palette, the A value is
-    * set to opaque for now.
+   /* First, riffle the RGB colours into an RGBA8 palette.
+    * The alpha component is set to opaque for now.
     */
-   for (i = 0; i < (1 << row_info->bit_depth); i += 16)
+   for (i = 0; i < 256; i += 16)
    {
       uint8x16x3_t v = vld3q_u8((png_const_bytep)(palette + i));
       w.val[0] = v.val[0];
@@ -61,9 +55,9 @@ png_riffle_palette_rgba(png_structrp png_ptr, png_row_infop row_info)
       riffled_palette[(i << 2) + 3] = trans_alpha[i];
 }
 
-/* Expands a palettized row into RGBA. */
+/* Expands a palettized row into RGBA8. */
 int
-png_do_expand_palette_neon_rgba(png_structrp png_ptr, png_row_infop row_info,
+png_do_expand_palette_neon_rgba8(png_structrp png_ptr, png_row_infop row_info,
     png_const_bytep row, png_bytepp ssp, png_bytepp ddp)
 {
    png_uint_32 row_width = row_info->width;
@@ -103,9 +97,9 @@ png_do_expand_palette_neon_rgba(png_structrp png_ptr, png_row_infop row_info,
    return i;
 }
 
-/* Expands a palettized row into RGB format. */
+/* Expands a palettized row into RGB8. */
 int
-png_do_expand_palette_neon_rgb(png_structrp png_ptr, png_row_infop row_info,
+png_do_expand_palette_neon_rgb8(png_structrp png_ptr, png_row_infop row_info,
     png_const_bytep row, png_bytepp ssp, png_bytepp ddp)
 {
    png_uint_32 row_width = row_info->width;

diff --git a/pngpriv.h b/pngpriv.h
@@ -1,7 +1,7 @@
 
 /* pngpriv.h - private declarations for use inside libpng
  *
- * Copyright (c) 2018 Cosmin Truta
+ * Copyright (c) 2018-2019 Cosmin Truta
  * Copyright (c) 1998-2002,2004,2006-2018 Glenn Randers-Pehrson
  * Copyright (c) 1996-1997 Andreas Dilger
  * Copyright (c) 1995-1996 Guy Eric Schalnat, Group 42, Inc.
@@ -2119,19 +2119,19 @@ PNG_INTERNAL_FUNCTION(png_uint_32, png_check_keyword, (png_structrp png_ptr,
 
 #if PNG_ARM_NEON_IMPLEMENTATION == 1
 PNG_INTERNAL_FUNCTION(void,
-                      png_riffle_palette_rgba,
-                      (png_structrp, png_row_infop),
+                      png_riffle_palette_rgba8,
+                      (png_structrp),
                       PNG_EMPTY);
 PNG_INTERNAL_FUNCTION(int,
-                      png_do_expand_palette_neon_rgba,
+                      png_do_expand_palette_neon_rgba8,
                       (png_structrp,
                        png_row_infop,
                        png_const_bytep,
                        const png_bytepp,
                        const png_bytepp),
                       PNG_EMPTY);
 PNG_INTERNAL_FUNCTION(int,
-                      png_do_expand_palette_neon_rgb,
+                      png_do_expand_palette_neon_rgb8,
                       (png_structrp,
                        png_row_infop,
                        png_const_bytep,

diff --git a/pngread.c b/pngread.c
@@ -1,7 +1,7 @@
 
 /* pngread.c - read a PNG file
  *
- * Copyright (c) 2018 Cosmin Truta
+ * Copyright (c) 2018-2019 Cosmin Truta
  * Copyright (c) 1998-2002,2004,2006-2018 Glenn Randers-Pehrson
  * Copyright (c) 1996-1997 Andreas Dilger
  * Copyright (c) 1995-1996 Guy Eric Schalnat, Group 42, Inc.
@@ -994,6 +994,12 @@ png_read_destroy(png_structrp png_ptr)
    png_ptr->chunk_list = NULL;
 #endif
 
+#if defined(PNG_READ_EXPAND_SUPPORTED) && \
+    defined(PNG_ARM_NEON_IMPLEMENTATION)
+   png_free(png_ptr, png_ptr->riffled_palette);
+   png_ptr->riffled_palette = NULL;
+#endif
+
    /* NOTE: the 'setjmp' buffer may still be allocated and the memory and error
     * callbacks are still set at this point.  They are required to complete the
     * destruction of the png_struct itself.

diff --git a/pngrtran.c b/pngrtran.c
@@ -1,7 +1,7 @@
 
 /* pngrtran.c - transforms the data in a row for PNG readers
  *
- * Copyright (c) 2018 Cosmin Truta
+ * Copyright (c) 2018-2019 Cosmin Truta
  * Copyright (c) 1998-2002,2004,2006-2018 Glenn Randers-Pehrson
  * Copyright (c) 1996-1997 Andreas Dilger
  * Copyright (c) 1995-1996 Guy Eric Schalnat, Group 42, Inc.
@@ -1161,7 +1161,20 @@ png_init_palette_transformations(png_structrp png_ptr)
          png_ptr->transformations &= ~(PNG_COMPOSE | PNG_BACKGROUND_EXPAND);
    }
 
-#if defined(PNG_READ_EXPAND_SUPPORTED) && defined(PNG_READ_BACKGROUND_SUPPORTED)
+#ifdef PNG_READ_EXPAND_SUPPORTED
+#ifdef PNG_ARM_NEON_INTRINSICS_AVAILABLE
+   /* Initialize the accelerated palette expansion, if applicable. */
+   if ((png_ptr->transformations & PNG_EXPAND) != 0)
+   {
+      if ((png_ptr->num_trans > 0) && (png_ptr->bit_depth == 8))
+      {
+         png_ptr->riffled_palette = (png_bytep)png_malloc(png_ptr, 256 * 4);
+         png_riffle_palette_rgba8(png_ptr);
+      }
+   }
+#endif /* PNG_ARM_NEON_INTRINSICS_AVAILABLE */
+
+#ifdef PNG_READ_BACKGROUND_SUPPORTED
    /* png_set_background handling - deals with the complexity of whether the
     * background color is in the file format or the screen format in the case
     * where an 'expand' will happen.
@@ -1182,24 +1195,25 @@ png_init_palette_transformations(png_structrp png_ptr)
              png_ptr->palette[png_ptr->background.index].blue;
 
 #ifdef PNG_READ_INVERT_ALPHA_SUPPORTED
-        if ((png_ptr->transformations & PNG_INVERT_ALPHA) != 0)
-        {
-           if ((png_ptr->transformations & PNG_EXPAND_tRNS) == 0)
-           {
-              /* Invert the alpha channel (in tRNS) unless the pixels are
-               * going to be expanded, in which case leave it for later
-               */
-              int i, istop = png_ptr->num_trans;
-
-              for (i=0; i<istop; i++)
-                 png_ptr->trans_alpha[i] = (png_byte)(255 -
-                    png_ptr->trans_alpha[i]);
-           }
-        }
+         if ((png_ptr->transformations & PNG_INVERT_ALPHA) != 0)
+         {
+            if ((png_ptr->transformations & PNG_EXPAND_tRNS) == 0)
+            {
+               /* Invert the alpha channel (in tRNS) unless the pixels are
+                * going to be expanded, in which case leave it for later
+                */
+               int i, istop = png_ptr->num_trans;
+
+               for (i = 0; i < istop; i++)
+                  png_ptr->trans_alpha[i] =
+                      (png_byte)(255 - png_ptr->trans_alpha[i]);
+            }
+         }
 #endif /* READ_INVERT_ALPHA */
       }
    } /* background expand and (therefore) no alpha association. */
-#endif /* READ_EXPAND && READ_BACKGROUND */
+#endif /* READ_BACKGROUND */
+#endif /* READ_EXPAND */
 }
 
 static void /* PRIVATE */
@@ -4320,9 +4334,11 @@ png_do_expand_palette(png_structrp png_ptr, png_row_infop row_info,
                    * but sometimes row_info->bit_depth has been changed to 8.
                    * In these cases, the palette hasn't been riffled.
                    */
-                  i = png_do_expand_palette_neon_rgba(png_ptr, row_info, row,
+                  i = png_do_expand_palette_neon_rgba8(png_ptr, row_info, row,
                       &sp, &dp);
                }
+#else
+               PNG_UNUSED(png_ptr)
 #endif
 
                for (; i < row_width; i++)
@@ -4349,8 +4365,10 @@ png_do_expand_palette(png_structrp png_ptr, png_row_infop row_info,
                dp = row + (size_t)(row_width * 3) - 1;
                i = 0;
 #ifdef PNG_ARM_NEON_INTRINSICS_AVAILABLE
-               i = png_do_expand_palette_neon_rgb(png_ptr, row_info, row,
+               i = png_do_expand_palette_neon_rgb8(png_ptr, row_info, row,
                    &sp, &dp);
+#else
+               PNG_UNUSED(png_ptr)
 #endif
 
                for (; i < row_width; i++)
@@ -4767,22 +4785,8 @@ png_do_read_transformations(png_structrp png_ptr, png_row_infop row_info)
    {
       if (row_info->color_type == PNG_COLOR_TYPE_PALETTE)
       {
-#ifdef PNG_ARM_NEON_INTRINSICS_AVAILABLE
-         if ((png_ptr->num_trans > 0) && (png_ptr->bit_depth == 8))
-         {
-            /* Allocate space for the decompressed full palette. */
-            if (png_ptr->riffled_palette == NULL)
-            {
-               png_ptr->riffled_palette = png_malloc(png_ptr, 256*4);
-               if (png_ptr->riffled_palette == NULL)
-                  png_error(png_ptr, "NULL row buffer");
-               /* Build the RGBA palette. */
-               png_riffle_palette_rgba(png_ptr, row_info);
-            }
-         }
-#endif
          png_do_expand_palette(png_ptr, row_info, png_ptr->row_buf + 1,
-            png_ptr->palette, png_ptr->trans_alpha, png_ptr->num_trans);
+             png_ptr->palette, png_ptr->trans_alpha, png_ptr->num_trans);
       }
 
       else

diff --git a/pngstruct.h b/pngstruct.h
@@ -1,7 +1,7 @@
 
 /* pngstruct.h - header file for PNG reference library
  *
- * Copyright (c) 2018 Cosmin Truta
+ * Copyright (c) 2018-2019 Cosmin Truta
  * Copyright (c) 1998-2002,2004,2006-2018 Glenn Randers-Pehrson
  * Copyright (c) 1996-1997 Andreas Dilger
  * Copyright (c) 1995-1996 Guy Eric Schalnat, Group 42, Inc.
@@ -228,10 +228,6 @@ struct png_struct_def
                                * big_row_buf; while writing it is separately
                                * allocated.
                                */
-#ifdef PNG_READ_EXPAND_SUPPORTED
-   /* Buffer to accelerate palette transformations. */
-   png_bytep riffled_palette;
-#endif
 #ifdef PNG_WRITE_FILTER_SUPPORTED
    png_bytep try_row;    /* buffer to save trial row when filtering */
    png_bytep tst_row;    /* buffer to save best trial row when filtering */
@@ -396,6 +392,12 @@ struct png_struct_def
    /* deleted in 1.5.5: rgb_to_gray_blue_coeff; */
 #endif
 
+/* New member added in libpng-1.6.36 */
+#if defined(PNG_READ_EXPAND_SUPPORTED) && \
+    defined(PNG_ARM_NEON_IMPLEMENTATION)
+   png_bytep riffled_palette; /* buffer for accelerated palette expansion */
+#endif
+
 /* New member added in libpng-1.0.4 (renamed in 1.0.9) */
 #if defined(PNG_MNG_FEATURES_SUPPORTED)
 /* Changed from png_byte to png_uint_32 at version 1.2.0 */

diff --git a/pngwrite.c b/pngwrite.c
@@ -1,7 +1,7 @@
 
 /* pngwrite.c - general routines to write a PNG file
  *
- * Copyright (c) 2018 Cosmin Truta
+ * Copyright (c) 2018-2019 Cosmin Truta
  * Copyright (c) 1998-2002,2004,2006-2018 Glenn Randers-Pehrson
  * Copyright (c) 1996-1997 Andreas Dilger
  * Copyright (c) 1995-1996 Guy Eric Schalnat, Group 42, Inc.
@@ -948,10 +948,6 @@ png_write_destroy(png_structrp png_ptr)
    png_free_buffer_list(png_ptr, &png_ptr->zbuffer_list);
    png_free(png_ptr, png_ptr->row_buf);
    png_ptr->row_buf = NULL;
-#ifdef PNG_READ_EXPANDED_SUPPORTED
-   png_free(png_ptr, png_ptr->riffled_palette);
-   png_ptr->riffled_palette = NULL;
-#endif
 #ifdef PNG_WRITE_FILTER_SUPPORTED
    png_free(png_ptr, png_ptr->prev_row);
    png_free(png_ptr, png_ptr->try_row);