cp: Refactor video frame extraction to improve PyNvCodec availability check (1511) into r1.1.0

[thomasdhc]

beep boop [🤖]: Hi @abhinavg4 👋,

we've cherry picked #1511 into  for you! 🚀

Please review and approve this cherry pick by your convenience!

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[thomasdhc]

/ok to test 12456a7

[greptile-apps]

Greptile Summary

This cherry-pick refactors how PyNvCodec availability is detected and surfaced across the video frame extraction pipeline. The _PYNVC_AVAILABLE flag is now set in nvcodec_utils.py (the single source of truth) and imported directly by video_frame_extraction.py, eliminating a duplicate try/except block. Simultaneously, the deprecated nvcv import is replaced with the cvcuda equivalents throughout NvVideoDecoder.generate_decoded_frames, and the test suite is updated to remove all nvcv mocks and correctly handle the two distinct cvcuda.as_tensor call sites via side effects.

Key changes:

• _PYNVC_AVAILABLE moved from a local re-declaration in video_frame_extraction.py to a single authoritative export from nvcodec_utils.py
• nvcv.as_tensor / nvcv.as_image / nvcv.Format.U8 replaced with cvcuda.as_tensor / cvcuda.as_image / cvcuda.Format.U8
• PyNvcFrameExtractor is not None guard removed since PyNvcFrameExtractor is now always a real class (import is unconditional)
• A TODO comment correctly flags that decoded_frame.nvcv_image() (a still-deprecated PyNvVideoCodec API) is still in use
• The hard unconditional import in video_frame_extraction.py introduces slightly tighter coupling compared to the previous isolation guard, though in practice it is safe given nvcodec_utils always exports these names

Confidence Score: 4/5

• This PR is safe to merge with minor caveats — the core logic is correct and well-tested, though a deprecated API call and slightly tighter import coupling remain.
• The refactoring correctly centralizes _PYNVC_AVAILABLE, removes the nvcv dependency, and keeps all fallback paths intact. Tests are comprehensive and updated consistently. Two low-severity issues prevent a perfect score: (1) decoded_frame.nvcv_image() is still used and flagged with a TODO, meaning the deprecation migration is incomplete, and (2) the unconditional import in video_frame_extraction.py is less resilient to unexpected module-level failures in nvcodec_utils compared to the previous guarded import, though this is unlikely to cause problems in practice.
• No files require special attention for blocking issues; nemo_curator/utils/nvcodec_utils.py line 272 has a known TODO for the remaining deprecated API.

Important Files Changed

Filename	Overview
nemo_curator/utils/nvcodec_utils.py	Centralizes _PYNVC_AVAILABLE flag, replaces deprecated nvcv import with cvcuda, and migrates nvcv API calls to cvcuda equivalents; a TODO comment remains about the still-deprecated nvcv_image() method.
nemo_curator/stages/video/clipping/video_frame_extraction.py	Simplifies PyNvCodec availability check by importing _PYNVC_AVAILABLE directly from nvcodec_utils rather than re-implementing the try/except locally; logic is clean and correct.
tests/utils/test_nvcodec_utils.py	Updates all mocks from nvcv to cvcuda and adds as_tensor_side_effect helpers to differentiate the two cvcuda.as_tensor call sites; tests appear complete and correct for the refactored implementation.

Flowchart

flowchart TD
    A[Module Import] --> B{nvcodec_utils try/except}
    B -->|Success: cvcuda + pycuda + PyNvVideoCodec| C[_PYNVC_AVAILABLE = True]
    B -->|ImportError / RuntimeError| D[_PYNVC_AVAILABLE = False\nAll GPU vars = None]
    C --> E[video_frame_extraction.py\nimports _PYNVC_AVAILABLE & PyNvcFrameExtractor]
    D --> E
    E --> F[VideoFrameExtractionStage.setup]
    F --> G{decoder_mode == pynvc?}
    G -->|No| H[No extractor created\nFFmpeg used directly]
    G -->|Yes| I{_PYNVC_AVAILABLE?}
    I -->|True| J[Create PyNvcFrameExtractor\nGPU-accelerated decoding]
    I -->|False| K[pynvc_frame_extractor = None\nFallback to FFmpeg]
    J --> L[VideoFrameExtractionStage.process]
    K --> L
    L --> M{pynvc_frame_extractor is not None?}
    M -->|True| N[PyNvcFrameExtractor call\nNvVideoDecoder via PyNvVideoCodec]
    M -->|False| O[get_frames_from_ffmpeg CPU]
    N --> P{Exception?}
    P -->|Yes| O
    P -->|No| Q[Return GPU tensor]
    N --> R[NvVideoDecoder.generate_decoded_frames]
    R --> S[decoded_frame.nvcv_image - deprecated\ncvcuda.as_image + cvcuda.as_tensor]
    S --> T[cvcuda.reformat_into NCHW→NHWC]
    T --> U[Queue frame tensor]

Loading

_{Last reviewed commit: 12456a7}

[greptile-apps]

_{3 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Deprecated nvcv_image() API still in use

The TODO comment correctly flags that decoded_frame.nvcv_image() is deprecated, but the call remains in production code. This PR migrates nvcv.as_tensor/nvcv.as_image to their cvcuda equivalents, yet the upstream call into PyNvVideoCodec still uses the old nvcv_image() surface. Consider checking the PyNvVideoCodec changelog for the replacement API (e.g. a direct tensor accessor) to complete the migration and avoid relying on a deprecated interface that may be removed in a future SDK release.

[greptile-apps]

Hard import may fail loudly when GPU deps are absent

The old code used a try/except guard so that video_frame_extraction.py could be imported safely on CPU-only hosts. The new unconditional import from nemo_curator.utils.nvcodec_utils import _PYNVC_AVAILABLE, PyNvcFrameExtractor relies on nvcodec_utils itself catching ImportError/RuntimeError internally and always exporting both names, which it does. This is correct, but it represents a tighter coupling: if nvcodec_utils ever raises during module-level initialization for any reason other than those two exceptions (e.g. an OSError or AttributeError from a partial GPU driver install), the import of this file will now propagate the exception rather than gracefully degrading. The previous isolation made that failure mode impossible. Consider whether this tighter coupling is acceptable for all deployment environments.

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}