Initial JPEG XL support for image input/output #4055

1div0 · 2023-11-18T16:38:42Z

Description

This draft PR is intended for adding JPEG XL support.

Tests

Checklist:

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).
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.

linux-foundation-easycla · 2023-11-18T16:38:46Z

The committers listed above are authorized under a signed CLA.

✅ login: lgritz / name: Larry Gritz (a2d3aa5, b24a108, c06f573, 90750f6, 5aceb5c, 8f2299c, 847f0a0, a09937e, dd85255, be72bbc, 27e898e, 28ddc59, 6b99ff4, ffadbdc, 35ee9f1, b5ab45d, cb5ddc2, 5720ab1, 39a7a55, 2226201, f1079a3, 82eccfe, 01c1e88, 5a71ce3, 9022ffc, a51518a, ef06678, ff4124c, e7387ab, 3444cf8, ffc1345, 439ec0d, 536c646, 3037b71, a342d74, 86d40a2, 9091e93, 779b744, a762525, 7be367e, 07c17b5, 7a73e5f, a6585e5, f6f9e26, d30e9fc, a4d484f, a0a74a3, 1488f45, e92123d, f61e28e, 74705a1, ac5dca6, 74c5f0b, 802ad7d, 58516aa, 35b2f1c, 88dd2f4, 70a2bd4, 3d5df4d, 377f467, b01b089, cce46a5, 1b5c20c, ca95735, 3635d53, e169c3e, 40a03e0)
✅ login: 1div0 / name: Peter Kovář (9896689, dcc6dbd, a80702e, dc69099, 9808a9e, 5d593ff, 19848e0, a11f045, 2bfc294, 65f9e14, 00ad4dc, f16981c, 5aaab06, a45a502, 641cfc8, c58c8ea, 69c4392, 64b8dd6, 5366465, 1dee843, 1438af4, 6f15533, 9d391d8, c086df7, 835a42a, cef3e00, 0a3e780, 2e4a0fe, 2ae90e3, 2776531, 981010e, b9a51b2)
✅ login: jessey-git / name: Jesse Yurkovich (97e1d03)
✅ login: AngryLoki (e5b819f)
✅ login: domin144 / name: Dominik Wójt (942ad02, a8c385a, 14c47fa)
✅ login: AdamMainsTL (65a0888)
✅ login: aras-p / name: Aras Pranckevičius (be4b97c)
✅ login: kohakukun / name: Aura Munoz (b8edfa0)
✅ login: antond-weta / name: Anton Dukhovnikov (e7ee3ef)
✅ login: jreichel-nvidia (793b3eb)

lgritz

You have this marked as "draft", so I know you aren't seeking a final review, but I thought I'd add some comments at this stage. Mostly just some organizational tips.

I'm not commenting yet on the code in jxlinput.cpp, because I figure it's all still in-progress. I also assume that at some point, you will add something to the testsuite that exercises this code so we have confirmation that it correctly reads jxl files.

lgritz · 2024-02-19T16:36:38Z

src/cmake/modules/FindJXL.cmake

@@ -0,0 +1,27 @@
+# Find libjxl


Please put our usual 3-line copyright boilerplate here

lgritz · 2024-02-19T16:37:48Z

src/cmake/modules/FindJXL.cmake

+# Find libjxl
+# Will define:
+# - JXL_FOUND
+# - JXL_INCLUDE_DIRS directory to include for libjxl headers


I believe the more standard name for this is JXL_INCLUDES

This comment still applies

lgritz · 2024-02-19T16:47:02Z

src/cmake/modules/FindJXL.cmake

+if(JXL_FOUND)
+  set(JXL_LIBRARIES ${JXL_LIBRARY} ${JXL_THREADS_LIBRARY})
+  set(JXL_INCLUDE_DIRS ${JXL_INCLUDE_DIR})
+endif(JXL_FOUND)


If this is all found, in addition to those variables, please also set up targets in the modern CMake style. You can find an example of this in our FindLibheif.cmake module.

Also, it is important for this module to set a JXL_VERSION so we can check version range. A good example of how to extract this from header files can be found in our FindOpenVDB.cmake

This comment still applies

src/jxl.imageio/jxloutput.cpp

lgritz · 2024-02-19T16:54:11Z

src/jxl.imageio/jxl_pvt.h

+class JxlInput final : public ImageInput {
+public:
+    JxlInput() { init(); }
+    ~JxlInput() override { close(); }
+    const char* format_name(void) const override { return "jxl"; }
+    int supports(string_view feature) const override
+    {
+        return (feature == "exif" || feature == "ioproxy");
+    }


I would say that by default, the definition for JxlInput class should be in jxlinput.cpp if that's the only place that reads it.

Some of our format plugins have a format_pvt.h that contains definitions that will be needed by both the input and output classes. You may want to wait until there is an implemented output (when you will know what needs to be shared) before making an extra header for the shared definitions.

Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

It's started failing on GHA and is old enough that it's not worth trying to track down what's broken. We still have MacOS 12 and 13 tests in the matrix. Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

…Foundation#4056) Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

Switch "bleeding edge" test to gcc13, "latest releases" test to gcc12 Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

…ainst (AcademySoftwareFoundation#4058) Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

…n#4057) Used to be no problem, but after a recent VSCode update, this seems to have been causing errors when asking VSCode to format. No idea why this recently became problematic. Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

…ademySoftwareFoundation#4060) The important bug fix is in oiiotool image input when autocc is used, where we reversed the sense of a test and did the autocc-upon-input if the file's color space was equivalent to the scene_linear space (pointlessly), and skipped the conversion if they were different (oh no, big bug!). Along the way: * Add missing try/catch around OCIO call that should have had it. * Some very ugly SPI-specific recognize-by-name color space heuristics. I hate this, sorry, but it solves a really important problem for me. * A bunch of additional debugging messages during color space inventory, enabled only when debugging, so nobody should see these but me. * A couple places where we canonicalize the spelling of "oiio:ColorSpace". Note that there is still an issue with the color space classification using OCIO 2.3+'s identification of built-in equivalents by name. These are shockingly expensive. I will return to this later. Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

…areFoundation#4062) A recent change (PR 4026) tried to include Imath.h in cases where there was no SSE support (to provide a fallback implementation of matrix invert, because simd.h's native one requires SSE), but it had a mistake -- it used `#if !OIIO_SIMD_SSE` prior to that symbol being defined! This caused the header to always be included, which is not what we wanted, as we desired for that header to not be exposed in the public headers. This patch offers a temporary/partial fix by simply moving that guarded include later in the file to after OIIO_SIMD_SSE is defined. For architectures that support SSE, at least, this achieves our goal of not exposing Imath.h in public headers. I need this patch right away to solve a particular problem I'm running into. Longer term, we note that this isn't a full solution -- on non-SSE architectures (like ARM), it still ends up exposing Imath.h against our desire. For today, so be it. A better fix would be to rewrite our current SSE-requiring invert to change the SSE intrinsics therein to the wrapped kind we use with our simd classes, which will let it work fine with ARM. But I feel like that needs to be done carefully and with more testing than I can afford at this minute when I need this fix. I will return to it shortly, but need this patch now. Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

…oundation#4061) - Fix perceptual test false positive by removing the initialization of `cr` to avoid losing the information when leaving block scope Included additional test with parameter `-fail 1`, which was giving a false positive pass for a perceptual difference. This was due to the fact that the result variable `cr` was getting out of scope for the perceptual calculation and the first `if (cr.nfail <= imagesize_t(allowfailures)) {` was testing for the result of the absolute difference instead of the perceptual difference. Before: ``` Comparing "img1.exr" and "img2.exr" PASS ``` After: ``` Comparing "img1.exr" and "img2.exr" 64 x 64, 3 channels Mean error = 0 RMS error = 0 Peak SNR = 0 Max error = 1 @ (5, 17, R) values are 0.1, 0.1, 0.1 vs 0.1, 0.6, 0.1 0 pixels (0%) over 0.008 121 pixels (2.95%) over 1 121 pixels (2.9541%) failed the perceptual test FAILURE ``` Signed-off-by: Aura Munoz <aura.munoz@autodesk.com> Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

…wareFoundation#4063) Add a few more assertion checks and error messages in imagebufalgo_test. I observe that if somebody runs the testsuite with `$OCIO` set to a config that doesn't contain the required color space names for this test, it will fail. As a backstop, use the default built-in config if the initial request fails. That will always give a working processor between these spaces if OCIO >= 2.2 is being used. Also one more tiny SPI-specific enhancement to hacky identification of nonconforming color space names in our old configs. Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

…twareFoundation#4065) Slightly different output to match for the combo of OCIO >= 2.2, but simultaneously Python 2.7 -- which was not in our CI test matrix, so I missed it, but ran into it at work. Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

Don't want that included for Cuda compilation, either. Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

…ademySoftwareFoundation#4071) ## simd.h improvements: vint4::load from unsigned char pointer got pre-SSE4 code path. Testing on Ryzen 5950X / VS2022 (with only SSE2 enabled in the build): - vint4 load from unsigned char[]: 946.1 -> 4232.8 Mvals/sec vint4::store to unsigned char pointer got simpler/faster SSE code path, and a NEON code path. Additionally, it got test correctness coverage, including what happens to values outside of unsigned char range (current behavior just masks lowest byte, i.e. does not clamp the integer lanes). - vint4 store to unsigned char[]: 3489.8 -> 3979.3 Mvals/sec - vint8 store to unsigned char[]: 5516.9 -> 7325.3 Mvals/sec NEON code path as tested on Mac M1 Max (clang 15): - vint4 store to unsigned char[]: 4137.2 -> 6074.8 Mvals/sec ## Tests vint4 store to unsigned char pointer got actual correctness checking test, which seemingly was lacking before (only had a benchmark test). Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

In some environments, only one kind of the library could be available. Choose proper fallback in such cases. Prefer static library, if LINKSTATIC is ON and dynamic otherwise. Signed-off-by: Dominik Wójt <domin144@o2.pl> Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

…ySoftwareFoundation#4073) Without getting the version properly, we weren't disabling use of a deprecated field for OpenJpeg >= 2.5, as we thought we were. This fixes recently broken CI when building against the current openjpeg master. Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

…eFoundation#4074) There was a flaw in PNGOutput::write_scanlines when the y range of scanlines to write consisted of less than the whole image, and alpha de-association was needed -- we used the whole-image size instead of the y range of scanlines size. Fixes AcademySoftwareFoundation#4044 Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

I didn't have the condition right -- the `if` isn't respected in the `pull_request` section, so this job was running on every PR instead of just the ones with "macarm" in the branch name. Refactor to get the conditional on the job instead. Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

This change adds OCIO functionality to iv. To enable OCIO, first set the $OCIO environment variable to point with a path to the OCIO config. "ocio://default" should work as well. If a valid config provided, a new menu gets added under "View/OCIO" with an on/off toggle and submenus to select the image color space and display/view and optional look. If an image color space, display and view are provided as command line parameters ("image-color-space", "display", "view"), then the OCIO mode is switched on automatically. If no image color space is provided as command line parameters, the top most color space of the config get pre-selected in the menus. If no display or view is provided as command line parameters, the default values get pre-selected in the menus. In OCIO mode the exposure and gamma adjustments are done by OCIO. The exposure is done in scene linear, gamma is still in display space. The exposure adjustments don't match between the OCIO and non-OCIO modes. When OCIO fails to initialise the pipeline for any reason, an error message is shown in the status bar, and the non-OCIO path gets used. TODO: - All channel shuffling is done by the existing shader and happens before the OCIO step. We may want to discuss/change that? - The initial image color space gets selected from the command line parameter, or the first color space in the config. There is no attempt being made to use the image metadata. - The exposure/gamma values get baked into the shader, which gets recompiled on every change. We may want to use uniforms instead. - DONE --------- Signed-off-by: Anton Dukhovnikov <antond@wetafx.co.nz> Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

…le (AcademySoftwareFoundation#4033) Historically, ImageBuf has provided Iterator for writing to mutable IB's and ConstIterator for reading into IBs. If you initialized an Iterator to an IB that was ImageCache-backed (and therefore by definition not mutable), it would convert the IB to the mutable kind (basically by allocating the memory and reading the file through the IC, thus freeing the IB of its dependence on the cache. An important bug was fixed by PR 3997, which kinda proved that people were not depending on this behavior, since it would tend to fail. But it also got me thinking about to what degree the whole Iterator vs ConstIterator was really necessary, or did it just make things harder for users. In this proposed patch, we change the Iterator behavior so that instead of immediately making the IB writable as soon as the Iterator was instantiated and associated with the IB, we instead treat it as a ConstIterator UNTIL it actually tries to write a value, at which point we ensure that the IB is writable. The bottom line is that this seems to work. If you don't want to have to think about Iterator vs ConstIterator, you don't have to. Iterator is fine. If you don't actually write into any of the pixels, it behaves just like a ConstIterator, including being perfectly happy with an IC-backed IB. Performance metrics indicate that there is NO penalty for doing so -- if you are only reading pixel values, traversing an IB with Iterator is the same speed as doing it with ConstIterator. I had one sleepless night after implementing this when suddenly I realized that this was totally not thread-safe, that somehow multiple iterators traversing the same IB would screw each other up if one of them wrote to a pixel and converted the image. So I put in a test to have many threads concurrently traversing the same IB, some with ConstIterator, some with Iterator but only reading, and some with Iterator and writing. I have not been able to trigger any failures or crashes. It seems fine. (I did use the mutex a little more aggressively than before.) Why is this not as dangerous as I thought? Mainly because once an iterator (of either variety) thinks it's dealing with an IC-backed ImageBuf, basically, it will just happily keep using the IC all on its own, even if the IB that it's supposedly traversing has, because another thread's iterator has written to a pixel, "localized" the image and severed the IC dependence. Then if it also needs to write, it will (safely, because of a mutex) see that the image is already localized/writable, and start using that representation. So herein lies a very important caveat about using IB iterators: If you have multiple iterators traversing the same IB and any of them are *writing* to the image, the iterators may not see a globally temporally consistent version of the image. Put in plain English: iterators that are strictly reading may see an IC-backed disk image as it was on disk, and not as it has since been modified by a different active iterator. So if you are modifying an image in place, beware of using multiple iterators, because a reading iterator may or may not see the changes that a writing iterator made to a pixel value. Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

…oundation#4075) Please look this over. If you are one of the duplicated names and I haven't chosen your preferred canonical email (I made my best guess), please comment here with corrections. Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

…twareFoundation#4085) Fix for AcademySoftwareFoundation#4084. Will now check if the given filename exists before checking for REST parameters. Before: ``` bin % ./oiiotool \?.png -o \?-output.png oiiotool ERROR: read : "?.png": ImageInput::create() called with malformed filename Full command line was: > oiiotool ?.png -o ?-output.png ``` After: ``` bin % ./oiiotool \?.png -o \?-output.png ``` No output, `?-output.png` created. Signed-off-by: AdamMainsTL <adam.mains@topazlabs.com> Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

## Description Minimal change to update link to OpenEXR's new Test Images page. I poked around a bit to find a "friendlier" variant of the link but could find none. Note that the section containing this link implies that folks need to download these files themselves and should be "placed in a sibling directory to oiio-testimages". The problem is that not even OpenEXR lists the repository for the test images nor does the page linked above offer a download. Should we list out the openexr-images repo address too and say to just clone that? Fixes AcademySoftwareFoundation#4042 --------- Signed-off-by: Jesse Yurkovich <jesse.y@gmail.com> Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

lgritz · 2024-03-06T17:42:04Z

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

lgritz · 2024-03-06T19:11:37Z

src/jxl.imageio/jxlinput.cpp

+public:
+    JxlInput() { init(); }
+    ~JxlInput() override { close(); }
+    const char* format_name(void) const override { return "jpeg xl"; }


No space, please. People might use the format_name to construct filenames or other uses where spaces will not be good. Let's call it "jpegxl".

Understood. I mentioned non breaking space instead. However I am OK with jpegxl name. Will modify ASAP.

lgritz · 2024-03-06T19:17:34Z

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.

Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

lgritz · 2024-03-06T19:20:04Z

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).

Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

1div0 · 2024-03-06T19:42:02Z

Renamed. L👀king at the test suite now.

1div0 · 2024-03-07T08:40:58Z

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")

lgritz · 2024-03-07T17:09:07Z

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.

Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

1div0 · 2024-03-08T08:52:17Z

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()

lgritz · 2024-03-08T16:30:42Z

src/jpegxl.imageio/jxloutput.cpp

@@ -263,7 +263,8 @@ JxlOutput::save_image()
                                     size);
    DBG std::cout << "status = " << status << "\n";
    if (status != JXL_ENC_SUCCESS) {
-        DBG std::cout << "JxlEncoderAddImageFrame failed.\n";
+        JxlEncoderError err = JxlEncoderGetError(m_encoder.get());
+        DBG std::cout << "JxlEncoderAddImageFrame failed with error " << err << "\n";


Don't forget a call to errorfmt() here.

Thanks. I guess 4 channel image, i.e. RGB+α is not properly handled yet. Will fix ASAP and try w/ Blender.

goodusername123 · 2024-03-09T04:19:01Z

Apologies if this isn't exactly the right time or place to ask this, But are there any plans to support depth/z channels and maybe even thermal channels? (could possibly be in a later pull request since this one is just "Initial" support?). This would be very useful for projects like Blender to have depth/z maps/data stored/saved in rendered images for example.

libjxl does have support for handling these channel types and there's even a JXL file with such channels here as an example: https://saklistudio.com/jxltests/suz-multichannel-800px.jxl (RGB + Alpha (empty) + Depth + Thermal)

I'm really not too familiar with OpenImageIO as a whole so maybe this would be out of scope? although it seems like the EXR support/code has depth/z channel handling in some form via z_channel? unsure, though I do see a few references to z_channel in the docs.

lgritz · 2024-03-09T05:29:56Z

OpenImageIO can deal with any number of channels having any names. So yes, totally within scope and nothing special at all.

z_channel and alpha_channel are just a special designations in the ImageSpec to mark it for ease of clients of the library so you don't need to go searching for them every time. But it's in no way implies that those are the only channels you can have.

Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

1div0 · 2024-03-09T18:35:24Z

JPEG XL export to RGB with α channel tested by Blender.

lgritz · 2024-03-10T17:09:46Z

src/libOpenImageIO/imageinout_test.cpp

@@ -368,6 +368,18 @@ test_all_formats()
            OIIO_CHECK_ASSERT(ok && "Failed read/write comparison");
            if (ok)
                std::cout << term.ansi("green", "OK\n");
+            else {
+                const float* read_pixels = (const float*)pixels.data();


Let's at least add a comment here saying that since checked_read reads the data as float values, it's safe to cast this to float*.

lgritz · 2024-03-10T17:11:55Z

src/libOpenImageIO/imageinout_test.cpp

+                size_t n = pixels.size() / sizeof(float);
+                float sum_of_absolute_differences = 0.0;
+
+                for (size_t i = 0; i < n; i++) {
+                    float difference = read_pixels[i] - orig_pixels[i];
+                    sum_of_absolute_differences += fabs(difference);
+                    std::cout << "["<< i << "] " << orig_pixels[i] << " " << read_pixels[i] << " δ " << difference << "\n";
+                }
+                std::cout << "sum of absolute differences " << sum_of_absolute_differences << "\n";
+            }


This is the section that's failing the clang-format check.

lgritz · 2024-03-10T17:16:42Z

src/jpegxl.imageio/jxloutput.cpp

+    {
+        return (feature == "alpha" || feature == "nchannels"
+                || feature == "exif" || feature == "ioproxy"
+                || feature == "tiles");


I think that since this is buffering all output until the end, it can support "random_access" also because it doesn't care what order you send the scanlines.

lgritz · 2024-03-10T17:18:01Z

Now that we pass the imageinout_test, I believe that we are ready for a merge if you fix the clang format issue! Then we can come back to beef up this format with other improvements.

Signed-off-by: Larry Gritz <lg@larrygritz.com>

lgritz · 2024-03-16T22:12:48Z

I pushed a few other minor changes. Tests pass now. Mergins.

lgritz · 2024-03-16T22:17:52Z

@1div0 Don't forget we have some important outstanding issues, including

Document in builtinformats.rst
Handle different pixel types -- right now, always reads/writes float.
Compression options
EXIF
Test cases in the testsuite

ssh4net · 2024-03-25T08:16:01Z

@1div0 I can probably add support for UINT8, UINT16, and FLOAT16 output. But I want to be sure we are not referring to each other.

1div0 · 2024-03-25T10:12:06Z

@ssh4net OK, I can add EXIF.

1div0 marked this pull request as draft November 18, 2023 16:38

1div0 mentioned this pull request Nov 19, 2023

[FEATURE REQUEST] Support for JPEG XL #3908

Open

1div0 force-pushed the JPEG XL branch from f229cec to ab25297 Compare November 19, 2023 12:10

1div0 changed the title ~~Initial JPEG XL support for image input~~ Initial JPEG XL support for image input/output Feb 16, 2024

lgritz reviewed Feb 19, 2024

View reviewed changes

1div0 and others added 24 commits February 24, 2024 15:46

Initial JPEG XL support for image input

9808a9e

Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

Avoid macro redefined error

a80702e

Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

chore: more switching fprintf/etc to new style print (AcademySoftware…

ac5dca6

…Foundation#4056) Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

ci: test against gcc-13 (AcademySoftwareFoundation#4059)

ef06678

Switch "bleeding edge" test to gcc13, "latest releases" test to gcc12 Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

doc: Update INSTALL.md to reflect the latest versions we've tested ag…

a762525

…ainst (AcademySoftwareFoundation#4058) Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

Bump patch level

5720ab1

Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

build: fix include guard (AcademySoftwareFoundation#4066)

a4d484f

Don't want that included for Cuda compilation, either. Signed-off-by: Larry Gritz <lg@larrygritz.com> Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

lgritz reviewed Mar 6, 2024

View reviewed changes

Let's call the format "jpegxl"

1dee843

Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

Rename the JPEG XL plug-in

cef3e00

Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

Extended encoder error reporting

f16981c

Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

lgritz reviewed Mar 8, 2024

View reviewed changes

1div0 added 2 commits March 9, 2024 16:38

Add α channel

2ae90e3

Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

Print out subtle difference between written and read image

c086df7

Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>

lgritz reviewed Mar 10, 2024

View reviewed changes

lgritz added 3 commits March 16, 2024 09:42

clang-format

e169c3e

Signed-off-by: Larry Gritz <lg@larrygritz.com>

Limit how much error spews when images don't match

40a03e0

Signed-off-by: Larry Gritz <lg@larrygritz.com>

Clean up approximate matches for lossy formats

3635d53

Signed-off-by: Larry Gritz <lg@larrygritz.com>

lgritz merged commit 41a26ca into AcademySoftwareFoundation:master Mar 16, 2024
26 checks passed

jonnyawsom3 mentioned this pull request Mar 29, 2024

JXL Image Support Yellow-Dog-Man/Resonite-Issues#1562

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial JPEG XL support for image input/output #4055

Initial JPEG XL support for image input/output #4055

1div0 commented Nov 18, 2023

linux-foundation-easycla bot commented Nov 18, 2023 •

edited

Loading

lgritz left a comment

lgritz Feb 19, 2024

lgritz Feb 19, 2024

lgritz Mar 1, 2024

lgritz Feb 19, 2024

lgritz Mar 1, 2024

lgritz Feb 19, 2024

lgritz commented Mar 6, 2024

lgritz Mar 6, 2024

1div0 Mar 6, 2024

lgritz commented Mar 6, 2024

lgritz commented Mar 6, 2024

1div0 commented Mar 6, 2024

1div0 commented Mar 7, 2024

lgritz commented Mar 7, 2024

1div0 commented Mar 8, 2024

lgritz Mar 8, 2024

1div0 Mar 8, 2024

goodusername123 commented Mar 9, 2024

lgritz commented Mar 9, 2024

1div0 commented Mar 9, 2024

lgritz Mar 10, 2024

lgritz Mar 10, 2024

lgritz Mar 10, 2024

lgritz commented Mar 10, 2024

lgritz commented Mar 16, 2024

lgritz commented Mar 16, 2024

ssh4net commented Mar 25, 2024

1div0 commented Mar 25, 2024

Initial JPEG XL support for image input/output #4055

Initial JPEG XL support for image input/output #4055

Conversation

1div0 commented Nov 18, 2023

Description

Tests

Checklist:

linux-foundation-easycla bot commented Nov 18, 2023 • edited Loading

lgritz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgritz commented Mar 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgritz commented Mar 6, 2024

lgritz commented Mar 6, 2024

1div0 commented Mar 6, 2024

1div0 commented Mar 7, 2024

lgritz commented Mar 7, 2024

1div0 commented Mar 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

goodusername123 commented Mar 9, 2024

lgritz commented Mar 9, 2024

1div0 commented Mar 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgritz commented Mar 10, 2024

lgritz commented Mar 16, 2024

lgritz commented Mar 16, 2024

ssh4net commented Mar 25, 2024

1div0 commented Mar 25, 2024

linux-foundation-easycla bot commented Nov 18, 2023 •

edited

Loading