Skip to content

fix(hls): keep a reconfigured rendition's segments in its playlist - #7436

Merged
moq-bot[bot] merged 19 commits into
mainfrom
quest/m1/hls-reinit-same-codec
Nov 2, 2026
Merged

moq-bot[bot] merged 19 commits into
mainfrom
quest/m1/hls-reinit-same-codec

Conversation

@kixelated

@kixelated kixelated commented Oct 31, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

When a rendition changes resolution under the same codec string, as video1 does in the conformance test of moq-dev/moq#7387, the export already starts a new instance with a new init and segment tag. The hand-over between the two instances still loses or mislabels segments:

  1. Renditions::sync dropped the retired instance from the playlist URI at once. The replacement started with an empty window, so the segments already listed disappeared, which RFC 8216 6.2.2 forbids.
  2. The new config's first group can reach the old instance's records watcher before the catalog update does. The retiring instance claims it, so it is muxed under the old init and the replacement starts one group late.

This PR fixes (1) only. (2) stays in the quest, which waits on the plan to remove the catalog's hls field.

Decisions

  • ✅ Land only fix (1) here; drop the hand-over marker options (maintainer, 2026-11-02). The maintainer wants to remove the catalog's hls field (rs/hang/src/catalog/hls.rs and the JS zod schema). That removal is planned separately via /quest-plan, not here. That makes the open marker decision moot, so it is superseded and none of its options is built:
    • A catalog start field naming the first record of the new config: not chosen.
    • A new record track per decoder config: not chosen.
    • A marker in each record: not chosen.
      The quest is rewritten to the remaining hand-over race, which is designed after the removal plan.
  • ✅ Re-scope this quest to the hand-over, [M] to [L] (maintainer, 2026-10-31). The playlist keeps the old instance's segments, each with its own EXT-X-MAP, behind EXT-X-DISCONTINUITY instead of jumping (RFC 8216 6.2.2). The pick of the hand-over group must not depend on which track's data arrives first. See the design-change comment. The 2026-11-02 decision narrows what lands here to the first half.
    • Delete this quest and plan the hand-over fresh: not chosen.
  • ✅ The fixture repair lands in moq-dev/moq#7387 (maintainer, 2026-10-31).
    • Carry the conformance test into this PR: not chosen.
  • Earlier, ✅ fix the fixture first (maintainer, 2026-10-31).

Changes

  • The playlist keeps a retired instance's segments. A replacement on the same record track holds its predecessor (Rendition::previous). Its media playlist lists the predecessor's rows first, trimmed as one window with its own, until the window slides past them. Then it releases the predecessor. Each predecessor segment carries the hash of the init it was muxed for, and render_media writes a new EXT-X-MAP wherever that changes.
  • The replacement starts behind a discontinuity. Its records watcher waits until the predecessor's window closes, so their rows never interleave and the discontinuity numbers stay in order. Its first row starts a new discontinuity.
  • Old URLs keep resolving while listed. Rendition::segment routes a URL to the instance whose window holds it, matching on tag and position, since a config that returns gets its old tag back. Rendition::init also serves a predecessor's init. playable and playlist count the predecessor's rows and wait for its init.
  • Bounded. A retired instance that neither lists nor waits to list anything passes on what it held instead of stacking. Predecessors are released only once a consistent window read shows they can list nothing more. A publisher restart (set_generation) drops them.
  • The DASH manifest still lists only the current instance's segments. doc/bin/hls.md says so.
  • The quest is rewritten to what remains: the hand-over race (2), waiting on the catalog hls removal plan. Resized [L] to [M], since only the pick remains.
  • A numbering jump is never bridged. Rows a predecessor listed before a jump in segment numbering (records skipped across the hand-over) are dropped, as a skip already clears one instance's window. Otherwise media sequence numbers would shift once they slid out. Found in review.
  • Init requests are served from the instance that has the bytes. A predecessor's built init answers its hash before the replacement builds its own, which can block on media, and only that instance's authorization applies. Found in review.
  • An inherited playlist needs no init of its own. An inline-parameter-set replacement builds its init only from a segment of its own, but playlist() and media_playlist() required it, so the predecessor's segments vanished for the whole hand-over. Now an instance's init is required only when a listed row maps it. Found by the OpenAI and pullfrog reviews (thread).
  • DASH readiness waits for the replacement's own segment. The manifest lists only the current instance's segments, but its waits (Broadcaster::playable, renamed represented, and video_init) counted inherited rows. A manifest loaded during a reconfigure answered 404, or kept audio only, and a DASH player keeps the adaptation sets of the first manifest it loads. They now wait on Rendition::represented, which counts only the instance's own rows. The HLS waits keep the inherited readiness. Found in review.
  • Merged main (no conflicts). After the merge, main's lints failed the build: the tests slept on tokio's clock (now kio::clock::sleep). moq-relay also re-exported listener::Io unused without its websocket feature, as moq-cli builds it. That fails any just check that selects moq-cli or moq-hls, and no open PR fixed it, so it is gated on the feature here.

