GraphicsMagick: Several improvements involving WPG, BMP, PNM, an...

GraphicsMagick Commits <[email protected]> Sun, 08 Oct 2023 12:44:23 -0500
Newsgroups gmane.comp.video.graphicsmagick.cvs
Message-ID <mailman.7122.1696787087.1961.graphicsmagick-commit@lists.sourceforge.net>
changeset e5a32c8a0135 in /hg/GraphicsMagick
details: http://hg.GraphicsMagick.org/hg/GraphicsMagick?cmd=changeset;node=e5a32c8a0135
summary: Several improvements involving WPG, BMP, PNM, and Pixel Cache

diffstat:

 ChangeLog                              |   21 ++++++
 PerlMagick/t/png/read.t                |   26 +++++--
 VisualMagick/installer/inc/version.isx |    4 +-
 coders/bmp.c                           |  104 ++++++++++++++++++--------------
 coders/pnm.c                           |   46 ++++++++++++++-
 coders/wpg.c                           |    2 +-
 magick/pixel_cache.c                   |   99 +++++++++++++++++++++---------
 magick/version.h                       |    4 +-
 www/Changelog.html                     |   42 +++++++++++++
 www/api/pixel_cache.html               |   51 ++++++++-------
 10 files changed, 286 insertions(+), 113 deletions(-)

diffs (truncated from 729 to 500 lines):

diff -r debb806be563 -r e5a32c8a0135 ChangeLog
--- a/ChangeLog	Mon Oct 02 19:36:00 2023 +0200
+++ b/ChangeLog	Sun Oct 08 12:44:21 2023 -0500
@@ -1,3 +1,24 @@
+2023-10-08  Bob Friesenhahn  <[email protected]>
+
+	* coders/wpg.c (ReadWPGImage): Add a FIXME comment for a bug yet
+	to be fixed.
+
+	* PerlMagick/t/png/read.t: Add a test case for BMP with embedded
+	PNG.  Something is still wrong given that reading using PerlMagick
+	is returning two frames rather than one.
+
+	* magick/pixel_cache.c: Update documentation to reflect that
+	GetImagePixels() has been deprecated since 2008, and update
+	mention of it to document use of AccessMutableIndexes() or
+	AccessImmutableIndexes().
+
+	* coders/pnm.c (ReadPNMImage): Add traces prior to each decoding
+	loop to identify the sub-format being decoded.
+
+	* coders/bmp.c (ExtractNestedBlob): Pass pointer to image list to
+	satisfy requirements of AppendImageToList() and
+	DeleteImageFromList().
+
 2023-10-02  Fojtik Jaroslav  <[email protected]>
 
 	* coders/bmp.c: Attempt to read BMP with 48bpp.
diff -r debb806be563 -r e5a32c8a0135 PerlMagick/t/png/read.t
--- a/PerlMagick/t/png/read.t	Mon Oct 02 19:36:00 2023 +0200
+++ b/PerlMagick/t/png/read.t	Sun Oct 08 12:44:21 2023 -0500
@@ -1,5 +1,5 @@
 #!/usr/local/bin/perl
-# Copyright (C) 2003 GraphicsMagick Group
+# Copyright (C) 2003-2023 GraphicsMagick Group
 # Copyright (C) 2002 ImageMagick Studio
 # Copyright (C) 1991-1999 E. I. du Pont de Nemours and Company
 #
@@ -13,7 +13,7 @@
 # Contributed by Bob Friesenhahn <[email protected]>
 #
 
-BEGIN { $| = 1; $test=1; print "1..6\n"; }
+BEGIN { $| = 1; $test=1; print "1..7\n"; }
 END {print "not ok $test\n" unless $loaded;}
 use Graphics::Magick;
 $loaded=1;
@@ -24,14 +24,14 @@
 
 #
 # 1) Test Black-and-white, bit_depth=1 PNG
-# 
+#
 print( "1-bit grayscale PNG ...\n" );
 testRead( 'input_bw.png', q//,
   '552ee0b3b9159ceedc4ce30125e9db38d2045d53b941844a72ac5f5f38335454' );
 
 #
 # 2) Test Monochrome PNG
-# 
+#
 ++$test;
 print( "8-bit grayscale PNG ...\n" );
 testRead( 'input_mono.png', q//,
@@ -39,7 +39,7 @@
 
 #
 # 3) Test 16-bit Portable Network Graphics
-# 
+#
 ++$test;
 print( "16-bit grayscale PNG ...\n" );
 testRead( 'input_16.png', q//,
@@ -47,7 +47,7 @@
   'c87cc12715f3e0619d6fe871bd8132f88facffb3d38dd8869cb262ec6b9c4cef' );
 #
 # 4) Test 256 color pseudocolor PNG
-# 
+#
 ++$test;
 print( "8-bit indexed-color PNG ...\n" );
 testRead( 'input_256.png', q//,
@@ -55,7 +55,7 @@
 
 #
 # 5) Test TrueColor PNG
-# 
+#
 ++$test;
 print( "24-bit Truecolor PNG ...\n" );
 testRead( 'input_truecolor.png', q//,
@@ -63,9 +63,19 @@
 
 #
 # 6) Test Multiple-image Network Graphics
-# 
+#
 ++$test;
 print( "MNG with 24-bit Truecolor PNGs...\n" );
 testRead( 'input.mng', q//,
   'eb089161ebc5ab3964cdec1b72628c4d1c29ebd78f333a3d4a0d47c614fb3897' );
 
+#
+# 7) Test PNG in BMP format
+
+# FIXME: Somehow we get an extra preceding blank frame from the
+# PNG-compressed BMP reader/sample although there is no other evidence
+# of multiple frames from the BMP reader!
+++$test;
+print( "PNG in BMP format ...\n" );
+testRead( 'input_PNG.bmp[1]', q//,
+  '88956c2242aab1c8f23d4074ed4f313316413cf79ccf1fea8f650f30bd8e2e42' );
diff -r debb806be563 -r e5a32c8a0135 VisualMagick/installer/inc/version.isx
--- a/VisualMagick/installer/inc/version.isx	Mon Oct 02 19:36:00 2023 +0200
+++ b/VisualMagick/installer/inc/version.isx	Sun Oct 08 12:44:21 2023 -0500
@@ -10,5 +10,5 @@
 
 #define public MagickPackageName "GraphicsMagick"
 #define public MagickPackageVersion "1.4"
-#define public MagickPackageVersionAddendum ".020230928"
-#define public MagickPackageReleaseDate "snapshot-20230928"
+#define public MagickPackageVersionAddendum ".020231008"
+#define public MagickPackageReleaseDate "snapshot-20231008"
diff -r debb806be563 -r e5a32c8a0135 coders/bmp.c
--- a/coders/bmp.c	Mon Oct 02 19:36:00 2023 +0200
+++ b/coders/bmp.c	Sun Oct 08 12:44:21 2023 -0500
@@ -549,68 +549,71 @@
 }
 
 
-static Image *ExtractNestedBlob(Image * image, const ImageInfo * image_info, int ImgType, ExceptionInfo * exception)
+static Image *ExtractNestedBlob(Image ** image, const ImageInfo * image_info, int ImgType, ExceptionInfo * exception)
 {
   size_t
     alloc_size;
+
   unsigned char
     *blob;
 
-  alloc_size = GetBlobSize(image) - TellBlob(image);
+  alloc_size = GetBlobSize(*image) - TellBlob(*image);
 
-  if(alloc_size > 0 &&
-     (blob = MagickAllocateResourceLimitedMemory(unsigned char *,alloc_size)) != NULL)
-  {
+  if (alloc_size > 0 &&
+      (blob = MagickAllocateResourceLimitedMemory(unsigned char *,alloc_size)) != NULL)
+    {
       /* Copy JPG to memory blob */
-     if(ReadBlob(image,alloc_size,blob) == alloc_size)
-     {
-        Image *image2;
-        ImageInfo *clone_info;
+      if (ReadBlob(*image,alloc_size,blob) == alloc_size)
+        {
+          Image *image2;
+          ImageInfo *clone_info;
 
-        clone_info = CloneImageInfo(image_info);
+          clone_info = CloneImageInfo(image_info);
+          (void) strlcpy(clone_info->magick, (ImgType==BI_JPEG)?"JPEG":"PNG", sizeof(clone_info->magick));
 
-              /* BlobToFile("/tmp/jnx-tile.jpg", blob,alloc_size,exception); */
+          /* BlobToFile("/tmp/jnx-tile.jpg", blob,alloc_size,exception); */
 
-         (void) strlcpy(clone_info->filename, (ImgType==BI_JPEG)?"JPEG:":"PNG:", sizeof(clone_info->filename));
-         if ((image2 = BlobToImage(clone_info,blob,alloc_size,exception))
-                  != NULL)
-         {
-                  /*
-                    Replace current image with new image while copying
-                    base image attributes.
-                  */
-            (void) strlcpy(image2->filename, image->filename,
-                                 sizeof(image2->filename));
-            (void) strlcpy(image2->magick_filename, image->magick_filename,
-                                 sizeof(image2->magick_filename));
-            (void) strlcpy(image2->magick, image->magick,
-                                 sizeof(image2->magick));
+          /* (void) strlcpy(clone_info->filename, (ImgType==BI_JPEG)?"JPEG:":"PNG:", sizeof(clone_info->filename)); */
+          FormatString(clone_info->filename,"%sblob-%px", ImgType==BI_JPEG?"JPEG:":"PNG:", blob);
+          if ((image2 = BlobToImage(clone_info,blob,alloc_size,exception))
+              != NULL)
+            {
+              /*
+                Replace current image with new image while copying
+                base image attributes.
+              */
+              (void) strlcpy(image2->filename, (*image)->filename,
+                             sizeof(image2->filename));
+            (void) strlcpy(image2->magick_filename, (*image)->magick_filename,
+                           sizeof(image2->magick_filename));
+            (void) strlcpy(image2->magick, (*image)->magick,
+                           sizeof(image2->magick));
             DestroyBlob(image2);
-           image2->blob = ReferenceBlob(image->blob);
+            image2->blob = ReferenceBlob((*image)->blob);
 
-            if ((image->rows == 0) || (image->columns == 0))
-               DeleteImageFromList(&image);
+            if (((*image)->rows == 0) || ((*image)->columns == 0))
+              DeleteImageFromList(image);
 
-            AppendImageToList(&image, image2);
+            AppendImageToList(image, image2);
          }
-         DestroyImageInfo(clone_info);
-         clone_info = (ImageInfo *) NULL;
-         MagickFreeResourceLimitedMemory(blob);
-      }
+          DestroyImageInfo(clone_info);
+          clone_info = (ImageInfo *) NULL;
+          MagickFreeResourceLimitedMemory(blob);
+        }
       else
-      {
-        MagickFreeResourceLimitedMemory(blob);
-              /* Failed to read enough data from input */
-        ThrowException(exception,CorruptImageError,UnexpectedEndOfFile, image->filename);
-      }
-  }
+        {
+          MagickFreeResourceLimitedMemory(blob);
+          /* Failed to read enough data from input */
+          ThrowException(exception,CorruptImageError,UnexpectedEndOfFile, (*image)->filename);
+        }
+    }
   else
-  {
+    {
       /* Failed to allocate memory */
       ThrowException(exception,ResourceLimitError,MemoryAllocationFailed,
-                     image->filename);
-  }
-  return(image);
+                     (*image)->filename);
+    }
+  return(*image);
 }
 
 
@@ -893,7 +896,7 @@
                                           "  Compression: UNKNOWN (%u)",bmp_info.compression);
               (void) LogMagickEvent(CoderEvent, GetMagickModule(),
                                     "  Number of colors: %u\n"
-                                    "  Important colors: %u",
+                                    "    Important colors: %u",
                                     bmp_info.number_colors, bmp_info.colors_important);
             }
 
@@ -1199,7 +1202,7 @@
           {
             MonitorHandler previous_handler;
             previous_handler = SetMonitorHandler(0);
-            image = ExtractNestedBlob(image, image_info, bmp_info.compression, exception);
+            image = ExtractNestedBlob(&image, image_info, bmp_info.compression, exception);
             (void) SetMonitorHandler(previous_handler);
             if (exception->severity >= ErrorException)
                 ThrowBMPReaderException(CoderError,JPEGCompressionNotSupported,image)
@@ -1218,7 +1221,7 @@
           {
             MonitorHandler previous_handler;
             previous_handler = SetMonitorHandler(0);
-            image = ExtractNestedBlob(image, image_info, bmp_info.compression, exception);
+            image = ExtractNestedBlob(&image, image_info, bmp_info.compression, exception);
             (void) SetMonitorHandler(previous_handler);
             if (exception->severity >= ErrorException)
                 ThrowBMPReaderException(CoderError,PNGCompressionNotSupported,image)
@@ -1949,6 +1952,15 @@
     image=image->previous;
 */
   CloseBlob(image);
+#if 0
+  if (logging)
+    {
+      const size_t list_length = GetImageListLength(image);
+      (void) LogMagickEvent(CoderEvent,GetMagickModule(),
+                            "%lu image%s in list", list_length, list_length > 1 ? "s" : "");
+    }
+#endif
+
   if (logging)
     (void) LogMagickEvent(CoderEvent,GetMagickModule(),"return");
   return(image);
diff -r debb806be563 -r e5a32c8a0135 coders/pnm.c
--- a/coders/pnm.c	Mon Oct 02 19:36:00 2023 +0200
+++ b/coders/pnm.c	Sun Oct 08 12:44:21 2023 -0500
@@ -304,6 +304,43 @@
     XV_332_Format /* P7 332 */
   } PNMSubformat;
 
+static const char *PNMSubformatToString(const PNMSubformat f)
+{
+  const char *s = "unknown";
+
+  switch (f)
+    {
+    case Undefined_PNM_Format:
+      s = "Undefined";
+      break;
+    case PBM_ASCII_Format: /* P1 */
+      s = "PBM ASCII";
+      break;
+    case PGM_ASCII_Format: /* P2 */
+      s = "PGM ASCII";
+      break;
+    case PPM_ASCII_Format: /* P3 */
+      s = "PPM ASCII";
+      break;
+    case PBM_RAW_Format: /* P4 */
+      s = "PBM RAW";
+      break;
+    case PGM_RAW_Format: /* P5 */
+      s = "PGM RAW";
+      break;
+    case PPM_RAW_Format: /* P6 */
+      s = "PPM RAW";
+      break;
+    case PAM_Format: /* P7 */
+      s = "PAM";
+      break;
+    case XV_332_Format: /* P7 332 */
+      s = "XV 332 icon";
+      break;
+    }
+  return s;
+}
+
 #if defined(HAVE_OPENMP)
 #  define PNMReadUseOpenMP 1
 
@@ -721,6 +758,8 @@
             register PixelPacket
               *q;
 
+            (void) LogMagickEvent(CoderEvent,GetMagickModule(),
+                                  "Reading %s pixel data",PNMSubformatToString(format));
             for (y=0; y < (long) image->rows; y++)
               {
                 q=SetImagePixels(image,0,y,image->columns,1);
@@ -772,6 +811,8 @@
               is_grayscale,
               is_monochrome;
 
+            (void) LogMagickEvent(CoderEvent,GetMagickModule(),
+                                  "Reading %s pixel data",PNMSubformatToString(format));
             is_grayscale=MagickTrue;
             is_monochrome=MagickTrue;
             for (y=0; y < (long) image->rows; y++)
@@ -849,6 +890,8 @@
               is_grayscale,
               is_monochrome;
 
+            (void) LogMagickEvent(CoderEvent,GetMagickModule(),
+                                  "Reading %s pixel data",PNMSubformatToString(format));
             is_grayscale=MagickTrue;
             is_monochrome=MagickTrue;
             for (y=0; y < (long) image->rows; y++)
@@ -934,7 +977,8 @@
               pnm_read_threads;
 #endif
 
-            (void) LogMagickEvent(CoderEvent,GetMagickModule(),"Reading PAM");
+            (void) LogMagickEvent(CoderEvent,GetMagickModule(),
+                                  "Reading %s pixel data",PNMSubformatToString(format));
 
             ImportPixelAreaOptionsInit(&import_options);
 
diff -r debb806be563 -r e5a32c8a0135 coders/wpg.c
--- a/coders/wpg.c	Mon Oct 02 19:36:00 2023 +0200
+++ b/coders/wpg.c	Sun Oct 08 12:44:21 2023 -0500
@@ -1604,7 +1604,7 @@
                  ThrowReaderException(CorruptImageError,InvalidColormapIndex,image);
 
               image->colors=WPG_Palette.NumOfEntries;
-              if (!AllocateImageColormap(image,image->colors))
+              if (!AllocateImageColormap(image,image->colors)) /* FIXME: Oss-fuzz 61394, Trashes image->storage_class of previous image, which might be from ExtractPostscript()! */
                 ThrowReaderException(ResourceLimitError,MemoryAllocationFailed,image);
 
               for (i=WPG_Palette.StartIndex;
diff -r debb806be563 -r e5a32c8a0135 magick/pixel_cache.c
--- a/magick/pixel_cache.c	Mon Oct 02 19:36:00 2023 +0200
+++ b/magick/pixel_cache.c	Sun Oct 08 12:44:21 2023 -0500
@@ -1,5 +1,5 @@
 /*
-% Copyright (C) 2003 - 2022 GraphicsMagick Group
+% Copyright (C) 2003 - 2023 GraphicsMagick Group
 % Copyright (C) 2002 ImageMagick Studio
 %
 % This program is covered by multiple licenses, which are described in
@@ -125,6 +125,14 @@
 
 
 /*
+  Indexes are valid if the image storage class is PseudoClass or the
+  colorspace is CMYK.
+*/
+#define PixelCacheImageIndexesValid(image)      \
+  ((image->storage_class == PseudoClass) ||     \
+   (image->colorspace == CMYKColorspace))
+
+/*
   Enum declaractions.
 */
 typedef enum
@@ -918,11 +926,20 @@
     if ((cache_info->reference_count != 1) || (cache_info->read_only))
       fprintf(stderr,"SetCacheNexus: Thread %d enters (cache_info = %p, reference_count=%lu, read_only=%u)\n",
               omp_get_thread_num(),image->cache, cache_info->reference_count, cache_info->read_only);
+#if 0
     if (cache_info->type == UndefinedCache)
       fprintf(stderr,"SetCacheNexus: Pixel cache is not open!\n");
-    if ((image->storage_class != cache_info->storage_class) ||
+#endif
+    if ((cache_info->storage_class != UndefinedClass) &&
+        (image->storage_class != cache_info->storage_class))
+      fprintf(stderr,"SetCacheNexus: Pixel cache storage class mis-match! (image: %s, cache: %s)\n",
+              ClassTypeToString(image->storage_class),ClassTypeToString(cache_info->storage_class));
+#if 0
+    if ((cache_info->colorspace != UndefinedColorspace) &&
         (image->colorspace != cache_info->colorspace))
-      fprintf(stderr,"SetCacheNexus: Pixel cache storage class or colorspace mis-match!\n");
+      fprintf(stderr,"SetCacheNexus: Pixel cache colorspace mis-match! (image: %s, cache: %s)\n",
+              ColorspaceTypeToString(image->colorspace), ColorspaceTypeToString(cache_info->colorspace));
+#endif
   }
 #endif
 
@@ -2446,6 +2463,23 @@
           }
         }
     }
+#if 0
+  /*
+    Trace image-directed changes to cache storage class or color space
+  */
+  if ((cache_info->storage_class != UndefinedClass) &&
+      (image->storage_class != cache_info->storage_class))
+    (void) LogMagickEvent(CacheEvent,GetMagickModule(),
+                          "storage_class %.1024s %s --> %s",
+                          cache_info->filename, ClassTypeToString(cache_info->storage_class),
+                          ClassTypeToString(image->storage_class));
+  if ((cache_info->colorspace != UndefinedColorspace) &&
+      (image->colorspace != cache_info->colorspace))
+    (void) LogMagickEvent(CacheEvent,GetMagickModule(),
+                          "colorspace %.1024s %s --> %s",
+                          cache_info->filename, ColorspaceTypeToString(cache_info->colorspace),
+                          ColorspaceTypeToString(image->colorspace));
+#endif
 
   /*
     Save the open mode.
@@ -2456,8 +2490,7 @@
     Indexes are valid if the image storage class is PseudoClass or the
     colorspace is CMYK.
   */
-  cache_info->indexes_valid=((image->storage_class == PseudoClass) ||
-                             (image->colorspace == CMYKColorspace));
+  cache_info->indexes_valid=PixelCacheImageIndexesValid(image);
 
   if (image->ping)
     {
@@ -2705,9 +2738,9 @@
 %                                                                             %
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 %
-%  AccessImmutableIndexes() returns the colormap indexes associated with
-%  the last call to AcquireImagePixels(). NULL is returned if colormap
-%  indexes are not available.
+%  AccessImmutableIndexes() returns the read-only colormap indexes
+%  associated with the last call to AcquireImagePixels(). NULL is
+%  returned if colormap indexes are not available.
 %
 %  The format of the AccessImmutableIndexes() method is:
 %
@@ -2727,7 +2760,7 @@
 {
   assert(image != (const Image *) NULL);
   assert(image->signature == MagickSignature);
-  return GetCacheViewIndexes(AccessDefaultCacheView(image));
+ return AcquireCacheViewIndexes((ViewInfo *) AccessDefaultCacheView(image));
 }
 
 /*
@@ -2741,11 +2774,11 @@
 %                                                                             %
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 %
-%  AccessMutableIndexes() returns the colormap indexes associated with
-%  the last call to SetImagePixels() or GetImagePixels(). NULL is returned
-%  if colormap indexes are not available.
-%
-%  The format of the AccessMutagleIndexes() method is:
+%  AccessMutableIndexes() returns the writeable colormap indexes associated
+%  with the last call to SetImagePixels() or GetImagePixels(). NULL is
+%  returned if colormap indexes are not available.
+%
+%  The format of the AccessMutableIndexes() method is:
 %
 %      IndexPacket *AccessMutableIndexes(Image *image)