webtrees icon indicating copy to clipboard operation
webtrees copied to clipboard

EXIF orientation is ignored when generating thumbnails

Open gmarcon opened this issue 2 years ago • 9 comments

The EXIF Orientation attribute is ignored when generating thumbnails, leading to wrongly orientated thumbnails when the EXIF orientation field is different than 1.

This is due to the wrong usage of the Intervention Image library, as the orientate method needs a file path and not a stream to work correctly (see https://image.intervention.io/v2/api/orientate "Image object must be instantiated from file path to read the EXIF data correctly"), and media files in Webtrees are always abstracted through the Flysystem storage library that only provides streams and not access to the physical file.

This is surely a limitation of the Intervention Image library (that probably loses the orientation EXIF data when creating the image object from the stream), but it is not a bug in the library (as the documentation clearly states that it does not support that functionality).

A workaround in Webtrees should be easy to create, as in my understanding media files can be either local files or URLs, and thumbnails are generated only for local files.

I dumped the return of the exit() function with var_export for a file read through Flysystem from within ImageFactory, using as publicly available test image:

array (
  'FileDateTime' => 0,
  'FileSize' => 593747,
  'FileType' => 2,
  'MimeType' => 'image/jpeg',
  'SectionsFound' => 'COMMENT',
  'COMPUTED' =>
  array (
    'html' => 'width="1800" height="1200"',
    'Height' => 1200,
    'Width' => 1800,
    'IsColor' => 1,
  ),
  'COMMENT' =>
  array (
    0 => 'CREATOR: gd-jpeg v1.0 (using IJG JPEG v80), quality = 90
',
  ),
)

And the same reading the file directly from the file system, where the Orientation EXIF attribute can be seen, confirming that it is a missing feature in the Intervention Image library:

array (
  'FileName' => '1aeed661a2abb51a129cc224482f4760a8dd4299.jpg',
  'FileDateTime' => 1667762377,
  'FileSize' => 348796,
  'FileType' => 2,
  'MimeType' => 'image/jpeg',
  'SectionsFound' => 'ANY_TAG, IFD0',
  'COMPUTED' =>
  array (
    'html' => 'width="1800" height="1200"',
    'Height' => 1200,
    'Width' => 1800,
    'IsColor' => 1,
    'ByteOrderMotorola' => 1,
  ),
  'Orientation' => 3,
  'XResolution' => '72/1',
  'YResolution' => '72/1',
  'ResolutionUnit' => 2,
  'YCbCrPositioning' => 1,
)

These issues are probably for the same reason: https://github.com/fisharebest/webtrees/issues/3553 https://github.com/fisharebest/webtrees/issues/4252

gmarcon avatar Nov 06 '22 21:11 gmarcon

Thanks for this.

The upstream issue (https://github.com/Intervention/image/issues/745) was fixed with a code-change that required PHP 7.2.

So I assumed that once people upgrade their servers to PHP 7.2, their problems would disappear.

I guess not...

in my understanding media files can be either local files or URLs

At present, this is true. But the reason for using FlySystem is that I want to be able to use remote filesystems, such as DropBox, OneDrive, GoogleDrive, AmazonS3, etc.

So, although we can currently read files directly from /data/media - we can't rely on this going forward.

We would need to copy the file to a local temp-file first.

But I'm surprised/disappointed that intervention doesn't do this. It would make more sense to submit a fix upstream than for every application to code a workaround...

fisharebest avatar Nov 07 '22 07:11 fisharebest

https://www.php.net/manual/en/function.exif-read-data.php says that since PHP 7.2, it can read exif info from streams as well as files.

Testing locally, this appears to be correct:

php > var_dump(exif_read_data(fopen('image.jpg', 'rb'))['Orientation']);
int(6)
php > var_dump(exif_read_data('image.jpg')['Orientation']);
int(6)

Maybe there is a bug in this logic?

https://github.com/Intervention/image/blob/master/src/Intervention/Image/Commands/ExifCommand.php#L30

fisharebest avatar Nov 07 '22 08:11 fisharebest

The issue is surely not the standard PHP handling of EXIF data, that works correctly on a seekable stream from version 7.2. I think the problem is related to how the Intervention Image library represents an image instantiated from a stream, as soon as you instantiate the image the EXIF data is lost. The logic in https://github.com/Intervention/image/blob/master/src/Intervention/Image/Commands/ExifCommand.php#L30 cannot help, as the EXIF data is just not there anymore... (compare the two outputs above: besides Orientation also other EXIF tags are lost: XResolution, YResolution, ...). Until an update in the Intervention Image library to support the feature, a workaround in Webtrees is needed, otherwise thumbnails for images with the Orientation tag will not correctly generated. I sent you a pull request for a workaround that appears to work but feel free to correct it and please excuse my poor PHP skills.

gmarcon avatar Nov 07 '22 08:11 gmarcon

I think the problem is related to how the Intervention Image library represents an image instantiated from a stream, as soon as you instantiate the image the EXIF data is lost.

I see it now. It reads the image data from the stream, and creates a new stream from this image data.

I'll see if it is possible to fix it.

fisharebest avatar Nov 07 '22 09:11 fisharebest

I did some additional testing and noted different behaviours for the Imagick and gd drivers of the Intervention Image library when loading from a stream (I was using the gd driver in my previous tests and I noticed that the primary driver for Webtrees is Imagick, with a fall back on gd).

Imagick resets the EXIF Orientation to 0 (I dumped the image to be sure that it was not automatically rotated with ->orientate() and it was not, therefore still wrong thumbnails in Webtrees), while gd loses all EXIF information.

I opened the following issue as a feature request in the Intervention Image project: https://github.com/Intervention/image/issues/1190

gmarcon avatar Nov 07 '22 14:11 gmarcon

I've created a fix for the issue in intervention/image. I'll post the PR shortly.

fisharebest avatar Nov 07 '22 18:11 fisharebest

I submitted a few PRs. The relevant one is https://github.com/Intervention/image/pull/1192

I tested this with webtrees, and it works as expected.

fisharebest avatar Nov 07 '22 18:11 fisharebest

My PR was just declined (along with many others), as the intervention library has now been rewriten. The documentation says that autorotate should now work.

The latest version of the library only works with PHP 8.1, whereas webtrees 2.1 needs to support PHP 7.4

I will update the development code (will become webtrees 2.2) to use it, but obviously won't be able to back-port the fix to the 2.1 branch.

fisharebest avatar Dec 14 '23 13:12 fisharebest

Looking forward to v2.2 then. I still have thumbnails rotated the wrong way.

miqrogroove avatar Dec 15 '23 12:12 miqrogroove