Merge pull request #2241 from wiredfool/sunrle

SunImagePlugin fixes

.travis.yml

@@ -42,6 +42,10 @@ install:
   # libimagequant
   - pushd depends && ./install_imagequant.sh && popd
   
+  # extra test images
+  - pushd depends && ./install_extra_test_images.sh && popd  
+  
+
 before_script:
 # Qt needs a display for some of the tests, and it's only run on the system site packages install
   - "export DISPLAY=:99.0"

PIL/ImageFile.py

@@ -150,15 +150,16 @@ class ImageFile(Image.Image):
 
         if use_mmap:
             # try memory mapping
-            d, e, o, a = self.tile[0]
+            decoder_name, extents, offset, args = self.tile[0]
-            if d == "raw" and a[0] == self.mode and a[0] in Image._MAPMODES:
+            if decoder_name == "raw" and len(args) >= 3 and args[0] == self.mode \
+                   and args[0] in Image._MAPMODES:
                 try:
                     if hasattr(Image.core, "map"):
                         # use built-in mapper  WIN32 only
                         self.map = Image.core.map(self.filename)
-                        self.map.seek(o)
+                        self.map.seek(offset)
                         self.im = self.map.readimage(
-                            self.mode, self.size, a[1], a[2]
+                            self.mode, self.size, args[1], args[2]
                             )
                     else:
                         # use mmap, if possible
@@ -167,7 +168,7 @@ class ImageFile(Image.Image):
                         size = os.path.getsize(self.filename)
                         self.map = mmap.mmap(fp.fileno(), size, access=mmap.ACCESS_READ)
                         self.im = Image.core.map_buffer(
-                            self.map, self.size, d, e, o, a
+                            self.map, self.size, decoder_name, extents, offset, args
                             )
                     readonly = 1
                     # After trashing self.im, we might need to reload the palette data.

PIL/SunImagePlugin.py

@@ -38,6 +38,21 @@ class SunImageFile(ImageFile.ImageFile):
 
     def _open(self):
 
+        # The Sun Raster file header is 32 bytes in length and has the following format:
+
+        #     typedef struct _SunRaster
+        #     {
+        #         DWORD MagicNumber;      /* Magic (identification) number */
+        #         DWORD Width;            /* Width of image in pixels */
+        #         DWORD Height;           /* Height of image in pixels */
+        #         DWORD Depth;            /* Number of bits per pixel */
+        #         DWORD Length;           /* Size of image data in bytes */
+        #         DWORD Type;             /* Type of raster file */
+        #         DWORD ColorMapType;     /* Type of color map */
+        #         DWORD ColorMapLength;   /* Size of the color map in bytes */
+        #     } SUNRASTER;
+
+
         # HEAD
         s = self.fp.read(32)
         if i32(s) != 0x59a66a95:
@@ -48,30 +63,70 @@ class SunImageFile(ImageFile.ImageFile):
         self.size = i32(s[4:8]), i32(s[8:12])
 
         depth = i32(s[12:16])
+        data_length = i32(s[16:20])  # unreliable, ignore. 
+        file_type = i32(s[20:24])
+        palette_type = i32(s[24:28]) # 0: None, 1: RGB, 2: Raw/arbitrary
+        palette_length = i32(s[28:32])
+        
         if depth == 1:
             self.mode, rawmode = "1", "1;I"
+        elif depth == 4:
+            self.mode, rawmode = "L", "L;4"
         elif depth == 8:
             self.mode = rawmode = "L"
         elif depth == 24:
-            self.mode, rawmode = "RGB", "BGR"
+            if file_type == 3:
+                self.mode, rawmode = "RGB", "RGB"
             else:
-            raise SyntaxError("unsupported mode")
+                self.mode, rawmode = "RGB", "BGR"
+        elif depth == 32:
+            if file_type == 3:
+                self.mode, rawmode = 'RGB', 'RGBX'
+            else:
+                self.mode, rawmode = 'RGB', 'BGRX'
+        else:
+            raise SyntaxError("Unsupported Mode/Bit Depth")    
         
-        compression = i32(s[20:24])
+        if palette_length:
+            if palette_length > 1024:
+                raise SyntaxError("Unsupported Color Palette Length")
 
-        if i32(s[24:28]) != 0:
+            if palette_type != 1:
-            length = i32(s[28:32])
+                raise SyntaxError("Unsupported Palette Type")
-            offset = offset + length
+            
-            self.palette = ImagePalette.raw("RGB;L", self.fp.read(length))
+            offset = offset + palette_length
+            self.palette = ImagePalette.raw("RGB;L", self.fp.read(palette_length))
             if self.mode == "L":
-                self.mode = rawmode = "P"
+                self.mode = "P"
+                rawmode = rawmode.replace('L', 'P')
             
-        stride = (((self.size[0] * depth + 7) // 8) + 3) & (~3)
+        # 16 bit boundaries on stride
+        stride = ((self.size[0] * depth + 15) // 16) * 2  
 
-        if compression == 1:
+        # file type: Type is the version (or flavor) of the bitmap
+        # file. The following values are typically found in the Type
+        # field:
+        # 0000h	Old
+        # 0001h	Standard
+        # 0002h	Byte-encoded
+        # 0003h	RGB format
+        # 0004h	TIFF format
+        # 0005h	IFF format
+        # FFFFh	Experimental
+
+        # Old and standard are the same, except for the length tag.
+        # byte-encoded is run-length-encoded
+        # RGB looks similar to standard, but RGB byte order
+        # TIFF and IFF mean that they were converted from T/IFF
+        # Experimental means that it's something else.
+        # (http://www.fileformat.info/format/sunraster/egff.htm)
+
+        if file_type in (0, 1, 3, 4, 5):
             self.tile = [("raw", (0, 0)+self.size, offset, (rawmode, stride))]
-        elif compression == 2:
+        elif file_type == 2:
             self.tile = [("sun_rle", (0, 0)+self.size, offset, rawmode)]
+        else:
+            raise SyntaxError('Unsupported Sun Raster file type')
         
 #
 # registry

Tests/test_file_sun.py

@@ -1,7 +1,10 @@
-from helper import unittest, PillowTestCase
+from helper import unittest, PillowTestCase, hopper
 
 from PIL import Image, SunImagePlugin
 
+import os
+
+EXTRA_DIR = 'Tests/images/sunraster'
 
 class TestFileSun(PillowTestCase):
 
@@ -16,10 +19,32 @@ class TestFileSun(PillowTestCase):
         # Assert
         self.assertEqual(im.size, (128, 128))
 
+        self.assert_image_similar(im, hopper(), 5) # visually verified
+        
         invalid_file = "Tests/images/flower.jpg"
         self.assertRaises(SyntaxError,
                           lambda: SunImagePlugin.SunImageFile(invalid_file))
 
+    def test_im1(self):
+        im = Image.open('Tests/images/sunraster.im1')
+        target = Image.open('Tests/images/sunraster.im1.png')
+        self.assert_image_equal(im, target)
+
+
+    @unittest.skipIf(not os.path.exists(EXTRA_DIR),
+                     "Extra image files not installed")
+    def test_others(self):
+        files = (os.path.join(EXTRA_DIR, f) for f in
+                   os.listdir(EXTRA_DIR) if os.path.splitext(f)[1]
+                   in ('.sun', '.SUN', '.ras'))
+        for path in files:
+            with Image.open(path) as im:
+                im.load()  
+                self.assertIsInstance(im, SunImagePlugin.SunImageFile)
+                target_path = "%s.png" % os.path.splitext(path)[0]
+                #im.save(target_file)
+                with Image.open(target_path) as target:
+                    self.assert_image_equal(im, target)
         
 if __name__ == '__main__':
     unittest.main()

appveyor.yml

@@ -19,6 +19,7 @@ install:
 - git clone https://github.com/python-pillow/pillow-depends.git c:\pillow-depends
 - xcopy c:\pillow-depends\*.zip c:\pillow\winbuild\
 - xcopy c:\pillow-depends\*.tar.gz c:\pillow\winbuild\
+- xcopy /s c:\pillow-depends\test_images\* c:\pillow\tests\images
 - cd c:\pillow\winbuild\
 - c:\python34\python.exe c:\pillow\winbuild\build_dep.py
 - c:\pillow\winbuild\build_deps.cmd

depends/install_extra_test_images.sh

@@ -0,0 +1,11 @@
+#!/bin/bash
+# install extra test images
+
+if [ ! -f test_images.tar.gz ]; then
+    wget -O 'test_images.tar.gz' 'https://github.com/python-pillow/pillow-depends/blob/master/test_images.tar.gz?raw=true'
+fi
+
+rm -r test_images
+tar -xvzf test_images.tar.gz
+
+cp -r test_images/* ../Tests/images

libImaging/SunRleDecode.c

@@ -24,6 +24,8 @@ ImagingSunRleDecode(Imaging im, ImagingCodecState state, UINT8* buf, int bytes)
 {
     int n;
     UINT8* ptr;
+    UINT8 extra_data = 0;
+    UINT8 extra_bytes = 0;
 
     ptr = buf;
 
@@ -39,6 +41,7 @@ ImagingSunRleDecode(Imaging im, ImagingCodecState state, UINT8* buf, int bytes)
 
             n = ptr[1];
 
+
             if (n == 0) {
 
                 /* Literal 0x80 (2 bytes) */
@@ -55,10 +58,33 @@ ImagingSunRleDecode(Imaging im, ImagingCodecState state, UINT8* buf, int bytes)
                 if (bytes < 3)
                     break;
 
+                /* from (http://www.fileformat.info/format/sunraster/egff.htm)
+
+                   For example, a run of 100 pixels with the value of
+                   0Ah would encode as the values 80h 64h 0Ah. A
+                   single pixel value of 80h would encode as the
+                   values 80h 00h. The four unencoded bytes 12345678h
+                   would be stored in the RLE stream as 12h 34h 56h
+                   78h.  100 pixels, n=100, not 100 pixels, n=99.
+
+                   But Wait! There's More! 
+                   (http://www.fileformat.info/format/sunraster/spec/598a59c4fac64c52897585d390d86360/view.htm)
+
+                   If the first byte is 0x80, and the second byte is
+                   not zero, the record is three bytes long. The
+                   second byte is a count and the third byte is a
+                   value. Output (count+1) pixels of that value.
+
+                   2 specs, same site, but Imagemagick and GIMP seem
+                   to agree on the second one.
+                */
+                n += 1; 
+
                 if (state->x + n > state->bytes) {
-		    /* FIXME: is this correct? */
+                    extra_bytes = n;  /* full value */
-		    state->errcode = IMAGING_CODEC_OVERRUN;
+                    n = state->bytes - state->x;
-		    return -1;
+                    extra_bytes -= n;
+                    extra_data = ptr[2];
                 }
 
                 memset(state->buffer + state->x, ptr[2], n);
@@ -70,25 +96,17 @@ ImagingSunRleDecode(Imaging im, ImagingCodecState state, UINT8* buf, int bytes)
 
         } else {
 
-	    /* Literal (1+n bytes block) */
+            /* Literal byte */ 
-	    n = ptr[0];
+            n = 1; 
 
-	    if (bytes < 1 + n)
+            state->buffer[state->x] = ptr[0];
-		break;
 
-	    if (state->x + n > state->bytes) {
+            ptr += 1;
-		/* FIXME: is this correct? */
+            bytes -= 1;
-		state->errcode = IMAGING_CODEC_OVERRUN;
-		return -1;
-	    }
-
-	    memcpy(state->buffer + state->x, ptr + 1, n);
-
-	    ptr += 1 + n;
-	    bytes -= 1 + n;
 
         }
 
+        for (;;) {
             state->x += n;
         
             if (state->x >= state->bytes) {
@@ -106,6 +124,22 @@ ImagingSunRleDecode(Imaging im, ImagingCodecState state, UINT8* buf, int bytes)
                 }
             }
             
+            if (extra_bytes == 0) {
+                break;
+            }
+
+            if (state->x > 0) {
+                break; // assert
+            }
+
+            if (extra_bytes >= state->bytes) {
+                n = state->bytes;
+            } else {
+                n = extra_bytes;
+            }
+            memset(state->buffer + state->x, extra_data, n);
+            extra_bytes -= n;
+        }
     }
 
     return ptr - buf;