Repository navigation
sensing: Argus capture backend and encoder DMA-BUF input (draft; unbuilt, no Argus access) - #1429
gareth-morgan-nv wants to merge 8 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
|
6d1bf9a to
cf8dba3
Compare
71b6bbf to
2c38aba
Compare
cf8dba3 to
0288b95
Compare
2c38aba to
27dad9c
Compare
0288b95 to
5341f7a
Compare
27dad9c to
2602fd7
Compare
5341f7a to
ab263f4
Compare
2602fd7 to
74384ee
Compare
ab263f4 to
b9c18a5
Compare
74384ee to
32272f2
Compare
The plan for an Argus capture backend and DMA-BUF encoder input behind the capture interface, with its open precondition (per-frame correlation) and the fact that no Argus SDK or camera has been available to test any of it. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
…t (not built or run) ArgusCapture streams each camera into NvBufSurface NV12 DMA-BUFs through an Argus buffer stream; frames carry a lease that returns the buffer to the camera once the hardware encoder is finished with it. JetsonEncoder gains a DMA-BUF input mode (submit_dmabuf), LiveKitVideoOutput a leased-frame path, and the sender selects SIPL or Argus at startup. Both backends are optional builds (SENSING_WITH_SIPL, SENSING_WITH_ARGUS, the latter off by default). No Argus SDK, Argus camera or Jetson was available: the Argus and encoder changes are unbuilt and unrun. The rest is g++ syntax-checked and formatted. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
…ord the NV12M plane contract as unverified Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
…d modules conditional SENSING_WITH_ARGUS now finds libnvargus_socketclient (the nvargus-daemon client, as the repository's Argus example does); the single-process libnvargus is an explicit SENSING_ARGUS_SINGLE_PROCESS option. The plan keeps build-time backend options as the path and adds a shared-module boundary only if a target shows the executable cannot start with the unused SDK linked. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
…e acquire loop start() stops the sessions already repeating if a later camera fails to start, and acquire_latest() takes at most one pool's worth of buffers per call and no longer leaks the frame it holds when an acquire fails. Not built or run (no Argus SDK). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
The extra message() and endif() made CMake fail to configure with 'Flow control statements are not properly nested'. Reported by the Orin build. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
The plan lives in the git-ignored agentPlans/ folder instead. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
…e only the selected backend The initialised EGL display is owned by a member, so a constructor that throws after eglInitialize no longer leaks it. An explicit --camera-backend initialises only that SDK instead of probing both. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
32272f2 to
e28d3bc
Compare
b9c18a5 to
69d7c23
Compare
Built and run on an AGX Orin — Argus backend works with 3 GMSL camerasTested the Argus path on real hardware, since the PR says it had never been built or run:
Result: all three cameras capture at the sensor's 60 fps and publish 2560×1984 H.264 at the 30 fps cap (~24–30 fps observed), DMA-BUF straight into NVENC. Fixes needed to get thereBranch
Observations not fixed in that branch
Full build/run log excerpts available on request. |
NVdejung
left a comment
There was a problem hiding this comment.
🤖 Automated first-pass review (Claude, for Dejun) — I'll follow up personally where needed.
Summary — This PR adds an Argus backend that captures into NV12 block-linear NvBufSurfaces and passes their fds straight to the Jetson encoder. It also adds a capture factory, independent SIPL/Argus build switches, and a leased DMA-BUF path through the LiveKit output. The code follows the repository's argus_sender closely and the lease ownership reads correctly. However, the Argus build cannot link as written, and a fresh configure cannot find the Argus headers. I reviewed it as a draft, though GitHub doesn't mark it as one. Paths are under src/plugins/sensing/ unless given in full.
Must fix
core/argus_capture.cpp:219—NvBufSurf::NvAllocateis defined in the Multimedia API sampleNvBufSurface.cpp.JETSON_MMAPI_SOURCES(CMakeLists.txt:144) does not compile that file, socamera_plugin_sensingwill not link with-DSENSING_WITH_ARGUS=ON.argus_senderbuilds the same seven MMAPI classes and carriesexamples/camera_streamer/argus_sender/src/NvBufSurfaceLite.cpp:31for exactly this reason; the sample file would also pull in libnvbufsurftransform. Fix: callNvBufSurfaceAllocate()directly (libnvbufsurface is already linked) and take the fd fromsurfaceList[0].bufferDesc.CMakeLists.txt:109— the Argusfind_pathuses${JETSON_MMAPI_DIR}/argus/includeas its hint, butJETSON_MMAPI_DIRis only searched for later, atCMakeLists.txt:123. The README documents-DJETSON_MMAPI_DIRas optional. On a fresh configure without it, the hint becomes/argus/includeand configure stops with "needs the Argus headers". Fix: move the Argus block below the Multimedia API section.
Suggestions
core/argus_capture.cpp:359— Argus stampsmonotonic_nswhen the poll loop acquires the buffer. SIPL stamps it on its own frame thread when the frame is ready (core/sipl_camera.cpp:923). The output paces on that stamp with 10% slack (core/livekit_video_output.cpp:169, "so poll jitter does not drop a frame that is due"), and the loop polls every half frame. When capture and publication rates are equal (30/30 by default), this will likely pace out a frame each time the poll phase wraps. Check thepaced outcount on the target. Stamping on arrival keeps the pacing's assumption true, e.g. a per-camera thread on a blockingacquireBuffer.core/livekit_video_output.cpp:66— the check that refuses DMA-BUF input withscale_div> 1 runs afterLiveKitSession::connectandpublish_video(core/sensing_camera.cpp:77). So Argus with--livekit-scale-div=2joins the room before it fails. Also check it in the no-network loop atcore/sensing_camera.cpp:55.core/argus_capture.hpp:28— nothing tiesbuffer_count = 10tokOutputBuffers = 6(core/jetson_encoder.cpp:28). A lease is released only when its index is reused (core/jetson_encoder.cpp:325), so the encoder pins 6 buffers. Addingpending_dma, the worker's frame and a fresh acquire, the peak is 9 of 10. The comment counts only the encoder's 6 and the fresh acquire. Derive one constant from the other, orstatic_assertthe margin.core/argus_capture.cpp:299—query()throws if any device lacks mode K (core/argus_capture.cpp:152). One camera with fewer modes therefore fails the probe and--list-sensorsfor every camera; skip or flag such devices instead. TheISensorModecasts here and at line 119 are dereferenced without a null check.- Comments that are still SIPL-only:
core/sensing_types.hpp:21,app/main.cpp:82,app/main.cpp:130,core/sensing_camera.hpp:26andapp/main.cpp:431. The last says capture is "unaffected" by the poll loop, but with Argus the poll loop is what returns buffers to the camera. The class comment atcore/livekit_video_output.hpp:51still describes only the snapshot path. app/main.cpp:414—DISPLAYis unset after the probes, but its comment says it must be gone before the first EGL call. The Argus probe (app/main.cpp:336) now loads libargus before that point. This is likely harmless with the daemon client. ForSENSING_ARGUS_SINGLE_PROCESS=ON, moving theunsetenvabove the probes removes the doubt.- Nits:
- No tests were added. The Argus requested-id matching at
app/main.cpp:336could sit besideselect_camera_backendand be covered intest_camera_backend.cpp. - The description still lists
ARGUS_PLAN.md, which 1b2f214 removed. - The long line at
app/main.cpp:134appears to have made clang-format re-indent all ofprint_usage. - If Argus returns no metadata,
sequencestays 0, andcore/sensing_camera.cpp:119treats every later frame as a re-read and drops it. A counter fallback avoids this.
- No tests were added. The Argus requested-id matching at
Conflicts
- With
gmorgan/RearchBackend: the branch is up to date (0 commits behind) and a local 3-way merge finds no conflicting files, so no rebase is needed. It lands after#1428and#1426. #1437(gareth-morgan-nv, stacked on this branch) — 10 shared files includingcore/argus_capture.cpp, with no textual conflict. The semantic risk:#1437aligns camera time. With Argus,monotonic_nsis the poll time, and thecapture_tscuser data carries Argus nanoseconds instead of SIPL's counter, with nothing on the wire saying which.#1437needs a rebase after any stamping change here.#1443(gareth-morgan-nv, stacked above#1437) — 9 shared files, with no textual conflict; it needs a rebase after changes here.#1440(gareth-morgan-nv) touches only the README and is low risk.#998and#1078(jiwenc-nv, other bases, conflicts not computed) — both shareCMakeLists.txt;#998also sharescore/CMakeLists.txt,app/main.cpp,core/sensing_camera.*andcore/jetson_encoder.*. This stack re-indents the SIPL CMake block, makes the SIPL sources conditional and changes theSensingCameraconstructor, so textual conflicts are likely. Whichever lands second should rebase.
Change-by-change (16 files)
I read every hunk. Nothing was built or run for this review.
| File / area | What changed | Makes sense? |
|---|---|---|
CMakeLists.txt |
SIPL lookup behind SENSING_WITH_SIPL (default ON); optional Argus lookup that links the daemon client by default |
JETSON_MMAPI_DIR is known (must fix 2) |
core/CMakeLists.txt |
SIPL sources and yuv_to_rgba.cu made conditional; capture_factory.cpp and argus_capture.cpp added; SENSING_HAS_SIPL / SENSING_HAS_ARGUS defines |
NvBufSurf::NvAllocate (must fix 1) |
core/argus_capture.hpp, .cpp |
New backend: one session per camera; buffer stream into 10 NV12 block-linear surfaces; repeating request pinned to the mode's minimum frame duration; acquire keeps the newest frame; the lease keeps the backend alive | ✅ mirrors argus_sender; |
core/camera_capture.hpp |
PixelKind and CameraInfo::pixels added; the DMA-BUF contract no longer describes pitches and offsets |
✅ as described |
core/capture_factory.hpp, .cpp |
make_capture() added; SIPL config mapping moved here from sensing_camera.cpp |
✅ |
core/jetson_encoder.hpp, .cpp |
EncoderConfig::input; DMA-BUF output plane with a wrapper and a lease per index; shared take_input(); submit_dmabuf(); drain releases a lease and sends an fd -1 EOS |
✅ matches argus_sender; |
core/livekit_video_output.hpp, .cpp |
input added to the config; submit() takes a mutable frame; one-frame pending_dma mailbox, with the replaced frame released outside the lock; encode_dma(); track_in_flight() factored out; no snapshot slots for DMA |
✅; |
core/sensing_camera.hpp, .cpp |
Takes an opened CameraCapture; passes pixels to the output; error message reworded |
✅; |
core/sensing_types.hpp |
argus_sensor_mode added |
✅; sensor_id comment stale |
app/main.cpp |
--argus-mode added; probes behind build flags, and an explicit backend skips the other backend's probe; --list-sensors lists the chosen backend's cameras; uses make_capture(); usage text reflowed |
✅; DISPLAY unset too late |
README.md |
Documents backend selection, the build switches and the Argus caveats | ✅ |
Reviewed at e28d3bc · 16 files · 2026-11-06 18:47 PST
Description
Draft. Stacked on #1428 (base is
gmorgan/RearchBackend), which is stacked on #1426. This branch holds the Argus capture backend and the encoder's DMA-BUF input. There is no Argus SDK, Argus camera or Jetson available to the author, so none of the Argus code has been built or run, and the encoder and output changes are unbuilt too. It is written against the Jetson Multimedia API and the repository's existing Argus example as the reference.core/argus_capture.*, built only with-DSENSING_WITH_ARGUS=ON, off by default): one capture session per camera, an Argus buffer stream into a pool of NvBufSurface NV12 DMA-BUFs, a repeating request pinned to the sensor mode's frame rate, and a non-blocking acquire that keeps the newest frame and returns older ones to the camera. Frames carry a lease that returns the buffer to the camera, and keeps the Argus objects alive until it does. The sensor timestamp and capture id are passed through unscaled.JetsonEncoder::submit_dmabuf): the output plane switches to V4L2 DMA-BUF memory for an encoder created for it (one input mode per encoder). The capture buffer's lease is held until the plane returns that input buffer, and released at once for a dropped frame. No CPU or CUDA copy.--livekit-scale-divabove 1.--camera-backend=auto|sipl|argusprobes both backends;SENSING_WITH_SIPL=OFFbuilds an Argus-only rig that needs no SIPL headers or configuration;--argus-mode=Kpicks the sensor mode.src/plugins/sensing/ARGUS_PLAN.md, with its status and open precondition.Not done
Type of change
Testing
clang-format-14clean; the capture factory, output, sensing camera andmain.cpppass a g++ syntax check (with and withoutSENSING_HAS_SIPL).argus_capture.cppandjetson_encoder.cppcould not be syntax-checked here (they need the Argus and Multimedia API headers).Checklist
SKIP=check-copyright-year pre-commit run --all-filesgit commit -s) per the DCO🤖 Generated with Claude Code