From 62b0abd57225047bbc93726f141e37470d55325b Mon Sep 17 00:00:00 2001 From: Nicolas Date: Tue, 15 Sep 2026 20:18:29 +0200 Subject: [PATCH] Fix casting past the first segment, and the toolbar's real auto-hide bug Two bugs reported together, both surfaced by actually trying this for real. Casting: "the .m3u8 needs to be passed instead of a raw [url]". The master playlist itself was fine - the initial master.m3u8?castToken=... request already carries the token. What broke immediately after: every URI line inside that playlist gets rewritten (EmbyClient.rewritePlaylist) to route back through this app's own party-stream/:partyId/raw?u=... proxy, and that rewrite never carried the castToken forward. A browser never noticed (it's still sending its cookie regardless of the URL), but a Chromecast has no cookie to fall back on, so every segment/variant- playlist fetch after the first one 401'd, breaking playback moments after the receiver appeared to connect. Fixed by threading the same castToken into every rewritten line (rewritePlaylist(text, routeKey, castToken?), sourced from req.query.castToken right inside proxyResponse). The self-hosted burn-in path had the identical gap for a different reason - ffmpeg's own bare relative segment filenames drop any query string when resolved against a URL that had one, per ordinary URL-resolution rules - fixed the same way with a new pure appendCastTokenToSegments(text, castToken), extracted specifically so it's unit-testable without a real ffmpeg session. Player toolbar: "still doesn't work, mobile and PC" - the auto-hide fix from two commits ago hadn't actually landed for anyone. Real bug, not a browser quirk this time: becomeHost's 3-second heartbeat got wired to the same onDriverStateChanged function real discrete play/pause/seeked events use, which also re-arms the auto-hide countdown - correct for a genuine event, wrong for a heartbeat that fires unconditionally every 3s regardless of any real change. With the hide delay also at 3000ms, every heartbeat tick cancelled and restarted the countdown on the exact cadence it needed to run uninterrupted to ever complete - so for a host, on any platform, the bar could never actually finish hiding. Fixed by giving the heartbeat its own function, reportHeartbeat - same state sync, without the scheduleControlsHide call. Verified: full typecheck/test/build (122/122 server tests, including new coverage for rewritePlaylist's token propagation - had none at all before this - and appendCastTokenToSegments). Confirmed the heartbeat fix directly: 3 simulated heartbeat ticks call the hide-timer's reset zero times, one simulated real event still calls it once. Not verified: a real Chromecast fetching real rewritten segment URLs - no device reachable from this sandbox. Co-Authored-By: Claude Sonnet 5 --- client/src/views/player/Player.ts | 33 +++++++++- docs/architecture.md | 83 +++++++++++++++++++++++++ server/src/emby/EmbyClient.ts | 18 +++++- server/src/services/TranscodeService.ts | 37 ++++++++++- server/test/embyClient.test.ts | 53 +++++++++++++++- server/test/transcodeService.test.ts | 16 ++++- 6 files changed, 230 insertions(+), 10 deletions(-) diff --git a/client/src/views/player/Player.ts b/client/src/views/player/Player.ts index 3e8f939..269e62d 100644 --- a/client/src/views/player/Player.ts +++ b/client/src/views/player/Player.ts @@ -432,12 +432,39 @@ function renderVideoPlayer(root: HTMLElement, partyId: string, party: PartyPubli * session's RemotePlayerController events (startCasting's own * onRemoteStateChanged callback) alike - so the button icon, the * seek bar, and the party broadcast all stay in lockstep regardless - * of which driver or which gesture actually caused the change. */ + * of which driver or which gesture actually caused the change. + * Re-arms the auto-hide countdown too (e.g. resuming from pause must + * restart it, not leave the bar stuck open forever) - deliberately + * NOT used for the periodic heartbeat below, which needs the same + * state-sync but must NOT touch the countdown - see its own comment + * for the real bug that came from getting this mixed up once. */ function onDriverStateChanged() { updatePlayPauseBtn(); updateProgressBar(); sendLocalState(); - scheduleControlsHide(); // e.g. resuming from pause must re-arm the countdown, not leave the bar stuck open forever + scheduleControlsHide(); + } + + /** + * The 3s heartbeat's own tick - reported "still playing at position + * X" regardless of whether anything actually changed, unlike every + * other caller of onDriverStateChanged above, which only ever fires + * on a genuine discrete event. Deliberately does NOT call + * scheduleControlsHide(): a real bug, not a hypothetical one - with + * CONTROLS_HIDE_DELAY_MS also at 3000ms, wiring the heartbeat straight + * to onDriverStateChanged meant it re-armed the auto-hide countdown + * on the same cadence the countdown itself needed to elapse + * uninterrupted to ever fire - so for a host, the bar could never + * auto-hide at all, on any platform, unless they happened to stop + * touching the player at exactly the right phase relative to the + * heartbeat's own timer. icon/progress bar are cheap to refresh here + * too (the dedicated progressTicker already does the latter far more + * often, so this is only a safety net for the former). + */ + function reportHeartbeat() { + updatePlayPauseBtn(); + updateProgressBar(); + sendLocalState(); } let isDraggingProgress = false; @@ -582,7 +609,7 @@ function renderVideoPlayer(root: HTMLElement, partyId: string, party: PartyPubli video.addEventListener('pause', onDriverStateChanged); video.addEventListener('seeked', onDriverStateChanged); video.addEventListener('ended', () => { if (hasNextQueued) conn?.next(); }); - if (!heartbeat) heartbeat = window.setInterval(onDriverStateChanged, 3000); + if (!heartbeat) heartbeat = window.setInterval(reportHeartbeat, 3000); // Burn-in (image) subtitle options are host-only (see // renderSubtitleOptions) - becoming host mid-session must unlock diff --git a/docs/architecture.md b/docs/architecture.md index 65ba2f0..93471b4 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -2845,6 +2845,89 @@ new per-row button renders correctly alongside the existing controls, at both desktop and phone width (the phone-width wrap is the same `@media` rule `views/series` already relies on, untouched here). +## Casting broke after the first segment: the rewritten playlist URLs never carried the castToken + +Reported directly: "for cast, the .m3u8 needs to be passed instead of a +raw [url]". The master playlist itself was never the problem - `POST +.../cast-token` mints the token, and the initial `master.m3u8?castToken=...` +request already carries it. What broke immediately after: every URI +line inside that playlist gets rewritten (`EmbyClient.rewritePlaylist`) +to route back through this app's own `party-stream/:partyId/raw?u=...` +proxy instead of hitting Emby directly - and that rewrite never carried +the castToken forward at all. A browser doesn't notice, because it's +still sending its session cookie on every one of those follow-up +requests regardless of what's in the URL. A Chromecast has no cookie to +fall back on (see `CastTokenService`) - so its segment/variant-playlist +fetches hit `sessionForStream` with no auth of any kind and 401'd, +breaking playback moments after the receiver appeared to connect. + +Fixed by threading the *same* castToken the master-playlist request +itself carried into every rewritten line - `rewritePlaylist(text, +routeKey, castToken?)`, sourced from `req.query.castToken` right inside +`proxyResponse` (which already receives `req`, so no new parameter +needed anywhere upstream of it). Omitted entirely when there wasn't one +(the normal browser/cookie case - unaffected either way, since a +browser was never relying on this). + +The self-hosted burn-in path (`TranscodeService`) had the exact same +gap for a different reason: ffmpeg's own HLS muxer writes bare relative +segment filenames into its `master.m3u8`, which a browser resolves +against the request's own URL (keeping its cookie without even +noticing) but which - per ordinary URL-resolution rules, not a bug in +ffmpeg or this app - silently drops any query string when resolved +against a base URL that had one. Fixed the same way: a new pure +`appendCastTokenToSegments(text, castToken)` (extracted specifically so +this could be unit-tested directly, unlike `serveMasterPlaylist` itself +which needs a real ffmpeg session and on-disk files to exercise at all) +appends `?castToken=...` to each segment line when a request arrives +with one; the plain `fs.createReadStream(...).pipe(res)` fast path is +untouched when it doesn't. + +Verified: full `typecheck`/`test`/`build` (122/122 server tests, +including new coverage for `rewritePlaylist`'s token propagation - had +no test at all before this, for the rewriting itself or the token fix - +and for `appendCastTokenToSegments`). **Not verified**: an actual +Chromecast fetching real rewritten segment URLs - no device reachable +from this sandbox, same standing limitation as the rest of "Casting to +a Chromecast" above. + +## The player toolbar still wasn't auto-hiding: a heartbeat and its own hide-timer, both every 3s + +Reported right after the mobile touch fix: "the toolbar still doesn't +work, mobile and PC" - meaning the *other* auto-hide fix hadn't +actually landed for anyone, not just touch devices. Root cause this +time: an actual bug in this session's own previous edit, not a browser +quirk. `becomeHost`'s 3-second heartbeat (`sendLocalState`, needed +regardless of any real activity, so a follower's periodic resync never +goes stale) got rewired to call the same `onDriverStateChanged` the +native `