[Televiz] make RTP opt-in, fix isaacteleop resolution, refresh Televiz docs - #912
[Televiz] make RTP opt-in, fix isaacteleop resolution, refresh Televiz docs#912farbod-nv wants to merge 1 commit into
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe change updates Televiz and camera streaming documentation for CloudXR, RTP, OAK-D stereo capture, source builds, and XR lock modes. The camera setup script adds RTP validation, package-source fallback, source-build support, dependency checks, structured diagnostics, and mode-specific completion instructions. CloudXR websocket dependencies now use direct imports, allowing normal import errors to propagate. Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant camera_viz.sh
participant _install_deps.sh
participant VirtualEnvironment
Operator->>_install_deps.sh: Select RTP, wheel, or source-build setup
_install_deps.sh->>VirtualEnvironment: Install and validate dependencies
Operator->>camera_viz.sh: Start camera or loopback mode
camera_viz.sh->>VirtualEnvironment: Validate RTP dependencies
camera_viz.sh->>VirtualEnvironment: Start the configured streaming path
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/source/getting_started/televiz.rst`:
- Around line 35-42: Update the Televiz pip installation command in the
getting-started instructions to include the --upgrade option alongside --pre,
ensuring an installed compatible final release is replaced by the newest
available pre-release.
In `@examples/camera_viz/scripts/_install_deps.sh`:
- Line 320: Update the dependency installation step around the uv installer
command to download a pinned, versioned installer into a local file, verify its
signature or checksum against a trusted value, and execute it only after
verification succeeds; do not pipe remote curl output directly to sh.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: fb90b750-7699-486e-9fce-1aeacb10755a
📒 Files selected for processing (6)
docs/source/getting_started/televiz.rstdocs/source/references/camera_streaming.rstexamples/camera_viz/README.mdexamples/camera_viz/camera_viz.shexamples/camera_viz/scripts/_install_deps.shsrc/python/isaacteleop/cloudxr/wss.py
| Isaac Teleop publishes a release candidate from every release-branch commit, so new Televiz | ||
| features are generally available in a pre-release well before the next final release. ``pip`` | ||
| ignores pre-releases by default. If something described on this page is missing from your | ||
| install, ask for the newest pre-release instead: | ||
|
|
||
| .. code-block:: bash | ||
|
|
||
| pip install --pre isaacteleop |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target file ---'
cat -n docs/source/getting_started/televiz.rst | sed -n '1,55p'
printf '%s\n' '--- repository references ---'
rg -n --glob '!docs/source/getting_started/televiz.rst' 'pip install( --upgrade)? --pre isaacteleop|isaacteleop|release candidate' .
printf '%s\n' '--- pip version and option help ---'
python3 -m pip --version
python3 -m pip install --help | rg -n -- '--upgrade|--pre'Repository: NVIDIA/IsaacTeleop
Length of output: 50375
🌐 Web query:
site:pip.pypa.io/en/latest/user_guide.html pip install --upgrade installed version --pre
💡 Result:
Use:
python -m pip install --upgrade --pre <package>--upgradeupdates an already installed package.--prepermits pre-release and development versions.- Without
--pre, pip generally selects stable versions only. (pip.pypa.io)
Citations:
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import subprocess, sys
print(subprocess.run([sys.executable, "-m", "pip", "--version"], capture_output=True, text=True).stdout.strip())
print(subprocess.run([sys.executable, "-m", "pip", "install", "--help"], capture_output=True, text=True).stdout)
PYRepository: NVIDIA/IsaacTeleop
Length of output: 158
Use --upgrade with --pre.
If a compatible final release is installed, pip install --pre isaacteleop can leave it unchanged. Use pip install --upgrade --pre isaacteleop.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/source/getting_started/televiz.rst` around lines 35 - 42, Update the
Televiz pip installation command in the getting-started instructions to include
the --upgrade option alongside --pre, ensuring an installed compatible final
release is replaced by the newest available pre-release.
| ;; | ||
| esac | ||
| fi | ||
| curl -LsSf https://astral.sh/uv/install.sh | sh |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- file metadata ---'
file examples/camera_viz/scripts/_install_deps.sh
printf '%s\n' '--- relevant installer context ---'
sed -n '295,330p' examples/camera_viz/scripts/_install_deps.sh
printf '%s\n' '--- script entry points and call sites for the installer block ---'
rg -n -C 3 'uv/install\.sh|uv\b|install_deps' examples/camera_viz/scripts/_install_deps.sh examples/camera_viz -g '*.sh' -g '*.md' -g '*.toml'
printf '%s\n' '--- repository policy headers ---'
sed -n '1,12p' examples/camera_viz/scripts/_install_deps.shRepository: NVIDIA/IsaacTeleop
Length of output: 220
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- relevant installer context ---'
sed -n '295,330p' examples/camera_viz/scripts/_install_deps.sh
printf '%s\n' '--- script header ---'
sed -n '1,12p' examples/camera_viz/scripts/_install_deps.sh
printf '%s\n' '--- installer references ---'
rg -n -C 3 'uv/install\.sh|uv\b|install_deps' examples/camera_viz/scripts/_install_deps.sh examples/camera_viz -g '*.sh' -g '*.md' -g '*.toml'Repository: NVIDIA/IsaacTeleop
Length of output: 37564
Unverified Remote Installer Execution (CWE-494): Download of Code Without Integrity Check
Reachability: External · Exploitability: Difficult
Verify the uv installer before execution.
curl -LsSf https://astral.sh/uv/install.sh | sh executes current remote content with setup-user privileges. If the endpoint or delivery pipeline is compromised, attacker-controlled commands run during setup or Jetson deployment. Download a versioned installer, verify its signature or checksum, then execute the verified local file.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/camera_viz/scripts/_install_deps.sh` at line 320, Update the
dependency installation step around the uv installer command to download a
pinned, versioned installer into a local file, verify its signature or checksum
against a trusted value, and execute it only after verification succeeds; do not
pipe remote curl output directly to sh.
Source: Linters/SAST tools
e12db23 to
5559c33
Compare
Setup changes ------------- * RTP is now opt-in via --with-rtp (was --no-rtp), matching --with-zed. The GStreamer stack, PyGObject source build, and native codec are only needed for split mode, so direct-mode setup no longer pays for them. --sender-only implies the flag, since streaming is the sender's whole job and a bare --sender-only would otherwise install a sender that cannot send. loopback and `source: rtp` configs now preflight the deps and point at the fix rather than dying on an ImportError. * isaacteleop is resolved down a ladder instead of a fixed pin: newest final release satisfying >= 1.4, else newest release candidate, else a source build of the checkout (after asking; --build-from-source skips the prompt). release/X.Y.x branches publish rc1 builds continuously while a final can lag, and PEP 440 keeps pre-releases out of a plain >= 1.4 -- so `isaacteleop>=1.4` was unsatisfiable and setup failed. Availability is probed with `uv pip compile`, which resolves against the configured index without consulting the venv, so a stale install cannot make a tier look satisfiable. The ladder prefers finals with no edit needed once one ships. * Install isaacteleop[cloudxr]. XR is the default display mode and the viewer launches the CloudXR runtime itself, so the extra is not optional here -- without it `run --mode xr` died on missing websockets. * Catch broken installs that import cleanly. An interrupted install leaves a package directory without __init__.py, which Python imports as an empty namespace package: `import cupy` succeeds and every attribute is missing. The smoke check now rejects any module whose __file__ is None, and cupy's repair guard probes an attribute rather than importability -- previously both passed a venv that failed at the first frame with AttributeError. * Rework setup output: a configuration banner up front (mode, venv, python, cuda, extras) then one step per unit of work, errors prefixed "error:" instead of the internal script name, and a closing line that says what to run next instead of "done." * Reconcile with 26cab2d, which added the cloudxr extra concurrently. The ladder carries the extra on every tier via ISAACTELEOP_EXTRAS, so its follow-up `uv pip install isaacteleop[cloudxr]` after a --wheel install is dropped: uv does activate extras on a direct wheel path (verified -- `<path>.whl[cloudxr]` pulls websockets on uv 0.11.8), and the unpinned re-install was a second index round trip. * Offer the OAK-D udev rule. Without it depthai reports "Insufficient permissions to communicate with X_LINK_UNBOOTED device" and the camera is never detected. Prompted like the apt deps, and only when an OAK-D is actually attached and no rule already covers vendor 03e7 -- matched by content, since the rule ships under several names (50-movidius.rules, 80-movidius.rules, the librealsense rules) and a name check would write a duplicate. Declining warns and continues; the venv is still valid. * Warn when the bootstrapped uv is off PATH. setup installs uv to ~/.local/bin and exports PATH for its own process only, so the caller's shell was left without it and the documented `uv pip install` steps died with "uv: command not found". * Drop the websockets ImportError guard in cloudxr/wss.py. Its advice (--find-links=install/wheels) only applied to source builds, and the plain ImportError is more useful. Docs ---- * televiz.rst: lead with what Televiz is (the visualization module) and its target (stereo XR over CloudXR); window/offscreen are noted as development and debugging aids. * camera_streaming.rst: document the resolution ladder, --with-rtp and --build-from-source, and correct the setup description now that the codec build and GStreamer probing are RTP-only. * build_from_source: the uv installer puts uv in ~/.local/bin, which is not on PATH in most shells -- say so, next to the install command. * camera_viz README: document the OAK-D udev rule and the manual commands. Kept out of the published docs -- setup handles it, so the guide has no reason to carry it. * Docs no longer name the minimum isaacteleop version; the specifiers in _install_deps.sh are the single source of truth, and the setup messages interpolate them instead of repeating the number. * camera_viz README: refresh for 1.4 -- OAK-D stereo is wired (mono/stereo/stereo_rgb), add the gimbal lock mode, list the realsense and synthetic_xr_3up configs, and describe the new setup flags. Signed-off-by: Farbod Motlagh <fmotlagh@nvidia.com>
5559c33 to
6d63fba
Compare
Fixes camera_viz setup on a clean machine, and refreshes the Televiz / camera_streaming docs.
Setup
RTP is opt-in via
--with-rtp(was--no-rtp), matching--with-zed. The GStreamer stack, PyGObject source build, and native codec are split-mode-only, sodirect-mode setup no longer pays for them.
--sender-onlyimplies the flag — streaming is the sender's whole job, and a bare--sender-onlywould otherwise install asender that cannot send.
loopbackandsource: rtpconfigs preflight the deps and point at the fix instead of dying on anImportError.isaacteleopis resolved down a ladder instead of a fixed pin: newest final release satisfying>= 1.4, else newest release candidate, else a source build of thecheckout (after asking;
--build-from-sourceskips the prompt).release/X.Y.xbranches publish rc builds continuously while a final can lag, and PEP 440 keepspre-releases out of a plain
>= 1.4— so the oldisaacteleop>=1.4pin was unsatisfiable and setup failed outright. Availability is probed withuv pip compile, whichresolves against the configured index without consulting the venv, so a stale install can't make a tier look satisfiable. Finals win automatically once one ships, with no
edit.
Install
isaacteleop[cloudxr]. XR is the default display mode and the viewer launches the CloudXR runtime itself, so the extra isn't optional here — without itrun --mode xrdied on missingwebsockets.Catch broken installs that import cleanly. An interrupted install leaves a package directory without
__init__.py, which Python imports as an empty namespacepackage:
import cupysucceeds and every attribute is missing. The smoke check now rejects any module whose__file__isNone, and cupy's repair guard probes anattribute rather than importability. Previously both passed a venv that failed at the first frame with
AttributeError.Reworked setup output: a config banner up front (mode, venv, python, cuda, extras), then one step per unit of work; errors prefixed
error:instead of the internalscript name; a closing line saying what to run next instead of
done.Dropped the
websocketsImportErrorguard incloudxr/wss.py— its--find-links=install/wheelsadvice only applied to source builds, and the plainImportErroris more useful.Docs
televiz.rst— leads with what Televiz is (the visualization module) and its target (stereo XR over CloudXR); window/offscreen noted as development and debugging aids.Documents
--prefor pre-release installs rather than a version that needs editing every release.camera_streaming.rst— documents the resolution ladder,--with-rtp, and--build-from-source; corrects the setup description now that the codec build and GStreamerprobing are RTP-only.
camera_viz/README.md— refreshed for 1.4: OAK-D stereo is wired (mono / stereo / stereo_rgb), added thegimballock mode, listed therealsenseandsynthetic_xr_3upconfigs, documented the new flags.Type of change
--no-rtpis removed rather than aliased, and RTP now defaults off — anyone passing--no-rtpgets a hard error, and anyone relying on setup provisioning RTP by defaultmust add
--with-rtp.deployis unaffected (--sender-onlyimplies the flag).Testing
Ubuntu 22.04, x86_64, RTX 6000 Ada, CUDA 13.1.
--full,--full --with-rtp,--sender-only,--sender-only --with-rtpresolveWITH_RTPcorrectly;--no-rtperrors out rather than beingsilently ignored.
1.5.41rc1; neither + checkout +non-interactive → aborts with the
--build-from-sourcehint; neither +--build-from-source→ builds from$REPO_ROOT; neither + not a checkout → clone/--wheelguidance;
--wheelshort-circuits.__init__.py, passes healthy modules, and reports genuinely-missing ones distinctly;the cupy probe passes a healthy venv and trips on the broken shape that caused the original
AttributeError.# local | rtptrailing comment doesn't false-positive.sphinx -b htmlbuilds with zero warnings; all cross-references resolve.bash -non both scripts,py_compileonwss.py.camera_viz.sh run configs/zed.yaml --mode xrreproduced the missing-websocketsfailure this fixes.Not covered: a full clean-machine
setupend-to-end, and the Jetsondeploypath.Checklist
SKIP=check-copyright-year pre-commit run --all-filesgit commit -s) per the DCOSummary by CodeRabbit
New Features
Bug Fixes