OpenImageIO Initial JPEG XL support for image input/output

Description

This draft PR is intended for adding JPEG XL support.

Tests

Checklist:

[x] I have read the contribution guidelines.
[ ] I have updated the documentation, if applicable.
[ ] I have ensured that the change is tested somewhere in the testsuite (adding new test cases if necessary).
[ ] 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.

Nov 18 '23 16:11 1div0

The committers listed above are authorized under a signed CLA.

:white_check_mark: login: lgritz / name: Larry Gritz (a2d3aa59675a2ebcff0ac98b8d57138a77dadaed, b24a108d6a26710e58d87e353b9ab529d0b4b62b, c06f5735fcbdbda488bb467af5f7774ecd8a1176, 90750f6193ff2f55b1871200635469f94c41f738, 5aceb5c5edd20b8853dbbc297b0536446021ee1b, 8f2299cfd6a753c95b88a446ee905061b1f15896, 847f0a025bdb38bcdea4fed29663ee089e62ab05, a09937ece5d7a1328d8ab66ff9f6e668a488ff12, dd852558c1bbb56341c5cfa80be4142bd42c74eb, be72bbc42dcad8fa1fdc6c90e40b39f1602b8595, 27e898ec2bf1604ade1bc5a80bfe1eab8eb0d4eb, 28ddc59ecad6b1436215b697564a72dd435fbf01, 6b99ff49bacdd0445c941e6d6a4a3730aba735b0, ffadbdc017c85097f8f0b1932ca197d85b07abda, 35ee9f1eceff8d082c5edf7cd1039b5e5df12968, b5ab45dab95795a1695ae146943200ac040e951b, cb5ddc23644bbb5bd1337cd16fddd6eb5aac1642, 5720ab12df26d7679cd1a4742ca246d54f7d5c31, 39a7a55385aeb597f95f6473c6c71f14578e70c7, 22262018395d2e53ca8c3c91046d86980fa23d2f, f1079a38cedfa0ca4315ec42cbb80bf722916dda, 82eccfe323fa459b6578d0540dc93cbf1c92fdf5, 01c1e88c8a3f62d9c4cc75422fdcafce954b291b, 5a71ce3b9a86721028da0d3a8962dd3fa54b738c, 9022ffcfd21ed6c9430ae1e867ffa74000a45fac, a51518a9451d626bc4d203e5ad8b9f9c9cd2ce1a, ef06678d89c4ebcc76c758b327c677017486c3d0, ff4124c943d2528c36bb46372883a8f3f74c801b, e7387ab55f1034f84ce35d3b38c1d44a98d360b5, 3444cf889a1dc37cb3d330b98f6747326d77367b, ffc13450574e8c3316936d18f9c9d80d3d3bf4bd, 439ec0dff0f2d3aa438d1f6317781350ba0177c5, 536c6466e9b1cf8603163b64891c4066c9de6a70, 3037b7111cad7ae5089432a5f759ea4f4edaa6db, a342d74618c12e48e09a851b415ec1d4d567bc3b, 86d40a20553b25bc89bb68704a3ebf7640ff0024, 9091e93a24656243064a5cc0ae02f197d38d5ac2, 779b744749f1e14edb56bcc39b145cd0ec861b24, a762525f46e234c714b216ae69832b919370db4d, 7be367e58b5c574391b3180ea4c91d1de4e22c7d, 07c17b55cf0dca0ff4c58feddd43b85860f7552a, 7a73e5f1f0b9d5927cb95c62e7d57260a0149e7f, a6585e54bf4030c140f1af41de5b2946cddde4f3, f6f9e26289c6eda0c0a40e455f511af736435e6b, d30e9fce2d0c7774becf3a7205dadf7020ca875c, a4d484f4f914db158f7c669a93d089db89248518, a0a74a3aa007f29363ed224341bd34f07eb318de, 1488f4589f589cceee95ee28e3d1289253603c15, e92123d51f16117c531523a7bf09e7865460f910, f61e28ec1feed21e56e9bbe519b95bb356974e1d, 74705a185e2045b2f51d45c373bfd09556110a54, ac5dca69c7276381e3fb163f8fb0eef35d928f70, 74c5f0b6e7a4b137c626375c000da62d5e005db5, 802ad7d9ead6649039a298d2eb63fa96662180a2, 58516aaf652f727d2b9f4c04ab539d3d67925924, 35b2f1cc3b0b6eff1ef498added4c98644d1dc25, 88dd2f41ea499eee5339dddea45a612fda5d7e72, 70a2bd440f59dfcfee05cb256e77b792e25a3f45, 3d5df4dc299cb44776db173a335e8b1646cd37f2, 377f467ee71a298919e7a7b1d0099231f682c5b7, b01b089d4482735498dfaee383bcf0e176521836, cce46a58b15dca36a6f63bc766f3d67f6ad92d6e, 1b5c20cf875b6898e226bd83133cf8eca73d8c89, ca957358354b1bdef95c58ad9a149a400ea66aed, 3635d53c8386f275340db804915fcc357f36ae0c, e169c3e878c01caa7c3b726b7b10506872edfdd1, 40a03e0c73313c05102463a8f0a715dbf92910ea)
:white_check_mark: login: 1div0 / name: Peter Kovář (989668977d134dd395aa5d52fe90a7fb1ed5bdbf, dcc6dbd26b79f6c75f14773a1fb87a6ac51533f6, a80702e10ae1b3f65bedc01efa165fe2b86dd03e, dc6909907dfee9a7defc91e0652d3b933b37a81b, 9808a9e4aa6d53e3e15b1316140b2b630d76541b, 5d593ffcb5e0a31499bdd13562bb3dd5838c7994, 19848e04e410545a67a30bb7a52037187ebd41d2, a11f045a547c4203af7e78355ee6d7420597da84, 2bfc294005c4bbc5d8ec33f015a4eebbd23a43cd, 65f9e14f2d2a425d0acf720fa149dd5c3cd6db05, 00ad4dcc28b89c9f268422e303e1cd4b8e09b2c1, f16981cc49a6f9f4e2362b3984a4705f5e734d92, 5aaab063055cef7b46ee84f9d6a890d190ec05dd, a45a5028c246a8182f930c7d41bfab3d734ea13f, 641cfc8ba2f7b8695ae8c1c582aa6570c466c8dc, c58c8ead0e791f32d34371a53565ffb43318db65, 69c4392d0c8e6dec37b0c1fbff3551ccf5648825, 64b8dd69e60c9443678d72e0085028d1ffeb0a3d, 5366465902580666a876060b1c433039058fd6f0, 1dee843707ddb516f68954d113cd1b0d125f1e94, 1438af49a75d629af842aed51b276acb0a2c32a5, 6f1553383eef58f1a0d408b04e80ff937730b495, 9d391d89b80a501c4eb99aa3f1cafaeb4a268f5c, c086df794a94d69db19d800fc200bcab24ab6c5c, 835a42a60a0107ee6c037229e6da81ba6d74fede, cef3e00b957cd9933ddc4591e7d971205d92f6ca, 0a3e78050a791785ba4bbc306837e5203a6321cf, 2e4a0feee91feda0498cbebf69e0b3f3c72b5f62, 2ae90e332e938af490b4fff08f64bb80e8bea47e, 2776531c2b36fa7f4f8db96fc6e99494866a09cd, 981010e1e14320046d0b99ff9eb601b78b4c686e, b9a51b2902879f730bfd284cec226520e4b56da5)
:white_check_mark: login: jessey-git / name: Jesse Yurkovich (97e1d03fe02c999c18e132b2a613d19772075d0a)
:white_check_mark: login: AngryLoki (e5b819f7827e5497faebfef022b64e2c23ab80a1)
:white_check_mark: login: domin144 / name: Dominik Wójt (942ad021299991dfb6c5b8f3e5f4b434d8666d1f, a8c385a5a0d1e50a74f6088426d0a1be7afe6238, 14c47fafb9ce1191921c4848e71145420fd1ecfd)
:white_check_mark: login: AdamMainsTL (65a0888a076c12cf2f9ab4b7b939ea90fd26e970)
:white_check_mark: login: aras-p / name: Aras Pranckevičius (be4b97c6125431f7fd4ed38ee14066ad05376acc)
:white_check_mark: login: kohakukun / name: Aura Munoz (b8edfa026fcfca24a8c0d9427d47b5dade433840)
:white_check_mark: login: antond-weta / name: Anton Dukhovnikov (e7ee3efbb2e8896b0a49e776937f25aadc2a9b50)
:white_check_mark: login: jreichel-nvidia (793b3ebdcb0f8fd142190e002ee1179073b29d99)

Nov 18 '23 16:11 linux-foundation-easycla[bot]

The CI is failing on Mac. I bet that this is because the Mac CI tests are the only ones with libjxl installed at all, so they are the only ones building this code. That's probably something else to address, would be nice for at least one Linux build to try jxl. But maybe the reason it's failing on Mac is that it's using a different version of libjxl than you are, and something in their API changed?

Mar 01 '24 05:03 lgritz

The CI is failing on Mac. I bet that this is because the Mac CI tests are the only ones with libjxl installed at all, so they are the only ones building this code. That's probably something else to address, would be nice for at least one Linux build to try jxl. But maybe the reason it's failing on Mac is that it's using a different version of libjxl than you are, and something in their API changed?

Found JXL 0.9.1, while the latest is 0.10.1, which I am using now. I may add conditional compilation in order to have some backward compatibilty. Or wait until the newest release will be available everywhere? That takes ages.

Mar 01 '24 09:03 1div0

Your call on how far back you want to support libjxl. You could set the MIN_VERSION in the checked_find_package call to reject a version that's too old.

If you want 0.10 to be the minimum version, that is less work now in the sense that you don't need any #if now, though you may later when 0.11 or whatever comes out. We'll probably never again be in a position where we can dictate that the only version we support is the very latest.

On the minus side, 0.10 minimum would mean our CI wouldn't currently build and test JPEGXL at all. In that case, I think we would need a build_jxl.bash like the other files in src/build-scripts, which we can run during the CI setup to ensure it's available in the versions you want to test, and that also makes it handy for end users who want a simple way to install it so that OIIO can find it.

Mar 01 '24 17:03 lgritz

Tried that already.

https://ffmpeg.org/pipermail/ffmpeg-devel/2023-July/311369.html

Mar 01 '24 18:03 1div0

Your call on how far back you want to support libjxl. You could set the MIN_VERSION in the checked_find_package call to reject a version that's too old.

If you want 0.10 to be the minimum version, that is less work now in the sense that you don't need any #if now, though you may later when 0.11 or whatever comes out. We'll probably never again be in a position where we can dictate that the only version we support is the very latest.

On the minus side, 0.10 minimum would mean our CI wouldn't currently build and test JPEGXL at all. In that case, I think we would need a build_jxl.bash like the other files in src/build-scripts, which we can run during the CI setup to ensure it's available in the versions you want to test, and that also makes it handy for end users who want a simple way to install it so that OIIO can find it.

0.10 as the minimum required version seems to be the good choice, especially with the latest improvements: https://cloudinary.com/blog/jpeg-xl-and-the-pareto-front

Mar 03 '24 13:03 1div0

OK, I'm with with making jxl have a supported minimum of 0.10.

But that means that we should write a build_libjxl.bash script, and use it for at least one of our CI tests so that we are for sure building and testing this code during CI.

Mar 03 '24 16:03 lgritz

Guess what? Homebrew JUST upgraded their jpeg-xl package to 0.10.1. Let me force these CI tests to run again, it may run on the right version on the Mac CI, which would let us directly test you new coe.

Mar 03 '24 18:03 lgritz

Weird. I tried myself, and even with libjxl 0.10.1, I'm seeing errors related to JxlDecoderGetICCProfileSize and JxlDecoderGetColorAsICCProfile being passed wrong arguments -- in jxl/decoder.h, neither of those functions seem to take the &format argument you're passing. How are you getting it to work on your end?

Mar 03 '24 18:03 lgritz

I have a few general questions about the plans for this area.

For output:

It looks like this will always write out using a 32bit float format. Are there plans to allow the caller to decide what bit-depth and format to write with?
There's no control over compression settings of any kind. That seems less than ideal.

For input:

Apparently JXL allows both associated and unassociated alpha. How does this work in practice and does this mean it's even more important to at least respect the oiio:UnassociatedAlpha attribute?

I'll review the actual code a bit tomorrow.

Mar 04 '24 09:03 jessey-git

I have a few general questions about the plans for this area.

For output:

* It looks like this will always write out using a 32bit float format. Are there plans to allow the caller to decide what bit-depth and format to write with?

* There's no control over compression settings of any kind. That seems less than ideal.

For input:

* Apparently JXL allows both associated and unassociated alpha. How does this work in practice and does this mean it's even more important to at least respect the `oiio:UnassociatedAlpha` attribute?

I'll review the actual code a bit tomorrow.

This code is still best described as "Initial JPEG XL support". There are so many options and so little time_t.

Mar 04 '24 09:03 1div0

Guess what? Homebrew JUST upgraded their jpeg-xl package to 0.10.1. Let me force these CI tests to run again, it may run on the right version on the Mac CI, which would let us directly test you new coe.

Weird. I tried myself, and even with libjxl 0.10.1, I'm seeing errors related to JxlDecoderGetICCProfileSize and JxlDecoderGetColorAsICCProfile being passed wrong arguments -- in jxl/decoder.h, neither of those functions seem to take the &format argument you're passing. How are you getting it to work on your end?

It was unfortunate impedance mismatch between three versions of the libjxl I had installed. So cleaned up and checked against the latest and greatest official release.

Mar 04 '24 16:03 1div0

Guess what? Homebrew JUST upgraded their jpeg-xl package to 0.10.1. Let me force these CI tests to run again, it may run on the right version on the Mac CI, which would let us directly test you new coe.

Just poured jpeg-xl--0.10.1.x86_64_linux.bottle.tar.gz and going to test against this version as well.

Silly idea: what is preventing us to use Homebrew installation on the Linux CI runs similarly to macOS?

Mar 04 '24 17:03 1div0

This looks like it's getting very close. Only minor comments about the code itself, as you can see.

Big missing items:
* Documentation -- need a section in builtplugins.rst

* Tests -- the testsuite needs to exercise this in some way, like we do for the other formats.

Color management is unfinished. Nor proper metadata handling. Still so much to do and so little free(time_t).

Mar 04 '24 17:03 1div0

Silly idea: what is preventing us to use Homebrew installation on the Linux CI runs similarly to macOS?

No particular reason, it just never came up. For CI, at least, we tend to try to use the "native" packager. So adding rpm/yum packages in gh-installdeps.bash is what we usually do for Linux. I'd be slightly afraid that a dependency from homebrew might not be representative of what they'd get installing packages the way most Linux users do -- different version, installed in a different place, who knows.

Mar 04 '24 18:03 lgritz

Looks like we build successfully on Mac now with jpeg-xl installed via Homebrew!

But we are failing the testsuite, specifically the imageinout_test. That may be tricky, and I'm ok with your modifying imageinout_test.cpp, circa line 324, to simply skip the jxl format for now. We can restore it in subsequent iterations over this code and addition of the tests.

Mar 04 '24 18:03 lgritz

Looks like we're building ok on Mac with jpeg-xl installed. So I think the minimum requirement for getting this initial PR merged into master are:

Enforce the version limit in externalpackages.cmake.
Fix the clang-format CI failure.
Disable the format (temporarily) in imageinout_test -- we'll come back to this
Resolve what you want to use for the format_name() method.

Then with this as a baseline, the next set of necessary improvements are (not necessarily in this order):

Documentation
Handling alpha correctly (associated vs unassociated)
Track down what's going wrong with imageinout_test
Adding a testsuite/jpegxl that tests basic read and write of jxl files, including a few combinations of supported features.
Full support of all the pixel data types the jxl supports.
Compression options.
Right now, I think you are reading in the entire image at once into a (large) buffer and doling out pieces via memcpy as requested from the caller. I haven't read the libjxl docs -- does it support asking or passing ranges of scanlines or tiles? I don't know if the jxl format allows such things easily.
Getting the supports() method reporting the right set of things.

I'm sure we'll think of other things, but it's ok to get the big hunk of initial experimental support in now (as long as it builds for everyone correctly and passes the tests we already have), and then can have a series of smaller PRs to complete support and bring it all up to parity with the other formats.

Mar 04 '24 19:03 lgritz

Looks like we build successfully on Mac now with jpeg-xl installed via Homebrew!

But we are failing the testsuite, specifically the imageinout_test. That may be tricky, and I'm ok with your modifying imageinout_test.cpp, circa line 324, to simply skip the jxl format for now. We can restore it in subsequent iterations over this code and addition of the tests.

I will try now the macOS Sonoma build in the QEMU KVM based virtual machine.

Mar 04 '24 19:03 1div0

Adding support for streaming mode comes to top of my wish list. Also, it would be cool to handle stereo and multiview images in this age.

Mar 04 '24 19:03 1div0

Stereo and multiview should probably look like "subimages" in OIIO speak. Like how OpenEXR handles "parts" which are also used for stereo.

Mar 04 '24 19:03 lgritz

It took bit longer to set up all dependencies in the virtual machine. However, I encountered this:

[ 17%] Built target ustring_test [ 17%] Building CXX object src/libOpenImageIO/CMakeFiles/OpenImageIO.dir/imagebufalgo.cpp.o [ 18%] Building CXX object src/libOpenImageIO/CMakeFiles/OpenImageIO.dir/imagebufalgo_pixelmath.cpp.o [ 18%] Building CXX object src/libOpenImageIO/CMakeFiles/OpenImageIO.dir/imagebufalgo_channels.cpp.o [ 19%] Building CXX object src/libOpenImageIO/CMakeFiles/OpenImageIO.dir/imagebufalgo_compare.cpp.o [ 19%] Building CXX object src/libOpenImageIO/CMakeFiles/OpenImageIO.dir/imagebufalgo_copy.cpp.o [ 20%] Building CXX object src/libOpenImageIO/CMakeFiles/OpenImageIO.dir/imagebufalgo_deep.cpp.o [ 20%] Building CXX object src/libOpenImageIO/CMakeFiles/OpenImageIO.dir/imagebufalgo_draw.cpp.o /Users/peter.kovar/1div0/OpenImageIO/src/libOpenImageIO/imagebufalgo_draw.cpp:743:20: error: unused variable 'default_font_name' [-Werror,-Wunused-variable] static const char* default_font_name[] = { "DroidSans", "cour", "Courier New", ^ 1 error generated. make[2]: *** [src/libOpenImageIO/CMakeFiles/OpenImageIO.dir/imagebufalgo_draw.cpp.o] Error 1 make[1]: *** [src/libOpenImageIO/CMakeFiles/OpenImageIO.dir/all] Error 2 make: *** [all] Error 2 nice make 593,42s user 102,51s system 99% cpu 11:38,20 total peter.kovar@Peters-iMac-Pro x86-64 % uname -a Darwin Peters-iMac-Pro.local 23.2.0 Darwin Kernel Version 23.2.0: Wed Nov 15 21:54:10 PST 2023; root:xnu-10002.61.3~2/RELEASE_X86_64 x86_64

Mar 05 '24 19:03 1div0

I see. I think this is a problem where the static variable is declared unconditionally, but it's only used if Freetype dependency is found and enabled. It's unrelated to your code.

I will submit a separate PR that fixes this case. But in the mean time, you can either install freetype or disable that warning.

Mar 05 '24 20:03 lgritz

In the meantime I am analyzing https://github.com/libjxl/libjxl/issues/3326 in order to implement streaming encoding and decoding in the OIIO.

Mar 06 '24 10:03 1div0

Can you clarify what "streaming" means in this context?

Mar 06 '24 17:03 lgritz

I strongly recommend

git mv src/jxl.imageio src/jpegxl.imageio

and doing other fixups necessary (CMakeLists.txt, the stuff in imgeioplugins.cpp, etc.). We want

The plugin-in name (which is taken from the directory name)
The canonical name returned by the format_name() functions

to match exactly. The reason I want you to do this in time for the initial merge is that if we do the "git mv" later, I think it won't transfer the history and attribution properly, it will look like a new file is created and a git log or git blame won't necessarily show your contributions properly (I think).

You also need to fix imageinout_test to exclude this format from the test, for now.

Mar 06 '24 19:03 lgritz

We're very close! Let's fix the directory name to match the format_name, and then I think we have everything we need for an initial merge (with lots of improvements to make later, a piece at a time is fine).

Mar 06 '24 19:03 lgritz

Renamed. L👀king at the test suite now.

Mar 06 '24 19:03 1div0

I am getting this weird error in the test.

jpegxl (jxl): Writing imageinout_test-jpegxl.jxl ... OK PtexReader error: read failed (EOF) PtexFile: imageinout_test-jpegxl.jxl /2TB/usr/src/github.com/1div0/OpenImageIO/src/libOpenImageIO/imageinout_test.cpp:360: FAILED: in && "Could not create reader" Writing Proxy jpegxl ... OK Writing bad to bad/bad.jxl ... OK (Could not open file "bad/bad.jxl")

Mar 07 '24 08:03 1div0

Yeah, that's the failure we've been seeing in CI with the imageinout_test. Like I said, I think it's acceptable for right now to disable this new format from that test, so we can get an initial merge with everything you've done so far, and then come back with all the little fixes including whatever is necessary to restore this test.

In effect, I want to checkpoint what you've got so far, so that every additional change can be viewed individually as a small patch to consider, rather than continuing to pile things onto this one, which means we need to look at your whole patch again every time there's a modification. Let's accept all the parts that seem good so far and iterate.

Mar 07 '24 17:03 lgritz

With the debug build of JPEG XL library latest modification revealed error in the writer.

m_filename = imageinout_test-jpegxl.jxl m_basic_info 64×64×4 lib/jxl/encode.cc:1264: Invalid number of color channels JxlOutput::write_scanlines(ybegin = 0, yend = 64, ...) data = 0x3605ac0 nvals = 16384 JxlOutput::close() JxlOutput::save_image() data = 0x364d920 size = 65536 lib/jxl/encode.cc:2218: Basic info or color encoding not set yet status = 1 JxlEncoderAddImageFrame failed with error 129 JxlOutput::close()

Mar 08 '24 08:03 1div0