Skip to content

fix(mediastream): serve the requested episode on direct play, and fix dropped progress updates - #926

Open
Vincius-dev wants to merge 2 commits into
5rahim:mainfrom
Vincius-dev:fix/mediastream-direct-play-cache
Open

fix(mediastream): serve the requested episode on direct play, and fix dropped progress updates#926
Vincius-dev wants to merge 2 commits into
5rahim:mainfrom
Vincius-dev:fix/mediastream-direct-play-cache

Conversation

@Vincius-dev

Copy link
Copy Markdown

Fixes #925.

Opening this alongside the issue as a concrete reference for the approach. Happy to close it and discuss there first, or to split it into two PRs, since the two fixes are independent.

What problem does this solve?

Two bugs hit while watching a local library from a browser with direct play.

Going to the next episode plays the previous one. /api/v1/mediastream/direct serves whichever file currentMediaContainer points at, and every file was given that exact same URL. The URL identifies a slot, not a file, so the browser treats consecutive episodes as one resource and replays the cached bytes of the previous one. The endpoint also sent no cache headers, unlike /subs and /att. Devtools shows the browser revalidating the new episode with the previous file's If-Range, and "Disable cache" makes the bug go away.

AniList progress is not updated. Two independent paths in mediacore swallow it, described in the issue: a PlaybackLoadedEvent for a new playback is rejected as stale, and the CompletedEvent handler reads the active playback state after the session may already have been cleared.

What changed?

Direct play (commit 1)

  • DirectPlayStreamUrl(hash) builds /api/v1/mediastream/direct?hash=<hash>. The hash is already computed for every MediaContainer, so each file now gets a distinct URL and the browser cannot mix them up.
  • ServeEchoDirectPlay returns 404 when the requested hash is not the loaded file. The check is skipped when the param is absent so a client holding an older stream URL keeps working.
  • HandleMediastreamDirectPlay sends no-store, same as the sibling media endpoints.
  • On the page, the stream URL is now tied to the file it was requested for. Selecting an episode changes filePath before the new container is requested, so without this the player mounts with the previous episode's URL for one render. Also switched the direct HMAC token separator to &, since the URL now carries a query param.

Progress (commit 2)

  • A PlaybackLoadedEvent takes over the session instead of being rejected as stale.
  • The playback state is snapshotted when the CompletedEvent is ingested, while it is still guaranteed to belong to that event, and the update runs from that snapshot. This mirrors what TerminatedEvent already does for continuity.
  • Removed directstream.updateCompletedProgress. Its type assertion never matched a real stream, so it was dead code, and it ignored both the AutoUpdateProgress setting and the current entry progress. Making it work would have introduced those two bugs, and mediacore already covers the case correctly.

Why this approach?

Putting the hash in the URL rather than, say, disabling the cache alone: no-store on its own fixes the symptom but the endpoint would still be able to serve a file the client did not ask for whenever the client and server are briefly out of sync. With the hash the client either gets the file it asked for or a 404, never the wrong episode silently.

For the progress fix I kept the coordinator as the single owner of the update rather than reviving the directstream fallback, so the setting and the "progress already ahead" check stay in one place.

How was it tested?

go test ./internal/mediacore/... ./internal/mediastream/... ./internal/directstream/... ./internal/handlers/..., plus npm run typecheck and npm test for the web changes.

New tests, all of which fail without the corresponding fix:

  • internal/mediastream/directplay_test.go — the URL is unique per file, the loaded file is served, a hash for a file that is not loaded gets a 404 and does not leak the loaded file's content, and a request without a hash still works.
  • internal/handlers/mediastream_test.go — the direct play response is never cacheable.
  • internal/mediacore/mediacore_test.go — progress is updated on completion, for the next episode both with and without a terminate in between, when the session is torn down right after completion, and not at all when the setting is off.

Note: TestUsesPrivilegedCommandSettings in internal/handlers already fails on a clean checkout here, unrelated to this change.

Risks and limitations

  • The hash check is opt-in on the request side, so a stale client is not protected, only a current one. That keeps it backwards compatible.
  • no-store means seeking far back may refetch instead of reading from the HTTP cache. Media playback buffers in memory anyway, and correctness matters more here.
  • The client-side race fix only covers /mediastream. The transcode path shares the same constant URL shape (master.m3u8), but HLS re-requests the playlist and the transcoder is restarted per file, so I left it alone rather than widening the change.

AI Disclosure

  • Tool(s) used: Claude Code (Opus).
  • What AI was used for: investigating both bugs from the symptoms, locating the responsible code paths, writing the fixes and the tests, and drafting this description.
  • Relevant prompts or instructions: asked it to investigate the two bugs I was hitting, understand how the project is architected, then fix both following existing conventions and covering them with tests.
  • What I manually reviewed or changed: I reproduced both bugs on my own server, captured the devtools evidence in the issue myself, and confirmed the direct play one disappears with "Disable cache". I reviewed the diff and the tests.
  • How I verified the change fits Seanime's architecture: the progress update stays in mediacore, which already owns that effect, and the cache headers follow the existing /subs and /att handlers. The tests use testmocks helpers and live in {file}_test.go as CONTRIBUTING asks.

The direct play endpoint serves whichever file is currently loaded and its
stream URL was the same for every file, so the browser reused the cached
response of the previous episode. Add the file hash to the URL, reject
requests for a file that is not loaded, and stop caching the response.

Also ignore the stream URL of the previous episode while the new media
container is being requested, otherwise the player loads it for a moment.
Two cases dropped the AniList progress update:

- a PlaybackLoadedEvent for a new playback on the same target and client was
  rejected as stale, so every event of the new session was ignored
- the CompletedEvent handler read the active playback state asynchronously,
  which a TerminatedEvent (auto next episode) could clear first

The state is now captured when the event is ingested. Also drop the
directstream fallback, its type assertion never matched a real stream and it
ignored both the AutoUpdateProgress setting and the current entry progress.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: Direct play serves the previous episode, and AniList progress is not updated

1 participant