You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
refactor(mux): lay the TS export's tables from current state - #7171
Since moq-dev/moq#6289, export ts encoded a PAT and PMT into packets each time tables were due and queued them on the schedule with the media unit they led. The version was fixed at that push. A table change could not reach a copy already queued, so following a moved PMT PID (PAT changes) would have meant rewriting queued copies.
Approach
The schedule keeps the tables as they stand.Schedule::set_psi takes the current PAT+PMT packets and clock PID. Every unit of tables that has not begun to go out lays them, so a change is one state update. A unit already on its way finishes as it began, so a section never mixes two states. Table units still ride the schedule as before, keeping their deadlines, T-STD (TBsys/Bsys) admission, holds on added PIDs, and leading of the unit they precede. Only where their bytes come from changes.
The export owns that state.Psi holds the PMT as it stands. Export::restate encodes it and sets it on the schedule whenever tables are led or revise changes the PMT. Each Slot now reports how many table units begun in it, so the export knows which tables actually went out.
The revision rule from fix(mux): the TS export follows a PMT revision at the next version #6289 is kept, measured from the last tables that began. A PMT that differs from the last copy to begin goes out at the next version. A revision overtaken before any copy of it began takes no version. Tables that return to the last laid state keep its version. The PAT keeps its version until its network entry changes, as before.
The break flag after an instance switch rides the first copy of the tables to go out (Psi::flagged). It is spent once that copy begins.
The opening order from moq-dev/moq#7048 and the T-STD schedule are untouched: open, enforce, and admission still read the same units.
Byte identity
To compare outputs I temporarily hooked Export::pop (not committed) to log a digest of every frame returned, per test. I ran the whole moq-mux suite on main (6dbfa59) and on this branch:
Every test's export output is byte-identical, including all export, round-trip, timing, recovery, and revision tests (204 of 205 tests that export).
The one exception, import::a_splice_on_an_anchored_input_waits_for_its_media, anchors its timebase to the wall clock (its own comment says so). It differs between any two runs.
The revision and export tests also matched across 5 repeated runs on each side.
One full run of the branch showed 13 revision tests differing. Three later full runs, and a raw-byte dump of those same tests on both sides, matched main exactly. I couldn't reproduce or explain that one run.
Because every existing test is byte-identical, none of them hits a revision landing while an older copy is still queued. The new tests below cover that path.
Tests
schedule::the_tables_lay_as_they_stand_when_they_begin: two queued copies, a change before either begins, then another while the first is mid-flight (it spans slots under Bsys). The first goes out whole in the second state, the other in the third. Without the refresh it fails, and also if the refresh touches a copy already begun.
schedule::only_the_first_tables_flag_the_break: padded and not, only the first of two copies carries the flagged packets.
export::a_revision_takes_the_version_after_the_last_laid: the version follows the last tables laid, a revert takes the last laid's version, and a NIT change re-encodes the PAT at the next version.
just rs check-test -p moq-mux: 1832 passed. Clippy --all-targets -D warnings, fmt, doc, shear, and sort are clean.
Choices made without asking
The quest said to reuse what --sync does to lay tables. --sync was removed in moq-dev/moq#6554, after the closing comment it was quoted from, so there was no second path to reuse or avoid. The non-sync schedule is the only path, and the state lives there.
Copies stay as schedule units, refreshed from state until they begin. They are not generated per slot outside the schedule. That keeps the T-STD accounting, deadlines, and byte identity with no change to admission.
The multiplex rate and the added holds stay per lead, as before. They are timing, not table content.
PAT changes is rewritten to start from this. Its blocker link is gone, and its Plan names the hooks a move uses: update Psi::pmt_pid, call Schedule::set_tables, restate, and give the PAT the next version in Psi::current when the PID differs.
Impact
No public API, wire, or output change for any existing fixture. A live export whose PMT revises while an older copy is still queued now sends that copy revised, without burning a version on the state it replaced.
The export queued an encoded PAT and PMT with each media unit it led, its
version fixed at push. Now the schedule keeps the tables as they stand
(`Schedule::set_psi`) and every copy not yet begun lays them, so a table
change is one state update; a copy already on its way finishes as it began.
A revision takes the version after the last copy to begin, so one overtaken
before any copy of it began takes none. The break flag after a switch rides
the first copy to go out.
A static program's output is byte-identical: every moq-mux test's export
output matches main's.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
changed the title
The TS export lays its tables from current state
refactor(mux): lay the TS export's tables from current state
Oct 29, 2026
The reason will be displayed to describe this comment to others. Learn more.
Automated review by review (OpenAI)
Reviewed head 028a409fbaec6c4dfb62440cfe56a11a29276334 against base 6dbfa59b190727e39e4eb0cf2a020e30c6af7d27.
No actionable correctness bugs found in the reviewed changes. Keeping current PSI state separate from queued timing/holds is a sound direction; refreshing only unstarted table units preserves section consistency, and the begun/finished bookkeeping keeps revision versions and completed-output state distinct.
Static review covered the changes and surrounding scheduling, table admission, discontinuity handling, revision, and rewind paths, plus the new regression tests. I did not execute tests or reproduce the reported byte-identity comparisons; T-STD behavior and runtime output equivalence were not independently validated.
The TS export now derives PAT and PMT packets from current PSI state and supplies them to the schedule. The schedule updates eligible queued table units when PSI changes and reports how many table units begin in each slot. The export tracks table snapshots for copies as they begin and finish, and restates tables when required. The related quest documentation now describes the table rework as landed and removes the separate quest entry.
Priority: Normal
Merge Risk: Minimal · up to 028a4
No actionable merge-blocking risk was established. The change is mergeable after normal checks.
Pre-merge checks: 5
Check name
Status
Explanation
Title check
The title "refactor(mux): lay the TS export's tables from current state" matches the main change. The PR makes the schedule hold the current PAT and PMT packets and clock PID, so table units that have…
Description check
The description names the quest document it completes and explains the problem, approach, byte-identity testing, new tests, and choices made. It is directly related to the changeset in rs/moq-mux and …
Docstring Coverage
Docstring coverage is 90.91% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentati…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Title check
Explanation
The title "refactor(mux): lay the TS export's tables from current state" matches the main change. The PR makes the schedule hold the current PAT and PMT packets and clock PID, so table units that have not begun lay tables from current state. The title is specific and concise, and it follows a conventional commit style.
Full details: Description check
Explanation
The description names the quest document it completes and explains the problem, approach, byte-identity testing, new tests, and choices made. It is directly related to the changeset in rs/moq-mux and the quest files. It is not off-topic.
Full details: Docstring Coverage
Explanation
Docstring coverage is 90.91% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 22 functions across 2 files. (1 skipped: 1 unsupported.)
Finishing Touches
✨ Simplify code
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Some tools did not complete. Review the errors below.
Clippy (1.99.0)
Clippy execution timed out
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
The reason will be displayed to describe this comment to others. Learn more.
🔇 Additional comments: 1
quest/m1/ts-reliability/pat-lifecycle.md-27-30: 1
27-30: Minor · Maintainability & Code Quality · Quick win
⚠️Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.
Correct the description of Psi::current. It does not version the PAT on a PMT PID change.
The text says Psi::current gives the PAT the next version when the PID differs from the last laid. The code does not do this.
Psi::current in rs/moq-mux/src/container/ts/export.rs compares only laid.pmt and laid.network.
Laid has no pmt_pid field.
If only Psi::pmt_pid changes, current returns laid.clone() and keeps the old PAT.
The sentence is accurate only for the planned work. Reword it so it does not describe landed behavior. Also state that the move work must add the PMT PID to Laid and to the comparison in Psi::current.
📝 Proposed wording
-already on its way finishes as it began. A move then updates `Psi::pmt_pid`,
-calls `Schedule::set_tables` for the new PID, and restates; `Psi::current`
-gives the PAT the next version when the PID differs from the last laid.
+already on its way finishes as it began. A move then updates `Psi::pmt_pid`,
+calls `Schedule::set_tables` for the new PID, and restates. `Laid` does not
+record the PMT PID yet, so this quest adds it there and to `Psi::current`'s
+comparison, which then gives the PAT the next version when the PID differs
+from the last laid.
Review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 3a51414b-382d-4d9c-aebe-28419500790e
Commits
Reviewing files that changed from the base of the PR and between 6dbfa59 and 028a409.
Files selected for processing: 5
quest/m1/ts-reliability/README.md
quest/m1/ts-reliability/pat-lifecycle.md
quest/m1/ts-reliability/psi-state.md
rs/moq-mux/src/container/ts/export.rs
rs/moq-mux/src/container/ts/schedule.rs
Files with no reviewable changes: 2
quest/m1/ts-reliability/README.md
quest/m1/ts-reliability/psi-state.md
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.
Review: Codex reacted 👍 on head 028a409fba (no findings), which counts as a review per CONTRIBUTING.md. The automated OpenAI review on the same head found no actionable bugs. No unresolved threads.
Checks: all required checks passed. CodeRabbit and CodeQL Analyze (rust) are optional and were still running.
Conflicts: the head merges cleanly with current main and with moq-dev/moq#7165, which also touches quest/m1/ts-reliability/README.md. No base merge was needed.
quest check --base origin/main: ok. The PR deletes psi-state.md and rewrites pat-lifecycle.md to what remains.
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
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.
kixelated
changed the title
kixelated
left a comment
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Completes quest/m1/ts-reliability/psi-state.md, the table rework the maintainer named when closing moq-dev/moq#6301.
Problem
Since moq-dev/moq#6289,
export tsencoded a PAT and PMT into packets each time tables were due and queued them on the schedule with the media unit they led. The version was fixed at that push. A table change could not reach a copy already queued, so following a moved PMT PID (PAT changes) would have meant rewriting queued copies.Approach
Schedule::set_psitakes the current PAT+PMT packets and clock PID. Every unit of tables that has not begun to go out lays them, so a change is one state update. A unit already on its way finishes as it began, so a section never mixes two states. Table units still ride the schedule as before, keeping their deadlines, T-STD (TBsys/Bsys) admission, holds on added PIDs, and leading of the unit they precede. Only where their bytes come from changes.Psiholds the PMT as it stands.Export::restateencodes it and sets it on the schedule whenever tables are led orrevisechanges the PMT. EachSlotnow reports how many table unitsbegunin it, so the export knows which tables actually went out.Psi::flagged). It is spent once that copy begins.open,enforce, and admission still read the same units.Byte identity
To compare outputs I temporarily hooked
Export::pop(not committed) to log a digest of every frame returned, per test. I ran the wholemoq-muxsuite onmain(6dbfa59) and on this branch:import::a_splice_on_an_anchored_input_waits_for_its_media, anchors its timebase to the wall clock (its own comment says so). It differs between any two runs.One full run of the branch showed 13 revision tests differing. Three later full runs, and a raw-byte dump of those same tests on both sides, matched
mainexactly. I couldn't reproduce or explain that one run.Because every existing test is byte-identical, none of them hits a revision landing while an older copy is still queued. The new tests below cover that path.
Tests
schedule::the_tables_lay_as_they_stand_when_they_begin: two queued copies, a change before either begins, then another while the first is mid-flight (it spans slots under Bsys). The first goes out whole in the second state, the other in the third. Without the refresh it fails, and also if the refresh touches a copy already begun.schedule::only_the_first_tables_flag_the_break: padded and not, only the first of two copies carries the flagged packets.export::a_revision_takes_the_version_after_the_last_laid: the version follows the last tables laid, a revert takes the last laid's version, and a NIT change re-encodes the PAT at the next version.just rs check-test -p moq-mux: 1832 passed. Clippy--all-targets -D warnings, fmt, doc, shear, and sort are clean.Choices made without asking
--syncdoes to lay tables.--syncwas removed in moq-dev/moq#6554, after the closing comment it was quoted from, so there was no second path to reuse or avoid. The non-sync schedule is the only path, and the state lives there.addedholds stay per lead, as before. They are timing, not table content.Psi::pmt_pid, callSchedule::set_tables, restate, and give the PAT the next version inPsi::currentwhen the PID differs.Impact
No public API, wire, or output change for any existing fixture. A live export whose PMT revises while an older copy is still queued now sends that copy revised, without burning a version on the state it replaced.
Suggested follow-ups
🤖 Generated with Claude Code
(Written by Claude Opus 5.5)