Repository navigation
fix(hls): anchor injected subtitle renditions to the origin's timestamps - #14
Conversation
External subtitles injected as WebVTT renditions on a remote HLS source (superuser404notfound#316) appeared early when the origin's media timestamps do not start at the playlist's time 0. An ffmpeg MPEG-TS transcode cut with -copyts -max_delay 5000000 puts the video 10 s after source time, and every cue showed 10 s early; direct play was in sync. The served .vtt was whole-program cues in source time with no X-TIMESTAMP-MAP, so AVPlayer pinned cue time 0 to MPEG-TS timestamp 0 while it placed the A/V on the playlist timeline from the segments' own timestamps. After building the rewritten master, the proxy now reads the head of the segment the load opens on (the one AVPlayer fetches first, so an on-demand transcoder is not sent to another position): the first video PES PTS for MPEG-TS, tfdt over the track's mdhd timescale for fMP4. The anchor is that timestamp minus the segment's playlist start, and the renditions are served with X-TIMESTAMP-MAP=MPEGTS:<anchor>,LOCAL:00:00:00.000 and unchanged cue times. The probe runs in the background under one deadline that covers all of its reads (the segment head and, for fMP4, the init segment), through the authorizing relay when the load has one, and the .vtt handler waits for it inside its existing wait. An anchor near zero, a byte-range or encrypted playlist, an unresolvable EXT-X-DEFINE reference, or a failed probe keeps the plain WebVTT. Tests cover a +10 s MPEG-TS origin (MPEGTS:900000 with unchanged cues), a zero-offset origin and a failed probe (plain body), start-segment selection with EXT-X-DEFINE substitution, fMP4 tfdt parsing, the negative-anchor map, the authorized probe's headers and range, and an fMP4 probe whose segment arrives late and whose init segment stalls returning within the budget.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
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 |
Silo Kody — review completeReview finished. Check the inline comments for findings and verify each suggestion against the code and tests. Reviewing changes in Silo
Review settingsReview OptionsThe following review options are enabled or disabled:
|
![silo-kody[bot]](https://avatars.githubusercontent.com/u/287058192?s=60&v=4){kind=small}
Two inputs from a broken or malicious HLS origin could crash the player while the subtitle timestamp anchor was read or written. The MP4 box walk added a 64-bit extended box size to the current offset. A size of Int.max after any complete box passed the Int.max check and then overflowed on the addition. The walk now compares the size with the bytes left instead, so such a box ends the walk and the rendition keeps the plain WebVTT body. X-TIMESTAMP-MAP converted the anchor's 90 kHz tick count to Int64 before wrapping it to 33 bits. A version-1 tfdt of UInt64.max over a timescale of 1 is finite but far past Int64, and the conversion trapped. The ticks are now wrapped while still a Double. An anchor that is not finite (a playlist whose EXTINFs sum to NaN) is dropped by the probe, and timestampMap answers one with the identity map.
Silo Kody — review completeReview finished. Check the inline comments for findings and verify each suggestion against the code and tests. Reviewing changes in Silo
Review settingsReview OptionsThe following review options are enabled or disabled:
|
Resolve the subtitle proxy against main's credential scoping (NAT-105) and duration bounds (NAT-1): the variant fetch takes the CredentialScope, the program duration goes through sumSegmentDurations, and the timestamp probe sends credentials per URL with credentials.headers(for:). Prepared keeps main's renditionNames. The anchor tests serve their disk origin over HTTP, since the bounded playlist fetch now refuses a non-HTTP answer.
… before its slot A Jellyfin-style origin restarts its transcode at the keyframe before the requested slot (-noaccurate_seek -copyts), so the segment the timestamp probe reads starts up to a keyframe interval before its playlist start and the probe measured a negative anchor. Its media timestamps are still source time, so the mapped rendition showed every cue early by the keyframe gap, and AE#616 then matched lines at a zero offset and left sourceTime ahead of the picture. Only a positive anchor is an offset of the media timestamps (a muxer delay), so negative anchors now keep the plain body and the lead stays AE#616's to measure. The +10 s MPEG-TS case is unchanged.
Silo Kody — review completeReview finished. Check the inline comments for findings and verify each suggestion against the code and tests. Reviewing changes in Silo
Review settingsReview OptionsThe following review options are enabled or disabled:
|
|
@superuser404notfound, I checked how this PR interacts with your AE#616 clock correction (upstream superuser404notfound#617, merged here as 19a222f), and there was a conflict. I fixed it in ae677a5. What was wrong. The timestamp probe reads the segment the load opens on and computes its first media timestamp minus the segment's playlist start. On a Jellyfin-style origin ( What changed. Only a positive anchor is now applied. A muxer delay only moves timestamps later, so that covers the +10 s MPEG-TS case this PR targets. A negative anchor keeps the plain body, which is the behaviour before this PR, and the lead is left for AE#616 to measure. The new test Please confirm one thing. An origin can have both a positive muxer offset and a keyframe restart, for example MPEG-TS with |
|
Thanks for checking this against AE#616 and for the regression test. I don't think the mixed case resolves itself. The probe observes one number, With the map at The negative branch has the mirror case. If So "positive only" is exact when One way to separate the two: |
The probe reads the muxer delay minus the keyframe-restart gap as one number, so keeping only a positive measurement is exact when either is zero. Document the mixed case next to renditionAnchor(measured:), what it does to AE#616, why Silo's own HLS output does not reach it, and the follow-up that would read the delay directly.
|
@superuser404notfound, thanks, that is right: the probe sees Silo's own server does not hit the mixed case. Every HLS job uses
So on Silo the probe reads I also merged AI disclosure: written with Claude Code (claude-opus-5-5). |
Silo Kody — review completeReview finished. Check the inline comments for findings and verify each suggestion against the code and tests. Reviewing changes in Silo
Review settingsReview OptionsThe following review options are enabled or disabled:
|
# Conflicts: # CHANGELOG.md
|
Thanks, the doc comment on |
|
@Rhainland, one more question: once this lands in the Silo fork, would you open it as a PR against superuser404notfound/AetherEngine as well? The anchor fixes a real gap in the superuser404notfound#316 renditions for any origin with a muxer delay, not only Silo's, and upstream has no equivalent yet. If you do, that PR would also be the natural place for superuser404notfound#763 (reading |
Yes, I'll open it upstream once #14 merges here. It won't apply as-is: the probe goes through the fork's refreshable-authorization relay, which isn't upstream, so I'll port it onto upstream |
…e#14 Move the pin from the PR branch head 38140f88 to its merge commit 4c57d95a on AetherEngine main. Both commits have the same source tree (63e3ec86), so the engine code that ships does not change. Refresh the reviewed build-input and selector snapshots for the new project.yml. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* chore(deps): pin AetherEngine with the HLS subtitle timestamp fix Moves the AetherEngine pin to the revision that serves injected WebVTT subtitle renditions with an X-TIMESTAMP-MAP matching the origin's media timestamps. On Silo's MPEG-TS transcodes, external subtitles on the remote-HLS route showed every cue 10 s early. The revision also brings the engine commits merged since the previous pin: dropping credential headers from HTTP hops after an HTTPS redirect, and a 16-thread cap for software decoding. Found during the Series validation, silo-apple#337 C10. * chore(deps): pin AetherEngine to the keyframe-restart anchor fix * ci: refresh the reviewed build-input snapshot for the AetherEngine pin The platform selector seals project.yml. This branch changes only the AetherEngine revision line, which owns no source path, so the reviewed snapshot and the selector seal are refreshed for it. * chore(deps): pin AetherEngine to the merge of Silo-Server/AetherEngine#14 Move the pin from the PR branch head 38140f88 to its merge commit 4c57d95a on AetherEngine main. Both commits have the same source tree (63e3ec86), so the engine code that ships does not change. Refresh the reviewed build-input and selector snapshots for the new project.yml. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Quick104 <31828688+Quick104@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
3 participants
Summary
External subtitles that the engine injects as WebVTT renditions into a remote HLS master (superuser404notfound#316) showed every cue 10 s early on Silo's MPEG-TS transcodes; direct play was in sync. The served
.vtthad noX-TIMESTAMP-MAP, so AVPlayer tied cue time 0 to MPEG-TS timestamp 0, while ffmpeg's-copyts -max_delay 5000000output starts at source time plus 10 s. The proxy now reads the origin's starting timestamp and serves the renditions with a matching map.No issue filed; found during Silo's iOS Series validation (Silo-Server/silo-apple#337, case C10).
What changed
tfdtover the track'smdhdtimescale for fMP4.X-TIMESTAMP-MAP=MPEGTS:<anchor>,LOCAL:00:00:00.000and unchanged cue times. A negative anchor keeps the plain body: it comes from a transcode restarted at the keyframe before the slot, whose media timestamps are still source time, and upstream AE#616 corrects that lead..vtthandler waits for it inside its existing wait.EXT-X-DEFINEreference, or a failed probe keeps the plain WebVTT body, as before.CHANGELOG.md,docs/architecture.mdanddocs/formats.mddescribe the behaviour.Test plan
-copyts). The subtitle comes from the server as WebVTT and is injected by the proxy.MPEGTS:900000with unchanged cues), a zero-offset origin and a failed probe (plain body), start-segment selection withEXT-X-DEFINEsubstitution, fMP4tfdtparsing, the negative-anchor map, the authorized probe's headers and range, and an fMP4 probe whose segment arrives late and whose init segment stalls, returning within the budget.Checklist
CHANGELOG.mdupdatedfeat(...),fix(...),chore(...))Notes
A draft silo-apple PR moves the app's engine pin to this commit once it merges: Silo-Server/silo-apple#642.
AI Disclosure
Note
Anchor injected HLS subtitle renditions to origin media timestamps
External subtitles on remote HLS sources previously served cues at playlist-relative time 0, misaligning them with the origin's actual media timestamps. This adds a timestamp probe that reads bounded heads of the first segment opened by the load.
EXT-X-DEFINEvariables andEXT-X-MAP), then reads the first MPEG-TS video PES PTS or fMP4tfdtfrom a bounded head of the segment and its initialization segment, all within a 20-second deadline.X-TIMESTAMP-MAPheader when a resolved anchor is available.Macroscope summarized ae677a5.