-
Notifications
You must be signed in to change notification settings - Fork 22
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
cantaloupe java opts to fix jai processor for tiffs #326
cantaloupe java opts to fix jai processor for tiffs #326
Conversation
We now have this running live and formerly erroring objects are now working - e.g. I didn't write an issue up for this - do we need one? |
@noahwsmith - what buildkit tag were you running for cantaloupe? I'm on
|
Hmm looks like maybe you're overriding the processor for TIFFs to use Jai. Should we be supporting Jai? Cantaloupe seems to advise against it From https://cantaloupe-project.github.io/manual/4.0/processors.html#JaiProcessor Warning Development on JAI ended a long time ago, and a minor incompatibility has cropped up in Java 9. Given that supporting JAI is likely to become more problematic as time goes on, this processor should be considered deprecated, and it may be removed in a future release. |
Yes, on this project Jai is better (except this error). Jai isn't the default (which is correct) but this PR does allow Jai to work better than it does out of the box without these params. It is an edge case for sure. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given Cantaloupe's docs advising against Jai, I do not think we should be adding support for the Jai processor.
I think the best path forward is to add a new environment variable CANTALOUPE_JAVA_OPTS
. After that environment variable is created, any docker container could add Jai support for TIFFs using
docker run \
--env CANTALOUPE_JAVA_OPTS="--add-opens java.base/sun.nio.ch=ALL-UNNAMED --add-opens java.base/java.io=ALL-UNNAMED --add-exports java.desktop/sun.awt.image=ALL-UNNAMED -Dcom.sun.media.jai.disableMediaLib=true" \
islandora/cantaloupe:some-future-tag
Thanks @joecorall I think that's a perfect compromise. I'll test this soon and we can get it merged this week... |
@noahwsmith any luck with applying @joecorall's suggested changes? |
allow java opts params Co-authored-by: Joe Corall <[email protected]>
I accepted this suggested change and there are now errors on the docker build: |
@noahwsmith We need to also set the env var in the Dockerfile and update the README |
Even with those changes it does still error out
|
remove quotes around new variable Co-authored-by: Joe Corall <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$ make bake TARGET=cantaloupe
$ ./gradlew cantaloupe:test
Starting a Gradle Daemon (subsequent builds will be faster)
> Task :cantaloupe:tests:ServiceStartsWithDefaults:test
Container cantaloupe-servicestartswithdefaults-cantaloupe-1 Stopping
Container cantaloupe-servicestartswithdefaults-cantaloupe-1 Stopped
BUILD SUCCESSFUL in 15s
11 actionable tasks: 2 executed, 9 up-to-date
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good stuff
Deployed and working on our client site - thanks! |
We're getting errors like this on certain TIFFs:
The Cantaloupe boot sequence says