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

get_value(Vips::META_LOADER) fails for gif files #93

Closed
raimondasv opened this issue Nov 22, 2016 · 7 comments
Closed

get_value(Vips::META_LOADER) fails for gif files #93

raimondasv opened this issue Nov 22, 2016 · 7 comments
Labels

Comments

@raimondasv
Copy link

Might be the issue with libvips, still, reporting it here. We are using Vips::META_LOADER property to detect file types and correct Content-Type header. Unfortunately, it fails for gifs.

require 'vips'

gif = Vips::Image.new_from_file('image.gif')
puts gif.get_value(Vips::META_LOADER)

produces

.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/ruby-vips-1.0.3/lib/vips/image.rb:699:in `get_value': Field vips-loader not found. (Vips::Error)
	from gif.rb:4:in `<main>'

Gif loader is present in the system and Gif files can be loaded and manipulated. Just accessing vips-loader header value fails.

Same code for jpegs:

require 'vips'

gif = Vips::Image.new_from_file('image.jpg')
puts gif.get_value(Vips::META_LOADER)

produces

jpegload

Maybe there are different means to detect image formats?

@jcupitt jcupitt added the bug label Nov 22, 2016
@jcupitt
Copy link
Member

jcupitt commented Nov 22, 2016

Hi, I agree, this looks like a libvips bug. I'll have a look.

jcupitt added a commit to libvips/libvips that referenced this issue Nov 22, 2016
vips__image_copy_fields_array() used to wipe meta on the output before
merging the input images. This broke foreign.c in some cases, since it
likes setting some meta on the output before calling the subclass
loaders, and they sometimes call write().

see libvips/ruby-vips#93
@jcupitt
Copy link
Member

jcupitt commented Nov 22, 2016

OK, working for me now:

$ irb
irb(main):001:0> require 'vips'
=> true
irb(main):002:0> x = Vips::Image.new_from_file "walk.gif"
=> #<Vips::Image:0x55bfd4bf3e90 ptr=0x55bfd4bb2010>
irb(main):003:0> x.get_value Vips::META_LOADER
=> "gifload"

Thank you for reporting this dumbness!

I'll try to think of a workaround for you in the meantime.

@raimondasv
Copy link
Author

Amazing, @jcupitt thank you for doing this!

@jcupitt
Copy link
Member

jcupitt commented Nov 23, 2016

Looks like you can call vips_foreign_find_load() directly:

http://www.vips.ecs.soton.ac.uk/supported/current/doc/html/libvips/VipsForeignSave.html#vips-foreign-find-load

irb(main):001:0> require 'vips'
=> true
irb(main):009:0> Vips::Foreign.find_load("/home/john/pics/k2.jpg")
=> "VipsForeignLoadJpegFile"
irb(main):011:0> Vips::Foreign.find_load("/home/john/pics/walk.gif")
=> "VipsForeignLoadGifFile"

It gives you the canonical name of the loader class rather than the nickname, but it's obvious what it means. It should work on your libvips. There's Vips::Foreign.find_load_buffer() as well, which will sniff the image type from a string.

@raimondasv
Copy link
Author

👍

I've tried to avoid this because new_from_buffer would do detection again, no? However, the overhead seems to be small, so we are going to use Vips::Foreign.find_load_buffer for now and later update to and use get_value(Vips::META_LOADER)

@jcupitt
Copy link
Member

jcupitt commented Nov 23, 2016

Yes, it'll detect again, but detection is mostly quick. If you build your libvips without magick support and without support for things like openslide (very slow detection), it should be fast enough, I'd think.

@jcupitt
Copy link
Member

jcupitt commented Jan 30, 2017

I'll close, I think this is done.

@jcupitt jcupitt closed this as completed Jan 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants