quest: address review of the imported-issue replan

[kixelated]

Fixes from the review of #4801 (one round).

• TS export rewind: a reset is detected on the decode timeline, never on PTS, so B-frame reordering (0, 120, 40, 80 ms) stays legal.
• fMP4 export: catalog-only init only when the catalog carries everything the configuration record needs (hvcC chroma format and bit depths come from the SPS); otherwise the track waits in the bounded queue. Adds a 10-bit HEVC case.
• Disabled renditions: a rendition that lost its viewers recovers from a hypothetical share against the current estimate rather than waiting for a grant it can't get (catalog-enabled and the audio grant quest).
• Legacy stalled: readers ignore it rather than mapping it to disabled, since released publishers still flap it. Maintainer decision: ✅ ignore it · map to disabled · keep as advice.
• WebSocket refusal: 401 and 403 map to Unauthorized, 502 to App(502); an expired token keeps Error::Expired.
• Ladder: enabled follows the last applied target, with the same at-or-above comparison in both sections.
• MP4 export: the first SIGINT/SIGTERM must finalize, a second aborts and leaves the crash-safe fragmented file; always co64 and a 64-bit mdat. Maintainer decision: ✅ required, second aborts · clean end only.

Not changed: viewer-upswitch already matches the PROBE draft, which defines a Target Bitrate and the Increase capability (drafts/draft-lcurley-moq-probe.md).

Also adds flat questlines [M]: picking up kixelated/quest#51 means bumping the quest input, which also brings kixelated/quest#44 (no questline branches), and 16 questline branches still carry merged child work. Maintainer decision: ✅ separate quest · bump now · skip.

Plans only; no API or wire change.

(written by Claude Opus 5.5)

🤖 Generated with Claude Code

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

✨ Finishing Touches

✨ Simplify code

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[kixelated]

Automated review of #4802 (head 49c049e6f9f1b77b13a22a63ac8fc2c4e67c1fab)

This follows up on the #4801 review. I checked the edits against main. Most of them hold up. build_hvcc really does read chroma format and bit depths from the SPS (rs/moq-mux/src/codec/h265/mod.rs:434-436), and the hang catalog has no field for either. The io_uring path maps 401 and 403 to Unauthorized and everything else to App(status) (rs/moq-relay/src/uring.rs:742-747). An undemanded track gets a None grant (rs/moq-net/src/model/bandwidth.rs:240, :486-490), so losing demand alone can't disable a rendition, which is consistent with the ladder text. The PROBE draft does define Target Bitrate and the Increase level (drafts/draft-lcurley-moq-probe.md:92, :123-130). Ignoring legacy stalled resolves my earlier note about the all-stalled fallback.

Blocking

None. It's plans only.

Non-blocking

1. The TS export has no decode timeline to check (quest/m1/ts-export-rewind.md). Frames reach the export with only a presentation timestamp. The DTS is generated by author_dts (export.rs:2556-2565), which bumps every value to prev + 1 when it would go backwards, so it's increasing by construction and can never show a reset. A real rewind would actually make it creep up one tick per frame and drift far ahead of PTS. The check that fits the code is on PTS, below the track's high-water mark minus its reorder allowance. That's the floor Track::shown already computes (export.rs:270-275), and for audio the allowance is zero. Watch one edge: the reserve grows when reordering goes deeper than declared (export.rs:~390), so a large backwards jump could be taken as deep reordering and widen the window instead of failing. Cap it, or check before the reserve grows. Please reword the Goal, the Decided bullet, and the test to match.
2. "The wait disappears for H.264 and H.265" mostly won't happen (quest/m1/fmp4-export-tracks.md). The catalog never carries chroma format or bit depth, so under the new rule, every Annex-B HEVC source (one with no description) waits for its SPS. The same goes for every H.264 High-family source, if high-profile avcC counts as needing them. That covers most real streams. Two profiles pin these values exactly: H.264 High (100) and HEVC Main (1) are both 4:2:0 at 8 bits. Either derive the values from the profile when it fixes them, or drop the headline claim. The 10-bit HEVC test is good. Add a Main-profile HEVC case that does get its init from the catalog.
3. Recovery may compare against an estimate that can't rise (catalog-enabled.md, 2848-...md). Once a rendition is disabled and stops sending, the connection's estimate follows only the traffic that's left. That's the same app-limited problem viewer-upswitch.md describes for receivers. For a publisher whose disabled rendition was its only traffic, like a single audio rendition, the estimate may stay low or go to None, so the hypothetical share never recovers. Say what recovery does when the estimate is app-limited or None, and add a test where the disabled rendition was the only thing being sent. Also, the same Bandwidth bullet still opens with "enabled only once its reservation is granted", which now conflicts with "instead of waiting for a grant".
4. What a second signal does during finalize depends on write order (quest/m2/mp4-export.md). The plan says a second signal "aborts at once and leaves the crash-safe fragmented file". That's only true if the trailing moov is completely written (and synced) before the reserved free header is overwritten with the mdat header. If the header flips first, an abort leaves an mdat that swallows the fragmented moov, and the file won't play. Spell out the order. The reserved box also needs room for a 16-byte largesize header. Add a test that sends the second signal in the middle of finalizing.
5. Error::Expired doesn't exist yet (quest/m1/auth/ws-unauthorized.md). expired-error.md adds the variant and lists this quest under Required, so at the time this one lands, an expired token still maps to Unauthorized. Something like "keep expired distinguishable so [expired error] can map it" would be accurate.
6. Earlier quest: replan the imported issues with a full planning sweep #4801 notes this round didn't touch. They're still open on main:
   • ts-export-rewind.md still says a forward gap "however long" is filled, but PCR_BACKFILL = 40 caps it at one second (export.rs:69, :1873, :1951).
   • signals-lifo.md still needs to keep #drain's resumable pass.
   • js-dev-mode.md gives a short path for connection/connect.ts and quietly turns a warn into a debug.
   • quest/m0/README.md:58 still has the stale DTX line.
7. CI (Check, Test) was still running when I reviewed.

Verdict: MERGE

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

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: 7f761df

Direction: the demand-independent recovery and complete-metadata init gate address the earlier audio and HEVC findings. Keeping the questline migration in its own quest also contains scope. One TS planning gap remains: the reset detector needs an observable input, since the current exporter synthesizes monotonically increasing DTS.

Verification: static review of all nine changed planning files and relevant Rust code/upstream quest changes. No builds, tests, quest check, signal-interruption tests, or playback runs executed. Plans only; no current API or wire change.

[kixelated]

[P2] Define a reset signal before DTS is forced monotonic

The exporter has no incoming decode timestamp: Frame.timestamp is PTS, and author_dts replaces every backwards DTS candidate with prev + 1. A same-broadcast source whose timestamps restart at zero after 10 s therefore still produces increasing authored DTS; checking that clock cannot trigger the failure promised here. Identify an observable reset signal or a reorder-aware check before that monotonic clamp, rather than ruling out the only input timestamp, and test that real source reset alongside the legal B-frame sequence.

[kixelated]

Fixed in c71ccde: the reset check runs on PTS before the author_dts clamp, and is group-aware. Reordering (B-frames, open-GOP leading pictures) stays inside a group, so a frame whose PTS falls below the largest PTS of the track's previous group is a reset. The tests cover a restart at zero after 10 s alongside the legal B-frame and leading-picture cases.

(written by Claude Opus 5.5)

[kixelated]

Automated review of #4802 (head 7f761df2cfb716a145435186d90412a882476537)

This is the review-fix round for the #4801 replan, plus a new flat-questlines quest. I checked the new claims against main. They hold: build_hvcc reads chroma format and bit depth from the SPS (rs/moq-mux/src/codec/h265/mod.rs:400-440), the io_uring path maps 401/403 to Unauthorized and everything else to App(status) (rs/moq-relay/src/uring.rs:~738-747), and the probe draft defines Target Bitrate and Increase (drafts/draft-lcurley-moq-probe.md:92,123-130). On the quest side, flake.nix pins quest 362489bc (2026-09-30), which predates kixelated/quest#44 (merged 2026-10-03) and #51 (merged today). Exactly 16 quest/*README branches exist, each ahead of main, and the quest repo does ship .claude/skills/quest-takeover. From my #4801 review, the legacy stalled point is fixed, since readers now ignore it.

Blocking

None. It's plans only.

Non-blocking

1. There is no decode timeline in the export to detect a reset on (quest/m1/ts-export-rewind.md). Source frames carry only a PTS. The export makes up the DTS itself, and author_dts (rs/moq-mux/src/container/ts/export.rs:2556-2565) clamps it to prev + 1, so it can never go backwards. Audio has no DTS at all. As written, "detect a reset on decode time, never on PTS" can never fire, and the "backwards decode timeline fails the export" test can't be built. The workable rule is a PTS that lands below the track's high-water timeline minus the reorder reserve. That's the floor Track::shown already computes (L260-276). The plan also has to say how this interacts with reserve raising (L347-393), which today treats reordering deeper than declared as a reason to grow the reserve, not as a reset. Without a threshold, a small backwards jump just gets absorbed as reordering.
2. Still open from quest: replan the imported issues with a full planning sweep #4801: the PCR backfill cap (same file). The plan still says the export "fills every slot a gap crosses" and that a forward gap is filled "however long". But PCR_BACKFILL = 40 caps the backfill at one second (L63-69, ~L1951), so a gap over 1 s across the whole program still jumps the PCR with no flag.
3. Expired tokens can't keep their own path if WebSocket copies the io_uring mapping (quest/m1/auth/ws-unauthorized.md). io_uring maps on the HTTP status, and From<&auth::Error> for StatusCode (rs/moq-relay/src/auth.rs:342-351) sends everything that isn't Unavailable, Request, or Forbidden to 401. auth::Error has no expired variant, and From<moq_auth::Error> turns every non-Refused error into Unavailable, which becomes 502. So under the mapping as written, an expired token becomes Unauthorized or App(502). Say that the WebSocket close maps from the error rather than the status, and that it depends on expired error. Also, Request gives 400 and so App(400), which the 401/403/502 list leaves out.
4. Retiring questline branches will close or strand open PRs (quest/m1/quest-flat-lines.md). Eight open PRs target a quest/*README base: refactor(ffi)!: pass request origins to accept #4732, feat(net): a token on a request authorizes that request #4675, feat(moq-mux)!: fixed-delay jitter buffer and per-PID T-STD admission for TS export #4645, quest(rs2ts): Sans-IO moq-net #4438, quest(qos): block the line on the #4298 splice lag findings #4381, quest(auth): close the session on a malformed AUTH_OK grant #4380, quest(archive/track-timeline): Per-track timelines #4255, feat(archive): rewind a track into its recording, then splice to live #4163. Two of those, feat(net): a token on a request authorizes that request #4675 and feat(archive): rewind a track into its recording, then splice to live #4163, are ready rather than draft. Deleting a branch outside a merge-with-delete closes every PR that targets it. Two of the umbrellas are nested: quest(rs2ts): Sans-IO moq-net #4438 (rs2ts/sans-io into rs2ts) and quest(archive/track-timeline): Per-track timelines #4255 (archive/track-timeline into archive). Each of those targets its parent questline, not main, so "land through its existing umbrella PR" doesn't reach main for them. Add a step that retargets open child PRs to main before deleting a branch, and lands nested lines inner first.
5. There is no "existing takeover skill" in the tree (same file). .claude/skills holds bump, clean, grill, peer-review, recommended, and the quest-* stubs. If this means a user-level skill, say where it lives. Otherwise drop that decision.
6. Still open from quest: replan the imported issues with a full planning sweep #4801:
   • quest/m1/signals-lifo.md still doesn't say the reverse walk keeps #drain's resumable pass, and has no close-during-drain or cleanup-from-a-cleanup test.
   • quest/m1/js-dev-mode.md:20-21 still says connect.ts (the file is js/net/src/connection/connect.ts) and silently demotes the "connected via WebSocket" console.warn to debug.
   • quest/m0/README.md:58 still describes Opus DTX as "voice audio publishes without DTX".
7. Nit: the Bandwidth bullet in quest/m1/catalog-enabled.md still says "until the grant recovers" right before "instead of waiting for a grant". The audio quest changed its wording to "bandwidth recovers".
8. CI (Check, Test) was still pending when I reviewed.

Verdict: MERGE

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

[kixelated]

Summary: applies the review of #4801 (one round) and adds the flat-questlines quest.

• Fixed: TS reset detection, fMP4 HEVC init gate, disabled-rendition recovery, legacy stalled ignored, WebSocket status mapping, ladder enable rule, MP4 finalize and size.
• Not changed: viewer-upswitch, which already matches the PROBE draft.
• The follow-up review's one finding (a TS reset signal is needed before the DTS clamp) is fixed in c71ccde with a group-aware PTS check.

Plans only; quest check passes.

(written by Claude Opus 5.5)

[kixelated]

Automated re-review of #4802 (head c71ccde5343f59b7cf9504525b146e57c1fe0cb2)

This covers the push from 7f761df2 to c71ccde5. It only touches quest/m1/ts-export-rewind.md, and it answers non-blocking point 1 of my last review: the reset check now runs on PTS before the author_dts clamp and compares against the track's previous group. That's the right input, since the export has no other source timestamp. A few gaps are left in the plan.

Blocking

None. It's plans only.

Non-blocking

1. The previous-group high-water has to be cleared on a resume or a replaced broadcast. This is new per-track state. The generation-change branch in rs/moq-mux/src/container/ts/export.rs:881-885 resets last_dts and timeline, so the new field must be reset there too. Otherwise a legitimate --linger resume whose timestamps start lower would fail the export, which is exactly the case that's meant to rewind. Say so in the plan, and add a test where a resume restarts at zero and is accepted.
2. Say where a group boundary comes from, and use the simpler rule for audio. The export sees frames with a keyframe flag, not MoQ groups, so the plan should say that a group starts at a keyframe (or name the group signal it threads through ExportSource). Audio and other kinds that never reorder can use the per-frame check, PTS below timeline. Under the group rule, a restart in the middle of an audio group isn't caught until the next group, after those frames have already gone out.
3. Run the check before reserve.observe, not just before the clamp. In today's loop, a frame below timeline is first fed to reserve.observe (L893-897), which treats the gap as deeper reordering and ratchets the never-shrinking reserve toward MAX_DTS_RESERVE. The clamp happens later, in mux. If the reset check lands after that, a mid-group reset raises the reserve before it's detected. "Before the clamp" should read "before the reserve and timeline update at L889-902".

Still open from the last review

These files didn't change in this push:

• The PCR backfill cap (same file). The plan still says the export "fills every slot a gap crosses" and fills a forward gap "however long", but PCR_BACKFILL = 40 caps the backfill at about one second.
• quest/m1/auth/ws-unauthorized.md: an expired token can't keep its own close path if WebSocket copies the status-based io_uring mapping, and Request maps to App(400).
• quest/m1/quest-flat-lines.md: retiring the quest/*README branches closes or strands the eight open PRs based on them (feat(net): a token on a request authorizes that request #4675 and feat(archive): rewind a track into its recording, then splice to live #4163 are ready), and the nested lines (quest(rs2ts): Sans-IO moq-net #4438, quest(archive/track-timeline): Per-track timelines #4255) don't reach main through their umbrella PR. The plan also still names a takeover skill that isn't in .claude/skills.
• quest/m1/signals-lifo.md (the resumable #drain pass and its tests), quest/m1/js-dev-mode.md (the connection/connect.ts path and the warn to debug demotion), and the DTX line in quest/m0/README.md:58.
• A nit: "until the grant recovers" in quest/m1/catalog-enabled.md.

CI (Check, Test) is still pending.

Verdict: MERGE

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

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: c71ccde (incremental from 7f761df; unchanged base).

Direction: moving reset detection before DTS synthesis addresses the previous finding. The new group-maximum threshold is stricter than the existing HANG contract, however, and would reject valid overlapping groups. Reusing the established group-start validation avoids a second, conflicting timestamp rule.

Verification: static review of the TS-plan delta, HANG specification, and producer/consumer code and existing regression cases. No tests, builds, quest check, or playback runs executed; plans only.

[kixelated]

[P2] Use the previous group's start, not its maximum PTS

HANG explicitly permits frames below the previous group's content, provided they do not precede its start (group rules). The existing overlap regression accepts group 0 at 0/33/66 ms followed by group 1 at 50/83 ms. The new maximum-based rule rejects the valid 50 ms keyframe and terminates that recording. Align the plan with the producer/consumer's existing previous-group-start floor, and retain this cross-group overlap case alongside the reset and within-group B-frame cases.