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
HEIF plugin #4782
HEIF plugin #4782
Conversation
I think that given the licensing issues, this is the only way forward, and presuming it works, then thanks very much for thinking of this. There is certainly a large number of people requesting this feature over in #2806 @wiredfool, did you have any thoughts on this approach? |
Re: the licensing issues, On another note I should point out that |
Python 3.5 will be EOL and dropped in the next release: #4746. |
A brief search turned up with an ImageMagick libheif repository which claims to be intended for building libheif on Windows, so it should be possible:
If this gets merged, I am willing to attempt to contribute scripts for Windows builds to pyheif (e.g. using GitHub Actions). |
Just a quick comment to the first question you asked - yes it IS wanted! Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry this is a stale review. 2020.
I think this is a reasonable approach from a licensing point of view, and it should be shippable that way. I'm not sure that we'd ever get libheif in the binary builds, but this would be the sort of thing that would work at arms length if someone were to install pyheif separately.
I'm a concerned with the amount of work that's done in open. I think it might be worth seeing if the pyheif project would be amenable to having a basic call into the library for size/mode/etc without the load of a full decode.
|
||
def _open(self): | ||
b = self.fp.read() | ||
if not _accept(b): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general we should not be reading the whole file in _open. It does appear that pyheif does that, which is somewhat disappointing. I'm not sure if it's possible to get around that, except for the initial magic check.
heif_file = pyheif.read(b) | ||
self._size = heif_file.size | ||
self.mode = heif_file.mode | ||
self.fp = io.BytesIO(heif_file.data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The contract on _open is that we don't do expensive operations, so decoding the entire images and storing a copy violates that.
Unfortunately, it doesn't appear that pyheif makes it easy to get the metadata that we require for the _open command without decoding the entire image.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I thought I did have a metadata-only call but maybe I'm not using it. I'll check it out.
Haha yes, I think it will be easier to register like this from I will still try to implement @wiredfool's advice, much appreciated |
To whom it may concern. I've made heif-image-plugin where fixed many issues including described here. For example:
|
Hey, this is a DRAFT version of a HEIF plugin using
pyheif
. Just running it up the flagpole ;)If so I'll proceed and add exif, color profile, doco and tests.
#2806