sul-dlss / assembly-image

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

have tests for image data bigger than 4G ... that use libvips. #57

Closed ndushay closed 2 years ago

ndushay commented 2 years ago

There are specs for this already that are skipped by default; see in image_spec.rb L56, L76. It uses ImageMagick for creating the images, we probably want to switch that to libvips.

Work with Tony C and ... ?? as to how to do this.

In image.rb, image_data_size method https://github.com/sul-dlss/assembly-image/blob/main/lib/assembly-image/image.rb#L143-L146 needs to be exercised with large image.

The test generates 4GB images (and its 4GB for the source and 4GB for the temp tiff), so that storage needs to be available - (on local VMs, on CI ..)

ndushay commented 2 years ago

Tony says:

There were tests in the rspec for it that are skipped by default, but I don't know if they'll still work as intended. They're dynamically generated, so it is likely just a matter of making sure the spec_helper can still generate the images. (If I remember right, it was just using ImageMagick to generate them, so if those binaries are available it should still work.)

ImageMagick is on our VMs, as it is a dependency of libvips.

ndushay commented 2 years ago

ACTION for this ticket:

jcoyne commented 2 years ago

@ndushay can you explain why 4G is the threshold you picked?

ndushay commented 2 years ago

@calavano I'm going to assign this ticket to you;

@jcoyne ... the code we have now has this limit - it may have to do with filetype size limits.

jcoyne commented 2 years ago

Does anyone know what is limiting this to 4G?

andrewjbtw commented 2 years ago

I believe tiff has an upper bound at 4 GB. Larger images (which we do get sometimes) use the bigtiff file format.

jcoyne commented 2 years ago

Would this just be testing that kakadu can handle a Bigtiff image? Presumably there are tests in kakadu for this. I'm wondering if there's anything within the software we are writing that would require such a test.

ndushay commented 2 years ago

Would this just be testing that kakadu can handle a Bigtiff image? Presumably there are tests in kakadu for this. I'm wondering if there's anything within the software we are writing that would require such a test.

https://github.com/sul-dlss/assembly-image/blob/main/lib/assembly-image/jp2_creator.rb#L103-L106

Tony C. brought this up, hence the ticket. This ticket exists for a real reason. There is a real need for us to know OUR code handles images bigger than 4g. Your questiions are welcome, but you see above I said "The code we have now has this limit" ... I would have expected that to be enough for even @jcoyne to have faith in there being a real need for this ticket.

calavano commented 2 years ago

@jcoyne The test-case would be for the temporary TIFF generation. "Normal" TIFF files have a limit of 4GB from the format's use of 32-bit offsets, BigTIFF uses 64-bit offsets to get around this. We need to ensure that when there is >4GB of image data the temporary TIFF is saved using 64-bit offsets. We currently calculate this by checking to see if height * width * channels is >4GB and enabling it when true.

jcoyne commented 2 years ago

Thank you @calavano that's helpful.

ndushay commented 2 years ago

Since we are now using bigtiff for all temporary tiffs, there is no longer a separate code path for large image data and this ticket is moot.