[gareth-morgan-nv]

Summary

The copy between the receiver's CUDA IPC buffer and the encoder input becomes a series of GPU steps (GpuStep) that run in order on one CUDA stream, with no host copy. None run by default: with no flag the picture is only copied or shrunk, as before.

• --gpu-step NAME[=ARGS] adds a step, repeatable, run in the order given at the source size; a stream's :DIV downscale (now itself a step) runs after them.
• sine-warp is an example step only. It is not calibrated rectification; it shows the shape of a step.
• Adding a step is one class plus one line in make_gpu_step(). GPU_PROCESSING.md describes the system, the step contract and how lens rectification would be written as a step (a remap table computed once from the calibration and sampled per frame for luma and chroma).
  Stacked on the capture-time PR (it touches the same bridge files).

Testing

ctest 8/8, including a new gpu_pipeline test (no steps copies, identity warp equals a copy, a visible change otherwise, steps run in series, bad names, parameters and sizes are refused). A live run through the mock, receiver and bridge with --gpu-step sine-warp=14,90 ran at 30 fps with 0 slot overruns and the published picture shows the ripple. The default path (no flag) is covered by the unit test; I did not repeat the live run without the flag on this head. Nothing here has run on real cameras.

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

• Source mode: Auto
• Repository: NVIDIA/IsaacCapture
• Branch: gmorgan/CameraStreamingTiming
• File: .coderabbit.yaml
• Commit: 57c7ab4
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Enterprise
• Run ID: 5203db5d-2249-497f-8967-502990073519

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• Trigger review

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

[gareth-morgan-nv]

@NVshawnhwang this is where rectification goes. Between the receiver's CUDA IPC buffer and the encoder input, each picture now passes through a series of GPU steps (gpu_ops/inc/gpu_ops/gpu_step.hpp), none enabled by default. sine-warp is an example step only, not rectification. A rectification step is the same shape: compute the remap table from the camera calibration once in prepare(), upload it, and sample through it per frame for luma and chroma. SineWarpStep in gpu_ops/inc/gpu_ops/sine_warp.hpp is the template and its comment walks through it; GPU_PROCESSING.md covers how to add a step.

🤖 Agent-generated comment (Claude Code).

[NVdejung]

🤖 Automated first-pass review (Claude, for Dejun) — I'll follow up personally where needed.

Summary — This PR turns the copy from the receiver's IPC slot to the encoder input into an ordered GpuPipeline of GpuSteps on the worker's CUDA stream. It makes the :DIV box downscale the last step and adds an opt-in --gpu-step flag with a sine-warp example, a gpu_pipeline test and a design doc. I read every hunk plus the worker, encoder and CUDA-context code around them. With no flag the bridge issues the same copy and shrink calls as before. prepare() allocates and the worker thread launches in the same primary context, and the warp kernels stay in bounds. No blockers. The main gaps are a series test that cannot see a broken step-to-step hand-off, and a start-time size check that was lost.

Must fix
None found. Internal names: the scan of the added lines, title, description and commits found nothing, and the new docs name no people, stations or internal resources.

Suggestions

• tests/gpu_pipeline/test_gpu_pipeline.cpp:120 — The series check chains sine-warp=0 with the downscale. The first step is an identity, so the test still passes if the downscale reads the source instead of m_scratch[0], or if the first step never runs. The hand-off that --gpu-step plus :DIV relies on is therefore untested.
  • The "two warps differ from one" check named in the comment does not exist.
  • in_series (L125) is computed and discarded, and shrunk holds 128x96.
  • Fix: compare sine-warp=8,32 followed by the downscale against the downscale of a separate sine-warp=8,32 output.
  • Give source and destination padded pitches as test_box_downscale.cpp does: every buffer here has pitch == width, so a width-for-pitch slip in a step would still pass.
  • The default path was rewritten and the description says the no-flag live run was not repeated on this head. One short run with and without :DIV would close that.
