Renamed Jpeg2K variable to prevent masking Image reduce method

[radarhere]

Resolves #4343

The new Image reduce method does not work for Jpeg2K images, as Jpeg2KImagePlugin has a reduce variable that masks it. The variable is not purely internal, but is actually tested behaviour -

Pillow/Tests/test_file_jpeg2k.py

Lines 112 to 116 in ef4a0b2

	def test_reduce(self):
	with Image.open("Tests/images/test-card-lossless.jp2") as im:
	im.reduce = 2
	im.load()
	self.assertEqual(im.size, (160, 120))

This PR renames the reduce variable to load_reduce for the sake of moving forward, while also keeping the tested behaviour by checking if reduce is callable.

[homm]

Oh my god (

How does this variable work? Shouldn't it be just .draft() method?

[homm]

This property is related to -r option of opj_decompress: https://github.com/uclouvain/openjpeg/wiki/DocJ2KCodec#usage-1

It scales image in two times for every reducing level (exactly algorithm is unknown and is I see, it doesn't match JPEG DCT scaling or .reduce() method).

$ opj_decompress -i ../imgs/radial.jp2 -o ../imgs/_out.r-2.opj.bmp -r 2

The problem is in Pillow it not only scales the image, but it also crops it by the same times. So with reduce = 2 you get 2048 / 4 / 4 = 128 pixels image instead of 2048 / 4 = 512.

In [2]: im = Image.open('../imgs/radial.jp2') 
   ...: print(im) 
   ...: im.reduce = 2 
   ...: im.load() 
   ...: im.save('../imgs/_out.r-2.png') 
   ...:  

<PIL.Jpeg2KImagePlugin.Jpeg2KImageFile image mode=L size=2048x2048 at 0x7F3D58AABEF0>

Another problem is unlike console decoder, Pillow fails on images of some sizes.

In [2]: from PIL import Image 
   ...: im = Image.open('../imgs/radial.rgb.jp2') 
   ...: print(im) 
   ...: im.reduce = 2 
   ...: im.load() 
   ...: im.save('../imgs/_out.r-2.png') 
   ...:  

<PIL.Jpeg2KImagePlugin.Jpeg2KImageFile image mode=RGBA size=2049x2047 at 0x7FB7867849B0>
OSError: broken data stream when reading image file

I've checked Pillow 2.7.0, it operates the same. So I'm considering this functionality as never worked and suggest to add deprecation warning on Jpeg2KImageFile.scale set and save the value in private property instead of renaming it.

In future, if someone wants to add this functionality, it should be added in .draft() method.

[homm]

@al45tair This property was added by you at the very beginning. Can you confirm it's broken or I miss something?

[al45tair]

@homm Honestly, I haven't looked at this code in ages, but from your description it does sound like it's broken. It should have done the same thing as the OpenJPEG command line; if it doesn't, it's wrong.

[homm]

@al45tair do you want to try to fix this and reimplement as .draft() method?

[al45tair]

I'm sorry, but I'm not working in this area any more and I don't really have the time to work on Pillow as well as everything else that's going on.

[radarhere]

Regardless of any feelings about how well .reduce works for Jpeg2000 images, I think that the API should be restored for the sake of backwards compatibility. If people aren't sold on the idea of load_reduce, I'm happy to remove that from this PR.

[homm]

@radarhere I've created PR with suggestions from #4344 (comment).

radarhere#5

[homm]

The design decision is needed. In the current version, the very common core functionality is partially broken since the opened image could be in any format including jpeg2000:

from PIL import Image
im = Image.open(image_path)
im.thumbnail((100, 100))

We can fix it by deprecating undocumented already partially broken Jpeg2KImageFile.reduce property. There is a small chance that someone uses it, so until the next big release we need to keep backward compatibility for this property. There is no problem to use the value which the user assigns to this property. The question is what we should return to the user.

Option 1

This PR. Return the bound method until any value is set, otherwise, return given value.

from PIL import Image
im = Image.open("Tests/images/16bit.cropped.j2k")
print(im.reduce)  # <bound method Image.reduce of ...>, backward incompatability
im.reduce = 5
print(im.reduce)  # 5, as expected
im.thumbnail((100, 100))  # TypeError since im.reduce is not an method

Option 2

radarhere#5 Keep the value in private property, always return the method.

from PIL import Image
im = Image.open("Tests/images/16bit.cropped.j2k")
print(im.reduce)  # <bound method Image.reduce of ...>, backward incompatability
im.reduce = 5
print(im.reduce)  # <bound method Image.reduce of ...>, backward incompatability
print(im._reduce)  # 5
im.thumbnail((100, 100))  # Ok, im.reduce is an method

So the truth is in both cases we introduce backward incompatibility but in my opinion, the second option is more predictable.

[radarhere]

@homm looking into this, I want to point out that your Option 2 code isn't actually viable. Looks like any size value that causes it to call reduce also causes ValueError: box can't exceed original image size.

That said, even if it is scenario where ultimate success is not possible, maybe Pillow still shouldn't throw a TypeError. So I've created PR #4474 as an alternative to this.

With that PR,

from PIL import Image
im = Image.open("Tests/images/16bit.cropped.j2k")
print(im.reduce)  # <bound method Image.reduce of ...>, backward incompatability
im.reduce = 5
print(im.reduce)  # 5, as expected
im.thumbnail((100, 100))  # the reduce method is still called

[homm]

Looks like any size value that causes it to call reduce also causes ValueError: box can't exceed original image size.

This is due to another Jpeg2KImagePlugin bug. It reduces self.size on each self.load() call.

In [12]: from PIL import Image 
    ...: im = Image.open("Tests/images/test-card-lossless.jp2") 
    ...: im.reduce = 1 
    ...: im
Out[12]: <PIL.Jpeg2KImagePlugin.Jpeg2KImageFile image mode=RGB size=640x480 at 0x7FF0B810AD90>

In [13]: im.load() 
    ...: im
Out[13]: <PIL.Jpeg2KImagePlugin.Jpeg2KImageFile image mode=RGB size=320x240 at 0x7FF0B810AD90>

In [14]: im.load() 
    ...: im
Out[14]: <PIL.Jpeg2KImagePlugin.Jpeg2KImageFile image mode=RGB size=160x120 at 0x7FF0B810AD90>

In [15]: im.load() 
    ...: im
Out[15]: <PIL.Jpeg2KImagePlugin.Jpeg2KImageFile image mode=RGB size=80x60 at 0x7FF0B810AD90>

[radarhere]

Closed in favour of #4474