Add support for parsing auxiliary images

[johncf]

Fixes (partially) #117 .

Changes proposed in this pull request:

• Added a property aux_image_ids to CtxImage that returns a list of non-depth non-alpha auxiliary image IDs.
• Added a method get_aux_type(aux_id) to CtxImage that returns a string representing the aux_type of the given aux_id.
• Added a key aux to HeifImage.info dict which is also a dict mapping from aux_type strings to a list of aux_ids.
• Added a method get_aux_image(aux_id) to CtxImage that returns a CtxImage corresponding to the given aux_id.
• Added a method get_aux_image(aux_id) to HeifImage which returns an instance of a new class HeifAuxImage wrapping the above CtxImage, after ensuring that the auxiliary image is 8-bit monochrome.
• Added a method get_aux_image(aux_id) to HeifFile which calls the same method of the primary image.

[bigcat88]

After this line

pillow_heif/pillow_heif/misc.py

Line 480 in b4b5d64

self.depth_image_list: list = []

should be added self.aux_image_ids: list = []
at least for tests to start and not fail on the first one

[bigcat88]

second:

GitHub was right, please return to_pillow implementation to the HeifDepthImage or test_depth_image will fail.

that step is need to copy info dict. (at least for now)

as I understand we do not need any additional metadata for AUX the images so in HeifAuxImage we does not need to overload to_pillow method, but for HeifDepthImage need to keep old behaviour to not break anything.

---

third:

please add some tests for aux images. It will be nice to have coverage for all code except raise NotImplementedError

---

in general it looks fine to me such implementation.

[johncf]

Thanks for the review. I'll make those changes later today or tomorrow. I wanted to get your opinion on a slightly different way of representing this auxiliary info. From a usage perspective, people will likely have an aux-type in mind to access. So with what I proposed above, they'll have to iterate through all the aux IDs and check for that aux-type. How about having heif_image.info["aux"] map to a dict from aux_type to a list of aux_ids?

So I can do:

aux_type = "urn:com:apple:photo:2020:aux:hdrgainmap"
assert aux_type in heif_image.info["aux"]
aux_id = heif_image.info["aux"][aux_type][0]
hdrgainmap = heif_image.get_aux_image(aux_id)

instead of something like:

aux_type = "urn:com:apple:photo:2020:aux:hdrgainmap"
aux_ids = [aux_id for aux_id, metadata in heif_image.info["aux"].items()
                  if metadata["type"] == aux_type]
assert len(aux_ids) > 0
hdrgainmap = heif_image.get_aux_image(aux_ids[0])

What do you think?

---

Side note: Please take a look at #298 too

[bigcat88]

What do you think?

Personally, I am satisfied with both methods, but I am more interested in how I can get these aux images when using Pillow?

[johncf]

GitHub was right, please return to_pillow implementation to the HeifDepthImage or test_depth_image will fail.

that step is need to copy info dict. (at least for now)

You're right! That was a mistake on my end. I had misunderstood the code when I made that change.

as I understand we do not need any additional metadata for AUX the images so in HeifAuxImage we does not need to overload to_pillow method, but for HeifDepthImage need to keep old behaviour to not break anything.

True, but I've added an overloaded method for HeifAuxImage too since it might be useful to check info["aux_type"] later on. (Update: I felt it's better to keep HeifAuxImage simple for now.)

please add some tests for aux images.

I'll add tests soon, and I have a sample heic image that I took on my iphone (having hdrgainmap) and cropped so the size is ~230KB. Shall I simply add it to the repo at tests/images/heif_other? (Does this repo use Git LFS?)

but I am more interested in how I can get these aux images when using Pillow?

Right now, only registering a pillow load hook will not be enough to access AUX images since they're not available directly from info. One will need to use pillow_heif directly something like this:

heif_file = pillow_heif.open_heif("hdr-sample.heic")
aux_id = heif_file.info["aux"]["urn:com:apple:photo:2020:aux:hdrgainmap"][0]
aux_image = heif_file.get_aux_image(aux_id)

The reason I avoided loading AUX images during the initialization of HeifImage is because we currently don't support all possible aux image formats. So if we simply ignore such aux images, a user might think that the aux image is missing from their file (or at least take some time to realize that it's a feature that pillow-heif is missing). But the way I've implemented it now communicates the library's limitations clearly. Do you have an alternate suggestion?

[bigcat88]

I'll add tests soon, and I have a sample heic image that I took on my iphone (having hdrgainmap) and cropped so the size is ~230KB. Shall I simply add it to the repo at tests/images/heif_other? (Does this repo use Git LFS?)

No, repo does not use Git LFS. And yes, just add the file to the tests/images/heif_other

The reason I avoided loading AUX images during the initialization of HeifImage is because we currently don't support all possible aux image formats. So if we simply ignore such aux images, a user might think that the aux image is missing from their file (or at least take some time to realize that it's a feature that pillow-heif is missing). But the way I've implemented it now communicates the library's limitations clearly. Do you have an alternate suggestion?

what you said makes sense, since it's a feature in the beta version, it doesn't have to be implemented for the pillow_plugin.

[codecov]

Codecov Report

Attention: Patch coverage is 95.45455% with 1 line in your changes missing coverage. Please review.

Project coverage is 99.89%. Comparing base (ba64851) to head (62f733f).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
pillow_heif/heif.py	94.11%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##            master     #297      +/-   ##
===========================================
- Coverage   100.00%   99.89%   -0.11%     
===========================================
  Files           10       10              
  Lines          956      976      +20     
===========================================
+ Hits           956      975      +19     
- Misses           0        1       +1     

Files with missing lines	Coverage Δ
pillow_heif/__init__.py	100.00% <ø> (ø)
pillow_heif/as_plugin.py	100.00% <100.00%> (ø)
pillow_heif/misc.py	100.00% <100.00%> (ø)
pillow_heif/options.py	100.00% <100.00%> (ø)
pillow_heif/heif.py	99.64% <94.11%> (-0.36%)	⬇️

[johncf]

One of the images in the repo already had hdrgainmap aux image. So I just used it in the new test.

[bigcat88]

As far as I understand, the PR is ready?

If it is not too much trouble, could you also modify this test:

pillow_heif/tests/options_test.py

Line 27 in ba64851

def test_options_change_from_plugin_registering(register_opener):

In theory, three lines should be added there, just like with DEPTH_IMAGES

[johncf]

As far as I understand, the PR is ready?

Yes

three lines should be added there, just like with DEPTH_IMAGES

Done!

[bigcat88]

Thanks a lot for the great PR, the release will most likely be at the end of the week.