Skip to content

Conversation

@MarkLeone
Copy link

  • Gdeflate compression can yield multiple 64K pages.
  • Each chunk includes number of pages and size of each page.
  • Off by default. gdeflate calls are wrapped and conditionally compiled in.
  • CMake and Bazel builds can optionally fetch NVIDIA fork of libdeflate.
    Additional info and discussion on openexr-dev@lists.aswf.io

…late.

- Gdeflate compression can yield multiple 64K pages.
- Each chunk includes number of pages and size of each page.
- Off by default.  gdeflate calls are wrapped and conditionally compiled in.
- CMake and Bazel builds can optionally fetch NVIDIA fork of libdeflate.
@linux-foundation-easycla
Copy link

CLA Not Signed

@cary-ilm
Copy link
Member

Thanks for this! I'll look into the failing CI checks this weekend, I think I know what's going on.

The project policy is to require a signed Contributor License Agreement. I don't recall if Nvidia has an agreement in place, but is that something you can follow up on?

Also, we require signed git commits. Could you amend your commit with "-s"? That adds the "Signed-off-by" line. Note it's not sufficient to submit an additional commit, they all have to be signed, so you'll need to amend the commit and force-push. Or close this PR and submit a fresh one.

message(STATUS "Using externally provided libdeflate: ${EXR_DEFLATE_VERSION}")
# For OpenEXR.pc.in for static build
set(EXR_DEFLATE_PKGCONFIG_REQUIRES "libdeflate >= ${EXR_DEFLATE_VERSION}")
elseif(OPENEXR_FORCE_FETCHCONTENT_DEFLATE)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOT OPENEXR_FORCE_INTERNAL_DEFLATE AND OPENEXR_FORCE_FETCHCONTENT_DEFLATE?

Otherwise the check for gdeflate support below doesn't work if both are somehow set to ON. Or have a mechanism (throw an error?) to ensure they're exclusive?

)

# Toggle this to fetch the NVIDIA libdeflate fork, then update OPENEXR_ENABLE_GDEFLATE in BUILD.bazel.
_enable_gdeflate = False
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could have a flag - like in https://github.com/bazelbuild/bazel-central-registry/blob/0040dfca0c14b4eda9e079fb51fde2567c39a8d9/modules/boost.asio/1.89.0/overlay/BUILD.bazel#L5

In boost.asio one can decide to use OpenSSLvs. BoringSLL vs. No SSL support

Untested (inspiered by boost.asio):

string_flag(
    name = "defalte",
    build_setting_default = "libdeflate",
    values = [
        "libdefalte",
        "ibdefalate_nvl",
    ],
    visibility = ["//visibility:public"],
)

config_setting(
    name = "libdeflate",
    flag_values = {":defalte": "libdeflate"},
)

config_setting(
    name = "libdeflate_nv",
    flag_values = {":defalte": "libdeflate_nv"},
)

...

    deps = [
      ...
    ] + select({
        ":libdeflate": ["@libdeflate//:deflate"], # or maybe use default attirbute here
        ":libdeflate_nv": ["@libdeflate_nv//:deflate"]
    }),

Someone who uses OpenEXR (or your CI job) can than add the flag
build --@openexr//:deflate=libdeflate_nv (this could also be added to the .bazelrc)

@cary-ilm
Copy link
Member

cary-ilm commented Oct 18, 2025

@MarkLeone, looking at this a little deeper, I think the better configuration is to simply vendor in the source code directly from NVIDIA/libdeflate, and avoid the FetchContent altogether. Look in share/util/check_deflate.sh, which is a helper script to be run manually whenever a new version of libdeflate is released. In essence, that's what you've done, but with a forked repo. I think if you change the URL in that script to https://github.com/NVIDIA/libdeflate, then run it, the CMake configuration doesn't to change much at all.

src/lib/OpenEXRCore/compression.c includes only the files out of libdeflate that are needed, the rest are ignored. You'll need to add whatever gdeflate-related files are needed there.

@cary-ilm
Copy link
Member

@MarkLeone, it looks like NVIDIA/libdeflate is based on a very old fork of ebiggers/libdeflate (v1.8 from 2021), which was before libdeflate introduced cmake support, which complicates the setup, since your library has no cmake config. I started working on fixing the CI to test against an external build of NVIDIA/libdeflate, but that's going to require some extra hoops to jump through.

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.

4 participants