diff --git a/docs/architecture.md b/docs/architecture.md index 80df52d..fb66bab 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -348,6 +348,63 @@ banner/`.followerNotice` gap measured 15px after the margin fix, versus 2px before it. The host-only controls (`becomeHost`, settings menu, next-episode button) were re-checked after this change and still work. +## Player track selection: forcing our pick onto Emby's own negotiation + +First real-world bug report once someone actually watched something end to +end: picking a language or subtitle track in the settings menu had no +effect on playback. Streaming itself working at all (this is the first +confirmation of that - see below) narrowed it down: `PlaybackInfo` and its +`TranscodingUrl` are reachable and produce a playable HLS stream, but the +specific track requested wasn't the one that came out the other end. + +Likely cause, reasoned from how Emby/Jellyfin's own web client actually +negotiates this (not verified against this exact server - no way to, from +here): that client sends `AudioStreamIndex`/`SubtitleStreamIndex` as part +of a `POST /Items/{id}/PlaybackInfo` body alongside a `DeviceProfile`, not +as bare query-string params on a `GET` - which is all this app has ever +sent. Emby's `TranscodingUrl` reflects whatever `PlaybackInfo` actually +negotiated, so if the plain `GET` doesn't apply those params, the returned +URL just encodes Emby's own default track choice regardless of what we +asked for - and the old code used that URL as-is. Separately, even a +correctly-selected subtitle track has a second way to silently not show up: +Emby can deliver text subtitles either burned into the video +(`SubtitleMethod=Encode`) or as their own selectable HLS rendition +(`SubtitleMethod=Hls`) - this player's hls.js integration never wires up +`subtitleTrack`/`subtitleTracks` to pick a rendition, so a `Hls`-method +subtitle would be present in the manifest but never actually rendered. + +Fixed defensively rather than by trying to exactly replicate Emby's own +client (`server/src/index.js`'s `/master.m3u8` route + its new +`applyPlaybackOverrides` helper): whichever URL we end up with - Emby's own +`TranscodingUrl`, or the hand-built fallback (which, before this fix, +dropped the audio/subtitle/quality query params entirely - a second, +independent way the same symptom could happen if a title ever took that +path) - our own `AudioStreamIndex`/`SubtitleStreamIndex`/ +`MaxStreamingBitrate` are forced onto its query string afterward, +overwriting whatever Emby put there. Picking a subtitle also forces +`SubtitleMethod=Encode`; picking "Aucun" removes both +`SubtitleStreamIndex` and `SubtitleMethod` rather than just not adding one, +so it also cancels a subtitle Emby's own negotiation had defaulted to. This +means the negotiation call itself no longer has to be trusted at all - it +only needs to return *a* playable `MediaSource`, since every parameter this +app's UI actually exposes gets forced afterward regardless. + +Verified as far as possible without real Emby access: `applyPlaybackOverrides` +is pure `URLSearchParams` manipulation, unit-tested in isolation (adding a +subtitle Emby didn't pick, removing one Emby had defaulted to, re-sending an +unchanged selection across an audio-only change, quality override, and the +no-selection-yet initial-load case) - all five cases produce the expected +final query string. Separately confirmed client-side, by mocking `Hls` to +capture what URL `loadSource` was actually called with: the initial load +already sends the default-selected track's index, and each dropdown change +re-sends a full reload with exactly the accumulated params expected +(switching audio → `AudioStreamIndex` changes; adding a subtitle → both +params present; back to "Aucun" → `SubtitleStreamIndex` dropped again). +What's **not** verified is the actual Emby round trip - whether +`applyPlaybackOverrides`'s forced params, once they reach the real server, +produce a transcode that really carries the requested track. That needs a +real retest. + ## Assumptions to verify **Confirmed against the real server** (not a guess — actually exercised, @@ -359,41 +416,41 @@ error message) renders correctly end to end. The `--theme-primary-color-*` etc. custom properties `client/css/app.css` is built from were read directly off this exact server's live dark theme, not guessed. -**Not yet exercised against the real server** — a successful login hasn't -happened yet (no real credentials used in testing), so nothing past that -point is confirmed: +**Login and basic usage are now confirmed for real** — updated once actual +bug reports started coming from real usage rather than mocked testing +(overlay/pause-request feedback, the series-detail request, and the +track-selection bug above, which is only reachable once a stream is already +playing). That means the following are confirmed working end to end against +this real server, not just reasoned about, even though none were checked in +isolation: response field names (`AccessToken`, `User.Id`/`Name`, item +`Id`/`Name`/`Overview`/`ProductionYear`/`Type`, `Views`' `CollectionType`, +`Shows/{id}/Seasons`/`Episodes`, `Items/Latest`'s plain-array shape, +`PlaybackInfo`'s `MediaStreams`), the `X-Emby-Authorization`/`X-Emby-Token` +header format, `rewritePlaylist` against a real Emby-generated playlist, and +`PlaybackInfo` → `TranscodingUrl` → HLS producing a real watchable stream. +None of this was independently exercised - it's inferred from "the features +built on top of it work in practice, and nobody's reported the classes of +failure that would follow from any of it being wrong" - worth keeping in +mind if something in this area breaks in a way that doesn't fit whatever +specific bug is being chased at the time. + +**Genuinely not yet confirmed:** - **`/Users/{userId}` for token validation** was dropped — the proxy redesign no longer needs it at all (sessions are validated once at login and cached server-side, not re-checked against Emby per-request). -- **Streaming**: the first version hand-built - `/Videos/{id}/master.m3u8?VideoCodec=h264&AudioCodec=aac` directly, and it - broke in the field: Emby's server threw `ArgumentNullException: Value - cannot be null. (Parameter 'key')` — almost certainly an internal - dictionary keyed by something that URL never supplied (`MediaSourceId`, - `PlaySessionId`, `DeviceId` are the usual suspects). Fixed by calling - `GET /Items/{id}/PlaybackInfo?UserId=...` first and using the - `MediaSource`'s own `TranscodingUrl` — the same thing real Emby clients do - — falling back to a hand-built URL (now at least carrying - `MediaSourceId`/`PlaySessionId`/`UserId` from that response) only if no - `TranscodingUrl` comes back. Not yet confirmed against a real response: - whether `TranscodingUrl` is reliably present, and whether it's always HLS - (a direct-play-eligible source might not transcode at all, which `hls.js` - on the client wouldn't handle - see Known limitations). -- **HLS playlist rewriting** (`rewritePlaylist` in `embyProxy.js`) — the - logic is straightforward (rewrite every non-`#` line) but has only been - reasoned about, never run against a playlist Emby actually generated. If - video loads the manifest but segments 404 or hang, start here. -- **Response field names** (`AccessToken`, `User.Id`, `User.Name`, - `ServerId`, `Items[].Id/Name/Overview/ProductionYear/Type`, - `/Users/{id}/Views` returning library folders the same shape as regular - items) — long-standing Emby/Jellyfin convention, not independently - re-confirmed for this Emby version. -- **`X-Emby-Authorization` header format** — same caveat as above. +- **Player track selection** (`AudioStreamIndex`/`SubtitleStreamIndex` + actually taking effect) — see the section just above; a fix is in place, + reasoned from how Emby's API is known to work, but not yet re-tested for + real. +- Whether `TranscodingUrl` is reliably present for every source type (a + direct-play-eligible source might not transcode at all, which `hls.js` on + the client wouldn't handle — see Known limitations), and whether it's + always HLS. - **The Emby web client deep-link shape** (`/web/index.html#!/item?id=X&serverId=Y`) used for "Voir sur Emby" — matches the long-standing convention, not confirmed against this exact - build's routing. + build's routing (nobody's reported clicking it yet either way). ## UI: Emby-like shell, per-type browsing, episode queue diff --git a/server/src/index.js b/server/src/index.js index 992999a..5e83837 100644 --- a/server/src/index.js +++ b/server/src/index.js @@ -233,10 +233,14 @@ app.get('/api/catalog/stream/:itemId/master.m3u8', async (req, res) => { // TranscodingUrl carrying whatever Emby actually needs. // // AudioStreamIndex/SubtitleStreamIndex/MaxStreamingBitrate come from the - // player's language/subtitle/quality pickers - forwarded straight through - // so Emby bakes the right selection into the transcode itself (subtitles - // end up burned into the video this way, not a separate toggleable track - - // simpler than sidecar WebVTT, at the cost of a re-transcode to change). + // player's language/subtitle/quality pickers, forwarded into the + // negotiation on the off chance Emby's GET-based PlaybackInfo honors them - + // but NOT trusted to have taken effect (this app never sends the + // POST-with-DeviceProfile body real Emby web clients use for that + // negotiation, so there's no guarantee a plain GET applies them the same + // way). applyPlaybackOverrides below forces our selection onto whatever + // URL we end up with, Emby-negotiated or hand-built, so a track pick + // can't silently no-op regardless of which path Emby's response takes. const infoParams = new URLSearchParams({ UserId: session.userId }); for (const key of ['AudioStreamIndex', 'SubtitleStreamIndex', 'MaxStreamingBitrate']) { if (req.query[key]) infoParams.set(key, req.query[key]); @@ -258,18 +262,58 @@ app.get('/api/catalog/stream/:itemId/master.m3u8', async (req, res) => { // have one at all) - the hand-built fallback below now at least supplies // the parameters PlaybackInfo reveals (MediaSourceId, PlaySessionId, // UserId) instead of guessing, but is still a fallback, not the primary path. - const target = source.TranscodingUrl - || `/Videos/${itemId}/master.m3u8?${new URLSearchParams({ + let target; + if (source.TranscodingUrl) { + const [urlPath, urlQuery] = source.TranscodingUrl.split('?'); + const params = new URLSearchParams(urlQuery || ''); + applyPlaybackOverrides(params, req.query); + target = `${urlPath}?${params}`; + } else { + const params = new URLSearchParams({ UserId: session.userId, MediaSourceId: source.Id || '', PlaySessionId: info.PlaySessionId || '', VideoCodec: 'h264', AudioCodec: 'aac' - })}`; + }); + applyPlaybackOverrides(params, req.query); + target = `/Videos/${itemId}/master.m3u8?${params}`; + } await proxyEmbyResponse(req, res, target, session.embyToken, itemId); }); +/** + * Forces this app's own audio/subtitle/quality selection onto a transcode + * URL, whether that URL came from Emby's own PlaybackInfo negotiation or + * was hand-built as a fallback - see the route above. Without this, a pick + * from the player's settings menu could silently do nothing if Emby's + * negotiation didn't apply the query-string override the same way its own + * web client's POST-with-DeviceProfile request would have. + * + * SubtitleMethod=Encode is forced whenever a subtitle track is requested: + * left to its own default, Emby may instead deliver text subtitles as a + * separate selectable HLS rendition (SubtitleMethod=Hls) rather than + * burning them into the video - and this player never wires up hls.js's + * subtitleTrack API to pick one, so a rendition-based subtitle would be + * present in the manifest but never actually shown. Encode guarantees the + * subtitle is part of the one video stream this player already renders. + * Picking "Aucun" removes any subtitle Emby's own negotiation added, rather + * than just not adding a new one - otherwise turning subtitles off from a + * state where Emby had already picked a default would be a no-op too. + */ +function applyPlaybackOverrides(params, query) { + if (query.AudioStreamIndex) params.set('AudioStreamIndex', query.AudioStreamIndex); + if (query.SubtitleStreamIndex) { + params.set('SubtitleStreamIndex', query.SubtitleStreamIndex); + params.set('SubtitleMethod', 'Encode'); + } else { + params.delete('SubtitleStreamIndex'); + params.delete('SubtitleMethod'); + } + if (query.MaxStreamingBitrate) params.set('MaxStreamingBitrate', query.MaxStreamingBitrate); +} + // Everything an HLS playlist references (variant playlists, segments) comes // back through here - see embyProxy.rewritePlaylist. app.get('/api/catalog/stream/:itemId/raw', async (req, res) => {