• gpu_ops/gpu_pipeline.cpp:97 — Silent behaviour change. The old constructor ran box_downscale_supported(w, h, 1) on every stream, which refused an odd or zero source at start. Now, with no steps and no :DIV, nothing checks the source: the track is published and the worker then dies in NvencEncoder ("NV12 needs non-zero even dimensions"). Check for a non-zero even source at the top of GpuPipeline::prepare().
• gpu_ops/gpu_pipeline.cpp:100 — prepare() clears m_inputs but not m_scratch. The contract ("once per picture size") invites a second call, which would leak the first buffers while enqueue() keeps using them, even if they are now too small. Free and clear m_scratch first, or document prepare() as one-shot.
• bridge/main.cpp:125 — A bad --gpu-step is only parsed in the StreamWorker constructor, after the CUDA context and the LiveKit connection exist, and it exits 1 without usage. Call make_gpu_step() once while parsing and return 2 on std::invalid_argument, like --timestamp-basis.
• GPU_PROCESSING.md:165 — The doc says to measure step cost with encode in the stats line, but encode times only encoder.encode() (bridge/stream_worker.cpp:250), after the stream sync. Step time never shows there. Only capture-to-ready includes it, and that needs a realtime sender clock. Fix the sentence, or add an enqueue-to-sync stat.
• README.md:39 — The rectification walk-through is now written three times: in the README, in GPU_PROCESSING.md:133 and in the SineWarpStep comment (sine_warp.hpp:42). The step table appears twice. Keep one copy in GPU_PROCESSING.md and link to it. The README overview (L15, L18) and stream_worker.hpp:75 still say the picture is only copied or box-averaged.
• bridge/stream_worker.cpp:177 — A failing step still reports "GPU copy from the IPC slot failed" without saying which step. Have GpuPipeline::enqueue() name the failing step, which will help whoever debugs a first rectification step.

Conflicts

• With gmorgan/CameraStreamingTiming (the head of #1479): 0 commits behind and the local 3-way merge is clean, so no rebase is needed.
• With open PRs: this sits on top of a stack (#1410 → #1476 → … → #1478 → #1479 → this). It shares 5 files with #1478 and 4 with #1479, all by the same author, with no textual conflicts. Semantic risk: this PR deletes the copy/shrink block and the :DIV start check in stream_worker.cpp that those PRs add, and moves them into copy_nv12 and BoxDownscaleStep. Any review change to that block in #1478 or #1479 has to be ported to gpu_pipeline.cpp, and this PR then rebased. Merge it after #1479. #1410 and #1476 share no files.

Change-by-change (12 files, paths under examples/camera_livekit_bridge/)

File / area	What changed	Makes sense?
GPU_PROCESSING.md (new)	Where steps sit, CLI use, step contract, adding a step, rectification walk-through, what the bridge does around steps	⚠️ encode stat claim is wrong; overlaps README and header
README.md	New "GPU steps" section: step table and a summary of how to add a step	⚠️ duplicates GPU_PROCESSING.md; overview lines not updated
bridge/main.cpp	Repeatable --gpu-step NAME[=ARGS] into WorkerConfig::gpu_steps; usage text	✅ / ⚠️ validated only after connecting
bridge/stream_worker.hpp	WorkerConfig::gpu_steps with doc	✅
bridge/stream_worker.cpp	Per-stream GpuPipeline (steps, then box downscale when DIV > 1); size check via prepare(); encoder size from output_size(); per-frame copy lambda replaced by pipeline.enqueue()	✅ same calls on the default path / ⚠️ odd-source start check lost
gpu_ops/CMakeLists.txt	Adds sine_warp.cu, gpu_pipeline.cpp	✅
gpu_ops/inc/gpu_ops/gpu_step.hpp (new)	ImageSize, GpuStep, GpuPipeline, the two factories	✅ / ⚠️ includes box_downscale.hpp only for the plane types; a small nv12_planes.hpp would be cleaner
gpu_ops/gpu_pipeline.cpp (new)	Plain copy, BoxDownscaleStep, pipeline prepare (sizes, scratch) and enqueue, make_gpu_step with the sine-warp parser	✅ / ⚠️ no source check with 0 steps; prepare() not re-entrant
gpu_ops/inc/gpu_ops/sine_warp.hpp (new)	SineWarpParams, kernel API, SineWarpStep, rectification how-to comment	✅ clearly labelled as an example
gpu_ops/sine_warp.cu (new)	Bilinear luma/chroma ripple, NaN-safe inside test, video-range fill, parameter checks	✅ bounds, rounding and chroma siting check out; zero amplitude is an exact identity
tests/gpu_pipeline/CMakeLists.txt (new)	gpu_pipeline ctest, skip code 77	✅ picked up by the tests/ glob (8 tests, matching the description)
tests/gpu_pipeline/test_gpu_pipeline.cpp (new)	No-step copy, identity, ripple, chain, refusals	⚠️ chain check blind to the hand-off; dead in_series; unpadded pitches

_{Reviewed at e58a463 · 12 files · 2026-11-10 21:50 PST}