HEIC Images and Make tag

[helgeerbe]

Discussed in #157

^{Originally posted by techtron79 August 14, 2021}
As before, I am running picframe on a Pi 3. I made the library changes described at https://www.thedigitalpictureframe.com to enable HEIC images. The changes worked and picframe does display the images. However, there are a couple of issues:

1. I get the following warning for each of the HEIC images during initialization:
   WARNING:get_image_meta.GetImageMeta:get_size failed on /home/pi/Pictures/test3.HEIC -> cannot identify image file '/home/pi/Pictures/test3.HEIC'
   As I said, the images display, so perhaps I should just ignore this warning? As info, when I inspect the picframe database, the height and width values for the HEIC images are 0.
2. Portrait/landscape determination does not seem to work for HEIC. A sample image is attached. picframe displays it as landscape, but other programs display it properly as portrait.
   test3_HEIC.zip

Not sure anything can be done about these issues since they may caused by the underlying libraries, but I just wanted to pass them on. Thanks for any help.

[helgeerbe]

I wrote some unit tests and found out that width and height ist not returned for heic images. The reason is, that PILLOW Image.size()is not working for heic images. See get_image_meta.py

def get_size(self):
        try: # corrupt image file might crash app
            return Image.open(self.__filename).size
        except Exception as e:
            self.__logger.warning("get_size failed on %s -> %s", self.__filename, e)
            return (0, 0)

If it's fine for you, I will test first for exif tags widthand height and will just use Image.size()as fallback.

One other thing is, that Maker is trying to use "Exif Make" but it should be "Image Make". I will change it, too.

[helgeerbe]

Interessing. In the heic example width and height could be extracted with

width = self.get_exif('EXIF ExifImageWidth')
height = self.get_exif('EXIF ExifImageLength')

My jpg sample images don't have any tags for width and height. This information seems to be extracted directly from the jpg file.

Does anybody of you has other examples for width and height tags?

[jgodfrey]

If it's fine for you, I will test first for exif tags widthand height and will just use Image.size()as fallback.

My feeling is that EXIF data (or image meta data in general) is a bit of a minefield as there are so many applications that just aren't "good citizens" when it comes to properly maintaining it.

I'd be a little concerned about trusting width/height values found in EXIF tags as they're likely to be "copies" of some original data that wasn't updated after a resize or crop operation. Certainly, the EXIF tags should be somewhere in the list of "things to try if more obvious things have failed", but they should be quite far down the list in my opinion.

I mentioned it elsewhere here, but I wonder about using phheif (already used by the project) for this case? I haven't tried it, but it does (apparently) support width/height data for HEIC images.

[helgeerbe]

• @jgodfrey you are right. I will try to use pheif for heic and heif images to extract width and hight.
• will correctImage Make instead of Exif Make,
• Image Make should be exported by mqtt as well (maybe reading the wrong tag, this field was alway empty)
• Cache is extracting Exif Rating. I don't have any image where this tag is set. Lightroom is using XMP Rating.
  • Should XMP Ratingbe used instead?
  • Exporting rating might be a good idea, if we provide also a filter (e. g. show only images with 5 star rating)?

I will fix heif support and the make tag.
Rating is more a kind of enhancement. If needed we should create a new enhancement issue.

[paddywwoof]

For .heic and .heif files we have to do

picframe/picframe/viewer_display.py

Line 176 in 64e1fa9

def __check_heif_then_open(self, fname):

which uses Image.readbytes but we pass it pyheif.read(self.__filename).size so that should be the mechanism used not PIL.Image.open(self.__filename).size

I would be happier if one value of size was extracted in one place - duplicating the heic specific code in viewer_display and get_image_meta seems non-optimal to me (for maintenance, bug fixing etc), but I'm not sure the best way to do it.

[helgeerbe]

Good point. But we open images twice. First to extract the meta info, second to display the image. In order not to spread the heir/heic code I was thinking to create a image helper module containing check_heif_then_open function. This could be used in viewer_display.py as well in image_cache.py

[helgeerbe]

I moved __check_heif_then_open as a static method to GetImageMeta

@staticmethod
    def get_image_object(fname):
            ext = os.path.splitext(fname)[1].lower()
            if ext in ('.heif','.heic'):

Considering the image object as meta information this works.

So in viewer_display.py I replaced __check_heif_then_open

# Load the image(s) and correct their orientation as necessary
            if pics[0]:
                im = GetImageMeta.get_image_object(pics[0].fname)
                if pics[0].orientation != 1:
                     im = self.__orientate_image(im, pics[0].orientation)
                if im is None:
                    return None
            if pics[1]:
                im2 = GetImageMeta.get_image_object(pics[1].fname)
                if pics[1].orientation != 1:
                     im2 = self.__orientate_image(im2, pics[1].orientation)

So now same code is used to open an image anywhere.

[jgodfrey]

@helgeerbe,

Looks like it was already this way in the existing code, but since you're in there now... We should probably reorder that check for a None image. So, this (in two places):

if pics[0]:
    im = GetImageMeta.get_image_object(pics[0].fname)
    if im is None:
        return None
    if pics[0].orientation != 1:
         im = self.__orientate_image(im, pics[0].orientation)

[helgeerbe]

@jgodfrey

I changed it that way

            # Load the image(s) and correct their orientation as necessary
            if pics[0]:
                im = get_image_meta.GetImageMeta.get_image_object(pics[0].fname)
                if im is None:
                    return None
                if pics[0].orientation != 1:
                     im = self.__orientate_image(im, pics[0].orientation)
                
            if pics[1]:
                im2 = get_image_meta.GetImageMeta.get_image_object(pics[1].fname)
                if im2 is None:
                    return None
                if pics[1].orientation != 1:
                     im2 = self.__orientate_image(im2, pics[1].orientation)

There was no check for im2 is None. Hope it is fine to return None, if the opening of the second image pair fails.

[helgeerbe]

OK, I found out, why the heic image is displayed with the wrong orientation. Reading the exif:

• DEBUG:exifread: ExifImageWidth: (0xA002) Long=4032 @ 526
• DEBUG:exifread: ExifImageLength: (0xA003) Long=3024 @ 538
• DEBUG:exifread: Orientation: (0x0112) Short=Rotated 90 CW @ 42 => 6

But converting the heic image to a PIL.Image object rotates the image implicitly. So Image.size returns:

• width 3024
• height 4032

Due to orientation = 6 we rotate the image explicitly again.

The solution is simple. get_orientation will alway return 1 for heic images. The only disadvantage I see, if you do some statistics about orientation (e. g. how many images are taken cw 90 degrees?). This will not work for heic images.

[jgodfrey]

But converting the heic image to a PIL.Image object rotates the image implicitly

I haven't looked, but is that a documented behavior?

[helgeerbe]

I found this strukturag/libheif#227 . So it seems to be a known issue. Problem is, we are reading exif info before converting the heif image to an PIL.Image object. In my understanding it's OK to assume that all heif PIL.Images objects have an oriation = 1.
I've seen code examples which removes the orientation tag before saving a converted PIL.Image object as jpg with exif-metadata.

[helgeerbe]

I tested it with some iphone images. I took pics rotating the phone 90 degrees for each pic. All 4 images are displayed now correctly.

[jgodfrey]

The solution is simple. get_orientation will alway return 1 for heic images. The only disadvantage I see, if you do some statistics about orientation (e. g. how many images are taken cw 90 degrees?). This will not work for heic images.

I'm not wild about the db containing "incorrect" orientation data for HEIC images. That is, a HEIC image may very well be rotated in the image file, but we'd now record it as 1 (upright) because we know it'll be fixed automatically when it's opened. If PIL stops auto-orienting this case, or if we'd stop using PIL for some reason, we would no longer have the info in the db needed to properly orient the image.

I wonder if it might be better to continue to store the actual image orientation in the db (as we do today), but NOT reorient HEIC images when loading them for display?

To do that, we'd either have to not make these calls for HEIC images:

picframe/picframe/viewer_display.py

Line 276 in 64e1fa9

if pics[0].orientation != 1:

or maybe update __orientate_image() to return the original image unchanged if called with a HEIF image (though, it'd need to know the image type, which it doesn't know today).

Just thinking out loud. Any thoughts?

[jgodfrey]

Hope it is fine to return None, if the opening of the second image pair fails.

Yeah, I think that's OK. We could be more clever in this case and display the first (already validated) image of the pair by itself. But, short of that, the changes are definitely an improvement over what we had.

[helgeerbe]

Should be closed with merge #165