python-pillow / Pillow

Python Imaging Library (Fork)
https://python-pillow.org
Other
12.32k stars 2.23k forks source link

Change in behavior with JPEG Images in 11.0.0 #8472

Open davidmezzetti opened 1 month ago

davidmezzetti commented 1 month ago

The following code that worked in Pillow 10.4 no longer works with Pillow 11.0.

import pickle

from io import BytesIO

from PIL import Image

im = Image.open("books.jpg")

p = pickle.dumps(im)
im = pickle.loads(p)

# Fails in 11.0, works in 10.4
output = BytesIO()
im.save(output, format=im.format, quality="keep")

Error stack.

Traceback (most recent call last):
  File "test.py", line 15, in <module>
    im.save(output, format=im.format, quality="keep")
  File "python3.9/site-packages/PIL/Image.py", line 2605, in save
    save_handler(self, fp, filename)
  File "python3.9/site-packages/PIL/JpegImagePlugin.py", line 700, in _save
    subsampling = get_sampling(im)
  File "python3.9/site-packages/PIL/JpegImagePlugin.py", line 643, in get_sampling
    if not isinstance(im, JpegImageFile) or im.layers in (1, 4):
  File "python3.9/site-packages/PIL/JpegImagePlugin.py", line 396, in __getattr__
    raise AttributeError(name)
AttributeError: layers

This has been traced to the following change in src/PIL/JpegImagePlugin.py

Pillow 10.4.0

Code

def get_sampling(im):
...
    if not hasattr(im, "layers") or im.layers in (1, 4):
        return -1
...

Pillow 11.0.0

Code

def get_sampling(im: Image.Image) -> int:
...
    if not isinstance(im, JpegImageFile) or im.layers in (1, 4):
        return -1
...

Suggested fix

def get_sampling(im: Image.Image) -> int:
...
    if not isinstance(im, JpegImageFile) or not hasattr(im, "layers") or im.layers in (1, 4):
        return -1
...

Note that the following hack does workaround the issue in 11.0.

im.layers = 1
im.save(output, format=im.format, quality="keep")
aclark4life commented 1 month ago

Is this a bug or feature? It looks like Pillow got more strict in verifying that im was a JpegImageFile ?

davidmezzetti commented 1 month ago

That is a good question. Serializing an image and reloading it changes how it looks.

I wasn't sure if this check for the layers attribute was at one point intentional for this use case or just coincidental.

Yay295 commented 1 month ago

Is there a reason you want to pickle the ImageFile instead of just opening the image again?

davidmezzetti commented 1 month ago

The serialization is happening in a different part of the code. This example is for simplicity in reproducing.

radarhere commented 1 month ago

I've created #8476 as a possible solution to this - rather than ignoring the fact that layers was removed by the pickling process, let's make sure it stays present.