sul-dlss / assembly-image

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

preserve image orientation in jp2 if set via rotation flag in the source image #37

Closed andrewjbtw closed 2 years ago

andrewjbtw commented 3 years ago

Describe the issue

The jp2-create step creates jp2s from source images like TIFFs or JPGs. When the source image uses a rotation flag in image metadata to indicate how applications should render the image, this flag is not preserved in the generated JP2, resulting in images that are not rotated correctly.

User impact

Users end up having to rotate the images themselves using settings that do not simply write the rotation information into the metadata and then reaccession the files. This can be a long and frustrating process, especially since the original images will have been rendered correctly on their own computers if they are using software that reads the rotation flag, so they might not see the issue until after the images have been accessioned. Goobi and many common image editing applications do read this flag, so the issue can be hard to detect in advance.

Expected behavior

Generated JP2s should have the same rotation as their source images. I'm not clear how best to go about this but one option seems to be to have the JP2 image rotated as part of the jp2-create process.

ndushay commented 2 years ago

Tony C says:

When the input image has the orientation flag set, it is not being honored by KDU compress.

The way to handle this would be to rotate the image when saving the temporary tiff:

@calavano will comment on this ticket indicating how to identify the orientation flag and how to rotate the image in the tiff -- this can be done with the ruby-vips gem.

calavano commented 2 years ago

When using ruby-vips, image orientation will be preserved when passing the autorotate: true option. e.g.:

image = Vips::Image.new_from_file(input_file_name, autorotate: true)

ndushay commented 2 years ago

There is test data for a rotated image from PR #60.

It would be great to get a spec written for this so Tony doesn't have to check manually in the future; work with Tony to have a spec for this.

ndushay commented 2 years ago

It's also possible we'll delete those files from #60, or some of them - ask @calavano if we want to generate an image for this test or use an existing image.

justinlittman commented 2 years ago

I started with spec/test_data/color_rgb_srgb_rot90cw.tif:

exiftool spec/test_data/color_rgb_srgb_rot90cw.tif
ExifTool Version Number         : 12.42
*****
Orientation                     : Rotate 90 CW
*****

And then executed:

irb(main):001:0> vips_image = Vips::Image.new_from_file('spec/test_data/color_rgb_srgb_rot90cw.tif')
=> #<Image 36x43 uchar, 3 bands, srgb>
irb(main):002:0> vips_image.tiffsave('/tmp/test.tif')

Without any changes, the orientation is preserved:

exiftool /tmp/test.tif
ExifTool Version Number         : 12.42
*****
Orientation                     : Rotate 90 CW
*****

What am I misunderstanding?

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 (e.g. on stage or qa ...)

ndushay commented 2 years ago

I believe the action for this ticket is to write a spec that ensures we don't lose the correct rotation, now that we are using ruby-vips.

I believe this was busted before we switch to vips.

I think the trick will be to figure out how to query for the image rotation with vips .. in a spec.

ndushay commented 2 years ago

Andrew says (slack message):

I believe the problem is the JP2 will not have the rotation flag. So the temporary tiff should be rotated-in-the-data and not rely on the rotation flag. The ultimate test is what the JP2 comes out to.

calavano commented 2 years ago

@justinlittman The problem we've had here is that Kakadu will ignore the orientation flag in the image header and creates the jp2 based on the image data. Rotating the image data according to the orientation flag for the temporary tiff gets around this problem. This problem isn't introduced by ruby-vips, it would be solving an old problem. Until now, we've just been ensuring that accessioned images have no or the default orientation flag (default is the image origin at top-left)

ndushay commented 2 years ago

So if we use

image = Vips::Image.new_from_file(input_file_name, autorotate: true)

when creating the temporary tiff AND

we expose the saved temporary tiff (it's path at least)

then: we can have a spec that checks for the correct (rotated) pixels.

Tony can help with the test for this; pairing with Tony may be useful.