OpenImageIO
OpenImageIO copied to clipboard
feat(jpeg): Support reading Ultra HDR images
Description
Initial feature request: https://github.com/AcademySoftwareFoundation/OpenImageIO/issues/4424
Add support in the jpeg plugin for reading Ultra HDR images using the reference codec libultrahdr: https://github.com/google/libultrahdr
Tests
Images used for testing during development: https://github.com/MishaalRahmanGH/Ultra_HDR_Samples
Checklist:
- [x] I have read the contribution guidelines.
- [x] I have updated the documentation, if applicable. (Check if there is no need to update the documentation, for example if this is a bug fix that doesn't change the API.)
- [x] I have ensured that the change is tested somewhere in the testsuite (adding new test cases if necessary).
- [x] If I added or modified a C++ API call, I have also amended the corresponding Python bindings (and if altering ImageBufAlgo functions, also exposed the new functionality as oiiotool options).
- [x] My code follows the prevailing code style of this project. If I haven't already run clang-format before submitting, I definitely will look at the CI test that runs clang-format and fix anything that it highlights as being nonconforming.
The committers listed above are authorized under a signed CLA.
- :white_check_mark: login: mugulmd / name: Loïc Vital (b5c78cafccdb62af914cac51604745f6be5019b2, 8e6ed95f4e29a2b8954e17b64fb47c6b225e653c, 16d1d6218d7d513cb961293ec7438a4c4c5c3751, bb9018c2d7bef662be104110d79a6c756079d5a2, 52295c110021c98046f2fab6a94b7295355a1d0d, a792186522fd9c4bca47cf0592be241abc4dbddc, 09962ea060ecb7d8bc985889f54735e7ca55e58e, 14610042e0998fb709c764e6a2d19b5ed2160a04, c9de51080ac6b5f2b50e1bc65ddd8d57923df26e, 39185b49b9066ebc980998859dbbc835337972b4, 56b26472a2f7707ca7d01ac768b41f5ac2b4efc4, 31943e5126f4f485e1e565de5a2b97498e3f69d5, cdf5c3dcacb333d49b15322aa86544f5b464c4d4, 10706fc472eca0788b72bda9392718fd2dcd24c7, 45038c549673d13b771bf2ba3b253288fd1174a3, 3061eeb5f34ffa70f4503c39799a70e7fb4c2441, 81f395652b37bdf004baccb11775788dfad5e399, 3d8bb6a9a1e0dbd11cc4a9a6c8c779f6ce4bf3cb)
@lgritz quick question: I will need to add at least one Ultra HDR image for the test suite. The ones I use for testing locally weigh a bit more than 2M. Should I add them directly to the OpenImageIO repo, or should I create a PR in OpenImageIO-images to add them there ?
Images repo, please
What's happening with icc is that the libuhdr's build is passing parameters to the compiler that it doesn't recognize. Its build system probably assumes that if it's building on Linux, it must be either gcc or clang. But icc doesn't take those particular arguments.
I think the easiest thing is to just set the icc test (in ci.yml) to disable the use of libuhdr by adding to its setenvs: the extra definition DISABLE_libuhdr=1. The icc compiler is basically discontinued, so it's really not important to test that particular case with libuhdr support. It's not worth the trouble to try to fix it for an optional dependency.
Once I get the CLA authorized by my company I'll be able to merge the PR containing the test image and then I'll re-introduce the test case in the test suite :)
I think the libheif failures are unrelated to this PR. Just ignore those for the moment, I'll address them separately.
It seems that the test I added for reading Ultra HDR images is not passing in the CI for some reason. I just checked it's passing on my local setup, so it must be an issue with how we build/link libultrahdr. I'll try to fix that :)
Just out of curiosity, what was the fix that addressed the failed CI tests?
You have just one test (icx) failing. Once that is fixed, I think this is ready to get final review and be merged.
@lgritz it seems libuhdr had trouble locating the jpeg library. I think (but I'm not 100% sure) that it comes from the find_package(JPEG) instruction in libuhdr's CMakeLists.txt, which will redefine the JPEG_INCLUDE_DIRS and JPEG_LIBRARIES variables, and make them point to a different JPEG install (as it seems most of the containers used in the CI already have a JPEG install by default, but we're trying to use the jpeg-turbo that we manually installed).
By looking into CMake's official FindJPEG.cmake, I noticed that the JPEG_INCLUDE_DIR and JPEG_LIBRARY variables are not necessarily redefined, therefore we can use them to point to the jpeg-turbo install, and pass them to libuhdr's build process.
I also noticed that something similar was done for libTIFF, so I figured this might be a good idea, and apparently I got lucky :)
Hey @lgritz I just did a few checks on the CI and it seems the icx test is failing only because libuhdr gets compiled with icx, so it seems to be a compiler issue. I have to admit I don't know much about icx, so can I suggest we override the compiler with g++ for the moment on that specific CI run ? I see we're already doing that for OCIO. (we can also create an issue on the libuhdr repo, it's possible they never tested compiling with icx and there might be some extra work to do on their side to support it properly)
so can I suggest we override the compiler with g++ for the moment on that specific CI run ?
Yes, definitely! The point of the icx test is to make sure OIIO builds and runs correctly with that compiler. It's not our responsibility to make sure the dependencies also do so. If there's a problem with any dependency and a specialty compiler, we should just switch to building that dependency itself with the standard system compiler (gcc in this case) and move on.
Alright @lgritz I think we're good to go. Thanks for the review(s) :) If there's anything more you'd like to add don't hesitate !
There's still quite a few features from libuhdr we could expose in oiio, but yeah that felt like a pretty good starting point !