Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix error with loading paletted BMP images using Pillow #388

Merged
merged 8 commits into from Oct 1, 2018

Conversation

addisonElliott
Copy link
Contributor

Any 8-bit paletted BMP image is unable to be loaded using the Pillow image plugin with imageio but this pull request fixes that. The error that comes up when attempting to load an 8-bit paletted BMP image is:
ValueError: cannot reshape array of size XXX into shape (XXX, 3)

This error occurs in pil_get_frame when attempting to apply the palette to get the data in RGB format.

This error comes from the fact that the raw palette is retrieved from the Pillow image and parsed rather than using the converted/stored palette. In other words, im.palette.getdata() is the raw data that gets saved upon retrieving the data into im.getpalette(). The data in im.getpalette() is converted into one format (such as RGB) while the im.palette.getdata() can be in many different formats.

The reason that im.getpalette() is not used in the pil_get_frame function is because there is some issues with reading animated GIFs with a different color table for each frame causes some issue. To be specific, the dispose method must be set to 1 meaning that each new frame should be pasted on top of the old frame, which is kinda weird/tough to do when you have different color tables. See this issue here for more information, but it may be awhile to get a fix.

As a result of using the raw palette data, we need to know the format it is in. For BMP's, most of the time the format is going to be BGRX where X is just an unused byte. This is what causes the error is because the code currently tries to resize it to be 3 channels no matter what.

In Pillow, the im.palette class saves the raw mode of the data but once it is saved into im.getpalette() (via im.putpalette()), then the raw mode is set to none. This is fine if we don't ever use the raw data from the palette, but we do so we need to save it.

Solution: My way of fixing this is to save the rawmode before it is discarded in another placeholder variable and then restore it when pil_get_frame is called.

A new test is added to load bitmaps. This test will fail until the data is added to the imageio-binaries repository: see here.

@addisonElliott
Copy link
Contributor Author

Will make another push to restart the tests since the image is in the binaries now.

Should force CI to rerun and succeed...
@coveralls
Copy link

coveralls commented Sep 30, 2018

Coverage Status

Coverage increased (+0.01%) to 91.089% when pulling dd2906d on addisonElliott:fix-pillow-bmp-palette into 2d58975 on imageio:master.

Not used, caused error in CI
Believe it is causing issues with getting CI to pass
@almarklein
Copy link
Member

Thanks for the fix and this solid PR!

@almarklein almarklein merged commit 1e08c70 into imageio:master Oct 1, 2018
@addisonElliott addisonElliott deleted the fix-pillow-bmp-palette branch January 8, 2019 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants