Repository navigation
test(cli): keep the TS resume capture on its media clock - #5710
Conversation
The same-instance resume test paused its capture at the PCR-rate gate, waited for the export's first write, then reconnected. With no new media, the export's acquisition waited out its two-delay bound, so the capture stood still about 2.4 s under load. The export runs a 1 s delay behind the source, so every resumed frame arrived late and was dropped; the test passed only when a late-flushed tail happened to carry a newer timestamp. Feed the capture on a fixed 150 ms schedule that bursts whatever a stall held back, publish until the export has written, and use a 5 s CBR remux of the same source so media remains after the catalog gate and the export's delay. Loaded runs now drop no frames. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Outcome: the drops came from the fixture, not the exporter or relay. The capture paused about 2.4 s while the export acquired its clock, which is longer than the export's 1 s delay. The capture now stays on its schedule, and it uses a 5 s clip. Loaded runs: 22/22 passed, all with (Written by Claude Opus 5.5) |
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. |
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit 1d90c8d.
No actionable bugs found. Reusing one absolute pacing interval across startup and reconnect is a focused fixture correction, and the longer capture preserves a meaningful tail without widening timeouts or weakening the new-content assertion. The test still exercises actual connection loss, verifies replacement of the relay copy under the same epoch, and requires output timestamps beyond the input prefix supplied before disconnect.
Verification limits: static review of the diff, full test fixture and relevant TS export/follower code only. I did not run tests, reproduce load conditions or independently decode the binary media asset; the reported passing runs are author-provided.
(Written by OpenAI)
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe export integration test now uses a five-second TS capture, paced in 150 ms intervals, across a same-instance publisher reconnect. It checks output timestamps against the published prefix and verifies that output after the reconnect mark contains the capture’s new tail. The test retains checks for the catalog clock and PSI version. The Required list entry and investigation document for the TS resume late-frames quest were removed. Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly identifies the main test change: keeping the TypeScript resume capture on its media clock. |
| Description check | |
The description directly explains the fixture timing issue, the continuous capture fix, the longer test asset, validation results, and test-only scope. |
| Docstring Coverage | |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches
-
✨ Simplify code
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Some tools did not complete. Review the errors below.
🔧 Clippy (1.98.1)
-
Clippy execution timed out
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 @coderabbitai help to get the list of available commands.
|
Merge summary
(Written by Claude Opus 5.5) |
Completes
quest/m1/test-flakes-2/ts-resume-late-frames.md.Cause: the fixture
I looped
the_same_instance_within_the_linger_continueson a host at load 60 to 170 (24 cores), with the exporter stderr and timings printed. In 5 of 6 runs the exporter loggedframe missed its deadline; dropped, 22 to 32 frames per run. The run with no drops was the only one where the export wrote within 134 ms.--delay) behind the source, so a 2.4 s pause in the media made every resumed frame late, and they were dropped.A real publisher keeps its media clock running through a connection loss, so the exporter is behaving as designed. The fixture was the problem.
Fix
tokio::time::interval, which bursts whatever chunks a stall held back, and it keeps publishing until the export has written. Waiting on the export or the reconnect no longer pauses the media.bbb_cbr.tsran out before the export could write (2 s gate plus 1 s delay). I addedbbb5s_cbr.ts, a 5 s, 400 kb/s CBR remux of the samescte35/bbb5s.tssource (255 KB; the ffmpeg command is in the const doc). Only this test uses it.After the fix, 6 sequential runs and 16 runs in batches of 8 concurrent copies, all under load, passed with
dropped=0. Each export output the whole capture.Impact
ts-video-content: no. That quest's suspect is the zeromax_delaylive subscription at startup. Here the drops came from the fixture's own pause in the media.Checks
just checkpassed: 3196 default, 466, 192, 97 and 10 tests, plus lint, flake and quest checks.Decisions
missed its deadlinecheck on the exporter's stderr). It would catch this exact regression, but it ties the verdict to host scheduling under extreme load. The maintainer confirmed keeping only the new-content assertion the quest asked for.Suggested follow-ups
--linger 10safter the clean finish. A shorter linger that still covers the reconnect would speed up CI. Recommendation: a small quest, only if CI time matters.(Written by Claude Opus 5.5)
🤖 Generated with Claude Code