[kixelated]

Problem

quest/m0/ci/recovery-clips.md, found by moq-dev/moq#6987's profile: nextest runs each test in its own process, so the OnceLock in Profile::clip never shared anything. Every recovery_test case re-encoded its FFmpeg clip and re-decoded it for the reference pictures.

Change

Test-only, in rs/moq-mux/src/container/ts/recovery_test.rs:

• One ffmpeg(args, input) helper replaces the two hand-rolled Command blocks (the encode and decode). It fails on a non-zero status or any logged error, as both did.
• cached(args, input) runs it once per FFmpeg build. The entry is keyed by a hash of the FFmpeg binary's resolved path and mtime, the arguments, and the input bytes. It lives in recovery-ffmpeg/ beside the test binary (target/debug/recovery-ffmpeg/).
  • Each entry has its own lock (std::fs::File::lock). The first process that needs an entry makes it and the others wait for it.
  • Entries are written to a .partial file and renamed into place, so a killed process leaves no torn entry.
• The encodes and the reference decodes of each encode go through cached. Every other decode, the ones that grade the export, still runs FFmpeg each time.
• Because the reference decode is keyed by its input bytes, splice stays in-process. The Resized and Spliced profiles share their first encode with Closed.

Every case runs as before. No assertion changed.

Result

CPU for moq-mux container::ts::recovery_test: 19 tests, the per-test-process wait4 user+system time from the moq-dev/moq#6914 wrapper (SHA-256 d16e86fb...). Same machine, means of 2 to 3 runs each.

	Cold cache (first run)	Warm cache
main	11.99 s	11.99 s
encodes cached	8.98 s	6.74 s
encodes and reference decodes cached	8.09 s	5.20 s

• Encode: saves 3.0 s cold and 5.3 s warm.
• Reference decode: saves a further 0.9 s cold and 1.5 s warm.
• Total: a cold run, such as a fresh CI checkout, saves 3.9 s (33%). Each of its 7 distinct encodes and 7 reference decodes runs once. A warm local run saves 6.8 s (57%).
• That is more than the 35 to 40% the quest estimated because main's x265 still sizes its thread pools by host cores. moq-dev/moq#6987 changes that.

Per-test CPU seconds

Test	main	Encodes, cold	Encodes, warm	Both, cold	Both, warm
hevc_keeps_radl_after_a_hole	1.15	1.11	0.41	1.12	0.33
hevc_trims_only_orphaned_rasl	1.11	1.08	0.41	1.10	0.33
a_skip_across_a_resolution_change_takes_the_new_parameters	0.86	0.63	0.53	0.58	0.39
a_declared_discontinuity_survives_loss_either_side	0.85	0.74	0.46	0.65	0.31
a_lost_discontinuity_marker_still_breaks_the_clock	0.85	0.45	0.45	0.31	0.30
a_skipped_group_resumes_at_the_next	0.61	0.39	0.38	0.31	0.30
a_skip_across_the_wrap_keeps_the_clock	0.60	0.56	0.39	0.58	0.31
a_cut_group_keeps_what_it_delivered	0.60	0.39	0.38	0.46	0.32
skewed_audio_survives_a_video_skip	0.59	0.38	0.39	0.31	0.31
a_clean_path_loses_nothing	0.57	0.37	0.37	0.31	0.29
a_stalled_track_leaves_the_others_running	0.56	0.36	0.37	0.28	0.29
a_stall_drops_what_it_made_late	0.55	0.34	0.35	0.26	0.27
a_congested_path_sheds_what_it_made_late	0.55	0.45	0.35	0.38	0.28
a_path_stall_drops_what_it_made_late	0.54	0.33	0.33	0.25	0.26
open_gop_trims_only_the_orphans	0.52	0.51	0.32	0.53	0.25
a_join_starts_on_a_whole_group	0.51	0.31	0.31	0.24	0.24
a_late_catalog_starts_on_a_whole_group	0.49	0.31	0.30	0.24	0.23
the_graders_fail_what_they_should	0.47	0.25	0.25	0.17	0.18
the_bottleneck_admits_only_what_fits	0.00	0.00	0.00	0.00	0.00
Total	11.99	8.97	6.74	8.09	5.20

Choices made without asking

• A lock-guarded cache instead of committed clips or a nextest setup script.
  • Committing the clips would add about 2 MB of binary (7 encodes) to the history for every regeneration. It still needs a regeneration recipe, and it would still re-decode the reference in every process unless the hashes were committed too.
  • A setup script would have to duplicate the encode arguments outside Rust, and plain cargo test would lose the clips.
  • The cache needs no new tooling or dependency, and it works the same under nextest and cargo test.
• The cache outlives a run. It is cleared with the target directory, so a warm local run skips FFmpeg's encodes entirely.
  • Stale entries for an old FFmpeg build or old arguments stay until target/ is cleaned. The current set is 2.4 MB.
  • The key's hash is std::hash::DefaultHasher, which a Rust release may change. That only costs a re-encode.
• The FFmpeg build is identified by its resolved path plus mtime, not ffmpeg -version.
  • The version probe would cost about 18 ms of CPU per test process, roughly 0.35 s per run.
  • In the Nix dev shell the store path already changes with any input, x264 and x265 included.
  • Outside Nix, an x264/x265 library upgrade that leaves the ffmpeg binary untouched keeps the old clips. That is harmless: the tests grade the export against whatever clip they import.

Conflicts

moq-dev/moq#6987 edits the same Encode::run and decode: x265 pools=none, a murmur3 framehash, and one TS concatenation. Whichever lands second needs a small hand merge. Its argument changes need nothing else: they change the cache keys, so they invalidate the cache themselves.

Impact

• Public API: none.
• Wire: none.
• Production behavior: none (test-only).

Validation

• just check passed: 4,136 Rust tests, 60 skipped. quest check --base origin/main passed.
• just rs ts-recovery passed: the nightly two-minute arm runs through the same cache.
• Runs on this branch: main is cdd0196338, and the branch is 080f2f5f3d.
• The "encodes cached" rows are this branch with Reference::new temporarily back on the uncached decode.
• A cold run deletes target/debug/recovery-ffmpeg/ first. The command was just rs test -p moq-mux --lib -E 'test(/^container::ts::recovery_test::/)' under the wrapper, with zero retries.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

You'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 14 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 7b9b6981-367b-4e32-856d-c2f7fbe3952b

Commits

Reviewing files that changed from the base of the PR and between cdd0196 and 080f2f5.

Files selected for processing: 3

• quest/m0/ci/README.md
• quest/m0/ci/recovery-clips.md
• rs/moq-mux/src/container/ts/recovery_test.rs

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

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-28T10:26:16.228344Z	080f2f5	Draft marked ready

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: 080f2f5

No actionable bugs found. The test-only cache is a reasonable, dependency-free way to share encodes and reference decodes across nextest processes: per-entry locking serializes producers, partial-file rename avoids publishing interrupted writes, and argument/input changes invalidate entries. Crucially, exported-output grading still runs fresh FFmpeg decodes and retains the existing assertions. Keeping splice transformations outside the cache preserves their coverage without duplicating fixture recipes.

Verification: static inspection of the changed helper, clip construction, reference parsing/comparison and grader paths. I did not run tests or reproduce the reported CPU savings. FFmpeg identity is intentionally path/mtime-based, not a complete fingerprint of dynamically linked libraries.

[kixelated]

Merge summary: no changes needed. CI passed on 080f2f5f3d (12 checks), Codex (thumbs up) and the automated OpenAI review both reported no findings, and there are no unresolved threads. quest check --base origin/main passed on the head. Landing through the merge queue (squash).

Suggested follow-up from the PR body: moq-dev/moq#6987 touches the same Encode::run and decode, so whichever lands second needs a small hand merge.

(Written by Claude Opus 5.5)