Impact

  • Public API: none. Rendition::segment and Rendition::init now resolve a predecessor's URLs too.
  • Wire: none.

Choices made without asking

  • A predecessor's segments are listed only once its init is built. playlist() waits for it. media_playlist() answers "not ready" while it is missing, so a player never gets an EXT-X-MAP that 404s.
  • Recording cursors are unchanged. They already drain the retired instance and then follow the replacement (Removed, then Added).
  • The two failing tests were updated to the new behavior. a_reconfigure_versions_the_init_url now expects the old init URL to keep resolving while its segments are listed: the hash is content-addressed, so it always names the old bytes.
  • The remaining quest is sized [M] and keeps no Required entry, since the removal plan has no quest yet. Its Plan says to add one once the plan lands.

Suggested follow-ups (for /quest-plan)

  • [S] In-place reconfigure keeps the published jitter and delay floor. Rendition::set (rs/moq-mux/src/catalog/tracks.rs) on an already-published rendition should floor unsupplied jitter and delay at the catalog's current values, as Rendition::estimate does. Otherwise an H.264 or H.265 same-identity reconfigure after any raise fails with JitterDecreased and aborts the TS import. It blocks the conformance test in moq-dev/moq#7387 once its fixture is repaired.
  • DASH across a reconfigure. The manifest lists only the current instance's segments, so a DASH player loses the predecessor's at a resolution change. Multi-period output or a per-segment init would fix it.
  • The catalog hls removal is now planned as an m0 quest in moq-dev/moq#7616 (quest/m0/hls-media-grid.md), which lists this quest under Related. Once it lands, this quest should Require it or fold the remaining hand-over pick into it. The retention this PR adds (each listed segment under its own init, routing, DASH listing only the current instance) still applies under the grid: only the record-track hand-over (Handoff.next, the records watcher wait) is rewritten.
  • Final AAC segment duration (moq-dev/moq#7437) and fMP4 B-frame decode times stay in their existing quests.

Checks

  • just check origin/main (inside nix develop, through quest gate): pass at 22f6e32.
  • quest check --base origin/main: pass.
  • New regression tests: an_inherited_playlist_needs_no_init_of_its_own, dash_readiness_waits_for_the_replacements_own_segment (each fails without its fix), a_reconfigure_keeps_serving_its_predecessors_segments, a_predecessors_pending_first_segment_is_kept, reconfigures_listing_nothing_hold_nothing, a_returning_config_serves_both_instances_segments, playlist::tests::a_predecessor_keeps_its_own_map, and rendition::tests::a_numbering_jump_cuts_the_inherited_rows.
  • The moq-dev/moq#7387 conformance test was not rerun. Its same-codec switch still needs the hand-over pick (2), its fixture repair, and the jitter follow-up.

Review

mechanical fix by quest land watch: merged main at d5cf1154d3be75dbda9543eb3194adc1bded6d5d with the quest merge driver (head 33c7a8f)

Codex (gpt-6.1-sol, medium effort) via quest review --base origin/main, in a new session after the pullfrog finding. Round 2 reviewed head 22f6e32 and reported no findings.

  • Round 1 (head 7326d3c, the pullfrog fix):
    • [medium] Inherited rows released the DASH manifest's readiness waits, though the manifest lists only the replacement's own segments. Fixed: video_init and Broadcaster::represented wait on the instance's own rows.

Earlier sessions: the 2026-11-02 session (3 rounds, through head cd87090) fixed the numbering-jump bridge, the predecessor init blocking on the replacement's build, and the init authorization. The first session (3 rounds, through head e392601) fixed the empty-instance churn, returning-tag routing, and the closed-read race. The OpenAI review of cd87090 and pullfrog's of b83bd44 both reported the inherited-init gap fixed above.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

kixelated and others added 2 commits October 31, 2026 02:22
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
@kixelated kixelated changed the title HLS export follows a resolution change under the same codec string docs(quest): diagnose same-codec HLS conformance blocker Oct 31, 2026
kixelated and others added 2 commits October 31, 2026 11:57
The interleaving fix makes the export replace the instance; the predicate
already suffices. The remaining failures are in the instance handoff, so
the quest needs re-scoping before implementation.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title docs(quest): diagnose same-codec HLS conformance blocker docs(quest): rerun same-codec HLS case with a repaired fixture Oct 31, 2026
kixelated and others added 4 commits October 31, 2026 13:03
A same-codec reconfigure replaced the rendition's instance and dropped
its window from the playlist URI at once. The replacement now keeps
listing its predecessor's segments, under their own init and URLs,
until the window slides past them, and lists its own behind an
EXT-X-DISCONTINUITY and a new EXT-X-MAP (RFC 8216 6.2.2).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An empty retired instance passes on what it kept instead of stacking,
an empty predecessor is kept until it can list nothing more, and a
segment URL reaches the instance whose window holds it, even when a
returning config shares its tag.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Design change: this quest is re-scoped from "treat a same-codec decoder change as a new instance" (already true on main) to the instance hand-over, resized [M] to [L] (maintainer, 2026-10-31).

  • The playlist URI keeps the retired instance's segments, each under its own EXT-X-MAP, and lists the replacement's behind EXT-X-DISCONTINUITY instead of jumping to an empty window (RFC 8216 6.2.2).
  • The first group after a config change must be listed under the new init, and the pick of that group must not depend on whether the catalog or the HLS records reach the export first.

The first half is implemented in rs/moq-hls/src/export/ at e392601. The second half can't be done in the export alone: the catalog and the records are separate tracks, and nothing on the wire says which record starts the new config. The PR body's ## Decisions needed lays out the publisher-side marker options.

(Written by Claude Opus 5.5)

@kixelated kixelated changed the title docs(quest): rerun same-codec HLS case with a repaired fixture fix(hls): keep a reconfigured rendition's segments in its playlist Oct 31, 2026
kixelated and others added 2 commits November 1, 2026 20:59
The playlist fix lands here; the hand-over pick waits on the plan to
remove the catalog's hls field (maintainer, 2026-11-02).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated and others added 3 commits November 1, 2026 21:48
The merged hand-over tests slept on tokio's clock, which main's clippy
config now disallows, and moq-relay re-exported Io unused without the
websocket feature (as moq-cli builds it).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ts first

Review round 1: rows skipped across a hand-over left inherited rows
ahead of a jump, so media sequence numbers shifted once they slid out.
And an old playlist's init request built the replacement's init first,
which can block on media.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review round 2: a denied predecessor refused init requests for hashes it
doesn't hold.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated marked this pull request as ready for review November 2, 2026 06:46
@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 9 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

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: ffa13f26-aed1-470a-ae50-35600dca6d07
Commits

Reviewing files that changed from the base of the PR and between d5cf115 and 33c7a8f.

Files selected for processing: 10
  • doc/bin/hls.md
  • quest/m1/README.md
  • quest/m1/hls-conformance.md
  • quest/m1/hls-reinit-same-codec.md
  • rs/moq-hls/src/export/mod.rs
  • rs/moq-hls/src/export/playlist.rs
  • rs/moq-hls/src/export/rendition.rs
  • rs/moq-hls/src/export/renditions.rs
  • rs/moq-hls/src/export/segments.rs
  • rs/moq-hls/src/server/routes.rs
  • Autofix · 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

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit: cd87090

[P2] Serve an inherited-only playlist without requiring the replacement's init — rs/moq-hls/src/export/rendition.rs:853–858, 888–890. After an inline-parameter-set codec reconfigures, the retained predecessor can already have playable rows and a cached init while the replacement has no listed rows. playlist() nevertheless calls the replacement's load_init(): init_read() consults only self.live, so it returns None until a new segment is listed, and the playlist disappears despite all inherited rows already having valid maps. media_playlist() independently rejects the same case at the unconditional built_init check. Require the current init only when the snapshot actually contains current-instance rows, and serve the predecessor-only snapshot immediately. Add an inline-codec regression that builds the old init, reconfigures, withholds replacement records, and still receives the old media playlist. This is separate from the explicitly deferred first-group/config attribution race.

Direction: retaining and trimming one combined window, with per-instance maps and segment routing, is a reasonable bounded extension of the existing replacement path. The narrowed hand-over scope is explicit; no new wire marker is needed for this part.

Verification: static GitHub review of the changed code, surrounding initialization/retention paths, and regression tests only. I did not run tests, builds, or HLS playback/conformance checks.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

An inline-parameter-set codec's replacement refuses to serve the predecessor's already-listed segments until it builds its own init, which it cannot do without listing a new segment. During that hand-over the media playlist goes empty, which is the gap this PR set out to close. This was raised in the prior automated review and remains unaddressed at b83bd44.

Reviewed changes

  • Retains a retired instance's segments. A replacement on the same record track holds its predecessor (Rendition::previous) and lists its rows ahead of its own, trimmed as one window, until they slide out.
  • Per-segment init maps. playlist::Segment gains map; render_media emits a new EXT-X-MAP after the discontinuity wherever the map changes, so each predecessor segment stays under the init it was muxed for.
  • Ordered hand-over. The replacement's records watcher waits for the predecessor's window to close before subscribing, so rows never interleave and the new config's first row starts a discontinuity.
  • Bounded. retire passes on a predecessor that neither lists nor waits on, inherited releases ancestors once no rows remain, and restart drops them.
  • Routing. Rendition::segment and Rendition::init resolve a predecessor's URLs while listed.
  • Tests + build. New regression tests; moq-relay gates its Io re-export behind websocket.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

Comment thread rs/moq-hls/src/export/rendition.rs Resolved
kixelated and others added 4 commits November 2, 2026 09:26
An inline-parameter-set replacement builds its init only from a segment of
its own, but `playlist()` and `media_playlist()` required it before
rendering, so the predecessor's segments vanished for the whole hand-over.
Require an instance's init only when the listed rows map it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The DASH manifest lists only the current instance's segments, but its
readiness waits counted a predecessor's inherited rows, so a manifest
loaded during a reconfigure dropped the replacement's video. Wait on the
instance's own rows there (`represented`), keeping inherited readiness for
the HLS playlist.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit: 22f6e32

No new actionable bugs found. The prior P2 inherited-only playlist/init finding is fixed: only instances contributing listed rows require an init, with coverage for serving predecessor rows before the replacement has media and then switching to its own map. This also addresses the identical Pullfrog finding.

Direction: separating DASH's own-row readiness from HLS's inherited-row readiness is appropriate, and the focused regression tests cover the distinction without changing the retained-window design. The deferred first-group/config attribution race remains outside this fix.

Verification: static review of the two substantive fixes, surrounding initialization/readiness paths and tests, accounting for merged base changes. No builds, tests, or HLS/DASH playback checks run.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

This run re-reviewed the two commits since the prior pullfrog review at b83bd44, both fixes to the reconfigure hand-over:

  • Serve inherited segments before the replacement's own init. media_playlist now requires the current instance's init only when a listed row maps it (an own row is present, or the snapshot is empty), and playlist builds only the inits the listed segments map, re-rendering if the instance lists its first segment mid-flight. An inline-parameter-set replacement therefore serves its predecessor's segments during the hand-over instead of returning Ok(None). This addresses the prior review's [IMPORTANT] thread.
  • Wait for the replacement's own segment before a DASH manifest. New Rendition::represented/poll_represented count only the instance's own rows; renditions::video_init and the manifest route now use them while the HLS playlist keeps inherited readiness, so a manifest loaded during a reconfigure does not reflect a predecessor's inherited rows.
  • Regression tests. an_inherited_playlist_needs_no_init_of_its_own and dash_readiness_waits_for_the_replacements_own_segment.

I verified render_media only consumes the init argument for rows whose map is None, so the empty placeholder passed when every listed row is inherited is never written into an EXT-X-MAP. cargo check -p moq-hls --no-default-features passes without warnings. The new tests could not be executed here (the workspace-hack dev-dependency pulls libspa/libpipewire, absent in this environment), so they were reviewed statically.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@kixelated
kixelated added this pull request to the merge queue Nov 2, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Nov 2, 2026
@moq-bot

moq-bot Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

quest land watch: the merge queue removed this pull request at 22f6e32 because it conflicts with main (merge_conflict).

The land fixer works on it in the background, and the watcher re-enqueues it at the back of the queue once it is fixed or judged flaky, or hands it back here with what it needs.

(written by quest land watch)

@moq-bot
moq-bot Bot enabled auto-merge November 2, 2026 20:39
@moq-bot

moq-bot Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

quest land watch: armed auto-merge on this pull request at 33c7a8f, so it joins the back of the merge queue once its required checks pass, with main at d5cf115 merged in by the quest merge driver.

(written by quest land watch)

@moq-bot
moq-bot Bot added this pull request to the merge queue Nov 2, 2026
Merged via the queue into main with commit f654d4a Nov 2, 2026
9 of 10 checks passed
@moq-bot
moq-bot Bot deleted the quest/m1/hls-reinit-same-codec branch November 2, 2026 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant