feat(jpeg-xl): ICC read and write for JPEG-XL files (issue 4649) #4905
Conversation
Signed-off-by: Shane Smith <[email protected]>
lgritz
left a comment
lgritz
left a comment
There was a problem hiding this comment.
This looks great! Let's see how it comes out in the CI, but if that all passes I think it's good to go.
lgritz
added
file formats
src/jpegxl.imageio/jxloutput.cpp
Outdated
| length)) {} | ||
| errorfmt("JxlEncoderSetICCProfile failed\n"); |
There was a problem hiding this comment.
Is this correct? Did you mean to have an else clause that contains the errorfmt? Or should it be JXL_ENC_SUCCESS != ... and remove the {}?
Signed-off-by: Shane Smith <[email protected]>
|
Your code looks great (though you can see there is a very minor formatting change needed to pass the clang-format test). I think the reason so many CI tests are failing is that libjxl is an optional dependency -- if it's found, jxl support will be enabled, but if the library is not found, its not a hard error, it just disables jxl support. Some of the jobs in our test matrix have (or build) libjxl, but others do not. But the test cases you added are unconditional -- so of course they are not passing for the CI jobs where jxl is not supported by the build configuration. I think the way to proceed is to make a NEW test, jxl, with just these commands pertaining to jxl. In testing.cmake, add that test but specify the |
Signed-off-by: Shane Smith <[email protected]>
|
Thanks @lgritz, that was helpful. I think I have made the necessary changes. |
Signed-off-by: Shane Smith <[email protected]>
lgritz
left a comment
lgritz
left a comment
There was a problem hiding this comment.
I only have two extremely minor comments. If you agree with them, you should be able to apply my suggestions by clicking the buttons.
Co-authored-by: Larry Gritz <[email protected]> Signed-off-by: shanesmith-dwa <[email protected]>
Co-authored-by: Larry Gritz <[email protected]> Signed-off-by: shanesmith-dwa <[email protected]>
lgritz
changed the title
This PR addresses [4649](#4649) implementing ICC read and write support for JPEG-XL. Fixes #4969 The jxl testsuite directory was added with tests. --------- Signed-off-by: Shane Smith <[email protected]>
3 participants
Description
This PR addresses 4649 implementing ICC read and write support for JPEG-XL.
Fixes #4969
Tests
The oiiotool-attribs testsuite was updated and tested.