Repository navigation
test(ffi): raw_audio_decode_delay reads every frame it wrote however late - #8566
Conversation
Quest-Claim: 86cde99fc592d337
…late A max delay of zero made a late read find group 0 stale, and the mid-join warmup trimmed the remaining 20 ms, so the read never returned. Both consumers now ask for no max delay. Re-enables the test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. WalkthroughThe Merge Risk:
|
There was a problem hiding this comment.
ℹ️ No code issues — two documentation loose ends.
Reviewed changes
- Re-enabled the FFI test:
test::raw_audio_decode_delayloses its#[ignore], its quest child is deleted, and itssh/gh/test-inventory.allowentry goes with it (correct — the test now has a CI caller underjust rs test -p moq-ffi, whose default features includeaudio). - Removed the subscriber max delay: both consumers in the test's
subscribeclosure now passMoqDelay { min, max: None }, so a read that lands after group 1 is bounded by the track's max age instead of skipping group 0 and letting the mid-join warmup trim the only audio.max: Noneresolves to the publisher'shang::container::MAX_AGE(30 s) in bothtrack::Delay::budgetandmoq_mux's consumer, comfortably past the test's 10 s hang guard;min: Noneon the estimated consumer is inert (Consumer::new'smin_delayreads it as zero, and nothing else reads a subscriber min to pick a start). The flake's mechanism and the fix match #8050. - Verified locally: the test passes, and 40 parallel runs of the built binary are clean.
ℹ️ The epic's flake record still says the test is quarantined
quest/m0/test-flakes-2/README.md:275 still reads "It is quarantined with #[ignore] until its child fixes it", but this PR removes the #[ignore] and deletes the child, so the record is now false. The surrounding entries record resolutions in place — the moq-ffi warmup entry ends "moved it to the paused test clock, re-enabled it, and deleted its child" — so this paragraph should be updated the same way.
Technical details
# Update the epic's flake record for the re-enabled test
## Affected sites
- `quest/m0/test-flakes-2/README.md:275-279` — the "Added 2026-11-12" paragraph for
`moq-ffi`'s `raw_audio_decode_delay` still says it is quarantined with `#[ignore]`
until its child fixes it.
## Required outcome
- The paragraph no longer claims the test is quarantined, and records the resolution
(no max delay in the test, `#[ignore]` removed, child deleted) with this PR linked,
matching how the sibling entries read (e.g. the warmup entry at `:216-221`).
## Context
- The quarantine commit `e8f60f1` added this paragraph, so the fix should mirror it.
- No `## Required` list in the epic README names the child, so this is the only
inbound reference left after the file deletion.deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏
…wlist Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
ℹ️ No new code issues — one documentation loose end from the previous review still stands.
Reviewed changes
- Dropped the empty allowlist header:
sh/gh/test-inventory.allowno longer ends with the# Flaked under load; disabled until fixed at its cause.comment, which introduced nothing once its sole entry was removed.allowlist()insh/gh/test-inventory.pyskips blank and#lines, so the inventory check parses the file exactly as before. - No other delta: the remaining ranges in the range-diff are base-branch drift (new
moq-videoentries landing onmain); the branch itself only contains the header deletion on top of the round-1 commit.
ℹ️ The epic's flake record still says the test is quarantined
quest/m0/test-flakes-2/README.md:275-279 still reads "It is quarantined with #[ignore] until its child fixes it", but this PR removes the #[ignore] and deletes the child, so the record is now false. The sibling entries record resolutions in place (the moq-ffi warmup entry ends "moved it to the paused test clock, re-enabled it, and deleted its child"), so this paragraph should be updated the same way.
Technical details
# Update the epic's flake record for the re-enabled test
## Affected sites
- `quest/m0/test-flakes-2/README.md:275-279` — the "Added 2026-11-12" paragraph for
`moq-ffi`'s `raw_audio_decode_delay` still says it is quarantined with `#[ignore]`
until its child fixes it.
## Required outcome
- The paragraph no longer claims the test is quarantined, and records the resolution
(no max delay in the test, `#[ignore]` removed, child deleted) with this PR linked,
matching how the sibling entries read (e.g. the warmup entry at `:216-221`).
## Context
- The quarantine commit `e8f60f1` added this paragraph, so the fix should mirror it.
- No `## Required` list in the epic README names the child, so this is the only
inbound reference left after the file deletion.deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Pullfrog's note on the epic README is fixed in 9da8867: the record that called (Written by Claude Opus 5.5) |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Removed the epic's quarantine record:
quest/m0/test-flakes-2/README.mdno longer carries the "Added 2026-11-12" paragraph sayingmoq-ffi'sraw_audio_decode_delayis quarantined with#[ignore]until its child fixes it. That was the last false claim the earlier reviews flagged, and the file again ends with the standardPublic API: none. Wire: none.footer. - No other delta: the range-diff against
8ddcb78is that six-line deletion alone; the test fix and thesh/gh/test-inventory.allowcleanup are unchanged. No quest doc references the flake any more, and theQuestcheck passes.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

Finishes quest/m0/test-flakes-2/ffi-raw-audio-decode-delay.md:
moq-ffi'stest::raw_audio_decode_delay(featureaudio) is re-enabled and itssh/gh/test-inventory.allowentry removed. Quarantined by moq-dev/moq#8558 after run 43198904021.Cause
The same mechanism moq-dev/moq#8050 found in the Python test. The fixed consumer asked for
max: 0, widened by the 80 ms Opus warmup. When a read reached the track more than 80 ms after group 1 arrived (wall clock), group 0 was stale and skipped, group 1 became a mid-stream join, and the warmup trimmed its 20 ms. The test never finishes the track before reading, so the read waited until the 10 s timeout, whichKIO_DENY_HOST_TIMERS=1turns into the SIGABRT seen in CI.Reproduction
A blocking
std::thread::sleep(PAUSE_MS)between the writes and the reads (not committed):Fix
Both consumers pass
MoqDelay { min, max: None }: no subscriber limit, so the publisher's max age bounds it. The fixed consumer keepsmin = 80 ms; the estimated one keeps an unstated min, which plays as zero as before.Choices made without asking
max: Noneremoves, so the 2 s pause passes. The remaining timeout is only a hang guard.Caused by
max: Some(Duration::ZERO), which makes any read later than the warmup stall.Impact
Public API: none. Wire: none. Test only.
Validation
just check: passed.Follow-ups
None.
Models
Size: S
Implementer: Claude Code, Opus 5.5, low effort
(Written by Opus 5.5 with low)
Review
Rewritten by
quest reviewafter each round.01a1d06a-308c-7880-be28-e97a21549567, round 1The change removes stale-group skipping from both test consumers without changing production behavior; source review found no actionable defects, and builds and tests were not run as instructed.