Repository navigation
quest: plan the Finished handle, the catalog finish fix, and video's declared-end stall #8232
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
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| # [XS] A failed catalog finish is final | ||
|
|
||
| ## Goal | ||
|
|
||
| `moq_mux::catalog::Producer::finish` is never half-finished: when finishing | ||
| either catalog track fails, the producer reports that failure for good and | ||
| both catalog tracks end. It is never left closed with a track still open. | ||
|
|
||
| ## Plan | ||
|
|
||
| Planned 2026-11-07 (maintainer, /quest-complete), a follow-up of | ||
| [moq-dev/moq#8210](https://github.com/moq-dev/moq/pull/8210), which fixed | ||
| this shape in the importers and the container, audio, and JSON producers | ||
| but left the catalog producer outside its list. | ||
|
|
||
| `finish` (`rs/moq-mux/src/catalog/producer.rs`) stores | ||
| `closed = Some(Error::Closed)` before `self.outputs.hang.finish()?` and | ||
| `self.outputs.msf_track.finish()?`. If the hang track's finish fails, the | ||
| MSF track is never finished, yet every later call reports a clean `Closed`. | ||
| Record the terminal outcome after the fallible work, as #8210 did: keep the | ||
| first error, abort or end the other track, and store the failure rather | ||
| than `Closed`. Test a failing track finish. | ||
|
|
||
| Public API: none. Wire: none. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| # [XS] Video's declared-end freeze is not a stall | ||
|
|
||
| ## Goal | ||
|
|
||
| `<moq-watch>` doesn't count a stall while video holds its last picture | ||
| through a publisher's declared end, as audio already doesn't for a declared | ||
| silence. The stall count, stalled time, and the echo `stalls`/`stalled` | ||
|
coderabbitai[bot] marked this conversation as resolved.
Resolved
|
||
| feedback agree between audio and video. | ||
|
|
||
| ## Plan | ||
|
|
||
| Planned 2026-11-07 (maintainer, /quest-complete), a follow-up of | ||
| [moq-dev/moq#8209](https://github.com/moq-dev/moq/pull/8209), which made | ||
| watch count a stall only while a rendition plays. It left this one case as | ||
| on `main`: a picture held through a declared end for longer than the 500 ms | ||
| buffering timer still counts as a stall. Give video's playback state a | ||
| declared-end phase, as audio's handles a declared silence, so the buffering | ||
|
coderabbitai[bot] marked this conversation as resolved.
Resolved
|
||
| timer doesn't run there. Test a declared end held past 500 ms. | ||
|
|
||
| Public API: none. Wire: none. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| # [L] Finishing a producer returns a Finished handle | ||
|
|
||
| ## Goal | ||
|
|
||
| `finish(self)` consumes a producer and returns a `Finished` handle, so | ||
| appending after finish is a compile error instead of a runtime one. The | ||
|
coderabbitai[bot] marked this conversation as resolved.
Resolved
|
||
| handle keeps what finishing still allows: aborting, which reports a later | ||
| failure and releases the cache, and, where the type supports it today, | ||
| inserting below the end, such as a late group under a finished track's end. | ||
| It has no append. One shape covers moq-net's `track::Producer` and | ||
| `group::Producer` and the media producers and importers, with JS mirrors. | ||
|
coderabbitai[bot] marked this conversation as resolved.
Resolved
|
||
|
|
||
| ## Plan | ||
|
|
||
| Decided 2026-11-07 (maintainer, /quest-complete), a follow-up of | ||
| [moq-dev/moq#8210](https://github.com/moq-dev/moq/pull/8210), which made a | ||
| failed finish final with explicit state and recommended against | ||
| `finish(self)`. The maintainer chose the typestate: | ||
|
|
||
| - ✅ **Scope: moq-net and media.** moq-net's `track::Producer` and | ||
| `group::Producer`, plus #8210's producers and importers (the container | ||
| producer, the MKV, fMP4, FLV, and TS importers, the audio and JSON | ||
| producers), and their `js/net` and `js/hang` mirrors. Not chosen: moq-net | ||
|
kixelated marked this conversation as resolved.
Resolved
|
||
| only; media only. | ||
| - ✅ **Name: `Finished`**, as `track::Finished`, `group::Finished`, and so | ||
| on. | ||
|
|
||
| Today each producer borrows on finish and enforces the rest at runtime. | ||
| `track::Producer::finish(&self)` (`rs/moq-net/src/model/track.rs`) sets the | ||
| end after `max_sequence`, so appending at or above it fails, while older | ||
| groups below it can still arrive. `group::Producer::finish(&mut self)` | ||
| (`rs/moq-net/src/model/group.rs`) borrows so that `abort` can still report | ||
| a later failure and its cache stays readable. `Finished` makes exactly that | ||
| contract the type. Holding a finished importer or producer, which #8210 | ||
| documents as a use, becomes holding its `Finished`. Decide per type what | ||
| `Finished` exposes beyond abort, such as read access to the cache or insert | ||
| below the end, without adding anything the producer can't do today. | ||
|
|
||
| This breaks every caller that finishes and then keeps using the producer. | ||
| List them in the PR, with the migration for each package in its Impact section. | ||
|
|
||
| Public API: breaking in moq-net, moq-mux, moq-audio, moq-json, and their JS | ||
| packages. Wire: none. | ||
|
|
||
| ## Related | ||
|
|
||
| - [Track and group end-of-life](/quest/m1/explicit-state/track-end.md) - the outcome enums `Finished` would wrap | ||
| - [A failed finish is final](https://github.com/moq-dev/moq/pull/8210) - the explicit state this builds on | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.