Align pixel orientation with exif:Orientation in HEIC decoder #1233

rudchenkos · 2018-07-31T13:13:21Z

It turns out that in HEIC the pixels are never rotated and the metadata only
indicates the orientation of the sensor when the photo was taken.

Hence, there was a discrepancy between Image->orientation and actual pixel
orientation in images loaded with the heic coder. Particularly, this led to
incorrect behavior of the -auto-orient which in this case tried to compensate
non-existing pixel rotation.

This is a fix for #1232

The fix provides two solutions for the problem described above: reset exif:Orientation to "1" in HEIC decoder output (default behavior) or rotate pixels to match the original exif:Orientation, as some application might rely on the original sensor rotation. The latter is enabled by heic:preserve-orientation read define.

It turns out that in HEIC the pixels are never rotated and the metadata only indicates the orientation of the sensor when the photo was taken. Hence, there was a discrepancy between `Image->orientation` and actual pixel orientation in images loaded with the `heic` coder. Particularly, this led to incorrect behavior of the `-auto-orient` which in this case tried to compensate non-existing pixel rotation. ImageMagick#1232

rudchenkos · 2018-07-31T13:16:18Z

A question to the maintainers: where in the docs is the best place to describe the new option heic:preserve-orientation?

dlemstra · 2018-07-31T21:02:18Z

Is it not enough to call SetImageProperty(image,"exif:orientation","1",exception); after the image has been read? The exif profile will be updated before the image is written with the new value.

rudchenkos · 2018-08-01T07:37:36Z

@dlemstra Absolutely. In most common cases it is really enough to reset exif:Orientation after the image is read. This is is the default behavior after my patch.

Unfortunately, there are application which rely on the original pixel rotation for historical reasons. This is why I have also implemented the option heic:preserve-orientation.

dlemstra

Can you do the final tweaks so we can merge this?

dlemstra · 2018-08-31T19:41:09Z

coders/heic.c

@@ -85,6 +86,8 @@ static MagickBooleanType
  WriteHEICImage(const ImageInfo *,Image *,ExceptionInfo *);
 #endif

+static Image *CompensateOrientation(Image *image, ExceptionInfo *exception);


Could you move this method above the ReadHEICImage method, near IsHeifSuccess? We can then remove this line.

Of course, will do 👍

dlemstra · 2018-08-31T19:44:08Z

coders/heic.c

+   */
+  option=GetImageOption(image_info,"heic:preserve-orientation");
+  if (IsStringTrue(option) == MagickTrue)
+    image = CompensateOrientation(image, exception);


This statement leaks memory. The methods FlipImage, RotateImage return a new image.

Oh, my. That's right, I will free the old image here

dlemstra · 2018-08-31T19:44:55Z

coders/heic.c

  return(GetFirstImageInList(image));
 }
+
+/* An inverse of AutoOrientImage */
+static Image *CompensateOrientation(Image *image, ExceptionInfo *exception)


Image *image should be const Image *image.

Makes sense, I will change it to a const pointer type

There is a problem. There is a bypass statement which needs to return a non-const result from the const parameter:

value=GetImageProperty(image,"exif:Orientation",exception); if (value == NULL) { return image; }

So I can factor this EXIF check out of this function (to the call site at ReadHEICImage) or simply don't put the const. Which way do you think is better?

Good point. I missed that, probably better to not change the signature to a const.

rudchenkos · 2018-09-03T08:01:06Z

Hi @dlemstra,
I have applied the suggestions in the fixup commit (to be squashed).

About the docs, it seems the most appropriate place to document heic:preserve-orientation is command-line-options.html right after hough-lines:accumulator description. One thing is confusing me though, that line (www/command-line-options.html:84) is very long which makes me think it is generated by a tool, rather than written manually. But, from the other hand I cannot find where from and how could it have been generated.

Should I just edit that command-line-options.html?

dlemstra · 2018-09-03T10:42:14Z

You should edit the command line options here: https://github.com/ImageMagick/Website

dlemstra · 2018-09-03T10:43:35Z

coders/heic.c

