Skip to content

quest(tstd): plan send-ahead within the delay and mux-rate hold - #4681

Merged
kixelated merged 2 commits into
quest/m1/tstd/READMEfrom
quest/m1/tstd/plan-followups
Oct 2, 2026
Merged

kixelated merged 2 commits into
quest/m1/tstd/READMEfrom
quest/m1/tstd/plan-followups

Conversation

@kixelated

Copy link
Copy Markdown
Collaborator

Two T-STD questline children planned from #4645 and t0ms's review. The full decision paper trail is in #4680.

  • quest/m1/tstd/send-ahead.md [M]: the total lag is --delay, with send-ahead coming out of the same budget and capped at the decoder buffer's reach. The default delay becomes 1 s.
  • quest/m1/tstd/mux-rate-hold.md [S]: ts import holds the catalog until the mux rate is measured (a VBR source publishes without it after the window), so the export is constant-rate from its first packet.

Both require delay.md (#4645). Public API / wire: none (quest files only).

Decisions (paper trail):

  • Output trails the source by 2× --delay → ✅ carve send-ahead out of the delay (alternatives: fix in feat(moq-mux)!: fixed-delay jitter buffer and per-PID T-STD admission for TS export #4645, accept 2×)
  • Default once carved out → ✅ raise to 1 s (alternatives: fail loud at 500 ms, default from the stream)
  • First ~2 s unpadded → ✅ import holds the catalog until measured (alternatives: export waits, leave it)
  • VBR never settles → ✅ publish without muxRate after the window (alternative: keep holding)

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Review (head 3c6d7dfe59f7c8a6136e7e6371c3bef5720b5d31)

MERGE — quest-only plan for two #4645 follow-ups; claims match the draft’s code and t0ms’s grading. No blockers.

What this does

Adds send-ahead.md [M] (carve schedule pre-load out of --delay, default 1 s) and mux-rate-hold.md [S] (import withholds catalog until the mux-rate meter settles), and lists both under quest/m1/tstd/README.md. Paper trail correctly points at #4680.

Claim checks (vs #4645 head / current mux_rate.rs)

  • Double lag: Schedule::window is the export delay and a_joiner_runs_at_the_delay_from_its_first_output asserts 2 * DELAY — matches the send-ahead write-up.
  • Default --delay is 500ms (rs/moq-cli/src/args.rs Transport::delay).
  • Meter window is 2 s (WINDOW = 2 * PCR_HZ in mux_rate.rs); VBR never publishes (vbr_never_publishes); catalog is released on first PMT today, with muxRate patched in later via record_mux_rate.
  • Harness skip: on #4645, pcr-timing.py’s pcr-schedule skips intervals before first_null when --mux-rate is set — matches “grade from the first null packet”.
  • Mid-stream rate: Schedule docs already say a rate that turns up mid-stream takes over next slot — the “remove the grace” cleanup is real.
  • hrd9m / 0.93 s EB reach / CNN ~1.7 s @ 500 ms: grounded in t0ms’s #4645 comments (e.g. delivery 1,706 ms @ 500 ms; “decoder buffer only lets video go 0.93 s ahead” at 2 s).

Non-blocking

  1. send-ahead.md “~1.7 s at 500 ms” ≠ 2×delay. 2×500 ms is 1.0 s; the 1,706 ms CNN delivery figure also includes join/source lead (t0ms breaks this out on hrd9m). The unit test is the clean 2× proof; suggest “~1.7 s delivery (≈2×delay plus join/source lead)” so implementers don’t expect carving alone to cut 1.7 s → 0.5 s. The later “drop by about a delay” re-grade ask is the right prediction.
  2. mux-rate-hold.md “window closes” is underspecified for VBR. Meter keeps rolling windows and never times out; “settle or window closes, whichever first” needs an explicit rule (e.g. after one WINDOW of PCR time with published() == None, drop the reservation and omit muxRate). Otherwise the hold can mean “wait forever on pathological VBR.”
  3. Optional: delay.md Related still only lists byte-schedule; a link to these two children would help navigation (README Required already has them).

CI Check/Test still pending on this head; quest markdown only.

This is an automated review, not the maintainer's decision
(Written by Grok)

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Automated review by review (OpenAI)

Reviewed 3c6d7df.

  1. [P2] Specify an EOF release for short imports (quest/m1/tstd/mux-rate-hold.md:18–22). Beyond the existing VBR-window comment, a valid TS shorter than 2 s cannot complete even one measurement window: Meter::pcr advances it in PCR time, not elapsed wall time. Neither release condition in this plan can fire. This matters because Programs::finish calls Import::finish and then catalog.finish; the latter closes the catalog tracks without publishing a held reservation. Extending the initial catalog hold as specified would therefore let a short import finish with media but no discoverable catalog. Require EOF to release the resolved catalog without muxRate before closing it, and add a sub-2-second CBR import regression. This is a missing requirement in the plan, not an executed runtime regression in this docs-only PR.

Direction: the single total-delay budget follows the measured double-buffering problem. Reusing the importer's existing catalog reservation is simpler than adding another wait state in export, provided its termination is explicit.

Verification: inspected all three changed documents, the meter and catalog/finish paths, #4645's scheduling and join test, and #4680's decisions. No tests were run; there are no executable changes. CI Check was still running.

(Written by review (OpenAI))

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Addressed review in 4ef90de: the mux-rate hold is bounded by one WINDOW of PCR time, end of input releases a held catalog without muxRate before catalog.finish (with a sub-2 s CBR test), and the 1.7 s figure is broken out so carving is expected to cut about one delay. Skipped the optional delay.md Related link, since #4645 deletes that quest.

(Written by Claude Opus 5.5)

@kixelated

Copy link
Copy Markdown
Collaborator Author

Re-review (head 4ef90de0e84e89a51543497adff361c506cdef69)

MERGE — notable push after prior MERGE on 3c6d7dfe. Closes both earlier non-blocking notes and adds a real short-TS edge case. Still quest-only; no blockers.

What changed (vs 3c6d7dfe)

  • mux-rate-hold.md: hold bound is now explicit — settle, or one WINDOW of PCR time with published() == None, whichever first — and calls out that Meter keeps rolling / never times out. Adds end-of-input release without muxRate before catalog.finish, plus a short-CBR catalog test.
  • send-ahead.md: the ~1.7 s @ 500 ms figure is clarified as 2×delay plus join/source send-ahead; carving alone cuts by about one delay, not to 0.5 s.

Prior findings

  1. Fixed — ~1.7 s ≠ pure 2×500 ms wording (send-ahead).
  2. Fixed — VBR / “window closes” underspecified (mux-rate-hold). Matches #4645 mux_rate.rs: WINDOW = 2 * PCR_HZ, rolling windows, no timeout, vbr_never_publishes.

Claim check (new bits vs #4645 import.rs / Meter)

  • Today initial_reservation drops on first PMT and finish() never releases a still-held reservation. Once the hold stretches past PMT until the meter settles, a TS shorter than one window would finish with the reservation unpublished — the new end-of-input bullet is required, not polish.
  • Bound in PCR time (not wall clock) matches how Meter advances; no-PCR / truncated files fall through to the end-of-input release.

Non-blocking

  1. Optional: if a source is VBR for the first window then later settles, import can still patch muxRate mid-stream via record_mux_rate, but the plan also removes the export’s mid-stream grace. That combination leaves late-settling rates unused — fine if intentional (treat as never-CBR-from-start); worth one sentence so implementers don’t keep the grace “just in case.”
  2. Still optional: delay.md Related could link these two children (README Required already does).

Depends on draft #4645; paper trail #4680. CI Check/Test queued on this head; markdown only.

This is an automated review, not the maintainer's decision
(Written by Grok)

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Automated review by review (OpenAI)

Reviewed 4ef90de, the one-commit delta from 3c6d7df; the base is unchanged.

Prior finding addressed in the plan: mux-rate-hold.md:25–28,32–34 now requires EOF to release the catalog before closure and specifies a short-CBR regression.

  1. [P2] Wait for a judged window, not just WINDOW elapsed (quest/m1/tstd/mux-rate-hold.md:18–21). The new literal PCR-time cutoff can expire before the referenced meter has evaluated a perfectly constant-rate source. Meter::pcr only adds completed ≥500 ms samples to its window. With PCRs every 40 ms, those samples are 520 ms long: at 2.00 s only 1.56 s is in the completed window, and the first judgment/publication occurs at 2.08 s. The proposed bound therefore releases this CBR catalog without muxRate, recreating the unpadded startup the quest aims to remove. End the hold when the first measurement window has actually been judged, stable or unstable, with the explicit EOF exception; add a 40 ms PCR-spacing case to the first-catalog test.

Direction: reuse the meter's completion event rather than maintain a second elapsed-time cutoff. The EOF rule and clarified delivery-latency explanation are useful improvements.

Verification: inspected both changed documents and the head's meter; independently checked the sample-pooling thresholds with a small arithmetic model. No Rust or integration tests were run; this remains a plan-only change. CI Check was in progress.

(Written by review (OpenAI))

@kixelated
kixelated merged commit 1293c8d into quest/m1/tstd/README Oct 2, 2026
3 checks passed
@kixelated
kixelated deleted the quest/m1/tstd/plan-followups branch October 2, 2026 00:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant