Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: JPEG-XL Support (encoding: "jxl") #653

Merged
merged 24 commits into from
Nov 12, 2024

Conversation

william-silversmith
Copy link
Contributor

Hi Jeremy,

My lab has been exploring JPEG-XL for some time, and I finally got it working in my neuroglancer branch. I think it would be a good contribution to the ecosystem.

CloudVolume and Igneous both currently support JPEG-XL.

The implementation here uses the jxl-oxide implemention in Rust which I found easier to integrate than the C++ reference implementation.

Let me know what you think!

This was tested with both Grayscale and RGB color.

@jbms
Copy link
Collaborator

jbms commented Oct 31, 2024

Hi, can you add support for an area constraint in addition to the width and height constraints, as in: 7ae9f64

@william-silversmith
Copy link
Contributor Author

sure, will do.

@william-silversmith
Copy link
Contributor Author

Ok! I think I set it up correctly.

@jbms
Copy link
Collaborator

jbms commented Nov 5, 2024

Can you fix the formatting (see updated CONTRIBUTING.md) and also fix the wasm reproducibility issue?

@william-silversmith
Copy link
Contributor Author

Seems like even though generating on MacOS (on the same machine) is consistent, ubuntu is doing something different. I'll have to try again on an Ubuntu machine.

@william-silversmith
Copy link
Contributor Author

@jbms I rebuilt the wasm on an Ubuntu machine. Please see if the reproducibility issue is fixed by running the check again.

@william-silversmith
Copy link
Contributor Author

Hi just wanted to check in and see if this is good to merge. <3 Thanks for all your hard work Jeremy. Once this merges, we'll start using JXL (lossless at the very least) on an industrial scale.

@jbms jbms merged commit 94f5862 into google:master Nov 12, 2024
22 checks passed
@william-silversmith william-silversmith deleted the wms_jxl branch November 13, 2024 02:47
@jbms
Copy link
Collaborator

jbms commented Nov 27, 2024

@william-silversmith There seems to be a reproducibility issue with the build -- do you think you can take a look? I tried pinning the docker container hash, which should be done anyway, but that does not seem to be sufficient.

@william-silversmith
Copy link
Contributor Author

It might be that I didn't save a Cargo.lock file so if a dependency changes, it could change the binary. What I can do is open a new PR with a lock file generated.

@jbms
Copy link
Collaborator

jbms commented Nov 27, 2024

It might be that I didn't save a Cargo.lock file so if a dependency changes, it could change the binary. What I can do is open a new PR with a lock file generated.

You did include a Cargo.lock file, so I think it may be something else.

@william-silversmith
Copy link
Contributor Author

I see a Cargo.toml on github, but not a lock file in the repo. Am I missing it? Sorry!

@jbms
Copy link
Collaborator

jbms commented Nov 28, 2024

Oh, good point --- I thought I checked for the Cargo.lock myself but I must have misread it --- thanks.

I applied the fix myself in this PR: #672

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants