Enable exiv2 ISOBMFF support for CR3 and HEIF files #8751
Conversation
Well it is a first step, maybe better not even opening the file no? And we the work is done in rawspeed it will open-up fine. So to me it may well be in for 3.6 even if no rawspeed support done yet. Maybe some others will have another point of view.... |
|
This breaks on systems with exiv2 < 0.27.4 And the configuration fails. |
Sould be fixed now, it was a stupid idea to call find_package() multiple times with different versions. |
| check_cxx_symbol_exists(EXV_ENABLE_BMFF "exiv2/exiv2.hpp" HAVE_EXV_ENABLE_BMFF) | ||
| if(HAVE_EXV_ENABLE_BMFF) | ||
| add_definitions(-DHAVE_LIBEXIV2_WITH_ISOBMFF=1) | ||
| set(DT_SUPPORTED_EXTENSIONS ${DT_SUPPORTED_EXTENSIONS} cr3 heif CACHE INTERNAL "") |
There was a problem hiding this comment.
Why not 'heic' as well?
Btw, even though we can read metadata from heif/heic now, we still can't decode the images since dt is not leveraging libheif AFAIK.
There was a problem hiding this comment.
Hm... I'd need to check but ISOBMFF files enabled with introduction of this should be: CR3, HEIF, HEIC & AVIF
//EDIT:
And JPEGXL
There was a problem hiding this comment.
avif is already added to the list of supported extensions if we detect libavif, and we can actually decode those images...
So it's a bit of a hodgepodge now, what does "supported extension" mean? You can decode the image (CR3 is still missing in rawspeed, HEIF/HEIC as mentioned), you can read the metadata, any level of support...?
There was a problem hiding this comment.
IMHO, this line should be removed completely, and supported extensions added in other places once the required decoding libraries are available like it is the case for all other extensions now.
Otherwise, great approach, like it better than directly checking for EXV_ENABLE_BMFF in source code.
There was a problem hiding this comment.
And another thought: maybe it makes more sense to make sure and remove cr3 from the list if BMFF support is not detected instead?
I would still leave heif/heic, avif and jxl extensions up to library availability, as metadata is not so crucial there, they are not raw files...
There was a problem hiding this comment.
I was not aware that HEIF can also have HEIC file extension. I mainly focused for the CR3 thing. After some research, the HEIF files from Canon have the .HIF extension. So we should add all variants (and I expect there are more than this three).
I've not added avif because this is already added when libavif is found.
I would like to keep cr3 there (as ISOBMFF exiv2 support is the only requirement if rawspeed support is done) and strip out the heif/heic/hif part and move to libheif detection.
While scrolling through the code I figured out it would be easy to implement HEIF support via libheif. I'm working on a PR for this.
This PR may supersede #8340.
Exiv2 has introduced support for ISOBMFF files (CR3, HEIF, AVIF) with 0.27.4 (still only a release candidate is available).
This patch checks for 0.27.4 and for EXV_ENABLE_BMFF. As exiv2 has decided to make this feature optional, it must be
explicitly enabled during compile time.
I explicitly don't depend on EXV_ENABLE_BMFF in exif.cc. Instead I introduce a new define HAVE_LIBEXIV2_WITH_ISOBMFF
and extend DT_SUPPORTED_EXTENSIONS with CR3 and HEIF if all requirements meet. This should prevent people from
importing CR3 files if their distro has not shipped exiv2 with ISOBMFF support yet.
As this only enables the EXIF metadata support for CR3 but not for the raw pixel format (this must be done in rawspeed),
I recommend to revert this patch if 3.6 will be released without CR3 rawspeed support, as reading metadata
without access to the pixel data makes no sense :-)
I've verified exif metadata import on Debian Bullseye with exiv2 0.27.4-rc2.
