Add support for the lj2k and zstd compression modes. - #5495
QuantaDude wants to merge 8 commits into
Conversation
|
|
| set (_openexr_new_compression "1") | ||
| else () | ||
| set (_openexr_new_compression "0") |
There was a problem hiding this comment.
This name is likely to seem dated in two years when zstd isn't "new" and something else implemented in 2027 is added.
There was a problem hiding this comment.
my bad, I'll name it more specific.
| URL http://github.com/AcademySoftwareFoundation/openexr-images) | ||
| # For OpenEXR >= 3.1, be sure to test with the core option on | ||
|
|
||
| if (OpenEXR_VERSION VERSION_GREATER_EQUAL 3.1.10) |
There was a problem hiding this comment.
Why 3.1.10? Isn't this a feature of 3.5?
There was a problem hiding this comment.
The diff seems messy, I set a conditional variable in the cmake checking if the openEXR version is 3.5.0 or above and then I simply passed it as a env variable to the compression tests. I had to duplicate this existing if block two times.
Anyway, now I've a better way to deal with this using set_property()
There was a problem hiding this comment.
Please check the newer commit, I now append the OIIO_OPENEXR_LJ2K_ZSTD_SUPPORT environment variable to the openexr-compression test when openexr version is at least 3.5.0.
| endif () | ||
|
|
||
| if (OpenEXR_VERSION VERSION_GREATER_EQUAL 3.1) | ||
| # For OpenEXR >= 3.5 we need to add the new compression tests, with the core library |
There was a problem hiding this comment.
Comment doesn't match the if. Which version are we talking about here?
| # For OpenEXR >= 3.5 | ||
| if (OpenEXR_VERSION VERSION_GREATER_EQUAL 3.1.10) | ||
| oiio_add_tests (openexr-compression |
There was a problem hiding this comment.
Comment doesn't match the condition
QuantaDude
left a comment
There was a problem hiding this comment.
I improved testing.cmake, changed the env variable, simplified how it's set and bumped OpenEXR_BUILD_VERSION in build_OpenEXR.cmake.
I have also created a new reference out-legacy.txt file for the test to pass on older OpenEXR versions.
Does openexr_ver in ci.yml need to be set to v3.5.0 as well?
| URL http://github.com/AcademySoftwareFoundation/openexr-images) | ||
| # For OpenEXR >= 3.1, be sure to test with the core option on | ||
|
|
||
| if (OpenEXR_VERSION VERSION_GREATER_EQUAL 3.1.10) |
There was a problem hiding this comment.
Please check the newer commit, I now append the OIIO_OPENEXR_LJ2K_ZSTD_SUPPORT environment variable to the openexr-compression test when openexr version is at least 3.5.0.
It should be for the "latest releases" jobs, but NOT for any of the other jobs, which are intentionally sampling building against a variety of OpenEXR versions. |
|
@lgritz I updated the git commit hash for the local, latest builds for OpenEXR and fixed clang-format CI errors. Not sure why the |
| - float | ||
| - aperture | ||
| * - ``compression`` | ||
| * - ``compression`` |
There was a problem hiding this comment.
This line seems to have changed for no reason
|
Some of the CI failures you're getting revealed a separate problem with OpenEXR and zstd and static libraries. I have an OIIO-side fix here: #5498 And an OpenEXR side here: AcademySoftwareFoundation/openexr#2673 |
- `lj2k` and `zstd` compression mapping in the OpenEXR C++ and OpenEXR Core input paths. - `lj2k` and `zstd` compression selection in the OpenEXR output path. - LJ2K quality handling with the OpenEXR-supported range of `1` to `150`, defaulting to `110`. - ZSTD compression-level handling with the OpenEXR-supported range of `1` to `22`, defaulting to `5`. - Regression coverage for both compression modes in the `openexr-compression` tests, including the OpenEXR Core path. - Reference EXR files and expected output for the new test cases. - Bumped OpenEXR version in `src/build-scripts/build_openexr` from `v3.4.15` to `v3.5.0` - Updated the compression attributes documentation in `src/doc/builtinplugins.md` Signed-off-by: Abhirup Bhattacharyya <abhirup27022001@outlook.com>
Signed-off-by: Abhirup Bhattacharyya <abhirup27022001@outlook.com>
Raise the local build OpenEXR default to 3.5.0 in build_OpenEXR.cmake, matching the version already bumped in the build shell script. Rename the OIIO_OPENEXR_NEW_COMPRESSION environment variable to OIIO_OPENEXR_LJ2K_ZSTD_SUPPORT, old env variable would have read oddly once something else lands in a future OpenEXR release. The lj2k/zstd env var is now added via set_property(TEST ... APPEND PROPERTY ENVIRONMENT ...) once OpenEXR_VERSION >= 3.5.0, after both the base and .core test variants already exist, rather than being threaded through ENVIRONMENT arguments on a separate set of oiio_add_tests() calls. Add testsuite/openexr-compression/ref/out-legacy.txt so the test still passes on OpenEXR < 3.5.0, where lj2k/zstd aren't compiled in. Signed-off-by: Abhirup Bhattacharyya <abhirup27022001@outlook.com>
Signed-off-by: Abhirup Bhattacharyya <abhirup27022001@outlook.com>
Signed-off-by: Abhirup Bhattacharyya <abhirup27022001@outlook.com>
…x docs indent - Added a guard in testing.cmake before setting the `OIIO_OPENEXR_LJ2K_ZSTD_SUPPORT` env variable property for the `openexr-compression` test. To fix openImageIO from not being able to configure and build when the `BUILD_TESTING`, `OIIO_BUILD_TESTS` flag options were not set to `ON`. Signed-off-by: Abhirup Bhattacharyya <abhirup27022001@outlook.com>
be1d0db to
dcc7927
Compare
Signed-off-by: Abhirup Bhattacharyya <abhirup27022001@outlook.com>
|
It's looking like the lj2k.exr is such a close match that it, too, is getting mixed up with zip.exr and leading to failures because it's nondeterministic which of the two matching files it will find first. Hmmm... Maybe you can intentionally choose a lower quality "lj2k" compression to force it to be distinct from the lossless compression? (And to prove to ourselves that there is not some bug that is forcing lj2k to always be equivalent to htj2k and not actually be lossy?) |
|
Oh, also, we should be testing htj2k as well as lj2k, obviously. |
|
@lgritz It's only failing in the bleeding edge build. I checked the bleeding edge's CI (which build on OpenEXR's main branch) CI's build log says openexr is version 4.0.0. I'll commit again with a new exr reference. |
Well, that's not good -- a NaN is creeping in somewhere! Maybe a problem in OpenEXR main? |
|
yes, I can confirm. The main branch's LJ2K compression is producing corrupted output. |
|
D'oh! We were so close to having this wrapped up! |
|
@lgritz I opened an OpenEXR issue regarding this. |
|
Hopefully, with a straightforward repro case, somebody can fix the problem in OpenEXR right away. In the mean time, would you like to add a htj2k to this test as well? |
Added HTJ2K compression tests when OpenEXR 3.4.0 or newer is available, and add expected output for the `htj2k32` and `htj2k256` compression modes. Created a new reference file `out-exr3.1.txt` for idiff when OpenEXR version is older than `3.4.0`. Signed-off-by: Abhirup Bhattacharyya <abhirup27022001@outlook.com>
Description
closes #5482
lj2kandzstdcompression mapping in the OpenEXR C++ and OpenEXR Core input paths.lj2kandzstdcompression selection in the OpenEXR output path.LJ2K quality handling with the OpenEXR-supported range of
1to150, defaulting to110.ZSTD compression-level handling with the OpenEXR-supported range of
1to22, defaulting to5.Regression coverage for both compression modes in the
openexr-compressiontests, including the OpenEXR Core path.Reference EXR files and expected output for the new test cases.
Bumped OpenEXR version in
src/build-scripts/build_openexrfromv3.4.15tov3.5.0Updated the compression attributes documentation in
src/doc/builtinplugins.mdTests
output
Checklist:
and if I used AI coding assistants, I have an
Assisted-by: TOOL / MODELline in the pull request description above. => I have only used ChatGPT for researching, clarifying a few things.
behavior.
PR, by pushing the changes to my fork and seeing that the automated CI
passed there. (Exceptions: If most tests pass and you can't figure out why
the remaining ones fail, it's ok to submit the PR and ask for help. Or if
any failures seem entirely unrelated to your change; sometimes things break
on the GitHub runners.)
fixed any problems reported by the clang-format CI test.
corresponding Python bindings. If altering ImageBufAlgo functions, I also
exposed the new functionality as oiiotool options. => No new APIs, just added two new compression modes for OpenEXR.