Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose transformations #555

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

homm
Copy link
Contributor

@homm homm commented Oct 28, 2021

This PR introduce interface for reading transformations for given heif_image_handle.

Rationale

The current rotation implementation is pretty straight and that is why inefficient on modern CPUs. For example, It could be up to 8.5 times slower than Pillow's implementation.

In [2]: from HeifImagePlugin import Image, pyheif

In [3]: data = open('./full.heic', 'rb').read()

In [5]: %%timeit 
   ...: heif_file = pyheif.read(data) 
   ...: image = Image.frombytes( 
   ...:     heif_file.mode, heif_file.size, heif_file.data,
   ...:     "raw", heif_file.mode, heif_file.stride)
   ...:
683 ms ± 11 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

In [6]: %%timeit 
   ...: heif_file = pyheif.read(data, apply_transformations=False) 
   ...: image = Image.frombytes( 
   ...:     heif_file.mode, heif_file.size, heif_file.data,
   ...:     "raw", heif_file.mode, heif_file.stride)
   ...:
387 ms ± 10.7 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

In [13]: %%timeit 
    ...: heif_file = pyheif.read(data, apply_transformations=False) 
    ...: image = Image.frombytes( 
    ...:     heif_file.mode, heif_file.size, heif_file.data,
    ...:     "raw", heif_file.mode, heif_file.stride)
    ...: image = image.transpose(Image.ROTATE_270) 
    ...:
422 ms ± 5.66 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

Time spent to rotation in libheif: 683 - 387 ms = 296 ms.
Time spent to rotation in Pillow: 422 - 387 ms = 35 ms.

There is already an flag ignore_transformations which could be used to prevent libheif from transforming the image. Unfortunately there was no way to get image transformation properties from the file.

So, the new struct heif_transformations is introduced. It describes image's transformations (information from irot, imir and clap boxes) in strict order (unlike in the file itself, where operations could be in arbitrary order).

In addition, for some cases, transformation from this struct could be used for EXIF construction and actual image rotation could be totally avoided.

@bigcat88
Copy link
Contributor

An additional flag would also be useful to perform all transformations except for the final rotation as @kmilos says.
Just expose and read transformations is not enough, that will disable cropping, that used for non-odd size images.

@homm
Copy link
Contributor Author

homm commented Jun 19, 2023

I see there is "API to list all transformation properties" was added by @farindk in 602baf4 (v1.16.0) without any pull requests, discussion. Also there was no feedback on this pull request.

Implemented API works with heif_context and heif_item_id. As well known, the structure which holds both of this values is heif_image_handle. It is widely used in libheif API to get all properties of the image, including size, metadata, related depth and auxiliary_images. There was no functions in previous version where you need something else than heif_image_handle.

You can look at how workflow is organized in pyheif, for example. Please note, there is no usage of context outside of _read_heif_container. Any other information is received from handler. This was a good design.

Starting from v1.16.0 we need to store the context somewhere else while working with image handlers. There is a way to get heif_item_id required by heif_image_handle_get_item_id, but there is no way to get context back from the handler. Unfortunately this is not a good design anymore. I need a wrapper for handler to store both handler, item_id and context despite the fact that handler is this wrapper itself.

@bigcat88
Copy link
Contributor

In my opinion, the current approach is more universal, since it allows you to get heif_item without getting the image handle, and in which case weed out images that are not of interest immediately after heif_context_get_list_of_top_level_image_IDs, and then go work with image handles.

@farindk
Copy link
Contributor

farindk commented Jun 19, 2023

but there is no way to get context back from the handler.

I have no problems with adding a function to get the heif_context from an heif_image_handle. Similarly, We can also add some convenience functions to use heif_image_handle in place of heif_context + heif_item.

I'd like to give you some background why heif_item is used in many of the new APIs: there are properties (e.g. udes) that can be attached to images and also non-image items, e.g. rgan items. Thus it feels logical to operate on item item-ID level. There are also some features in the pipeline where it is better to operate on items directly. E.g. accessing image grids or overlay layers separately. For these kind of features, we probably need to have a more lower-level API (with item-IDs) that works close along the standard, and a higher level API (using image handles) for those who just want a quick, easy interface.

Concerning returning the transformations in a 'canonical' order: this may have some nasty consequences. E.g. rotating and cropping an image may require a conversion to a different chroma format while the other order of rotation and cropping may not require this. Now, in HEIFv2, they specified a scaling transform, which makes things even more difficult.

But I like the idea and didn't abandon it. We just have to be very careful that we don't maneuver ourselves into a dead end.

@homm
Copy link
Contributor Author

homm commented Jun 19, 2023

@bigcat88

  1. You can easily get heif_image_handle from heif_context and heif_item_id. Yan can't get heif_context if you already have heif_image_handle. heif_image_handle is more universal, its definition of universality.
  2. You likely already have heif_image_handle since it required literally for anything including getting width and height. Look at heif_info.cc: heif_item_get_transformation_properties followed by heif_image_handle_get_ispe_width(handle).
  3. It's just inconsistent. If heif_context + heif_item_id more convenient, why wouldn't use it in all other APIs?
  4. heif_context_get_list_of_top_level_image_IDs is not the only way how to get image_IDs. There is also heif_image_handle_get_list_of_depth_image_IDs and heif_image_handle_get_list_of_auxiliary_image_IDs which works with handles and heif_image_handle_get_depth_image_handle which return one handle from another handle without context.

@homm
Copy link
Contributor Author

homm commented Jun 19, 2023

Concerning returning the transformations in a 'canonical' order: this may have some nasty consequences.

Just for the record, I don't have any questions to this part. It's up to library: provide "close to the file" or more general format.

@homm
Copy link
Contributor Author

homm commented Jul 8, 2023

There are questions about the new API #919 #920

@farindk
Copy link
Contributor

farindk commented Jul 20, 2023

@homm I've added an API function to get the heif_context from an heif_image_handle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants