-
-
Notifications
You must be signed in to change notification settings - Fork 246
chore(quest): plan the ten unlabeled issues #3539
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
c8d0c38
4cedf01
1c9b206
05d8b39
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| # [XS] TRACK_STATUS is refused, not dropped | ||
|
|
||
| ## Goal | ||
|
|
||
| A moq-transport peer that sends TRACK_STATUS gets an immediate refusal with the | ||
| registered NOT_SUPPORTED code on every negotiated draft, instead of a warning in | ||
| our log and a request that hangs until the peer's timeout. TRACK_STATUS itself | ||
| stays unimplemented: all it adds over TRACK_INFO is a snapshot of the latest | ||
| object, and answering it honestly needs either a new API or a throwaway | ||
| SUBSCRIBE. PR #3519 built the latter and was closed for it. | ||
|
|
||
| ## Plan | ||
|
|
||
| `rs/moq-net/src/ietf/publisher.rs` matches `ietf::TrackStatus::ID` with a | ||
| warning and an empty future; `session.rs` already routes the stream there, so | ||
| nothing is session-fatal. Mirror `run_publish_stream` in `subscriber.rs`, which | ||
| answers PUBLISH with NOT_SUPPORTED and closes the writer explicitly (#3348): | ||
|
|
||
| - Decode the request so the stream is consumed, reply with the per-draft | ||
| refusal (TRACK_STATUS_ERROR on draft-14, REQUEST_ERROR from draft-15 on) using | ||
| the code type [IETF error codes](/quest/m0/ietf-error-codes.md) introduces, | ||
| and close the writer. On draft-14 and 15 the request rides a virtual stream | ||
| over the control stream whose reset is a no-op, so the explicit reply is the | ||
| only way bytes reach the peer. | ||
| - `js/net/src/ietf/publisher.ts` already replies; make it send the same code. | ||
| - Tests: a byte-exact transport-log test per version that a TRACK_STATUS request | ||
| yields the refusal and nothing else. | ||
|
|
||
| Branch from dev, where the error registry lands. | ||
|
|
||
| ## Required | ||
|
|
||
| - [IETF error codes](/quest/m0/ietf-error-codes.md) - the registered NOT_SUPPORTED value per draft comes from its code type | ||
|
|
||
| ## Closes | ||
|
|
||
| - [#3492](https://github.com/moq-dev/moq/issues/3492) - close this issue when the quest finishes |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| # [S] A connect fails on auth only once every transport has | ||
|
|
||
| ## Goal | ||
|
|
||
| `moq_native::Client::connect` against a WebTransport-only endpoint succeeds | ||
| whenever the QUIC dial succeeds, even when the WebSocket fallback is refused | ||
| first. Cloudflare's relays answer every non-WebTransport request with 403, and | ||
| today that 403 landing inside the 200 ms fallback delay fails the whole connect | ||
| as Forbidden while the QUIC arm is still in flight, indistinguishable from a bad | ||
| token. An auth error ends the race only when the other arm has also failed, and | ||
| a moq-ffi caller can disable the WebSocket fallback the way libmoq and the CLI | ||
| already can. | ||
|
|
||
| Boundaries: blind subscription, consuming a broadcast the peer never announced, | ||
| stays refused. A client here may hold several connections and cannot pick one | ||
| for a bare path, so the announcement is the signal a broadcast is online; a | ||
| relay that accepts SUBSCRIBE_NAMESPACE and never publishes a namespace is | ||
| non-conformant, not a gap in this repo. | ||
|
|
||
| ## Plan | ||
|
|
||
| - `race_transport_connect` in `rs/moq-native/src/client.rs` returns on | ||
| `err.is_auth()` from either arm. Record an auth error like any other failure | ||
| and keep polling the other arm. When both are done, the result is an auth | ||
| error only if both arms rejected authentication; otherwise report the | ||
| non-auth failure, which stays retryable, because `Reconnect::run` exits on | ||
| `is_auth()` and a fallback endpoint that answers every non-WebTransport | ||
| request with 403 must not stop the client from retrying a QUIC dial that | ||
| merely failed transiently. Flip | ||
| `race_transport_connect_stops_on_quic_auth_error` and add: WebSocket 403 then | ||
| QUIC success connects; both arms refusing reports Forbidden; a transient QUIC | ||
| failure after a WebSocket 403 reports the QUIC error and the reconnect loop | ||
| retries. | ||
| - `MoqClient` in `rs/moq-ffi/src/session.rs` gains `set_websocket_enabled` and | ||
| `set_websocket_delay` beside `set_tls_disable_verify`, mapping onto | ||
| `websocket::Client::{enabled, delay}`. libmoq already exports | ||
| `moq_client_set_websocket_enabled` and `_delay`; mirror the two setters in | ||
| `py/moq-rs`, `swift`, `kt`, `go/wrapper/moq`, `dart/moq`, and | ||
| `doc/lib/{py,swift,kt,go,dart}`. | ||
|
|
||
| On main, additive. | ||
|
|
||
| ## Closes | ||
|
|
||
| - [#3532](https://github.com/moq-dev/moq/issues/3532) - close this issue when the quest finishes |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| # [S] TS export survives a content restart on a continuous timeline | ||
|
|
||
| ## Goal | ||
|
|
||
| `moq export ts` keeps emitting every elementary stream across a content join | ||
| on a source whose transport timeline is continuous: PCR, PTS and DTS monotone | ||
| on the wire, continuity counters unbroken, no `discontinuity_indicator`. That is | ||
| what a real encoder produces at a hard cut. The only backwards step in that | ||
| scenario is the importer's: the legacy audio importer extrapolates timestamps | ||
| from the last PES header, so after a resync at the join it re-locks a frame a | ||
| few milliseconds below its own extrapolated high-water mark. A rewind detected | ||
| on one track may cost the program one clock and PSI reset, but it never fences | ||
| another track for good, and the true rewind recovery #3375 added, a looping | ||
| file where every track steps backwards, keeps withholding stale frames. | ||
|
|
||
| Boundaries: the consumer's rewind detection and the legacy audio importer's | ||
| resync are untouched here. Retiring inferred rewinds altogether is | ||
| [Monotonic timeline](/quest/m1/monotonic-timeline.md). | ||
|
|
||
| ## Plan | ||
|
|
||
| Since #3375, `rewind(backwards)` in `rs/moq-mux/src/container/ts/export.rs` | ||
| bumps the program epoch and, on a backwards boundary, leaves every track that | ||
| already has a timeline in the old epoch. `Track::admit` then discards that | ||
| track's frames until it both changes its discontinuity counter and steps below | ||
| its own high-water mark. A continuous source supplies neither. The consumer's | ||
| rewind check in `rs/moq-mux/src/container/consumer.rs` has no tolerance, so the | ||
| sub-frame backwards step the legacy importer produces on an MPEG-1 audio resync | ||
| at the join is read as a rewind, and video plus primary audio are fenced | ||
| permanently while the passthrough PSI, AC-3 and teletext continue. #3533 | ||
| measured 0.31 Mb/s against a 9.5 Mb/s source with no recovery over 40 minutes, | ||
| bisected to #3375, and showed a single-track source is immune because the fence | ||
| needs a bystander. | ||
|
|
||
| - Give the fence an exit. A fenced track joins the new generation when its own | ||
| timeline steps back, as today, or when the program clock driven by the joined | ||
| tracks passes its pending frame's timestamp. On a true rewind the video | ||
| track's own boundary arrives long before the reset clock climbs back to its | ||
| stale frames, so they are still discarded, which is what | ||
| `rewind_flags_the_break_once_across_tracks` asserts. On the #3533 join the | ||
| reset clock sits a few milliseconds below video's next frame, so video | ||
| re-joins within one frame. Implement it in `Track::admit` against the | ||
| exporter's watermark rather than as a timer: the exit is a clock comparison, | ||
| never a deadline. A frame at or below the watermark is admitted, a frame | ||
| above it stays fenced, and `rewind()` clears the watermark, so nothing | ||
| re-joins until a joined track has emitted; `fill` passes the current | ||
| watermark into admission so the rule has one definition. | ||
| - Regression test in `export_test.rs`: two tracks on a continuous transport | ||
| timeline whose content restarts, the audio track alone stepping back by less | ||
| than one frame at the join; video and audio keep emitting across it, and the | ||
| join costs at most one PCR discontinuity and one PSI re-emission. The existing | ||
| `rewind_re_emits_tables_and_resumes_the_clock` and | ||
| `rewind_flags_the_break_once_across_tracks` keep passing unchanged. | ||
| - Land after [TS timebase discontinuity](/quest/m0/ts-forward-discontinuity.md) | ||
| (PR #3529), which edits the same functions and makes the legacy importer | ||
| declare its breaks. | ||
|
|
||
| ## Required | ||
|
|
||
| - [TS timebase discontinuity](/quest/m0/ts-forward-discontinuity.md) - PR #3529 rewrites the same boundary handling; rebase on it rather than race it | ||
|
|
||
| ## Closes | ||
|
|
||
| - [#3533](https://github.com/moq-dev/moq/issues/3533) - close this issue when the quest finishes | ||
|
|
||
| ## Related | ||
|
|
||
| - [Monotonic timeline](/quest/m1/monotonic-timeline.md) - deletes the inferred rewind that triggers the false boundary | ||
| - [Gap discontinuity](/quest/m1/gap-discontinuity.md) - the declared-break model the exporter will read instead |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| # [M] Publisher priority survives a moq-transport hop | ||
|
|
||
| ## Goal | ||
|
|
||
| A track's publisher priority reaches a moq-transport subscriber the way it | ||
| already reaches a moq-lite one. A relay that ingests over either protocol and | ||
| serves over moq-transport stamps every group header with the track's priority | ||
| instead of 0, a peer that declares DEFAULT_PUBLISHER_PRIORITY (0x21) has it | ||
| recorded on the track, and a subgroup header that omits its priority resolves | ||
| to that declared default. The audio-ahead-of-video intent of a linear SSAI | ||
| publisher is what the next relay sees, and the moqtest conformance requirement | ||
| that a relay preserve publisher priority is met. | ||
|
|
||
| Boundaries: priority is track-scoped, as `track::Info::priority` and | ||
| draft-lcurley-moq-lite define it, fixed for the lifetime of the track. A | ||
| subgroup priority that differs from its track's collapses to the track value. | ||
| The local send queue keeps ranking by each subscription's own priority. | ||
|
|
||
| ## Plan | ||
|
|
||
| The model has the field: `track::Info::priority` (higher first) rides TRACK_INFO | ||
| on moq-lite, and `rs/moq-net/src/ietf/priority.rs` converts to the wire's | ||
| lower-first byte. The IETF side ignores it in both directions: | ||
|
|
||
| - `rs/moq-net/src/ietf/publisher.rs` builds the group header with | ||
| `publisher_priority: 0`. | ||
| - `rs/moq-net/src/ietf/properties.rs` names TIMESCALE (0x08) and | ||
| DEFAULT_PUBLISHER_GROUP_ORDER (0x22); 0x21 falls through the unknown path. | ||
| - `rs/moq-net/src/ietf/group.rs` decodes an absent priority flag as a literal | ||
| 128. | ||
| - `rs/moq-net/src/ietf/subscriber.rs` builds `track::Info::default()` from | ||
| SUBSCRIBE_OK with only timescale and latency. | ||
|
|
||
| Steps: | ||
|
|
||
| - Properties: add DEFAULT_PUBLISHER_PRIORITY (0x21) beside 0x22. Encode it on | ||
| SUBSCRIBE_OK and PUBLISH from `info.priority` through `priority::to_wire`; | ||
| decode it into `track::Info::priority` on the subscriber through | ||
| `priority::from_wire`. The property block is written from draft-17 on and | ||
| read from draft-16 on, as `Properties::encode` already gates; draft-14 and 15 | ||
| have no block at all, so on those drafts the priority travels only in the | ||
| group header and an absent flag resolves straight to the draft's fallback. | ||
| - Group header: the publisher stamps `priority::to_wire(track.info().priority)` | ||
| where it reads the timescale today. The decoder resolves an absent flag to the | ||
| track's declared default first and only then to the draft's fallback; confirm | ||
| that fallback against each negotiated draft's text instead of keeping 128 by | ||
| assumption, and cite the section in the type's docs. On the subscriber the | ||
| header value is decoded and then dropped: the model has no per-group | ||
| priority, so an explicit subgroup value that disagrees with the track's | ||
| declared priority never overrides `track::Info::priority`, and a test pins | ||
| that a conflicting header leaves the track's priority unchanged. | ||
| - Mirror in `js/net/src/ietf/publisher.ts`, `object.ts`, and `properties.ts`. | ||
| - Tests: 0x21 round-trips on SUBSCRIBE_OK on every draft that carries the | ||
| block and is absent from the bytes on the ones that do not; a lite-ingested track | ||
| with priority N serves over moq-transport with header priority `to_wire(N)`; | ||
| a subgroup without the flag decodes to the declared default; a relay | ||
| integration test where hang audio (priority 80) and video (60) arrive at a | ||
| moq-transport subscriber with distinct header priorities. Run the interop | ||
| runner's priority case if it has one. | ||
|
|
||
| Additive, on main. No draft change: 0x21 is IETF-registered and moq-lite already | ||
| specifies the field. | ||
|
|
||
| ## Closes | ||
|
|
||
| - [#3534](https://github.com/moq-dev/moq/issues/3534) - close this issue when the quest finishes | ||
|
|
||
| ## Related | ||
|
|
||
| - [IETF error codes](/quest/m0/ietf-error-codes.md) - the sibling sweep of the moq-transport registries | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| # [XS] Every WebKit browser takes the WebSocket path | ||
|
|
||
| ## Goal | ||
|
|
||
| js/net refuses WebTransport on every WebKit engine, not only the Safari brand. | ||
| Chrome, Firefox and Edge on iOS and iPadOS are WKWebView and hit the same WebKit | ||
| bug (319818: flow-control credit never refills, so a session dies after roughly | ||
| 7,600 streams or 16 MiB), yet `isWebTransportUserAgentSupported` admits them | ||
| today and their sessions freeze after about two minutes of playback. | ||
|
|
||
| ## Plan | ||
|
|
||
| `js/net/src/connection/browser.ts` gates with Bowser `satisfies({ safari: "<0" })`. | ||
| Add the engine check: a browser whose OS is iOS or iPadOS, or whose engine is | ||
| WebKit, returns false, and Firefox keeps its version gate. Tests in | ||
| `browser.test.ts` with user agents for Safari on macOS, Chrome on iOS, Firefox on | ||
| iOS, Edge on iOS, and desktop Chrome and Firefox. Extend the Safari note in | ||
| `doc/lib/js/index.md` to WebKit. | ||
|
|
||
| ## Related | ||
|
|
||
| - [Safari WebTransport](/quest/m2/safari-webtransport.md) - relaxes this gate once WebKit ships the fix | ||
| - [#2388](https://github.com/moq-dev/moq/issues/2388) - the WebKit tracking issue that gate quest closes |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| # [XS] moq-cli runs several adaptive import stages on one connection | ||
|
|
||
| ## Goal | ||
|
|
||
| Two encoding stages over one connection, such as `moq ... capture -- capture`, | ||
| are accepted, and both encoders target shares of the connection's estimate that | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| sum to at most it, surplus left unclaimed when a ceiling binds. The refusal in `rs/moq-cli/src/args.rs`, "a stage that encodes to fit | ||
| the connection's bandwidth estimate assumes it's the only publisher", is gone | ||
| with the test that asserts it. | ||
|
|
||
| ## Plan | ||
|
|
||
| The allocator #2854 landed on dev is what the refusal stood in for: `main.rs` | ||
| mints one `bandwidth::Allocator` per connection and every encoding sender | ||
| reserves against it (the `moq-video` encode producer, `moq-audio` capture), so | ||
| the guard refuses a configuration the allocator already divides by track | ||
| priority. | ||
|
|
||
| - Delete the `adaptive` / `imports == 1` ensure and | ||
| `an_adaptive_capture_must_be_the_only_import`. Keep | ||
| `audio_only_capture_is_not_bandwidth_adaptive` only if `uses_bandwidth` still | ||
| has a reader; otherwise delete both. | ||
|
Comment on lines
+19
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For two ordinary AGENTS.md reference: AGENTS.md:L65-L68 Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Keeping the refusal lifted. The allocator is advisory by design (#2854: a track that ignores its share still sends what it sends and the transport sheds the excess), and audio holding its configured rate below a smaller grant is the same on one capture stage as on two, so the guard never protected against it. #2848 is the fix for audio and the quest now says so rather than requiring it. (written by Claude Fable 5.1) |
||
| - Regression: two capture stages on one connection whose grants sum to at most | ||
| the estimate and rank by priority, next to the allocator's | ||
| `concurrent_tracks_split_the_estimate`, plus an args test that runs the same | ||
| validation entry point the CLI does and accepts the combination, not one that | ||
| only parses it. | ||
| - Audio reserves its configured rate and does not follow a smaller grant until | ||
| [#2848](/quest/m1/2848-follow-the-bandwidth-grant-in-moq-audio-instead-of.md); | ||
| that is the allocator's documented advisory contract and holds for one | ||
| capture stage exactly as for two, so it is not a reason to keep the refusal. | ||
| - `doc/bin/cli.md` "Multiple stages" drops any mention of the limit. | ||
|
|
||
| Branch from dev. | ||
|
|
||
| ## Closes | ||
|
|
||
| - [#2815](https://github.com/moq-dev/moq/issues/2815) - close this issue when the quest finishes | ||
|
|
||
| ## Related | ||
|
|
||
| - [#2848](/quest/m1/2848-follow-the-bandwidth-grant-in-moq-audio-instead-of.md) - audio following its grant | ||
| - [#2859](/quest/m1/2859-passthrough-imports-reserve-no-bandwidth-so-a-co-resident.md) - passthrough imports joining the same allocator | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Normalize explicit subgroup priority to the track value.
The goal says priority is track-scoped, and a subgroup value that differs from
track::Info::prioritymust collapse to the track value. The plan only covers publisher stamping and the absent-flag fallback.GroupHeader::decodecurrently preserves an explicit wire priority and has no track metadata. Define subscriber-side normalization after track properties are decoded, and add a test for an explicit conflicting subgroup priority.Also applies to: 49-50
🤖 Prompt for AI Agents