-
Notifications
You must be signed in to change notification settings - Fork 71
[WIP] nvImageCodec v0.7.0 decoding #998
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
base: main
Are you sure you want to change the base?
Conversation
- Pin nvimgcodec/nvimagecodec deps to 0.7.0 in deps/conda/pyproject - Make cuslide2 plugin a MODULE + add linkable core lib for tests - Avoid global include_directories; use target_include_directories
Make nvimgcodec dynlink include path PUBLIC on cuslide2_core so the MODULE plugin target can compile cuslide.cpp (includes headers that include <nvimgcodec.h>).
PyPI provides nvidia-nvimgcodec-cu13 as 0.7.0.11
jameslamb
left a comment
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 see that this has [WIP] but since it was opened as a non-draft PR I assumed you wanted a review. Sorry if that's not true.
I left one small comment on the pinning changes. Please note you'll also have to manually update the conda recipes for libcucim:
cucim/conda/recipes/libcucim/conda_build_config.yaml
Lines 19 to 20 in ec077ca
| nvimgcodec_version: | |
| - "0.6.0" |
Could you also add a brief description to the PR explaining the goal? That'd help reviewers... I'm unsure about the purpose of some of the cucim.kit.cuslide2/CMakeLists.txt changes.
| - cuda-cudart-dev | ||
| - libnvjpeg-dev | ||
| - libnvimgcodec-dev>=0.6.0,<0.7.0 | ||
| - libnvimgcodec-dev==0.7.0 |
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.
| - libnvimgcodec-dev==0.7.0 | |
| - libnvimgcodec-dev>=0.7.0,<0.8.0 |
Is it necessary to pin so tightly (==0.7.0 here and ==0.7.0.11 further down in the file)? Generally I think we'd prefer to have looser constraints... that reduces the risk of conflicts when cucim is installed alongside other software.
Would you consider something like this? (here and in every other place touched in this file)
No description provided.