+      Image
+        *original_image = image;
+      image = CompensateOrientation(original_image, exception);
+      DestroyImageList(original_image);


You should handle this inside the CompensateOrientation method. You are now destroying the image when exif:Orientation is not set.

Oh, true. Fill fix promptly.

Pushed.
I am sorry for the silly mistake

rudchenkos · 2018-09-03T12:03:07Z

Pull request for docs: ImageMagick/Website#19

dlemstra

Thanks for the patch. Needs a few code style tweaks but I will apply them myself.

rudchenkos · 2018-09-03T18:35:01Z

Dank u wel, Dirk!

)

dlemstra reviewed Aug 31, 2018

View reviewed changes

rudchenkos force-pushed the fix-heic-orientation branch from 133db37 to fd199ae Compare September 2, 2018 19:10

dlemstra requested changes Sep 3, 2018

View reviewed changes

fixup! Align pixel orientation with exif:Orientation in HEIC decoder

84a98b1

rudchenkos force-pushed the fix-heic-orientation branch from fd199ae to 84a98b1 Compare September 3, 2018 11:30

rudchenkos mentioned this pull request Sep 3, 2018

Describe 'heic:preserve-orientation' ImageMagick/Website#19

Merged

dlemstra approved these changes Sep 3, 2018

View reviewed changes

dlemstra merged commit 41fa21a into ImageMagick:master Sep 3, 2018

eriken mentioned this pull request Sep 21, 2018

rotate is broken since 0.2.2 monostream/tifig#26

Closed

liuzhe0223 mentioned this pull request Dec 26, 2018

Orientation info from HEIC decoder does not match actual pixel orientation ImageMagick/ImageMagick6#32

Closed

liuzhe0223 mentioned this pull request Jan 2, 2019

Fix 'heic:preserve-orientation' ImageMagick/Website#21

Merged

skmskmskmskm mentioned this pull request Jul 14, 2019

Wrong orientation for HEIC image d2phap/ImageGlass#559

Closed

rudchenkos deleted the fix-heic-orientation branch April 26, 2020 12:14

rudchenkos added a commit to rudchenkos/ImageMagick that referenced this pull request Apr 27, 2020

fixup! Correct HEIC orientation for the ping operation (ImageMagick#1233

f081564

)

dlemstra pushed a commit that referenced this pull request Apr 27, 2020

Correct HEIC orientation for the ping operation (#1233) (#1940)

17f60ed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Align pixel orientation with exif:Orientation in HEIC decoder #1233

Align pixel orientation with exif:Orientation in HEIC decoder #1233

rudchenkos commented Jul 31, 2018 •

edited

rudchenkos commented Jul 31, 2018

dlemstra commented Jul 31, 2018

rudchenkos commented Aug 1, 2018

dlemstra left a comment

dlemstra Aug 31, 2018

rudchenkos Sep 2, 2018

dlemstra Aug 31, 2018

rudchenkos Sep 2, 2018

dlemstra Aug 31, 2018

rudchenkos Sep 2, 2018

rudchenkos Sep 2, 2018 •

edited

dlemstra Sep 2, 2018

rudchenkos commented Sep 3, 2018

dlemstra commented Sep 3, 2018

dlemstra Sep 3, 2018

rudchenkos Sep 3, 2018

rudchenkos Sep 3, 2018

rudchenkos commented Sep 3, 2018

dlemstra left a comment

rudchenkos commented Sep 3, 2018

Align pixel orientation with exif:Orientation in HEIC decoder #1233

Align pixel orientation with exif:Orientation in HEIC decoder #1233

Conversation

rudchenkos commented Jul 31, 2018 • edited

rudchenkos commented Jul 31, 2018

dlemstra commented Jul 31, 2018

rudchenkos commented Aug 1, 2018

dlemstra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rudchenkos Sep 2, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rudchenkos commented Sep 3, 2018

dlemstra commented Sep 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rudchenkos commented Sep 3, 2018

dlemstra left a comment

Choose a reason for hiding this comment

rudchenkos commented Sep 3, 2018

rudchenkos commented Jul 31, 2018 •

edited

rudchenkos Sep 2, 2018 •

edited