diff --git a/.claude/skills/plan-issues/SKILL.md b/.claude/skills/plan-issues/SKILL.md index ea261e9c75..d5a88012e9 100644 --- a/.claude/skills/plan-issues/SKILL.md +++ b/.claude/skills/plan-issues/SKILL.md @@ -4,4 +4,4 @@ description: Plan quests from open GitHub issues without the quest label. --- Call /plan-quests for repository's open GitHub issues without the `quest` label. -Add the `quest` label to these issues after the PR merges. +Add the `quest` label to these issues once the PR is open, so another planner skips them. diff --git a/quest/AGENTS.md b/quest/AGENTS.md index 16eebac7aa..787c525880 100644 --- a/quest/AGENTS.md +++ b/quest/AGENTS.md @@ -80,7 +80,8 @@ Current decisions, open questions, or implementation guidance. child owns: the end-to-end test, the docs page. - New work joins the milestone matching its priority, at its rank. - Every issue under `Closes` carries the `quest` GitHub label - (`gh issue edit --add-label quest`), applied when the quest lands. + (`gh issue edit --add-label quest`), applied when the PR creating the + quest opens and removed if that PR closes unmerged. `Related` is context and gets none. - A release or pin bump that unblocks work is its own quest holding the condition as a plain-text `Required` bullet; every dependent requires it. diff --git a/quest/m0/README.md b/quest/m0/README.md index a37d03ba03..aa41bc85a5 100644 --- a/quest/m0/README.md +++ b/quest/m0/README.md @@ -94,6 +94,7 @@ do not add another media abstraction or a renderer crate during stabilization. ## Quests - [Release](/quest/m0/release.md) - the release moq.pro adopts: binding docs, an upgrade page, and a staging soak gate it rather than the merge +- [Audio quality harness](/quest/m0/audio-quality-harness/README.md) - a playout latency regression fails a run instead of arriving as a bug report, and its recorder supplies the jitter target's replay traces - [Audio jitter target](/quest/m0/audio-jitter-target/README.md) - the audio playout target is a measured estimate of arrival timing in both languages, not a round-trip guess - [A/V clock](/quest/m0/plan-av-clock.md) - the audio playhead drives Sync.reference while audio plays, through per-track sync handles - [SD rendition for bbb](/quest/m0/bbb-sd.md) - `just pub bbb` publishes a pre-encoded 360p rung beside the 720p source, for localhost demos and moq.pro's fleet demo diff --git a/quest/m0/audio-jitter-target/README.md b/quest/m0/audio-jitter-target/README.md index 9f9600f3e1..6ce2effd6e 100644 --- a/quest/m0/audio-jitter-target/README.md +++ b/quest/m0/audio-jitter-target/README.md @@ -25,6 +25,20 @@ additive and target `main`: the native knob is a new field on a `#[non_exhaustive]` struct, and the browser estimator is a new module plus a new `spread` observation. +Decided for landing: the line merges to `main`, not `dev`, with a changelog +note for two behavior changes treated as fixes. `@moq/watch` `Sync` takes a +numeric delay literally instead of adding the rendition delay on top +([#3954](https://github.com/moq-dev/moq/pull/3954)), and `moq play --delay` +defaults to `auto` instead of `100ms` +([#3967](https://github.com/moq-dev/moq/pull/3967)). The old additive delay +was wrong, and both compile unchanged for existing callers. The line branch is +about 200 commits behind `main` with conflicts in `js/watch/src/sync.ts` and +`rs/moq-cli`; merge `main` in (never rebase the shared branch) before +finishing the watch quest. The raw #3477 traces are gone, so record fresh +traces with the [audio quality +harness](/quest/m0/audio-quality-harness/README.md) instead of asking the +reporter; they replace the #3477 traces wherever the quests name them. + The algorithm is written down at `doc/concept/audio-jitter.md`, with a conformance corpus beside it that both implementations will read. @@ -71,6 +85,6 @@ buffer against uneven arrivals. ## Related -- [Audio quality harness](/quest/m1/audio-quality-harness/README.md) - the automated proof, built on its own schedule +- [Audio quality harness](/quest/m0/audio-quality-harness/README.md) - the automated proof, and the recorder of the traces the watch quest replays - [Time stretch](/quest/m1/watch-audio-time-stretch.md) - inaudible convergence, on top of this - [Plan: A/V clock](/quest/m0/plan-av-clock.md) - the clock this target eventually feeds diff --git a/quest/m0/audio-jitter-target/watch.md b/quest/m0/audio-jitter-target/watch.md index 188de90dc9..a1a258f61e 100644 --- a/quest/m0/audio-jitter-target/watch.md +++ b/quest/m0/audio-jitter-target/watch.md @@ -74,11 +74,11 @@ against it is likely cheaper than patching the branch's: came up on the 100 ms chip, so something is restoring or overriding it. Pin that down: a stored preference silently winning over the default is its own bug, and it also means auto gets far less real exposure than it looks like. -- Replay the recorded traces from #3477 rather than synthetic ones of the same - shape. They are on the reporter's fork (`fperex/moq`, branch - `debug/rt-audio`) with the raw ndjson attached to release - `rt-audio-traces-2026-09-06`. Trim a copy into the repository and replay it - through both rings in `replay.test.ts`. +- Replay recorded traces rather than synthetic ones of the same shape. The + #3477 traces are gone, so record fresh ones with the [browser + harness](/quest/m0/audio-quality-harness/browser.md) (decided with the + maintainer instead of asking the reporter). Trim a copy into the repository + and replay it through both rings in `replay.test.ts`. - Manual run against the public relay on Chrome and Safari, the two rows the issue measured. Measure the publisher's audio encoder input-to-output lag in the same run using the reporter's instrumented harness; #3518 fixed the known @@ -95,6 +95,10 @@ reading `probe`; removing it is part of the `SyncInput` reshape in [Plan: A/V clock](/quest/m0/plan-av-clock.md). Land the estimator so that quest can adopt it without a second estimator change. +## Required + +- [Browser harness](/quest/m0/audio-quality-harness/browser.md) - records the arrival traces this quest replays + ## Related - [Plan: A/V clock](/quest/m0/plan-av-clock.md) - reshapes `SyncInput` around the per-track spread this quest produces diff --git a/quest/m1/audio-quality-harness/README.md b/quest/m0/audio-quality-harness/README.md similarity index 86% rename from quest/m1/audio-quality-harness/README.md rename to quest/m0/audio-quality-harness/README.md index a40a280920..5861c81352 100644 --- a/quest/m1/audio-quality-harness/README.md +++ b/quest/m0/audio-quality-harness/README.md @@ -23,9 +23,11 @@ both passing. The starting point is not a blank page. The reporter on #3477 already built a working browser harness on their fork (`fperex/moq`, branch `debug/rt-audio`): a CDP driver, a beacon sink, a trace analyzer, a ring replay, a five-scenario -`bench.sh`, and a `compare.mjs` that prints before-and-after tables, with 130 -raw ndjson traces attached to release `rt-audio-traces-2026-09-06`. Upstream -that rather than reinventing it. +`bench.sh`, and a `compare.mjs` that prints before-and-after tables. Upstream +that rather than reinventing it. The raw traces it shipped with are gone, so +this harness records fresh ones, and the [jitter target's watch +quest](/quest/m0/audio-jitter-target/watch.md) replays them (decided with the +maintainer during the merged-PR audit). Jitter comes from the seeded userspace UDP shaper the transport drills run under (`rs/moq-shaper`, documented in `test/drill/README.md`), not from a fake @@ -44,8 +46,8 @@ each of those dimensions moves the expected floor. ## Quests -- [Browser](/quest/m1/audio-quality-harness/browser.md) - upstream the fork's harness, grade it against a budget, run it nightly -- [Native](/quest/m1/audio-quality-harness/native.md) - the same profiles and budgets through `moq play` on a dummy device +- [Browser](/quest/m0/audio-quality-harness/browser.md) - upstream the fork's harness, grade it against a budget, run it nightly +- [Native](/quest/m0/audio-quality-harness/native.md) - the same profiles and budgets through `moq play` on a dummy device ## Related diff --git a/quest/m1/audio-quality-harness/browser.md b/quest/m0/audio-quality-harness/browser.md similarity index 94% rename from quest/m1/audio-quality-harness/browser.md rename to quest/m0/audio-quality-harness/browser.md index f73d5bb8dc..887f864544 100644 --- a/quest/m1/audio-quality-harness/browser.md +++ b/quest/m0/audio-quality-harness/browser.md @@ -64,8 +64,9 @@ budget, or a schedule. each of those moves the expected floor. A profile-only key silently grades one row against another's threshold. Tightening a budget is then a visible diff and loosening one needs a reason in review. -- Trim the released ndjson traces from `rt-audio-traces-2026-09-06` into a - fixture and replay them too, so a real recorded arrival pattern is graded - next to the synthetic profiles. +- Record real arrival traces (the #3477 release traces are gone), trim them + into a fixture, and replay them too, so a real recorded arrival pattern is + graded next to the synthetic profiles. The jitter target's watch quest + replays the same fixture. - Add the lane to `nightly.yml`, and extend its header comment with why this one is not a PR gate. diff --git a/quest/m1/audio-quality-harness/native.md b/quest/m0/audio-quality-harness/native.md similarity index 97% rename from quest/m1/audio-quality-harness/native.md rename to quest/m0/audio-quality-harness/native.md index 07970c1e82..a614645dd3 100644 --- a/quest/m1/audio-quality-harness/native.md +++ b/quest/m0/audio-quality-harness/native.md @@ -42,5 +42,5 @@ difference in totals alone proves nothing about the estimator. ## Required -- [Browser](/quest/m1/audio-quality-harness/browser.md) - defines the metric schema, the budget file, and the extracted shaper +- [Browser](/quest/m0/audio-quality-harness/browser.md) - defines the metric schema, the budget file, and the extracted shaper - [Native jitter target](/quest/m0/audio-jitter-target/native.md) - the estimator this lane grades and compares against the browser; without it there is no target series and the budgets would be set against a playout path that holds nothing diff --git a/quest/m0/plan-av-clock.md b/quest/m0/plan-av-clock.md index ca329d397b..22f14a12c6 100644 --- a/quest/m0/plan-av-clock.md +++ b/quest/m0/plan-av-clock.md @@ -39,6 +39,15 @@ Recommendations for the implementation: - The text renderer is the third track: it reads `sync.now()` (`js/watch/src/text/renderer.ts:261`) to drive the cue clock and prune cues at `:263-265`. +- Close the player gaps [#4170](https://github.com/moq-dev/moq/pull/4170) + left, since the handles own them. `Sync.received` only ever lowers its + reference, so after the earliest subscribed track leaves, playback stays + anchored to it; a track's handle going away must release its part of the + reference, the same expiry the jitter target's arrival minimum has. Text + renditions register no floor with `Sync`, so their catalog `delay` is + ignored; the text handle registers one like audio and video. The MSF + catalog schema (`js/msf/src/catalog.ts`) accepts a negative `delay` and + folds it into absent; refuse it on decode instead. ## Required diff --git a/quest/m1/README.md b/quest/m1/README.md index 0855737150..7a2e5b3b87 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -17,6 +17,7 @@ transport, benchmark tooling); worktrees isolate commits, not semantics. ## Quests +- [Drill sensitivity](/quest/m1/drill-sensitivity.md) - the nightly drill-sensitivity job passes: the subscriber-leaks-broadcasts mutation applies to the current lite subscriber again - [BBR classic ECN](/quest/m1/bbr-classic-ecn.md) - Startup and bandwidth probing respond to CE marks before the bottleneck drops packets - [lite-07 count settle](/quest/m1/lite-count-settle.md) - moq-lite-07 subscribers stop waiting for a subscription's tail once SUBSCRIBE_END's stream count is reached - [Dropped sources](/quest/m1/dropped-sources.md) - track consumers see the producer's real error on every end path, never `Dropped` @@ -41,6 +42,36 @@ transport, benchmark tooling); worktrees isolate commits, not semantics. - [Bindings caught up](/quest/m1/announce-live-bindings.md) - moq-ffi, libmoq, and every wrapper yield the same flat announce event, `Live` included - [Optional max age](/quest/m1/ietf-max-age.md) - max age is optional, set only by the publisher, and crosses moq-transport as MAX_CACHE_DURATION - [IETF announce count](/quest/m1/ietf-announce-count.md) - an opt-in moq-transport extension carries the replay count, so IETF announce consumers go live without a timer +- [kio waiter overflow](/quest/m1/kio-waiter-lost.md) - a retained `Waiter` past 8 lists stops adding a duplicate entry to lists it already recorded +- [moqsink keyframe latch](/quest/m1/moqsink-keyframe-latch.md) - a header-only buffer after a break no longer permanently invalidates a moqsink video pad +- [Capture re-anchor](/quest/m1/capture-reanchor.md) - a repeating or restarting device clock never rewinds native capture during a fast backlog drain +- [Splice edge cases](/quest/m1/splice-edges.md) - an unstamped successor, a pruned segment's boundary group, and a warm head during a takeover are each handled correctly +- [Track tail hardening](/quest/m1/track-tail-hardening.md) - Rust and JS wait out a track's tail by the same rules, with the known hang, count, truncation, grace, and memory holes closed +- [Session death parity](/quest/m1/session-death.md) - a local close ends tracks cleanly in both languages, and JS group readers see the session's error on session death +- [JS scoped routes](/quest/m1/js-scoped-routes.md) - a scoped JS origin reader announces the preferred route among those in its scope, like Rust, with the fan-out benchmarked +- [JS subtree at max depth](/quest/m1/js-pattern-depth.md) - `Pattern.subtree` returns the literal path at 32 segments like Rust, pinned by a shared pattern.json vector +- [moq play decode schedule](/quest/m1/play-decode-schedule.md) - `moq play` video keeps valid pictures across rewinds, reordering deeper than 100 ms, and decoder batches larger than three +- [moqsrc stop](/quest/m1/moqsrc-stop.md) - moqsrc's stop blocks until its session ends, without deadlocking on a blocked pad push +- [More tests under load](/quest/m1/test-flakes-2.md) - the second round of load-only failures, fixed at the cause +- [UnknownSession log flood](/quest/m1/unknown-session-logs.md) - streams reset before their WebTransport header stop being reported as UnknownSession at WARN +- [Merge queue](/quest/m1/merge-queue.md) - the required checks run on `merge_group`, so a stale green check can no longer break main +- [Wire compatibility](/quest/m1/wire-compat.md) - a nightly run tests this checkout against the last published release for tokens, session wire, and catalog/container +- [Accept-side flags](/quest/m1/cli-given-flags.md) - dial-only and local verbs refuse every `--listen-*` flag instead of ignoring it +- [JS catalog path](/quest/m1/js-catalog-path.md) - `@moq/net` broadcast consumers expose their path and `Catalog.watch` rejects escaping references, like Rust +- [Full codec string](/quest/m1/publish-codec-string.md) - browser-published video carries the encoder's full RFC 6381 codec string, so native players decode it +- [TS export jitter](/quest/m1/ts-export-jitter.md) - the video reorder bound follows later catalogs and observed reordering, so a late B-frame never reorders TS output +- [TS import shared shift](/quest/m1/ts-import-shared-shift.md) - unflagged loop wraps move audio and video by one shift, so A/V sync holds across wraps +- [PipeWire duplicate cameras](/quest/m1/pipewire-dup-cameras.md) - a webcam lists once with PipeWire enabled +- [Catalog wall clock](/quest/m1/catalog-wall-clock.md) - `Clock::wall_clock` keeps the catalog's full precision instead of truncating to milliseconds +- [Capture control](/quest/m1/capture-control.md) - on dev, `encode::Capture` replaces `CaptureOptions`, an unsupported `cut()` errors, and dropping the last `Control` cancels in-flight opens +- [Video surface](/quest/m1/video-surface.md) - on dev, moq-ffi's `native` becomes `surface`, refused on platforms with no surface +- [HLS discontinuity sequence](/quest/m1/hls-discontinuity-sequence.md) - on dev, `Segment::discontinuity` is the absolute sequence, so every cursor agrees +- [Strict Redirect::resolve](/quest/m1/redirect-resolve.md) - on dev, `Redirect::resolve` can no longer quietly turn a refused redirect into a redial +- [RTMP TLS only](/quest/m1/rtmp-tls-only.md) - an RTMP listener configured for TLS can refuse plaintext instead of sniffing and serving it +- [HLS linger](/quest/m1/hls-linger.md) - `moq_hls::Server` serves an ended broadcast for its playlist window plus grace, so the moq.pro edge drops its own pool +- [Gateway live clock](/quest/m1/gateway-live-clock.md) - moq-srt, moq-rtmp, and HLS import publish on the broadcast clock, so encoder reconnects don't restart timestamps +- [iroh versions](/quest/m1/iroh-lite-wip.md) - `iroh://` negotiates the configured versions, so `moq-lite-07-wip` can be opted into +- [Go and Dart doc samples](/quest/m1/doc-samples-go-dart.md) - Go and Dart doc samples compile against their wrappers - [Data jitter](/quest/m1/data-jitter.md) - JSON and binary tracks with a capture time advertise a detected `delay` and `jitter` - [Data capture in bindings](/quest/m1/data-capture-bindings.md) - moq-ffi and every wrapper pass a data frame's capture time, and the JSON window producer takes one @@ -92,7 +123,6 @@ transport, benchmark tooling); worktrees isolate commits, not semantics. - [Perf](/quest/m1/perf/README.md) - eliminate measured hot-path costs across moq-uring, kio, and the moq-net model - [#2924](/quest/m1/2924-moq-relay-tls-rotation-is-not-atomic-across-thread-per.md) - every listener on both runtimes shares one reloadable served identity, so rotation is atomic and generate works with workers - [#2964](/quest/m1/2964-quic-workers-dropping-one-split-server-resizes-the.md) - integrate the dev worker owner with hardened socket-group formation -- [Audio quality harness](/quest/m1/audio-quality-harness/README.md) - a playout latency regression fails a run instead of arriving as a bug report - [Benchmark regressions in CI](/quest/m1/bench-ci.md) - PRs get a non-blocking comparison of the Criterion benches they affect, and a nightly trend on main alerts on regressions - [Benchmark comparisons](/quest/m1/performance-comparisons.md) - retained evidence, repeated paired runs, and uncertainty for performance claims - [#3126](/quest/m1/3126-moq-bench-every-readme-example-fails-to-parse-and.md) - moq-bench reports per-interval latency percentiles so the ramp leaves the steady state diff --git a/quest/m1/archive/README.md b/quest/m1/archive/README.md index 8c388a19bc..b772e4d8f2 100644 --- a/quest/m1/archive/README.md +++ b/quest/m1/archive/README.md @@ -110,6 +110,7 @@ owned by that prerequisite, not duplicated in archive storage. - [Browser archive](/quest/m1/archive/browser.md) - the same contract for browser-published broadcasts - [Offline archive HLS](/quest/m1/archive/hls.md) - render playlists from the archive timeline and fetch segment media lazily - [DVR rewind](/quest/m1/archive/dvr.md) - seek through a bounded archive and return to live playback +- [Enrollment flake](/quest/m1/archive/enrollment-flake.md) - the opening-snapshot test waits for real enrollment, not the `.info` file - [Archive proof](/quest/m1/archive/proof.md) - prove persistence ordering, selective reads, exact FETCH replay, and timeline-only HLS generation ## Related diff --git a/quest/m1/archive/enrollment-flake.md b/quest/m1/archive/enrollment-flake.md new file mode 100644 index 0000000000..4395024703 --- /dev/null +++ b/quest/m1/archive/enrollment-flake.md @@ -0,0 +1,26 @@ +# [XS] The opening-snapshot test waits for real enrollment + +## Goal + +`archive::tests::an_opening_snapshot_records_every_rendition` in +`rs/moq-cli/src/archive.rs` passes under load. It fails with "writer closed" +because it treats a rendition's `.info` file as proof the export enrolled +that track, then finishes the tracks and the catalog; under load the writer +can still be subscribing when the tracks end. Seen while landing +[#4169](https://github.com/moq-dev/moq/pull/4169). + +## Plan + +- Wait on the signal enrollment actually produces (the subscription reaching + the publisher, or an explicit event from the exporter), not a file that + appears earlier. No longer timeout and no retry. +- If the exporter can genuinely lose a rendition that ends right after it + appears in the catalog, that is a bug in the exporter, not the test; fix it + there. + +Public API: none. Wire: none. + +## Related + +- [More tests hold up under load](/quest/m1/test-flakes-2.md) - the same + round of load-only failures on `main` diff --git a/quest/m1/audio-codecs/README.md b/quest/m1/audio-codecs/README.md index 372157036a..2a23be20c5 100644 --- a/quest/m1/audio-codecs/README.md +++ b/quest/m1/audio-codecs/README.md @@ -43,6 +43,7 @@ ready now. ## Quests +- [Named codecs](/quest/m1/audio-codecs/named-codecs.md) - `decode::Kind::Named` keeps the published codec names and picks backends internally; must land before the line merges - [HE-AAC refusal](/quest/m1/audio-codecs/he-aac-refusal.md) - implicit-SBR HE-AAC over TS is refused instead of half-decoded as the LC core - [AAC PCE](/quest/m1/audio-codecs/aac-pce.md) - a channel_config of 0 parses the program config element instead of guessing stereo - [Layout](/quest/m1/audio-codecs/layout.md) - the settled `Layout` carries up to 7.1 through decode, resample, playback, and the FFI diff --git a/quest/m1/audio-codecs/encode-audiotoolbox.md b/quest/m1/audio-codecs/encode-audiotoolbox.md index 0f86953835..401142b291 100644 --- a/quest/m1/audio-codecs/encode-audiotoolbox.md +++ b/quest/m1/audio-codecs/encode-audiotoolbox.md @@ -18,6 +18,11 @@ the encode seam as the platform candidate on macOS and iOS. - Regression: a stereo and a 5.1 encode round-trip through the AudioToolbox decoder and through symphonia (stereo only), with timestamps continuous across the priming. +- Decided in [#4183](https://github.com/moq-dev/moq/pull/4183): the uniffi + `MoqAudioEncoderOutput::frame_duration_us` default moves from 20000 to 0 + (the codec's own frame) with this quest, so `aac()` works without an + explicit 0. Changing a published binding default is a break, so that + change targets `dev`. ## Required diff --git a/quest/m1/audio-codecs/named-codecs.md b/quest/m1/audio-codecs/named-codecs.md new file mode 100644 index 0000000000..4bba08d514 --- /dev/null +++ b/quest/m1/audio-codecs/named-codecs.md @@ -0,0 +1,27 @@ +# [XS] Kind::Named keeps its codec names + +## Goal + +`moq_audio::decode::Kind::Named` accepts what published moq-audio 0.1.6 +accepts, the codec names (`"opus"`, `"aac"`, `"pcm"`), so the line can merge +to `main` without breaking callers. [#4131](https://github.com/moq-dev/moq/pull/4131) +switched it to backend names (`"libopus"`, `"symphonia"`, `"pcm"`) on the line +branch, which turns every existing `Named("opus")` or `Named("aac")` into +`Error::Unsupported` after an upgrade. This must land before the line PR +[#4081](https://github.com/moq-dev/moq/pull/4081) merges to `main`. + +## Plan + +- Decided: keep codec names; backends are picked internally. Which backend + decodes AAC (AudioToolbox, Media Foundation, symphonia) is the library's + choice through `Auto` and `Software`, not a string a caller has to know and + that changes per platform. This reverses #4131's reasoning on purpose: the + published contract wins over matching moq-video's backend naming. +- Make `encode::Kind` read the same way. It is unpublished, so drop `Named` + there or give it the same codec meaning, whichever leaves the two enums + mirrored; don't keep backend names on one side only. +- Tests that forced a backend by name move to a crate-private seam. +- Docs and the `Kind` doc comments list the codec names. + +Public API: `decode::Kind::Named` returns to its published meaning, so the +line stays additive on `main`. Wire: none. diff --git a/quest/m1/auth/README.md b/quest/m1/auth/README.md index 6ec02ba9d1..2dc399d5df 100644 --- a/quest/m1/auth/README.md +++ b/quest/m1/auth/README.md @@ -96,6 +96,8 @@ existing lite-06 ALPN. each lite-06 cell's grant and that a publish outside it fails loud - [Unauthorized reset](/quest/m1/auth/unauthorized.md) - a subscription that loses access resets with a dedicated UNAUTHORIZED stream code +- [AUTH_OK preflight](/quest/m1/auth/auth-ok-preflight.md) - an unencodable IETF grant answers NOT_SUPPORTED with nothing written, as JS already does +- [AUTH endings](/quest/m1/auth/error-codes.md) - an out-of-range AUTH_ERROR code is refused, and both sides settle and recompute grants when a stream ends - [Origin narrowing](/quest/m1/auth/narrowing.md) - a live grant narrows in place: subscriptions outside it reset, publishes outside it abort, and relay revalidation stops closing the session diff --git a/quest/m1/auth/auth-ok-preflight.md b/quest/m1/auth/auth-ok-preflight.md new file mode 100644 index 0000000000..eecead51ac --- /dev/null +++ b/quest/m1/auth/auth-ok-preflight.md @@ -0,0 +1,24 @@ +# [XS] An AUTH_OK that cannot be sent is refused, not half-written + +## Goal + +When the Rust IETF acceptor's grant cannot be encoded as one AUTH_OK for any +reason, it answers `AUTH_ERROR { NOT_SUPPORTED }` and nothing of the AUTH_OK +reaches the wire, the same as JS. Today the preflight in +`rs/moq-net/src/ietf/auth.rs` (`serve_issue`) catches only +`EncodeError::Unsupported`, so a prefix grant too large for the `u16` message +size falls through to `encode_message`, which fails mid-write and ends the +stream with a transport error instead of a refusal the presenter can read. +Found in review of [#4124](https://github.com/moq-dev/moq/pull/4124). + +## Plan + +- Refuse on any sizing error, not only `Unsupported`, including the message + size ceiling the writer enforces. Never trim or widen the grant to make it + fit: withholding it is the only safe answer. +- Check whether the lite AUTH_OK path has the same gap and fix both if so. +- Regression test mirroring JS's "a grant too large for one message is + refused before anything is written": the presenter sees `Unsupported` and + no AUTH_OK bytes were written. + +Public API: none. Wire: none. diff --git a/quest/m1/auth/error-codes.md b/quest/m1/auth/error-codes.md new file mode 100644 index 0000000000..76dfcbe1f5 --- /dev/null +++ b/quest/m1/auth/error-codes.md @@ -0,0 +1,36 @@ +# [S] AUTH endings are exact on both sides + +## Goal + +The loose ends Codex left on [#4062](https://github.com/moq-dev/moq/pull/4062) +are closed, so every way an AUTH stream ends reports what actually happened: + +- A lite AUTH_ERROR whose code does not fit a `u32` is a protocol violation. + Today `rs/moq-net/src/lite/session.rs` saturates it with + `u32::try_from(refused.code).unwrap_or(u32::MAX)`, so distinct peer codes + collapse into one and the app sees a code the peer never sent. +- JS `AuthSession.close()` in `js/net/src/auth_session.ts` clears its tokens + but never recomputes the union, so a retained `auth.grant` keeps reporting + a live grant after the session closed. Rust already clears it. +- A JS presenter whose acceptor ends the grant with a clean FIN exits the read + loop without closing its own write half, unlike the AUTH_ERROR branch, so + the acceptor's `Issued.closed` stays pending until the session ends. +- A Rust `Issued::closed()` never resolves if the session drops the task + serving that token (`AuthServe` in the lite publisher, `Serve` in + `ietf/auth.rs`): only the task's own completion records `issue.peer`. + +## Plan + +- Fail loud on the unrepresentable code rather than widen the error type: + the codes AUTH_ERROR carries are session codes, which are `u32` everywhere + else. +- Settle a dropped serve task from a drop path (a guard that records the + session's error on `Issue` and wakes waiters), so no exit path can forget + it. +- One regression test per item, each failing without its fix. + +The fifth deferred item, a grant recheck after async origin resolution, +belongs to [Origin narrowing](/quest/m1/auth/narrowing.md) with the other +watcher races. + +Public API: none. Wire: none. diff --git a/quest/m1/auth/narrowing.md b/quest/m1/auth/narrowing.md index 0be54eceec..9a909bfbe5 100644 --- a/quest/m1/auth/narrowing.md +++ b/quest/m1/auth/narrowing.md @@ -29,6 +29,17 @@ A narrowing always succeeds. A changed root still closes the session. handle and cursor (`OriginScope` in `rs/moq-net/src/model/origin.rs`), so narrowing needs shared state. If handles need a tree to share it, keep the ceiling on the grant node itself (an `Arc` shared by clones and children). +- Close the revocation races Codex found on + [#4179](https://github.com/moq-dev/moq/pull/4179), which the same watcher + mechanism owns: an in-flight lite FETCH keeps serving after its path is + revoked (Rust and JS check only at accept, and Rust's dropped fetches reset + `CANCELLED`, not `UNAUTHORIZED`); a Rust lite subscription still in + `Establish` when the grant shrinks is dropped with `CANCELLED` instead of + aborted `UNAUTHORIZED`; and the JS lite publisher and subscriber arm their + grant watchers only after an await (`demand()`, `#openSubscribe`), and + `Getter.subscribe` does not replay, so a shrink during setup is missed for + good. Every request holds one watcher from its first check to its end, and + rechecks when the watcher is armed. - Publish side: routes and broadcasts the session published outside the narrowed grant abort, so consumers see `Unauthorized` just as the subscribe side does. Draining is not an option: a group may stay open as long as its diff --git a/quest/m1/auth/request-token.md b/quest/m1/auth/request-token.md index 34f77640e1..fa93750186 100644 --- a/quest/m1/auth/request-token.md +++ b/quest/m1/auth/request-token.md @@ -41,28 +41,35 @@ on the unknown key, and the legacy drafts silently ignore it. alone ends with `EXPIRED_AUTH_TOKEN` or `UNAUTHORIZED`. A session grant that shrinks cancels the requests it covered, as for any request, through [Origin narrowing](/quest/m1/auth/narrowing.md). -- Relay: each such request attaches its own lease through the - `Client::attach` path [Relay tokens](/quest/m1/auth/relay-refresh.md) - builds, once per request, with no sharing across requests carrying the - same bytes; the lease is dropped with the request. The request is resolved - against the origin with the path checked against that lease's grant, not - through the session's scoped origin handle. +- Relay: each such request gets its own lease from a per-request call on + `moq_auth::Client`, not the `Client::attach` that [Relay + tokens](/quest/m1/auth/relay-refresh.md) builds. `attach` connects with the + connection's id, and the auth server treats that as one more grant on the + session that never POSTs `end`, so a request token would widen the whole + connection for its life. The per-request call carries the token in + `moq_auth::Request.token` with its kind (a CAT reaches the CAT verifier, + not the JWT one) and the request's path, is never counted as a session + grant, and ends when the request ends. Two requests carrying the same bytes + get two leases. The request is resolved against the origin with the path + checked against that lease's grant, not through the session's scoped + origin handle. Name the call while implementing. - `js/net` mirrors the decode and the default refusal. - Docs: `doc/bin/relay/auth.md` states the order (session grant, then the request's token, then `UNAUTHORIZED`), that a request token covers only its request, and how a peer refreshes with REQUEST_UPDATE. - Tests: a SUBSCRIBE outside the session grant succeeds with a covering token and is refused `UNAUTHORIZED` without one; its token grants nothing to a - second SUBSCRIBE; a request inside the session grant never calls the + second SUBSCRIBE, and through the relay the auth server never counts it as + a session grant and sees its lease end with the request; a request inside the session grant never calls the verifier; a REQUEST_UPDATE token keeps a subscription alive past the old token's expiry; an expired request token ends only that request; with no consumer a token-bearing request is refused `Unsupported`; one legacy and one strict draft, Rust and JS. Public API: additive on `moq_net::auth::Request` (the request it belongs -to). Wire: none new; the parameter already exists in every supported draft. +to) and on `moq_auth::Client` (the per-request lease). Wire: none new; the parameter already exists in every supported draft. ## Required -- [Relay tokens](/quest/m1/auth/relay-refresh.md) - supplies the per-token - lease and `Client::attach` path each request uses +- [Relay tokens](/quest/m1/auth/relay-refresh.md) - supplies the lease + revalidation the per-request lease reuses diff --git a/quest/m1/auth/token-in-band.md b/quest/m1/auth/token-in-band.md index ee68cf6fb7..51aa256231 100644 --- a/quest/m1/auth/token-in-band.md +++ b/quest/m1/auth/token-in-band.md @@ -44,7 +44,12 @@ AUTH can carry the full grant once the pattern-interest prerequisite lands. AUTH-capable session, so no token is ever granted twice. On moq-transport the first token also rides the AUTHORIZATION TOKEN setup option (`ietf::token::into_setup`, `USE_VALUE`, token type 0), which - scopes at accept the way the URL does. + scopes at accept the way the URL does. While the URL also carries it, the + auth server sees the same credential twice. Decided by the maintainer (on + #4211): `moq auth serve` admits a SETUP token equal to the `?jwt=` value + and refuses only two different credentials. #4278 shipped refusing both + (`serve::Refusal::TwoTokens`, pinned by a test), so this quest changes that + and its test. Never send different values in the two places. - The relay admits on the URL, then widens. An anonymous connection today is admitted with the public grant when one is configured and refused otherwise; with this quest a connection with no URL credential and no @@ -62,7 +67,9 @@ AUTH can carry the full grant once the pattern-interest prerequisite lands. - Tests: a client with a configured token against an AUTH-capable relay is scoped exactly as the URL variant and the URL carries the token only while an old version is offered; the same client against a lite-05 relay still - authenticates through the URL; two configured tokens union; a client with + authenticates through the URL; a moq-transport client offering a draft + without AUTH, carrying the token in both the URL and the setup option, is + admitted by `moq auth serve`; two configured tokens union; a client with in-band tokens only and no public grant is admitted, and one that presents nothing is refused at the deadline; the cross-language harness runs with tokens configured. diff --git a/quest/m1/capture-control.md b/quest/m1/capture-control.md new file mode 100644 index 0000000000..0072552113 --- /dev/null +++ b/quest/m1/capture-control.md @@ -0,0 +1,43 @@ +# [S] Capture Control: settled name, loud cut, prompt cancel + +## Goal + +The capture handles from [#4184](https://github.com/moq-dev/moq/pull/4184), +on `dev` only, get their final shape before release: + +- `encode::CaptureOptions` is `encode::Capture` in both moq-audio and + moq-video. +- `Control::cut()` on video tells the caller when the backend cannot force a + keyframe. Today the driver logs one warning on `CutUnsupported` and keeps + the GOP cadence, so a recording or resume boundary silently never appears. +- Dropping the last `Control` ends the driver promptly, including while the + startup probe, `capture::open`, `Sink::open`, or an encode is in flight. + Today those awaits never see the handle close, so a camera or permission + prompt can outlive its owner. + +## Plan + +Decided: + +- Rename to `encode::Capture`, which reads as the capture half next to + `encode::Options`. Update moq-cli and the docs; moq-ffi and libmoq do not + call the capture paths, so no binding mirror exists today. +- `cut()` fails loud with an error rather than logging. The encoder is opened + lazily, so the handle may not know yet; the probe already opens one, which + is one place to learn it early. Choose between `cut()` returning + `Result` (refusing once the backend is known) and the driver ending with + `CutUnsupported`, and record the choice here. +- Race every await in the driver against the controls closing, rather than + only the idle wait, so the probe and the demand-driven opens both cancel. + +This is a `dev` break layered on #4184; land it on `dev` before the release +that first publishes these handles. + +Tests: a backend without forced keyframes surfaces `CutUnsupported` to the +caller; dropping the last `Control` during a slow fake open or probe returns +from `Driver::run` without finishing the open. + +## Related + +- [Video keyframe flag](/quest/m1/video-keyframe-flag.md) - the same cut throttle, counting cadence keyframes +- [Capture clock source](/quest/m2/capture-clock-source.md) - drops the `clock` field from these options diff --git a/quest/m1/capture-reanchor.md b/quest/m1/capture-reanchor.md new file mode 100644 index 0000000000..df3ebb1fb5 --- /dev/null +++ b/quest/m1/capture-reanchor.md @@ -0,0 +1,21 @@ +# [XS] Native capture re-anchors above its last timestamp + +## Goal + +A device clock that repeats or restarts never rewinds native video capture, +even while the pump drains a backlog faster than real time. `FrameChannel::push_native` +in `rs/moq-video/src/capture/channel.rs` re-anchors such a frame to its +arrival time ([#4125](https://github.com/moq-dev/moq/pull/4125)). During a +fast drain the previous frame's mapped timestamp can sit ahead of arrival, so +the re-anchor lands below it: source 0 and 40 ms arriving 1 ms apart publish +near 0 and 40 ms, and an immediate reset to zero publishes near 2 ms. Past a +closed group that is `TimestampRewind`, which stops capture. + +## Plan + +Remember the last mapped timestamp and floor the re-anchor strictly above it, +including the fallback when the checked arithmetic fails. The mapping keeps +advancing with the device clock from there. + +Unit regression in `channel.rs`'s mapping tests: two frames drained 1 ms apart +with a 40 ms source step, then a source reset, maps above the second frame. diff --git a/quest/m1/catalog-wall-clock.md b/quest/m1/catalog-wall-clock.md new file mode 100644 index 0000000000..1b25deeeef --- /dev/null +++ b/quest/m1/catalog-wall-clock.md @@ -0,0 +1,28 @@ +# [XS] Catalog wall clock keeps sub-millisecond precision + +## Goal + +`hang::catalog::Clock::wall_clock` returns the wall time at the precision the +catalog carries, not truncated to milliseconds. The wire field is `{ wall, +timescale }` with a `u32` timescale defaulting to microseconds, and `pts` +arrives at its own timescale, but `wall_clock` divides down to Unix +milliseconds before building the `SystemTime`, which holds nanoseconds. The +capture clock fixtures from [#4125](https://github.com/moq-dev/moq/pull/4125) +assert at millisecond precision because of it. + +## Plan + +- Rust: compute the offset from the moq epoch in nanoseconds (or as a + `Duration` from whole seconds plus the remainder at the clock's timescale) + and add it to `UNIX_EPOCH + MOQ_EPOCH_UNIX_MILLIS`. Keep the existing + overflow and JSON-safe range checks. Tighten the fixtures that currently + assert milliseconds. +- JS: `wallClockTime` in `js/hang/src/catalog/clock.ts` returns a `Date`, + which only holds milliseconds. Leave its return type alone unless a caller + needs more; note the platform limit in its doc so the two sides are not + mistaken for a mismatch. +- Callers that format the value (HLS `EXT-X-PROGRAM-DATE-TIME`, DASH + `availabilityStartTime`) choose their own output precision; check none + relied on the truncation. + +Public API and wire: none. diff --git a/quest/m1/cli-given-flags.md b/quest/m1/cli-given-flags.md new file mode 100644 index 0000000000..2b270033e9 --- /dev/null +++ b/quest/m1/cli-given-flags.md @@ -0,0 +1,24 @@ +# [XS] Dial-only and local verbs refuse every accept-side flag + +## Goal + +`moq fetch`, `moq ls` (on the [CLI inspect](/quest/m1/cli-inspect/README.md) +line), and the local verbs behind `Invocation::reject` refuse any listener +flag they would never use, as they already do for `--listen` and the cluster +flags. Today `MoqSide::given()` in `rs/moq-cli/src/args.rs` lists only some +of them, so `--listen-version`, `--listen-tls-*`, `--listen-preferred-*`, and +`--listen-quic-lb-*` are silently ignored. Codex found it on +[#4121](https://github.com/moq-dev/moq/pull/4121). + +## Plan + +- Cover every accept-side flag. Prefer a list that cannot fall behind the + server config, such as deriving it from the parsed partial or a test that + walks every `--listen-*` flag clap knows and asserts `given()` reports it, + over extending the hand-written array again. +- While there, check whether the local verbs also ignore `--connect-*` + flags; `reject` is meant to refuse the whole MoQ side. +- Test that each verb refuses a representative flag from every family, + naming it. + +Public API: none (CLI refuses input it used to ignore). Wire: none. diff --git a/quest/m1/cpp/README.md b/quest/m1/cpp/README.md index a48bae4194..75b88fb0ec 100644 --- a/quest/m1/cpp/README.md +++ b/quest/m1/cpp/README.md @@ -47,10 +47,28 @@ remote we own, the latter two fetching the prebuilt tarball so consumers never need a Rust toolchain or the bindgen fork. vcpkg lands first; the Conan recipe reads the same release manifest so a release bumps both. +Confirmed in [#4100](https://github.com/moq-dev/moq/pull/4100): + +- The fork's base is LiveKit's PR #1 (`uniffi-0.31-async`), not PR #5, + which runs a worker thread per in-flight future and has no `then()` or + async callback interfaces, both of which OBS needs. +- Fork tags keep upstream's `v+v` scheme with a + `-kixelated.N` pre-release (`v0.11.0-kixelated.1+v0.32.2`), matching the + Dart fork, so they never collide with an upstream tag. +- Cancelling abandons the future rather than delivering an error: a generic + `E` has no cancelled variant, and a `std::variant` would + burden every call site. +- Callback interfaces are refused under `error_style = "expected"` until a + consumer needs one; their bridge is built on `std::exception_ptr`. +- MSVC is covered by the post-merge nightly, not a branch dispatch: branches + never dispatch the nightly. + ## Quests - [Generator](/quest/m1/cpp/generator.md) - the uniffi 0.32 C++ generator with futures and expected-style errors, pinned and generating `cpp/ffi` in CI - [Package](/quest/m1/cpp/package.md) - the `cpp/moq` wrapper, CMake package, release tarball, interop client, and docs +- [Cancel](/quest/m1/cpp/cancel.md) - a cancelled or consumed future reports `valid() == false`, like `std::future`, and a read of it aborts with a message naming the misuse +- [C++ standard](/quest/m1/cpp/cxx-standard.md) - a consumer that sets C++23 only on its own target links the package - [OBS migration](/quest/m1/cpp/obs.md) - the OBS plugin moves from libmoq handles and trampolines to the generated C++ ## Related diff --git a/quest/m1/cpp/cancel.md b/quest/m1/cpp/cancel.md new file mode 100644 index 0000000000..a3974224bb --- /dev/null +++ b/quest/m1/cpp/cancel.md @@ -0,0 +1,34 @@ +# [XS] A cancelled future says so before it is read + +## Goal + +A caller can tell a cancelled or consumed future is dead before reading it, +and a read of one fails with a message naming the misuse. Today the generated +`uniffi::Future` declares `void cancel() noexcept` on an lvalue, so +`future.cancel(); future.get();` compiles and aborts with nothing to check +first; `cpp/moq/README.md` documents the abort instead of giving callers a +guard. + +## Plan + +- Mirror `std::future`, the idiom C++ callers know: once cancelled, consumed + by `then()`, or moved from, a future is invalid and `valid()` returns false. + Calling `get()` on an invalid future is a precondition violation, as it is + for `std::future`, and it aborts with a message naming `cancel`/`then`/move + rather than a bare abort. Decided with the maintainer during the #4307 + review. +- Rejected: `&&`-qualifying `cancel()`, since `std::move(f).cancel(); f.get();` + still compiles; and `get()` returning an error, since expected mode has no + error value for it: generic `E` has no invalid-state variant, and the line + README already rejects `std::variant`. +- Decided in [#4100](https://github.com/moq-dev/moq/pull/4100): cancel + abandons the future, so no continuation runs. That still holds. +- The `Future` template lives in the kixelated `uniffi-bindgen-cpp` fork, so + this is a fork change and a new `-kixelated.N` tag, then the pin bump in + `flake.nix` and every place its comment lists. +- Update the README to point callers at `valid()`, and check it in the + in-tree callers that can hold a cancelled future (`cpp/moq`, the probe, + `cpp/obs`). Test `valid()` after `cancel()`, after `then()`, and after a + move. + +Public API: additive `valid()` on the unreleased C++ package. Wire: none. diff --git a/quest/m1/cpp/cxx-standard.md b/quest/m1/cpp/cxx-standard.md new file mode 100644 index 0000000000..e051fadd9f --- /dev/null +++ b/quest/m1/cpp/cxx-standard.md @@ -0,0 +1,29 @@ +# [XS] A C++23 consumer links the package + +## Goal + +A consumer that raises the standard only on its own target +(`target_compile_features(app PRIVATE cxx_std_23)`) either links against the +`moq-cpp` package or is told clearly how to configure it. Today it gets link +errors: that requirement does not flow backward into the separate `moq-cpp` +static library, which compiles `moq.cpp` at `cxx_std_17` and so picks +`tl::expected` while the app's `` picks `std::expected`. The ABI +guard turning that into a link error is correct; the surprise is not. Codex +found it on [#4187](https://github.com/moq-dev/moq/pull/4187). + +## Plan + +- Fix the misleading comment in `cpp/moq/cmake/moq-cpp-config.cmake.in` + (and the matching one in `cpp/moq/CMakeLists.txt`): the bindings compile + with the project's standard (`CMAKE_CXX_STANDARD`) or C++17, not with + whatever the including target asks for. +- Then choose the smallest fix that makes the C++23 path work: document + `CMAKE_CXX_STANDARD` (what the probe does) in `cpp/moq/README.md` and the + C++ docs page, or add a package option that sets the standard `moq-cpp` + compiles with. Prefer documentation unless a consumer (OBS, Unreal) needs + the option. +- Test the documented configuration: the package test already builds at + several standards; add one that builds the app at C++23 the documented + way. + +Public API: none, or one CMake option. Wire: none. diff --git a/quest/m1/doc-samples-go-dart.md b/quest/m1/doc-samples-go-dart.md new file mode 100644 index 0000000000..af68aa5f44 --- /dev/null +++ b/quest/m1/doc-samples-go-dart.md @@ -0,0 +1,30 @@ +# [S] Go and Dart doc samples compile against their wrappers + +## Goal + +The Go and Dart pages in `doc/lib/go/` and `doc/lib/dart/` are checked the +way [#4049](https://github.com/moq-dev/moq/pull/4049) checks Python, Kotlin, +Swift, and C: every fenced sample is extracted by `doc/lib/samples.sh` and +type-checked against the wrapper it documents (`go vet` for `go/wrapper`, +`dart analyze` for `dart/moq`), so a renamed wrapper method breaks the +language's check instead of silently breaking the docs. Today `samples.sh` +knows neither language. + +## Plan + +- Add `go` and `dart` to `samples.sh` with the same shape: one function per + sample, imports hoisted, inputs a sample leaves undefined (`client`, + `opusInit`, `pts`) supplied by a prelude the caller compiles alongside. +- Go is stricter than the others: a file needs a `package` clause, imports + come as single lines or a parenthesized block, and unused locals and + imports are compile errors. Handle what the samples actually use; prefer + adjusting a sample so it reads naturally and still compiles over growing + the extractor. +- Wire each into its language's `check` recipe (`go/justfile`, + `dart/justfile`) the way `py/justfile` and `rs/justfile` call it, and add + `doc/lib/samples.sh` and the doc page to the paths that trigger those + checks in CI. +- Prove it by renaming one wrapper method locally and watching each check + fail on the doc sample. + +Public API: none. Wire: none. diff --git a/quest/m1/drain/README.md b/quest/m1/drain/README.md index 96d9195bb8..619d820354 100644 --- a/quest/m1/drain/README.md +++ b/quest/m1/drain/README.md @@ -49,6 +49,7 @@ by the stop deadline and encoder reconnect. - [Client goaway](/quest/m1/drain/client-goaway.md) - the JavaScript client migrates on GOAWAY with a handover and the guarded redirect the Rust client already has, and the Rust drain path gets its regression test +- [JS GOAWAY requests](/quest/m1/drain/js-goaway-requests.md) - after GOAWAY the JS client opens no new request on the old session, like Rust ## Related diff --git a/quest/m1/drain/js-goaway-requests.md b/quest/m1/drain/js-goaway-requests.md new file mode 100644 index 0000000000..bf8de798b2 --- /dev/null +++ b/quest/m1/drain/js-goaway-requests.md @@ -0,0 +1,35 @@ +# [S] JS refuses new requests after GOAWAY + +## Goal + +Once a `@moq/net` session has received GOAWAY, it opens no new subscribe, +fetch, or announce-interest stream on that session, on lite and IETF, as +moq-net already refuses them with `Error::GoingAway`. Existing subscriptions +keep flowing until the handover ends, and a refused request is served by the +replacement session rather than failing the caller. + +## Plan + +The drain line's JS migration (https://github.com/moq-dev/moq/pull/4143) +keeps the old session serving after GOAWAY but left its subscribers unaware +of it, so during the handover an origin request can still open a stream on a +session the peer asked us to leave. The lite draft says the recipient must +not open new streams after GOAWAY, and a compliant draining relay rejects +them. Rust checks before every new open in both wires (`check_going_away` in +the lite subscriber, the `going_away` checks in the IETF one). + +Guidance: + +- The drain signal already reaches the connection wire view + (`wireOf(session).goaway`). Hand it to both subscribers and check it at + every open site, including the draft-14 to -16 adapter route. +- The refusal matters as much as the check: the origin should see it as this + route declining, so the request moves to the replacement's route (the + origin already keeps the outranked route until the new one answers). Match + what the Rust origin does with `GoingAway` rather than inventing a policy. +- Test on lite and IETF with the existing mock transports: after GOAWAY a new + request opens no stream on the old session and resolves on the new one, + while an existing subscription keeps receiving groups. + +No public API change is expected; the error is internal unless a caller can +observe it today. diff --git a/quest/m1/drill-sensitivity.md b/quest/m1/drill-sensitivity.md new file mode 100644 index 0000000000..5f2232c610 --- /dev/null +++ b/quest/m1/drill-sensitivity.md @@ -0,0 +1,22 @@ +# [XS] The subscriber-leaks-broadcasts mutation applies again + +## Goal + +The nightly `Tests (test drill-sensitivity)` job passes. It fails on +[run 36240326747](https://github.com/moq-dev/moq/actions/runs/36240326747) +because `test/drill/mutations/subscriber-leaks-broadcasts.patch` no longer +applies ("1 out of 2 hunks FAILED" on `rs/moq-net/src/lite/subscriber.rs`) +after a later change to the lite subscriber, so the +`cancel_under_backpressure_releases_the_reader` drill proves nothing. + +## Plan + +- Retarget the patch the way + [#3953](https://github.com/moq-dev/moq/pull/3953) did: find where the lite + subscriber now releases the broadcasts a session fed when it ends, and + remove that behavior again. Keep the failure message the patch declares, or + update it if the drill now fails with a different but still correct one. +- If the release moved somewhere a patch cannot remove cleanly, the drill may + be pointing at the wrong layer; say so rather than force a patch. +- Prove it with `just test drill-sensitivity subscriber-leaks-broadcasts`, and + run the other two mutations to confirm they still apply. diff --git a/quest/m1/dropped-sources.md b/quest/m1/dropped-sources.md index 9e29b46896..08a0ce43bb 100644 --- a/quest/m1/dropped-sources.md +++ b/quest/m1/dropped-sources.md @@ -17,6 +17,11 @@ end carries no cause since #4031, so only track errors are in scope. over a mock session, and map JS `PUBLISH_DONE` Unauthorized to #4179's shared error. Preserve causes at the source rather than remapping `Dropped` at consumers. +- The same goes for revocation. Leftovers from #4179's review: a bridged + revocation reaches Rust IETF subscribers as `PUBLISH_DONE` InternalError, a + JS relay reports a route revocation as INTERNAL_ERROR, and the bindings + neither document nor test `is_auth` for a stream-scoped Unauthorized. Each + should surface Unauthorized. Public API: none expected; error values consumers observe change. Wire: none. diff --git a/quest/m1/gateway-live-clock.md b/quest/m1/gateway-live-clock.md new file mode 100644 index 0000000000..ee39a7b1c4 --- /dev/null +++ b/quest/m1/gateway-live-clock.md @@ -0,0 +1,32 @@ +# [S] Gateways publish on the broadcast clock + +## Goal + +`moq-srt`, `moq-rtmp`, and HLS import map source timestamps onto the +catalog's broadcast clock, as `moq import` does since +[#4122](https://github.com/moq-dev/moq/pull/4122). Today only the CLI calls +`.live()`; the gateways publish the encoder's timestamps verbatim, so the +advertised wall time is wrong for a source whose PTS does not start near zero, +and an encoder reconnect that restarts its timestamps rewinds or is refused. + +## Plan + +Decided: each gateway opts into `live()`. The importer default stays +verbatim, since tests and callers that pin `Config::with_clock` rely on it. + +Guidance: + +- SRT: `rs/moq-srt/src/ts.rs` builds the `ts::Import`. RTMP: the server's + publish path and the pull path in `dial.rs` build `FlvImport`. +- HLS import (`rs/moq-hls/src/import.rs`) runs one fMP4 importer per + rendition, and `live()` gives each its own anchor, so renditions would map + their first frames to different instants. They share one source clock and + need one mapping: share the `Anchor` across the broadcast's importers, or + pick another way to anchor the playlist once. Audio and video in separate + HLS renditions are the case to test. +- A reconnect that reuses the broadcast is where this pays off; check each + gateway's reconnect keeps the same importer (and anchor) or deliberately + starts a new one. +- Tests per gateway: a source starting at a large PTS publishes near the + broadcast clock's now, and a restart to zero continues forward. +- Docs: the gateway pages under `doc/bin/` that describe timestamps. diff --git a/quest/m1/hls-discontinuity-sequence.md b/quest/m1/hls-discontinuity-sequence.md new file mode 100644 index 0000000000..a191a9e2aa --- /dev/null +++ b/quest/m1/hls-discontinuity-sequence.md @@ -0,0 +1,37 @@ +# [S] moq-hls segments carry the absolute discontinuity sequence + +## Goal + +Every `moq_hls::export::Segment` reports the discontinuity sequence it belongs +to, the value a recorder writes as `EXT-X-DISCONTINUITY-SEQUENCE`, so two +cursors on sibling renditions agree no matter when each was created. On `dev`, +`Segment::discontinuity` is a `u64` count of breaks since the cursor's +previous segment ([#4068](https://github.com/moq-dev/moq/pull/4068)). A cursor +created after its rendition rebinds starts its count from its own first row, +so it disagrees with a sibling on the baseline. + +## Plan + +Decided: + +- Report the absolute sequence the timeline fanout already stamps on each + row, instead of the difference between rows. A recorder writes + `EXT-X-DISCONTINUITY-SEQUENCE` from the first segment and an + `EXT-X-DISCONTINUITY` wherever the value changes. +- Land it on `dev` before the next moq-hls release, so it ships in the same + breaking release as the existing `bool` to `u64` change rather than + breaking the field twice. + +Guidance: + +- `Position` in `export/segments.rs` then only tracks `after`; the + skip/emit baseline logic goes away. Check the serve path's playlist + rendering (`rendition.rs`, `playlist.rs`) already derives its tags from the + same stamp. +- The sequence is absolute within one `Broadcaster`. Document what a recorder + should do when its broadcaster is rebuilt and the sequence restarts. +- Update the field doc and the tests that assert per-cursor counts; add one + where a cursor created after a rebind reports the same sequence as a cursor + that has run since the start. +- moq.pro's recorder and index store the count today; note the change for its + pin bump. diff --git a/quest/m1/hls-linger.md b/quest/m1/hls-linger.md new file mode 100644 index 0000000000..9da2c54d1c --- /dev/null +++ b/quest/m1/hls-linger.md @@ -0,0 +1,41 @@ +# [S] moq-hls keeps an ended broadcast for its playlist window + +## Goal + +A player polling `moq_hls::Server` can finish the last segments of a broadcast +that just ended, and a republish of the same name takes over cleanly. Today +`Server` evicts a broadcaster as soon as its broadcast closes, so the playlist +404s mid-window. The moq.pro edge works around it with its own pool on top of +`Broadcaster` that holds an ended broadcast for the playlist window plus a +grace period and checks for a republish +([#3964](https://github.com/moq-dev/moq/pull/3964)). Once moq-hls owns this, +the edge deletes its pool. + +## Plan + +Decided: moq-hls owns the retention through an `export::Config` field (the +playlist window plus a grace period), so every embedder gets it and the edge +has nothing to duplicate. `Config` is `#[non_exhaustive]`, so the field is +additive on `main`. + +Guidance: + +- The eviction lives in `server/mod.rs` (`evict_closed` and the + `is_closed` checks in `Server::broadcaster`). An ended broadcaster keeps + serving its final playlist, marked ended if the renditions know it, until + the linger expires. +- A republish during the linger replaces the ended broadcaster. Decide + whether a request for the name resolves the new broadcast first and falls + back to the ended one only when nothing is announced; match what the edge + pool does today. +- Segments are fetched from the relay cache on request, so the linger is only + useful within the cache's retention; say so on the field, as `window` does. +- A zero linger keeps today's behavior. Pick the default with the edge's + current value in mind. +- Tests: a request inside the linger after close still gets the playlist and + a cached segment; after it expires the name 404s; a republish inside the + linger is served. + +## Related + +- [Export linger](/quest/m1/export-linger.md) - the CLI exporter's wait for a broadcast to return; same idea, separate code diff --git a/quest/m1/ietf-max-age.md b/quest/m1/ietf-max-age.md index cd7f99a48e..04c6f11ce2 100644 --- a/quest/m1/ietf-max-age.md +++ b/quest/m1/ietf-max-age.md @@ -30,8 +30,12 @@ survives an IETF hop the way it already survives moq-lite 05+. group always kept, so the mapping is approximate. Accept that rather than modeling a second clock. - moq-lite-07 (still WIP, off by default) makes TRACK_INFO's Max Age - optional: the value plus one, with 0 meaning none. Lite05/06 map `None` to - the largest varint in both directions. Update + optional: the value plus one, with 0 meaning none. Lite05/06 have no + absent value, so `None` is a sentinel both languages can represent: send + 2^53-1 (`Number.MAX_SAFE_INTEGER`) and read any value at or above it as + `None`. Decided over the largest varint (2^62-1) because JS reads Max Age + as a u53, so that sentinel would not survive a JS hop + ([#4188](https://github.com/moq-dev/moq/pull/4188)). Update `drafts/draft-lcurley-moq-lite.md` in the same PR. - EXPIRES stays 0 on send and ignored on receive. It is subscription lifetime, not retention. diff --git a/quest/m1/iroh-lite-wip.md b/quest/m1/iroh-lite-wip.md new file mode 100644 index 0000000000..e733761633 --- /dev/null +++ b/quest/m1/iroh-lite-wip.md @@ -0,0 +1,31 @@ +# [XS] iroh honors the configured versions + +## Goal + +An `iroh://` client or listener configured with `moq-lite-07-wip` negotiates +it, as `https://`, `moqt://`, TCP, and Unix sockets already do. A version +list that iroh cannot offer is refused at startup, never silently replaced. + +## Plan + +https://github.com/moq-dev/moq/pull/4148 took `moq-lite-07-wip` out of +`moq_net::ALPNS` so it is opt-in only, but `rs/moq-tokio/src/iroh.rs` builds +its listener ALPNs, its dial offers, and its H3 subprotocols from that +constant rather than the configured `Versions` (`versions.alpns()`, which +`noq.rs`, `server.rs`, and `client.rs` use). So the opt-in is accepted by +config and then ignored on iroh. Thread the configured versions through +instead. + +The relay's WebSocket listener and `moq-ffi`'s transport also read +`moq_net::ALPNS` directly. Check whether they have the same hole; fix them +here if it is the same small change, otherwise report it. + +Decided by the maintainer: the finalized lite-07 ALPN stays `moq-lite-07`, +as `drafts/draft-lcurley-moq-lite.md` already says. Peers from the yanked +0.3.2 / 0.15.3 releases advertise `moq-lite-07` with an older framing, and +could land on it with a finalized peer; the maintainer accepts that risk +rather than burn the identifier. Nothing in this quest changes the ALPN. + +## Related + +- [iroh opt-in for moq-relay](/quest/m1/relay-iroh-opt-in.md) - a separate change: whether the relay builds iroh at all diff --git a/quest/m1/js-bundle-trims.md b/quest/m1/js-bundle-trims.md index e0d85fa0db..a34822567b 100644 --- a/quest/m1/js-bundle-trims.md +++ b/quest/m1/js-bundle-trims.md @@ -34,6 +34,12 @@ Guidance: puts module loading on the fallback path. Measure the connect time it adds, and drop this trim if the fallback gets noticeably slower. - Report the before and after first-load sizes in the PR. +- Add a CI check that imports the built `@moq/watch` dist outside a browser + (the `bun -e 'await import("./dist/index.js")'` that verified + [#4217](https://github.com/moq-dev/moq/pull/4217)). Unit tests run on + `src`, so a bundled browser-only dependency that breaks Node, Bun, or SSR + imports only shows up in the dist, and these trims move exactly those + imports around. Cover `@moq/publish` the same way if it is cheap. ## Related diff --git a/quest/m1/js-catalog-path.md b/quest/m1/js-catalog-path.md new file mode 100644 index 0000000000..230a37e60d --- /dev/null +++ b/quest/m1/js-catalog-path.md @@ -0,0 +1,27 @@ +# [XS] JS catalog watch rejects escaping broadcast references + +## Goal + +`Catalog.watch` in `@moq/hang` rejects a catalog whose rendition or track +`broadcast` reference escapes the broadcast, as Rust `Catalog::subscribe` +does with `EscapingBroadcast` ([#3935](https://github.com/moq-dev/moq/pull/3935)). +JS cannot check today: `watch` takes a `Broadcast.Consumer`, which has no +path to resolve the reference against, so `../../../other` passes in JS and +fails in Rust. + +## Plan + +Decided: make the path public on `@moq/net`'s `Broadcast.Consumer` and have +`Catalog.watch` read it, so the `watch()` call shape does not change. + +Guidance: + +- Mirror Rust's `broadcast::Info::path`: the origin stamps each handle with + the path it was requested or announced at, relative to the cursor's root, + and a standalone broadcast has an empty path, so any `..` escapes. +- Resolve every video, audio, text, JSON, and binary reference with + `Path.tryResolve` against it and reject the update when it returns + `undefined`, as `js/hang/src/catalog/path.ts` already describes. Keep the + relative values in the yielded catalog unchanged. +- Tests: an escaping reference is rejected, a sibling reference under the + same root passes, and a standalone broadcast rejects `..`. diff --git a/quest/m1/js-pattern-depth.md b/quest/m1/js-pattern-depth.md new file mode 100644 index 0000000000..3921c8eec1 --- /dev/null +++ b/quest/m1/js-pattern-depth.md @@ -0,0 +1,21 @@ +# [XS] JS subtree at max depth + +## Goal + +`Pattern.subtree` in `@moq/pattern` accepts a 32-segment path (the wire path +limit) and returns the literal path, as Rust's `Pattern::subtree` does since +https://github.com/moq-dev/moq/pull/4284. A 33-segment path still throws +`TooManySegments`. + +## Plan + +`js/pattern/src/index.ts` always appends `**`, so a max-depth path becomes 33 +segments and throws, which empties an announce subscription under that prefix +just as it did in Rust. Nothing can sit beneath a max-depth path, so the path +itself is the whole subtree. + +The Rust regression is a unit test in `rs/moq-pattern/src/pattern.rs`, which +JS never runs. Move the max-depth case (and the 33-segment refusal, if the +vector shape grows an error field the way `literal` has one) into the shared +`rs/moq-pattern/tests/pattern.json`, so both languages run the same vector +and the next divergence fails in whichever side lags. diff --git a/quest/m1/js-scoped-routes.md b/quest/m1/js-scoped-routes.md new file mode 100644 index 0000000000..27f91eb06c --- /dev/null +++ b/quest/m1/js-scoped-routes.md @@ -0,0 +1,37 @@ +# [S] JS scoped readers pick from their own routes + +## Goal + +A scoped `@moq/net` origin reader, and a connection publishing that scoped +view, announces a prefix whenever some route there overlaps its scope, and +presents the preferred route among those, as moq-net does. For example, with +a cheap `*/chat` route and a costlier `*/video` route both at `""`, a +`room/video` reader sees the video route instead of nothing. + +The cost of `forwardAnnounced` fanning one advertisement out across interest +streams is measured, and fixed only if the sweep shows it matters. + +## Plan + +Correctness first. `OriginState.snapshot()` in `js/net/src/origin.ts` keeps +one `preferredEntry` per path before `#listed` applies the reader's scope, +and `rebuildOriginated()` makes the same early choice, so a reader whose +scope excludes the globally preferred entry drops the path even though +`bestEntry` would route a request through another one. moq-net filters each +entry by the cursor's scope (`RouteEntry::overlaps` in +`rs/moq-net/src/model/origin.rs`) before choosing. Keep the candidates until +the scope applies. The shared unscoped snapshot is the fast path the +`js/net/bench/broadcasts.ts` sweep guards; keep it, and run that sweep before +and after. Raised by Codex on https://github.com/moq-dev/moq/pull/4234. + +Then the fan-out. `forwardAnnounced` in `js/net/src/connection/forward.ts` +opens one announce stream per interest head and keeps a `Dynamic` plus a +`drive` task per stream, so a peer advertising a prefix above several heads +(`""` for `a/**` and `b/**`) is stored and repriced once per head, and closing +one stream can drop the entry a request was using while an identical one +remains. Extend the nightly JS benchmark with a sweep over heads and routes +(the repo's fan-out rule), then decide: if cost grows with heads times routes +at realistic sizes, share or reference-count received prefixes across +streams; if not, record the numbers in the PR and leave it. + +No public API or wire change is expected. diff --git a/quest/m1/kio-waiter-lost.md b/quest/m1/kio-waiter-lost.md new file mode 100644 index 0000000000..ae4e29321b --- /dev/null +++ b/quest/m1/kio-waiter-lost.md @@ -0,0 +1,26 @@ +# [XS] kio waiter overflow stays idempotent + +## Goal + +A `kio::Waiter` that has registered on more than 8 lists still registers for +free on each list it recorded, so a retained standalone `Waiter` polled +repeatedly no longer appends a duplicate entry to those lists every time. + +## Plan + +`Waiter::record` in `rs/kio/src/waiter.rs` returns early once `lost` is set, +before it matches the list's tag against the recorded slots. `Park` retires +such a waiter, so it never notices, but `Waiter` is public and a caller that +keeps one across polls grows every list it sits on, one live entry per +register until the list drains, and gets a duplicate wake for each. + +Match the recorded tags first and only then fall back on `lost`. This was +written with the regression test `overflow_keeps_recorded_lists_idempotent` +during https://github.com/moq-dev/moq/pull/4240, which squash-merged before +it was pushed, so it needs rewriting. Keep the recorded-tag probe the tight +common-case loop it is today; `rs/kio/benches/waiter.rs` shows whether it +moved. + +A list past the 8 slots is unrecorded and still has to append on every +register, which is the pre-#4240 behavior. Say so on `Waiter::register` so a +standalone caller knows the bound, rather than growing the record array. diff --git a/quest/m1/merge-queue.md b/quest/m1/merge-queue.md new file mode 100644 index 0000000000..002b723959 --- /dev/null +++ b/quest/m1/merge-queue.md @@ -0,0 +1,31 @@ +# [S] Merges go through a merge queue + +## Goal + +A pull request cannot break `main` by merging on checks that ran against an +older `main`. [#4137](https://github.com/moq-dev/moq/pull/4137) did exactly +that: it merged cleanly, but combined with +[#4089](https://github.com/moq-dev/moq/pull/4089) it stopped `moq-ffi` from +compiling on `main` until [#4157](https://github.com/moq-dev/moq/pull/4157) +fixed it. The `main` ruleset requires `Check` and `Test` but not strictly, +and there is no merge queue, so nothing re-runs them on the combined tree. + +## Plan + +- Decided: a GitHub merge queue, not a strict ruleset. Strict status checks + would force every open PR to update and re-run whenever `main` moves; a + queue tests the combination once, at merge time. +- Make the workflows ready: every workflow providing a required check + (today `Check` and `Test` in `.github/workflows/check.yml`) also runs on + `merge_group`. `just ci` scopes by diffing against `GITHUB_BASE_REF`, + which a merge group does not set, so pass the group's base + (`github.event.merge_group.base_sha`) explicitly. Check the concurrency + group and the `closed`-only skip still behave for queue refs. +- Document it in `CONTRIBUTING.md`: PRs merge through the queue, a + dequeued PR means the combination failed, and how agents enqueue (the + merge skills use `gh pr merge`, which enqueues when a queue is on). +- The ruleset change (enable the queue, choose squash) is the maintainer's + act, after the workflow change lands on `main`. Hand it over with the + settings to use rather than changing it. + +Public API: none. Wire: none. diff --git a/quest/m1/moqsink-keyframe-latch.md b/quest/m1/moqsink-keyframe-latch.md new file mode 100644 index 0000000000..d86c1e638a --- /dev/null +++ b/quest/m1/moqsink-keyframe-latch.md @@ -0,0 +1,23 @@ +# [XS] moqsink waits for a published keyframe after a break + +## Goal + +A video pad in `moqsink` survives a pause or segment break whose first buffer +carries only codec headers. Today `Media::write` in +`rs/moq-gst/src/sink/pad.rs` clears its `keyframe` latch after any successful +decode. A header-only buffer decodes without emitting a frame, clears the +latch, and the next delta frame hits `MissingKeyframe` outside the guarded +arm, which permanently invalidates the pad +([#4239](https://github.com/moq-dev/moq/pull/4239)). + +## Plan + +Keep the latch set until the importer actually publishes a keyframe, not +until the first buffer parses. That needs `import::Track::decode` to say +whether it emitted a frame, or a comparable signal; prefer reusing what the +importer already knows over inferring it in the pad. + +Regression test beside `video_pause_drops_deltas_until_the_next_keyframe`: a +break followed by a header-only buffer (SPS/PPS alone for H.264), then a +delta, then a keyframe. The delta drops and the keyframe publishes; the pad +stays valid. diff --git a/quest/m1/moqsrc-stop.md b/quest/m1/moqsrc-stop.md new file mode 100644 index 0000000000..d3de732b61 --- /dev/null +++ b/quest/m1/moqsrc-stop.md @@ -0,0 +1,27 @@ +# [S] moqsrc waits for its session to end on stop + +## Goal + +`moqsrc`'s PAUSED to READY transition returns only after its session task and +connection have ended, as `moqsink` does since +[#4074](https://github.com/moq-dev/moq/pull/4074). Today +`SessionController::stop` in `rs/moq-gst/src/source/imp.rs` signals shutdown +and spawns a task to await the join, so an application that sets NULL and +exits right away can race aws-lc's exit destructors against an in-flight +dial, the silent SIGABRT the sink fix closed. + +## Plan + +Blocking needs care: the session task pushes buffers into src pads +(`block_in_place(|| pad.push(buffer))`), and a push blocked downstream while +`stop` waits on the task is a deadlock. Unblock the pads first (set them +flushing, or otherwise make a pending push return) before waiting, the way +GStreamer sources normally stop their streaming threads. + +Reuse the sink's wait: it parks the thread on a waker instead of entering an +executor, and uses `block_in_place` when called from a multi-thread runtime +worker. Consider sharing it between the two elements rather than copying. + +Tests mirroring the sink's: stop returns after the connection closes, stop +from a runtime worker does not deadlock, stop inside another executor does +not panic, and stop while a push is blocked downstream returns. diff --git a/quest/m1/obs-moq-video/README.md b/quest/m1/obs-moq-video/README.md index 17660ac693..3362f55a46 100644 --- a/quest/m1/obs-moq-video/README.md +++ b/quest/m1/obs-moq-video/README.md @@ -12,7 +12,7 @@ Attempt GPU delivery immediately, starting on macOS. Windows and Linux can ship Initial video decoding covers H.264, HEVC, and AV1 where moq-video has an available backend. Unsupported codecs produce an actionable error; do not retain an FFmpeg fallback. VP8/VP9 return through their own follow-up quest. Audio playback covers Opus, AAC-LC, and PCM. -Publishing remains opt-in, with one **Use MoQ encoders** choice for video and audio. Keep the existing OBS encoder mode. Internal OBS encoder adapters call moq-video/moq-audio, preserving OBS's A/V handling and the existing encoded MoQ output. The combined choice is enabled only when both adapters are present. Start with H.264, supported HEVC, and Opus; defer AV1/AAC encoding and PCM publishing UI. Keep bitrate separate from **Low latency** (default), **Balanced**, and **Quality** presets. Presets describe supported buffering/compression controls, not an end-to-end delay promise. +Publishing remains opt-in, with one **Use MoQ encoders** choice for video and audio. Keep the existing OBS encoder mode. Internal OBS encoder adapters call moq-video/moq-audio, preserving OBS's A/V handling and the existing encoded MoQ output. The combined choice is enabled only when both adapters are present. Start with H.264, supported HEVC, and Opus; defer AV1/AAC encoding and PCM publishing UI. Keep bitrate separate from **Low latency**, **Balanced** (default), and **Quality** presets. Presets describe supported buffering/compression controls, not an end-to-end delay promise. The quests separate portable decoding, platform GPU delivery, audio, and publishing so each can land and be validated independently. The existing CPU decode path is a fallback primitive, not a GPU implementation: it explicitly converts every surface to I420. Native frame ownership must cross the FFI boundary without that conversion. @@ -24,6 +24,7 @@ The quests separate portable decoding, platform GPU delivery, audio, and publish - [Linux decoded frames](/quest/m1/obs-moq-video/decode-linux.md) - present supported native decoded surfaces with visible CPU fallback - [Linux bundle](/quest/m1/obs-moq-video/linux-bundle.md) - attach a portable Linux x86_64 tarball to every obs-moq release once FFmpeg is gone - [Encoder presets](/quest/m1/obs-moq-video/presets.md) - define and measure shared low-latency, balanced, and quality policies +- [Preset parity](/quest/m1/obs-moq-video/preset-parity.md) - audio stores and reports its preset like video, defaults to Balanced, and the preset claims hold - [Audio publishing](/quest/m1/obs-moq-video/audio-publish.md) - back an internal OBS Opus encoder with moq-audio - [Video publishing](/quest/m1/obs-moq-video/adapter.md) - back an internal OBS video encoder with moq-video and expose the combined opt-in mode - [Rate control](/quest/m1/obs-moq-video/rate-control.md) - the plugin reserves its bitrate and retunes the OBS encoder to the grant diff --git a/quest/m1/obs-moq-video/adapter.md b/quest/m1/obs-moq-video/adapter.md index 752ca5a002..2338fef15b 100644 --- a/quest/m1/obs-moq-video/adapter.md +++ b/quest/m1/obs-moq-video/adapter.md @@ -9,7 +9,7 @@ One opt-in Use MoQ encoders choice publishes OBS video and audio through moq-vid - Register an internal OBS video encoder, backed by `moq_video::encode::Sink`. Retain `MoQOutput::EncodedPacket` and existing catalog handling. Do not replace the output with raw publication: OBS's encoded-output flag is output-wide, and bypassing it would duplicate A/V integration. - Add a clean codec-only moq-ffi encoder type with owned handles and packet draining, extending shared primitives from the audio adapter where appropriate. Avoid backend internals and caller cleanup callbacks. The existing raw-video publishing API couples encoding to publication and is not the packet adapter. - Start with H.264 by default and HEVC where supported. Keep reordering disabled; resolve OBS keyframe flags, Annex-B headers/decoder configuration, DTS/PTS and drain semantics explicitly, since Rust encoded output currently contains only timestamp and payload. Do not advertise unsupported AV1 encoding. -- Use the shared Low latency, Balanced, and Quality presets with bitrate separate. Expose a single Use MoQ encoders option only once audio and video adapters both work. Retain the existing OBS encoder selection as an explicit alternative; do not silently switch back to OBS codecs after a MoQ codec failure. +- Use the shared Low latency, Balanced, and Quality presets with bitrate separate, defaulting to Balanced. Expose a single Use MoQ encoders option only once audio and video adapters both work. Retain the existing OBS encoder selection as an explicit alternative; do not silently switch back to OBS codecs after a MoQ codec failure. - Establish bounded submission/packet queues, explicit raw-frame drop behavior, thread confinement, cancellation, late completion, device loss, resize and color metadata. Never block OBS's graphics thread on network backpressure. The CPU path is a correctness/fallback baseline; platform quests establish accelerated input. - Test rejection, saturation, drain, stop during encode, delayed completion, and repeated start/stop. Validate real decoded pixels and audio continuity, matched timestamps, preset reporting, and frame-to-packet latency. Validate the new binding docs, feature combinations, package dependencies and native plugin linking. diff --git a/quest/m1/obs-moq-video/macos.md b/quest/m1/obs-moq-video/macos.md index f56120b6c2..aa8265c9be 100644 --- a/quest/m1/obs-moq-video/macos.md +++ b/quest/m1/obs-moq-video/macos.md @@ -9,6 +9,11 @@ An OBS compositor frame reaches moq-video's VideoToolbox encoder without a GPU-t - Inspect OBS's OpenGL compositor, `encode_texture2`, and mac-videotoolbox input path. Determine whether the output allocation is IOSurface-backed and exportable. OBS's encoder currently copies CPU planes into its own pixel buffer; a CVPixelBuffer in moq-video alone does not remove that upstream readback. - Prefer retained IOSurface/CVPixelBuffer storage in the format VideoToolbox accepts. Otherwise prototype GPU color conversion/blit into an IOSurface-backed NV12 pool. Specify GL/Metal/CoreVideo interop, graphics-context thread affinity, completion fences, and when OBS may recycle the source. - Reuse the native PixelBuffer surface and the moq-video VideoToolbox backend. Retain the destination until encoding completes, including dropped submissions and cancellation. Bound the pool and handle resolution/HDR changes and device failure. +- Measure and map the encoder presets on VideoToolbox while on the + hardware. Today it applies real-time, no-reordering controls and reports + `LowLatency` whatever preset was asked; give Balanced and Quality the + controls VideoToolbox has (real-time off, quality or speed priority) + without reordering, and report what took. - Verify no CPU readback using GPU/API traces and copy counters. Compare direct import or GPU blit against the CPU baseline at 1080p60 and 4K where supported. Check decoded color bars and moving timestamps, latency percentiles, audio sync, stop/restart, and long-running pool reuse on Apple hardware. ## Required diff --git a/quest/m1/obs-moq-video/preset-parity.md b/quest/m1/obs-moq-video/preset-parity.md new file mode 100644 index 0000000000..d2c2cd8623 --- /dev/null +++ b/quest/m1/obs-moq-video/preset-parity.md @@ -0,0 +1,47 @@ +# [S] Audio presets mirror video, and the preset claims hold + +## Goal + +The encoder presets from [#4099](https://github.com/moq-dev/moq/pull/4099) +read the same in moq-video and moq-audio, and nothing they promise is false. +The shape is settled; this finishes it: + +- Audio mirrors video: `moq_audio::encode::Settings` stores the preset it was + given and reads it back, and the audio encoder reports an `Applied` the way + `moq_video::encode::Encoder::applied` does. Today audio only has + `Settings::with_preset`, which rewrites `frame_duration` and forgets the + preset. +- The audio default agrees with itself: `Preset::default()` is `LowLatency` + (10 ms), but `Settings::new()` builds 20 ms, which is `Balanced`. +- The `Preset` doc in `rs/moq-video/src/encode/encoder.rs` no longer claims + that no preset reorders frames: V4L2 leaves reordering and queue depth to + the driver and MediaCodec's no-B-frame setting is an unconfirmed hint. Only + `Applied::preset` confirms it. +- `rs/moq-video/examples/encode-presets.rs` scores PSNR against the right + source frames after the encoder skips some (it ignores the `.skipped` + file today), and refuses an unknown preset name instead of printing the + header and exiting 0. + +## Plan + +- Decided: `encode::Preset` and `Applied { preset: Option, controls: + String }` are the API; don't reopen them. Audio already has its `Preset` + and gains the same `Applied` under `moq_audio::encode`. +- The audio enum default follows `Settings::new()`, not the other way + round: its 20 ms is published in moq-audio 0.1.6 and matches the JS + publish path, so `Preset::default()` becomes `Balanced` for audio. OBS + defaults to Balanced as well (decided with the maintainer), so the plugin + rides the published default instead of overriding it. +- Fail loud in the example: an unknown name is an error listing the valid + ones. +- The line PR ([OBS native codecs](/quest/m1/obs-moq-video/README.md)) must + call out the video default changes #4099 made: NVENC P4 to P1, and + openh264 medium to low complexity. +- Known gap: VAAPI reports `LowLatency` whatever was asked, and V4L2 and + MediaCodec report unconfirmed; no quest owns measuring and mapping presets + for them. Media Foundation and VideoToolbox are owned by + [Windows GPU input](/quest/m1/obs-moq-video/windows.md) and + [macOS GPU input](/quest/m1/obs-moq-video/macos.md). + +Public API: additive on moq-audio (the stored preset and its `Applied` +report); the unpublished audio `Preset` default changes. Wire: none. diff --git a/quest/m1/obs-moq-video/windows.md b/quest/m1/obs-moq-video/windows.md index 787a739efd..886b0b6f4e 100644 --- a/quest/m1/obs-moq-video/windows.md +++ b/quest/m1/obs-moq-video/windows.md @@ -9,6 +9,13 @@ The moq-video OBS encoder consumes compositor output through D3D11 without CPU s - Inspect OBS `encoder_texture` shared handles and `encode_texture2` lock_key/next_key semantics. Record whether the source is packed RGB or split NV12 planes, its adapter LUID, and the lifetime OBS guarantees after the callback returns. - Reuse the native D3D11 surface where the encoder accepts the same device and format. Otherwise GPU-convert/blit into a bounded encoder-owned NV12 pool before returning the OBS synchronization key. An AddRef alone does not prevent OBS from overwriting pooled pixels. - Audit keyed mutex/fence sequencing, asynchronous completion, incompatible adapters, software fallback, resolution changes, and device removal. Start with Media Foundation, which accepts D3D11 surfaces. The current NVENC backend is Linux-only and directly imports CUDA surfaces there; Windows NVENC/D3D11 support is separate backend work. Do not infer interoperability from both APIs accepting a texture handle. +- Measure and map the encoder presets on Media Foundation while on the + hardware. Today `applied()` reports "AVLowLatencyMode requested, + unconfirmed" for every preset, because a refused knob is only logged and + the MFT is configured on the first frame. Make low-latency mode + confirmable (read it back, or fail when it is refused) so `Applied::preset` + can name a preset, and give Balanced and Quality distinct controls where + the MFT has them. - Trace readback and GPU copy counts on real Windows hardware. Verify pixels and A/V timestamps with a subscriber, compare latency and utilization to the CPU baseline, and exercise cancellation while textures remain in flight. Include hybrid-GPU and device-mismatch rejection where available. ## Required diff --git a/quest/m1/pipewire-dup-cameras.md b/quest/m1/pipewire-dup-cameras.md new file mode 100644 index 0000000000..c9ad2a46fc --- /dev/null +++ b/quest/m1/pipewire-dup-cameras.md @@ -0,0 +1,24 @@ +# [XS] A webcam lists once with PipeWire enabled + +## Goal + +`moq_video::capture::cameras()` on Linux with the `pipewire` feature lists a +webcam once. Today a UVC webcam appears as its V4L2 device and again as the +PipeWire node that wraps it ([#4022](https://github.com/moq-dev/moq/pull/4022)). + +## Plan + +Decided: hide PipeWire camera nodes with `device.api = v4l2` whose device the +V4L2 backend already listed, keeping the shorter list over exposing the +backend choice. Nodes V4L2 cannot see stay: libcamera cameras (a Raspberry Pi +CSI camera) and everything inside a sandbox, where V4L2 lists nothing. + +Guidance: + +- Match on the node's V4L2 device path property (`api.v4l2.path`), not the + description, so two identical webcams stay distinct. +- Only the listing changes. `pipewire:` still opens a hidden node, and + the `pipewire` default keeps resolving by priority. +- Update the `cameras()` doc that currently says a webcam appears once per + backend, and cover the filter with a unit test over scanned node + properties. diff --git a/quest/m1/play-decode-schedule.md b/quest/m1/play-decode-schedule.md new file mode 100644 index 0000000000..0bc9b79b54 --- /dev/null +++ b/quest/m1/play-decode-schedule.md @@ -0,0 +1,36 @@ +# [S] moq play decodes on a schedule that survives rewinds and deep reordering + +## Goal + +`moq play`'s video path keeps every valid picture and drops only stale ones, +across a timeline rewind, a B-frame reorder deeper than 100 ms, and a decoder +batch larger than the queue. Three Codex findings on +[#4241](https://github.com/moq-dev/moq/pull/4241) went unanswered and all +still hold in `rs/moq-cli/src/play/video.rs`: + +- After a discontinuity rewinds timestamps, pictures the decoder still holds + from the old generation pass the `< floor` check (an old 10 s picture is + above a new 0 s floor), get scheduled far ahead, and can make the cap evict + valid new pictures. +- Decode waits until `DECODE_AHEAD` (100 ms) before a frame's presentation. A + reference frame whose PTS sits far past the B-frames encoded after it + delays them past their own presentation when reordering is deeper than + that; H.264 allows a 16-frame DPB. +- `decoded()` inserts a whole batch under one lock and trims to + `MAX_FRAMES` (3), so a decode or final flush that returns more than three + pictures loses all but the newest three. + +## Plan + +- Rewind: identify stale output by generation, not timestamp order. Flush or + reset the decoder at the break and discard what it returns, or tag frames + with the generation they were submitted in. +- Decode ahead: schedule by the earliest presentation the pending access + units can still produce, or size the lead from the rendition's catalog + `jitter` (the reorder depth), rather than a fixed 100 ms. +- Batches: let the presenter drain a large batch rather than trimming it in + one step, or apply the cap only when the window is actually stalled. The + cap exists to bound memory while presentation lags, not to cut a flush tail. +- Regressions on the fake presenter/decoder rig from #4241: a rewind with + pictures held across it, a stream with reordering deeper than 100 ms, and a + flush returning more than three pictures. diff --git a/quest/m1/publish-codec-string.md b/quest/m1/publish-codec-string.md new file mode 100644 index 0000000000..b86f6c557b --- /dev/null +++ b/quest/m1/publish-codec-string.md @@ -0,0 +1,33 @@ +# [S] js/publish advertises the full codec string + +## Goal + +Every video rendition `@moq/publish` puts in the catalog carries a full +RFC 6381 codec string, so native players can decode what browsers publish. +Today the encoder probe in `js/publish/src/video/encoder.ts` falls back to the +bare hints `vp09`, `avc1`, `av01`, and `hev1` when no specific string is +supported, and the catalog publishes the hint as the codec. Rust `hang` parses +those as `VideoCodec::Unknown`, so moq-video and the other native consumers +refuse the track ([#4095](https://github.com/moq-dev/moq/pull/4095)). + +## Plan + +Decided: advertise the codec string from the encoder's own output, +`EncodedVideoChunkMetadata.decoderConfig.codec`, rather than the probe input. +Rust keeps refusing bare hints: the decoder gate needs the profile before it +subscribes. + +Guidance: + +- The catalog is built from the resolved config today, before any frame is + encoded. Either hold the rendition out of the catalog until the first + output reports its `decoderConfig`, or update it then; keep the stall and + jitter reporting working either way. +- Check what each browser returns for a bare hint. If one echoes the hint + back, derive the string from the bitstream (SPS for H.264/H.265, the + sequence header for AV1, the uncompressed header for VP9), or fail loud + rather than publish a string no native player accepts. +- A reconfigure (resolution or codec change) can change the string; the + catalog follows it. +- Tests with the fake `VideoEncoder`: a bare-hint probe whose output reports + a full string publishes the full string. diff --git a/quest/m1/publish-lazy-file.md b/quest/m1/publish-lazy-file.md index ea1fd906fe..fe1d333bd0 100644 --- a/quest/m1/publish-lazy-file.md +++ b/quest/m1/publish-lazy-file.md @@ -14,6 +14,12 @@ Decided in planning: - Load the file source with a dynamic `import()` when a file source is selected. +- Open the picker before anything is awaited: `File.prompt()` must run + synchronously in the click handler, and the decoder (mediabunny) loads + lazily once a `File` arrives. Awaiting the `import()` first spends the + click's transient user activation, so the browser blocks the picker + (Codex on [#4257](https://github.com/moq-dev/moq/pull/4257)). So the + picker stays in the eager bundle and only the decode path is lazy. - Keep `ALL_FORMATS`, so any container mediabunny reads still works. Leave mediabunny bundled into publish's dist rather than making it external. diff --git a/quest/m1/redirect-resolve.md b/quest/m1/redirect-resolve.md new file mode 100644 index 0000000000..5c3f7e56d9 --- /dev/null +++ b/quest/m1/redirect-resolve.md @@ -0,0 +1,29 @@ +# [XS] Strict or private Redirect::resolve + +## Goal + +`moq_tokio::Redirect` no longer offers a public way to turn a refused or +malformed GOAWAY redirect into "dial the current address". A caller either +gets the same refusal `Connection` acts on, or cannot call it at all. + +## Plan + +The drain line (https://github.com/moq-dev/moq/pull/4143) made +`Connection` end with `Error::RefusedRedirect` on a malformed or +policy-refused URI instead of redialing the peer that asked it to leave, and +made a certificate pin refuse a host change. It left the public +`Redirect::resolve` lenient to avoid a break: it falls back to the current +URL on any refusal and never sees the pin, so it answers differently from +the connection it documents. + +Recommendation: make it private. Nothing outside `moq-tokio` calls it (only +its own unit tests), and the repo keeps things private until a consumer +needs them. If a consumer turns up, the alternative is returning the same +`Result>` as the internal `target`, so empty and refused stay +distinct. Removing or changing a published method is a break, so this +targets `dev`; update `doc/lib/rs` if it mentions the method. + +## Required + +- [Graceful relay drains](/quest/m1/drain/README.md) - the stricter `Connection` lands with the line +- `dev` has merged `main` after the drain line lands diff --git a/quest/m1/rtmp-tls-only.md b/quest/m1/rtmp-tls-only.md new file mode 100644 index 0000000000..e281f2021a --- /dev/null +++ b/quest/m1/rtmp-tls-only.md @@ -0,0 +1,32 @@ +# [XS] RTMP listener can refuse plaintext + +## Goal + +An operator who configures TLS on the RTMP ingest listener can require it. +Today a TLS-configured listener sniffs the first byte and serves any +non-ClientHello connection as plaintext `rtmp://` on the same port +(`rs/moq-rtmp/src/listen.rs`), so setting `tls` silently keeps accepting +unencrypted stream keys. The sniffing behavior stays available; it is just no +longer the only choice. + +## Plan + +Decided by the maintainer during the merged-PR audit: add a mode rather than +drop the sniffing. #3964 shipped mixed mode after a bot review, not a human +one, and nobody chose it for operators who want TLS only. + +- Model it so a TLS-only listener without a TLS config cannot be expressed, + for example an enum carrying the `ServerConfig` (plaintext, TLS required, + TLS or plaintext) instead of a bool beside the `Option`. Keep today's + behavior as the default for an existing `tls` config unless that reads + wrong once written; say which in the PR. +- A plaintext connection to a TLS-only listener is refused at the first byte, + with a log line naming the peer, not left to time out. +- Thread the choice through the relay and gateway configs that build this + listener, and update `doc/bin/rtmp.md` and any relay config docs that + describe RTMPS. +- Test both modes: a plaintext client is refused by TLS-only and served by + mixed. + +Public API: additive on moq-rtmp's listen config if the default holds; a +published break (dev) if the field's type changes. Wire: none. diff --git a/quest/m1/session-close.md b/quest/m1/session-close.md index 39ad6edba2..499c51e678 100644 --- a/quest/m1/session-close.md +++ b/quest/m1/session-close.md @@ -22,3 +22,11 @@ end and returns a promise, which is a published break, so that change targets `doc/concept/moq-lite.md` says a graceful close withdraws announces and an abort does not. No new page. + +The interop runner is the consumer that found this +([#4209](https://github.com/moq-dev/moq/pull/4209)): after a successful +publish it calls `abort`, so the relay never sees `PUBLISH_NAMESPACE_DONE` and +the next run is told the namespace is already published. Switch it to the +graceful `close()` once that exists. If the calling code lives outside this +repository (moq-interop-runner), that change is a PR there and needs the +maintainer's approval before posting. diff --git a/quest/m1/session-death.md b/quest/m1/session-death.md new file mode 100644 index 0000000000..22ac9be5c5 --- /dev/null +++ b/quest/m1/session-death.md @@ -0,0 +1,42 @@ +# [S] Session death parity + +## Goal + +Rust and JS end a session's tracks and groups the same way: + +- A session closed locally, on purpose, ends the tracks it was receiving + cleanly in both languages. Groups still in flight end as they do today. +- A session that dies (peer close, transport failure) ends its tracks and + its group readers with the session's error, carrying the peer's code, + never the raw transport error, `Dropped`, or `Cancel`. + +## Plan + +https://github.com/moq-dev/moq/pull/4120 made a dying session end its tracks +with the session's error in both languages, and left two gaps. + +Decided by the maintainer: + +- **A local close is a close, not an error.** JS already ends tracks cleanly + on `close()`. Rust has no clean end short of a finished track, so #4120 + ends them with the close error (previously `Cancel` or `Dropped`). Rust + needs a clean end at the current edge that is not a declared end: readers + get what was delivered and then `None`. Keep #4120's rule that an abort + before a declared end settles still wins, and #4116's clean end for a + dropped producer after `finish_at`; the new path must not mask either. + Whether this is a new `track::Producer` method or a driver-internal path + is an API call, so keep it private unless a consumer needs it. +- **JS group readers see the session's error.** Tracks go through + `sessionCause` in `js/net/src/error.ts`, but `runGroup` in the lite and + IETF subscribers closes a group with the raw error. Route it the same way. + +Extend the existing session-death tests (Rust +`a_session_death_ends_the_track_with_its_error`, the JS lite and IETF +integration cases) with a local close and with a group reader, on lite and +IETF. Behavior change, no signature change on either side unless the Rust +clean end needs a public method. + +## Related + +- [Track tail hardening](/quest/m1/track-tail-hardening.md) - the tail rules this must not mask +- [Graceful session close](/quest/m1/session-close.md) - what a local close sends the peer diff --git a/quest/m1/splice-edges.md b/quest/m1/splice-edges.md new file mode 100644 index 0000000000..aa0d904500 --- /dev/null +++ b/quest/m1/splice-edges.md @@ -0,0 +1,37 @@ +# [S] Splice edge cases + +## Goal + +Three spliced-track cases in `rs/moq-net` stop losing or mis-judging groups, +each with a regression test that fails before its fix: + +- A group whose successor segment's first servable group is still unstamped + keeps an unbounded reach, instead of borrowing a later segment's start and + being skipped or ended with `Error::Old`. +- A reader still draining a pruned segment has its boundary group judged + against the later segments, so a stale boundary group is skipped like any + other. +- A reader draining a warm head keeps it when the upstream fails over again + before the track parks. + +## Plan + +Codex raised all three on https://github.com/moq-dev/moq/pull/4103 and +https://github.com/moq-dev/moq/pull/4104 after the fixes there landed; none +was answered. + +- `served_start` in `rs/moq-net/src/model/resume.rs` says an unstamped group + stops the search, but `find_map` reads a segment's `None` (no group, or its + first group has no frame yet) as a miss and moves on. The two cases need + to be distinguishable, as the per-track `served_start` in + `rs/moq-net/src/model/track.rs` already treats them. +- `ResumeState::successor` finds the cursor's segment by id, so a segment + `prune` removed while a reader still drains it yields no successor at all. + Resolve it from the boundary and the remaining later segments instead. +- An ordinary takeover in `rs/moq-net/src/model/origin.rs` (`Action::Splice` + with no warm copy) overwrites `io.head` with `None`. Dropping the + `WarmGroup` aborts the cached head that a reader resumed from an earlier + park may still be reading, and the next park cannot rebuild the full group. + Keep the existing head when the splice has no new one to take. + +These are independent; one PR is fine since they share the splice tests. diff --git a/quest/m1/test-flakes-2.md b/quest/m1/test-flakes-2.md new file mode 100644 index 0000000000..384faa3f0e --- /dev/null +++ b/quest/m1/test-flakes-2.md @@ -0,0 +1,58 @@ +# [S] More tests hold up under load + +## Goal + +A second round after [#4286](https://github.com/moq-dev/moq/pull/4286): +tests that pass alone but have failed under a loaded `just check` pass +reliably, each fixed at its cause, never by raising a timeout or adding a +retry. + +- moq-cli `fetch::tests::a_frame_read_times_out` asserts on a 500 ms wall + deadline ([#4084](https://github.com/moq-dev/moq/pull/4084)). +- moq-cli `complete::tests::a_stage_broadcast_picks_the_catalog_to_read` + ([#4084](https://github.com/moq-dev/moq/pull/4084)) and + `the_catalog_format_on_the_line_is_honored` + ([#4089](https://github.com/moq-dev/moq/pull/4089)). +- moq-net `model::group::test::drop_unfinished_warns` counts WARNs through a + global tracing capture, so another test's WARN, or a missed one, changes + the count ([#4104](https://github.com/moq-dev/moq/pull/4104)). The + `model::track` test of the same name uses the same helper. +- moq-tokio `broadcast_race_quic_wins` binds TCP `:0` and then UDP on the + same number, which nothing reserves: the collision + [#4084](https://github.com/moq-dev/moq/pull/4084) removed from its sibling + after [#4055](https://github.com/moq-dev/moq/pull/4055) papered over it with + a retry. +- `just test media` late join failed once after + [#4181](https://github.com/moq-dev/moq/pull/4181): "joined at frame 111, 16 + frames behind 127", against a budget of one GOP (15). + +## Plan + +- Timing tests: prefer a paused clock over wall time (`moq-cli`'s + subscribe tests already use `#[tokio::test(start_paused = true)]`), or + assert on an event instead of a deadline. If a test is slow under load + because the code under test is slow, fix that. +- WARN counting: capture per test (a scoped subscriber or a filter on the + test's own span) instead of a process-global count. +- The race test shares one port only so both transports sit behind one URL. + Separate `:0` ports fix the bind collision but not the race itself: with + `websocket.delay = 0` either arm can legitimately win under load. Make the + order deterministic the way #4084 did, holding the WebSocket arm until QUIC + has connected, rather than asserting on a real race; no retry. #4084's follow-ups + (`tests/reconnect.rs` `spawn_server`, `tests/worker.rs` `free_udp_port`) + are the same probe-and-rebind pattern; fix them here if cheap. +- Media late join: first decide whether 16 frames is a real regression (the + player joining at the previous GOP's keyframe) or an off-by-one in how the + fixture samples the live edge. Fix whichever it is; don't widen the + budget without a reason. +- Prove it by running `just check --all` several times on a loaded machine, + as the first round did. + +Public API: none. Wire: none. + +## Related + +- [Archive enrollment](/quest/m1/archive/enrollment-flake.md) - the same + kind of flake on the archive line, where its test lives +- [Interop flakes](/quest/m1/interop-flakes.md) - port reservations and the + pause click in the interop harness diff --git a/quest/m1/tooling/README.md b/quest/m1/tooling/README.md index 073918b847..a96ed7e11c 100644 --- a/quest/m1/tooling/README.md +++ b/quest/m1/tooling/README.md @@ -20,6 +20,7 @@ requires the one before it, so they land as one line of pull requests. ## Quests - [Thin justfiles](/quest/m1/tooling/justfiles.md) - recipe bodies move to `sh/`, one impact map scopes check/fix/test, self-tests and guards are deleted +- [Windows cross-check](/quest/m1/tooling/windows-cross-check.md) - a Linux cross-check of moq-video for Windows runs per PR when moq-video is selected - [Workflows call just](/quest/m1/tooling/workflows-call-just.md) - no workflow `run:` step names a `.sh`; every script a workflow needs has a recipe - [Binary release workflow](/quest/m1/tooling/release-binary.md) - moq-cli and moq-relay share one reusable workflow behind two thin callers - [FFI release workflow](/quest/m1/tooling/release-ffi.md) - the five `release-*-ffi.yml` share the moq-ffi target matrix and artifact staging diff --git a/quest/m1/tooling/windows-cross-check.md b/quest/m1/tooling/windows-cross-check.md new file mode 100644 index 0000000000..5bda413332 --- /dev/null +++ b/quest/m1/tooling/windows-cross-check.md @@ -0,0 +1,36 @@ +# [XS] Windows cfg breaks in moq-video fail the PR + +## Goal + +A pull request that breaks moq-video's `#[cfg(target_os = "windows")]` code +fails its own checks instead of the next nightly. Today only `just rs +windows` compiles that code, on a Windows runner once a day, so +[#4036](https://github.com/moq-dev/moq/pull/4036) had to repair a Media +Foundation test that had been broken on `main` for a while. That PR showed a +Linux host can catch it: +`cargo check -p moq-video --no-default-features --features capture +--all-targets --target x86_64-pc-windows-msvc` reproduced the errors and +passed with the fix, with no Windows host and no C toolchain. + +## Plan + +- Run that check per PR whenever the impact map selects moq-video, as a + recipe the workflow calls. `openh264` stays off because its vendored C++ + needs MSVC; the Media Foundation, D3D11, and capture code is what the + check is for. +- The dev shell's toolchain carries only `wasm32-unknown-unknown`; add the + `x86_64-pc-windows-msvc` std target to `flake.nix` so the check runs the + same locally and in CI. Measure what it adds to the shell and the check. +- Fix the stale `rs/justfile` comment above `windows`: it says cross-compiling + cannot stand in because openh264 is non-optional, but openh264 is now a + feature. The nightly Windows job still owns linking, the other crates, and + running the tests. +- Prove it by reintroducing one of #4036's errors on a branch and watching + the PR check fail. + +Public API: none. Wire: none. + +## Required + +- [Thin justfiles](/quest/m1/tooling/justfiles.md) - the impact map that + selects moq-video diff --git a/quest/m1/track-tail-hardening.md b/quest/m1/track-tail-hardening.md new file mode 100644 index 0000000000..d6fbb68bf4 --- /dev/null +++ b/quest/m1/track-tail-hardening.md @@ -0,0 +1,79 @@ +# [M] Track tail hardening + +## Goal + +moq-net and `@moq/net` wait out a subscription's tail by the same rules, and +the known holes in those rules are closed. A finished IETF subscription never +hangs its request task or under-reports its stream count, a group whose +header arrived is never silently dropped from a clean end, the grace never +cuts off a stream still being read, and tail bookkeeping stays bounded on a +lossy track. + +## Plan + +Track tail landed in https://github.com/moq-dev/moq/pull/4086 (JS) and +https://github.com/moq-dev/moq/pull/4116 (Rust), and Codex's findings on both +were left for this pass. Fix each in the language named and check whether +the other has the same hole. + +Bugs: + +- **Rust IETF publisher, END_OF_TRACK not cancellable.** After the group + streams drain, `write_end_of_track` in `rs/moq-net/src/ietf/publisher.rs` + runs outside the race against `stream.reader.poll_closed` and the session, + so with uni-stream credit exhausted an unsubscribe leaves the request task + parked forever, never sending PUBLISH_DONE. +- **Rust IETF publisher, stream count.** END_OF_TRACK is counted only if the + whole write succeeds, while every other stream counts once opened. A reset + END_OF_TRACK whose header still arrives can then land after the subscriber + met the count and retired the alias. +- **Rust IETF subscriber, truncated first object.** `open_group` peeks the + first object with `?` before creating the group, so a stream reset or + truncated after its header drops that group, and since the stream still + counts, the track can end clean without it. Keep the END_OF_TRACK peek, but + create and abort the named group on any other peek failure. +- **Rust grace cuts off an arrived stream.** `Settle::poll` in + `rs/moq-net/src/tail.rs` returns when the grace fires even if an + END_OF_TRACK stream is mid-read, so the track finishes at the live edge and + the later marker is ignored. JS's `Tail.settle` already waits for active + streams; do the same. +- **JS lite floor.** `Math.max(entry.start, bounds.start)` in + `js/net/src/lite/subscriber.ts` ignores a SUBSCRIBE_UPDATE that lowered the + floor after SUBSCRIBE_START, so a newly requested lower group reordered + behind the FIN is dropped. `bounds.start` alone is wrong too: a subscriber + that asked below the announced start would wait for groups never promised. + The owed start has to remember the floor each request was answered at. + Check whether Rust's `SubStream::owed` has the same shape. + +Open questions. The maintainer chose to plan these without settling them; +decide in the PR, applying the choice to both languages: + +- **Are lost datagrams owed before a track ends?** Both `Tail`s account a + datagram only when it arrives, so a lost one leaves a hole in the owed span + and the end waits out the whole grace (JS was reported to wait and Rust + not, but Rust's `covers(owed)` reads the same way despite the comment in + `route_datagram`; confirm with a test first). Recommendation: no, datagrams + are best effort. On lite-07 the SUBSCRIBE_END stream count + ([lite-count-settle](/quest/m1/lite-count-settle.md)) settles without + looking at sequences, which makes this moot there; older versions keep the + grace as the documented stopgap. +- **Does a group at or past the declared end abort the track, or only that + group?** Rust aborts the track with `ProtocolViolation`; JS aborts the group + and ends clean. Recommendation: abort the track in both. The peer + contradicted its own end, and the repo fails loud on malformed input. +- **How is tail memory bounded?** Rust `Tail.accounted` gains a range per + permanent gap for the life of the subscription, and so does JS's. A gap + older than the grace can no longer be waited for, so folding it in as + accounted loses nothing. Recommendation: that, which bounds the ranges by + the gaps inside the grace window; lite-07 needs only the stream count. + +Regression tests go in `rs/moq-net/tests/track_tail.rs` (its `hold_unis` +mock makes the reorder deterministic) and the JS tail tests. Wire output only +changes if the stream-count fix does, and that is a correction to what the +drafts already require. + +## Related + +- [Track tail interop](/quest/m1/track-tail-interop.md) - the Rust-JS check that both sides now agree +- [lite-07 count settle](/quest/m1/lite-count-settle.md) - replaces sequence coverage with the stream count on lite-07 +- [Session death](/quest/m1/session-death.md) - how a tail ends when the session dies under it diff --git a/quest/m1/ts-export-jitter.md b/quest/m1/ts-export-jitter.md new file mode 100644 index 0000000000..5e0b138748 --- /dev/null +++ b/quest/m1/ts-export-jitter.md @@ -0,0 +1,57 @@ +# [S] moq export ts: video reorder bound follows the stream + +## Goal + +Two `moq export ts` legs of one broadcast interleave video and audio +identically even when a B-frame arrives late. Today the media-time interleave +from [#4001](https://github.com/moq-dev/moq/pull/4001) treats a video track as +advanced past a candidate once its high-water mark, less its DTS reserve, +passes it. Without catalog `jitter` that reserve is the 16-tick +`DEFAULT_DTS_RESERVE`, so a B-frame presenting below the mark can still land +after audio it should precede, and the order depends on arrival again. A +`jitter` that shows up in a later catalog is ignored too: `update_catalog` +returns early once the PAT/PMT is built, before the per-track refresh. + +## Plan + +Decided: + +- Keep refreshing a video track's reserve from catalog `jitter` after the PMT + is emitted. The early return exists to lock the track layout, not the + per-track timing, and the importer often fills `jitter` only once it has + seen reordering. +- When `jitter` is absent, derive the reserve from the stream rather than + refuse it. The bitstream's reorder depth (H.264 VUI + `max_num_reorder_frames`, HEVC `sps_max_num_reorder_pics`) counts + pictures, not time, so it becomes a deterministic 90 kHz bound only with + fixed timing: the VUI `fixed_frame_rate_flag` with its tick, or the + catalog framerate. With both, the reserve is depth times frame duration, + known from the first keyframe before any frame is emitted. AV1 and VP9 + present in decode order and need no reserve. Otherwise (variable frame + rate, or nothing declared), grow the reserve from the reordering actually + observed (how far a frame's PTS falls below the track's high-water mark); + it only grows, so a stream that never reorders keeps today's tiny reserve. +- Refusing to export video without `jitter` was rejected as too extreme. +- Treating unknown jitter as unbounded was tried in #4001 and rejected: the + stall never clears when video leads, breaking + `quiet_track_is_emitted_around_then_rejoins`. + +Guidance: + +- The same reserve feeds `author_dts`, so growing it mid-stream shifts DTS + further behind PTS. The existing monotonic clamp keeps DTS from stepping + back; check the PCR, which backs off by the largest reserve, stays ahead of + every DTS written. +- Only the observed fallback is nondeterministic: a B-frame deeper than any + seen before can reorder output once per new maximum (Codex on #4307). + Refusing such streams was ruled out by the maintainer. Say + so in a comment and log each growth, so an undeclared stream is visible + rather than silently misordered. +- Tests in `export_test.rs`: two exporters with a late B-frame, no `jitter`, + and a declared reorder depth at a fixed frame rate produce byte-identical + output from the first frame; an undeclared stream converges after its deepest reorder; and a + `jitter` arriving in a catalog after the PMT raises the reserve. + +## Related + +- [TS export byte schedule](/quest/m1/ts-export-byte-schedule.md) - also reshapes PCR placement in `export.rs` diff --git a/quest/m1/ts-import-shared-shift.md b/quest/m1/ts-import-shared-shift.md new file mode 100644 index 0000000000..6dd1fa513a --- /dev/null +++ b/quest/m1/ts-import-shared-shift.md @@ -0,0 +1,44 @@ +# [S] moq import ts: one re-anchor shift per program + +## Goal + +An unflagged loop wrap in `moq import ts` moves audio and video forward by the +same amount, so A/V sync holds across any number of wraps. Today each +elementary stream owns a `Reanchor` and grows its shift to reach its own live +edge ([#3997](https://github.com/moq-dev/moq/pull/3997)). Audio and video +edges sit on their own last frame starts, so each wrap drifts A/V by the +difference in frame durations: about 12 ms at 30 fps with 48 kHz AAC, or +1.7 s a day on a 10-minute loop. + +## Plan + +Decided: a program-level shared shift in `rs/moq-mux/src/container/ts/import.rs`. +Every stream of a program applies the same shift, grown once per wrap by the +largest amount any stream needs to clear its edge, which preserves the source's +inter-stream offsets. A timebase break (PCR discontinuity) clears it for the +whole program, as it already does per stream. + +Guidance: + +- The shift must be known before any stream emits a frame of the new + generation (Codex on #4307): a first stream that grows only enough for + itself leaves a later-arriving stream below its edge, and growing again + then drifts the two. Decided: hold each stream's post-wrap frames until + every live stream of the program has shown its new PTS, then take the + maximum growth once. A stream that stays silent is bounded by the + existing liveness timeout (#3489), not waited on forever. The + `Anchor`/`Lane` split behind `live()` in `moq_mux::clock` solves the same + problem for restarts and may be reusable. +- A stream whose own edge is still above the shifted timestamp after the + shared growth (its tail ran longer) is the case that forces growing by the + maximum. Landing on its edge is accepted today; keep that trade-off. +- Sections already take the video shift, so cues keep following pictures. + MPEG-2 video (`Stream::Clock`) has no track and so no edge, but it should + read the shared shift too. +- Tests beside the existing loop-wrap tests: a muxed H.264 + AAC loop whose + period is not a multiple of either frame duration keeps the first audio and + video timestamps of each pass at the source offset across three wraps. + +## Related + +- [#3489](/quest/m1/3489-ts-import-stream-liveness.md) - per-PID liveness in the same importer; touches `Stream` but not the shift diff --git a/quest/m1/unknown-session-logs.md b/quest/m1/unknown-session-logs.md new file mode 100644 index 0000000000..f8285c35a4 --- /dev/null +++ b/quest/m1/unknown-session-logs.md @@ -0,0 +1,43 @@ +# [S] UnknownSession log flood + +## Goal + +moq.pro relays stop logging +`web_transport_moq::session: failed to decode unidirectional stream err=WebTransportError(UnknownSession)` +for streams that were simply reset or cut off before their WebTransport +header arrived. A stream that really names another session is still reported +as `UnknownSession`, and the error a caller sees says what happened. + +## Plan + +Seen in production at a rate like the old-group warnings +https://github.com/moq-dev/moq/pull/4208 fixed, on the same hosts. + +Likely root cause, to confirm with a test first: `decode_uni` (and +`decode_bi`) in `web-transport-moq`'s `session.rs` (repository +`moq-dev/noq`) map every failure to read the stream type or session ID to +`UnknownSession`. A read fails whenever the peer resets the stream before +those bytes arrive, which moq does routinely: a publisher resets a group's +stream when the group is superseded or expires, and without reliable reset +the header can be discarded with it. `poll_accept_uni` then logs it at WARN, +though its own comment says the stream "was probably reset early". + +- Reproduce in `web-transport-moq`'s tests: open a WebTransport uni stream, + reset it before the header is delivered, and assert the accept loop + reports a reset rather than `UnknownSession`. +- Fix the mapping at the source: carry the read's real cause (reset, closed, + truncated), keep `UnknownSession` for a session-ID mismatch, and log the + expected reset at debug. Not a log filter on the moq side. +- `web-transport-quinn` in `moq-dev/web-transport` carries the same code; + fix it too if it is still published. +- Release and bump the pin here. Confirm on a moq.pro relay that the rate + drops, and that any remaining `UnknownSession` lines are real mismatches. + +If the reproduction shows something other than early resets (for example a +peer really using another session ID), stop and report before changing the +log level. + +## Related + +- [Close codes on every transport](/quest/m1/close-codes.md) - another error mapping fix in the same crate +- [Reliable stream reset](/quest/m1/quic/reliable-reset.md) - keeps a reset stream's header, which removes most of these diff --git a/quest/m1/video-surface.md b/quest/m1/video-surface.md new file mode 100644 index 0000000000..d5e7cdc9f5 --- /dev/null +++ b/quest/m1/video-surface.md @@ -0,0 +1,36 @@ +# [XS] FFI decoded frames expose a surface, only where one exists + +## Goal + +moq-ffi names the decoder's retained picture the way moq-video does, and a +caller cannot opt into it on a platform that has none. On `dev` +([#4094](https://github.com/moq-dev/moq/pull/4094)) the frame's view is +`MoqVideoNative` from `native()`, enabled by `MoqVideoDecoderOutput.native`. +Only macOS has a variant (`PixelBuffer`). Elsewhere the opt-in still decodes +to native surfaces, `native()` always returns `None`, and a tiled VAAPI +DMA-BUF also fails `pixels()`, so the caller gets frames it cannot read. + +## Plan + +Decided: + +- Rename to surface naming, mirroring `moq_video::Surface`: + `MoqVideoSurface`, `surface()`, and the matching `MoqVideoDecoderOutput` + flag. "Native" named today's implementation, not the role. +- Refuse the opt-in at subscribe time on platforms with no surface variant + (Windows and Linux today) with a clear unsupported error. Each platform + lifts the refusal when its variant lands with hardware proof, in + [decode-windows](/quest/m1/obs-moq-video/decode-windows.md) and + [decode-linux](/quest/m1/obs-moq-video/decode-linux.md). + +Guidance: + +- Whether `moq_video::Output::Native` should follow the rename is a + separate call; ask before touching it. +- Update the wrappers and docs per the root `AGENTS.md` sync table: the Go, + Python, Swift, and Kotlin option docs mention the native surface flag, and + `quest/m1/obs-moq-video/source.md` and the release plan on `dev` name + `native`. +- A `dev` break on top of #4094, so it rides the same release. +- Test: the opt-in is refused where there is no variant, and on macOS + `surface()` returns the pixel buffer. diff --git a/quest/m1/wire-compat.md b/quest/m1/wire-compat.md new file mode 100644 index 0000000000..98cef18c3e --- /dev/null +++ b/quest/m1/wire-compat.md @@ -0,0 +1,61 @@ +# [M] Nightly wire compatibility against the last release + +## Goal + +A nightly run pits this checkout against the last published release and +fails when they stop understanding each other, before the break ships. +It covers, in both directions: + +- **Tokens:** the current `moq-auth` signs and the last published `moq-cli` + and `@moq/auth` verify, and the published side signs while the current + side verifies. +- **Session wire:** the current relay and clients against the last published + `moq-cli`, `moq-relay`, and `@moq/net`, on every lite and IETF version + both sides publish: publish, subscribe, announce, and fetch. +- **Catalog and container:** the last published `hang` and `@moq/hang` read + the current catalog and frames, and the current ones read theirs. + +## Plan + +Motivated by https://github.com/moq-dev/moq/pull/4190, where a token format +change broke every published credential and only moq-dev/smoke#49 noticed. +Neither existing harness asks this question: moq-dev/smoke tests published +against published, and `test/interop` builds every client from the checkout +(see `test/interop/README.md`). + +Decided by the maintainer: + +- **Nightly only**, not per PR. Installing released packages is slow, and + a break only needs catching before the next release. +- **Resolve the last release at run time** (the crates.io and npm registries' + newest non-yanked version), never a hand-bumped pin that goes stale. Log + the resolved versions in the run so a failure names both sides. +- **Not bindings.** Python, Go, Swift, Kotlin, and C stay with smoke and + `test/interop`. They wrap the same Rust, so the Rust lanes see their wire. + +Guidance: + +- Reuse `test/interop`'s relay and client drivers rather than build a second + harness. The new axis is which side comes from the registry, so a + "published" client source next to the checkout one may be enough. +- Derive the version matrix from what both sides accept (for example each + CLI's `--connect-version` choices), not a hand-kept list, so a new draft + joins the matrix and a dropped one leaves it without an edit. A version + only the checkout offers is skipped and logged. A version the last release + supports but the checkout no longer offers fails the run unless it is + acknowledged in the same skip list as planned breaks (decided with the + maintainer; silently dropping a published version is the regression this + exists to catch). +- Prefer released binaries (GitHub release assets) over `cargo install` of + the published crates if the build time threatens the nightly budget. +- Subscribers must decode the catalog and frames, not only see a non-empty + frame, or the container lane proves nothing. +- A deliberate break on `dev` is expected to fail against `main`'s release. + Run against `main` only, and document in the harness how a planned break + is acknowledged (for example a skip list that the next release clears). +- Wire the job into the existing nightly (`interop.yml` already has a + schedule) and document the recipe beside `just test interop`. + +## Related + +- [Track tail interop](/quest/m1/track-tail-interop.md) - another cross-language case in the same harness diff --git a/quest/m2/README.md b/quest/m2/README.md index 80fa09dd1b..3dfcb2f068 100644 --- a/quest/m2/README.md +++ b/quest/m2/README.md @@ -20,6 +20,8 @@ upstream release waits in [m4](/quest/m4/README.md). - [Catalog track identity](/quest/m2/catalog-tracks.md) - compare immutable track definitions with explicit catalog-to-group binding - [Archive recovery listing](/quest/m2/archive-recovery-listing.md) - a resumed DVR lists what changed since its checkpoint, not every stored group - [Archive backward timestamps](/quest/m2/archive-backward-timestamps.md) - a resumed recording refuses a track whose timestamps go backward +- [moq play drain tail](/quest/m2/play-drain-tail.md) - retired renditions and finite tracks play their last 10 ms of audio +- [Relay io_uring packages](/quest/m2/relay-io-uring-package.md) - Linux relay packages ship io_uring once the ring is on par with tokio - [Mobile ownership](/quest/m2/mobile-ownership.md) - decide whether Rust or platform code owns mobile capture, codecs, and rendering - [iOS capture](/quest/m2/mobile-capture-ios.md) - camera and screen capture if the mobile ownership decision selects Rust - [Android capture](/quest/m2/mobile-capture-android.md) - NDK/JNI capture using the existing codecs if mobile ownership selects Rust diff --git a/quest/m2/audio-loss-recovery.md b/quest/m2/audio-loss-recovery.md index 33480c6391..9be8b40dc0 100644 --- a/quest/m2/audio-loss-recovery.md +++ b/quest/m2/audio-loss-recovery.md @@ -25,4 +25,4 @@ audio settings. Existing wire compatibility must be demonstrated. ## Related -- [Audio quality](/quest/m1/audio-quality-harness/README.md) - quality and latency measurements +- [Audio quality](/quest/m0/audio-quality-harness/README.md) - quality and latency measurements diff --git a/quest/m2/audio-opus-backend.md b/quest/m2/audio-opus-backend.md index 3f1e682071..a981f0e007 100644 --- a/quest/m2/audio-opus-backend.md +++ b/quest/m2/audio-opus-backend.md @@ -24,4 +24,4 @@ before a separately scoped backend implementation. ## Related -- [Audio quality](/quest/m1/audio-quality-harness/README.md) - shared measurement infrastructure +- [Audio quality](/quest/m0/audio-quality-harness/README.md) - shared measurement infrastructure diff --git a/quest/m2/cat/present.md b/quest/m2/cat/present.md index 7a2cf3d821..82d11a0334 100644 --- a/quest/m2/cat/present.md +++ b/quest/m2/cat/present.md @@ -19,16 +19,22 @@ the option fails loud instead of dropping the credential. keeps taking a JWT; `--connect-cat ` (and `MOQ_CONNECT_CAT`) adds a CAT. One CAT per connection: it is the connection credential, so a configured CAT takes the setup option and the JWT that would have gone - there rides its AUTH stream instead. + there rides its AUTH stream instead, never the URL. `moq auth serve` + refuses a SETUP token beside a different `?jwt=` (see [Token in + band](/quest/m1/auth/token-in-band.md)), so a JWT on the URL would refuse the + CAT's connect; on a session without AUTH the JWT is simply absent, like + any extra token. - A CAT with any offered version that lacks the setup option (every lite version) fails `init` with `Unsupported` naming the token; `js/net` rejects the same way before dialing. - `moq` CLI publish and subscribe get the flag through the shared connect config; `doc/bin/cli.md` and `doc/lib/rs/moq-net.md` gain it. -- Tests: the server request sees kind `0x01` and the bytes on every draft - in Rust, JS, and across; a JWT still goes to the URL or AUTH stream when - a CAT holds the option; a lite offer with a CAT refuses at init; end to - end against `moq auth serve` with a CAT from `moq auth sign --format cat`. +- Tests: a Rust server's `Handshake::token()` sees kind `0x01` and the + bytes on every draft from both a Rust and a JS client (a JS server is out + of scope: #4278 added no JS accept-side API, since nothing in `js/net` authorizes an IETF session); a JWT rides its + AUTH stream and not the URL when a CAT holds the option; a lite offer with + a CAT refuses at init; end to end against `moq auth serve` with a CAT from + `moq auth sign --format cat`. Public API: additive on `moq-tokio` and `js/net`. Wire: none. diff --git a/quest/m2/cat/verify.md b/quest/m2/cat/verify.md index c77c2c532e..46df67ba0e 100644 --- a/quest/m2/cat/verify.md +++ b/quest/m2/cat/verify.md @@ -78,7 +78,10 @@ rather than a set of prefixed names; `@moq/auth` stays flat. vectors round-tripped both ways; expiry and not-before; the grant produced from every c4m-01 example; serve admitting a CAT over a real moq-transport session on every supported draft and refusing an unknown - token kind, a bad MAC, and a token plus `jwt`; the CLI round trip. + token kind, a bad MAC, and a CAT beside any `jwt` (a CAT is type `0x01` + and a `jwt` is a JWT, so equal bytes are still two credentials; only a + type-0 SETUP token equal to `jwt` is admitted once, per [Token in + band](/quest/m1/auth/token-in-band.md)); the CLI round trip. Public API: `moq_auth::cat` new, `moq auth serve` and `moq auth sign|verify` gain flags. Wire: none. diff --git a/quest/m2/latency-ledger.md b/quest/m2/latency-ledger.md index de59283362..9c5f1edad4 100644 --- a/quest/m2/latency-ledger.md +++ b/quest/m2/latency-ledger.md @@ -29,9 +29,9 @@ trade to get it running. This quest promotes them. ## Required -- [Audio quality harness](/quest/m1/audio-quality-harness/README.md) - defines the stage schema and lands the probes this promotes +- [Audio quality harness](/quest/m0/audio-quality-harness/README.md) - defines the stage schema and lands the probes this promotes ## Related -- [Audio quality harness](/quest/m1/audio-quality-harness/README.md) - defines the stages and is the first consumer +- [Audio quality harness](/quest/m0/audio-quality-harness/README.md) - defines the stages and is the first consumer - [QoS](/quest/m1/qos/README.md) - relay-side health, the same idea from the other end diff --git a/quest/m2/play-drain-tail.md b/quest/m2/play-drain-tail.md new file mode 100644 index 0000000000..bed7d7e0ed --- /dev/null +++ b/quest/m2/play-drain-tail.md @@ -0,0 +1,24 @@ +# [XS] moq play plays a finished track's last samples + +## Goal + +When `moq play` retires an audio rendition or reaches the end of a finite +track, every sample it wrote reaches the speaker. Today `drain` in +`rs/moq-cli/src/play/media.rs` returns once 10 ms or less is buffered and +drops the sink, which removes it from the mix, so each retired rendition and +finite track loses up to its last 10 ms +([#4154](https://github.com/moq-dev/moq/pull/4154)). The rendition-switch test +tolerates the gap. + +## Plan + +The 10 ms stop exists because polling the last partial period costs a wakeup +per iteration and never settles. Fix it at the sink instead of the poll: let +a dropped or finished `moq_audio::playback::Sink` play out what it holds +before leaving the mix, or give it an end-of-stream that the mixer honors, so +`drain` no longer needs a threshold. If that belongs in moq-audio's playback +API, keep the change additive. + +Tighten `an_audio_rendition_switch_leaves_no_gap` (the `play::fake::Recorder` +already models the cut on drop) so the lost tail fails it, and add a finite +track that asserts its final samples are heard. diff --git a/quest/m2/relay-io-uring-package.md b/quest/m2/relay-io-uring-package.md new file mode 100644 index 0000000000..23dc0e46d9 --- /dev/null +++ b/quest/m2/relay-io-uring-package.md @@ -0,0 +1,41 @@ +# [S] Linux relay packages ship io_uring + +## Goal + +The official Linux moq-relay builds (the `.deb`, `.rpm`, and tarballs from +`.github/workflows/moq-relay.yml`, and the Nix package in `nix/overlay.nix`) +are compiled with the `io-uring` feature, so `--runtime-io-uring` works on a +packaged relay. Today none of them enable it, yet the packaged systemd unit +sets `LimitMEMLOCK=infinity` for io_uring +([#4197](https://github.com/moq-dev/moq/pull/4197)): the unit prepares for a +runtime the binary cannot run. The runtime stays opt-in; this changes what +ships, not the default. + +## Plan + +- Decided: ship it, but only once the io_uring runtime is on par with the + tokio one for what a packaged relay promises: every configured protocol + served, the `[quic]` tuning honored, and closes delivered. Offering a flag + that silently serves less than the default runtime would be worse than not + offering it. Performance work is not a prerequisite. +- Enable the feature only on Linux targets. Confirm the zigbuild glibc 2.34 + build still links and that nothing new is needed at runtime. +- A kernel without the io_uring features the workers need must make + `--runtime-io-uring` fail loud at startup with the reason, not fall back. +- Package smoke check in the release workflow: start the packaged binary + with `--runtime-io-uring` on the runner, accept one session, and exit, + failing the release if it cannot. Check it under the unit's limits too, + since memlock is why #4197 touched the unit. +- Docs: `doc/bin/relay/` says packaged Linux builds include the runtime, how + to turn it on, and the memlock note. + +Public API: none. Wire: none. + +## Required + +- [moq-transport on io_uring](/quest/m1/uring-ietf.md) - a packaged relay + must not drop protocols when the ring is on +- [Flow-control windows](/quest/m1/uring-flow-control-windows.md) - the + `[quic]` section must not be refused at startup on the ring +- [Close before teardown](/quest/m1/quic/uring-close.md) - sessions on the + ring must end with their application close diff --git a/quest/m3/README.md b/quest/m3/README.md index 232e225d3b..2f76365b3d 100644 --- a/quest/m3/README.md +++ b/quest/m3/README.md @@ -24,3 +24,4 @@ condition clears, move the quest to the milestone its work belongs in. - [libmoq shutdown](/quest/m3/libmoq-shutdown.md) - OBS exits cleanly with the plugin loaded: a C ABI `moq_shutdown` stops the libmoq thread before the module is unloaded - [libmoq CMake library](/quest/m3/libmoq-cmake-lib.md) - the in-tree CMake build links the `libmoq.a` cargo reports, not a hardcoded `target/` path - [libmoq fetch](/quest/m3/libmoq-fetch.md) - libmoq gains an additive cached-group fetch entry point +- [Upstream forks](/quest/m3/upstream-forks.md) - offer the uniffi generator fixes our cpp, dart, and Python forks carry upstream, lowest priority diff --git a/quest/m3/upstream-forks.md b/quest/m3/upstream-forks.md new file mode 100644 index 0000000000..9ae49440ac --- /dev/null +++ b/quest/m3/upstream-forks.md @@ -0,0 +1,55 @@ +# [S] Offer the uniffi generator fixes upstream + +## Goal + +Every general fix our uniffi generator forks carry has been offered to its +upstream, or recorded as declined or MoQ-specific, so each fork shrinks +toward a pin on an upstream tag. Very low priority: the forks work, and the +first step is someone else's review. Every external post, issue, or PR needs +the maintainer's approval at the time it is made. + +One outcome per candidate: + +- **uniffi-bindgen-cpp** (`kixelated/uniffi-bindgen-cpp`, forked from + LiveKit's `uniffi-0.31-async`, PR #1; + [#4100](https://github.com/moq-dev/moq/pull/4100)): the two leak fixes (a + ready Rust future freed without `rust_future_complete`, and a dropped + foreign future that never completed its oneshot), the missing + `#include ` MSVC needs, the uniffi 0.32 port, and + `error_style = "expected"`. The bug fixes are the easy offer; the 0.32 port + and the expected style depend on LiveKit taking async at all. +- **uniffi-dart** (`kixelated/uniffi-dart`; + [#4072](https://github.com/moq-dev/moq/pull/4072)): the + `nix/uniffi-dart-record-error.patch` and the RustBuffer release fixes. Fix + the latent `lowerForeignBytes` leak first (borrowed `&[u8]` arguments + allocate a `ForeignBytes` nothing frees; the free belongs after the call, + not inside the lowering), and audit callback interfaces for the same + RustBuffer leak, so the upstream offer is complete. `moq_ffi` uses neither + today. +- **uniffi-rs Python typing** of data-carrying enum variants + ([#4049](https://github.com/moq-dev/moq/pull/4049)): the generated type + makes `moq.VideoEncoderKind.AUTO()` fail pyright, so + `doc/lib/py/index.md` carries a `pyright: ignore`. Fix it at the source + and drop the ignore once a release carries it. + +## Plan + +- Decided in #4100: fork tags keep upstream's `v+v` + scheme with a `-kixelated.N` pre-release, e.g. + `v0.11.0-kixelated.1+v0.32.2`, matching the Dart fork. It never collides + with an upstream tag. Under SemVer a pre-release sorts before its base, so + this only works because every pin names the exact tag; never let tooling + pick "the newest" fork tag (Codex on #4307). The Go fork (`v0.9.0+v0.32.0`) moves to it + at its next bump. +- Offer each fix with the regression test it landed with. When upstream + merges one, move the pin in `flake.nix` (and the places its comment lists) + and delete the carried patch. +- Record each outcome (merged, declined with the reason, or not offered + because it is MoQ-specific) in the PR that finishes this quest. + +Public API: none. Wire: none. + +## Related + +- [Upstream the fork](/quest/m1/quic/upstream.md) - the same practice for the noq fork +- [C++ through moq-ffi](/quest/m1/cpp/README.md) - the line that forked the C++ generator