Repository navigation
feat(web): show text and ASS subtitles in offline playback - #233
mia-riezebos wants to merge 4 commits into
Conversation
Embedded SubRip and WebVTT tracks of a Matroska download become native text track cues, and ASS tracks draw through libass (JASSUB) with the file's attached fonts. A small reader walks the file's cue index and the clusters near the playhead, since Mediabunny skips subtitle tracks. The Item's stored subtitle files now save with the download and join the same subtitle menu. Closes #177 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Fixed the CI lint failure caused by formatting in two new assertions in Task list (3/3 completed)
|
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Three subtitle-decoding issues should be fixed before merging: sparse cue indexes and CRC-prefixed clusters can lose subtitles, and embedded WebVTT metadata is rendered as dialogue.
Reviewed changes Full review of the offline subtitle path, from Item detail and download storage through track selection and rendering.
- Stored subtitles: Item detail exposes subtitle URLs; downloads save sidecars in OPFS and remove them with the media.
- Embedded subtitles: A new Matroska reader discovers tracks, reads nearby subtitle blocks, handles basic compression, and extracts attached fonts.
- Playback: The shared subtitle menu selects native text tracks or JASSUB-rendered ASS, with forced-track defaults and controls-aware positioning.
- Verification: All 292 web unit tests, the production web build, and Svelte diagnostics pass; targeted structural probes reproduced the findings below. Browser fixtures were not rerun because
ffmpegis unavailable.
gpt-6-astra ｜ 𝕏
| } | ||
| } | ||
| } | ||
| if (clusters.size === 0 && firstCluster !== null) |
There was a problem hiding this comment.
[P2] Do not treat Cues as a complete cluster inventory. When a file has cues for only some clusters, this condition disables the physical scan and read() never visits the others, permanently dropping their subtitles; a structural fixture with clusters at 0/5/10 seconds loses the middle subtitle when only its cue is removed.
Technical details
# Read clusters between cue anchors
## Affected sites
- apps/web/src/lib/offline/matroska.ts:259-288 builds the cluster index exclusively from Cues whenever any exist.
- apps/web/src/lib/offline/matroska.ts:364-375 reads only those indexed clusters.
## Required outcome
- Return subtitle blocks in the requested interval even when their containing clusters are not referenced by Cues.
- Use actual cluster timestamps for range boundaries rather than assuming CueTime is the containing cluster's Timestamp.
- Add a regression fixture containing an uncued cluster between two cued clusters.
## Suggested approach
- Treat cues as seek anchors and walk subsequent physical cluster elements through the requested interval, retaining the existing deduplication.
- The Matroska specification explicitly describes Cues as an index of "certain" clusters; indexing every subtitle frame is recommended, not mandatory: https://www.matroska.org/technical/cues.html| const head = await slice(file, element.start, element.start + 16); | ||
| const [first] = children(head); | ||
| if (first?.id === ids.timestamp) clusters.set(at, uint(head, first)); |
There was a problem hiding this comment.
[P2] Skip CRC-32 before looking for a cluster Timestamp. In a file without Cues, a cluster beginning with CRC-32 never enters clusters, so all its subtitles disappear; the Matroska ordering rules require CRC-32 to come first when present and recommend it for top-level elements. Walk the header children until Timestamp is found and cover CRC-prefixed clusters in the fallback test.
| new VTTCue( | ||
| block.start, | ||
| block.end ?? block.start + fallbackSeconds, | ||
| decoder.decode(block.data).replace(overrides, ""), |
There was a problem hiding this comment.
[P2] Decode the WebM WebVTT envelope before creating the cue. A D_WEBVTT/SUBTITLES block contains an identifier line and a settings line before the actual dialogue, so passing the entire payload here displays those fields as subtitle text and loses their positioning semantics.
Technical details
# Separate embedded WebVTT metadata from cue text
## Affected sites
- apps/web/src/lib/offline/subtitles.ts:162-174 uses the same raw-text conversion for every non-ASS embedded codec.
- apps/web/src/lib/offline/session.ts:37,47-68 advertises embedded webvtt tracks in WebM/Matroska downloads.
## Required outcome
- For D_WEBVTT/SUBTITLES and D_WEBVTT/CAPTIONS, assign only the payload to VTTCue.text and apply the identifier/settings as cue metadata.
- Preserve the separate mappings for SubRip and other supported codec IDs.
- Cover an embedded WebVTT cue with a nonempty identifier and position/alignment settings.
## Reproduction
- A structural WebM subtitle block containing `cue-id\nalign:start position:10%\nHello` reaches VTTCue with that entire string as its text; only `Hello` should be dialogue.
- The envelope is specified here: https://www.webmproject.org/docs/container/#storing-webvtt-cue-in-a-webm-block
…g keys bits-ui remounts a menu's focus scope when its opening frame ends, which moves focus to the trigger and back to the menu. chooseSubtitles focused a row and pressed Enter right after the click, so the key sometimes landed on the menu itself or, after the remount, on its back row. The Subtitles submenu never opened, or the choice never happened, and the offline subtitle tests timed out about one open in seven. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Important
No new issues found in these test changes. The three decoding findings from the previous review remain unresolved, so approval is still withheld.
Reviewed changes Reviewed the browser-test changes since the previous review at ba36bc2.
- Added focus synchronization: Waited for focus inside the opened settings menu, after its starting-style state, before sending subtitle-selection keys.
- Corrected cue-result types: Included
startin the evaluated cue type and formatted the assertions without changing their expectations.
Checked the focus wait against the test helper and installed bits-ui implementation. Browser fixtures were not rerun because ffmpeg is unavailable.
gpt-6-astra ｜ 𝕏
Offline playback (#225) showed no subtitles: the online player gets WebVTT from the server and has ASS and image subtitles burned in, and a download has no server. This adds text and ASS subtitles to offline playback, through the same subtitle menu as online.
Embedded tracks. Mediabunny, which does the remux, skips subtitle tracks, so
lib/offline/matroska.tsreads them from the downloaded file in place: the track list (with zlib and header-stripping compression), the cue index for cluster positions (or a walk of cluster headers when a file has no cues), font attachments, and the clusters from 10 s behind to 30 s ahead of the playhead, each once. SubRip and WebVTT blocks becomeVTTCues on a<track>, so they render and position like online captions and move up when the controls show. ASS blocks go to libass through JASSUB (processChunk), with the track header as the script and the file's attached fonts.Stored subtitle files. The item detail now lists the Item's stored subtitle files with their URLs. Starting a download saves each one next to the media in OPFS as
<versionId>.<track>; Delete removes them with it. They join the menu after the embedded tracks: SubRip and WebVTT as a<track>, ASS through JASSUB.Which shows. The menu is the online one (
PlayerSettings). With no choice made, a forced track shows. Image tracks (PGS, VobSub) aren't listed yet; that is #178. MP4 downloads list only the stored files, since their embedded text tracks (mov_text) aren't read.Also fixed (in #225's branch). Reopening a session at a position started two MSE runs in one tick; the aborted run's queued
updateendreleased the next run's first append early, and the following append threw mid-update ("The video stopped loading"). Appends now wait while the buffer is updating, not for the nextupdateend. Switching subtitles exposed it; switching audio and Version online could hit it too.Closes #177
New dependency
jassub2.5.23 (LGPL-2.1+, libass in WebAssembly), the renderer the issue names. Maintained (released two days ago), renders every ASS feature libass does on an OffscreenCanvas in a worker, and takes Matroska chunks and attached fonts directly. It loads only when an ASS track shows. Its two WebAssembly builds (2.1 MB each) and default font join the service worker's shell cache, so it works offline; that grows the shell by about 4.4 MB.No library reads Matroska subtitles from a
Blobwith random access (matroska-subtitlesstreams the whole file and was last released in 2022), so the reader is ~300 lines of our own EBML walking, limited to what subtitles need.Screenshots
Server stopped, playing downloads:
Tests
apps/server/src/api/offline-browser.test.ts(headless Chromium, server stopped), new cases. 9 pass, three runs in a row:store.test.ts: proves stored subtitles save with the download, a failed one is left out, and Delete removes them. 25 pass.subtitles.test.ts: the item detail lists a stored track with the URL the subtitle route serves.player-browser,playback-settings-browser,downloads-browser,browse.test,src/subtitlesagainst a local Postgres: 20 pass. Webbun test: 292 pass.bun run lint: 0 errors.bun run check: 0 errors.Made with claude-opus-5.5 using Claude Code in T3 Code.
🤖 Generated with Claude Code