-
Notifications
You must be signed in to change notification settings - Fork 50
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
magickload should have a format hint parameter #39
Comments
Hello @Flips01 , Huh that's weird. It seems to be just #!/usr/bin/env python
import sys
import pyvips
with open(sys.argv[1], "rb") as f:
buf = f.read()
img = pyvips.Image.magickload_buffer(buf)
print "width = ", img.width;
print "height = ", img.height; (you can call the ImageMagick load-from-buffer operation directly. Don't give I see:
So I guess it's probably an imagemagick bug. |
I'll see if I can find any other formats it fails for. |
I tried this: #!/usr/bin/env python
import os
import sys
import pyvips
for filename in sys.argv[1:]:
if not os.path.isfile(filename):
continue
if os.path.getsize(filename) > 10000000:
continue
try:
with open(filename, "rb") as f:
buf = f.read()
img = pyvips.Image.magickload_buffer(buf)
except:
# now try loading directly from the file
try:
img = pyvips.Image.new_from_file(filename)
print "failed for buffer, succeeded for file:", filename
except:
pass And on my image collection I got (removing format duplicates):
So it looks like there are quite a few minor formats where imagemagick is unable to load from a buffer but can load from a file. |
... I had a dig inside ImageMagick and the https://github.com/ImageMagick/ImageMagick/blob/master/coders/jpeg.c#L1584 But https://github.com/ImageMagick/ImageMagick/blob/master/coders/icon.c#L784 So to get the |
Could you provide an example for using the format hint? At least the following attempt doesn‘t work: pyvips.Image.new_from_buffer(buf, ‘‘, format=‘ICO‘) |
Sorry, format hints are not in the libvips API, they would need to be added. Let's tag this as an enhancement. |
Some magick coders (eg. ICO) don't sniff the filetype from the data, so when you try to load from a string, imagemagick is unable to pick the right decode path. Add a @Format option so callers can hint the filetype. see https://github.com/jcupitt/pyvips/issues/39
I had a moment so I hacked a format option in. I now see:
And I can view the Would you be able to test it? You'd need to build git master libvips, unfortunately. |
It doesn't seem to work. I've build libvips from master:
Using the format parameter in pyvips doesn't make any difference:
Also I've noticed that I'll get an exception when I use the format parameter for an JPEG image:
I would assume that a hint is more of a fallback option: When I supply an JPEG image and give an ICO-hint it should still be capable to handle it correctly. It already seems to work this way since it selects the correct loader but it should gracefully ignore the format parameter shouldn't it? |
Sorry, you're right, you'll have to call the imagemagick buffer loader directly. This seems to work:
I should have tested it. |
# tmp file content = https://www.welt.de/favicon.ico
tmp_file = '/dev/shm/imo_3tP2LK'
image = Image.magickload(tmp_file, format='ico') Seems to work with jcupitt/libvips@b7bac0d. But hinting the thumbnail operator seems not to work (with the embedded string option: I've prepared a example python test here.
Current output:
|
It seems to work for me. With your sample program I see:
with git master libvips. |
@jcupitt Try to rename the .ico file to a file without extension. |
Ah, of course, sorry. Yes, I see a failure here too, I'll investigate. |
You can see the problem here: https://github.com/jcupitt/libvips/blob/master/libvips/foreign/foreign.c#L520 When searching for a loader, it strips off any filename options, then calls the The A simple workaround would be to use Maybe a better fix would be to add our own It's a shame IM does not have a complete set of file format sniffers. |
Thinking about this a bit more, adding our own sniffers is a much better idea than the Let's remove |
though it only spots ICO for now see https://github.com/jcupitt/pyvips/issues/39
OK, there's a branch which adds a ICO sniffer. It should be simple to add any more we need. It's just this function: https://github.com/jcupitt/libvips/blob/remove-format/libvips/foreign/magick.c#L244 I now see:
Much nicer. |
Indeed much nicer. Just tested it and it seems to work! I now see:
(with the edited I'll let you know when more image formats needs to be sniffed. On libvips 8.6.4 we only had problems with The Can't wait for libvips 8.7. Let me know if anything needs to be tested prior to the release. |
That's great, thanks for testing, I'll merge to master and add a test for Yes, 8.7 RSN, hopefully. It's very late already :( |
I put 8.7.0-rc1 up. I'll give it a week for testing, then let's make 8.7.0. |
Wow nice! Issue 703 (I'm not sure if this already has been resolved in master) and 879 on the libvips issue tracker can safely be moved to another milestone (just minor issues that I've encountered). We would like to experiment with resizing animated GIF's some day (has no priority). For this In my opinion you can release 8.7 as it is now. |
Ooop, forgot to bump the milestones. Thanks! I'll push that lua-vips version too. I was waiting for something, but I forget what. |
Hi,
handling images that would cause a fallback towards ImageMagick doesn't work as expected. Given an exemplary image, located at https://www.welt.de/favicon.ico, the actual vips-cli is capable to handle it. A command like
vips copy favicon.ico favicon.jpeg
completes without any error. Also the following python code completes without any error:
But loading the image from the buffer will result in an error:
->
The text was updated successfully, but these errors were encountered: