Repository navigation
fix(hls): publish every rendition in the first catalog - #4861
Conversation
|
Quest The regression test fails on main with a first catalog of video 1 / audio 0, and passes with the pass-wide hold. Left as a draft. Holding on every pass, not only the first, is the choice worth a look: a quiet first pass publishes nothing, so the first catalog can be a later pass. (Written by Grok 4.7) |
Co-authored-by: Grok 4.7 <noreply@x.ai>
An HLS import reserved the catalog only when each rendition's init segment loaded, and that reservation ended before the next rendition existed, so the first snapshot listed only the first rendition. Hold a catalog reservation for the whole pass, beside the timeline reservation, and drop it when the pass ends. Co-authored-by: Grok 4.7 <noreply@x.ai>
669ab69 to
d7f2921
Compare
|
Rebased onto main ( The HLS importer holds a catalog reservation across each pass, so the first catalog lists every rendition that loaded in that pass. Each importer still releases its own reservation at its first frame; the pass hold is what covers the gap. Main removed the pass-wide timeline reservation in #4034, and this does not restore it. No public API change and no wire change: the catalog that is pushed is the one consumers read. Confirmed: hold across the pass, not only the first. A rendition with no media yet still cannot withhold the catalog past that pass. (Written by Grok 4.7) |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (3)
💤 Files with no reviewable changes (2)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review. WalkthroughThe HLS importer now holds a catalog reservation across a pass over active tracks. A regression test gates the audio playlist response and checks that the first catalog snapshot includes both video and audio renditions. The Required quest entry and its proposal document were removed. Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The catalog change has no identified merge-blocking issue. Normal checks remain appropriate before merging. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches✨ Simplify code
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. Comment |
Automated review: fix(hls): publish every rendition in the first catalogReviewed head: Holding a Findings (by severity)
VerdictMERGE. The fix is correct and well tested. Finding 1 is a real latency tradeoff on a degraded origin, so either bound the hold or confirm the unbounded first-pass wait is acceptable. Reviewed head This is an automated review, not the maintainer's decision |
|
On the automated review of
(Written by Grok 4.7) |
Problem
moq-hls import reserved the catalog only when a rendition's init segment loaded, inside
TrackState::ensure_map. That reservation ends at the rendition's first frame, and renditions are ingested one at a time, so the first rendition could publish before the others existed. A consumer's first snapshot then listed only that rendition and grew on later updates. Reported by Dryvnt in #4824.Approach
steptakes a catalogReservedafterensure_tracksand before the first ingest, and drops it when the pass ends. The first catalog therefore lists every rendition that loaded its init segment in that pass.Every pass holds, not only the first. A quiet playlist publishes nothing, so the first catalog may be a later pass. Once the catalog has been published, a reservation no longer gates updates, so later passes are not withheld. A rendition with no segments yet, or whose init fetch fails under
OnError::Warn, never reserves and cannot withhold the catalog past that pass.Main no longer reserves the timeline across a pass (#4034). This change does not put that hold back.
Impact
Alternatives
Hold only on the first pass. That still publishes a partial catalog when the first pass is quiet and the renditions load together on the next one.
Follow-ups
A later pass can still publish before a rendition that has no media yet, because that rendition never reserves. That is the bound this quest set.
Import at the first frame has landed (#4885). Each importer still releases at its first frame, before the next rendition is reserved; the pass hold covers that gap and drops at the end of the pass.
(Written by Grok 4.7)