Repository navigation
Anchor initPTS on video PTS at discontinuities reached in sequence - #9
Draft
littlespex wants to merge 10 commits into
Draft
littlespex wants to merge 10 commits into
littlespex wants to merge 10 commits into
Conversation
… a discontinuity generateIS binds initPTS to the lowest DTS across tracks (video-dev#7880, cherry-picked into 1.6.18 as 209a01b) so the first segment leaves composition offset headroom for segments loaded later at an earlier playlist time (video-dev#7932). At a discontinuity, timeOffset is the previous fragment's presentation end (updateFromToPTS: start + duration, where duration is endPTS - start). Mapping the new sequence's first DTS to that boundary presents its first frame one composition offset late. The next fragment's start is derived from that PTS, so the shift carries forward and repeats at every discontinuity. Keep the DTS anchor for fresh non-contiguous anchors (session start, seeks) and use the PTS anchor only when the remuxer is continuing contiguous content, which is the mapping discontinuities had before 1.6.18. Refs video-dev#8064 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… discontinuities keep the PTS anchor The remuxer used its own video contiguity flag to decide when a new timestamp offset continues the previous fragment. A level switch at a discontinuity resets contiguity (samples from another level must not be stitched), so that boundary still took the DTS anchor and shifted the timeline once. The transmuxer already distinguishes "consecutive" (next sn or part, any level) from "contiguous" (consecutive and same level). Carry consecutive in TransmuxState and hand it to Remuxer.resetTimeStamp, and let generateIS anchor on the first PTS whenever a fresh offset is computed for a consecutive fragment. Session start and seeks stay on the DTS anchor. Refs video-dev#8064 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
littlespex
commented
Oct 12, 2026
littlespex
left a comment
Member
Author
There was a problem hiding this comment.
Found one P2 issue with cross-level continuity detection; details and reproduction are inline.
Validation at bf71d9b: all 1,211 existing unit tests passed locally. A focused reproduction passes for an in-order SN 5 → 6 level switch and fails for the otherwise identical SN 5 → 106 switch, adding 80 ms of drift. The unrelated CI automation failure reports a missing repo-token.
… numbers Variant Streams may number Media Segments independently (RFC 8216 6.3.2), so a discontinuity switch to a level with another sequence-number base was marked non-consecutive and kept the DTS anchor, preserving the drift this branch fixes. Across a level switch, treat the fragment as consecutive when it starts where the last pushed fragment or part ends, within maxFragLookUpTolerance. Same-level sequencing and the contiguous flag are unchanged. Refs video-dev#8064 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Transmuxer.push is the only place TransmuxState.consecutive reaches Remuxer.resetTimeStamp, on both the inline and the worker path, and nothing exercised it: the specs covered the interface computing the flag and the remuxer honoring it. Drive push() with stubbed muxers through the discontinuity branches: a sequence reached in order (same level or level switch) resets the offset with consecutive set, a seek resets it cleared, the cached per-cc offset passes through, and a contiguous push does not reset at all. Also note on MP4Remuxer.isConsecutive that it comes from fragment sequencing, distinct from the sample-timing heuristic remuxAudio already calls "consecutive". Refs video-dev#8064 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…rule The cross-level continuity term compares the pushed part's start with the last pushed part's end, and only looks at the last fragment when there is one. Neither leg had a spec: add a level switch at a discontinuity between the last part of one fragment and the first part of the next, and a first push with nothing before it. Both assert the TransmuxState fields explicitly. Refs video-dev#8064 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…evel rule Review on video-dev#8083: the sequence-number leg assumed aligned numbering across variants, and across a level switch timeOffset is only as good as the playlist alignment, so binding the first PTS to it would turn drift into an overlap or a gap. Same-level discontinuities have an exact start from updateFromToPTS. isVideoContiguous is cleared by resetNextTimestamp exactly when the transmuxer reports a non-contiguous fragment and set when video was remuxed, so it is "contiguous, with previous video to continue from". Remove the consecutive field of TransmuxState, the second argument of Remuxer.resetTimeStamp and the presentation-time rule. A discontinuity that coincides with a level switch keeps the DTS anchor. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This branch has not been deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR will...
Opened upstream as video-dev#8083 (draft).
Stop the timestamp offset from growing at every discontinuity in remuxed MPEG-TS streams (video-dev#8064).
generateIShas boundinitPTSto the lowest DTS across tracks since video-dev#7880 (cherry-picked into 1.6.18 as 209a01b). At a discontinuity,timeOffsetis the previous fragment's presentation end:updateFromToPTSsets the next fragment's start tostart + duration, anddurationisendPTS - start. Mapping the new sequence's first DTS to that boundary presents its first frame one composition offset late. The next fragment's start is derived from that late PTS, so the shift carries forward and repeats at every discontinuity. On a live channel with server-stitched ads, each ad adds its composition offset to latency and nothing takes it back.The change keeps the DTS anchor for fresh anchors that do not follow the previous fragment (session start, seeks, cached per-cc offsets), which is the case video-dev#7932 and video-dev#7880 addressed. It uses the PTS anchor only when the fragment directly follows the last one pushed. The transmuxer already distinguishes "consecutive" (next sn or part, on any level) from "contiguous" (consecutive and same level); the second commit carries
consecutiveinTransmuxStateand passes it toRemuxer.resetTimeStamp, so a level switch at a discontinuity keeps the PTS anchor even though sample contiguity is reset. That is the mapping discontinuities had before 1.6.18.Why is this Pull Request needed?
Against 1.6.17 on the deltatre DAI test stream, each sequence starts 40, 120, 200, 280 and 320 ms late: the running total of the first frame's composition offset in each sequence. On a production live channel the same effect added 490 ms of latency across one 8-ad break. Logs and measurements are in the upstream issue.
Visual summary
How the shift accumulates on master, and why a consecutive fragment must bind its first PTS instead:
flowchart LR subgraph M["master since 1.6.18: bind the first DTS to timeOffset"] direction TB m1["timeOffset = previous sequence endPTS"] --> m2["first DTS at timeOffset<br/>first PTS at timeOffset + cts"] m2 --> m3["cts hole: first frame stretched,<br/>silent audio injected"] m3 --> m4["sequence endPTS = timeOffset + duration + cts"] m4 -- "next discontinuity inherits the cts" --> m1 end subgraph P["this PR: a consecutive fragment binds the first PTS to timeOffset"] direction TB p1["timeOffset = previous sequence endPTS"] --> p2["first PTS at timeOffset<br/>first DTS at timeOffset - cts"] p2 --> p3["no hole"] p3 --> p4["sequence endPTS = timeOffset + duration"] p4 -- "next discontinuity starts on time" --> p1 endWhere the
consecutiveflag is computed and consumed:sequenceDiagram participant SC as StreamController participant TI as TransmuxerInterface participant TX as Transmuxer (worker) participant RX as MP4Remuxer SC->>TI: push(frag, timeOffset = frag.start) Note over TI: nextInSequence = next sn or next part<br/>contiguous = not trackSwitch and nextInSequence<br/>consecutive = nextInSequence, or across a level switch<br/>|timeOffset - lastEnd| ≤ maxFragLookUpTolerance TI->>TX: push(data, chunkMeta, TransmuxState with discontinuity, contiguous, consecutive) TX->>RX: resetTimeStamp(initPTS[cc] or null, consecutive) opt not contiguous (level switch or seek) TX->>RX: resetNextTimestamp() end TX->>RX: remux(tracks, timeOffset) Note over RX: generateIS with no cached initPTS:<br/>consecutive ? bind first PTS : bind first DTSAre there any points in the code the reviewer needs to double check?
consecutiveas the discriminator. It is true when the pushed fragment or part is next in sequence (snDiff === 1, or the next part) on any level, or, across a level switch, when it starts where the last pushed fragment or part ends, withinmaxFragLookUpTolerance(0.25 s). Variant Streams may number Media Segments independently (RFC 8216 6.3.2), so the cross-level case cannot rely on sequence numbers.contiguousstill derives from the sequence rule alone, so sample stitching is unchanged.TransmuxStategets the field with a default ofcontiguousfor existing callers, andRemuxer.resetTimeStamptakes it as an optional second argument (the passthrough remuxer ignores it). Session start has no previous fragment and seeks are not consecutive, so both keep the DTS anchor.timeOffset === 0 && videoInitPTS < _initPTS.baseTime ? videoInitDTS : videoInitPTS) was tested too. It removes the growth, but it anchors the first segment of a session on PTS again, which is what Update minimum browser support documentation and build config video-dev/hls.js#7880 changed on purpose, and it dereferences_initPTSwhile it is null.computeInitPts. initPTS ping-pong prevents recovery on repeated dropped-append after PR #7797 video-dev/hls.js#7811 and Same segment repeatedly loaded shortly after scrubbing, with playback getting stuck permanently video-dev/hls.js#7677 are in the passthrough remuxer. The video stops for a moment, but the audio continues to play. video-dev/hls.js#6374, video (but not audio) playback freezes after seeking back video-dev/hls.js#6777, Playback stalling due to low buffer - only in Chrome video-dev/hls.js#5477 and the Pad initial PTS of remuxed MPEGTS to avoid video rendering issues in Chrome video-dev/hls.js#7805 CTS pad are separate code paths, and the pad only runs when!contiguous.Testing
Unit specs in
tests/unit/remux/mp4-remuxer.tsdriveremux()across synthetic discontinuities with the deltatre composition offsets. On master the videoendPTSlands 80 ms late at each boundary. With this change it lands 0 ms late, both for a same-level discontinuity and for one reached with a level switch (resetTimeStamp(null, true)plusresetNextTimestamp()). Two more specs pin the session-start and post-seek anchors to DTS. Transmuxer-interface specs check that a level switch at a discontinuity is flagged consecutive but not contiguous, including a switch to a level with a different sequence-number base (SN 5 to 106) that starts at the previous fragment end, and that a switch landing a fragment later is not consecutive.karma start karma.conf.js: 1213 passing.tsc --noEmitand eslint are clean.Browser check in Chrome 155 with the deltatre DAI stream, the whole VOD buffered sequentially, same page for every build. Start of each discontinuity sequence (
frag.startof its first fragment) and the delta against 1.6.17 in ms:Video
endPTSof every fragment follows the same pattern: 1.7.3 and canary end cc 4 320 ms late, this PR 40 ms. The constant 40 ms is the first segment's composition offset from the DTS anchor at session start, which Update minimum browser support documentation and build config video-dev/hls.js#7880 introduced on purpose. Remuxer warnings with this PR match 1.6.17 line for line, shifted by that constant: no hole and no audio injection at cc 1 and cc 2, and the 171 ms and 474 ms stream holes at cc 3 and cc 4. 1.7.3 and canary log the 86, 108, 251 and 514 ms holes and the 101, 80, 80 and 54 ms audio injections from the issue. With the second commit the remuxer logsReset initPTS ... consecutive: trueat each of the four discontinuities inside the worker and the numbers are unchanged.Seek back to unbuffered start of VOD re-downloads first fragment in infinite loop (video track never appended) on 1.6.16 — appears fixed in 1.7.0-beta.2 video-dev/hls.js#7932 scenario in the same browser (deeplearning.ai stream,
startPosition: 28.8,currentLevel = 1atMANIFEST_PARSED, seek to 0 after 6 s). This PR and master canary behave the same: initPTS anchored at 0.02 (1800/90000), sn=1 loaded once after the seek, video SourceBuffer gains [0.047, 10.057] and audio [0.013, 10.061],readyState4. 1.6.17 in the same harness anchors at 0.0867, re-downloads sn=1 every tick (stopped after 5), the video SourceBuffer never covers 0 andreadyStatestays 1, which is the reported bug.v1.6.18 regression: paused seek presents a diffferent frame than with 1.6.17 video-dev/hls.js#7977 page (elephants dream fixture, paused seek to 51.0 right after
MANIFEST_PARSED): this PR and master canary anchor initPTS at 1.400 (126000/90000), buffer from 0.083 and present the same frame (identical 8x5 luma signature from a canvas capture). 1.6.17 anchors at 1.483, 83 ms later (two frames at 24 fps), and presents a different frame, which is the difference that issue reported and Rob documented as intended. No change against master.Not covered in a browser: a level switch landing exactly on a discontinuity. The public test streams with discontinuities have a single level, so that path is covered by the unit specs only.
Resolves issues:
Refs video-dev#8064. Fork-only for internal review before proposing upstream.
Checklist
🤖 Generated with Claude Code