Skip to content

build(container): pin PyNvVideoCodec at 2.2.3 and move the in-tree FFmpeg to 9.0.1 (DYN-4044) - #14925

Merged
dmitry-tokarev-nv merged 9 commits into
mainfrom
dtokarev/pynvvideocodec-2.2.3
Oct 2, 2026
Merged

dmitry-tokarev-nv merged 9 commits into
mainfrom
dtokarev/pynvvideocodec-2.2.3

Conversation

@dmitry-tokarev-nv

@dmitry-tokarev-nv dmitry-tokarev-nv commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Tracked as DYN-4044.

Summary

PyNvVideoCodec had been declared as >=2.2.0 in all three requirements files since
that floor was introduced, when 2.2.0 was the newest release. A floor with no upper
bound pins nothing, so builds quietly resolved 2.2.1, 2.2.2 and now 2.2.3 as they were
published: the runtime-test images for main@5e21f9c618 all ship 2.2.3 while every
declared floor still read 2.2.0. container/deps/README.md already states the rule —
"Never use >= as it allows untested future versions… Every installed version should
be explicitly tested"
— and since the wheel's bundled FFmpeg is what the codec gate
inspects, the spec is now ==2.2.3 rather than a range.

2.2.3 vendors FFmpeg 9.0.1 where 2.2.0–2.2.2 vendored 8.1.2, which is what makes the
second half of this PR necessary. Moving the in-tree build to the same 9.0.1 would give
two independently configured FFmpegs the same DT_SONAMEs — libavformat.so.63,
libavutil.so.61, libavcodec.so.63. The loader resolves a needed name to exactly
one library per process, so whichever loads first serves both. Reproduced with both
trees present: PyNvVideoCodec's own capability collapses from 359 demuxers to 3, 50
bitstream filters to 6, and 183 muxers to 4
. --build-suffix=_dynamo renames our
libraries and their SONAMEs, so the two can coexist by construction.

Four parts:

  1. Spec >=2.2.0 → ==2.2.3 in requirements.{vllm,sglang,trtllm}.txt and in the
    runtime templates' guard constants. TRT-LLM's sort -V floor test and its separate
    case "$v" in 2.*) cap collapse into [ "$v" = "2.2.3" ]; the venv cross-check
    becomes [ "$vv" = "$v" ], comparing the image's two copies to each other rather
    than each to a bound.

  2. Replace the base image's copy explicitly. The vLLM base ships 2.0.4 (FFmpeg 8.1.1)
    and the TRT-LLM base ships 2.1.0 (FFmpeg 7.0.2); both bundle a complete FFmpeg
    including a libavcodec. The package manager uninstalls first — its RECORD is the
    only complete list of what the wheel wrote, reaching top-level samples/ and
    benchmarks/ trees (41 entries in 2.0.4) and a version-stamped FFmpeg source tarball
    outside site-packages — and the explicit rm follows it for the parts that must not
    depend on the installer honouring its own metadata. Each image then asserts what
    survived. The SGLang base ships none, so it gets the assertion without a removal.

  3. In-tree FFmpeg 8.1.2 → 9.0.1, built with --build-suffix=_dynamo. Three things
    follow from the rename, each of which needed fixing:

    • pkg-config. The .pc files are renamed too, so pkg-config libavformat stops
      resolving and ffmpeg-sys-next's probe fails the Rust build outright.
      Canonical-name symlinks restore it; each still reports -lavformat_dynamo, so
      consumers link the suffixed library.
    • The codec gate. **/libav*.so* does not match a suffixed name, so the scanner
      goes blind to every suffixed library. Three pairs of _dynamo deny globs restore it.
      Each pair covers the plain name and the auditwheel hash-renamed name, for example
      libavcodec_dynamo-<hash>.so.63. allow_paths re-permits ours by path prefix, so a
      third-party suffixed library still fails.
    • auditwheel. fnmatch('libavcodec_dynamo.so.63', 'libavcodec.so.*') is False,
      so leaving the excludes unsuffixed silently re-enables the library grafting they
      exist to prevent.

    video-rs 0.12 requires ffmpeg-next =9.0.0, so lib/llm moves both together with
    the lockfiles re-resolved.

  4. Tests. test_pynvvideocodec_spec.py (renamed from …_floor.py — nothing is a
    floor any more) pins the spec everywhere it is written; a new
    test_ffmpeg_version_consistency.py pins context.yaml ↔ native_packages.yaml ↔
    the codec policy's deny_components floor, which nothing compared before. Both are
    wired into pre-commit, because container/context.yaml and container/templates/
    are .dockerignored and so absent from the component images that run the suite.

A guard that was checking nothing, fixed here. The in-tree codec absence test ran
/usr/local/bin/ffmpeg with stderr discarded before ldconfig, and the binary carries
no RPATH — so against a stale cache it could not start, produced empty output, matched no
disallowed codec and passed. ldconfig now runs first and a positive VP9-encoder check
precedes the absence tests.

No runtime behaviour changes in Python — that diff is comments and docstrings.

Validation

Everything below was executed. Images are the main@5e21f9c618 runtime-test images,
which already carry 2.2.3. The original base-image work used
vllm/vllm-openai:v0.28.0-ubuntu2404, nvcr.io/nvidia/tensorrt-llm/release:1.3.0rc26
and lmsysorg/sglang:v0.5.18-cu130-runtime on an RTX 6000 Ada box. main has since moved
to vLLM v0.30.0, TensorRT-LLM rc28 and SGLang v0.5.19. The next section lists what was
checked again against those.

After merging main

The branch merges main at 3f46e3d3c5. Four files conflicted. In the three requirements
files, main added redis>=6.2.0,<9.0.0 next to the PyNvVideoCodec line. In
trtllm_runtime.Dockerfile, main rewrote the pip resolver note for its base bump. Each
resolution keeps both sides.

main's new base images change facts that these templates depend on. Each one was checked
again on the GPU box:

Claim How verified Result
TensorRT-LLM rc28 still ships 2.1.0, so the tripwire lets the replacement run importlib.metadata in release:1.3.0rc27 and 1.3.0rc28 2.1.0 in both. tensorrt-llm 1.3.0rc28 still requires PyNvVideoCodec~=2.1.0
The TensorRT-LLM replacement works on rc28 built the rendered RUN block verbatim, FROM release:1.3.0rc28 2.1.0 removed, 2.2.3 installed, 0 decoders in each bundled libavformat. pip prints the rc28 complaint that the comment quotes
vLLM v0.30.0 still ships 2.0.4 importlib.metadata in vllm-openai:v0.30.0-ubuntu2404 2.0.4. vllm 0.30.0 requires PyNvVideoCodec==2.0.4
The vLLM removal, install and assertion work on v0.30.0 built the three rendered RUN blocks verbatim, FROM vllm-openai:v0.30.0-ubuntu2404 one 2.2.3 distribution, only libavformat/libavutil .63/.61, one ffmpeg-9.0.1.tar.xz
vLLM 0.30.0's own decoder works with 2.2.3 PyNvVideoCodecVideoBackend.load_bytes on the H.264 and H.265 clips, with the base's 2.0.4 as a control the same under both: (8, 240, 320, 3) uint8, fps 10.0, same frame indices
SGLang v0.5.19 ships no PyNvVideoCodec importlib.metadata in lmsysorg/sglang:v0.5.19-cu130-runtime none
The lockfiles survived the auto-merge cargo metadata --locked in the root and lib/bindings/python, then cargo test --locked both clean. 65/65 media tests pass on the merged tree

The merge also exposed a gap in test_pynvvideocodec_spec.py. main's new DALI error
message names PyNvVideoCodec, and the test counted that echo when it decided which
instructions guard the package. It then read DALI's own 2.2.0 pin as a second
PyNvVideoCodec version. The test now ignores diagnostic lines for that decision too. A new
regression test covers it. With the old logic restored, the new test and the real
template both fail.

A separate commit corrects stale comments. Two were false before the merge: "The in-tree
FFmpeg is 8.1.2", and that 2.2.0 and 2.2.3 both sit "at or above the floor". The floor is
9.0.1. The others went stale with main's base bumps.

GPU run on the CI images for 06c2ff404e

All three -runtime-test images for this head ran on the RTX 6000 Ada box, one container
at a time.

Check Result
In-tree FFmpeg in each image 9.0.1, only lib*_dynamo names in /usr/local/lib, no unsuffixed copies
Every ELF file that needs an FFmpeg library resolves it vLLM 31, SGLang 30, TensorRT-LLM 53 files, 0 unresolved. These counts exclude Pillow's libavif, which the scan also matched. TensorRT-LLM holds three copies with distinct names: ours, PyNvVideoCodec's .so.63, and DALI's hash-renamed .so.62
Dynamo and PyNvVideoCodec in one process (vLLM image) in both import orders, /usr/local/lib/libavformat_dynamo.so.63 and PyNvVideoCodec/libavformat.so.63 are both mapped. NVDEC decodes the H.264 and H.265 clips to (8, 240, 320, 3) uint8 at 10 fps
dynamo/_core.abi3.so (vLLM image) needs four _dynamo libraries and resolves them in /usr/local/lib, so auditwheel did not graft them into the wheel
Unit and dependency suites 779 cases: 747 ran, 32 skipped, 12 failed. Each image's own test_no_software_video_codecs.py case passes, and so does TensorRT-LLM's test_no_opencv.py. The 12 failures are explained below
Serve end to end vLLM 4 of 5, SGLang 3 of 3 plus 1 declared skip, TensorRT-LLM 2 of 2. The vLLM failure is explained below

The unit suites include test_nvdec_decoder.py (36 of 36) and test_nvdec_decoder_gpu.py
(1 of 1) in each image, test_video_loader.py (28 of 28) in each image, the SGLang NVDEC
decoder (9 of 9), and the VP9 encode test (2 of 2) through the suffixed FFmpeg.

The serve runs that pass cover vLLM agg video, agg video with frontend decoding through the
_dynamo libraries, and NVDEC H.264 and H.265. They also cover SGLang agg video and both
E/PD video variants, and TensorRT-LLM NVDEC video and video_diffusion (VP9 output).
SGLang's video_agg_fd_qwen skips with "SGLang container lacks media-ffmpeg support", the
declared gate.

The failing vLLM serve test is mm_epd_video_qwen3-vl-2b_mixed_frontend_decoding. Its
encode worker never reports healthy, so the test times out at 600 s. It fails the same way
on main at 3f46e3d3c5, the commit this branch merged, so it exists on main without this
change. The test is marked post_merge only, and PR CI does not run it. It passed on main
at 5e21f9c618 on September 16. #15432 describes the cause, and #15434 fixed it on main on
September 30.

The 12 unit failures come from this PR's own test_pynvvideocodec_spec.py: 4 checks in each
framework image. Those images have no /workspace/container directory, and the
requirements checks did not skip when their files were absent. CI runs this module only in
the dynamo-runtime image, which ships container/deps, so CI stays green. Commit
3f080e6c09 fixes it: those checks now skip when their files are absent, and script mode
still fails. Inside the vLLM image the module then shows 3 passed, 8 skipped, 0 failed.

PyNvVideoCodec

Claim How verified Before After
Images already ship 2.2.3 while the floor reads 2.2.0 importlib.metadata.version in all three runtime-test images floor 2.2.0 / shipped 2.2.3 spec ==2.2.3 / shipped 2.2.3
2.2.3 bundles FFmpeg 9.0.1, 2.2.0–2.2.2 bundle 8.1.2 wheel contents, sha256-verified from PyPI, cp312 x86_64 + aarch64 libavformat.so.62.12.102, ffmpeg-8.1.2.tar.xz libavformat.so.63.1.101, ffmpeg-9.0.1.tar.xz
The bundled libraries register no codecs container/compliance/enumerate_bundled_decoders.py on the shipped libavformat.so.63 in all three images 2.2.0: 0 decoders / 0 encoders 2.2.3: 0 decoders / 0 encoders
NVDEC decode works either import order decode both fixtures after importing dynamo.llm first — the order that used to fail — in all three images n/a (2.2.3 is what ships) (8, 240, 320, 3) uint8 RGB, C-contiguous, fps 10.0, H.264 and H.265
No consumer depends on an API 2.2.3 drops 2.2.3 removes only SimpleDecoder.stop(); checked Dynamo, vLLM 0.28 multimodal/video.py, tensorrt_llm/media/decoding.py; imported tensorrt_llm.media.decoding against 2.2.3 — no caller; TRT-LLM's four APIs all present
from ast import Str is gone by the pin extracted __init__.py from the 2.2.0/2.2.1/2.2.2/2.2.3 wheels present through 2.2.1 absent from 2.2.2
The removal + assertions work on the real bases docker build of the rendered RUN blocks, extracted verbatim 2.0.4 / 2.1.0 present one distribution at 2.2.3, one ffmpeg-9.0.1.tar.xz, only libavformat/libavutil bundled
…and they actually gate 12 build controls, 4 positive / 8 negative — all 12 as intended (below)
The spec test fails on the pre-change tree ran it against git archive origin/main 7 of 8 checks fail 8 pass
…and pins every constant mutated each of the 10 version-bearing lines one at a time — every mutation fails; 0 survivors

In-tree FFmpeg 9.0.1 + --build-suffix=_dynamo

Claim How verified Before After
The SONAME collision is real both trees present, enumerating PyNvVideoCodec's own capability 359 demuxers / 50 bsfs / 183 muxers 3 / 6 / 4
The suffixed build works end to end docker build of the ffmpeg RUN block extracted verbatim from the rendered vLLM runtime Dockerfile 8.1.2, unsuffixed 9.0.1; libavformat_dynamo.so.63; no unsuffixed libav* installed
pkg-config still resolves by canonical name pkg-config --modversion/--libs in that image n/a 63.1.101 / 63.1.101 / 61.1.101 / 10.1.101 / 7.1.101; -L/usr/local/lib -lavformat_dynamo -lavcodec_dynamo -lavutil_dynamo -lswscale_dynamo
Rust compiles, links and runs against it cargo test -p dynamo-llm --no-default-features --features media-ffmpeg in that image — 65/65 media tests pass, incl. test_sampled_decode_matches_video_rs; binary DT_NEEDED = libavutil_dynamo.so.61, libavformat_dynamo.so.63, libavcodec_dynamo.so.63, libswscale_dynamo.so.10
…and that check is not vacuous injected a call to a nonexistent ffmpeg_next::Rational API into video.rs — error[E0599], exit 101; reverted tree exits 0
video-rs 0.12 requires ffmpeg-next =9.0.0 asked cargo to resolve video-rs 0.12.0 alongside ffmpeg-next = "=8.1.0" — dependency ffmpeg-next = "=9.0.0" of package video-rs v0.12.0
The codec gate still sees our libraries drove scan_filesystem on one tree with and without the three _dynamo deny globs, asserting the mutated policy's actual contents without: violations 0, allowed 0 with: allowed 6, and a planted third-party libavcodec_dynamo.so.63 is a violation
…and two tests pin those globs deleted the three globs and re-ran test_scan_codecs.py — exactly 2 failures, both the tests that name them; 21 unaffected
Hash-renamed suffixed libraries are denied scan_filesystem on planted libavcodec_dynamo-1a2b3c4d.so.63, libswscale_dynamo-1a2b3c4d.so.10 and libpostproc_dynamo-1a2b3c4d.so.59 under a wheel .libs/, with 4 controls 0 of 3 flagged, because no glob matched 3 of 3 flagged. The 4 controls do not change: unsuffixed hashed and plain suffixed third-party copies fail, ours is allowed, and libvpx is ignored
The extended test pins each suffixed glob deleted each of the 6 suffixed globs in turn the old test passed with libpostproc_dynamo.so* or libav*_dynamo-*.so* deleted each deletion fails test_third_party_suffixed_codec_lib_is_denied, and the new test fails on the old policy
The new globs add no violation to the shipped images scan_filesystem on / in the vLLM, SGLang and TensorRT-LLM runtime-test images for 06c2ff404e, with each policy 0 violations, 6, 6 and 17 exceptions, 22 allowed in each identical
The positive VP9 guard fires the same verbatim build guard ran before ldconfig with stderr discarded, so an unstartable binary passed it libvpx-vp9 matched during the build; encoders/decoders/parsers carry no denied codec
The three version declarations stay in lockstep new test_ffmpeg_version_consistency.py no check existed reverting either declaration to 8.1.2 exits 1; control and restore exit 0
Templates still render container/render.py, 11 framework/device/target combinations 11 OK 11 OK
Compliance suite container/compliance/tests/ — 99 passed, 2 skipped that need Debian (macOS, 88ab323da0). test_scan_codecs.py passes 23 of 23 in a Linux container with no network

Build controls. Positive: vLLM replace-and-assert; the same with a check that 2.0.4's
samples/basic/encode_config.json is gone (the orphan the uninstall-first ordering
prevents); SGLang; TRT-LLM. Negative, each of which must fail the build and did: base
ships no PyNvVideoCodec · assertion against the untouched 2.0.4 base · removal skipped,
so the presence guard must fire · a second dist-info forged beside the new one · a second
FFmpeg source tarball · a forged libpostproc · the bundled libraries deleted (the
vacuous-pass case) · SGLang with nothing installed · TRT-LLM with both the uninstall and
the external/ffmpeg removal dropped, which ships 7.0.2 beside 9.0.1. The TRT-LLM
"base is newer than the pin" branch was exercised directly over 2.1.0 / 2.2.3 / 2.2.4 /
2.3.0 / 3.0.0, since no published release sits above it.

Video suites, all three images — 558 executed, 15 skipped, 0 failed, 0 errors. Every
skip was read from the JUnit XML; all 15 are framework markers or one declared gate.

  • test_nvdec_decoder.py — 36 executed, 0 skipped, in each image
  • test_nvdec_decoder_gpu.py::test_nvdec_decodes_real_clips — executed and passed in each
  • test_no_software_video_codecs.py / test_no_opencv.py — each image's own case passed
  • test_video_encode_real.py (VP9 output) — 2 executed, 0 skipped
  • Serve end-to-end, every video configuration the three suites define: vLLM 5/5
    (mm_agg_video_nvdec_*_h264, _h265, mm_agg_video, *_frontend_decoding,
    mm_epd_video_*), TRT-LLM 2/2 (aggregated_multimodal_video_nvdec,
    video_diffusion), SGLang 3/3 (video_e_pd_qwen_nvdec, video_agg_qwen,
    video_e_pd_qwen) plus video_agg_fd_qwen skipped on the repo-declared
    enable_media_ffmpeg: "false" gate

That covers NVDEC H.264 and H.265 on all three backends, VP9 input, frontend decoding,
disaggregated E/P/D video, and VP9 output.

Review rounds. Thirteen inline comments from dynamo-review-agent; twelve applied,
one declined with evidence.

The human review left four inline comments. 88ab323da0 adds the hash-renamed deny globs
and removes the measured counts from the policy comment, because this description already
holds them. The shared install guard moves to #15582, as both reviewers agreed.

The one that mattered: the TRT-LLM system-site install was left unbounded at
>=2.2.3 after the requirements file gained ,<3, so a published 3.x could have landed
in system site-packages while the venv stayed on 2.x — and the block's own comment claims
the two specifiers are identical. The root cause was that the spec test scanned lower
bounds only; it now also compares a template's inline install specifier against the
declared spec. The exact pin removes the class of bug outright, and that check stays.

Two corrections to my own earlier claims, both from these rounds: the codec-waiver
SONAME series do not discriminate against each other (any narrowing unwaives the bare
.so symlinks and fails both, so one series suffices — the real fix there was that the
fixture named .so.60/.so.62, which no supported version ships); and
from ast import Str was removed in 2.2.2, not 2.2.1.

Declined, with evidence: removing test_the_spec_is_bounded_above as a subset of the
exact-spec cases. Those move with EXPECTED_SPEC; this one does not. Mutating all three
requirements files and every constant self-consistently to a range leaves the other cases
green and fails only that one.

Correction: common/tests/multimodal/test_embedding_transfer.py crashed with rc 139 in
the September 16 run, and an earlier version of this description called it an unrelated
crash to raise separately. The crash came from my test harness, not from the code. Those
containers ran with Docker's default 64 MB /dev/shm. With --shm-size=16g, the file
passes 13 of 13 in all three images for 06c2ff404e. With the default shm, it still
crashes in the same image. There is nothing to raise.

Where verification stopped. The FFmpeg 9.0.1 and suffix work is verified on x86_64
only, on the RTX 6000 Ada box, and the aarch64 path rests on CI. The aarch64 PyNvVideoCodec
wheel was checked at file level only: the same two libraries, the same SONAMEs, the same
ffmpeg-9.0.1.tar.xz. The minimal rc28 build has no /opt/dynamo/venv, so it skips the
venv cross-check, and only CI runs that branch. pre-commit run --all-files passes all 37
hooks at 88ab323da0.

🤖 Generated with Claude Code

@dmitry-tokarev-nv
dmitry-tokarev-nv requested review from a team as code owners September 16, 2026 06:00
@github-actions github-actions Bot added build backend::vllm Relates to the vllm backend backend::trtllm Relates to the trtllm backend multimodal container labels Sep 16, 2026
@coderabbitai

coderabbitai Bot commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 9be94c0a-2380-44e8-91ff-49305344d141

📥 Commits

Reviewing files that changed from the base of the PR and between 883db9b and f3d9919.

📒 Files selected for processing (10)
  • components/src/dynamo/common/multimodal/nvdec_decoder.py
  • components/src/dynamo/common/tests/multimodal/test_nvdec_decoder.py
  • container/compliance/tests/test_scan_codecs.py
  • container/deps/requirements.sglang.txt
  • container/deps/requirements.trtllm.txt
  • container/deps/requirements.vllm.txt
  • container/templates/sglang_runtime.Dockerfile
  • container/templates/trtllm_runtime.Dockerfile
  • container/templates/vllm_runtime.Dockerfile
  • tests/dependencies/test_pynvvideocodec_floor.py

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


Walkthrough

Changes

PyNvVideoCodec 2.2.3 compatibility

Layer / File(s) Summary
Dependency floor and consistency checks
container/deps/requirements.*.txt, tests/dependencies/test_pynvvideocodec_floor.py
All three requirement files now require PyNvVideoCodec >=2.2.3. New tests verify consistent requirement and Dockerfile guard values.
Runtime package replacement and validation
container/templates/*_runtime.Dockerfile, container/compliance/tests/test_scan_codecs.py
CUDA runtime templates remove or replace base-image installations and validate the package version, FFmpeg source tarball, and bundled libraries. Compliance coverage includes FFmpeg 8.1.2 and 9.0.1 SONAME variants.
Decoder and compatibility documentation
components/src/dynamo/common/multimodal/nvdec_decoder.py, components/src/dynamo/common/tests/multimodal/test_nvdec_decoder.py
NVDEC comments and test documentation update validated PyNvVideoCodec references to the 2.2.3 baseline. Warning-suppression behavior remains documented.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to f3d99

The dependency and runtime-image updates have no identified actionable merge risk.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 4 files. (6 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The description explicitly associates the pull request with tracking issue DYN-4044.
Out of Scope Changes check ✅ Passed The changes align with the stated objectives. They update PyNvVideoCodec and FFmpeg compatibility, container validation, compliance tests, and related documentation. No unrelated implementation change…
Title check ✅ Passed The title clearly identifies the PyNvVideoCodec 2.2.3 update and the FFmpeg 9.0.1 migration. It matches the primary changes.
Description check ✅ Passed The description provides detailed scope, implementation details, validation results, and the DYN-4044 reference. It does not use the template's explicit reviewer-start section or the required Related …
Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 4 files. (6 skipped: 6 unsupported.)

  • Fix all pre-merge checks with AI

Comment @coderabbitai help to get the list of available commands.

Comment thread components/src/dynamo/common/multimodal/nvdec_decoder.py
Comment thread components/src/dynamo/common/tests/multimodal/test_nvdec_decoder.py Outdated
Comment thread container/compliance/tests/test_scan_codecs.py Outdated
Comment thread tests/dependencies/test_pynvvideocodec_floor.py Outdated
Comment thread tests/dependencies/test_pynvvideocodec_floor.py Outdated
@dmitry-tokarev-nv
dmitry-tokarev-nv force-pushed the dtokarev/pynvvideocodec-2.2.3 branch 2 times, most recently from 81b1548 to 2629b7c Compare September 16, 2026 13:55
Comment thread container/templates/trtllm_runtime.Dockerfile Outdated
Comment thread container/templates/trtllm_runtime.Dockerfile Outdated
Comment thread container/templates/trtllm_runtime.Dockerfile Outdated
Comment thread container/templates/trtllm_runtime.Dockerfile Outdated
Comment thread container/templates/trtllm_runtime.Dockerfile Outdated
Comment thread tests/dependencies/test_pynvvideocodec_floor.py Outdated
Comment thread tests/dependencies/test_pynvvideocodec_spec.py
Comment thread tests/dependencies/test_pynvvideocodec_floor.py Outdated
@dmitry-tokarev-nv
dmitry-tokarev-nv force-pushed the dtokarev/pynvvideocodec-2.2.3 branch from 2629b7c to 663eac8 Compare September 16, 2026 15:06
Comment thread components/src/dynamo/common/multimodal/nvdec_decoder.py Outdated
Comment thread components/src/dynamo/common/multimodal/nvdec_decoder.py Outdated
Comment thread container/compliance/tests/test_scan_codecs.py Outdated
Comment thread container/deps/requirements.vllm.txt Outdated
Comment thread tests/dependencies/test_pynvvideocodec_floor.py Outdated
Comment thread tests/dependencies/test_pynvvideocodec_floor.py Outdated
Comment thread container/templates/trtllm_runtime.Dockerfile Outdated
@dmitry-tokarev-nv
dmitry-tokarev-nv force-pushed the dtokarev/pynvvideocodec-2.2.3 branch from c7eb2fa to 0ba2c13 Compare September 16, 2026 17:42

@dynamo-review-agent dynamo-review-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Previously reported defects still present:

  • Original discussion: The module docstring in tests/dependencies/test_pynvvideocodec_floor.py still recounts the prior >=2.2.0 to 2.2.3 drift. The lasting rationale is only that an unbounded lower bound can resolve newer releases, so the historical version sequence remains superfluous.

Comment thread container/deps/requirements.trtllm.txt Outdated
Comment thread container/deps/requirements.vllm.txt Outdated
dmitry-tokarev-nv and others added 2 commits September 16, 2026 14:39
The spec has read `>=2.2.0` since it was introduced, when 2.2.0 was the newest
release. A floor with no upper bound pins nothing, so builds resolved 2.2.1,
2.2.2 and now 2.2.3 as they were published without anything being restated: the
runtime-test images for main@5e21f9c618 all carry 2.2.3 while every declared
floor still read 2.2.0. `container/deps/README.md` already says why -- "Never use
`>=` as it allows untested future versions" -- so the spec becomes
`>=2.2.3,<3`, matching the bounded siblings in the same files. The TRT-LLM image
installs a second, system-site copy of its own and takes the same specifier; its
shell guards check the cap as a separate major-version test, because `sort -V`
can express a floor but not an upper bound.

2.2.3 vendors FFmpeg 9.0.1 where 2.2.0-2.2.2 vendored 8.1.2, which moves the
SONAMEs of the two libraries the wheel bundles: libavformat.so.62 -> .so.63 and
libavutil.so.60 -> .so.61. They no longer collide with the in-tree FFmpeg 8.1.2,
which ships .so.62/.so.60 built with different configure flags, so NVDEC decode
no longer depends on which copy the loader resolves first.

The bundled library content is unchanged by the FFmpeg major bump:
enumerate_bundled_decoders.py reports 0 decoders and 0 encoders for
libavformat.so.63 in all three images, the same as 2.2.0 reported for .so.62.

Also replace the base image's copy explicitly rather than leaving it to the
installer's upgrade path. Both the vLLM base (2.0.4, FFmpeg 8.1.1) and the
TRT-LLM base (2.1.0, FFmpeg 7.0.2) ship a complete FFmpeg including a libavcodec,
and both write files outside site-packages -- a version-stamped FFmpeg source
tarball -- that a package-directory glob does not reach. The package manager
uninstalls first, because its RECORD is the only complete list of what the wheel
wrote (it also covers top-level samples/ and benchmarks/ trees), and the explicit
rm follows it for the parts that must not depend on the installer honouring its
own metadata. Each image then asserts what survived: exactly one distribution on
sys.path inside the declared range, exactly one source tarball at the location
its own RECORD names, a libavformat and libavutil present so the negative checks
cannot pass vacuously, and none of the library families codec_policy.yaml denies.

Presence is tested with importlib.util.find_spec, not an import: PyNvVideoCodec's
__init__ dlopens libnvidia-encode.so.1, which no builder has, so an import raises
whether or not the wheel is installed and could never fail a build.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Dmitry Tokarev <dtokarev@nvidia.com>
… written

The spec is written in three requirements files; the TRT-LLM template installs a
system-site copy with its own inline specifier, and all three templates repeat
the lower bound as a guard constant. The templates cannot parse the requirements
file they are checking, since the point of those guards is to fail when the
installed version and the declared one disagree, so the duplication is deliberate
and needs a test rather than a comment.

Three things are pinned. The whole specifier in each requirements file; the
inline specifier in any template that installs the package, which is what catches
a template left at a floor while the requirements file gained a cap; and every
version literal the shell guards compare against. The last of those is
mutation-checked -- each of the ten floor-bearing lines individually fails the
suite. Two only became reachable after the scan was widened: a shell guard's two
sides sit on different lines (`printf '%s\n2.2.3\n'` and
`[ "$newest" != "2.2.3" ]`), and pinning only the side carrying the package name
let a half-finished bump invert the comparison. Diagnostic lines are excluded,
because that is where versions the guard is deliberately not comparing against
appear.

`test_the_spec_is_bounded_above` is kept deliberately, though it looks like a
weaker subset of the exact-spec cases. It is the only one that survives an edit
which drops the cap and updates EXPECTED_SPEC to match: mutating all three
requirements files and every constant self-consistently leaves six cases green
and fails only that one.

.dockerignore keeps `container/templates/*.Dockerfile` out of every build
context, so the component images that run this suite have the requirements files
but not the templates; those cases skip there, as `test_aisimulate_consistency`
already does for the planner Dockerfile. The `pynvvideocodec-spec` pre-commit
hook runs the module as a script against a real checkout, so the template half
stays enforced in CI rather than only on a developer's machine.

The codec-gate waiver fixture moves to the SONAMEs the shipped floor bundles
(.so.61/.so.63); it had been written against .so.60/.so.62, which no version at
or above the floor ships. One series is enough: the waiver globs are
SONAME-agnostic, and pinning either of them to any single series unwaives the
bare `.so` symlinks and fails the fixture whichever series it names.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Dmitry Tokarev <dtokarev@nvidia.com>
@dmitry-tokarev-nv
dmitry-tokarev-nv force-pushed the dtokarev/pynvvideocodec-2.2.3 branch from 0ba2c13 to a18618f Compare September 16, 2026 18:39

@dynamo-review-agent dynamo-review-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Previously reported defects still present:

  • Original discussion: The module docstring in tests/dependencies/test_pynvvideocodec_floor.py still recounts the prior >=2.2.0 to 2.2.3 drift; the durable test rationale is only that a lower-bound requirement can resolve newer releases, so the historical version-specific narration remains superfluous.
  • Original discussion: The module docstring still recounts the prior >=2.2.0 to 2.2.3 drift. The durable rationale is that an unbounded lower bound can resolve newer releases; the historical version sequence remains present.
  • Original discussion: The module docstring still recounts the prior >=2.2.0 to 2.2.3 drift. The lasting rationale is only that a lower bound can resolve newer releases, so the historical version sequence remains superfluous.
  • Original discussion: test_source_fps_prefers_stream_metadata still has a docstring that merely restates the test name and constructed assertion. The earlier version-specific wording was removed, but the remaining comment is still redundant.

@dmitry-tokarev-nv dmitry-tokarev-nv changed the title build(container): raise the PyNvVideoCodec floor to the validated 2.2.3 (DYN-4044) build(container): bound PyNvVideoCodec at the validated 2.2.3 (DYN-4044) Sep 16, 2026
`>=2.2.3,<3` still resolves whatever is newest inside the major at build time,
so the declared version stayed a floor rather than a record of what was
validated -- the same shape of failure that let the images run 2.2.3 while the
spec read `>=2.2.0` and nothing noticed. container/deps/README.md asks that
every installed version be explicitly tested, and here the wheel's bundled
FFmpeg is what the codec gate inspects, so an unaudited bundle must not be able
to arrive on its own.

The template guards follow the specifier. TRT-LLM's `sort -V` floor test and its
separate `case "$v" in 2.*)` cap collapse into `[ "$v" = "2.2.3" ]`, and the
venv cross-check becomes `[ "$vv" = "$v" ]`, which compares the image's two
copies to each other rather than each to a bound. The vLLM and SGLang heredocs
replace `parse(...) < parse(FLOOR)` with an equality against PINNED, so the
now-unused `parse()` helper goes.

The obsolete-base tripwire in trtllm_runtime.Dockerfile now also fires when the
base ships exactly 2.2.3, not only something newer: with an exact pin, a base
that already carries the pinned version makes the removal-and-reinstall block a
no-op that should be deleted.

Renames tests/dependencies/test_pynvvideocodec_floor.py to `..._spec.py`.
Nothing here is a floor any more, and a stale name is precisely the drift that
file exists to catch.

Validation: the renamed module passes 8/8 under pytest and exits 0 as the
pre-commit hook script, printing "PyNvVideoCodec spec ==2.2.3 is consistent
across 3 requirements files and 3 templates." `pre-commit run` over the changed
files passes every hook.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Dmitry Tokarev <dtokarev@nvidia.com>
…ix=_dynamo

PyNvVideoCodec 2.2.3 vendors FFmpeg 9.0.1, so moving the in-tree build to the
same release gives two independently configured builds the same DT_SONAMEs --
libavformat.so.63, libavutil.so.61, libavcodec.so.63. The loader resolves a
needed name to exactly one library per process, so whichever is loaded first
serves both, and the two builds enable disjoint feature sets. Reproduced on a
GPU box with both trees present: PyNvVideoCodec's own capability collapses from
359 demuxers to 3, 50 bitstream filters to 6, and 183 muxers to 4.

`--build-suffix=_dynamo` renames the libraries and their SONAMEs, so ours ship
as libavformat_dynamo.so.63 and cannot be mistaken for a vendored copy. Three
things follow from the rename and each needed fixing:

- pkg-config. The .pc files are renamed too, so `pkg-config libavformat` stops
  resolving and ffmpeg-sys-next's probe fails the Rust build outright.
  Canonical-name symlinks restore it; each still reports -lavformat_dynamo, so
  consumers link the suffixed library.
- The codec gate. `**/libav*.so*` does not match a suffixed name. Measured by
  rescanning one tree with and without the three new deny globs: violations
  1 -> 0 and allowed 6 -> 0, i.e. the scanner went blind to every suffixed
  library, ours and a planted third-party one alike. allow_paths re-permits ours
  by path prefix, so a third-party suffixed libavcodec still fails.
- auditwheel. fnmatch('libavcodec_dynamo.so.63', 'libavcodec.so.*') is False, so
  leaving the excludes unsuffixed silently re-enables the library grafting they
  exist to prevent.

video-rs 0.12 requires ffmpeg-next =9.0.0 -- cargo reports `dependency
ffmpeg-next = "=9.0.0" of package video-rs v0.12.0` when asked to resolve it
against 8.1.0 -- so lib/llm moves both together, with the lockfiles re-resolved.

Separately, the codec absence guard was checking nothing. It ran
/usr/local/bin/ffmpeg with stderr discarded before ldconfig, and the binary
carries no RPATH, so against a stale cache it could not start, produced empty
output, matched no disallowed codec and passed. ldconfig now runs first and a
positive VP9-encoder check precedes the absence tests, so an unstartable binary
fails loudly instead of quietly.

tests/dependencies/test_ffmpeg_version_consistency.py is new: context.yaml
decides what is compiled, native_packages.yaml is what the SBOM claims, and the
codec policy's deny_components floor is what that SBOM is measured against.
Nothing compared them, and only one direction failed loudly. All three read
8.1.2 before this change, so it pins an invariant the repo already held.

Validation on a GPU box (RTX 6000 Ada):
- The ffmpeg RUN block extracted verbatim from the rendered vLLM runtime
  Dockerfile builds clean. pkg-config resolves all five canonical names to
  -l*_dynamo; the positive VP9 check fires; encoders, decoders and parsers carry
  no denied codec; no unsuffixed libav* is installed.
- cargo test -p dynamo-llm --no-default-features --features media-ffmpeg:
  65/65 media tests pass, including test_sampled_decode_matches_video_rs and the
  decode paths. The test binary's DT_NEEDED entries are libavutil_dynamo.so.61,
  libavformat_dynamo.so.63, libavcodec_dynamo.so.63 and libswscale_dynamo.so.10.
  Injecting a call to a nonexistent ffmpeg-next API into video.rs fails the
  build, so the check is not vacuous.
- container/compliance/tests: 93 passed. Deleting the three new deny globs fails
  exactly the two tests that pin them. All 11 Dockerfile flavours render, and
  `pre-commit run` over the changed files passes every hook.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Dmitry Tokarev <dtokarev@nvidia.com>
@dmitry-tokarev-nv dmitry-tokarev-nv changed the title build(container): bound PyNvVideoCodec at the validated 2.2.3 (DYN-4044) build(container): pin PyNvVideoCodec at 2.2.3 and move the in-tree FFmpeg to 9.0.1 (DYN-4044) Sep 18, 2026
@rmccorm4 rmccorm4 mentioned this pull request Sep 27, 2026
dmitry-tokarev-nv and others added 4 commits September 30, 2026 01:04
Four files conflicted. Each resolution keeps both sides.

In container/deps/requirements.{vllm,sglang,trtllm}.txt, main added
redis>=6.2.0,<9.0.0 right after the PyNvVideoCodec line. This branch
rewrote that line. The resolution keeps the PyNvVideoCodec==2.2.3 block
and adds main's redis lines after it.

In container/templates/trtllm_runtime.Dockerfile, main updated the note
about the pip resolver complaint for its new base image. This branch
rewrote the same note for the exact pin. The resolution keeps the
exact-pin text and takes main's wording where it is more precise. The
quoted pip message names 1.3.0rc28, the tag that container/context.yaml
now pins. The resolution drops main's sentence about treating 2.2.3 as
whatever the build resolved. Under ==2.2.3, that sentence is false.

Checks on the merged tree:

- Both Cargo.lock files pass cargo metadata --locked.
- cargo test --locked in lib/llm with --features media-ffmpeg passes
  65 of 65 media tests against the suffixed FFmpeg 9.0.1.
- The new base images match what the removal blocks expect.
  TensorRT-LLM 1.3.0rc28 ships PyNvVideoCodec 2.1.0, vllm-openai
  v0.30.0 ships 2.0.4, and SGLang v0.5.19 ships none.

tests/dependencies/test_pynvvideocodec_spec.py fails on this tree,
because a new DALI error message from main names PyNvVideoCodec. The
next commit fixes the test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Dmitry Tokarev <dtokarev@nvidia.com>
main added a DALI guard to trtllm_runtime.Dockerfile whose error message
says "the way PyNvVideoCodec 2.2.0 links it into libavformat". The spec
test decided which instructions guard PyNvVideoCodec by looking at every
code line, including echo lines. So it counted the whole DALI
instruction, read DALI's own 2.2.0 pin as a PyNvVideoCodec version, and
failed the template for the wrong reason.

The test now leaves diagnostic lines out of that decision too, as it
already did when it collects versions. A new test feeds it a DALI-shaped
guard and a PyNvVideoCodec guard in one template and expects only 2.2.3.

Checks:

- With the fix, the module passes 9 of 9. With the previous logic
  restored, the new test and the real trtllm_runtime.Dockerfile case
  both fail.
- Each non-comment template line that carries 2.2.3 was changed to
  2.2.2, one at a time. All 6 code lines fail the check. The one line
  that passes is the "wanted PyNvVideoCodec 2.2.3" error text, which
  the test leaves out on purpose.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Dmitry Tokarev <dtokarev@nvidia.com>
Some comments on this branch were false after the merge, and some were
already false before it.

This branch made them false:

- trtllm_runtime.Dockerfile said "The in-tree FFmpeg is 8.1.2" and that
  the SONAME change alone keeps the two FFmpeg copies apart. The in-tree
  build is 9.0.1 now. The --build-suffix=_dynamo rename in
  wheel_builder.Dockerfile keeps them apart, and the note now says so.
- The same note put both 2.2.0 and 2.2.3 "at or above the floor". The
  floor is 9.0.1, so 2.2.0 (FFmpeg 8.1.2) sits below it.
- vllm_runtime.Dockerfile and the spec test still described the old
  range: "REPLACED at >=2.2.3", "held to the floor", FLOOR, and a
  >=2.2.3,<3 example.
- wheel_builder.Dockerfile kept a remark about an 8.1 -> 8.1.2 bump.

The merge made them false:

- TensorRT-LLM: the note named rc26 as the pinned base. main now pins
  rc28. Both rc27 and rc28 still ship PyNvVideoCodec 2.1.0.
- vLLM: 0.30.0 moved its PyNvVideoCodec consumer to
  vllm/multimodal/video_decoders/pynvvideocodec.py. It uses the same
  five names as before.

Checks: vLLM 0.30.0 decodes the H.264 and H.265 test clips through
PyNvVideoCodecVideoBackend.load_bytes with 2.2.3 installed. The result
matches the base's own 2.0.4: shape (8, 240, 320, 3), uint8, fps 10.0,
and the same frame indices. FFmpeg 9.0.1 still opens fd: for "-i -".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Dmitry Tokarev <dtokarev@nvidia.com>
…ner/

The vllm, sglang and trtllm runtime-test images have no /workspace/container
directory. Run by path inside the vLLM image for 06c2ff4,
test_pynvvideocodec_spec.py failed 4 checks with FileNotFoundError on
container/deps/requirements.*.txt. The template checks already skip when
their file is absent, and the requirements checks now do the same. CI did
not see this, because CI runs the module only in the dynamo-runtime image,
which ships container/deps.

Script mode still reports a missing requirements file as an error. The
pynvvideocodec-spec pre-commit hook runs that mode in a checkout.

Checks:

- Inside the vLLM runtime-test image, with the fixed file mounted over the
  image's copy: 4 failed before, 3 passed and 8 skipped after.
- Locally the module passes 11 of 11, and pre-commit passes.
- Removing the skip fails the new skip test. Making main() ignore skipped
  checks fails the new script-mode test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Dmitry Tokarev <dtokarev@nvidia.com>

@furionw furionw left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you

@peilii peilii left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The suffix is the right fix for the SONAME collision, and each knock-on (pkg-config symlinks, suffixed deny globs with the third-party-still-denied test, suffixed auditwheel excludes) is necessary and tested. Checked on this head: both new tests pass through the pre-commit hooks, both lockfiles pass cargo metadata --locked, the runtime cp -nL libav*.so* libsw*.so* globs still match the suffixed names, and the image-side codec test excludes the in-tree copy by path rather than name, so neither is affected by the rename. One structural comment inline.

Comment thread container/templates/vllm_runtime.Dockerfile
Comment thread container/compliance/policy/codec_policy.yaml
Comment thread container/templates/sglang_runtime.Dockerfile
Comment thread container/compliance/policy/codec_policy.yaml Outdated

@rmccorm4 rmccorm4 left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM other than a few small optional comments above - feel free to reject/defer.

auditwheel renames each library that it grafts into a wheel to
<name>-<hash>.so.N (copylib in auditwheel 6.8.2 repair.py). A grafted
libavcodec_dynamo.so.63 therefore lands as libavcodec_dynamo-<hash>.so.63.
The suffixed deny globs matched only the plain name, so scan_filesystem
gave no verdict for the hashed form. Each suffixed family now has the
same plain and hash-renamed pair as the families above it.

The policy comment no longer carries the measured counts from the first
review round. The PR description keeps them.

Checks:

- The extended test plants both names for libavcodec, libswscale and
  libpostproc under the .libs directory of a wheel. On the old policy it
  fails, because the 3 hashed paths are not violations.
- Deleting any one of the 6 suffixed globs fails the test. The old test
  still passed with the plain libpostproc_dynamo glob deleted.
- The vLLM, SGLang and TensorRT-LLM runtime-test images for 06c2ff4
  give the same scan result with both policies: 0 violations, and the
  same exceptions and allowed files.
- The compliance suite passes 99 tests on macOS. test_scan_codecs.py
  passes 23 tests in a Linux container with no network. pre-commit
  passes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Dmitry Tokarev <dtokarev@nvidia.com>
@dmitry-tokarev-nv
dmitry-tokarev-nv enabled auto-merge (squash) October 2, 2026 21:04
@dmitry-tokarev-nv
dmitry-tokarev-nv merged commit 895d78c into main Oct 2, 2026
220 of 223 checks passed
@dmitry-tokarev-nv
dmitry-tokarev-nv deleted the dtokarev/pynvvideocodec-2.2.3 branch October 2, 2026 23:28
nv-nmailhot pushed a commit that referenced this pull request Oct 5, 2026
… move the in-tree FFmpeg to 9.0.1 (DYN-4044) (#14925) (#15669)

Signed-off-by: Dmitry Tokarev <dtokarev@nvidia.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend::trtllm Relates to the trtllm backend backend::vllm Relates to the vllm backend build container multimodal size/XXL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants