failing "MAT (MatLab RGB 8-bit LSB integer) ..." test

[Petr]

Hi,

6.7.8-8 is failing for me when MALLOC_PERTURB_ is set at least for some nonzero values. See following code derived from this test:

Code: Select all

----- read-compare.pl -----
use Image::Magick;

print("MAT (MatLab RGB 8-bit LSB integer) ...\n");
$srcimage_name = 'input_rgb_lsb_08bit.mat';
$refimage_name = 'reference/read/input_rgb_lsb_08bit_mat.miff';

$srcimage=Image::Magick->new;
$srcimage->ReadImage("$srcimage_name");
$refimage=Image::Magick->new;
$refimage->ReadImage("$refimage_name");

$srcimage->Difference($refimage);

$normalized_mean_error=$srcimage->GetAttribute('mean-error');
print("mean-error: $normalized_mean_error\n");

Mean error seems to depend on MALLOC_PERTURB_ value:

Code: Select all

$ export MALLOC_PERTURB_=0
$ perl read-compare.pl
MAT (MatLab RGB 8-bit LSB integer) ...
mean-error: 0.0354644260035022
$ export MALLOC_PERTURB_=69
$ perl read-compare.pl
MAT (MatLab RGB 8-bit LSB integer) ...
mean-error: 0.24184922359772
$ export MALLOC_PERTURB_=1
$ perl read-compare.pl
MAT (MatLab RGB 8-bit LSB integer) ...
mean-error: 0.494382340266855
$ export MALLOC_PERTURB_=71
$ perl read-compare.pl
MAT (MatLab RGB 8-bit LSB integer) ...
mean-error: 0.235857124369786

[Petr]

valgrind output:

Code: Select all

[...]
==24898== Conditional jump or move depends on uninitialised value(s)
==24898==    at 0x634C670: ScaleAnyToQuantum (quantum-private.h:277)
==24898==    by 0x634EE61: ImportBlueQuantum (quantum-import.c:1107)
==24898==    by 0x6355DE6: ImportQuantumPixels (quantum-import.c:3416)
==24898==    by 0x819F22E: ReadMATImage (mat.c:926)
==24898==    by 0x622B1A6: ReadImage (constitute.c:535)
==24898==    by 0x622C81A: ReadImages (constitute.c:901)
==24898==    by 0x5F48D01: XS_Image__Magick_Read (Magick.xs:13041)
==24898==    by 0x4A6F51: Perl_pp_entersub (in /usr/bin/perl)
==24898==    by 0x49F715: Perl_runops_standard (in /usr/bin/perl)
==24898==    by 0x43CBB1: perl_run (in /usr/bin/perl)
==24898==    by 0x41EC5A: main (in /usr/bin/perl)
[...]
==24898== Conditional jump or move depends on uninitialised value(s)
==24898==    at 0x6215B14: IsImagesEqual (compare.c:1650)
==24898==    by 0x5F636E3: XS_Image__Magick_Mogrify (Magick.xs:9535)
==24898==    by 0x4A6F51: Perl_pp_entersub (in /usr/bin/perl)
==24898==    by 0x49F715: Perl_runops_standard (in /usr/bin/perl)
==24898==    by 0x43CBB1: perl_run (in /usr/bin/perl)
==24898==    by 0x41EC5A: main (in /usr/bin/perl)
[...]

[Petr]

Don't know so far if this is relevant:

Code: Select all

$ export $MALLOC_PERTURB_=69
[...]
Breakpoint 1, IsImagesEqual (image=0x8ce730, reconstruct_image=0x8c3030) at magick/compare.c:1607
1607	  for (y=0; y < (ssize_t) image->rows; y++)
(gdb) n
1620	    p=GetCacheViewVirtualPixels(image_view,0,y,image->columns,1,exception);
(gdb) n
1621	    q=GetCacheViewVirtualPixels(reconstruct_view,0,y,reconstruct_image->columns,
(gdb) n
1623	    if ((p == (const PixelPacket *) NULL) || (q == (const PixelPacket *) NULL))
(gdb) p *p
$1 = {blue = 47802, green = 11822, red = 10794, opacity = 47802}
(gdb) p *q
$2 = {blue = 17219, green = 11822, red = 10794, opacity = 0}

Code: Select all

$ export $MALLOC_PERTURB_=71
[...]
Breakpoint 1, IsImagesEqual (image=0x8cd290, reconstruct_image=0x8beb00) at magick/compare.c:1607
1607	  for (y=0; y < (ssize_t) image->rows; y++)
(gdb) n
1620	    p=GetCacheViewVirtualPixels(image_view,0,y,image->columns,1,exception);
(gdb) n
1621	    q=GetCacheViewVirtualPixels(reconstruct_view,0,y,reconstruct_image->columns,
(gdb) n
1623	    if ((p == (const PixelPacket *) NULL) || (q == (const PixelPacket *) NULL))
(gdb) p *p
$29 = {blue = 47288, green = 11822, red = 10794, opacity = 47288}
(gdb) p *q
$30 = {blue = 17219, green = 11822, red = 10794, opacity = 0}

[magick]

We can reproduce the problem and will have a patch for it in the next point release of ImageMagick. Thanks.

[Petr]

Thank you!

[broucaries]

can you post also this patch?

[magick]

Here are the patches:

Code: Select all

--- ImageMagick-6.8.1~/coders/mat.c     2013-01-15 07:47:35.012032065 -0500
+++ ImageMagick-6.8.1/coders/mat.c      2013-01-15 07:48:04.631798260 -0500
@@ -887,7 +887,7 @@
     {
       for (i = 0; i < (ssize_t) MATLAB_HDR.SizeY; i++)
       {
-        q=QueueAuthenticPixels(image,0,MATLAB_HDR.SizeY-i-1,image->columns,1,ex
ception);
+        q=GetAuthenticPixels(image,0,MATLAB_HDR.SizeY-i-1,image->columns,1,exce
ption);
         if (q == (PixelPacket *)NULL)
   {
     if (logging) (void)LogMagickEvent(CoderEvent,GetMagickModule(),

Code: Select all

--- ImageMagick-6.8.1~/magick/quantum-import.c  2013-01-15 07:47:51.836171241 -0500
+++ ImageMagick-6.8.1/magick/quantum-import.c   2013-01-15 07:49:25.611094994 -0500
@@ -1032,62 +1032,86 @@
 
   switch (quantum_info->depth)
   {
-    case 10:
+    case 8:
     {
-      Quantum
-        cbcr[4];
+      unsigned char
+        pixel;
 
-      pixel=0;
-      if (quantum_info->pack == MagickFalse)
-        {
-          register ssize_t
-            i;
+      for (x=0; x < (ssize_t) number_pixels; x++)
+      {
+        p=PushCharPixel(p,&pixel);
+        SetPixelBlue(q,ScaleCharToQuantum(pixel));
+        p+=quantum_info->pad;
+        q++;
+      }
+      break;
+    }
+    case 16:
+    {
+      unsigned short
+        pixel;
 
-          size_t
-            quantum;
+      if (quantum_info->format == FloatingPointQuantumFormat)
+        {
+          for (x=0; x < (ssize_t) number_pixels; x++)
+          {
+            p=PushShortPixel(quantum_info->endian,p,&pixel);
+            SetPixelBlue(q,ClampToQuantum((MagickRealType)
+              QuantumRange*HalfToSinglePrecision(pixel)));
+            p+=quantum_info->pad;
+            q++;
+          }
+          break;
+        }
+      for (x=0; x < (ssize_t) number_pixels; x++)
+      {
+        p=PushShortPixel(quantum_info->endian,p,&pixel);
+        SetPixelBlue(q,ScaleShortToQuantum(pixel));
+        p+=quantum_info->pad;
+        q++;
+      }
+      break;
+    }
+    case 32:
+    {
+      unsigned int
+        pixel;
 
-          ssize_t
-            n;
+      if (quantum_info->format == FloatingPointQuantumFormat)
+        {
+          float
+            pixel;
 
-          n=0;
-          quantum=0;
-          for (x=0; x < (ssize_t) number_pixels; x+=2)
+          for (x=0; x < (ssize_t) number_pixels; x++)
           {
-            for (i=0; i < 4; i++)
-            {
-              switch (n % 3)
-              {
-                case 0:
-                {
-                  p=PushLongPixel(quantum_info->endian,p,&pixel);
-                  quantum=(size_t) (ScaleShortToQuantum((unsigned short)
-                    (((pixel >> 22) & 0x3ff) << 6)));
-                  break;
-                }
-                case 1:
-                {
-                  quantum=(size_t) (ScaleShortToQuantum((unsigned short)
-                    (((pixel >> 12) & 0x3ff) << 6)));
-                  break;
-                }
-                case 2:
-                {
-                  quantum=(size_t) (ScaleShortToQuantum((unsigned short)
-                    (((pixel >> 2) & 0x3ff) << 6)));
-                  break;
-                }
-              }
-              cbcr[i]=(Quantum) (quantum);
-              n++;
-            }
+            p=PushFloatPixel(quantum_info,p,&pixel);
+            SetPixelBlue(q,ClampToQuantum(pixel));
             p+=quantum_info->pad;
-            SetPixelRed(q,cbcr[1]);
-            SetPixelGreen(q,cbcr[0]);
-            SetPixelBlue(q,cbcr[2]);
             q++;
-            SetPixelRed(q,cbcr[3]);
-            SetPixelGreen(q,cbcr[0]);
-            SetPixelBlue(q,cbcr[2]);
+          }
+          break;
+        }
+      for (x=0; x < (ssize_t) number_pixels; x++)
+      {
+        p=PushLongPixel(quantum_info->endian,p,&pixel);
+        SetPixelBlue(q,ScaleLongToQuantum(pixel));
+        p+=quantum_info->pad;
+        q++;
+      }
+      break;
+    }
+    case 64:
+    {
+      if (quantum_info->format == FloatingPointQuantumFormat)
+        {
+          double
+            pixel;
+
+          for (x=0; x < (ssize_t) number_pixels; x++)
+          {
+            p=PushDoublePixel(quantum_info,p,&pixel);
+            SetPixelBlue(q,ClampToQuantum(pixel));
+            p+=quantum_info->pad;
             q++;
           }
           break;
@@ -1102,9 +1126,8 @@
       for (x=0; x < (ssize_t) number_pixels; x++)
       {
         p=PushQuantumPixel(quantum_info,p,&pixel);
-        SetPixelRed(q,ScaleAnyToQuantum(pixel,range));
-        p=PushQuantumPixel(quantum_info,p,&pixel);
-        SetPixelGreen(q,ScaleAnyToQuantum(pixel,range));
+        SetPixelBlue(q,ScaleAnyToQuantum(pixel,range));
+        p+=quantum_info->pad;
         q++;
       }
       break;

[Petr]

That indeed helped, thanks.