Skip to content

Add ROCJPEG decoder - #1554

Open
NicolasHug wants to merge 21 commits into
mainfrom
rocm_decoder
Open

Add ROCJPEG decoder#1554
NicolasHug wants to merge 21 commits into
mainfrom
rocm_decoder

Conversation

@NicolasHug

@NicolasHug NicolasHug commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Ported from TV.

Would need #1553 first but trying to make progress on this now

@pytorch-bot

pytorch-bot Bot commented Jul 27, 2026

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/meta-pytorch/torchcodec/1554

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 Unclassified Failure

As of commit e8d2e04 with merge base d249ad5 (image):

UNCLASSIFIED FAILURE - DrCI could not classify the following job because the workflow did not run on the merge base. The failure may be pre-existing on trunk or introduced by this PR:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jul 27, 2026
Comment thread .github/workflows/linux_rocm.yaml Outdated
@jithunnair-amd

Copy link
Copy Markdown

@NicolasHug For ROCm versions >= 7.14, the RPATH solution would work (for Linux at least). For ROCm 7.2 and earlier, would you want us to implement the same bundling logic here that we had in torchvision?
Cc @akashveramd

@jithunnair-amd

jithunnair-amd commented Aug 7, 2026

Copy link
Copy Markdown

@NicolasHug For ROCm versions >= 7.14, the RPATH solution would work (for Linux at least). For ROCm 7.2 and earlier, would you want us to implement the same bundling logic here that we had in torchvision? Cc @akashveramd

Pardon, I see that the intent in this PR is not to bundle, and instead install the rocjpeg dependency explicitly at build and test time: packaging/install_rocjpeg.sh

@akashveramd In that case, assuming that this is a problem of locating librocjpeg, it should be a matter of ensuring that the ROCm 7.2 and earlier builds can locate the librocjpeg.so being installed via the above script.

@NicolasHug This might come down to using LD_LIBRARY_PATH or RPATH (but the latter would need to assume an /opt/rocm installation, which is not always strictly true). The LD_LIBRARY_PATH should fix it for the CI env, but users of torchcodec wheels may still see the issue. The latter should fix it for most torchcodec users (except those who have ROCm installed in a non-/opt/rocm path; they would need to set LD_LIBRARY_PATH anyway).

Please let us know what you'd prefer.

@NicolasHug

Copy link
Copy Markdown
Contributor Author

Ideally the ROCm TorchCodec wheel would ship the relevant rocjpeg libs, just like the CUDA wheels are shipping libnvjpeg.

But claude is saying this:

rocJPEG is not shipped by the torch-ROCm
wheel, and bundling it would drag in torch's ROCm libs under mismatched
(hashed) sonames. So we treat it as a runtime dependency provided by the
ROCm install, like FFmpeg. decode_jpeg(device='cuda') therefore needs
ROCm (with rocJPEG) present at runtime.

I haven't verified this claim - if you can find a way to properly bundle the rocjpeg libs, I'm happy to bundle them.

@jithunnair-amd

Copy link
Copy Markdown

Ideally the ROCm TorchCodec wheel would ship the relevant rocjpeg libs, just like the CUDA wheels are shipping libnvjpeg.

But claude is saying this:

rocJPEG is not shipped by the torch-ROCm
wheel, and bundling it would drag in torch's ROCm libs under mismatched
(hashed) sonames. So we treat it as a runtime dependency provided by the
ROCm install, like FFmpeg. decode_jpeg(device='cuda') therefore needs
ROCm (with rocJPEG) present at runtime.

I haven't verified this claim - if you can find a way to properly bundle the rocjpeg libs, I'm happy to bundle them.

@akashveramd please take note

@akashveramd

akashveramd commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

@NicolasHug: It seems the wheels is build and uploaded for ROCm 7.1 & 7.2. But the test runs only on 7.1. Do we want to run the test on both 7.1 & 7.2 or build only on one ROCm version?
FYI, I will be replacing ROCm 7.1/7.2 with 7.2/7.14 version.

@NicolasHug

Copy link
Copy Markdown
Contributor Author

I am happy to add more coverage if there is value there, but my priority is to make the 7.1 (or any other version) work correctly at the moment

LakshmiKumar23 added a commit to LakshmiKumar23/torchcodec that referenced this pull request Aug 18, 2026
Follow the same pattern as rocJPEG (PR meta-pytorch#1554) and RPP by linking rocDecode
directly at build time instead of using runtime loading via dlopen/dlsym.

This is the correct approach for ROCm distribution model where:
- PyPI wheel: CPU-only (ENABLE_ROCM not set, no rocDecode)
- ROCm nightlies wheel: Built with ENABLE_ROCM=1, assumes ROCm is installed

Changes:
- Remove RocDecodeRuntimeLoader.cpp and RocDecodeRuntimeLoader.h
- Add rocdecode::rocdecode to core_library_dependencies in CMakeLists.txt
- Remove rocDecodeAvailable_ flag and loadRocDecodeLibrary() call
- Simplify fallback logic to only check codec support, not library availability

Benefits:
- Simpler code, no dlopen/dlsym complexity
- Consistent with rocJPEG and RPP linking approach
- rocDecode always available when ENABLE_ROCM=1 (build-time guarantee)
- Aligns with PyTorch's separate wheel distribution model

Addresses PR review comment about linking via CMake instead of runtime loading.

Co-Authored-By: Claude <noreply@anthropic.com>
LakshmiKumar23 added a commit to LakshmiKumar23/torchcodec that referenced this pull request Aug 18, 2026
After removing the runtime loader, RocDecCache.h still had the include
directive which caused build failures. The runtime loader is no longer
needed since we link rocdecode directly.

Also add install_rocdecode.sh packaging script following the same
pattern as install_rocjpeg.sh from PR meta-pytorch#1554.

Co-Authored-By: Claude <noreply@anthropic.com>
LakshmiKumar23 added a commit to LakshmiKumar23/torchcodec that referenced this pull request Aug 18, 2026
Add ROCm device interface using rocDecode for hardware-accelerated video
decoding on AMD GPUs. Includes:
- RocmDeviceInterface for ROCm GPU decoding
- RocDecCache for decoder management
- RPP integration for color conversion
- Basic ROCm example
- Packaging script for CI

Direct linking approach following rocJPEG pattern from PR meta-pytorch#1554.

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot. module: rocm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants