Skip to content

webxr: disable the decoder warm-up timeouts by default - #1383

Merged
gareth-morgan-nv merged 1 commit into
mainfrom
gmorgan/disable-warmup-timeouts-by-default
Oct 28, 2026
Merged

gareth-morgan-nv merged 1 commit into
mainfrom
gmorgan/disable-warmup-timeouts-by-default

Conversation

@gareth-morgan-nv

@gareth-morgan-nv gareth-morgan-nv commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Default warmupBeginTimeoutMs and warmupEndTimeoutMs to 0, which disables decoder warm-up detection (since fix(webxr): let 0 disable the stream-attach and warm-up timeouts #1257). The defaults change in the component, the client's default configuration, and the two settings inputs and their labels.
  • A CloudXR runtime with no OpenXR app streams idle frames that count as warm-up frames, so the 10 s begin and 30 s end deadlines can tear down a healthy session that is only waiting for an app to start.
  • Callers, URL parameters and the OOB launch settings can still set a deadline. Stream-attach detection is unchanged.

Validation

  • npm test passes (134 tests); the mock tests pass explicit warm-up timeouts, so they don't depend on the defaults. eslint and prettier are clean, and pre-commit passes.
  • Not run: the Playwright suite, or a headset run with the new defaults.
  • Backporting to a release branch needs fix(webxr): let 0 disable the stream-attach and warm-up timeouts #1257 first: before it, a value of 0 is passed to setTimeout and fires immediately instead of disabling.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Configuration
    • Decoder warm-up begin and completion timeout checks are now disabled by default across the client and its configuration interface.
    • To enable these checks, set their timeout values to positive numbers. The configuration labels now identify the disabled defaults.

A CloudXR runtime with no OpenXR app streams idle frames that count as
decoder warm-up frames, so the 10 s begin and 30 s end deadlines can tear
down a healthy session that is only waiting for an app to start. Default
warmupBeginTimeoutMs and warmupEndTimeoutMs to 0, which disables the
detection; callers and the URL/OOB parameters can still set a deadline.
Stream-attach detection is unchanged.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
@coderabbitai

coderabbitai Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between a74babb and fae20e8.

Files selected for processing: 3
  • deps/cloudxr/webxr_client/helpers/react/CloudXRComponent.tsx
  • deps/cloudxr/webxr_client/src/CloudXR2DUI.tsx
  • deps/cloudxr/webxr_client/src/index.html

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Walkthrough

Warm-up begin and end timeout defaults change to 0 in the React component, default configuration, and UI. The component documentation and UI labels describe the timeouts as disabled by default.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~8 minutes

Possibly related PRs

  • NVIDIA/IsaacCapture#1365: Changes the SDK version associated with idle-frame handling, which interacts with the warm-up watchdogs.
Merge Risk: Minimal · up to fae20

No actionable merge-blocking risk is identified; the timeout-default changes are consistent across the component, configuration, and UI.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the primary change: disabling the WebXR decoder warm-up timeouts by default.
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 2 files. (1 skipped: 1 unsupported.)

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

✅ Preview deployed: https://NVIDIA.github.io/IsaacCapture/preview/pr-1383/client/

Built from commit fae20e8. It may take a minute for the updated preview to appear.

@fvaloppi-nv fvaloppi-nv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sgrizan-nv sgrizan-nv left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To confirm my understanding, I assume we're going to re-enable these timeouts once we have changes from Cloudxr.js?

@gareth-morgan-nv

Copy link
Copy Markdown
Contributor Author

To confirm my understanding, I assume we're going to re-enable these timeouts once we have changes from Cloudxr.js?

Yes, this is for 1.6.0

@gareth-morgan-nv
gareth-morgan-nv merged commit 8840c1b into main Oct 28, 2026
42 checks passed
@gareth-morgan-nv
gareth-morgan-nv deleted the gmorgan/disable-warmup-timeouts-by-default branch October 28, 2026 20:02
github-actions Bot added a commit that referenced this pull request Oct 28, 2026

This branch was successfully deployed

1 active deployment
dev — fae20e8f Deployed Oct 28, 2026 by gareth-morgan-nv via publish-wheel #6149
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants