-
Notifications
You must be signed in to change notification settings - Fork 654
Adding HTJ2K Encoding using the OpenJPH library. #4699
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
Merged
Merged
Changes from all commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
d6d331e
Initial commit for HTJ2K support.
richardssam e45e9e9
Use the compression flag for qstep, also default to lossless.
richardssam a8011bf
Adding testing.
richardssam ed4b96c
Adding docs.
richardssam 81d32da
Refining the test.
richardssam 61b5459
Clang formatting.
richardssam 4a8e567
Remove htj2k plugin merging it into the jpeg2000 plugin.
richardssam 4e090e6
Removing annoying file addition.
richardssam ce87082
Updated documentation to for jpeg2000 plugin (migrating from htj2k pl…
richardssam 0ed7e3c
Updating add_oiio_plugin code to support target_link_directories.
richardssam eca23b8
Revamped cmake configs, taking advantage of pkg-config.
richardssam 7786d2f
Adding support for openJPH reader, which is optional, but should impr…
richardssam 92d760a
Adding in openJPH support if enabled, which adds htj2k encoding support.
richardssam c41a551
Added some clarity to what openJPH is providing vs. OpenJpeg
richardssam 2b1141e
Removed the floating point support for now, until if its clear its pa…
richardssam 59686c0
Removed some debug output that shouldnt be there.
richardssam d93d170
Re-enabling the associateAlpha code.
richardssam e6be06e
Removed plugin reference, merged into the jpeg2000 code.
richardssam 8dcdcca
Technically these are part of the jpeg2000 plugin, but only if openjp…
richardssam 0af0325
Remove debug message.
richardssam b4fc92a
Fixing test for new arguments.
richardssam aadfaf1
Applying clang-formatting.
richardssam df5b9a9
0.21 is the baseline, recommended is not needed since its the same ve…
richardssam 00f7207
Whitespace cleanup, 3 lines between functions, removing more than one…
richardssam c432632
Removing un-needed global variable.
richardssam 4fa83c5
Add a default, just in case.
richardssam 814ae5d
Use std::unique_ptr instead of pointers.
richardssam 3bd6029
clang-format fixes.
richardssam 4548850
Another clang-format issue.
richardssam eee3848
WS
richardssam File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| # Copyright Contributors to the OpenImageIO project. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| # https://github.com/AcademySoftwareFoundation/OpenImageIO | ||
|
|
||
| # Module to find OPENJPH. | ||
| # | ||
| # This module will first look into the directories defined by the variables: | ||
| # - OPENJPH_ROOT | ||
| # | ||
| # This module defines the following variables: | ||
| # | ||
| # OPENJPH_INCLUDES - where to find ojph_arg.h | ||
| # OPENJPH_LIBRARIES - list of libraries to link against when using OPENJPH. | ||
| # OPENJPH_FOUND - True if OPENJPH was found. | ||
| # OPENJPH_VERSION - Set to the OPENJPH version found | ||
| include (FindPackageHandleStandardArgs) | ||
| include (FindPackageMessage) | ||
| include (SelectLibraryConfigurations) | ||
|
|
||
| if(DEFINED OPENJPH_ROOT) | ||
| set(_openjph_pkgconfig_path "${OPENJPH_ROOT}/lib/pkgconfig") | ||
| if(EXISTS "${_openjph_pkgconfig_path}") | ||
| set(ENV{PKG_CONFIG_PATH} "${_openjph_pkgconfig_path}:$ENV{PKG_CONFIG_PATH}") | ||
| endif() | ||
| endif() | ||
|
|
||
|
|
||
| find_package(PkgConfig QUIET) | ||
| if(PKG_CONFIG_FOUND) | ||
| pkg_check_modules(OPENJPH_PC QUIET openjph) | ||
| endif() | ||
|
|
||
| if(OPENJPH_PC_FOUND) | ||
| set(OPENJPH_FOUND TRUE) | ||
| set(OPENJPH_VERSION ${OPENJPH_PC_VERSION}) | ||
| set(OPENJPH_INCLUDES ${OPENJPH_PC_INCLUDE_DIRS}) | ||
| set(OPENJPH_LIBRARY_DIRS ${OPENJPH_PC_LIBDIR}) | ||
| set(OPENJPH_LIBRARIES ${OPENJPH_PC_LIBRARIES}) | ||
|
|
||
| if(NOT OPENJPH_FIND_QUIETLY) | ||
| FIND_PACKAGE_MESSAGE(OPENJPH | ||
| "Found OPENJPH via pkg-config: v${OPENJPH_VERSION} ${OPENJPH_LIBRARIES}" | ||
| "[${OPENJPH_INCLUDES}][${OPENJPH_LIBRARIES}]" | ||
| ) | ||
| endif() | ||
| else() | ||
| set(OPENJPH_FOUND FALSE) | ||
| set(OPENJPH_VERSION 0.0.0) | ||
| set(OPENJPH_INCLUDES "") | ||
| set(OPENJPH_LIBRARIES "") | ||
| if(NOT OPENJPH_FIND_QUIETLY) | ||
| FIND_PACKAGE_MESSAGE(OPENJPH | ||
| "Could not find OPENJPH via pkg-config" | ||
| "[${OPENJPH_INCLUDES}][${OPENJPH_LIBRARIES}]" | ||
| ) | ||
| endif() | ||
| endif() | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Just checking: OpenJPH doesn't have an exported cmake config?
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.
I dont think so no. I'll check what they have done with OpenEXR, so we dont have two different approaches.
Uh oh!
There was an error while loading. Please reload this page.
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.
The more recent 0.21.x version do, and I guess 0.21.2 should be the minimum required due to some other bugfixes as well. See also AcademySoftwareFoundation/openexr#1883
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.
Since we never had OpenJPH as a dependency before, there is no need to preserve any back compatibility, so we can set the minimum version to anything we want. If the most recent version makes the build process more foolproof by supplying the exported cmake config files and eliminating the need for a FindOpenJPH module, I think that's a totally valid reason to make it the minimum going forward.
An example of a potential counter-argument would be: if OpenJPH is widely used and an older version is probably on most developer's machines already, requiring the very newest is an extra burden for them, so maybe it's worth accommodating older versions. (But I suspect this is not the case with OpenJPH as it would be with something like libtiff or libjpeg... I bet most developers will have to install OpenJPH for the first time only after we add it as a dependency, so it shouldn't matter much if we require them to have a new version.)
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.
FWIW, I tend to agree; 0.21.x is available OOTB only recently, in the upcoming Fedora 42, Ubuntu 25.04, Debian 13 (trixie), etc.
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.
Hi, so I did clean up the cmake part of findJPH to use pkg-config.
However, it did require changes to add_oiio_plugin to add support for link_directories.
Otherwise the oiio build was asking for libraries it couldnt find. I'm not a cmake expert, so definately could use some eyes on this part, but it seems like I'm now taking advantage of something that should have been there already.