Improve TheRock build and optional dependency handling - #280
Open
essamROCm wants to merge 3 commits into
Open
Conversation
Signed-off-by: Essam Aly <essam.aly@amd.com>
essamROCm
requested review from
AryanSalmanpour and
LakshmiKumar23
as code owners
August 21, 2026 19:11
Contributor
There was a problem hiding this comment.
Pull request overview
Improves TheRock builds, optional dependency handling, raw H.265 decoding, and combined module testing.
Changes:
- Adds a TheRock development Dockerfile.
- Detects optional FFmpeg and test media availability.
- Fixes H.265 parsing and module coexistence.
- Improves CTest discovery and build-tree media setup.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Summary |
|---|---|
tests/module_coexistence_test.py |
Tests combined module imports. |
tests/CMakeLists.txt |
Adds conditional test and capability discovery. |
src/common/roc_pybuffer.cpp |
Enables module-local buffer bindings. |
samples/rocdecode/videodecoderaw.py |
Fixes H.265 Annex-B parsing. |
docker/rocpydecode-with-therock.dockerfile |
Adds the TheRock environment; repository verification and rocJPEG dependencies require changes. |
CMakeLists.txt |
Detects optional FFmpeg and configures test media. |
Suppressed comments (5)
CMakeLists.txt:243
- When FFmpeg is installed but the rocDecode FFmpeg utilities are absent, this branch marks the build as GPU-only. The install logic later uses
ROCPYDECODE_HOST_LIBRARY_FOUNDonly to omit the two CPU-backend scripts, so demux-dependent samples such asvideodecode.pyare still installed even thoughPyVideoDemuxeris not compiled (src/rocdecode/roc_pydecode.cpp:134-141). Running those installed samples then fails with missing module attributes. UseROCPYDECODE_USE_FFMPEGwhen filtering the installed samples and omit all demux/FFmpeg-dependent scripts for GPU-only builds.
elseif(FFMPEG_FOUND)
message(STATUS "FFmpeg found, but rocDecode FFmpeg utilities were not found under ${ROCDECODE_FFMPEG_UTILS_DIR}; building rocPyDecode without demux/CPU backend (GPU decode only).")
set(ROCPYDECODE_HOST_LIBRARY_FOUND FALSE)
else()
message(STATUS "FFmpeg not found; building rocPyDecode without demux/CPU backend (GPU decode only).")
docker/rocpydecode-with-therock.dockerfile:54
- The image does not clone or copy the repository, so this contributor-specific
WORKDIRpoints at an empty directory unless every user reproduces the same host mount path. A normal container launch therefore does not start in the checkout described by the build/test plan. Use a generic documented mount point (or/workspace) rather than embedding a personal workspace path.
WORKDIR /workspace/EssamWork/ROCPYDECODE
tests/CMakeLists.txt:387
decoder_api_test.pyonly calls the host-specificTestAll*APIs when they exist; its required API checks (AVCodec*, GPU decoder, packet, and surface APIs) are provided by the FFmpeg/GPU binding and do not requirerocdecode_HOST_LIBRARY. This new host-library condition therefore skips an otherwise applicable test on GPU-only installations. Gate this test on the FFmpeg binding (and media availability), not onRUN_ROCPYDECODE_HOST_TESTS.
if(ROCPYDECODE_USE_FFMPEG AND RUN_ROCPYDECODE_HOST_TESTS)
tests/CMakeLists.txt:220
- Both
ROCPYDECODE_PYTHONPATHandROCPYJPEG_PYTHONPATHalready prepend the configure-time$ENV{PYTHONPATH}(see lines 148-160). Consequently, an older installed module from the caller's environment is searched before these build/install outputs, so this coexistence test can import stale extensions and pass without testing the current artifacts. Put the module paths first and append the existing environment once.
"PYTHONPATH=${ROCPYDECODE_PYTHONPATH}:${ROCPYJPEG_PYTHONPATH}:$ENV{PYTHONPATH}"
tests/CMakeLists.txt:177
- When the installed test package is configured separately, this recomputes the FFmpeg capability from the test machine rather than from the already-built binding. A GPU-only rocPyDecode installed from a build where the rocDecode FFmpeg utilities were absent will be treated as FFmpeg-enabled if those files happen to be present later, causing demux/CPU CTests to run against symbols that were never compiled into
rocpydecode. Persist the build feature in install metadata or probe the installed module before enabling these tests.
if(NOT DEFINED ROCPYDECODE_USE_FFMPEG)
find_package(FFmpeg QUIET)
set(ROCDECODE_FFMPEG_UTILS_DIR "${ROCM_PATH}/share/rocdecode/utils/ffmpegvideodecode")
file(GLOB ROCDECODE_FFMPEG_UTILS_SOURCES "${ROCDECODE_FFMPEG_UTILS_DIR}/*.cpp")
set(ROCPYDECODE_USE_FFMPEG FALSE)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation
Enable reliable rocPyDecode builds and testing with current TheRock packages.
Technical Details
Test Plan
Build the image, configure and install rocPyDecode, import both modules, and run all applicable CTests.
Test Result
The image and source built and installed successfully. Raw H.264 and H.265 tests each decoded 601 frames, and the applicable tests passed.
The optional Dockerfile
The Docker image provides:
This enables rocPyDecode to configure, build, install, and run its applicable CTest suite.