Fix language/subtitle selection: force our pick onto Emby's negotiation
All checks were successful
Publier l'image Docker / publish (push) Successful in 14s
All checks were successful
Publier l'image Docker / publish (push) Successful in 14s
Bug report: picking an audio language or subtitle track in the player's
settings menu had no effect. First real-world bug reachable only once
streaming itself was already confirmed working end to end (this is that
confirmation, per docs/architecture.md).
Root cause, reasoned from how Emby's API actually works (no way to verify
directly - see below): real Emby web clients negotiate track selection via
a POST /Items/{id}/PlaybackInfo body with a DeviceProfile; this app only
ever sent AudioStreamIndex/SubtitleStreamIndex as bare GET query params,
which PlaybackInfo's negotiation likely doesn't apply the same way - so
the TranscodingUrl it returned just reflected Emby's own default track,
regardless of what was actually requested. Separately, even a correctly
selected subtitle has a second silent-failure path: Emby can deliver text
subtitles burned into the video (SubtitleMethod=Encode) or as their own
selectable HLS rendition (SubtitleMethod=Hls) - this player's hls.js setup
never wires up subtitleTrack/subtitleTracks, so a Hls-method subtitle would
sit in the manifest unused.
server/src/index.js: new applyPlaybackOverrides forces
AudioStreamIndex/SubtitleStreamIndex/MaxStreamingBitrate onto whichever URL
the /master.m3u8 route ends up with - Emby's own TranscodingUrl, or the
hand-built fallback (which, before this fix, dropped these three params
entirely - a second, independent way the same symptom could happen).
Picking a subtitle also forces SubtitleMethod=Encode; picking "Aucun"
deletes both SubtitleStreamIndex and SubtitleMethod rather than just not
adding one, so it also cancels a subtitle Emby's negotiation had defaulted
to on its own. The negotiation call no longer needs to be trusted to apply
these params - it only needs to return a playable MediaSource, since every
control this app's UI 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, removing one Emby had defaulted to,
re-sending an unchanged selection across an audio-only change, quality
override, the no-selection-yet initial load) - all pass. Confirmed
client-side too, by mocking Hls to capture what loadSource was actually
called with: initial load sends the default track's index, each dropdown
change re-sends a full reload with exactly the accumulated params expected.
What's not verified is the real Emby round trip - that needs a live
retest, which this environment has no credentials to do.
docs/architecture.md has the full writeup, and its "Assumptions to verify"
section got a broader pass while in there: several items long-flagged as
"not yet exercised against the real server" are now implicitly confirmed
by everything that's worked in real usage so far (response field names,
auth headers, playlist rewriting, basic HLS playback) - narrowed the list
down to what's genuinely still unconfirmed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
f6a8010647
commit
4b2d4822ef
2 changed files with 136 additions and 35 deletions
|
|
@ -348,6 +348,63 @@ banner/`.followerNotice` gap measured 15px after the margin fix, versus 2px
|
||||||
before it. The host-only controls (`becomeHost`, settings menu,
|
before it. The host-only controls (`becomeHost`, settings menu,
|
||||||
next-episode button) were re-checked after this change and still work.
|
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
|
## Assumptions to verify
|
||||||
|
|
||||||
**Confirmed against the real server** (not a guess — actually exercised,
|
**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
|
etc. custom properties `client/css/app.css` is built from were read directly
|
||||||
off this exact server's live dark theme, not guessed.
|
off this exact server's live dark theme, not guessed.
|
||||||
|
|
||||||
**Not yet exercised against the real server** — a successful login hasn't
|
**Login and basic usage are now confirmed for real** — updated once actual
|
||||||
happened yet (no real credentials used in testing), so nothing past that
|
bug reports started coming from real usage rather than mocked testing
|
||||||
point is confirmed:
|
(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
|
- **`/Users/{userId}` for token validation** was dropped — the proxy
|
||||||
redesign no longer needs it at all (sessions are validated once at login
|
redesign no longer needs it at all (sessions are validated once at login
|
||||||
and cached server-side, not re-checked against Emby per-request).
|
and cached server-side, not re-checked against Emby per-request).
|
||||||
- **Streaming**: the first version hand-built
|
- **Player track selection** (`AudioStreamIndex`/`SubtitleStreamIndex`
|
||||||
`/Videos/{id}/master.m3u8?VideoCodec=h264&AudioCodec=aac` directly, and it
|
actually taking effect) — see the section just above; a fix is in place,
|
||||||
broke in the field: Emby's server threw `ArgumentNullException: Value
|
reasoned from how Emby's API is known to work, but not yet re-tested for
|
||||||
cannot be null. (Parameter 'key')` — almost certainly an internal
|
real.
|
||||||
dictionary keyed by something that URL never supplied (`MediaSourceId`,
|
- Whether `TranscodingUrl` is reliably present for every source type (a
|
||||||
`PlaySessionId`, `DeviceId` are the usual suspects). Fixed by calling
|
direct-play-eligible source might not transcode at all, which `hls.js` on
|
||||||
`GET /Items/{id}/PlaybackInfo?UserId=...` first and using the
|
the client wouldn't handle — see Known limitations), and whether it's
|
||||||
`MediaSource`'s own `TranscodingUrl` — the same thing real Emby clients do
|
always HLS.
|
||||||
— 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.
|
|
||||||
- **The Emby web client deep-link shape**
|
- **The Emby web client deep-link shape**
|
||||||
(`/web/index.html#!/item?id=X&serverId=Y`) used for "Voir sur Emby" —
|
(`/web/index.html#!/item?id=X&serverId=Y`) used for "Voir sur Emby" —
|
||||||
matches the long-standing convention, not confirmed against this exact
|
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
|
## UI: Emby-like shell, per-type browsing, episode queue
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -233,10 +233,14 @@ app.get('/api/catalog/stream/:itemId/master.m3u8', async (req, res) => {
|
||||||
// TranscodingUrl carrying whatever Emby actually needs.
|
// TranscodingUrl carrying whatever Emby actually needs.
|
||||||
//
|
//
|
||||||
// AudioStreamIndex/SubtitleStreamIndex/MaxStreamingBitrate come from the
|
// AudioStreamIndex/SubtitleStreamIndex/MaxStreamingBitrate come from the
|
||||||
// player's language/subtitle/quality pickers - forwarded straight through
|
// player's language/subtitle/quality pickers, forwarded into the
|
||||||
// so Emby bakes the right selection into the transcode itself (subtitles
|
// negotiation on the off chance Emby's GET-based PlaybackInfo honors them -
|
||||||
// end up burned into the video this way, not a separate toggleable track -
|
// but NOT trusted to have taken effect (this app never sends the
|
||||||
// simpler than sidecar WebVTT, at the cost of a re-transcode to change).
|
// 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 });
|
const infoParams = new URLSearchParams({ UserId: session.userId });
|
||||||
for (const key of ['AudioStreamIndex', 'SubtitleStreamIndex', 'MaxStreamingBitrate']) {
|
for (const key of ['AudioStreamIndex', 'SubtitleStreamIndex', 'MaxStreamingBitrate']) {
|
||||||
if (req.query[key]) infoParams.set(key, req.query[key]);
|
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
|
// have one at all) - the hand-built fallback below now at least supplies
|
||||||
// the parameters PlaybackInfo reveals (MediaSourceId, PlaySessionId,
|
// the parameters PlaybackInfo reveals (MediaSourceId, PlaySessionId,
|
||||||
// UserId) instead of guessing, but is still a fallback, not the primary path.
|
// UserId) instead of guessing, but is still a fallback, not the primary path.
|
||||||
const target = source.TranscodingUrl
|
let target;
|
||||||
|| `/Videos/${itemId}/master.m3u8?${new URLSearchParams({
|
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,
|
UserId: session.userId,
|
||||||
MediaSourceId: source.Id || '',
|
MediaSourceId: source.Id || '',
|
||||||
PlaySessionId: info.PlaySessionId || '',
|
PlaySessionId: info.PlaySessionId || '',
|
||||||
VideoCodec: 'h264',
|
VideoCodec: 'h264',
|
||||||
AudioCodec: 'aac'
|
AudioCodec: 'aac'
|
||||||
})}`;
|
});
|
||||||
|
applyPlaybackOverrides(params, req.query);
|
||||||
|
target = `/Videos/${itemId}/master.m3u8?${params}`;
|
||||||
|
}
|
||||||
|
|
||||||
await proxyEmbyResponse(req, res, target, session.embyToken, itemId);
|
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
|
// Everything an HLS playlist references (variant playlists, segments) comes
|
||||||
// back through here - see embyProxy.rewritePlaylist.
|
// back through here - see embyProxy.rewritePlaylist.
|
||||||
app.get('/api/catalog/stream/:itemId/raw', async (req, res) => {
|
app.get('/api/catalog/stream/:itemId/raw', async (req, res) => {
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue