sul-dlss / assembly-image

Creates JP2 image derivatives of files.
Other
0 stars 1 forks source link

use ruby-vips instead of mini-exiftools for low level image stuff, when possible #54

Closed ndushay closed 2 years ago

ndushay commented 2 years ago

Tony C. believes that ruby-vips gem will provide a bunch of the functionality available to us from miniexif-tool gem. https://github.com/libvips/ruby-vips

See also #53

Look for the code that uses exif and see what can be replaced. If all of it - yay! Make a PR. If some of it, confer with Tony C as to when we should favor one tool over the other.

Tony C. is the PO for this work, and this ticket should get Tony's approval, not Andrew's.

Note that autodetection of mime type could introduce errors that Image-Magick (or libvips?) could barf on, for example.

ALERT:

ndushay commented 2 years ago

The main places to benefit:

and other low level image technical metadata.

ndushay commented 2 years ago

to get height and width:

you can do something like this.

require 'ruby-vips' # and in Gemfile

        vips_image = Vips::Image.new_from_file(file)
        { height: vips_image.height, width: vips_image.width }
ndushay commented 2 years ago

Actions for this ticket:

ndushay commented 2 years ago

Note: this is NOT blocked by libvips performance problems for common-accessioning; the assembly-image gem stands alone, and the main branch is already libvips; we revert to non-libvips for consumers via gem version.

Note further: for some changes, testing with consumers (common-accessioning and was-robot-suite for was seed thumbnail creation) may ultimately be warranted (as in on stage and qa ...)

aaron-collier commented 2 years ago

It is unclear to me if libvips is going to be useful in this gem. For instance https://github.com/sul-dlss/assembly-image/blob/main/spec/image_spec.rb#L76 is testing the output of exif.colorspace to be sRGB, and later on line 99 it's looking for Grayscale. This appears to be interpretation in libvips (I could be mistaken here) with outputs srgb (lowercase) and b-w instead of Grayscale.

irb(main):013:0> vimg = Vips::Image.new_from_file '/Users/amcollie/github/sul-dlss/assembly-image/spec/test_data/gray_gray_untagged.tif'
=> #<Image 43x36 uchar, 1 bands, b-w>
irb(main):014:0> vimg.interpretation
=> :"b-w"
irb(main):015:0> vimg = Vips::Image.new_from_file '/Users/amcollie/github/sul-dlss/assembly-image/spec/test_data/color_rgb_srgb.tif'
=> #<Image 43x36 uchar, 3 bands, srgb>
irb(main):016:0> vimg.interpretation
=> :srgb

This is an example where we can map the output possibly, but it's unclear if this is even the correct field to use for it.

I will hold on this until we can discuss between @ndushay and @calavano

jcoyne commented 2 years ago

I believe this is complete.