Silverstripe-framework: Image gets rotated on upload

[mediabeastnz]

When adding an image via either the files tab or via the html editor certain images seem to get rotated either by 90 degrees or a complete 180.

I'm guessing this has something to do with the image it's self as it doesn't happen to all of them.

The original upload is fine it's just the re sample version that gets rotated.

[sb-relaxt-at]

Maybe the exif information for orientation is set on the images (typically used by digital cameras for portrait shots) and this value might be discarded when resizing the image (90 degree mismatch) or interpreted and not deleted (resulting in a 180 degree mismatch).

P.S.: Just tried it, i can reproduce wrong rotation in silverstripe 3.0.x using an image with Exif Orientation: "Rotate 270 CW". I guess we would need either a call to imagerotate in ImagickBackend (depending on the Exif-Orientation of the image) or retain the orientation information when resizing the image.

[mediabeastnz]

This still seems to happen on 3.1.13

[micschk]

This module provides a quick fix for now (and maybe a solution to be implemented in core?): https://github.com/thisisbd/silverstripe-fixjpeg-orientation/blob/master/code/ImageExtension.php

[tractorcow]

This sounds like a fun bug to fix on a day someone gets the time.

In 4.x assets has changed quite a great deal; It's probably a good idea to address this there. Maybe fix it in UploadField even before being assigned to an image?

[dhensby]

This is most likely from images that have rotation meta data because they were taken on a phone

[digitall-it]

If you try to load the images in Github, they appear rotated too. So it is a problem with GD or ImageMagick. Meanwhile, if you are on Windows, use this program to correct whole folders at once.

[digitall-it]

This function could do the trick with plain GD and also checks for support.

function correctImageOrientation($filename) {
  if (function_exists('exif_read_data')) {
    $exif = exif_read_data($filename);
    if($exif && isset($exif['Orientation'])) {
      $orientation = $exif['Orientation'];
      if($orientation != 1){
        $img = imagecreatefromjpeg($filename);
        $deg = 0;
        switch ($orientation) {
          case 3:
            $deg = 180;
            break;
          case 6:
            $deg = 270;
            break;
          case 8:
            $deg = 90;
            break;
        }
        if ($deg) {
          $img = imagerotate($img, $deg, 0);        
        }
        // then rewrite the rotated image back to the disk as $filename 
        imagejpeg($img, $filename, 95);
      } // if there is some rotation necessary
    } // if have the exif orientation info
  } // if function exists      
}

[digitall-it]

Solution proposed by @micschk seems almost the same and already integrated, we should follow that instead.

[dhensby]

looks good, let's PR!

[digitall-it]

Tried thisisbd/silverstripe-fixjpeg-orientation on a fresh installation of v3.4.1 and it works fine.

[sb-relaxt-at]

Based on the comment in this fix, I assume that all exif data is being removed when rotating the image. While removing the Orientation field is required, it might be not desired for other fields, as there might be information concerning the location, the artist, camera in use and so on.

[digitall-it]

@sb-relaxt-at Yes, also there should be checked if rotation is actually lossless. We could use jhead but is a command line tool, surely lossless and just strips the orientation tag.

[SpiritLevel]

Just wondering if this animated image issue should be considered at the same time as the rotation one above, and, as @tractorcow suggests, for SS4 as well ?

More generally, it seems that there is a need for a SS-specific collection of helper functions to deal with things GD and Imagick don't handle quite the way we'd like. Despite what @dhensby says in the animated image issue, I think supporting half a dozen helper functions might be a good idea and not too onerous; there are lots of code snippets already out there to borrow from, requiring only minor tweaking to fit into SS, and good image handling would make SS stand out. Just thinking out loud...

[dhensby]

I'm 100% behind adding features to the image class offered by SS.

Regarding the issue with resizing animated gifs, I was only saying it's not a bug because SS _currently_ makes no promise to support resizing of gifs.

FWIW for 4.0 I'd much rather we scrap our image manipulation backends for something like https://github.com/Intervention/image than waste time adding features to ours that are already built somewhere else.

[SpiritLevel]

Thanks @dhensby

Sorry to misinterpret/misrepresent your prior comment :)

Yes, Intervention looks good. But isn't replacing the image manipulation backend a big job ? And, if this was opted for, would we not still need a few helper functions to deal with the immediate needs in 3.x ? Or, can Intervention go into 3.x "easily" ?

[digitall-it]

I opened an issue on the libgd library regarding this.

Another approach can be a little more drastical: change the resizing functions so that the rotation happens there, and not on image upload. As during resizing and resampling EXIF data is lost anyway, and for any other reference the original file is still present, this would do the trick.

We should create a function that acts as a wrapper around imagecreatefromjpeg and replace it in resize functions.

I suppose the only entry point on GD is from loadfrom in GD backend. That could be easier than we think, as I suppose that using that function means business, i.e. we are about to manipulate a copy of the image and already destroying its EXIF data.

That would also mean we do not rely on users actually uploading the images _via the CMS_, and this can avoiding some edge cases if we directly manipulate the assets folder outside the _resampled subdirectories, i.e. what if the users load the files via FTP, what if their filesytem triggers an event on file change, what if they want to compare the original image with another they have on disk, what if they keep in sync a local and remote copy of the assets folder, what if they use a tool like sspak, what if they restore a backup, and so on.

As this reduces the problem to a trivial solution with no apparent consequences, could I create a PR with this proposal? It will only work on GD, as I didn't check if ImageMagick is affected by the same issue (according to an Internet search, it seems like yes). If that's the case, a similar function can be injected here after setDefaultQuality().

[dhensby]

I like the idea of rotate when re-sampling BUT how does that affect the original image when it's displayed without being resampled?

[digitall-it]

The original image, when not resampled, should be treated by the OS and the browser, as they are orientation aware. I.e. when you load an image with orientation in Github, it appears incorrectly rotated if seen in the context of the chrome of the site, because it gets resampled by a non exif aware library. When indeed you open the original file either in-browser as a direct connected link or as a forced download, it gets to be shown correctly, as the major browsers and file browsers (Finder, Explorer and so on) know how to display them.

[sb-relaxt-at]

Unfortunately most browsers are not orientation aware when referring an image in an img tag. For example have a look at the following test image which is not rendered correctly (in Chrome). It is indeed rendered correctly when opened directly. There is no non-orientation aware library involved.

This seems to be the major reason against rotation-on-resampling. We are currently using a custom module for SilverStripe that rotates uploaded images immediately after upload. This seems to be the best way in order to allow using the "original" image as well.

P.S.: These are really useful test images: recurser/exif-orientation-examples

[digitall-it]

The rendering of images when transmitted in bit-per-bit fidelity is not the CMS concern. Thus, this type of workflow should be reserved for downloads, as we do not know if the image can contain custom metadata. Indeed, if this is the concern, we can add a function that returns the image autorotated at original size (will require resampling; will strip EXIF data; will not change original file, that is still available to read metadata).

I'm against the autorotation on upload, I'm in favour of autorotation on resize/crop/resample.

[digitall-it]

Please note that the issue opened on the GD library is going on; GD will apply autorotation on resampling itself in the near future. This could require no action by the Silverstripe CMS itself, OR it could check for functionality and autorotate on resize/crop/resample. Also note that GD library is implementing lossless transformations and EXIF data retaining on resampling.

[digitall-it]

@sb-relaxt-at you do not see the image correctly rendered just because it is resampled by GitHub in its own site chrome (GitHub is using a not EXIF aware library, probabily GD or ImageMagick theirselves) , but if you open it directly, Chrome will render it correctly (the original image retains the required EXIF data to render it correctly, and I am referring to orientation, color space, color profiles, and so on), thus reinforcing the argument in favour of autorotation on resample, not against it.

Indeed, all the test images you have shown, render correctly in the modern browsers when opened directly, and incorrectly when resampled by GitHub.

[sb-relaxt-at]

@digitall-it well I do agree that automatic rotation on upload is something that needs to be evaluated carefully, as it might not be the best default behaviour for all sites. A getter returning the rotated image in full resolution might be fine as well.

I just wanted to point out, that browsers do not respect exif orientation information when embedding an image via an img tag. The example presented does not use any resampling, it is just the raw image. But in order to show this more clearly, compare: in img tag versus raw image

P.S.: I just learned, that there might be a CSS property going to solve this.

[digitall-it]

Very clever, the CSS property is not very well supported yet, but interesting. Didn't bother to check out, you are right, the problem is in the img tag.

So, what are the use cases against moving the autorotation to the resampling phase instead of the uploading one? We could still have an autoRotate() function to be called if we need to display the image without resizing or cropping and leverage SS built-in caching functionality. I would rather prefer to having to apply it on my sites manually in each position I need a full size image in a tag (i.e. almost never) instead of having my EXIF data magically stripped off on all the images on upload without knowing why. As SS also has img tag generation abilities, this function chaining can be automated there. If instead the programmer wants to generate his own tag, and show the image in a tag, he has to chain an autoRotate function prior to display.

To make this function future-proof it could be called prepareForDisplay or something like that, as this could fixing other potential future issues (HDR images, live photos, and so on).

[dhensby]

Keeping a copy of the original image makes sense, rotating on manipulation also makes sense, though this still doesn't solve the problem where we want to load the original image on site but oriented correctly.

If GD is going to fix this problem upstream, how will that affect our patch/poly-fill?

[digitall-it]

@dhensby an appropriated chain function can be called to prepare image for display if no resizing is required. This function will strip EXIF metadata and will be applied automatically if using the framework image tag generation code or forTemplate. Some examples in template code:

• <p>$Image</p> outputs a complete image tag with a corrected image
• <p><img src="$Image.URL"></p> outputs a non corrected image
• <p><img src="$Image.prepareForDisplay.URL"></p> outputs a corrected image

Corrected image is fixed for display if needed (autorotation, one of the possible operations, can involve EXIF stripping and a little degradation as it essentially is a resampling at original size). The umbrella method can fix other problems in the future like incorrect colors or proportions, and easily unit tested.

For backward compatibility, I proposed in the GD issue that to avoid overcompensation the library should enable to turn off the feature, and/or to test for feature presence (if GD && !GD->exifAware()) as indeed the GD library is working to provide a better native implementation that an upper polyfill.
I proposed lossless transformation and EXIF data retention and they are already working on it.

[dhensby]

Can you provide a link to the issue with GD? Is Imagick working on it too?

[digitall-it]

Here there is a link to the issue with GD.

ImageMagick does that by passing an operator called -auto-orient as per their docs, but their whole paragraph "JPEG Format is Lossy" is outdated, as there are new techniques to rotate images without reducing their quality (formally I think a matrix rotation on iDCT coefficients data). Their docs go over jhead that implements it a few paragraphs later. Did not conduct any test to see if EXIF data is retained on transformations.

A fix in the backends to be triggered on image manipulation could implement both solutions. Can even try to use jhead if found in path.

[tractorcow]

I suggest that we only re-rotate images on resampling, but don't touch original images.

[dhensby]

Fixed in 4. A fix in 3 will be too disruptive