Fix 32-bit BMP loading (RGBA or RGBX) #1125

artscoop · 2015-03-04T17:19:25Z

PIL choked on perfectly valid BMP files (32 bits with Alpha). It could not handle valid RGBA masks to determine the raw format.
To clarify things, I:

Rewrote the BmpImagePlugin.BmpImageFile class to be far more readable
Made error messages more explicit (e.g. say that RLE bitmaps are unsupported)
Made a readable dict to contain BMP header information
Kept the existing security checks
Instead of reading palette info by chunks of 3/4 bytes, read the whole palette info at once and parse the data.
Now works with BMPv4/5 with Alpha (and can be exported to alpha PNG for example)
Tested load and save with RGB24, RGB8, RGB8L, RGB32 and RGBA32.
Tested with one bogus file. File not accepted, as expected.
Size of header attributes is made explicit in this code

I wanted to test more BMP formats, but I could not find that many images.
But for all the types I tested, it worked flawlessly.

PIL choked on perfectly valid BMP files (32 bits with Alpha). It could not handle valid RGBA masks to determine the raw format. To clarify things, I: - Rewrote the `BmpImagePlugin.BmpImageFile` class to be far more readable - Made error messages more explicit (e.g. say that RLE bitmaps are unsupported) - Made a readable dict to contain BMP header information - Kept the existing security checks - Instead of reading palette info by chunks of 3/4 bytes, read the whole palette info at once and parse the data. - Now works with BMPv4/5 with Alpha (and can be exported to alpha PNG for example) - Tested load and save with RGB24, RGB8, RGB8L, RGB32 and RGBA32. - Tested with one bogus file. File not accepted, as expected. I wanted to test more BMP formats, but I could not find that many images. But for all the types I tested, it worked flawlessly.

homm · 2015-03-04T17:25:46Z

PIL/BmpImagePlugin.py

 }


 def _save(im, fp, filename, check=0):
    try:
+        print im.mode


debug print

Oh my goodness. :/

Debug print left in _save. Removed.

artscoop · 2015-03-04T17:40:42Z

Seems like a test with a BMP image has failed (Tests/images/bmp/g/pal1wb.bmp, too different from expected result). Investigating.

It appears that {{{ The handling of 1bpp bitmaps is a little complicated. When reading 1bpp bitmaps, the palette is ignored. 1's are considered foreground, and they are considered black. 0's are considered background, and they are considered white. }}} so the raw mode has to be `1;I`

Use the provided image offset if there is palette data while the image is bitfielded.

Until tests pass.

Fixed loading of all types of provided images (+rgba). Added edge case where the header is reported as 40 bytes long with BITFIELDS (they start past the 40 bytes of the header). Loading fails for RLE, but IIRC, they're unsupported so it's normal.

Choked on roundtrip, where a P;1 image was returned instead of a 1 image.

...

Readapted some original code.

Getting bonkers but I need to know

I suspect the tests to check against an exact string when expecting an error

use offset provided

Various fixes on code broken or not passing tests

Again.

Some .cur file with alpha was loaded fully opaque with PIL. Fixed, and fixed the test to take that into account.

Fails on Python 3, tried some fixes before going the virtualenv3 route

Error happening in Python 3.x with P images: in original code, palette data was created from a list of bytestrings. Changed to a full bytestring. - `b"".join(list of bytestrings)` works in python 2.7 and 3.x - `b"".join(bytestring)` works in python 2.7 but fails in python 3.x No need to `join` anymore. Works in 3.x

wiredfool · 2015-03-06T21:19:36Z

