Repository navigation
feat(web): play a download offline by remuxing it in the browser - #225
mia-riezebos wants to merge 2 commits into
Conversation
A finished download plays from browser storage: Mediabunny remuxes the original file to fragmented MP4 in the page and the existing MSE attachment feeds it, now driven by runs instead of URLs. Seeks restart the remux at the keyframe before the target, read through the file's own index, so nothing is copied. The Downloads page offers Play for a download this browser plays and names the reason for one it can't. Closes #175 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou'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 11 minutes. View limit details
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 |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The offline player needs a reachable cold-start entry point, and backpressured remux runs need abort-aware cleanup before this is ready to merge.
Reviewed changes Reviewed the offline playback flow, its browser-storage and codec integration, and the shared progressive-player refactor.
- Local playback: Reads downloaded Files from OPFS and remuxes selected video/audio into fragmented MP4 with indexed seeks.
- Player integration: Adds offline session loading, local audio selection, codec refusal messages, and Downloads Play links.
- Shared streaming: Separates server fetch/retry handling from MSE attachment so online and offline playback consume the same run interface.
- Coverage: Adds codec-planning and offline browser tests. The 291 web unit tests, production build, and Svelte checks passed locally; the media browser suite was blocked by
Executable not found in $PATH: "ffmpeg". Safari HEVC playback remains unverified.
gpt-6.1-sol ｜ 𝕏
| variant="ghost" | ||
| size="icon-sm" | ||
| aria-label="Play" | ||
| href={offlineHref(record)} |
There was a problem hiding this comment.
[P1] Make the offline Play entry point reachable without the server. A cold load of /downloads still runs the browse layout's loadSession() and libraries.viewable(), so with the API down it shows only “Server unreachable” and “Try again”, not these Play links. The offline player's Back link also returns to that failing route, leaving downloaded Items inaccessible unless the viewer already knows their player URLs.
Technical details
# Offline Downloads navigation
## Affected sites
- `apps/web/src/routes/(browse)/downloads/+page.svelte:171`: the new offline playback entry point is inside the server-dependent browse layout.
- `apps/web/src/routes/(browse)/+layout.ts:9-12`: always loads the session and viewable Libraries before rendering Downloads.
- `apps/web/src/lib/components/Player.svelte:117`: offline Back targets `/downloads`.
- `apps/server/src/api/offline-browser.test.ts:49-52`: tests navigate directly to the player and bypass the user-facing entry point.
## Required outcome
- A viewer reopening the cached app with the server unavailable can reach their local downloads and start playback without knowing an Item or Version id.
- Returning from offline playback can display those downloads without requiring the API.
- Add a cold-navigation browser test through Downloads, rather than only opening a known player URL.
## Reproduction
- Serve the built shell with `/rpc/*` returning a non-JSON 503.
- Open `/downloads` in a fresh page.
- Observed: `Server unreachable`, connection explanation, and `Try again`; no Downloads list.| return; | ||
| } finally { | ||
| // A superseded run stops producing; a fetch body is already aborted. | ||
| if (stopped()) void reader.cancel().catch(() => {}); |
There was a problem hiding this comment.
[P2] Cancel the reader even when its buffer wait has not completed. This finally is unreachable while waitWhileFull() is awaiting timeupdate: that wait ignores the run's abort, and closing removes its listener without resolving the promise. A full-buffer backward seek retains the superseded remux run, and closing while backpressured never invokes its producer's cancel().
Technical details
# Backpressured run cancellation
## Affected sites
- `apps/web/src/lib/progressive.ts:145-153`: `waitWhileFull()` checks only the closed signal between waits, not the current run's signal; aborting the listener does not settle its promise.
- `apps/web/src/lib/progressive.ts:231-238`: reader cancellation depends on that wait completing.
- `apps/web/src/lib/offline/session.ts:105`: local producers ignore the openRun signal and rely on stream cancellation.
- `apps/web/src/lib/offline/remux.ts:206-208`: producer cleanup runs only when the stream is cancelled.
## Required outcome
- Superseding or closing a run promptly cancels its reader, including while paused with a full buffer.
- Buffer waits settle on both per-run and attachment abort; stale runs must not remain waiting for playback to catch up.
- Add cancellation coverage for a full-buffer seek and a full-buffer close without subsequent `timeupdate` events.
## Reproduction
- Fake MSE harness with buffered range 100..200 and initial position 100 enters the high-water wait.
- Seek backward to 0, then close the attachment without dispatching `timeupdate`.
- Observed: two runs opened, zero stream cancel callbacks, including after close.Two runs starting in one tick, as a session reopening at a position does, left the aborted run's updateend queued; the new run's first append resolved on it and the next append threw mid-update, stopping playback. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Important
No new issues found in this commit. The two unresolved findings from the previous review still need addressing before approval.
Reviewed changes Reviewed the MSE synchronization fix in f46c6c7 against the previously reviewed 3cbaa85.
- Replaced completion waits: Waited for
SourceBuffer.updatingto clear after appends, removals, and run-restart aborts, preventing staleupdateendevents from releasing an active append early. - Verified event ordering: Exercised the stale-event race with a fake-MSE harness; confirmed actual completion permits the next append. All 291 web unit tests and Svelte diagnostics passed. Real-browser media playback was not rerun for this delta.
gpt-6.1-sol ｜ 𝕏
|
Investigated the failed CI job: the only failure is the existing Task list (1/3 completed)
|
A download could be saved to the browser (#174) but not played: the player only knew how to ask the server for a stream. This is the playback slice of #173: a finished download plays from browser storage with the server off, and seeks.
How it plays. The player opens the download's OPFS file as a
Fileand Mediabunny remuxes it to fragmented MP4 in the page. The fragments go through the same MSE attachment the server's progressive stream uses (progressive.ts), which now takes "runs" instead of URLs: the online player hands itfetchRuns(url)(the old fetch, 503 queueing andx-stream-offsetmoved there unchanged), the offline one hands it a remux run. A seek outside the buffer cancels the run and starts a new one at the keyframe before the target, found through the file's own index (Matroska cues, MP4 sample tables). The remux pulls one fragment at a time and reads the file in place withBlob.slice, so nothing is copied: OPFS holds one copy per download.Which audio. The first audio track the browser plays natively. The audio menu lists every track; picking one that can't play shows why.
What can't play says why. The app checks the download's probed codecs against
MediaSource.isTypeSupportedbefore offering Play. The Downloads page shows Play only for downloads this browser plays, and names the reason in the caption otherwise: "This browser can't play HEVC", "Needs audio conversion" (Dolby, DTS, PCM; #176 turns this into conversion), or "Needs a server copy" (TrueHD, VC-1, MPEG-2, which no browser decodes). The player shows the same reason if opened anyway.Entry point. Play on the Downloads page opens
/play/{item}?version={version}&offline. That route skips the session check, so with the service worker's cached shell it opens with the server down. The offline session reports no progress; offline progress is #180.MSE append fix. Reopening a session at a position (a track or Version switch) starts two MSE runs in one tick; the aborted run's queued
updateendreleased the next append early and the one after threw mid-update. Appends now wait while the buffer is updating. Found while testing #233; it affected the online progressive player too.Closes #175
New dependency
mediabunny1.62.0 (MPL-2.0), the library the PRD chose. It is maintained (released yesterday), has no dependencies, reads Matroska, MP4 and MPEG-TS with their indexes, and writes fragmented MP4 with per-fragment callbacks, which is what MSE needs. It loads as a lazy chunk only when a download plays. The alternative, ffmpeg.wasm, is ~30 MB and copies the input into its own memory filesystem.Screenshots
Downloads, one download that plays and one this Chromium can't (no HEVC decoder):
Player with the server stopped, playing the H.264 download, and opening the HEVC one:
Tests
apps/server/src/api/offline-browser.test.ts(headless Chromium, no database; needsapps/webbuilt): serves the built shell, seeds OPFS with real ffmpeg fixtures, stops the server, then opens the player through the service worker. Proves a 4-minute H.264/AAC MKV plays, seeks past the buffered 90 s and reaches the end; an MP4 plays to the end; HEVC plays where the browser decodes it and otherwise shows "This browser can't play HEVC."; an AC-3-only download shows "Needs audio conversion."; and OPFS holds exactly one file per download after playing. A mutation that restarts every seek from 0 fails the seek test. 5 pass.apps/web/src/lib/offline/playable.test.ts: proves the audio choice and each reason for the codec mixes in the PRD's table. 8 pass.progressive.tschange:player-browser,playback-settings-browser,downloads-browser,next-episode-browserpass against a local Postgres.bun test(apps/web): 291 pass.bun run lint: 0 errors (the 9 existing warnings).bun run check: 0 errors.Not covered by an automated test: HEVC playing, since neither this machine's Chromium nor CI's Chrome has an HEVC decoder; Safari and Chrome-with-hardware need a manual check.
Made with claude-opus-5.5 using Claude Code in T3 Code.
🤖 Generated with Claude Code