Automatic scaling of Image.MAXBLOCK

[acdha]

There's a venerable old PIL bug which caused Image.save(optimize=True) to fail with JPEGs when the file size is over a certain threshold. The earliest reference I could find dates back to 1999:

http://mail.python.org/pipermail/image-sig/1999-December/000942.html

The workaround commonly used is to either hardcode something like this:

ImageFile.MAXBLOCK = <empirically determined constant (i.e. wild guess)>

or the more dynamic approach used in e.g. matthewwithanm/django-imagekit#50:

ImageFile.MAXBLOCK = max(img.size) ** 2

This seems like exactly the kind of thing Pillow could just fix once and for all.

[aclark4life]

Do you have a preference? And/or do you want to send a pull request? @cgohlke @wiredfool any thoughts?

[wiredfool]

I'd really like to fix a 14 year old bug. (Sounds like a fine whiskey actually. older than some decent ones.)

I'd have to dig to see what MAXBLOCK affects. I'd be especially wary if maxblock ends up affecting buffer sizes that get filled and passed to the lib* decoders.

Good (or at least some) test coverage of this would be nice, and go a long way toward mitigating any bad effects.

So, it looks like there's a comment in RawEncode.c

 /* FIXME: This encoder will fail if the buffer is not large enough to
 * hold one full line of data.  There's a workaround for this problem
 * in ImageFile.py, but it should be solved here instead.
*/

and in ImageFile.py

    # FIXME: make MAXBLOCK a configuration parameter
    bufsize = max(MAXBLOCK, im.size[0] * 4) # see RawEncode.c

My Wild Ass Guess is that when libjpeg is optimizing, it passes back the whole image in one big chunk. Otherwise, worst case it can do scanline by scanline, and the bufsize guarantees that it's big enough for one scan line.

If we bump up the MAXBLOCK, or the bufsize here, we're going to allocate that much memory each time we do an encode. The brute force method would be size[0]size[1] * 4, but that's going to be suboptimal for large images. max(size)*2 would be less than that, but it may not actually fix all the cases.

I'd be willing to dig more, assuming I can get a reproducable test case.

[acdha]

Strong +1 on expanding tests. I strongly suspect your guess is correct and this could have ripple effects with a codebase this old and squirrelly.

If you want to reproduce it, here's a transcript:

Python 2.7.2 (default, Oct 11 2012, 20:14:37) 
[GCC 4.2.1 Compatible Apple Clang 4.0 (tags/Apple/clang-418.0.60)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from PIL import Image, ImageFile
>>> i = Image.new("RGB", (4096, 4096), 0xff3333)
>>> i.save("/dev/null", format="JPEG")
>>> i.save("/dev/null", format="JPEG", optimize=True)
Suspension not allowed here
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/cadams/.virtualenvs/project/lib/python2.7/site-packages/PIL/Image.py", line 1437, in save
    save_handler(self, fp, filename)
  File "/Users/cadams/.virtualenvs/project/lib/python2.7/site-packages/PIL/JpegImagePlugin.py", line 471, in _save
    ImageFile._save(im, fp, [("jpeg", (0,0)+im.size, 0, rawmode)])
  File "/Users/cadams/.virtualenvs/project/lib/python2.7/site-packages/PIL/ImageFile.py", line 501, in _save
    raise IOError("encoder error %d when writing image file" % s)
IOError: encoder error -2 when writing image file
>>> ImageFile.MAXBLOCK = max(i.size) ** 2
>>> i.save("/dev/null", format="JPEG", optimize=True)
>>>

Ideally, this feels like something which should just work. Perhaps something as simple as having save() check the format and optimize flags and increase MAXBLOCK for that operation along with documenting that this can increase RAM consumption? The part which feels dirtiest is changing this by setting a variable on a class – simply moving the change closer to the function call seems like a good idea.

[wiredfool]

So, having poked around a bit, I can confirm a few things.

1. It does appear that the jpeg code is writing out optimized images in one push to the buffer.
2. The "Suspension not allowed here" message basically means that it tried to write, and there wasn't enough space.
3. That really boring image takes 8k/megapixel to save, one from my camera has a different constant there. In each case, MAXBLOCK had to be > than the final image file size.
4. MAXBLOCK appears to be allocated for every image encode, so just randomly upping it is a bad idea.
5. Passing in an open file instead of a filename doesn't change anything, because it's just doing the shuffling to and from the buffer in c instead of python, not pushing the fp all the way down into the library. (which I did with the tiff lib, in sort of an aggressive manner. )
6. It's probably not threadsafe to just change maxblock for the one operation, since it's a module level global variable.
7. I'm still thinking on this, but I think pushing the bufsize into an optional parameter on _save, initialized to MAXBLOCK, and then figuring out some sort of guess for the final jpeg size would be an appropriate fix.

from PIL import Image, ImageFile

def search(i):
  ImageFile.MAXBLOCK = 65536
  while True:
    try:
      i.save("/tmp/jpg", format="JPEG", optimize=True)
      print "Save succeded at MAXBLOCK=%d" % ImageFile.MAXBLOCK
      break
    except: 
      print "Save failed at MAXBLOCK=%d" % ImageFile.MAXBLOCK
      ImageFile.MAXBLOCK *= 2

def im(size=(1024,1024)):
  i = Image.new("RGB", size, 0xff3333)
  return i


for x in [128,512,1024,2048,4096,8192, 16384]:
  print "%d wide" %x
  search(im((x,2048)))

for y in [128,512,1024,2048,4096,8192, 16384]:
  print "%d tall" %y
  search(im((2048,y)))

[wiredfool]

It looks like the two plugins need special handling with maxblock are the JpegImagePlugin and the RawEncode.c. Unfortunately, the RawEncodes go straight into the save instead of through an image plugin, so there's no place to hang the hook to up the buffer size if necessary. So, there's still a bit of a special case in there, but at least the JpegImagePlugin can override as necessary.

[d-schmidt]

The only problem I've had with maxblock so far was an image withh an exif header >64k.
Will check the fix on monday,

[wiredfool]

This probably won't fix that case, as it's pretty specifically targeted at the optimized case. We might be able check the size of the chunks that we have and make sure MAXBLOCK is big enough to handle that.

[wiredfool]

@d-schmidt did you ever check the jpeg image plugin with a >64k exif size?

[d-schmidt]

What do you mean by "check"? We are using it, I'm increasing maxblock manually in the image converter. The converter is running 24/7.

[wiredfool]

It looks like it's actually pretty easy to put in a check for the exif size. I don't have an image to check for it, and looking for something lead me to this: https://en.wikipedia.org/wiki/Exif#Viewing_and_editing_Exif_data

Exif metadata are restricted in size to 64 kB in JPEG images because according to the specification this information must be contained within a single JPEG APP1 segment. Although the FlashPix extensions allow information to span multiple JPEG APP2 segments, these extensions are not commonly used. This has prompted some camera manufacturers to develop non-standard techniques for storing the large preview images used by some digital cameras for LCD review. These non-standard extensions are commonly lost if a user re-saves the image using image editor software, possibly rendering the image incompatible with the original camera that created it.

Anyway, if you could test a4a856b we could get a fix merged. Or, rather 7200c40 which actually passes travis on py3.

[d-schmidt]

Next week. Nikon seems to ignore the specification. Or it is a multiple segment spanning block, I don't know. But the raw binary exif block was above 65k on a lot of images.

[d-schmidt]

Your fix does not work so far. The exif is smaller than the limit (-4 bytes) but it still crashes. It is working just fine if I add ImageFile.MAXBLOCK = 2**20 before saving.

Suspension not allowed here
Traceback (most recent call last):
  File "converter.py3", line 254, in shrinkImage
    im.save(outFile, "JPEG", quality=quali, exif=orgInfo['exif'])
  File "e:\programme\Python32\lib\site-packages\pillow-2.0.0-py3.2-win32.egg\PIL\Image.py", line 1467, in save
    save_handler(self, fp, filename)
  File "e:\programme\Python32\lib\site-packages\pillow-2.0.0-py3.2-win32.egg\PIL\JpegImagePlugin.py", line 568, in _save
    ImageFile._save(im, fp, [("jpeg", (0,0)+im.size, 0, rawmode)], bufsize)
  File "e:\programme\Python32\lib\site-packages\pillow-2.0.0-py3.2-win32.egg\PIL\ImageFile.py", line 476, in _save
    raise IOError("encoder error %d when writing image file" % s)
IOError: encoder error -2 when writing image file
encoder error -2 when writing image file

analyzation mode started
loaded file:  e:\temp\testbilder_nikon_exif\aimg9.jpg
used parameters:  {'analyze': '', 'in': 'testbilder_nikon_exif\\aimg9.jpg'}

size width x height:  4288x2848
internal format:  JPEG
color mode:  RGB
contains exif:  True
exif length:  65532 bytes
contains icc profile:  False
transparency flag:  None

jpeg with exif found, trying jhead:
File name    : e:\temp\testbilder_nikon_exif\aimg9.jpg
File size    : 942957 bytes
File date    : 2012:09:26 22:09:45
Camera make  : NIKON CORPORATION
Camera model : NIKON D90
Date/Time    : 2012:08:21 00:19:35
Resolution   : 4288 x 2848
Flash used   : Yes (auto, return light detected)
Focal length : 38.0mm  (35mm equivalent: 57mm)
Exposure time: 0.017 s  (1/60)
Aperture     : f/5.0
ISO equiv.   : 800
Whitebalance : Auto
Metering Mode: pattern
GPS Latitude : ? ?
GPS Longitude: ? ?

[wiredfool]

4 bytes. (I'm guessing that those bytes are APP1.) So, they're in spec, and we're not handling it. I wonder if the JPEG lib needs to write all the metadata in one shot, or if it's just one block. More digging required, I guess. I'll take a look and see what I can find in the libjpeg code.

[MrKesn]

The same problem. Even #158 and 7200c40 did not work for me - I had to delete condition at all:

if "optimize" in info:
    bufsize = im.size[0]*im.size[1]

transformed into

bufsize = im.size[0]*im.size[1]

I can provide any additional information