Repository navigation
feat(hang): describe rendition warmup in catalogs - #5212
Conversation
Co-Authored-By: GPT-6 <noreply@openai.com>
Co-Authored-By: GPT-6 <noreply@openai.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 1 minute. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 723408054b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Co-Authored-By: GPT-6 <noreply@openai.com>
|
Ready to merge: the audio/video warmup rounding finding is fixed and resolved. Serialization now rounds fractional milliseconds upward while retaining optional zero; regression tests reproduce the original early-presentation bug. All current-head hosted checks pass, including Check, Test, Android, Windows, and macOS. Local Hang tests, draft validation, and full cross-language interop also pass. The unchanged local bundle-test time limits are documented in the PR; those fixtures pass in hosted CI. Public API/wire impact remains the approved optional warmup fields in Rust/JS and the catalog draft, with explicit upward rounding for the integer millisecond upper bound. No other scope changes. Enqueueing the reviewed correction at ab5200b under the current contribution policy for a push receiving no new review before CI completes. (Written by GPT-6) |
Problem
A receiver cannot tell how far before a seek point decoding must start for audio priming or gradual video refresh. Rounding fractional warmup down can allow presentation before decoder convergence.
Approach
Add optional millisecond
warmupto audio and video catalogs in Rust and JS. Serialize Rust durations with the existing private ceiling conversion, preserving explicit zero for warmup and the existing jitter/delay zero rules. Describe random access and rounding in the Hang draft. Omitted and zero warmup preserve immediate access.Impact
AudioConfig.warmupandVideoConfig.warmup(Rust duration, JS milliseconds).Alternatives
A generic metadata field would leave receiver behavior undefined. Nearest rounding understates an upper bound; fractional wire values would conflict with the integer catalog representation. No consumer behavior changes in this quest.
Validation
Audio and video sub-millisecond regressions fail before the correction and pass afterward, including 1 ns -> 1 ms and 333.2 ms -> 334 ms. All 119 Rust Hang tests, Hang lint/docs, 227 JS Hang tests, draft checks, and
just test interop --allpass.Full scoped validation passes 2,828 primary Rust tests, selected feature checks, JS builds/type checks, documentation, and workflow checks. Local JS bundling fixtures in
common/worklet.test.tsandpublish/src/file-bundle.test.tshit their existing five-second limits. Both files are unchanged from main; an identical standalone worklet build measured 4.73 seconds. No timeout or retry workaround was added. Fresh hosted CI validates the full gate on the updated head.Follow-ups
Consumer warmup and audio priming remain their existing quests. The optional nonnegative millisecond representation is unchanged. The local bundle-test timing limitation remains outside this catalog change.
(Written by GPT-6)