-
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
Changes from 22 commits
d6d331e
e45e9e9
a8011bf
ed4b96c
81d32da
61b5459
4a8e567
4e090e6
ce87082
0ed7e3c
eca23b8
7786d2f
92d760a
c41a551
2b1141e
59686c0
d93d170
e6be06e
8dcdcca
0af0325
b4fc92a
aadfaf1
df5b9a9
00f7207
c432632
4fa83c5
814ae5d
3bd6029
4548850
eee3848
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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. | ||
| # | ||
|
Comment on lines
+5
to
+6
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just checking: OpenJPH doesn't have an exported cmake config?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| # 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() | ||
Uh oh!
There was an error while loading. Please reload this page.