There are a ton of BMP images in the test suite, Tests/images/bmp/*. G == good, q== questionable, b==bad. Look at tests/test_bmp_reference.py (which you've probably seen, from the test failures).

artscoop · 2015-03-07T00:03:16Z

Exactly. At first, I did not know how to test a thorough set of images. Then I found the images, understood how Travis CI works, and then I had a very hard time catering for every test (I even had to fix a test, since before the fix, PIL could not detect the A channel of a valid 32 bit RGBA .cur file and expected this behaviour)

Does not change testing on other files, but fixes a case which previously made PIL collapse. The Bitmap was a 1x1 RGBA and provoked an exception in PIL, but every Image viewer can load it. Fixed code with comparison of header size, compression type and loading type of masks and fixed it.

aclark4life · 2015-04-01T01:31:03Z

Is this ready for 2.8.0?

homm · 2015-04-01T16:23:18Z

PIL/BmpImagePlugin.py

-    format = "BMP"
+    """ Image plugin for the Windows Bitmap format (BMP) """
+
+    #--------------------------------------------------------------- Description


Very strange comments style

Oh yes, sorry, gonna fix it very soon. This is the comment style automatically used by Eclipse, and it did fit me until very recently, when I started using flake8 to check coding style.

Fix 32-bit BMP loading (RGBA or RGBX)

aclark4life · 2015-04-01T19:23:39Z

Thanks, I'll fix the comments

techtonik · 2015-12-30T07:09:55Z

@artscoop can you investigate the regression in #1293 that is supposedly caused by this commit. You seem to be the best person to understand what is wrong there.

artscoop · 2015-12-30T07:11:41Z

Yes, will have a look at it today.

techtonik · 2015-12-30T10:01:42Z

@artscoop thanks! ;) 💃

artscoop · 2015-12-30T17:39:28Z

@techtonik , can you check if the version of Pillow in the https://github.com/artscoop/Pillow repo fixes or mitigates your problem ?

techtonik · 2015-12-30T18:03:03Z

@artscoop I can only check this change https://github.com/artscoop/Pillow/commit/6216d079f5bac0a7f13a43b76bf276542c1abd74 because I don't have Visual Studio to compile the whole extension. Let me check.

techtonik · 2015-12-30T18:11:03Z

Didn't help. The error message is the same:

Traceback (most recent call last):
  File "C:\Bin\saveclip.py", line 31, in <module>
    im = ImageGrab.grabclipboard()
  File "C:\Python27\lib\site-packages\PIL\ImageGrab.py", line 66, in grabclipboard
    return BmpImagePlugin.DibImageFile(io.BytesIO(data))
  File "C:\Python27\lib\site-packages\PIL\ImageFile.py", line 100, in __init__
    self._open()
  File "C:\Python27\lib\site-packages\PIL\BmpImagePlugin.py", line 208, in _open
    self._bitmap()
  File "C:\Python27\lib\site-packages\PIL\BmpImagePlugin.py", line 148, in _bitmap
    raise IOError("Unsupported BMP bitfields layout")
IOError: Unsupported BMP bitfields layout

and saveclip.py is:

#!/usr/bin/env python
#
# the code is in public domain
#
# history:
#   1.1  - mention what library should be installed
#   1.0  - make get_avail_filename() function

__version__ = '1.1'

import os, sys
try:
    from PIL import ImageGrab
except ImportError:
    # https://github.com/python-pillow/Pillow/issues/1293
    raise ImportError("Pillow is not installed (use Pillow==2.7.0)")

def get_avail_filename(basename, suffix='%0.2d.png'):
    """Find first non-existing filename with given numerical suffix"""
    counter = 1
    while True:
        newname = basename + suffix % counter
        if not os.path.exists(newname):
            return newname
        counter += 1

filename = 'clipboard.png'
if os.path.exists(filename):
  filename = get_avail_filename('clipboard', suffix='%0.2d.png')

im = ImageGrab.grabclipboard()
if im is None:
  sys.exit('Error: No image data in clipboard')
else:
  im.save(filename, 'PNG')
  print('Saved as %s' % filename)

techtonik · 2015-12-30T18:14:10Z

Uncommented line 147 just before the raise IOError("Unsupported BMP bitfields layout"), which is:

print(file_info['bits'], [hex(value) for value in file_info['rgb_mask']])

It gives this info: (32, ['0xff0000', '0xff00', '0xff'])

artscoop · 2015-12-30T18:21:45Z

I was just going to ask you to uncomment that line :)
This is exactly what I wanted to know.
I pushed an edit with a new mask added. Could you try with the latest master ? (you can just copy BMPImagePlugin.py from my repository)

techtonik · 2015-12-30T18:52:23Z

The error exception with the latest code is strange:

...
  File "C:\Python27\lib\site-packages\PIL\ImageGrab.py", line 66, in grabclipboard
    return BmpImagePlugin.DibImageFile(io.BytesIO(data))
  File "C:\Python27\lib\site-packages\PIL\ImageFile.py", line 107, in __init__
    raise SyntaxError(v)
SyntaxError: (32, (16711680, 65280, 255, 4279440405L))

I reverted and changed the line to dump rgba_mask and got this:

(32, ['0xff0000', '0xff00', '0xff', '0xff131415L'])

hugovk · 2015-12-30T18:55:33Z

(Top tip: if there's unit tests for this, you can enable AppVeyor for your repo and it'll build and test it on Windows. And good to do so on Travis CI, for Linux. They're free. And they will also help guard against any other regressions.)

techtonik · 2015-12-30T18:58:53Z

@hugovk yes, but it is not clear what unit test is for. Right now the data is in Windows clipboard. I can dump the binary for unit test if somebody tells me how.

techtonik · 2015-12-30T18:59:20Z

Tracing with pdb..

140                 import pdb; pdb.set_trace()
141                 if file_info['bits'] in SUPPORTED:
142                     if file_info['bits'] == 32 and (file_info['rgba_mask'] in SUPPORTED[file_info['bits']]) or (file_info['rgb_mask'] in SUPPORTED
[file_info['bits']]):
143  ->                     raw_mode = MASK_MODES[(file_info['bits'], file_info['rgba_mask'])]
144                         self.mode = "RGBA" if raw_mode in ("BGRA",) else self.mode
145                     elif file_info['bits'] in (24, 16) and file_info['rgb_mask'] in SUPPORTED[file_info['bits']]:
146                         raw_mode = MASK_MODES[(file_info['bits'], file_info['rgb_mask'])]
147                     else:
148                         print(file_info['bits'], [hex(value) for value in file_info['rgb_mask']])
(Pdb) file_info['bits']
32
(Pdb) file_info['rgba_mask']
(16711680, 65280, 255, 4279440405L)
(Pdb) n
KeyError: ((32, (16711680, 65280, 255, 4279440405L)),)
> c:\python27\lib\site-packages\pil\bmpimageplugin.py(209)_open()
-> self._bitmap()
(Pdb)

techtonik · 2015-12-30T19:02:12Z

And here comes the SyntaxError:

> c:\python27\lib\site-packages\pil\imagefile.py(101)__init__()
-> except (IndexError,  # end of data
(Pdb) n
> c:\python27\lib\site-packages\pil\imagefile.py(102)__init__()
-> TypeError,  # end of data (ord)
(Pdb) n
> c:\python27\lib\site-packages\pil\imagefile.py(103)__init__()
-> KeyError,  # unsupported mode
(Pdb) n
> c:\python27\lib\site-packages\pil\imagefile.py(104)__init__()
-> EOFError,  # got header but not the first frame
(Pdb) n
> c:\python27\lib\site-packages\pil\imagefile.py(105)__init__()
-> struct.error) as v:
(Pdb) n
> c:\python27\lib\site-packages\pil\imagefile.py(106)__init__()
-> logger.exception("%s")
(Pdb) n
No handlers could be found for logger "PIL.ImageFile"
> c:\python27\lib\site-packages\pil\imagefile.py(107)__init__()
-> raise SyntaxError(v)

techtonik · 2015-12-30T19:23:08Z

Here is the binary data from self.fp
dumpfp.zip

MXS2514 · 2016-07-06T04:56:42Z

Hello, the latest BmpImagePlugin.py seems can't work with me Test32bitbmp
When checking mode it is still RGB not expected RGBA
However, after googling this worked, both reading and saving to 32-bit png.

wiredfool · 2016-07-06T07:54:00Z

From the code:

     # 40 byte headers only have the three components in the bitfields masks,
     # ref: https://msdn.microsoft.com/en-us/library/windows/desktop/dd183376(v=vs.85).aspx
     # See also https://github.com/python-pillow/Pillow/issues/1293
     # There is a 4th component in the RGBQuad, in the alpha location, but it
     # is listed as a reserved component, and it is not generally an alpha channel

…ince python-pillow#1125)

homm reviewed Mar 4, 2015
View reviewed changes

Removed debug print

c0ce8d0

Debug print left in _save. Removed.

artscoop added 6 commits March 4, 2015 19:26

Remember correct image offset if bitfields+palette

497ddf9

Use the provided image offset if there is palette data while the image is bitfielded.

Quickfix number 4

c855177

Until tests pass.

Fix for all good_tests

7b657f6

Fixed loading of all types of provided images (+rgba). Added edge case where the header is reported as 40 bytes long with BITFIELDS (they start past the 40 bytes of the header). Loading fails for RLE, but IIRC, they're unsupported so it's normal.

Try to fix tests

f953b98

Choked on roundtrip, where a P;1 image was returned instead of a 1 image.

Test with original code

56439b7

...

artscoop closed this Mar 5, 2015

artscoop reopened this Mar 5, 2015

artscoop added 9 commits March 5, 2015 08:19

Use 1 and not P for basic 1bpp BMP

9346514

Readapted some original code.

Update BmpImagePlugin.py

c8fddb1

Getting bonkers but I need to know

Restored original error messages

82aa901

I suspect the tests to check against an exact string when expecting an error

Fixing .cur special header offset

d7a78f3

use offset provided

Fix .cur 32/rgba, offsets etc.

9e3af5a

Various fixes on code broken or not passing tests

Removed debug print

613d22f

Again.

Test fix for supported 32bit RGBA .cur file

514c55a

Some .cur file with alpha was loaded fully opaque with PIL. Fixed, and fixed the test to take that into account.

Update BmpImagePlugin.py

3106845

Fails on Python 3, tried some fixes before going the virtualenv3 route

aclark4life added the Needs Review label Apr 1, 2015

aclark4life added this to the 2.8.0 milestone Apr 1, 2015

aclark4life mentioned this pull request Apr 1, 2015

Release 2.8.0 on April 1, 2015 #1124

Closed

homm reviewed Apr 1, 2015
View reviewed changes

aclark4life added a commit that referenced this pull request Apr 1, 2015

Merge pull request #1125 from artscoop/patch-1

ea65087

Fix 32-bit BMP loading (RGBA or RGBX)

aclark4life merged commit ea65087 into python-pillow:master Apr 1, 2015

radarhere mentioned this pull request May 13, 2015

CurImagePlugin and Transparency #807

Closed

hugovk mentioned this pull request Jun 22, 2015

Problem with saving image from clipboard #1293

Closed

artscoop deleted the patch-1 branch December 30, 2015 16:24

radarhere added the BMP label Oct 22, 2022

akx added a commit to akx/Pillow that referenced this pull request Nov 14, 2023

IcoImagePlugin: remove transparency hack (32-bit DIBs are supported s…

3d5cca2

…ince python-pillow#1125)

akx added a commit to akx/Pillow that referenced this pull request Nov 14, 2023

IcoImagePlugin: remove transparency hack (32-bit DIBs are supported s…

70150db

…ince python-pillow#1125)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix 32-bit BMP loading (RGBA or RGBX) #1125

Fix 32-bit BMP loading (RGBA or RGBX) #1125

artscoop commented Mar 4, 2015

homm Mar 4, 2015

artscoop Mar 4, 2015

artscoop commented Mar 4, 2015

wiredfool commented Mar 6, 2015

artscoop commented Mar 7, 2015

aclark4life commented Apr 1, 2015

homm Apr 1, 2015

artscoop Apr 1, 2015

aclark4life commented Apr 1, 2015

techtonik commented Dec 30, 2015

artscoop commented Dec 30, 2015

techtonik commented Dec 30, 2015

artscoop commented Dec 30, 2015

techtonik commented Dec 30, 2015

techtonik commented Dec 30, 2015

techtonik commented Dec 30, 2015

artscoop commented Dec 30, 2015

techtonik commented Dec 30, 2015

hugovk commented Dec 30, 2015

techtonik commented Dec 30, 2015

techtonik commented Dec 30, 2015

techtonik commented Dec 30, 2015

techtonik commented Dec 30, 2015

MXS2514 commented Jul 6, 2016

wiredfool commented Jul 6, 2016 •

edited

Loading

Fix 32-bit BMP loading (RGBA or RGBX) #1125

Fix 32-bit BMP loading (RGBA or RGBX) #1125

Conversation

artscoop commented Mar 4, 2015

homm Mar 4, 2015

Choose a reason for hiding this comment

artscoop Mar 4, 2015

Choose a reason for hiding this comment

artscoop commented Mar 4, 2015

wiredfool commented Mar 6, 2015

artscoop commented Mar 7, 2015

aclark4life commented Apr 1, 2015

homm Apr 1, 2015

Choose a reason for hiding this comment

artscoop Apr 1, 2015

Choose a reason for hiding this comment

aclark4life commented Apr 1, 2015

techtonik commented Dec 30, 2015

artscoop commented Dec 30, 2015

techtonik commented Dec 30, 2015

artscoop commented Dec 30, 2015

techtonik commented Dec 30, 2015

techtonik commented Dec 30, 2015

techtonik commented Dec 30, 2015

artscoop commented Dec 30, 2015

techtonik commented Dec 30, 2015

hugovk commented Dec 30, 2015

techtonik commented Dec 30, 2015

techtonik commented Dec 30, 2015

techtonik commented Dec 30, 2015

techtonik commented Dec 30, 2015

MXS2514 commented Jul 6, 2016

wiredfool commented Jul 6, 2016 • edited Loading

wiredfool commented Jul 6, 2016 •

edited

Loading