Exif orientation correction and cropping of Exif oriented images

[tonytw1]

The Grid has never dealt with EXIF oriented images.

EXIF oriented images (say from raw camera files) appear 'incorrectly' rotated in the Grid UI and cannot be cropped correctly:

This is an interesting omission which could mean:

• The Guardian probably Photoshop everything which they upload directly.
• Wire services probably preprocess all of their content as well.
• Direct upload of original camera material must be a rare operation.

This PR is an opinion on how EXIF rotation could be handled in a way which is backwards compatible and protects the option of allowing manual rotations in the future. It also leads to begin able to filter for portrait or landscape images.

There is no pressing need to merge this as it appears no Grid users require it and it is a non trivial change:

• Mapping change so requires a migration ☠️
• Requires a change to imgops ☠️

This is working for what I use it for.
Please contact me if there is any desire to use this in the future.

This PR is stacked on top of #4347 which I think is possibly a bug and can be merged independently.

What does this change?

• Exif oriented JPEG images behave is well is manually rotated JPEG images.
• Additional orientation fields on the image to document metadata based orientation decisions.

New optional orientationMetadata, orientation and orientedDimensions fields under source asset.

orientationMetadata records all of the things about the raw source file which could effect it's display orientation.
The Exif orientation tag been the first thing.

The source dimensions field remains unchanged; it is the raw width and height of source file with no correction for orientation tags.

orientedDimensions shows what the user visible dimensions of this image would be if dimensions was corrected for the tags found in orientationMetadata.

orientation records the portrait or landscape orientation of the image, based on the oriented dimensions.
Potential to expose this as a searchable field.

To reduce the impact of this change we can omit these fields if there is no orientation tag or the tag has no material impact:

Thumbnails and crops are corrected for Exif orientation.

Thumbnail and master crop generation is updated to include rotations to correct the orientation if Exif orientation metadata is found.

imops previews are explicitly rotated to account for orientation

The imgops url includes an explicit rotate parameter.
☠️ The nginx image filter imgops service needs to be updated to accept a rotate value on the query parameter r.

Cropper uses corrected oriented dimensions

The cropper UI and backend use the orientedDimension when calculating the crop bounding box.

Crops of Exif oriented source images now work as expected because the master crop has had it's orientation corrected before the other crops are made.

CropSpec includes a new optional rotation field

Which records the orientation the user saw when this crop was made (if any).
Because no Grid image has ever been rotated, all the existing crops have the correct empty rotation field set all ready.

The rotation is the actual rotation applied after all orientation corrections (and future manual rotations) have been applied.

An Exif portrait image will show a rotation of 90 degrees.
In this way, all crops can still be reproduced from the source image.

There's an implicit ordering here... to reproduce, rotate than crop; suggests that one day images transforms might be represented as a sequence of operations.

These are new Elastic search fields. so it requires an Elastic migration to apply correctly:

query failed because: mapping set to strict, dynamic introduction of [orientation] within [source.dimensions] is not allowed type:
strict_dynamic_mapping_exception root cause ElasticError(strict_dynamic_mapping_exception,mapping set to strict,
dynamic introduction of [orientation] within [source.dimensions] is not allowed

Ignoring this won't break existing indexes but will mean Exif oriented images will refuse to upload.

How should a reviewer test this change?

Tested locally from my fork of the Grid against my fork of imgops.
Milage may vary for the Guardian's implementation of imgops.

How can success be measured?

Who should look at this?

Tested? Documented?

• locally by committer
• locally by Guardian reviewer
• on the Guardian's TEST environment
• relevant documentation added or amended (if needed)

[paperboyo]

💎💎💎!
Thank you, @tonytw1, this is beyond amazing. Fixes #403, a long-time serious deficiency in Grid.

The Guardian probably Photoshop everything which they upload directly.
Direct upload of original camera material must be a rare operation.
it appears no Grid users require it

This is among the most sought-after user-facing features. Several desks (who upload original material) are most affected (magazines, Feast, Sport, incl. via FTP). Central Production must have rotated hundreds of images over the years. Impact is worse as the only way to know orientation is there is to notice Grid can’t deal with it (as everything else can) and then original upload must also be deleted. Grid is full of wrongly oriented imagery.

If there is interest in merging this feature, then the preview calls will need to be back ported from imgproxy back to imgop (not a big deal if imgops can rotation images).
The nginx image filter imgops service needs to be updated to accept a rotate value on the query parameter r

Ideally, if only at a later stage, we should switch to imgproxy. It’s VIPS-based, likely much faster, will provide modern file format support to speed up previews in the future, allow for pre-render logic simplification and, crucially, understands colour management, so fixes #894, another oldie.

It also leads to begin able to filter for portrait or landscape images.

This would be beneficial to users, especially given some more recent crop-related work by @twrichards.

protects the option of allowing manual rotations in the future

A useful future enhancement indeed, to deal with already uploaded oriented imagery and to manually fix situations where capturing device wasn’t able to record orientation correctly (looking at you, foodies).

All in all, this is like your favourite car finally getting working windshield wipers. You never thought they are essential until it rained on a trip.

Pinging @davmacbea as we know our friends at the BBC also will benefit.

[davmacbea]

Totally agree with @paperboyo on the benefit of this - thank you @tonytw1

We were just talking about this issue last week at the BBC. It's a particular problem for images coming in from our newsgathering mobile app.

[andrew-nowak]

this looks great! I'm going to try and find time to have a play

one note:

Mapping change so requires a migration ☠️

It looks like all the mapping changes are additions, so migration shouldn't be necessary, the addition should be applicable with https://github.com/guardian/grid/tree/main/scripts#updatemapping.

That said, what would happen after a migration? If the orientation metadata is read from an image which previously had it but we ignored, I think that image (not its crops!) would become rotated in the grid? Which is great for all the images in the Grid which have orientation data but unfortunate for the images which have incorrect orientation metadata, and happened to be the correct way up in spite of the incorrect data (which looks to be exceedingly rare, at least in the Guardian's catalogue, but a few do exist). I think probably not a blocker? But since we'd have this data I think I'd be very keen to add ability to rotate images in the Grid

[paperboyo]

I think that image (not its crops!) would become rotated in the grid?

That would be desirable, yeah.

(…) unfortunate for the images which have incorrect orientation metadata, and happened to be the correct way up in spite of the incorrect data (which looks to be exceedingly rare, at least in the Guardian's catalogue, but a few do exist)

Acceptable edge case, I would say? And one day will be sorted when we add UI to store userMetadata overrides for orientation.

[tonytw1]

It looks like all the mapping changes are additions, so migration shouldn't be necessary, the addition should be applicable with https://github.com/guardian/grid/tree/main/scripts#updatemapping.

Yes. All the mapping changes are additions.
If this can be deployed without a migration then that's good to know.

which have orientation data but unfortunate for the images which have incorrect orientation metadata, and happened to be the correct way up in spite of the incorrect data

You are correct about this edge case.
The orientationMetadata field is populated from file metadata so this side effect would only appear after a reparsing of the original S3 image file. I believe this happens on a full reingest (rare event) rather than a migration (common event)?

This makes this issue a sleeper which might appear many years after the change.

It -might- be ok through; as the existing crops of these images were all still made with the editor exposed to rotation=0.
Existing crops will stay correct and can be rerendered correctly, but if a full reingest populated orientedMetadata you'd lose the ability to make any more crops with the same orientation.

[andrew-nowak]

This change adds the rotation arg to dev imgops for me (appreciate it'd be better to swap out imgops for a better service, but in the meanwhile...)

diff --git a/dev/imgops/nginx.conf b/dev/imgops/nginx.conf
index cc05582e4..cac970833 100644
--- a/dev/imgops/nginx.conf
+++ b/dev/imgops/nginx.conf
@@ -31,6 +31,7 @@ http {
       image_filter_interlace on;
       image_filter_jpeg_quality $arg_q;
       image_filter resize $arg_w $arg_h;
+      image_filter rotate $arg_r;

       # connect to the localstack docker container
       proxy_pass http://localstack:4566$request_uri;

[tonytw1]

Agree. This looks like what I used to test with imgops.

[tonytw1]

#4347 has been merged so I'm going to rebase and rereview this PR this week.

[tonytw1]

Rebased and suggestion applied.