Skip to content

decoder: implement blocking wait in nvmpi_decoder_get_frame() for AV_CODEC_FLAG_LOW_DELAY #10

Description

@gjrtimmer

Summary

nvmpi_decoder_get_frame(nvmpictx*, nvFrame*, bool wait) ignores its wait parameter — the body starts with (void)wait; and always returns -1 immediately when no frame is ready. The FFmpeg layer already passes avctx->flags & AV_CODEC_FLAG_LOW_DELAY, so users running ffmpeg -flags low_delay silently do not get low-delay semantics. Implement true blocking wait with a condition variable. (Formerly tracked in TODO.md, now covered by per-feature issues #30–#33; cluster FT-3 in https://github.com/gjrtimmer/jetson-ffmpeg/wiki/Fork-Analysis-Results.)

Prior art (fork network)

  • @cgutman's condition-variable blocking wait: commit caac9899 on the cybernhl low_latency branch (= moonlight_fixes + 1) — true blocking in nvmpi_decoder_get_frame when AV_CODEC_FLAG_LOW_DELAY is set. Eliminates polling; written for Moonlight game streaming.

Upstream references

Intended semantics

  • wait == false — unchanged: non-blocking, -1 when no filled frame is queued.
  • wait == true — block until: (1) a filled frame is available → 0; (2) EOS/decoder error is signalled → -1 (no frame will ever come); (3) bounded timeout expires → -1.

Implementation plan

  1. include/NVMPI_bufPool.hpp — the pool is deliberately non-blocking; add an optional blocking dequeue so encoder paths are unaffected:
    • std::condition_variable cv_filledBuf next to m_filledBuf (switch that path to std::unique_lock);
    • qFilledBuf() gains cv_filledBuf.notify_one() after the push;
    • new T dqFilledBuf(std::chrono::milliseconds timeout) waiting on a predicate (!filledBuf.empty() || shutdown);
    • new shutdown() setting a flag under the filled mutex + notify_all(), so blocked consumers wake when no producer will ever queue again.
  2. src/nvmpi_dec.cpp — wire shutdown signalling to every place the producer stops, otherwise a blocked waiter deadlocks:
    • dec_capture_loop_fcn() calls framePool->shutdown() on exit (EOS buffer, V4L2_EVENT_RESOLUTION_CHANGE error paths, dec->isInError());
    • audit all ctx->eos = true sites; make ctx->eos std::atomic<bool> (currently a plain bool written from two threads).
  3. Timeout — cv.wait_for() with a bounded timeout (100–500 ms, possibly retried) instead of an indefinite wait: FFmpeg's send/receive model tolerates spurious "no frame" returns, but an indefinite block on a stalled V4L2 pipeline would hang the whole ffmpeg process.
  4. include/nvmpi.h — update the doc comment ("the 'wait' flag is currently not honoured"). Signature unchanged → no ABI/API break for FFmpeg builds linked against libnvmpi.so.1.
  5. ffmpeg/dev/common/libavcodec/nvmpi_dec.c — already passes the flag; re-check res < 0 handling on EOS with wait=true. If anything changes, regenerate patches via ffmpeg/dev/update_patch.sh.

Risks

  • Behavior change for non-FFmpeg libnvmpi consumers passing wait=true (currently a no-op): release-notes entry + minor version bump.
  • Deadlock is the main failure mode: every capture-loop exit path must signal the pool.
  • Encoder symmetry: nvmpi_encoder_get_packet() has the same pattern; pool changes are generic so the encoder can adopt later (out of scope here).

Testing

  • JETSON_VARIANT=<variant> ./test/hw-test.sh baseline.
  • Low-delay run (ffmpeg -flags low_delay -c:v h264_nvmpi ...): frames returned per-packet rather than after pipeline fill.
  • Stream-end with wait=true: process exits, no hang.
  • ./test/smoke-all.sh (libnvmpi change → all seven versions).

Acceptance criteria

  • Blocking dequeue + shutdown signalling in NVMPI_bufPool.hpp
  • All capture-loop exit paths signal the pool; ctx->eos atomic
  • Low-delay per-packet latency demonstrated; EOS exits cleanly
  • ./test/smoke-all.sh green

Related

Credit: @cgutman (prior art).


Found during the 237-fork network sweep of the jetson-ffmpeg ecosystem (tracking · raw findings).

Activity

  1. gjrtimmer commented on Jun 15, 2026

    @gjrtimmer
    OwnerAuthor

    Starting work on #10 with a two-branch approach.

    Branch 1: refactor/10-decoder-split — mechanical file split of nvmpi_dec.cpp (~1040 lines) into modular files by concern. No behavioral changes. Establishes file structure for the feature.

    Branch 2: feat/10-decoder-blocking-wait — implements blocking wait with condition variables, tiered timeout, configurable AVOption. TDD with unit tests + hw test suites.

    Design spec: .work/2026-06-15-issue-10-decoder-blocking-wait-design.md
    Plan: .work/10-decoder-blocking-wait-plan.md

  2. gjrtimmer commented on Jun 16, 2026

    @gjrtimmer
    OwnerAuthor

    Progress update — Branch 1 (refactor/10-decoder-split)

    All 9/9 tasks complete:

    • Task 1: Created branch, posted plan comment
    • Task 2: Extracted nvmpictx struct to src/nvmpi_dec_internal.h (14ac9d4)
    • Task 3: Extract capture loop to nvmpi_dec_capture.cpp (2c35d23, acf52c7)
    • Task 4: Extract V4L2 plane setup to nvmpi_dec_planes.cpp (a7faf6e)
    • Task 5: Rename to nvmpi_dec_api.cpp + update CMakeLists (ca98f3d)
    • Task 6: Write docs/ARCHITECTURE.md (9569332)
    • Task 7: Update docs/DEVELOPMENT.md (b871361)
    • Task 8: Build verification — stubs build + try_build 7/7 green
    • Task 9: Push, MR !14 created, auto-merge set
  3. gjrtimmer commented on Jun 16, 2026

    @gjrtimmer
    OwnerAuthor

    Progress update: Branch 1 (refactor/10-decoder-split)

    All implementation tasks complete. Cross-version build verification running.

    Commits

    Commit Description
    14ac9d4 Extract nvmpictx struct to nvmpi_dec_internal.h
    2c35d23 Extract capture loop to nvmpi_dec_capture.cpp
    acf52c7 Fix copyNvBufToFrame forward declaration signature
    a7faf6e Extract V4L2 plane setup to nvmpi_dec_planes.cpp
    ca98f3d Rename nvmpi_dec.cpp → nvmpi_dec_api.cpp + update refs
    9569332 Add docs/ARCHITECTURE.md with modular split convention
    b871361 Update docs/DEVELOPMENT.md with file structure
    8e04d48 Status tracking file
    e64ef92 Design spec + implementation plan

    File layout after split

    File Lines Concern
    nvmpi_dec_api.cpp 492 Public API (create, put_packet, get_frame, flush, close)
    nvmpi_dec_capture.cpp 248 Capture thread loop, resolution-change handler
    nvmpi_dec_planes.cpp 188 V4L2 CAPTURE-plane alloc/teardown, color format selection
    nvmpi_dec_internal.h 109 Context struct, macros, forward declarations

    Pure mechanical move — no behavioral changes, no new code. Stubs build passes. Cross-version try_build.sh running now; will push + MR once verified.

    Next

    After this MR merges → Branch 2 (feat/10-decoder-blocking-wait) with the actual blocking wait implementation.

  4. gjrtimmer commented on Jun 16, 2026

    @gjrtimmer
    OwnerAuthor

    Progress update — Branch 1 (refactor/10-decoder-split) ✓ COMPLETE

    All 9 tasks finished. MR !14 merged.

    Final commits

    Commit Description
    14ac9d4 Extract nvmpictx struct to nvmpi_dec_internal.h
    2c35d23 Extract capture loop to nvmpi_dec_capture.cpp
    acf52c7 Fix copyNvBufToFrame forward declaration signature
    a7faf6e Extract V4L2 plane setup to nvmpi_dec_planes.cpp
    ca98f3d Rename nvmpi_dec.cpp → nvmpi_dec_api.cpp + update refs
    9569332 Add docs/ARCHITECTURE.md with modular split convention
    b871361 Update docs/DEVELOPMENT.md with file structure
    8e04d48 Status tracking file
    e64ef92 Design spec + implementation plan

    Verification

    • ./scripts/build.sh --stubs --clean — pass
    • ./ffmpeg/dev/try_build.sh — 7/7 FFmpeg versions build clean
    • Pure mechanical move — no behavioral changes, no new code
    • GitLab MR pipeline (52141) — passed (15/15 jobs, 7/7 hw tests)
    • MR !14 merged to main
  5. gjrtimmer commented on Jun 16, 2026

    @gjrtimmer
    OwnerAuthor

    Progress update — Branch 2 (feat/10-decoder-blocking-wait)

    Implementation complete. MR !15 created, auto-merge set, pipeline running.

    Commits

    Commit Description
    2f864bb Unit tests for NVMPI_bufPool blocking dequeue (TDD red phase)
    a7078c8 HW test suites: blocking-wait, lifecycle, perf metrics
    50990b8 NVMPI_bufPool: condition-variable blocking dequeue, shutdown, reset
    b5d7fde Atomic ctx->eos, wait_timeout_ms field in decoder context
    5bf3bbe Wire shutdown() to all capture-loop exit paths, atomic eos usage
    d96c774 Blocking wait in nvmpi_decoder_get_frame(), flush reset, close shutdown
    24bd677 wait_timeout AVOption in FFmpeg decoder wrapper (50-5000ms, default 500)
    875b41d Regenerate patches for all 7 FFmpeg versions
    cecb7bf THREAD_SAFETY.md, API_REFERENCE.md, updated BUILD/README/TODO
    579bdf3 Fix: suppress NVIDIA stdout in test latency measurement
    3877dfa Fix: replace flawed latency comparison with completion check

    What changed

    • NVMPI_bufPool: Added std::condition_variable, blocking dqFilledBuf(timeout) with 100ms granularity ticks, shutdown() + reset() for lifecycle management
    • nvmpictx::eos: Changed from bool to std::atomic<bool> — fixes data race between user thread and capture thread
    • Capture loop: All exit paths (EOS, error, external signal) now call framePool->shutdown() to unblock waiting consumers
    • get_frame(wait=true): Blocks up to wait_timeout_ms (default 500ms) via CV; returns -1 on timeout/shutdown
    • flush(): Calls framePool->reset() to clear shutdown flag before restarting capture thread
    • close(): Calls framePool->shutdown() before STREAMOFF to unblock blocked consumers before thread join
    • FFmpeg: wait_timeout AVOption (range 50-5000ms, 0=default 500ms) wired through nvDecParam

    Tests

    • 8 unit tests (test/unit/test_bufpool.cpp): non-blocking, blocking, timeout, shutdown, reset, concurrent stress
    • 3 HW suites: hw-decoder-blocking-wait (6 tests), hw-decoder-lifecycle (3 tests), hw-perf-blocking-wait (metrics)

    Docs

    • docs/THREAD_SAFETY.md: full two-thread model, synchronization primitives, deadlock prevention
    • docs/API_REFERENCE.md: complete libnvmpi API reference
    • Updated: docs/BUILD.md, test/README.md, TODO.md

    Verification

    • Unit tests: 8/8 pass (ctest --output-on-failure)
    • ./ffmpeg/dev/try_build.sh — 7/7 FFmpeg versions build clean
    • hw-decoder-blocking-wait + hw-perf-blocking-wait — 7/7 versions PASS
    • CI pipeline (52158) — running

    MR

    • GitLab MR !15 — auto-merge set, awaiting pipeline 52158
  6. added a commit that references this issue on Jun 16, 2026
    0ab653c
  7. gjrtimmer commented on Jun 16, 2026

    @gjrtimmer
    OwnerAuthor

    Resolved — blocking wait in nvmpi_decoder_get_frame()

    What was done

    Implemented true condition-variable blocking wait when wait=true (activated by FFmpeg's -flags low_delay). Previously the wait parameter was silently ignored.

    Core changes (libnvmpi):

    • NVMPI_bufPool: CV-based blocking dqFilledBuf(timeout) with 100ms granularity, shutdown() + reset() for lifecycle
    • ctx->eos: bool → std::atomic<bool> (fixes data race between user/capture threads)
    • All capture-loop exit paths call framePool->shutdown() to unblock waiting consumers
    • get_frame(wait=true) blocks up to configurable wait_timeout_ms (default 500ms)
    • flush() resets shutdown flag; close() calls shutdown before STREAMOFF + thread join

    FFmpeg integration:

    • wait_timeout AVOption (50–5000ms, 0 = default 500ms)
    • Patches regenerated for all 7 versions (4.2, 4.4, 6.0, 6.1, 7.0, 7.1, 8.0)

    Key commits

    • 50990b8 — NVMPI_bufPool CV blocking dequeue
    • b5d7fde — atomic eos + wait_timeout_ms
    • 5bf3bbe — shutdown wiring to capture loop
    • d96c774 — blocking wait in get_frame
    • 24bd677 — wait_timeout AVOption

    Validation

    • Unit tests: 8/8 pass (NVMPI_bufPool blocking, timeout, shutdown, concurrent stress)
    • HW tests: hw-decoder-blocking-wait (6 tests), hw-decoder-lifecycle (3 tests), hw-perf-blocking-wait (metrics) — all 7 FFmpeg versions PASS
    • CI pipeline 52160: all jobs green
    • MR !15 merged to main

    Docs added

    • docs/THREAD_SAFETY.md — full two-thread synchronization model
    • docs/API_REFERENCE.md — complete libnvmpi API reference
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions