Follow-up to the last fix (wrong Progress param shape): now that the request shape is corrected, the obvious next question is "is it actually being sent right, and is Emby accepting it" - answered from the logs alone, without cross-referencing anything else. PlaystateService.reportProgress now logs the exact outgoing request right before sending it (method, path, query string - everything actually sent) and its outcome right after (the real HTTP status and whether it was ok, or that the call threw), all under [watchparty:playstate]. EmbyClient's own generic [watchparty:emby] failure log still fires too, independently, with the fuller error body - this is a second, feature-scoped log so `docker logs | grep watchparty:playstate` alone shows the whole story for this specific sweep. Deliberately safe to log the full path: the query string here is only ever MediaSourceId/PositionTicks/IsPaused, never a token - the real embyToken travels as a header, not the URL - unlike Logger.stream's own request logging, which stays partial specifically because Emby's own TranscodingUrl can carry an api_key. Tests: request logging (path/params present, token absent), and outcome logging for a success, a non-ok status, and a thrown error each producing a distinct line. Full typecheck/test/build green (96/96 tests). docs/architecture.md updated. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2286 lines
137 KiB
Markdown
2286 lines
137 KiB
Markdown
# Architecture
|
||
|
||
## Goals
|
||
|
||
- Watch the same title, in sync, with other users of the same Emby server.
|
||
- Use Emby as the media source (catalog + streaming) without requiring
|
||
changes to Emby itself or its official clients.
|
||
- Access gated to people with an Emby account on this server — no anonymous
|
||
use, no separate identity system.
|
||
|
||
## Non-goals
|
||
|
||
- Chat/reactions — deliberately out of scope.
|
||
- OIDC/SSO — an OIDC provider (LemonLDAP::NG) is available in principle, but
|
||
the explicit choice was Emby-only auth: simpler, no extra infra dependency,
|
||
and everyone already has an Emby account.
|
||
- Full library navigation (Shows → Seasons → Episodes) in v1 — search-based
|
||
catalog only, to keep the first version achievable. Revisit if browsing
|
||
turns out to matter more than searching in practice.
|
||
|
||
## Why this replaced an Emby plugin
|
||
|
||
The first attempt (still in [`src/WatchParty/`](../src/WatchParty/), kept for
|
||
reference) was a real Emby Server plugin: sync via `ISessionManager`'s
|
||
existing remote-control commands, a settings page via `IHasWebPages`, and —
|
||
after a fair amount of reverse-engineering — a real user-facing page
|
||
integrated into Emby's own web client menu (`PluginPageInfo.EnableInUserMenu`,
|
||
undocumented but confirmed working; required a `data-controller` AMD module
|
||
to satisfy Emby's modern `ViewManager`, found by reading the target server's
|
||
actual `viewmanager.js`).
|
||
|
||
That plugin builds and installs correctly. What it hit a hard wall on: **a
|
||
button inside the video player itself**, or on an item's detail page. An
|
||
Emby developer confirmed on their own forum that plugins have no API for
|
||
this ("It's currently not possible for a plugin to do this"), and nothing
|
||
found in research since suggests that's changed. The only way to get a
|
||
button into Emby's own player would be patching Emby's shipped web client
|
||
files directly inside the Docker container — outside the plugin entirely,
|
||
guaranteed to break on every Emby image update, and requiring manual
|
||
reapplication each time. That trade-off wasn't worth it.
|
||
|
||
Building a standalone app sidesteps the question entirely: it's our own
|
||
player, so there's no "how do I add a button to someone else's UI" problem
|
||
in the first place, and sync can be tighter than nudging Emby's
|
||
remote-control commands ever allowed (see below).
|
||
|
||
Full trace of the plugin investigation (real errors, real fixes, sources
|
||
consulted) is in the git history of this file and of `src/WatchParty/` —
|
||
worth reading if a future Emby SDK update ever makes in-player integration
|
||
possible and this gets revisited.
|
||
|
||
## Components
|
||
|
||
```
|
||
server/ Node.js + Express + Socket.IO, TypeScript (compiled by tsc).
|
||
Proxies 100% of Emby access (catalog, images, HLS/audio
|
||
streaming) and owns party/room state + the realtime sync
|
||
channel - see "Auth and Emby access" right below for why it's
|
||
everything, not just party coordination, and "TypeScript and a
|
||
service-oriented architecture" further down for the current
|
||
file-by-file layout (one singleton service class per feature).
|
||
|
||
client/ Frontend, TypeScript, esbuild-bundled to one app.js + one
|
||
app.css (client/dist) - no framework. Talks only to server/,
|
||
never to Emby directly (api/embyApi.ts and api/party.ts are
|
||
both just fetch/Socket.IO wrappers around this app's own
|
||
/api/* routes). One folder per view, each colocating its own
|
||
.ts/.html/.css - see "TypeScript and a service-oriented
|
||
architecture" further down.
|
||
```
|
||
|
||
Both projects predate this exact shape - see git history and "TypeScript
|
||
and a service-oriented architecture" further down for the plain-JS ->
|
||
TypeScript rewrite itself, including why (a deliberate, requested choice,
|
||
not a default) and what changed structurally.
|
||
|
||
### Auth and Emby access: the server proxies everything
|
||
|
||
Revised after the first version: the browser used to call Emby directly
|
||
(catalog, images, streaming) and only used `server/` for party coordination.
|
||
That hit real CORS problems in practice (Emby/its reverse proxy don't
|
||
answer cross-origin preflights the way that needed), and separately wasn't
|
||
the wanted shape anyway — the browser holding a real Emby access token at
|
||
all was more exposure than necessary. Current design:
|
||
|
||
- The browser sends a username/password **once**, to
|
||
`POST /api/auth/login` on **our own backend** — never to Emby directly.
|
||
- Our backend (`server/src/embyAuth.js`) calls Emby's
|
||
`POST /Users/AuthenticateByName` itself, using `EMBY_SERVER_URL` from its
|
||
own environment (not something the user ever types — see `.env.example`).
|
||
On success it mints its own opaque session id, stores
|
||
`{embyToken, userId, userName}` server-side (in-memory, like party state),
|
||
and hands the browser only that session id, as an `httpOnly` cookie
|
||
(`wp_session`). The browser never sees or holds Emby's real access token.
|
||
- Every other request — catalog search, item details, images, HLS
|
||
streaming, party actions — goes to **our own origin only**
|
||
(`/api/...`), authenticated by that same cookie. `server/src/embyProxy.js`
|
||
is what actually forwards these to Emby server-side and streams the
|
||
response back, attaching the session's real Emby token itself.
|
||
- The `/api/catalog/stream/:itemId/master.m3u8` and `.../raw` routes proxy
|
||
HLS specifically: the master playlist is fetched from Emby server-side,
|
||
and every URI line in it is rewritten to point back through `.../raw`
|
||
(whether Emby wrote it as relative or absolute) — otherwise hls.js would
|
||
follow the playlist's own URLs straight to Emby the moment it tried to
|
||
fetch a segment, defeating the whole point. See `rewritePlaylist` in
|
||
`embyProxy.js`.
|
||
- Images (`<img src>`) and video (`hls.js`/`<video>`) still can't carry a
|
||
bearer header, but that's fine now: they're same-origin requests to our
|
||
own `/api/...` paths, so the session cookie rides along automatically —
|
||
no token in any URL anywhere, unlike the first version's `?api_key=...`.
|
||
|
||
This also means the client no longer needs to ask for a server URL at all —
|
||
one less field, one less way to get it wrong. `EMBY_SERVER_URL` lives only
|
||
in the backend's environment.
|
||
|
||
### Control model: host drives, everyone else follows
|
||
|
||
Now that we own the player, the classic watch-party model (Teleparty,
|
||
Amazon Watch Party, ...) fits better than the old plugin's "everyone keeps
|
||
their own controls, shared reference position" — that design was specifically
|
||
a concession to nudging *pre-existing, independently-running* Emby app
|
||
sessions we didn't control. Here, there's only one player implementation,
|
||
so:
|
||
|
||
- The party creator is the host. Their player has normal controls.
|
||
- Everyone else's `<video>` has controls hidden; a small notice says the
|
||
host is in charge.
|
||
- Host → server → everyone else, over Socket.IO
|
||
(`server/src/socketHandler.js`):
|
||
- Event-driven: every `play`/`pause`/`seeked` on the host's video sends
|
||
`{type:"playstate", isPaused, positionTicks}` immediately.
|
||
- Heartbeat: also sent every 3s regardless, so latecomers/drift get
|
||
corrected even without a discrete host action.
|
||
- Followers apply pause/unpause immediately, and only seek if drift
|
||
exceeds 1.5s (`DRIFT_TOLERANCE_SECONDS` in `client/js/views/player.js`)
|
||
— avoids fighting small timing noise with constant re-seeks.
|
||
- If the host leaves, the server promotes another member (same
|
||
fallback logic the old plugin used for its "reference" member).
|
||
|
||
### Party lifecycle
|
||
|
||
In-memory only on the backend (`server/src/partyManager.js`) — a restart
|
||
clears every party, which is fine: a party is meant to last one watch
|
||
session, not persist for weeks. `removeIdleParties` sweeps anything with no
|
||
activity for 4 hours.
|
||
|
||
## Realtime transport: Socket.IO, and two real memory leaks
|
||
|
||
The realtime layer was originally a raw WebSocket (the `ws` package). Two
|
||
things prompted a rework — a requirement (use Socket.IO instead) and a
|
||
specific worry (memory leaks) — and auditing for the second turned up two
|
||
real, concrete leaks, independent of which transport library is used:
|
||
|
||
1. **Server-side: dead connections never got cleaned up.** A raw `ws`
|
||
server has no idea a client is gone unless it receives a proper close
|
||
frame. A laptop put to sleep, a network drop, a killed tab that never
|
||
gets to send one — none of those fire `ws`'s `'close'` event, so the
|
||
`PartyMember`'s socket reference (and everything it closes over) would
|
||
sit in memory forever, and the party would never get cleaned up by
|
||
anything (`removeIdleParties` only looks at `lastActivityUtc`, which a
|
||
phantom connection doesn't affect). `ws`'s own docs recommend a manual
|
||
ping/pong heartbeat to detect this - Socket.IO has one built in
|
||
(`pingInterval`/`pingTimeout`), which is a real, not just cosmetic,
|
||
reason to prefer it here, not only the requested API ergonomics.
|
||
2. **Client-side: leaving the player any way other than its own "Quitter"
|
||
button leaked everything.** `router.js` only ever did
|
||
`currentRoot.innerHTML = ''` between routes — that destroys DOM nodes,
|
||
but nothing told the outgoing view's *JavaScript* to stop. Clicking a
|
||
sidebar link or the browser's back button while watching left the
|
||
player's 3-second heartbeat `setInterval`, its realtime connection, and
|
||
its `hls.js` instance all running against a detached `<video>` element
|
||
indefinitely - for as long as the tab stayed open. Fixed by having
|
||
`route()` handlers optionally return a cleanup function; `router.js`
|
||
calls whatever the previous view returned right before rendering the
|
||
next one, regardless of how navigation happened. `player.js` returns one
|
||
that clears the heartbeat, destroys `hls`, disconnects the socket, and
|
||
removes the one `document`-level click listener it adds (named, not
|
||
inline-anonymous, specifically so it's removable at all - an anonymous
|
||
listener added fresh on every visit would itself have been a slow leak
|
||
even with everything else fixed).
|
||
|
||
Both are now covered by the same class of test: drive the router through
|
||
several "join a party, then navigate away without clicking Quitter" cycles
|
||
and check nothing accumulates. Verified directly (see session history) by
|
||
instrumenting `document.addEventListener`/`removeEventListener` and
|
||
`setInterval`/`clearInterval` around three such cycles against a really-running
|
||
local server (Socket.IO connection included, not mocked) - adds and removes
|
||
matched exactly (3/3 listeners, 2/2 intervals across two cycles), nothing
|
||
left pending. The server-side heartbeat fix is Socket.IO's own
|
||
well-established mechanism, not custom code, so it wasn't separately
|
||
instrumented the same way.
|
||
|
||
`server/src/socketHandler.js` mirrors the old `wsHandler.js` protocol
|
||
closely (see its own top comment for the exact event list) - the one naming
|
||
difference is the error event, sent as `"errorMessage"` rather than
|
||
`"error"`, since Socket.IO reserves the latter for its own internal error
|
||
handling.
|
||
|
||
## Live lobby, host handoff, no join codes
|
||
|
||
Three related changes, all in service of "the home page should reflect
|
||
reality without a refresh":
|
||
|
||
- **The home page's party list is a live subscription, not a fetch.**
|
||
`client/js/party.js`'s `connectLobby` joins a `"lobby"` Socket.IO room
|
||
(`subscribeLobby` → `server/src/socketHandler.js`) and gets a fresh
|
||
snapshot on subscribe, then again every time `broadcastLobby(io)` runs -
|
||
which is now wired into every place party membership changes: creating a
|
||
party, joining one (both in `index.js`'s REST handlers), and leaving
|
||
(`applyLeave`, used by both the socket `leave`/`disconnect` handlers and
|
||
the REST leave endpoint, so this - and the host-handoff notification below
|
||
- only has to be right in one place). `home.js` returns this subscription
|
||
as part of its router cleanup function, same as the player does for its
|
||
own socket - an open lobby subscription left running after navigating away
|
||
would have been exactly the same class of leak just fixed above.
|
||
- **Host and elapsed time are shown per party**, per the request. Host name
|
||
and member count come from the same snapshot. Elapsed time doesn't: it
|
||
ticks client-side from a `createdUtc` timestamp the server includes once
|
||
(`home.js`'s `setInterval` re-renders every `.elapsed` label every
|
||
second) - pushing a recomputed duration from the server every second for
|
||
every viewer would be pure waste when the client can derive it from one
|
||
number. Verified live (see session history): faking a `parties` push with
|
||
a `createdUtc` two minutes in the past and waiting rendered "2:06", not a
|
||
static value.
|
||
- **No join codes.** `PartyManager` no longer generates or tracks one - see
|
||
its class comment. The home page's live list is now the only way to find
|
||
a party; a code that has to be copied and typed out of band doesn't add
|
||
anything on top of a list that's already always current. `POST
|
||
/api/parties/:partyId/join` replaces the old `:joinCode` route.
|
||
- **Host handoff is live, not just correct in the data model.**
|
||
`PartyManager.leave` already promoted another member to host when the
|
||
host left and destroyed the party once empty - that part predates this
|
||
change. What was missing: a player already open as a follower had no way
|
||
to find out it became host mid-session. `applyLeave` now emits a personal
|
||
`"hostChanged"` (to that one socket, not the room) when this happens;
|
||
`player.js`'s `becomeHost()` - the same function `init()` calls when you
|
||
already are the host - unlocks the controls live, no reload needed.
|
||
`becomeHost` is written to tolerate being called more than once safely:
|
||
`addEventListener` with the same function reference is a no-op if already
|
||
attached, and the heartbeat `setInterval` is explicitly guarded
|
||
(`if (!heartbeat)`) since `setInterval` has no such built-in idempotence -
|
||
calling it twice would leak the first interval's id, the exact class of
|
||
bug the rest of this pass was about fixing, not reintroducing.
|
||
|
||
One incidental bug found and fixed while touching this code: the home
|
||
page's "Rejoindre" button on a live party card used to `navigate()` straight
|
||
to the player **without ever calling the join endpoint first** - it worked
|
||
well enough to load the video and read basic party info (`GET
|
||
/api/parties/:id` never checked membership), but the realtime `"join"`
|
||
would be rejected server-side ("Tu n'es pas membre de cette party.") since
|
||
`socketHandler.js`'s join handler does check membership - meaning sync
|
||
silently never worked for anyone who joined this way. Fixed by awaiting
|
||
`joinParty()` before navigating, same as the library grid's tiles already
|
||
did correctly.
|
||
|
||
**Much later: reported as "the elapsed time shown is the movie's elapsed
|
||
time" - correct, but that was the bug.** The `createdUtc`-based label
|
||
above was never wrong about what it computed (party age), but ticking
|
||
wall-clock time regardless of pause state means it only ever *reads* as
|
||
the movie's own elapsed time by coincidence, for as long as nobody pauses
|
||
- once someone does, the two drift apart, and the number on the home page
|
||
keeps climbing through a pause the actual player has frozen. Fixed by
|
||
giving `LobbyPartyDto` the host's real position instead, for video
|
||
parties only (`positionTicks`/`isPaused`/`positionUpdatedUtc` - null for
|
||
music, where there's no single "elapsed" across a rotating queue of
|
||
tracks the way there is for one movie/episode - `Home.ts` falls back to
|
||
the original `createdUtc` behavior for those). The host is already
|
||
authoritative for this inside their own party's room (`"playstate"`); the
|
||
lobby snapshot just reads the same tracked value `SocketGateway` already
|
||
had, rather than a party-level position needing to be kept in sync
|
||
separately. `broadcastLobby` gained two more trigger points to keep this
|
||
reasonably fresh - an actual pause/play transition (not every 3s
|
||
heartbeat resend, same discipline as the existing playstate-logging
|
||
guard) and advancing the queue (`"next"` - `PartyService.advanceQueue`
|
||
already reset every member's position to zero for the new item, but
|
||
nothing had ever pushed that reset to the lobby before, so the card could
|
||
show a stale position, and a stale title/poster, until some unrelated
|
||
join/leave happened to refresh it). Verified against the real bundled
|
||
client with a faked `parties` push: a paused party's label held exactly
|
||
still across a real wait; a playing one, and a music party still on the
|
||
old `createdUtc` path, both kept ticking up correctly.
|
||
|
||
## Player: no page scroll
|
||
|
||
`.playerWrap` was `width:100%; height:100vh` sitting inside `.mainContent`,
|
||
which has its own padding - the two stacked (100vh *plus* padding) made the
|
||
whole shell taller than the viewport, so the page grew a scrollbar even
|
||
though the video itself was already full-height. Changed to
|
||
`position:fixed; inset:0` so it's sized against the real viewport
|
||
regardless of any ancestor's padding, and (like most players' theater mode)
|
||
covers the sidebar while watching - confirmed via computed style
|
||
(`position: fixed`, all four insets `0px`) rather than just asserted.
|
||
|
||
## Presence overlay and "request pause"
|
||
|
||
Requested as a pair: a semi-transparent overlay showing who's in the party,
|
||
and a way for a non-host to ask the host to pause without a chat feature
|
||
(deliberately out of scope — see Non-goals). Revised once after the first
|
||
pass, based on concrete feedback: the overlay moved from bottom-left to the
|
||
top of the player, and the "someone requested a pause" indicator became a
|
||
specific orange border + pause-icon badge rather than a generic pulse
|
||
animation.
|
||
|
||
- **Layout**: the player's top gradient bar is now `.topOverlay`, a column
|
||
of two stacked rows — `.actionBar` (title/members-text/next/settings/leave,
|
||
what used to be the whole bar) and, directly below it, `#presenceOverlay`.
|
||
Stacking them in normal flow inside one column means the avatar row always
|
||
lands right after the action bar regardless of how tall that bar renders
|
||
(it wraps to two lines on narrow screens) — deliberately not a second
|
||
absolutely-positioned block at a guessed pixel offset, which would either
|
||
overlap the action bar when it wraps or leave an inconsistent gap when it
|
||
doesn't.
|
||
- **Presence**: `client/js/views/player.js`'s `renderMembers()` — already
|
||
responsible for the text member list — now also rebuilds
|
||
`#presenceOverlay`, one circle per member. Each circle shows the first two
|
||
characters of `userName`, uppercased (`initialsFor`), on a background color
|
||
picked deterministically from a small fixed palette by hashing the name
|
||
(`colorForName`) — same person, same color, every render, with no need for
|
||
the server to assign or track one. `opacity: .75` on `.avatar` is the
|
||
"semi transparence" itself, applied to the whole circle (initials included)
|
||
rather than just its background, since a chat-bubble-style translucent chip
|
||
reads clearly enough at that opacity and avoids converting the hash color
|
||
to an rgba string just to keep text fully opaque.
|
||
- **Which circle is "yours"** is decided client-side by comparing
|
||
`member.userName === auth.userName` (the same `auth` singleton
|
||
`embyApi.js` already populates at login/session-restore) rather than
|
||
by `userId` — consistent with the rest of the client already treating
|
||
`userId` as a server/protocol-internal detail (see `hostChanged`'s empty
|
||
payload) and not something to key UI logic on directly.
|
||
- **Request pause**: your own circle — only when you're *not* the host, the
|
||
host already has direct controls — gets `.avatarClickable` and a click
|
||
handler that calls the party connection's `requestPause()`
|
||
(`client/js/party.js` → `"requestPause"` socket event). The server
|
||
(`server/src/socketHandler.js`) broadcasts `"pauseRequested"
|
||
{userId, userName}` to **everyone** in the party, not just the host, so
|
||
the whole group sees why playback is about to pause, and throttles it
|
||
server-side per member (5s, `member.lastPauseRequestUtc`) independently of
|
||
the client's own identical cooldown (`PAUSE_REQUEST_COOLDOWN_MS` in
|
||
`player.js`) — the server-side one is the real guard (anyone poking the
|
||
socket directly bypasses a client-only check); the client one just avoids
|
||
a pointless click doing nothing.
|
||
- Receiving `"pauseRequested"` shows a short banner (`#pauseRequestBanner`,
|
||
"`<name>` demande une pause") and flags the matching person's circle
|
||
(`.avatarPauseRequested`: an orange `#ff9800` border plus a small pause-icon
|
||
badge in its corner — `.avatarPauseBadge`, a plain inline two-bar SVG
|
||
rather than a Unicode glyph, since pause/play characters default to emoji
|
||
presentation on several platforms and look muddy at 10px) — both matched
|
||
by the event's `userId` against each circle's `data-user-id`, both clearing
|
||
automatically after `PAUSE_REQUEST_HIGHLIGHT_MS` (4s), for every member,
|
||
including the requester and the host — deliberately not host-only, so a
|
||
request landing while the host isn't looking at the tab is still visible
|
||
to everyone else, and the requester gets confirmation their own click
|
||
registered. This replaced an earlier plain box-shadow pulse animation and
|
||
a separate immediate-local-feedback ring on just the requester's own
|
||
circle (`.avatarSent`) — collapsed into one unambiguous visual language
|
||
driven purely by the server broadcast (which the requester also receives,
|
||
so there's no need for a separate client-only echo).
|
||
- `.followerNotice` picked up a `margin: 0` it was missing: it's a `<p>`,
|
||
and for an absolutely positioned element `bottom` places the *margin*
|
||
edge, so its default ~1em user-agent margin was quietly rendering it 13px
|
||
higher than its declared `bottom: 24px` ever suggested. Harmless on its
|
||
own until `.pauseRequestBanner` needed to sit a specific gap above it —
|
||
found by measuring both rects and seeing the gap read as 2px instead of
|
||
the ~15px the two `bottom` values alone implied.
|
||
- Nothing about this feature actually pauses anything automatically — "request"
|
||
is literal, the host still has to act on it. Adding an actual forced-pause
|
||
would be a real design decision (can any member override the host's
|
||
playback state? that cuts against "host drives, everyone follows") and
|
||
wasn't asked for.
|
||
|
||
Verified end to end with mocked `members`/`pauseRequested` Socket.IO events
|
||
against the real running local server (see session history): the presence
|
||
row renders directly under the action bar; initials/colors render correctly
|
||
for a two-member party; the host's circle never gets `.avatarClickable`
|
||
(neither when viewed by themselves nor by others); a `pauseRequested` event
|
||
gives the right circle `rgb(255, 152, 0)` as its computed border color and
|
||
`display: flex` on its badge (confirmed via computed styles, not just class
|
||
names) while leaving every other circle untouched; rapid repeat clicks on
|
||
your own circle within the cooldown collapse to exactly one `requestPause`
|
||
emission (three synchronous clicks in one script → the emitted-event count
|
||
increased by exactly one — an earlier two-separate-clicks check had looked
|
||
like a cooldown failure at first, but that was real wall-clock time between
|
||
two separate tool calls exceeding the 5s window, not a code bug); and the
|
||
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.
|
||
|
||
**Much later: a request toolbar replaced the avatar click, and grew a
|
||
second request.** Asked for directly: a visible toolbar for non-host
|
||
members instead of "click your own small circle" (never a very
|
||
discoverable affordance), and a second request alongside pause -
|
||
"please resume". `SocketGateway` gained `requestResume`/`resumeRequested`,
|
||
byte-for-byte the same shape as `requestPause`/`pauseRequested` and
|
||
independently throttled (its own `lastResumeRequestUtc`, not a shared
|
||
budget with pause - asking to pause right after asking to resume, or vice
|
||
versa, isn't the "immediate repeat" this throttle exists to stop). Kept
|
||
as a plain signal, deliberately, same as pause always was: broadcast to
|
||
everyone, no accept/deny state tracked anywhere - the host sees the same
|
||
banner/avatar highlight as everyone else and acts with their own existing
|
||
controls. `Player.ts`'s markup gained a `.bottomOverlay` wrapping the
|
||
banner, the follower notice, and the new toolbar as one flex column
|
||
(same "stack in normal flow, don't hand-pick a pixel offset per box"
|
||
fix `.topOverlay` already needed - see just above) - the two request
|
||
buttons, the banner, and the avatar highlight all share one
|
||
`showRequestBanner()` now, since pause and resume only ever differed by
|
||
which word appears in the banner text. Verified against the real bundle:
|
||
the toolbar shows for a follower and stays hidden for the host; each
|
||
button's own client-side cooldown collapses a rapid double-click to one
|
||
`requestResume` emission; a pushed `resumeRequested` shows the banner
|
||
with the right text and highlights the right avatar.
|
||
|
||
## 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.
|
||
|
||
**Round 2, after a retest: subtitles specifically still didn't show.**
|
||
Found a real gap in the fix above: `applyPlaybackOverrides` was only ever
|
||
applied to the *final* stream URL, not to the `PlaybackInfo` **negotiation**
|
||
call itself - so `SubtitleMethod=Encode` never reached Emby until after it
|
||
had already decided, during negotiation, whether this playback session
|
||
would be a full re-encode or a plain remux. Burning a subtitle into the
|
||
video is only possible with a real re-encode - appending
|
||
`SubtitleMethod=Encode` to a URL for a session Emby had already set up as a
|
||
remux plausibly does nothing, regardless of how correct the URL looks.
|
||
Fixed by applying `applyPlaybackOverrides` to the negotiation's own params
|
||
too (`server/src/index.js`), so Emby is asked to encode *before* it decides
|
||
how the session will actually be handled, not just told afterward.
|
||
Unit-tested (see session history): the negotiation call now measurably
|
||
carries `SubtitleMethod=Encode` whenever a subtitle is requested, same as
|
||
the final URL always did.
|
||
|
||
Also added `server/src/logger.js`'s `streamLog` (`[watchparty:stream]`,
|
||
alongside the party-lifecycle `partyLog` added separately - see "Server-side
|
||
logging" below) specifically because two rounds of blind, reasoned-but-
|
||
unverified fixes for this one bug is the point where guessing stops being
|
||
efficient: one `master.m3u8` request now logs its `itemId` and the
|
||
resolved `audio`/`subtitle`/`subtitleMethod`/`quality` values (never the
|
||
full URL or params blob - Emby's own `TranscodingUrl` can carry an
|
||
`api_key`, confirmed kept out of the log line by a dedicated test case),
|
||
so the next retest can confirm from `docker logs`/journalctl alone whether
|
||
a requested override actually reached Emby, without needing a devtools
|
||
session on whatever machine runs the server. If subtitles still don't show
|
||
once this log line confirms `subtitleMethod=Encode` really was sent, the
|
||
fault has been narrowed to Emby's own handling of that parameter rather
|
||
than anything before it in this app.
|
||
|
||
**Round 3, after a real retest: the override reaches Emby correctly - it's
|
||
just slow.** Not a repeat of Round 2's bug. Confirmed directly from the
|
||
browser's own error report this time (`SubtitleMethod%3DEncode` present,
|
||
URL-decoded, right there in the failing fragment's own request URL - even
|
||
better evidence than the server-side stream log, since it's the literal
|
||
resolved URL rather than what this app computed before sending it), so the
|
||
override chain genuinely works end to end: Emby *is* being asked to burn
|
||
the subtitle in, and does. The failure is downstream of that, in a place
|
||
Round 1/2 had no reason to look at: `hls.js` is constructed with no config
|
||
at all (`new Hls()`, defaults only), and its default fragment-load timeout
|
||
is 10 seconds - fine for a source Emby can remux almost instantly, too
|
||
tight for a source Emby has to fully re-encode from scratch, which
|
||
`SubtitleMethod=Encode` always requires (see the "not a rendition" reasoning
|
||
above - burning a subtitle in is fundamentally a heavier operation than
|
||
picking an audio track or even changing quality, neither of which
|
||
necessarily forces a re-encode the same way). hls.js retried 4 times, each
|
||
timing out the same way, then gave up with a fatal `fragLoadTimeOut` -
|
||
correctly reported by the existing error handling, just for a cause
|
||
("Emby hasn't finished transcoding yet") this player had never actually
|
||
hit before subtitles specifically forced a re-encode on what was probably,
|
||
until now, mostly direct-play/remux traffic.
|
||
|
||
Fixed by giving both players' `Hls` instances explicit, longer timeouts
|
||
(`manifestLoadingTimeOut`/`levelLoadingTimeOut`: 30s, `fragLoadingTimeOut`:
|
||
60s - `views/player/Player.ts`'s `HLS_CONFIG`, duplicated in
|
||
`musicBar/MusicBar.ts` for the same reason the rest of their hls.js setup
|
||
already is) rather than only patching the video player: music tracks go
|
||
through the exact same `PlaybackInfo` → transcode pipeline (see "Music:
|
||
participative playlist" above), so a slow-to-start transcode is a latent
|
||
risk there too, quality changes included, even though only video actually
|
||
has subtitles to trigger it today. Verified the config values are actually
|
||
honored, not silently ignored by a wrong property name, by constructing a
|
||
real `Hls` instance from the exact pinned CDN build
|
||
(`hls.js/1.5.17/hls.min.js`) with this config and reading back
|
||
`hls.config.fragLoadingTimeOut` etc. - all four came back as set, not as
|
||
hls.js's own defaults.
|
||
|
||
**Round 4: a real 500, once the timeout was no longer masking it - traced
|
||
to Emby, not this app.** Exactly what Round 2 said would happen "if
|
||
subtitles still don't show once this log line confirms `subtitleMethod=
|
||
Encode` really was sent": with Round 3's timeout fix giving Emby enough
|
||
rope to actually answer instead of hls.js giving up first, the segment
|
||
request came back with a real, fast 500 - and Emby's own server log
|
||
(shared directly, not this app's) shows exactly why: `[subtitles@f2] Failed
|
||
to load fontconfig fonts!`, thrown out of `StartFfMpeg`/
|
||
`GetDynamicSegment` before a response body ever exists. That's ffmpeg's
|
||
`subtitles` filter (burning text into the video via `libass`) failing to
|
||
find usable fonts/a fontconfig cache in whatever environment Emby's
|
||
transcoder runs in - a deployment/environment issue on the Emby side
|
||
(commonly a container image missing font packages, or a fontconfig cache
|
||
directory that isn't writable), with nothing in this app's request that
|
||
could cause or fix it. Confirmed not a regression to work around here:
|
||
this app deliberately forces `SubtitleMethod=Encode` (see
|
||
`applyPlaybackOverrides`'s own comment) specifically because it never
|
||
wires up `hls.js`'s rendition-based subtitle API - dropping that override
|
||
to dodge this 500 would silently bring back the very first version of
|
||
this bug (a subtitle picked, never shown) instead of fixing anything.
|
||
Nothing to change in this app for this one; it needs fonts/fontconfig
|
||
fixed wherever Emby's own container/host runs.
|
||
|
||
**Round 5: fontconfig fixed, and a *different* item still failed the same
|
||
way - added a real check instead of guessing further.** After Round 4's
|
||
fix, the font loaded fine (`Loading font file '/config/fonts/
|
||
GoNotoKurrent.ttf'`, no more "Failed to load fontconfig fonts") - but a
|
||
retest on the same item still failed, this time with `Could not find
|
||
codec parameters for stream N (Subtitle: hdmv_pgs_subtitle (pgssub))`.
|
||
`hdmv_pgs_subtitle` (PGS) is a Blu-ray **image/bitmap** subtitle format -
|
||
a pre-rendered picture per frame, not text. This app's forced
|
||
`SubtitleMethod=Encode` asks ffmpeg's `subtitles` filter to burn it in,
|
||
and that filter is `libass` - it renders *text* via a font (exactly what
|
||
Round 4 just fixed the ability to do), and has no code path for
|
||
compositing a bitmap at all. Picking a PGS track was never going to work
|
||
here, fontconfig or not. A second item, tested specifically to rule PGS
|
||
in or out as the whole explanation, hit the exact same wall - without
|
||
knowing that item's own subtitle codec, that result was genuinely
|
||
ambiguous (still PGS on a second file? or something else entirely?),
|
||
which is exactly the kind of guess this saga had already spent four
|
||
rounds on.
|
||
|
||
Rather than keep asking for one more log line per item, `getPlaybackOptions`
|
||
(`CatalogService.ts`) now reads each subtitle stream's own `Codec` from
|
||
the same `PlaybackInfo` response it already fetches, and reports
|
||
`supportsBurnIn: false` for the three image-based subtitle codecs
|
||
(`hdmv_pgs_subtitle`, `dvd_subtitle`, `dvb_subtitle` - `IMAGE_SUBTITLE_CODECS`).
|
||
The player's subtitle `<select>` (`views/player/Player.ts`) shows these
|
||
as disabled options labeled "(image, non disponible)" rather than hiding
|
||
them outright - the track still exists and is still listed, just not
|
||
pickable, so it's clear *why* rather than silently missing. This answers
|
||
the "is item 2 also PGS" question directly from the app itself instead of
|
||
needing another round of server-log archaeology, and prevents anyone
|
||
from picking a subtitle that's guaranteed to break playback the same way
|
||
again, for any item, without needing to know any of this history.
|
||
|
||
**Round 6: a third item, same failure, `supportsBurnIn` said it should
|
||
have worked - and finally, why chasing the message by hand kept failing.**
|
||
A YouTube-downloaded video (AV1 video, `mov_text` subtitle - genuinely
|
||
text, `supportsBurnIn` would report `true`) hit the exact same 500. Not
|
||
PGS this time - a third distinct cause. What finally moved this forward
|
||
wasn't the exception message (still never successfully captured by hand,
|
||
across several attempts at pointing at the right log line or the
|
||
browser's Network tab) but a detail that surfaced almost by accident:
|
||
Emby's own access log recorded the failing response as `Content-Type=
|
||
text/plain, Content-Length=5190` - a real, several-KB error body really
|
||
was being sent every time, it just never survived being reported back
|
||
here. `hls.js`'s own error object only ever exposes the generic HTTP
|
||
reason phrase (`"Internal Server Error"`), never the actual response
|
||
text; Emby's live log view apparently doesn't reliably surface the
|
||
exception's headline message either, at least not in whatever's easy to
|
||
copy from it. Both dead ends, independently, on every attempt.
|
||
|
||
Fixed at the root instead of trying a fourth way to ask for the same
|
||
missing text by hand: `EmbyClient.fetch` - the one function every single
|
||
Emby call in this app goes through, `fetchJson`'s JSON API calls and
|
||
`proxyResponse`'s streaming/image proxy alike - now logs Emby's status
|
||
and full response body itself, straight to `docker logs`, whenever a call
|
||
fails. Reads the body via `.clone()` specifically so the real caller
|
||
(whichever one it is) still gets an untouched, fully readable response of
|
||
its own - a `Response` body can only be consumed once, and this had to
|
||
not break either of the two very different ways this app's callers read
|
||
one. `pathAndQuery` is logged with `api_key` redacted, matching
|
||
`logStreamRequest`'s existing concern - this can be called with Emby's
|
||
own `TranscodingUrl` verbatim, which can carry one. Only failures get
|
||
logged at all - a deliberate choice consistent with how the rest of this
|
||
app's logging already works (`Logger.party`/`Logger.stream` also skip
|
||
routine/unchanged traffic) - so this adds real, previously-invisible
|
||
detail on exactly the calls worth seeing, and silence on the thousands
|
||
that don't need it.
|
||
|
||
The next 500 on this or any future item gets its real cause into
|
||
`docker logs` directly, without another round of trying to hand-copy it
|
||
out of a browser or Emby's own log viewer.
|
||
|
||
### The actual fix: subtitles rendered client-side, not burned in by Emby
|
||
|
||
Six rounds in, the pattern across every failure was the same shape: this
|
||
app's own request was never the problem (confirmed each time - the
|
||
override reached Emby correctly, `getPlaybackOptions` already correctly
|
||
told PGS apart from real text), but *something* about asking Emby to
|
||
burn a subtitle into the video itself kept finding a new way to fail -
|
||
missing fonts, a format `libass` fundamentally can't burn (a bitmap, not
|
||
text), then "Error starting ffmpeg" outright on a *third*, otherwise
|
||
unremarkable file with neither of those problems. Three failures, three
|
||
different causes, all downstream of the one thing every attempt had in
|
||
common: asking for a real-time video re-encode. Fixing this by finding
|
||
and patching every way ffmpeg could fail was an open-ended list; not
|
||
needing ffmpeg to do this at all closes the whole class at once.
|
||
|
||
Subtitles are now delivered as their own resource and rendered by the
|
||
browser's native `<track>` element, not encoded into the video:
|
||
|
||
- `CatalogService.proxySubtitle` (new) asks Emby to convert the track to
|
||
WebVTT - `GET /Videos/{itemId}/{mediaSourceId}/Subtitles/{index}/
|
||
Stream.vtt`, a real Emby endpoint their own web client already relies
|
||
on for exactly this, entirely separate from the transcode pipeline.
|
||
Needs its own `PlaybackInfo` round trip just for `MediaSourceId` (the
|
||
video's own negotiation doesn't share this with a `<track src>`'s
|
||
independent request). `applyPlaybackOverrides` lost its
|
||
`SubtitleStreamIndex`/`SubtitleMethod=Encode` branch entirely - nothing
|
||
about the video request needs to know a subtitle was even picked
|
||
anymore.
|
||
- `getPlaybackOptions`'s per-track flag was renamed `supportsBurnIn` →
|
||
`isTextBased` - same underlying fact (image vs. text codec,
|
||
`IMAGE_SUBTITLE_CODECS`), but the name shouldn't keep describing a
|
||
mechanism (burning in) this app no longer uses. A bitmap subtitle still
|
||
has no text for Emby to hand back as WebVTT either, so it's still
|
||
correctly disabled in the picker - the *reason* just changed.
|
||
- `views/player/Player.ts`'s subtitle `<select>` no longer reloads the
|
||
video at all (`reloadWithCurrentOptions`, still used for audio/quality,
|
||
never touches subtitles now) - `applySubtitleTrack` just removes
|
||
whatever `<track>` exists and, for a real pick, adds a fresh one
|
||
pointing at `subtitleUrl(itemId, index)`, then sets
|
||
`video.textTracks[0].mode = 'showing'` **synchronously**, right after
|
||
appending. Deliberately not waiting on the `<track>` element's own
|
||
`"load"` event for that: verified against a real browser that it
|
||
doesn't fire reliably at all (a known, general `<track>` quirk, not
|
||
specific to this app or this fix) - which would have left a
|
||
subtitle silently never shown, the exact symptom the *first* round of
|
||
this whole saga was originally about. The TextTrack object itself is
|
||
registered in `video.textTracks` the moment the element is connected,
|
||
before its content has actually loaded, and a browser honors `.mode`
|
||
being set that early regardless. Re-applied on every video (re)load,
|
||
not just when the subtitle picker itself changes, so switching audio or
|
||
quality - which does still reload the video - doesn't drop whatever
|
||
subtitle was showing.
|
||
|
||
Verified against the real compiled bundle: picking a text track adds
|
||
exactly one `<track>` with the right URL and flips it to `showing`;
|
||
picking "Aucun" removes it and `video.textTracks` goes back to empty; an
|
||
image-coded track is present in the list but disabled, exactly as
|
||
`isTextBased: false` says it should be; changing the audio track reloads
|
||
the video but the previously-selected subtitle's `<track>` survives it,
|
||
same element, same `src`, still `showing`.
|
||
|
||
## Assumptions to verify
|
||
|
||
**Confirmed against the real server** (not a guess — actually exercised,
|
||
see session history): `POST /Users/AuthenticateByName` really is reachable
|
||
from Node's server-side `fetch` with no CORS issue at all (server-to-server
|
||
has no CORS concept), returns a real 401 for bad credentials, and the whole
|
||
login round-trip (browser → our backend → real Emby → real rejection → our
|
||
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.
|
||
|
||
**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).
|
||
- **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 (nobody's reported clicking it yet either way).
|
||
|
||
## UI: Emby-like shell, per-type browsing, episode queue
|
||
|
||
Reworked to look and navigate more like Emby itself, and to treat each
|
||
collection type (movies / TV / music) as its own experience rather than one
|
||
generic grid:
|
||
|
||
- **`client/js/shell.js`** renders the persistent chrome once per login: a
|
||
sidebar (one plain-text link per library from `GET /api/catalog/libraries`
|
||
— no icons, no emoji prefix, per an explicit earlier request) + a
|
||
`#mainContent` element the router (`router.js`) swaps on every navigation.
|
||
`GET /api/catalog/libraries` itself only ever returns libraries whose
|
||
`CollectionType` is `movies`/`tvshows`/`music`/`musicvideos`
|
||
(`VISIBLE_COLLECTION_TYPES` in `server/src/index.js`) — Emby's `/Views` can
|
||
return all sorts of things this app has no UI for at all (boxset
|
||
collections, playlists, live TV, photos, books, a mixed-content folder
|
||
with no `CollectionType`), and filtering once at the source means the
|
||
sidebar, the home page's per-library carousels, and direct navigation to
|
||
`/library/:id` are all restricted the same way without repeating the
|
||
check in each place — a stale link to a since-hidden library's id just
|
||
renders `library.js`'s existing "Bibliothèque introuvable" state, since
|
||
it looks the id up in this same filtered list. `router.js` was made
|
||
idempotent
|
||
(`start(root)` can be called again safely) specifically so login → shell
|
||
boot can happen more than once in a page's lifetime without piling up
|
||
duplicate `hashchange` listeners — the same class of bug the earlier Emby
|
||
plugin work ran into with its page lifecycle, worth remembering.
|
||
- **`views/home.js`** (route `/home`): a horizontal carousel of every active
|
||
party server-wide (the live lobby subscription, not a fetch — see "Live
|
||
lobby, host handoff, no join codes" above) plus one more carousel per
|
||
library of its recently-added items (`GET /api/catalog/libraries/:id/recent`,
|
||
Emby's own `/Users/{id}/Items/Latest`) — skipping music (see below).
|
||
- **`views/library.js`** (route `/library/:id`): dispatches purely on the
|
||
library's `collectionType`. `movies`/`tvshows`/`musicvideos` all get a tile
|
||
grid (`GET /api/catalog/libraries/:id/items`, `Recursive=false` — Movies
|
||
and music videos as themselves, TV as Series tiles); `music` gets a tile
|
||
grid too, just of artists (`GET /api/catalog/libraries/:id/artists`) —
|
||
see "Music: participative playlist" below for the rest of that hierarchy.
|
||
- **Tiles** (`client/js/tiles.js`, shared by the home carousels and the
|
||
library grid so they can't drift apart): poster + name, hover-revealed
|
||
overlay with "Lancer une watch party" (only for directly-playable types —
|
||
`WATCH_PARTY_TYPES` in `server/src/index.js`, which had to grow a
|
||
`MusicVideo` entry once `musicvideos` became a visible library type,
|
||
otherwise every tile in it would render with no way to start a party at
|
||
all; a Series tile gets "Voir les épisodes" instead, linking to
|
||
`/series/:id`) and "Voir sur Emby" (opens Emby's own web client via a
|
||
`embyUrl` the backend builds from `EMBY_SERVER_URL` + the session's
|
||
`serverId`). `:hover`/`:focus-within` reveal it on mouse/keyboard;
|
||
`@media (hover: none)` keeps it always visible on touch devices, which
|
||
have no hover state at all.
|
||
|
||
Bug: "Voir les épisodes" used to render as a bare, default-blue link,
|
||
much less visible than the other two buttons next to it. The global
|
||
`button {}` rule (`app.css`) that gives "Lancer une watch party" its
|
||
solid green look is tag-scoped, so it never applied to "Voir les
|
||
épisodes" — the one `.tileBtn` that's an `<a>` (a plain hash link) rather
|
||
than a `<button>`. "Voir sur Emby" avoided the same fate only because it
|
||
additionally carries `.tileBtnLink`, which explicitly sets a background/
|
||
color for exactly this reason. Fixed by moving the solid-button look onto
|
||
`.tileBtn` itself, tag-agnostic, with `.tileBtnLink` now overriding just
|
||
the two properties that make it read as the secondary action. This also
|
||
happens to be the semantically correct fix, not just a visual patch: for
|
||
a Series tile "Voir les épisodes" *is* the primary action (a Series is
|
||
never itself directly playable, so the two buttons never compete for
|
||
primacy on the same tile) - it should look like one.
|
||
- **`views/series.js`** (route `/series/:id`): a detail header — poster,
|
||
title + year, overview, a "Voir sur Emby" link — above season tabs
|
||
(`GET /api/catalog/series/:id/seasons`, Emby's `/Shows/{id}/Seasons`) and
|
||
an episode list per season (`GET /api/catalog/series/:id/episodes`, Emby's
|
||
`/Shows/{id}/Episodes?SeasonId=...`), each episode startable as its own
|
||
party. The header is just `toItemDto`'s existing fields
|
||
(`GET /api/catalog/item/:id`, the same DTO shape as everything else) —
|
||
the season/episode browsing itself predates this pass and was already
|
||
wired end to end (library grid → Series tile → this route → "Lancer une
|
||
watch party" per episode); what this pass added was making the page
|
||
actually look like a detail page instead of a bare `<h1>` above the tabs.
|
||
`.linklike`'s CSS selector was widened from `button.linklike` to
|
||
`.linklike` so the new "Voir sur Emby" `<a>` picks it up too — there's no
|
||
global `a` reset in `app.css`, so an unclassed anchor would otherwise
|
||
fall back to the browser's default blue/underline against this dark theme.
|
||
|
||
### Episode queue / "next episode"
|
||
|
||
Starting a party from an episode (`POST /api/parties`) now has the backend
|
||
fetch the rest of that series (`/Shows/{seriesId}/Episodes`, no season
|
||
filter) and store everything from the clicked episode onward as the party's
|
||
`queue` (`server/src/partyManager.js`). The host gets an "Épisode suivant"
|
||
button once `hasNext` is true; clicking it (or the video's `ended` event
|
||
firing) sends a `"next"` Socket.IO event
|
||
(`server/src/socketHandler.js`), host-only, which advances `queueIndex`,
|
||
resets every member's known position, and broadcasts `{type:"itemChanged",
|
||
itemId, itemName, ...}` — every client (including the host) reloads the
|
||
player against the new `itemId` on that message. A movie or a
|
||
standalone-started episode just gets a one-item queue, so this is a strict
|
||
superset of the old single-item behavior, not a separate code path.
|
||
|
||
### Player: language, subtitles, quality
|
||
|
||
`GET /api/catalog/item/:id/playback-options` reads `PlaybackInfo`'s
|
||
`MediaSources[0].MediaStreams` and returns the Audio/Subtitle entries as
|
||
`{index, label}` pairs for two `<select>`s in the player's host-only
|
||
controls. Picking a value reloads the stream
|
||
(`/api/catalog/stream/:id/master.m3u8?AudioStreamIndex=&SubtitleStreamIndex=&MaxStreamingBitrate=`)
|
||
at the same position (`hls.js` destroyed and rebuilt, `currentTime`/paused
|
||
state preserved across the reload) rather than restarting from zero.
|
||
"Quality" is a fixed set of `MaxStreamingBitrate` presets (Auto/1080p/720p/
|
||
480p) forwarded to `PlaybackInfo` — the same simplification Emby's/
|
||
Jellyfin's own web clients use; it's a transcode ceiling, not true
|
||
client-side adaptive bitrate switching. Subtitles picked this way get
|
||
burned into the transcoded video (Emby's doing, via `SubtitleStreamIndex`)
|
||
rather than rendered as a toggleable `<track>` — simpler to implement
|
||
correctly than sidecar WebVTT, at the cost of a re-transcode to turn them
|
||
on/off.
|
||
|
||
### Confirmed vs. not, for all of the above
|
||
|
||
Driving an actual browser against this (see session history) confirms the
|
||
**rendering and interaction logic**: sidebar highlighting, home carousels
|
||
(parties + one row per non-music library), library grid dispatch (including
|
||
a `tvshows` library's Series tiles linking to `/series/:id` both via the
|
||
poster and the "Voir les épisodes" button), and the series page's detail
|
||
header + season-tab switching + episode list + starting a party from an
|
||
episode all render and navigate correctly. That test mocked every
|
||
`/api/...` response (canned JSON), since
|
||
exercising it for real needs a logged-in session — so what's **not**
|
||
confirmed yet is the shape of the real Emby responses this now depends on:
|
||
`Views` items actually carrying a `CollectionType` field the way assumed,
|
||
`/Shows/{id}/Seasons` and `/Shows/{id}/Episodes`, `Items/Latest`'s
|
||
plain-array response shape (handled defensively — see `index.js` — but not
|
||
seen for real), and `PlaybackInfo`'s `MediaStreams` shape for the track
|
||
pickers. The queue/"next episode" WebSocket flow and the player's
|
||
track/quality reload are new protocol surface with **no test coverage at
|
||
all yet** (no way to exercise real HLS playback without a live session) —
|
||
the first things to check once a real party actually plays something.
|
||
|
||
## Non-goals
|
||
|
||
Chat/reactions and OIDC/SSO remain out of scope - see Goals/Non-goals at
|
||
the top. Music was in this list for a while (see git history) - it's
|
||
implemented now, see its own section below.
|
||
|
||
## Server-side logging
|
||
|
||
Requested once the app was running for real on the user's own server:
|
||
visibility into what's actually happening, from `docker logs`/journalctl,
|
||
without needing to reproduce a bug through the UI first. `server/src/logger.js`
|
||
is deliberately tiny - `partyLog(message)` (`[watchparty:party]`) and
|
||
`streamLog(message)` (`[watchparty:stream]`, added a bit later for the
|
||
streaming route specifically - see "Player track selection" above), both
|
||
plain `console.log` with an ISO timestamp, plus a `who(userId, userName)`
|
||
formatter shared by every party-lifecycle call site - no logging library,
|
||
consistent with this app's general bias against dependencies it doesn't
|
||
need.
|
||
|
||
Covers the party's business lifecycle end to end, called from wherever the
|
||
state actually changes so every caller is covered without repeating the
|
||
check - `server/src/partyManager.js`'s `create`/`join`/`leave` (which also
|
||
covers a host handoff or the party being destroyed, both of which fall out
|
||
of `leave`)/`advanceQueue`/`removeIdleParties`, plus
|
||
`server/src/socketHandler.js`'s `playstate` and `requestPause` handlers,
|
||
since those two mutate member state directly rather than through a
|
||
`PartyManager` method.
|
||
|
||
The one thing this needed real care over: the host's player re-sends its
|
||
*current* play/pause state every 3 seconds regardless of whether anything
|
||
changed (the heartbeat - see "Realtime transport" above), on top of firing
|
||
once per actual `play`/`pause`/`seeked` event. Logging every `"playstate"`
|
||
message as it arrives would mean one log line every 3 seconds for every
|
||
active party, forever - noise that would drown out everything else within
|
||
minutes on a server with even a couple of parties running. The handler
|
||
instead compares the incoming `isPaused` against the member's *previous*
|
||
value and only logs on an actual flip, silently updating position on every
|
||
other message - so a party generates exactly one `"playstate"` line per
|
||
real pause or resume, never one per heartbeat tick.
|
||
|
||
Verified directly against the real shipped modules (not reimplemented
|
||
mocks) - see session history: a scripted `create`/`join`×2/`advanceQueue`
|
||
(twice — once real, once a no-op past the end of the queue, correctly
|
||
silent)/`leave`×3 (plain, host handoff, final-member destroy)/idle-sweep
|
||
sequence against the real `partyManager.js` produced exactly the expected
|
||
line for every step, in the expected order; a fake `io`/`socket` pair
|
||
driving the real `socketHandler.js` through a start → 5 unchanged
|
||
heartbeats → pause → 3 unchanged heartbeats → resume sequence logged
|
||
exactly 3 `"playstate"` lines (not 9) at exactly the right points, and a
|
||
throttled repeat `"requestPause"` produced no second `"pauseRequested"`
|
||
line - all assertions passed, counted programmatically rather than
|
||
eyeballed.
|
||
|
||
## Music: participative playlist
|
||
|
||
The last deferred piece from the original UI spec, built once asked for
|
||
directly. Three things needed designing before any code: the permission
|
||
model for the shared queue (asked, not assumed - see session history), how
|
||
audio playback fits the existing party/socket infrastructure, and how
|
||
"browse and add a track" works without leaving the player.
|
||
|
||
**Chosen model: free add, vote-to-skip, no host for playback.** Anyone in
|
||
the party can add a track to the shared queue at any time. Anyone can
|
||
pause/resume directly (reversible, low-stakes - unlike video's host-only
|
||
`"playstate"`). Skipping the *current* track needs a strict majority vote
|
||
of currently connected members - the one action asked to be gated, since
|
||
unlike adding (purely additive) or pausing (trivially undone), skipping
|
||
throws away where the party is now. This is a genuinely different shape
|
||
of shared state than video's "one host drives, everyone follows" - not a
|
||
size difference, a different model - which is exactly why it's a separate
|
||
service (`server/src/musicQueueService.js`, `MusicQueueService`) rather
|
||
than more branches inside `partyManager.js`/`socketHandler.js`'s existing
|
||
video-shaped logic.
|
||
|
||
### Data model and server-authoritative timing
|
||
|
||
`Party` gained a `mediaType: 'video' | 'music'` field
|
||
(`PartyManager.create`'s new last argument, inferred server-side in
|
||
`POST /api/parties` from the clicked item's Emby `Type` - `'Audio'` means
|
||
music, everything else already handled means video - the client never has
|
||
to know or say which). A music party's `party.music` object
|
||
(`{queue, current, skipVotes, advanceTimer}`) is entirely separate from
|
||
the fixed pre-built `party.queue`/`queueIndex` video parties use - a music
|
||
queue is live and mutable, not something computed once at creation.
|
||
|
||
No single client is "the position" for music the way a video host's
|
||
heartbeat is. Instead, `party.music.current.startedUtc` (a wall-clock
|
||
timestamp) plus the track's known duration is the only state that exists -
|
||
`getElapsedSeconds()` computes position on demand
|
||
(`Date.now() - startedUtc`, or a frozen `pausedElapsedMs` while paused),
|
||
never ticked or stored. This is also what makes auto-advance possible
|
||
without any client "reporting" a track ended (which, with several clients'
|
||
players all finishing within milliseconds of each other and no single
|
||
host, could race): `_scheduleAdvance` sets one `setTimeout` for exactly
|
||
when the current track's duration runs out, computed from the same
|
||
`startedUtc`. A track with no known duration just has nothing scheduled -
|
||
it relies on a manual vote-skip instead, same as it would if the timer
|
||
fired.
|
||
|
||
That timer is the one genuinely new leak surface this feature adds, given
|
||
how much this project has already had to fix real memory leaks (see
|
||
"Realtime transport" above) - `_clearTimer` runs before every reschedule
|
||
and from a new `destroy(party)` hook wired into **both** of
|
||
`partyManager.js`'s existing party-destruction paths (`leave`'s last-member
|
||
case and `removeIdleParties`' sweep), so a torn-down party can never leave
|
||
a timer firing against state that no longer exists in `PartyManager`'s map.
|
||
|
||
`MusicQueueService` needs to broadcast from inside that timer's callback,
|
||
which runs with no socket/request in scope at all - solved with
|
||
`attachIo(io, onTrackChanged)`, called once at server startup
|
||
(`index.js`, right next to where `attachSocketServer` itself is wired up),
|
||
the same "give the singleton what it needs once, up front" shape
|
||
`PartyManager`'s own singleton already has. `onTrackChanged` is wired to
|
||
`broadcastLobby(io)` specifically so a party's home-page card (preview
|
||
image/name) stays current even when the *timer*, not a person clicking
|
||
anything, is what changed what's playing.
|
||
|
||
### Catalog: Artists → Albums → Tracks
|
||
|
||
Three more dedicated Emby endpoints, same reasoning as `Shows → Seasons →
|
||
Episodes`: guessing folder structure via generic recursive `/Items` calls
|
||
is fragile, a real endpoint per level isn't. `GET /Artists?ParentId=...`
|
||
for a library's artists, `GET /Users/{id}/Items?ArtistIds=...
|
||
&IncludeItemTypes=MusicAlbum` for an artist's albums, `GET
|
||
/Users/{id}/Items?ParentId=<albumId>&IncludeItemTypes=Audio` for an
|
||
album's tracks. `toItemDto` grew `durationTicks`/`indexNumber`/
|
||
`artistName`/`albumName` - additive, `undefined` for every other item type,
|
||
so nothing existing had to change shape. `WATCH_PARTY_TYPES` grew
|
||
`'Audio'` (a track can start a party) - `MusicArtist`/`MusicAlbum` stay
|
||
un-playable, drill-down only, joining `Series` in `tiles.js`'s
|
||
`DRILL_DOWN` table (generalized from a single `seriesHref` special case to
|
||
a small per-type lookup once there were three, not two, drill-down
|
||
targets - `/artist/:id` → `artist.js`, `/album/:id` → `album.js`, each a
|
||
smaller version of the library-grid/episode-list shape already used
|
||
elsewhere).
|
||
|
||
### Streaming: the same route, one hint
|
||
|
||
Audio goes through the *exact same* `/api/catalog/stream/:itemId/master.m3u8`
|
||
route as video, not a parallel endpoint - Emby's `PlaybackInfo` negotiation
|
||
isn't video-specific, so the whole `TranscodingUrl`-first,
|
||
hand-built-fallback dance (see "Player track selection" above) applies
|
||
identically. The one real difference is the hand-built fallback's path
|
||
prefix (`/Videos/` vs `/Audio/`), which the response doesn't reliably
|
||
reveal on its own and looking up would cost an extra Emby round trip for
|
||
something the client already knows - so the client tags the request
|
||
(`streamUrl(itemId, {mediaType:'audio'})`, `audioStreamUrl()` in
|
||
`embyApi.js`) and the route reads it back
|
||
(`req.query.mediaType === 'audio'`). The music player uses hls.js for the
|
||
same reason the video player does: most browsers have no native `.m3u8`
|
||
support at all, and there's no reason to assume Emby's negotiation
|
||
produces anything *other* than HLS just because the media is audio.
|
||
|
||
### Player: not theater-mode, and why adding a track doesn't navigate away
|
||
|
||
`renderPartyPlayer` (route `/party/:id`, `player.js`) is now a small
|
||
dispatcher: it fetches the party once (both the video and music renderers
|
||
need `mediaType` before they can render anything at all) and delegates to
|
||
`renderVideoPlayer` or the new `renderMusicPlayer`
|
||
(`views/musicPlayer.js`). The dispatch itself can't be `async` despite
|
||
needing to `await` that fetch - `route()`'s contract needs its cleanup
|
||
function back *synchronously* (see `router.js`), and an `async` handler
|
||
would hand back a `Promise` instead, which `router.js`'s `typeof cleanup
|
||
=== 'function'` check silently skips - reintroducing the exact leak class
|
||
this project already spent real effort fixing once. Solved with a mutable
|
||
box: `renderPartyPlayer` returns a wrapper immediately, which calls
|
||
whichever real cleanup function the fetch eventually produces - or nothing
|
||
at all, correctly, if the party navigates away before that fetch even
|
||
resolves (nothing was set up yet in that case - no socket, no timer,
|
||
nothing to leak).
|
||
|
||
Unlike the video player, the music player is **not** full-viewport/theater
|
||
-mode (no `.playerWrap`-style `position:fixed`) - there's no video image to
|
||
declutter around, so it renders as a normal page and the sidebar stays up.
|
||
That's also what makes "add tracks while listening" simple: rather than
|
||
navigating to a separate browsing page and losing the player's own state
|
||
(and having to thread "which party am I adding to" through a chain of
|
||
query params across three route levels - considered, rejected as needless
|
||
complexity for what browsing-with-context would have bought), the player
|
||
has its own small embedded Artist → Album → Track browser
|
||
(`musicPlayer.js`'s `loadMusicLibraries`/`loadArtists`/`loadAlbums`/
|
||
`loadTracks`), reusing the exact same catalog endpoints, rendered inline
|
||
in a toggleable panel. Adding a track is REST
|
||
(`POST /api/parties/:partyId/tracks`, `addTrackToQueue()` in `party.js`),
|
||
not a socket event like `voteSkip`/`musicPlaystate` - deliberately, since
|
||
it needs to work from this same panel regardless of realtime-connection
|
||
state, and `MusicQueueService.addTrack`'s own `"queueUpdated"` broadcast
|
||
(via the `attachIo` wiring above) reaches everyone actually connected
|
||
either way, so nothing about the realtime side needed a second code path.
|
||
|
||
### A real bug found while testing, not just the mock's fault
|
||
|
||
`applyMusicState` (the "queueUpdated"/initial-render handler) used to
|
||
update the queue list and vote button *after* the current-track branch,
|
||
which calls `loadTrack()` - and `loadTrack()`'s hls.js setup can throw (a
|
||
bad manifest, hls.js failing to load from the CDN, any of it). Because
|
||
JavaScript doesn't roll back a function's earlier statements when a later
|
||
one throws, but *does* abort everything after the throw in the same call,
|
||
an exception there took the queue list and vote tally down with it even
|
||
though they have nothing to do with whether one specific track's audio
|
||
loaded - and worse, on the very first render, it escaped `renderMusicPlayer`
|
||
entirely into the dispatcher's `.catch()`, which alerts and navigates back
|
||
to `/home`, discarding the whole view. Fixed two ways: reordered
|
||
`applyMusicState` so the queue/vote/icon updates - which can't fail - run
|
||
*before* the current-track branch that can (mirroring `player.js`'s
|
||
`onItemChanged`, which already updates the title/next-button before
|
||
calling `loadVideo` for exactly this reason), and wrapped `loadTrack`'s
|
||
hls.js setup in its own `try/catch` so a failure there logs and stops,
|
||
rather than propagating out of the handler at all. Found by a test whose
|
||
own mock happened to be missing `Hls.Events` - which pointed at a real
|
||
fragility in the shipped code, not just an incomplete mock, so it's fixed
|
||
in `musicPlayer.js` itself, not worked around in the test.
|
||
|
||
### Verified
|
||
|
||
Server-side, directly against the real shipped modules, not reimplemented
|
||
mocks (see session history): a scripted sequence against the real
|
||
`musicQueueService.js` alone - init auto-starts the first track, a second
|
||
add while one plays queues rather than jumping the queue, a two-vote
|
||
majority skips and clears the tally, pause/resume freezes and correctly
|
||
resumes elapsed time, a short real track auto-advances on its own via a
|
||
genuine `setTimeout` after its duration, and `destroy()` clears a pending
|
||
timer (18 checks, instrumented `setTimeout`/`clearTimeout` counting to
|
||
catch anything left dangling - same technique used for the client-side
|
||
router-cleanup leak earlier in this project). A second pass through the
|
||
real `socketHandler.js` + `partyManager.js` together (fake `io`/sockets)
|
||
confirmed `voteSkip`/`musicPlaystate` are no-ops on a video party, a
|
||
non-host member can pause and resume a music party's track directly, a
|
||
two-connected-member vote-skip needs exactly two votes, and the resulting
|
||
`"queueUpdated"` really reaches the party's socket room (7 checks).
|
||
|
||
Client-side, against the real running local server with mocked
|
||
`fetch`/`Hls`/`io` (see session history): the full library → artist →
|
||
album browsing chain renders correct tiles and drill-down links at each
|
||
level; starting a party from a track correctly creates one and navigates;
|
||
the dispatcher correctly renders the music player (not the video one) for
|
||
a `mediaType:'music'` party and still renders the video player when
|
||
`mediaType` is absent (back-compat); the now-playing header, vote tally
|
||
(computed against the live member list, matching `musicQueueService.js`'s
|
||
own majority math), and queue list all render correctly and update
|
||
correctly on a `"queueUpdated"` push including a live track change (new
|
||
audio source loaded); play/pause and vote-skip clicks emit the right
|
||
socket events; the inline browse-and-add panel correctly walks
|
||
artist → album → tracks and POSTs the exact right payload shape to
|
||
`/api/parties/:partyId/tracks`; and the crash fixed above was confirmed
|
||
fixed by re-running the same broken-`Hls.Events` scenario through the
|
||
full dispatcher afterward - no alert, no navigation away, the queue/track
|
||
info render correctly regardless.
|
||
|
||
**Not verified** - the usual limit of testing without real Emby access:
|
||
whether `/Artists`/the album-tracks query really return what's assumed
|
||
here, whether a music item's `PlaybackInfo`/`TranscodingUrl` behaves the
|
||
same way a video's does (reasoned to, per "Streaming" above, not
|
||
observed), and the real audio quality/gapless-ness of playback through a
|
||
transcode. First things to check once this gets a real retest.
|
||
|
||
## Music bar polish + catalog search
|
||
|
||
Four smaller, real-usage-driven fixes, once the music bar and the
|
||
persistent-overlay redesign had actually been used for a while:
|
||
|
||
- **Vote-skip's "X/Y needed" didn't update on a join/leave.** The
|
||
majority threshold depends on the connected-member count, which
|
||
`renderMembers` (the `"members"` push) updates - but the label itself
|
||
was only ever recomputed inside `applyMusicState` (a `"queueUpdated"`
|
||
push - an actual vote or track change). Two different events, each
|
||
only refreshing half of what the label needs. Fixed by caching the
|
||
last `MusicPublicState` (`lastMusicState`) and factoring the label math
|
||
into its own `updateVoteSkipLabel()`, called from *both* handlers now.
|
||
- **The listen party you're already in still offered "Rejoindre" on the
|
||
home page.** `Home.ts`'s card rendering had no way to know "this one's
|
||
mine" - fixed by checking each card's `partyId` against
|
||
`musicBar/MusicBar.ts`'s own `getActiveMusicPartyId()` (already tracked
|
||
client-side, no server change needed) and swapping the button for an
|
||
inert "En écoute" label on that one card. Video has no equivalent case:
|
||
its player is a full-screen takeover, so you can't be looking at this
|
||
list while inside one.
|
||
- **A progress bar for the music bar.** Read-only, deliberately - see
|
||
`MusicBar.html`'s own comment: there's no host to arbitrate a seek in
|
||
this model, and `MusicQueueService` has no "jump to position" concept
|
||
server-side at all today (a real protocol addition, not a UI-only
|
||
change - out of scope for what was asked here). The fill's width is
|
||
just `audio.currentTime / durationSeconds`, ticked by the same interval
|
||
that already updates the position text.
|
||
- **Search bars for the catalog pages** (`views/library`, `views/artist`,
|
||
`views/album`). Client-side only, filtering whatever's already
|
||
rendered/fetched rather than a server round trip per keystroke - a
|
||
library page tops out at a few hundred items (`getLibraryItems`'s own
|
||
`Limit`), comfortably inside "just filter the DOM" territory. Shared as
|
||
`tiles.ts`'s `wireSearchFilter` so the three pages behave identically.
|
||
The existing `/api/catalog/search` endpoint (`CatalogService.search`,
|
||
video types only, never wired into any UI) is a separate, unrelated
|
||
feature - a cross-library global search, not asked for here - left
|
||
as-is.
|
||
|
||
Verified against the real compiled bundle for all four: a `"members"`
|
||
push alone (no vote, no track change) updates the skip label; the home
|
||
page's own active party shows "En écoute" while every other card still
|
||
shows "Rejoindre"; the progress fill tracks a simulated `currentTime`
|
||
correctly; typing into a library's search box hides non-matching tiles
|
||
live and the "Aucun résultat" notice appears only once nothing matches.
|
||
|
||
## TypeScript and a service-oriented architecture
|
||
|
||
A full rewrite, requested directly: both `server/` and `client/` are now
|
||
TypeScript, and the server is organized as one singleton service class per
|
||
feature rather than route handlers with inline logic. This is the largest
|
||
single change in this project's history - it touches nearly every file -
|
||
so this section covers the shape of it rather than every line moved.
|
||
|
||
### Why this needed real decisions, not just "add types"
|
||
|
||
This app was "no build step" from the very first version, deliberately -
|
||
the client's plain ES modules loaded straight in the browser, the server
|
||
ran its `.js` directly. TypeScript can't compile itself away at
|
||
runtime - something has to turn `.ts` into `.js` before either project
|
||
can run - so "TypeScript" and "no build step" are mutually exclusive, not
|
||
a small addition to the existing setup. Two independent decisions followed:
|
||
|
||
- **Server: plain `tsc`.** The server doesn't need bundling (Node resolves
|
||
`require()`s itself, there's no browser payload-size concern) - just
|
||
type-stripping and down-leveling, which is exactly what `tsc` does.
|
||
`server/dist` mirrors `server/src`'s structure 1:1.
|
||
- **Client: esbuild.** The client *does* need bundling (a browser
|
||
shouldn't have to make a dozen round trips for a dozen modules, and
|
||
there's no reason to hand-manage `<script type="module">` ordering
|
||
either) - esbuild was picked over a heavier tool (Vite, webpack)
|
||
because this app's whole client is a couple thousand lines across a
|
||
dozen files; esbuild's own built-in behaviors (bundling CSS reached
|
||
from JS/TS imports into a sibling `.css` file automatically, a `text`
|
||
loader for importing an HTML template as a string) cover everything
|
||
needed with a ~30-line hand-written build script
|
||
(`client/build.mjs`), not a config file for a tool built for much
|
||
bigger apps. Both are consistent with this project's running bias
|
||
against dependencies it doesn't need.
|
||
- Both projects compile **independently** - the server's `tsc` never
|
||
sees client code and vice versa. A real shared-types package was
|
||
considered and rejected as more structural commitment than this app's
|
||
size justifies; `server/src/types.ts` and `client/src/types.ts` have
|
||
their own overlapping copies of the DTO shapes instead (Party,
|
||
ItemDto, the music types, ...) - a normal, accepted trade-off for a
|
||
two-small-projects setup like this one.
|
||
- `npm run dev` in `server/` uses `tsx` (a single extra dev dependency)
|
||
to run straight from `.ts` with no separate compile step while
|
||
iterating; `npm run build` (`tsc`) is what actually ships, and what
|
||
`server/Dockerfile`'s build stage runs. The client's `npm run watch`
|
||
(esbuild's own watch mode) plays the same role there.
|
||
|
||
### Server: services, not route handlers with logic inline
|
||
|
||
The old `index.js` had ~15 route handlers with their actual logic - Emby
|
||
queries, DTO shaping, override-forcing - written directly inside the
|
||
`app.get(...)` callback. `server/src/index.ts` is now a thin wiring layer
|
||
only: every route reads the session, calls exactly one service method,
|
||
and returns what it gets back. The services, one singleton class per
|
||
feature (same shape `PartyService`/`MusicQueueService` already had before
|
||
this rewrite - this generalizes that pattern, it doesn't invent it):
|
||
|
||
- **`AuthService`** (from `embyAuth.js`) - login + the in-memory session store.
|
||
- **`CatalogService`** (new - previously inline in `index.js`) - every
|
||
Emby browsing/streaming call and the DTO shaping (`toItemDto`,
|
||
`applyPlaybackOverrides`) that used to be bare functions at the bottom
|
||
of the file. The single largest extraction.
|
||
- **`PartyService`** (renamed from `PartyManager`) - unchanged behavior,
|
||
now fully typed (see "A real type bug caught during the port" below).
|
||
- **`MusicQueueService`** - unchanged behavior, ported as-is (it was
|
||
already the newest, already-service-shaped code).
|
||
- **`SocketGateway`** (from `socketHandler.js`) - the realtime protocol.
|
||
Not stateful itself (no singleton *state*, unlike the four above) but
|
||
kept as a class for the same reason `Logger`/`EmbyClient` are: shape
|
||
consistency across `server/src`, not because every one of them
|
||
strictly needs to be a class.
|
||
- **`EmbyClient`** (from `embyProxy.js`) - low-level Emby HTTP +
|
||
response proxying. Deliberately *not* called a "service": it has no
|
||
state and isn't a feature on its own, just the shared plumbing
|
||
`CatalogService`'s routes are built on.
|
||
|
||
**Error handling got a real pass while this code was being touched
|
||
anyway.** Express 4 doesn't catch a rejected Promise from an `async`
|
||
route handler on its own - an unhandled rejection there would either
|
||
crash the process or hang the request, depending on Node version. Every
|
||
route is now wrapped in a small `asyncRoute()` helper
|
||
(`index.ts`) that forwards a thrown/rejected error to a single
|
||
error-handling middleware at the bottom of the file. `CatalogService`
|
||
methods throw a `CatalogFetchError` (carrying its own HTTP status -
|
||
e.g. `getRawItem` preserves Emby's real 404 instead of a blanket 502,
|
||
which the old code did per-route by hand) rather than touching an Express
|
||
response directly, so the same service method works whether it's called
|
||
from a route that has a `res` handy or (in principle) from anywhere else.
|
||
This wasn't a change asked for - it fell out of centralizing the routes
|
||
into services, and it's a real robustness improvement over the old
|
||
per-route `if (!embyRes.ok) { res.status(...).json(...); return; }`
|
||
repeated fifteen times with no handling at all for a request that
|
||
outright throws.
|
||
|
||
### Client: one folder per view, colocating .ts/.html/.css
|
||
|
||
Each view is now a small folder (`src/views/<name>/<Name>.ts` +
|
||
`.html` + `.css`) instead of one `.js` file with everything - including
|
||
the markup, as a big template literal - inline. The static shell of a
|
||
view's markup lives in its `.html` (imported as a raw string via
|
||
esbuild's `text` loader - see `global.d.ts`'s `declare module '*.html'`);
|
||
dynamic, per-item content (a tile grid, an episode list, a music queue)
|
||
still gets built as a template-literal-returning function inside the
|
||
`.ts`, same as before - only the outer, unchanging shell moved out.
|
||
Genuinely shared CSS (the tile grid, the presence-avatar circle, the
|
||
series/album detail header pattern, track-list rows) lives in small files
|
||
under `src/styles/`, imported by every `.ts` that needs it; `src/styles/base.css`
|
||
holds the truly global primitives (the theme's CSS variables, `button`/
|
||
`input`/`.linklike`, `.pageHeader`) and is imported exactly once, by
|
||
`app.ts`.
|
||
|
||
This is deliberately *not* pushed to the point of zero duplication or
|
||
class renaming across views that happen to share a visual pattern (the
|
||
series and album detail headers, for instance, keep their own
|
||
`seriesHeader`/`albumHeader` + cover-image classes, sharing only the
|
||
inner `seriesInfo`/`seriesYear`/`seriesOverview` block via
|
||
`styles/detail.css`) - renaming shared classes purely for tidiness would
|
||
have meant touching working, already-tested markup and CSS in lockstep
|
||
across files with no functional benefit, for no reason better than
|
||
aesthetics, in a change already large enough to review.
|
||
|
||
`Player.ts` keeps the same internal shape it had before this rewrite: one
|
||
file holding both the `/party/:id` route's dispatcher (`renderPartyPlayer`,
|
||
exported, registered in `app.ts`) and the video-specific renderer, which
|
||
imports `renderMusicPlayer` from the sibling `musicPlayer` folder - the
|
||
dispatcher pattern (fetch once, pick a renderer, hand back a synchronous
|
||
cleanup wrapper - see "Player: not theater-mode..." above) predates this
|
||
rewrite and didn't need to change shape, just types.
|
||
|
||
### A real type bug caught during the port
|
||
|
||
Porting `SocketGateway`'s `"members"` broadcast, it was tempting to reuse
|
||
the same `PublicMember` interface `PartyService.toPublicState()`'s REST
|
||
response uses. They're not the same shape: the REST response's
|
||
`members[]` includes `isPaused`/`positionTicks` per member; the socket
|
||
broadcast never did (a follower's player only ever reads *`"playstate"`*
|
||
events for position, never the member list, for that - see
|
||
`SocketGateway`'s original JS, which built a visibly smaller object
|
||
inline). Reusing the wrong interface would have type-checked fine (a
|
||
smaller object being assigned into a type asking for more fields is
|
||
exactly the class of mistake a structural type system like TypeScript's
|
||
*does* catch, but only if the two shapes are actually kept distinct) -
|
||
caught by comparing the ported code against the original JS line by line
|
||
rather than trusting the port "looked right", and fixed by giving the
|
||
socket broadcast its own `SocketMemberDto` type. Worth noting because
|
||
it's a fairly direct demonstration of what "clean TypeScript" is actually
|
||
for here: not decoration, a mistake this port itself introduced and then
|
||
caught before it shipped.
|
||
|
||
### A real Express+TypeScript friction point, resolved once
|
||
|
||
`tsconfig.json` for both projects enables `noUncheckedIndexedAccess` - a
|
||
stricter-than-`strict` flag that treats every indexed access
|
||
(`arr[i]`, and critically, Express's `req.params.foo`, which is typed as
|
||
an index signature) as possibly-`undefined`, to actually catch
|
||
out-of-bounds access. This is genuinely useful (`PartyService.advanceQueue`'s
|
||
`party.queue[party.queueIndex]` is exactly the kind of access this
|
||
catches if the index math is ever wrong) but makes every single
|
||
`req.params.xxx` read in `index.ts` a type error, since Express's own
|
||
types don't know a given route only ever matches with that param
|
||
present. Resolved once with a `param(req, name)` helper in `index.ts`
|
||
(asserts the value as `string` - a matched Express route with a required,
|
||
non-optional `:name` segment always has it, which none of these routes'
|
||
patterns violate) rather than either scattering non-null assertions at
|
||
every call site or writing out a params-generic interface per route for
|
||
15 routes.
|
||
|
||
### Verified
|
||
|
||
Both projects type-check cleanly in strict mode
|
||
(`npm run typecheck`/`tsc --noEmit`, no errors) - not just "compiles",
|
||
which `esbuild` would do even with type errors present, since it
|
||
transpiles without checking; the client's own separate `tsc --noEmit`
|
||
step is what actually guards against that gap.
|
||
|
||
Server: every unit/integration check written earlier for the JS version
|
||
(party lifecycle logging, the music queue's timer/vote/pause behavior
|
||
including real `setTimeout`-based auto-advance, the socket-level
|
||
integration through `SocketGateway`+`PartyService`+`MusicQueueService`
|
||
together, the library-type filter, the playback-override-forcing logic)
|
||
was first re-run ad hoc against the **compiled** `server/dist` output to
|
||
confirm the port itself changed nothing behaviorally, then formalized
|
||
into a real, committed suite: `server/test/*.test.ts` (one file per
|
||
service - `musicQueueService`, `partyService`, `socketGateway`,
|
||
`catalogService`), using Node's built-in `node:test` + `node:assert/strict`
|
||
(zero new dependencies) and run straight against the TypeScript source via
|
||
`tsx --test` (`npm test`) - no compile step, no separate test-runner
|
||
dependency. A dedicated `tsconfig.test.json` (extends the base config,
|
||
overrides `rootDir` to `.` and adds `test/**/*.ts` to `include`) lets
|
||
`npm run typecheck` cover the test files too without pulling them into
|
||
`dist/` when `npm run build` (plain `tsc`, still scoped to `src/`) runs.
|
||
The compiled server was also actually started (`node dist/index.js`) and
|
||
confirmed listening cleanly, both directly and via `tsx` for the dev path.
|
||
|
||
### Continuous integration
|
||
|
||
`.forgejo/workflows/publish-docker.yml` gained a `test` job that runs
|
||
before `publish` (`needs: test`): server `npm run typecheck` + `npm test`,
|
||
then client `npm run typecheck` + `npm run build`. It runs in a
|
||
`node:20-alpine` container - a separate container from `publish`'s
|
||
`docker:27-cli`, since that one has no Node at all (that's also why both
|
||
jobs clone sources by hand with `git init`/`fetch`/`checkout` rather than
|
||
`actions/checkout`, which is a JS action and needs Node to run). The two
|
||
jobs don't share a filesystem, so each clones independently; `publish`
|
||
only starts once `test` succeeds.
|
||
|
||
**A real failure on the very first two runs**, caught by actually watching
|
||
the pipeline rather than assuming green: `"test": "tsx --test
|
||
test/**/*.test.ts"` worked perfectly locally and failed on the runner with
|
||
`Could not find '.../test/**/*.test.ts'`, as if the literal glob string
|
||
had been handed to Node as a filename. It had - `node --test` gained
|
||
native glob-pattern CLI arguments only around Node 21/22, and this dev
|
||
machine's global `node` is v22, while the CI job (deliberately pinned to
|
||
`node:20-alpine`, matching `server/Dockerfile`'s runtime image) is v20 -
|
||
same script, two Node majors, one of which silently doesn't glob-expand
|
||
the argument at all and just looks for a file with `*` characters in its
|
||
literal name. Fixed by listing the four test files explicitly in the
|
||
script instead of trusting a glob to be interpreted the same way on every
|
||
Node version this project targets (`engines.node` is `>=18`) - a
|
||
one-time list-maintenance cost in exchange for not depending on a CLI
|
||
feature whose availability turned out to vary across the exact two Node
|
||
installs this project actually runs on.
|
||
|
||
Client: since esbuild bundles everything into one `app.js` (no more
|
||
individually-importable view modules the way separate `<script
|
||
type="module">` files were before), testing moved from "import one view
|
||
and call it directly" to driving the **real, compiled bundle** through
|
||
its actual boot → login → router → view chain, with `fetch`/`window.io`/
|
||
`window.Hls` mocked and real clicks/hash-navigation - arguably a more
|
||
realistic test than the old per-module approach, not just an adaptation
|
||
forced by the bundler. Confirmed this way: login, the sidebar, home's
|
||
carousels, the movies library, a video party (presence, host controls,
|
||
leave), the full music hierarchy (library → artist → album), a music
|
||
party (presence, vote-skip, pause toggle, the inline browse-and-add
|
||
panel posting the right payload), and the series page - all through
|
||
`client/dist`'s actual output, no thrown errors anywhere in the sequence.
|
||
|
||
**Not verified**: an actual `docker build` of the new multi-stage
|
||
Dockerfile - this dev environment has no Docker installed at all. The
|
||
Dockerfile's build stage runs the exact same `npm run build` commands
|
||
already confirmed working locally for both projects, just inside
|
||
`node:20-alpine` instead of this machine, so the main remaining risk is
|
||
something environment-specific (a native dependency, a Node version
|
||
mismatch) rather than the build commands themselves being wrong - still,
|
||
a real build is the first thing to try after this lands, before assuming
|
||
the Dockerfile is right.
|
||
|
||
## A `.hide` specificity bug: one CSS rule silently beating another
|
||
|
||
The player's settings gear (audio/subtitle/quality) stayed permanently
|
||
open regardless of the `hide` class ever being toggled on it. Root cause:
|
||
`.hide { display: none; }` (`styles/base.css`) and `.settingsPanel {
|
||
display: flex; ... }` (`views/player/Player.css`) are both single-class
|
||
selectors - **equal specificity** - so when an element carries both
|
||
classes, the cascade falls back to source order for whichever property
|
||
they both set, and `esbuild`'s CSS-from-JS bundling had `Player.css`
|
||
(imported inside a view module) land after `base.css` (imported once,
|
||
globally, at the top of `app.ts`) in the final `app.css`. The *later* rule
|
||
wins a specificity tie, so `.settingsPanel`'s own `display: flex` always
|
||
overrode `.hide`'s `display: none` - not a bug in the toggle logic at all,
|
||
the class was being added and removed correctly the whole time, it just
|
||
had no effect on this one element.
|
||
|
||
Fixed at the source of the utility class, not the one call site:
|
||
`.hide { display: none !important; }`. `!important` beats a same-
|
||
specificity non-`!important` rule regardless of bundling/source order, so
|
||
this closes off the entire bug *class* - any future component CSS that
|
||
sets `display` for its own visible state (several already do - tiles,
|
||
the shell, every carousel) is safe to combine with `.hide` without
|
||
silently losing to bundling order the way `.settingsPanel` did. Verified
|
||
by rebuilding and driving the real compiled bundle (mocked
|
||
`fetch`/`window.io`/`window.Hls`, same methodology as the TypeScript
|
||
rewrite above): before the fix, `getComputedStyle(settingsPanel).display`
|
||
read `"flex"` even with the `hide` class present; after, it correctly
|
||
reads `"none"` until the gear button is clicked and `"none"` again once
|
||
dismissed.
|
||
|
||
## Music bar: an overlay you keep browsing under, not a page
|
||
|
||
The music player was originally its own route (`/party/:id`, the
|
||
now-deleted `views/musicPlayer/`) - deliberately not theater-mode, but
|
||
still a full page swapped into `#mainContent` like any other view (see
|
||
"Music: participative playlist" above). That was fine for playback and
|
||
for a small embedded "browse and add" panel, but it meant navigating
|
||
anywhere else - the sidebar, an album to queue up more tracks, home -
|
||
tore the whole thing down: `router.ts` calls a route's cleanup and
|
||
replaces `#mainContent` on every hash change, and this view's cleanup
|
||
called `leaveParty`. There was no way to be "in" a listen party and also
|
||
browsing the rest of the app at the same time.
|
||
|
||
**Fix: move the player out of the router entirely.** `musicBar/MusicBar.ts`
|
||
is a Shell-level singleton - mounted once by `Shell.ts` into a `#musicBar`
|
||
element that's a sibling of `#mainContent`, never touched by route
|
||
changes - rendered as a fixed bottom bar (`position: fixed; bottom: 0`,
|
||
below the video player's `.playerWrap` z-index so a video takeover still
|
||
correctly covers it, though that's now moot in practice - see below).
|
||
Because it's a singleton rather than something re-rendered per visit, its
|
||
lifecycle isn't the render()-returns-a-CleanupFn shape every route uses -
|
||
it never registers as a route at all. Instead it exposes three plain
|
||
functions: `initMusicBar(root)` (wire the static DOM once, at boot),
|
||
`joinMusicParty(partyId, party)` and `leaveMusicParty()` (attach/detach a
|
||
live party on top of that already-wired DOM, called from wherever a party
|
||
is actually joined/left instead of from a route's own init/cleanup).
|
||
|
||
`views/player/Player.ts`'s dispatcher (`renderPartyPlayer`) is the one
|
||
place every "start/join a party" call site already funnels through
|
||
(home's live list, a tile's "Lancer une watch party", an album's "Écouter
|
||
ensemble" all just `navigate('/party/:id')` and let this route sort out
|
||
what kind it is), which made it the natural single choke point for two
|
||
new rules instead of touching every call site individually:
|
||
|
||
- **Music**: don't render a page at all - call `joinMusicParty` and
|
||
`navigate('/home')` right back (a no-op bounce when the user was
|
||
already on home, which is true for every reachable call site except
|
||
Album's - see below). No cleanup function is ever set for this branch;
|
||
`musicBar/MusicBar.ts` manages its own lifetime from here on.
|
||
- **Video**: call `leaveMusicParty()` unconditionally (a harmless no-op
|
||
if nothing's active) before `renderVideoPlayer`. This is the "playing
|
||
something else exits the listen party" rule, asked for directly - one
|
||
call, covers every entry point that can lead to a video party, since
|
||
they all pass through this same dispatcher first.
|
||
|
||
`views/album/Album.ts` is the one place that needed its own change beyond
|
||
the dispatcher: it's the only per-track "start a party" UI left now that
|
||
the music player's own inline browse panel is gone (browsing happens by
|
||
just navigating there normally, which is the whole point). Each track's
|
||
button does double duty - start a new party, or add to whichever one is
|
||
already active - decided by `getActiveMusicPartyId()` **at click time**,
|
||
not just when the row was rendered. That distinction matters: the button
|
||
*label* is only ever as fresh as the last render (harmless staleness - it
|
||
might still say "Écouter ensemble" for a few seconds after a party
|
||
started from elsewhere), but the actual *behavior* on click is always
|
||
correct because it re-checks live state at the moment it matters, not the
|
||
snapshot from render time. After a successful click, every button in the
|
||
list is relabeled in place from current state (starting a party from
|
||
track 3 means tracks 1, 2 and 4 can now be queued too) - a plain function
|
||
call from the click handler, not a subscription to musicBar's state, which
|
||
was deliberately not built: it would need the same mutable-box
|
||
synchronous-cleanup pattern `Player.ts` uses (Album's render is `async`,
|
||
and a subscription would need unsubscribing via a route cleanup to avoid
|
||
leaking a closure over detached DOM nodes) for a live-update that only
|
||
matters if someone else's action changes the bar's state while this exact
|
||
page happens to be open - a real but narrow case, and the click-time
|
||
re-check already makes the *only* thing that has to be correct safe
|
||
either way.
|
||
|
||
Also starting a party from Album no longer navigates to `/party/:id` at
|
||
all - it calls `joinMusicParty` directly and stays put, so queuing up
|
||
several tracks from the same album is just clicking their buttons too,
|
||
with no route round-trip in between.
|
||
|
||
Verified the same way as the rest of this app's client-side testing: the
|
||
real compiled bundle, `fetch`/`window.io`/`window.Hls` mocked, driven
|
||
through actual clicks and hash navigation. Confirmed: starting a party
|
||
from an album track leaves the sidebar and page in place and correctly
|
||
relabels the other tracks; adding a track posts to the already-active
|
||
party's id, not a new one; navigating into a video party hides the bar,
|
||
disconnects its socket, and posts a real `leave` for the music party
|
||
before the video player ever renders; a party's host still sees the
|
||
settings gear open/close correctly inside that same run (see the `.hide`
|
||
fix above).
|
||
|
||
## Device detection: telling a second tab apart from a second device
|
||
|
||
Requested directly, alongside the crash below (same underlying gap - see
|
||
its own comment on the connection between the two): "same account, same
|
||
device already in the party" should never offer "Rejoindre" friction at
|
||
all, and "same account, a genuinely *different* device" should ask before
|
||
disconnecting it, not silently steal the slot. `party.members` is
|
||
`Map<userId, PartyMember>` - one entry per Emby account, no concept of
|
||
*which physical device* holds it, so before this there was no way to even
|
||
ask the question "is this the same device or not."
|
||
|
||
- **Identity**: `client/src/deviceId.ts` - a UUID (`crypto.randomUUID()`)
|
||
generated once and kept in `localStorage`, not `sessionStorage`. That
|
||
choice is what makes "device" mean "browser," not "tab": two tabs of the
|
||
same browser share one `localStorage`, so they share one deviceId and
|
||
never conflict with each other, while a different browser or a
|
||
different computer always gets a fresh one - a real conflict. A
|
||
save/read failure (private browsing, storage disabled) falls back to a
|
||
per-page-load id, kept only in memory - correct behavior degrades to
|
||
"every reload looks like a new device," not a crash.
|
||
- **Protocol**: `"join"` now carries `{partyId, deviceId, force?}` instead
|
||
of just `{partyId}` (`ClientToServerEvents`/`api/party.ts`), and
|
||
`PartyMember` gained `deviceId: string | null` (null until a real socket
|
||
"join" ever succeeds - a member the REST join/create routes just
|
||
created has no live connection yet, so nothing to compare against). The
|
||
`"join"` handler in `SocketGateway` now reasons about three cases, not
|
||
one:
|
||
- No existing connection, or the same `deviceId` reconnecting (a reload,
|
||
a second tab, a dropped connection retrying) → attach silently, and if
|
||
an old connection for that same device is still technically open,
|
||
boot it first - it's the same physical device asking again, nothing to
|
||
confirm.
|
||
- A *different* `deviceId`, `force` not set → emit `"deviceConflict"` to
|
||
the *new* socket only (never broadcast) and stop there. The socket
|
||
stays connected but inert - not joined to the party room, the member's
|
||
`connection` untouched - until the client either re-sends `"join"`
|
||
with `force:true` or gives up.
|
||
- A different `deviceId` with `force:true` (the client re-sending after
|
||
the user confirms) → boot the old connection
|
||
(`errorMessage` + `disconnect(true)`) and take the slot, exactly like
|
||
the reconnect case above.
|
||
- **Client UX**: the answer to "what should happen" was asked directly -
|
||
a plain `confirm()` in both `Player.ts` and `MusicBar.ts`'s
|
||
`onDeviceConflict` callback ("...continuer ici va déconnecter l'autre
|
||
appareil"), calling the new `PartyConnection.confirmDeviceTakeover()`
|
||
(re-emits `"join"` with `force:true`) on yes, or backing out (`navigate
|
||
('/home')` for video, `leaveMusicParty()` for music - the bar has no
|
||
page to just walk away from) on no. Kept as a native blocking dialog
|
||
deliberately: this only ever fires once, right after opening a party,
|
||
before there's any local playback state a modal could awkwardly
|
||
interrupt.
|
||
- **A second, independent bug closed by the same pass**: `PartyService
|
||
.join()` used to unconditionally build a fresh member on *every* call,
|
||
even for a userId already present - so the REST "Rejoindre" route (which
|
||
calls this every time regardless of whether the caller is already a
|
||
member) silently reset `connection`/`isPaused`/`deviceId` on a re-click,
|
||
a page reload, or a retried request. `join()` is now idempotent: an
|
||
existing member is returned as-is. This isn't just tidiness - it's the
|
||
other half of the crash below's root cause (two connections silently
|
||
sharing one map slot), fixed independently of the guard that stops the
|
||
crash itself.
|
||
- **Generalizing the crash's own fix**: `"leave"`/`"disconnect"` used to
|
||
act on `party` (the connection's own closure variable) alone. With a
|
||
device takeover now a real possibility, the *superseded* connection's
|
||
own eventual `"disconnect"` (Socket.IO fires this automatically right
|
||
after the server calls `.disconnect(true)` on it) would otherwise still
|
||
be read as a real leave - ending the *new* connection's membership,
|
||
wrong host handoff, a party destroyed under a still-present member. Both
|
||
handlers now also check `party.members.get(userId)?.connection ===
|
||
socket` before acting - only the connection actually holding the slot
|
||
can end it.
|
||
- **What this deliberately does *not* do**: change what the home page's
|
||
live party list shows *before* you click "Rejoindre" - it still can't
|
||
tell "you're already in this one, same device" from "you're already in
|
||
this one, a different device" from "you're not in it at all," because
|
||
`LobbyPartyDto`'s one shared snapshot (`SocketGateway.lobbySnapshot`,
|
||
broadcast identically to the whole `"lobby"` room) carries no per-viewer
|
||
membership at all - giving it that would mean computing and sending a
|
||
personalized snapshot per subscriber instead of one shared broadcast, a
|
||
real protocol change of its own, not asked for here. In practice this
|
||
doesn't cost the feature much: for **video**, the only way to already be
|
||
"in" a party while looking at the home page at all is a second
|
||
tab/device (the player is a full-screen takeover on the one you're
|
||
using - see "Music bar polish" above for why music's simpler, same-tab
|
||
case could just be hidden client-side and video's can't), so the
|
||
conflict check above already *is* the meaningful moment - clicking
|
||
"Rejoindre" reaches it immediately, same-device reconnects silently,
|
||
a different device asks first. The outcome asked for (no friction for
|
||
your own device, a confirmation before disconnecting another) is there;
|
||
it just resolves one click later than a home-page button label would,
|
||
rather than needing a second, heavier protocol change to preempt it.
|
||
|
||
Verified with new `SocketGateway`/`PartyService` tests exercising the
|
||
real conflict/takeover/reconnect paths (a different device gets
|
||
`deviceConflict` and the existing connection is left untouched; `force:
|
||
true` boots the old connection and the new one takes the slot, and its
|
||
superseded predecessor's own later `"disconnect"` no longer destroys the
|
||
party; the same `deviceId` reconnecting never conflicts with itself), plus
|
||
a dedicated `PartyService.join()` idempotency test, and confirmed
|
||
`deviceId.ts`'s `localStorage` persistence survives a real page reload
|
||
against the compiled bundle in a real browser.
|
||
|
||
## Self-hosted ffmpeg: burning in PGS (image) subtitles ourselves
|
||
|
||
Asked for directly, right after the client-side `<track>` redesign
|
||
("Player track selection" above) closed every *text* subtitle case:
|
||
PGS/VobSub/DVB subtitles (Blu-ray/DVD rips) are bitmaps, not text -
|
||
there's nothing for Emby's `Stream.vtt` conversion to extract, so a
|
||
native `<track>` fundamentally can't render one. `getPlaybackOptions`
|
||
already flagged these `isTextBased: false`; until now the picker just
|
||
left them disabled.
|
||
|
||
**Why running our own ffmpeg can plausibly succeed where Emby's kept
|
||
failing** (see "Player track selection" for the whole saga): Emby's own
|
||
burn-in goes through libass (the `subtitles` filter), which is text-only
|
||
and needs fontconfig/real fonts - the exact thing that was missing, and
|
||
even once fixed, apparently still not enough (a different,
|
||
never-fully-diagnosed "Error starting ffmpeg" on an otherwise-unremarkable
|
||
file). A PGS stream decodes like a second video track; ffmpeg's
|
||
`overlay` filter (`[0:v][0:s:N]overlay`) composites it directly - no
|
||
text rendering, no libass, no fontconfig anywhere in this path. The
|
||
Dockerfile installs plain `ffmpeg` (`apk add ffmpeg`) - no font packages
|
||
at all, unlike what Emby's own text burn-in required.
|
||
|
||
**One shared stream per party, not per viewer** -
|
||
`server/src/services/TranscodeService.ts` keys its session map by
|
||
`partyId` alone. This mirrors "host drives, everyone follows" (see
|
||
"Realtime transport" above) rather than inventing a second model:
|
||
picking a burn-in subtitle is a host-only, party-*wide* choice
|
||
(`SocketGateway`'s `setBurnInSubtitle` handler, broadcasting
|
||
`burnInChanged` - byte-for-byte the same request/broadcast/react shape
|
||
`next`/`itemChanged` already use) - at the time this was written, a
|
||
*text* subtitle pick was still each viewer's own local choice; it later
|
||
joined this same host-only/party-wide model too, once people actually
|
||
used this together and noticed the mismatch - see "Text subtitles join
|
||
burn-in as a party-wide, host-only pick" further below. `Party` gained
|
||
`burnInSubtitleIndex`/
|
||
`burnInAudioIndex` (and `PartyPublicState` exposes them too) so a
|
||
follower joining mid-party, or a page reload, picks up the party's
|
||
current choice from a plain `getParty()` - same reason `itemId`/
|
||
`queueIndex` already work that way.
|
||
|
||
**Source**: ffmpeg is pointed straight at Emby's own *direct* (static,
|
||
untranscoded) file endpoint - `/Videos/{itemId}/stream.{container}
|
||
?Static=true&MediaSourceId={id}` - authenticated the same way
|
||
`EmbyClient.fetch` already does (`X-Emby-Token` via `-headers`, never
|
||
`api_key` in the URL). This is the one piece flagged **TODO(verify)**:
|
||
there's no Emby server reachable from the environment this was built in,
|
||
so the exact shape is unconfirmed against this app's own real server -
|
||
see `TranscodeService.ts`'s `buildSourceUrl`. If it's wrong, it's
|
||
isolated to that one function.
|
||
|
||
**Stream indices**: Emby's `MediaStreams[].Index` is one flat, absolute
|
||
number across video+audio+subtitle together (the "stream 6/7/8" this
|
||
app's own picker already surfaces) - ffmpeg has no notion of that at
|
||
all, only type-relative specifiers (`0:s:N`/`0:a:N`, counting only among
|
||
streams of that type). `absoluteToRelativeIndex` does the conversion -
|
||
pure array math, fully unit-tested with no ffmpeg or Emby involved (see
|
||
`transcodeService.test.ts`).
|
||
|
||
**Output**: real, progressively-written HLS (`-f hls`, a growing
|
||
`master.m3u8` + segments) to a temp directory, served by two new routes
|
||
(`GET /api/catalog/burn/:partyId/master.m3u8` and `.../:segment`) that
|
||
poll briefly for the next file to exist rather than failing immediately -
|
||
ffmpeg is still writing while a viewer's hls.js is asking for it.
|
||
Deliberately not "transcode the whole file first, then serve it": for a
|
||
two-hour movie that would mean a wait proportional to its length before
|
||
anyone could press play at all, which defeats the point of a *watch
|
||
party*. This also means the client needs no new player logic at all -
|
||
same hls.js, same manifest/segment polling, just pointed at a different
|
||
server path (`api/embyApi.ts`'s `burnInStreamUrl`) - see
|
||
`views/player/Player.ts`'s `loadVideo`.
|
||
|
||
**Lifecycle**: a session tracks `{process, outputDir, lastAccessUtc}`;
|
||
every request refreshes it. `PartyService.leave`'s destroy branch and
|
||
`.advanceQueue` (a new item's stream indices mean nothing on the old
|
||
one) both end a party's session immediately; `removeIdleParties`'s own
|
||
idle sweep and a shorter, dedicated `TranscodeService.reapIdleSessions`
|
||
interval in `index.ts` (30min idle, checked every 5min - shorter than
|
||
parties themselves, since this holds a real OS process + disk, not just
|
||
an in-memory object) are the backstop for anything that doesn't end
|
||
cleanly. ffmpeg's stderr is captured into a rolling buffer and logged
|
||
through a new `Logger.transcode()` (same reasoning as `Logger.emby` -
|
||
see "Server-side logging" above) so a failure is visible in `docker
|
||
logs` immediately instead of a stuck spinner in the browser.
|
||
|
||
**Player UI**: the subtitle `<select>`'s image-track options go from
|
||
unconditionally disabled to *host-only* selectable (`Player.ts`'s
|
||
`renderSubtitleOptions`, gated on the same `isHost` that already gates
|
||
`nextBtn`/the settings menu), labeled to say picking one restarts the
|
||
stream for everyone. While a burn-in stream is active, the audio/quality
|
||
selects are disabled (the burn-in is one fixed encode) and a follower's
|
||
subtitle select is disabled too, since their own text-subtitle/"Aucun"
|
||
choice would no longer do anything - only the host, who can also change
|
||
or clear it, keeps it interactive. Every member (host included) applies
|
||
a burn-in change purely by reacting to the resulting `burnInChanged`
|
||
broadcast (`onBurnInChanged`, reloading `loadVideo`) - never
|
||
optimistically, same discipline `applyRemoteState`/`onItemChanged`
|
||
already use for every other party-wide state change.
|
||
|
||
(A follower's subtitle select being disabled *only* while burn-in is
|
||
active, as just described, didn't last - see "Text subtitles join
|
||
burn-in as a party-wide, host-only pick" below, where the whole select
|
||
became host-gated unconditionally instead.)
|
||
|
||
Verified locally to the extent this dev environment allows (no Emby
|
||
server, no ffmpeg binary reachable here either): full typecheck/test/
|
||
build on both projects; `absoluteToRelativeIndex`, the exact ffmpeg
|
||
argument list, per-party session start/reuse/replace, and the idle
|
||
reaper are all unit-tested with `child_process.spawn` mocked (see
|
||
`TranscodeService.setSpawnForTesting` - a deliberate, explicit test seam
|
||
rather than relying on module-mock timing); `SocketGateway` tests cover
|
||
`setBurnInSubtitle`'s host-only gating and its `burnInChanged` broadcast.
|
||
The one thing that can't be proven here - an actual PGS file burning in
|
||
and playing against the real server - waits for a real try, the same
|
||
way the whole subtitle saga above was ultimately diagnosed from the
|
||
user's own pasted container logs, not local testing.
|
||
|
||
## A real server crash: one userId, two connections, one map slot
|
||
|
||
Reported directly from the running container's own crash output, not
|
||
found by testing: `TypeError: Cannot read properties of undefined
|
||
(reading 'isPaused')` inside `SocketGateway`'s `"playstate"` handler,
|
||
which took the entire Node process down - every active party, not just
|
||
one connection. At the time this was found, `party.members` was a
|
||
`Map<userId, PartyMember>` with no per-device identity at all - the same
|
||
gap the "Device detection" section right above eventually closed, once
|
||
that was actually asked for; this section describes the state of things
|
||
before that, and the immediate fix that had to stand on its own regardless
|
||
of whether a fuller fix ever followed. Two connections for the same
|
||
userId - two tabs, a second device, a `POST /api/parties/:id/join`
|
||
retried - end up sharing that one map slot: `PartyService.join`/`.create`
|
||
both just `members.set(userId, newMember)`, unconditionally overwriting
|
||
whichever member was there.
|
||
|
||
The actual crash needed one more step, and it's a real gap in
|
||
`PartyService.leave`, not just the sharing itself: when the connection
|
||
*currently occupying the slot* is the party's last member and leaves,
|
||
`leave()` deletes it, finds no successor to hand the host role to, and -
|
||
by design, per its own comment ("this stale value is never read again",
|
||
since the party is destroyed in the same call) - never updates
|
||
`party.hostUserId`. That assumption held as long as nothing else could
|
||
still reach the now-destroyed `Party` object. Something could: the
|
||
*other* connection for that same userId, still fully connected, whose own
|
||
closure captured the exact same `Party` object reference back when it
|
||
first joined. Its regular 3-second heartbeat (`"playstate"`) fires next,
|
||
checks `userId !== party.hostUserId` - both sides of that comparison are
|
||
the same stale userId, so it passes - and then reads
|
||
`party.members.get(userId)!` on a map that no longer has that entry at
|
||
all. `undefined!.isPaused` is where it actually threw.
|
||
|
||
Fixed at two levels, not just the one line:
|
||
|
||
- The immediate cause: `party.members.get(userId)!` in the `"playstate"`
|
||
handler is now a real `if (!member) return` guard, matching the
|
||
pattern every *other* handler already used for the same lookup
|
||
(`voteSkip`/`musicPlaystate`/`requestPause`) - this one was just missed.
|
||
- The systemic gap: nothing in `SocketGateway` had ever needed the
|
||
equivalent of `index.ts`'s `asyncRoute()` before, because nothing else
|
||
here had thrown - but Socket.IO has no built-in equivalent of Express's
|
||
error-handling middleware at all. A synchronous throw from *any*
|
||
listener propagates straight out of `Socket.emit()` and crashes the
|
||
whole process, unconditionally - not just this bug, any future one,
|
||
in any handler. Every `socket.on(...)` registration in `attach()` now
|
||
goes through a new `safe()` wrapper (log-and-swallow on an unexpected
|
||
exception) - the same "don't let one bad case take down every active
|
||
party" instinct `asyncRoute()` already applied to REST routes, just
|
||
never extended to the realtime side until this forced the question.
|
||
|
||
Verified with a real regression test reproducing the exact sequence
|
||
(two joins for one userId, the occupying one leaving as the party's last
|
||
member, the other's next heartbeat) - confirmed it doesn't throw, and
|
||
specifically confirmed it's the *guard* doing that (a clean early return)
|
||
rather than the generic `safe()` net merely catching and hiding an actual
|
||
throw, by temporarily reverting just the guard and watching that same
|
||
test correctly fail before putting it back.
|
||
|
||
Deliberately not "fixed" by changing how `join`/`leave` share that map
|
||
slot in this same pass - that real design work (identifying a device,
|
||
deciding what "same account, different device" should actually do
|
||
instead of silently overwriting) landed separately, right above, once
|
||
it had actually been asked for. This pass alone was enough to stop it
|
||
from ever being able to crash the server again in the meantime,
|
||
regardless of how - or whether - that got resolved.
|
||
|
||
## Playlist browsing + launch
|
||
|
||
Asked for directly, queued right after the ffmpeg burn-in work above:
|
||
see what's inside an Emby playlist, and launch it as a party pre-queued
|
||
with the whole thing - the same "start from one click, the rest queues
|
||
itself" shape starting from an episode already gets for its series.
|
||
|
||
**The sidebar needed nothing new**: Emby's `/Views` already returns a
|
||
`CollectionType: 'playlists'` entry once the user has at least one -
|
||
`CatalogService.VISIBLE_COLLECTION_TYPES` just had to stop dropping it,
|
||
and it shows up in `Shell.ts`'s sidebar through the exact same code path
|
||
every other library already does (see "UI: Emby-like shell" above).
|
||
`Library.ts`'s existing generic branch (`getLibraryItems` - direct
|
||
children of a `ParentId`) already returns a playlist library's own
|
||
playlists correctly; only music's artist grid needed a *dedicated*
|
||
endpoint, because a flat listing there would have returned raw tracks
|
||
instead of the artist grouping its own page needs. Playlists don't have
|
||
that problem - a playlist library's children already are the playlists
|
||
themselves.
|
||
|
||
**Tile drill-down, not a direct "start party" button**: like
|
||
Series/MusicArtist/MusicAlbum before it, a playlist tile links into its
|
||
own page (`tiles.ts`'s `DRILL_DOWN` table, `Playlist: { route:
|
||
'playlist', ... }`) rather than getting `canWatchParty`'s button
|
||
directly - a playlist itself was never something Emby could stream on
|
||
its own.
|
||
|
||
**Scope, deliberately narrower than it could be**: the new
|
||
`views/playlist/Playlist.ts` page is read-only past the one "Lancer
|
||
cette playlist" action - no per-item start/add the way `views/album`'s
|
||
track rows or `views/series`'s episode rows get. That's not laziness:
|
||
an Episode carries `SeriesId`, which is what lets `POST /api/parties`
|
||
reconstruct "queue the rest of the series from here" from just the
|
||
clicked item's id - a playlist item carries no equivalent
|
||
"which playlist(s) contain me" back-reference, so "start from item N,
|
||
queue the rest of *this playlist*" isn't cheaply reconstructable the
|
||
same way. Building that properly would mean threading a playlist id
|
||
through the click, a real protocol change for a page whose actual ask
|
||
was launching the playlist as a whole, not per-item granularity - left
|
||
for if it's ever actually asked for.
|
||
|
||
**Launching one**: `POST /api/parties` (`index.ts`) already special-cases
|
||
`item.Type === 'Episode'` to build a series queue; a playlist launch is
|
||
a sibling early branch (`item.Type === 'Playlist'`) calling a new,
|
||
separate `createPartyFromPlaylist` helper rather than merged into the
|
||
existing single-item path - that path is already working/relied-upon
|
||
and didn't need touching. Emby allows mixing movies/episodes and songs
|
||
in one playlist; this app's party model can't (a party is either
|
||
`'video'` or `'music'`). `CatalogService.splitPlaylistByMediaType` - a
|
||
small, pure, fully unit-tested function - decides which from the
|
||
playlist's *first* item and drops anything that doesn't match, logged
|
||
server-side so a mixed playlist isn't silently confusing from `docker
|
||
logs`. A video result builds a `QueueItem[]` exactly like the
|
||
episode-queue path already does; a music result seeds
|
||
`MusicQueueService.init` with the first track and `addTrack`s the rest -
|
||
the same mechanism `POST /api/parties/:id/tracks` already uses, just
|
||
called several times in a row here instead of once per click.
|
||
|
||
**Launching from the client**: `Playlist.ts`'s one button calls the
|
||
existing `createParty(playlistId)` - no new client API for *starting*
|
||
one, since the server already decides everything from the id alone, the
|
||
same as clicking any other item. It then branches on the created
|
||
party's `mediaType` (`joinMusicParty` vs. `navigate('/party/:id')`) -
|
||
the same check `Player.ts`'s own dispatcher already makes, just once,
|
||
at creation time instead of at every join.
|
||
|
||
Verified: the new `CatalogService` methods and `splitPlaylistByMediaType`
|
||
(homogeneous video, homogeneous music, mixed - first item's type wins)
|
||
are unit-tested with a mocked `EmbyClient`, matching every other
|
||
`catalogService.test.ts` case. Full typecheck/test/build on both
|
||
projects. The new page was driven through the same mocked-`fetch` +
|
||
real-bundle harness used throughout this session in a real browser -
|
||
confirmed the header/meta/item list render correctly from mocked catalog
|
||
data, and that clicking "Lancer cette playlist" reaches
|
||
`POST /api/parties` with the right id and correctly routes into the
|
||
video player for a video result (the same standing caveat as everywhere
|
||
else applies past that point - no real Emby server reachable here to
|
||
confirm actual playback).
|
||
|
||
## Sync watch-party progress back to Emby (PlaystateService)
|
||
|
||
Everything before this section reads from and proxies Emby; nothing
|
||
ever wrote anything back. Asked for directly: while someone's in a
|
||
party, push their progress to Emby's own `PlaystateService` API (its own
|
||
controller name in both Emby's and Jellyfin's server source) so their
|
||
own Emby library - resume position, last-played state - stays in sync
|
||
with what happened in the party, under *their own* account, not just
|
||
this app's.
|
||
|
||
**Scope, deliberately narrower than the full lifecycle**: only the
|
||
periodic `POST /Users/{UserId}/PlayingItems/{Id}/Progress` call is made.
|
||
No explicit start (`POST .../PlayingItems/{Id}`) or stop
|
||
(`DELETE .../PlayingItems/{Id}`) calls - Emby's own "now playing"
|
||
session dashboard won't show this app's streams, and the precise "mark
|
||
fully watched" heuristic (normally tied to a clean Stop near 100%) won't
|
||
fire deterministically. Steady Progress calls alone still update resume
|
||
position and last-played state either way, which is the actual core of
|
||
what was asked; wiring clean start/stop into every transition (item
|
||
change, leave, party destroy, a device takeover) is real complexity for
|
||
a secondary payoff, left as a documented follow-up rather than built
|
||
speculatively.
|
||
|
||
**The one real gap this ran into**: reporting a *follower's* own
|
||
progress needs their own position, but video parties only ever track
|
||
the *host's* (`PartyMember.isPaused`/`positionTicks` are written solely
|
||
by the host's own `"playstate"` handler - see "Realtime transport"
|
||
above). Building genuine per-follower position tracking would be a real
|
||
protocol addition of its own. Instead, every member's video report uses
|
||
the host's tracked position - already what their own player is supposed
|
||
to closely track via its drift correction (see "Player: no page
|
||
scroll"/`applyRemoteState`), so this is a reasonable approximation, not
|
||
a hack that quietly reports the wrong thing. Music has no such gap at
|
||
all: `party.music.current` plus its elapsed time is already one shared,
|
||
server-authoritative position every member already sees the same way -
|
||
nothing to approximate there.
|
||
|
||
**Getting each member's own token to where it's needed** turned out to
|
||
already be most of the way there. `SocketGateway.attach`'s `getSession`
|
||
callback is wired (`index.ts`) to `AuthService.getSession`, which
|
||
already returns the *full* `EmbySession` (including `embyToken`) - the
|
||
callback's own parameter type just narrowed it down to `{userId,
|
||
userName}` before anything downstream could read the rest. Widening
|
||
that type was the entire fix. `PartyMember` gained `embyToken: string |
|
||
null` (same null-until-known shape as this session's own `deviceId`
|
||
addition) - set in the socket `"join"` handler alongside `deviceId`, and
|
||
via a new, small `PartyService.setMemberEmbyToken(party, userId,
|
||
embyToken)` called right after each of the 4 REST create/join call
|
||
sites in `index.ts`, so a member has a valid token from the moment
|
||
they're a member, not only once their socket first connects. Never sent
|
||
to the client - `PartyService.toPublicState`'s member mapping never
|
||
included `deviceId` either, and doesn't grow this one.
|
||
|
||
**One new pure, tested function carries the real logic**:
|
||
`PartyService.getCurrentPlaybackState(party)` - video vs. music, per the
|
||
Approximation note above - null when there's genuinely nothing to
|
||
report (an empty music queue). Same "the interesting bit gets its own
|
||
tested function, the timer loop around it stays thin and untested" shape
|
||
this session already used twice (`absoluteToRelativeIndex`,
|
||
`splitPlaylistByMediaType`).
|
||
|
||
**Calling Emby** needed `EmbyClient.fetch` to grow beyond
|
||
GET-with-headers - it now optionally takes `method`/`body`
|
||
(JSON-encoded here, once, so every caller passes a plain object), every
|
||
existing call site unaffected since both are optional. New
|
||
`server/src/services/PlaystateService.ts` wraps the one call this needs.
|
||
Failures are caught and logged (new `Logger.playstate`) rather than
|
||
thrown - this runs on a plain `setInterval` (~20s, alongside
|
||
`removeIdleParties`/`TranscodeService.reapIdleSessions`'s own timers)
|
||
with no request to fail and no business ever disrupting real playback
|
||
over a background sync hiccup.
|
||
|
||
**TODO(verify) resolved - and the original guess was wrong.** Checked
|
||
directly against Emby's own real OpenAPI spec (swagger.emby.media,
|
||
pointed at a real server) after a "watch status" bug report suggested
|
||
this wasn't actually working: `POST /Users/{UserId}/PlayingItems/{Id}
|
||
/Progress` does exist at that exact path, but the spec declares
|
||
`PositionTicks`/`IsPaused` as *query string* parameters, not body
|
||
fields - and a third parameter, `MediaSourceId`, is *required* and
|
||
wasn't being sent at all. The request body schema this endpoint does
|
||
list (`Api.OnPlaybackProgress`) covers a handful of unrelated fields
|
||
(`PlaylistIndex`/`Shuffle`/`SleepTimerMode`/...), none of them these -
|
||
so the original guess was sending `PositionTicks`/`IsPaused` somewhere
|
||
Emby never reads them, while omitting a parameter Emby requires just
|
||
to accept the call at all. In practice this almost certainly meant
|
||
every progress report was failing outright, silently (best-effort by
|
||
design - see above - so nothing user-visible ever broke loudly; only
|
||
`[watchparty:emby]`'s own status+body capture would have shown it,
|
||
and no one had gone looking until this bug report).
|
||
|
||
Fixed: `reportProgress` now sends `MediaSourceId`/`PositionTicks`/
|
||
`IsPaused` as query parameters, no body at all. `MediaSourceId` comes
|
||
from `PartyStreamService`'s own cache - `CatalogService
|
||
.negotiateStreamTarget` already resolves a real one for every party's
|
||
shared stream negotiation (see "One shared Emby stream per party"
|
||
below), so `PartyStreamService` now caches it alongside the target
|
||
path and exposes it read-only via a new `getCachedMediaSourceId
|
||
(partyId)`. Deliberately passive - it never triggers a negotiation of
|
||
its own - so a party where no one has started playback yet this
|
||
session simply has nothing to report; `index.ts`'s sweep skips it, the
|
||
same way it already skips a party with nothing playing at all, rather
|
||
than paying for an extra PlaybackInfo round trip on every single tick
|
||
just to get a value the party's already-negotiated stream produces for
|
||
free.
|
||
|
||
While checking this, the app's other two Emby-API assumptions (the
|
||
ffmpeg direct-stream URL in `TranscodeService.buildSourceUrl`, and the
|
||
`/Playlists/{Id}/Items` endpoint in `CatalogService`) were re-checked
|
||
against the same real spec - both match with no missing required
|
||
parameter, consistent with them already having been tried for real
|
||
successfully.
|
||
|
||
**Made the sweep's own traffic visible, not just its failures** - asked
|
||
for directly, once fixing the request shape raised the obvious
|
||
follow-up question ("is it actually being sent correctly now?").
|
||
`reportProgress` now logs the exact outgoing request (`-> POST
|
||
/Users/{userId}/PlayingItems/{itemId}/Progress?MediaSourceId=...&...`)
|
||
right before sending it, and its outcome right after (`<- 200 ok` /
|
||
`<- 400 FAILED` / `<- threw ...`), all under the same
|
||
`[watchparty:playstate]` prefix - so `docker logs | grep
|
||
watchparty:playstate` answers "what did this send, and did Emby accept
|
||
it" on its own, without cross-referencing `EmbyClient`'s own generic
|
||
`[watchparty:emby]` failure log (which still fires too, independently,
|
||
with the fuller error body). Safe to log the full path here, unlike
|
||
`Logger.stream`'s own deliberately-partial logging - this query string
|
||
is only ever `MediaSourceId`/`PositionTicks`/`IsPaused`, never a token;
|
||
the real `embyToken` travels as a header, never in the URL, and the
|
||
tests confirm it never ends up in these lines either.
|
||
|
||
Verified: full typecheck/test/build on both projects. Unit tests cover
|
||
`getCurrentPlaybackState` (video reads the host, music reads the shared
|
||
queue, both branches, plus the empty-queue null case),
|
||
`setMemberEmbyToken`, `PlaystateService.reportProgress`'s corrected
|
||
query-string request shape (and that a thrown failure never
|
||
propagates), its own request/outcome logging (success, a non-ok
|
||
status, and a thrown error each produce a distinct, greppable line,
|
||
and the token itself never appears in one), and
|
||
`PartyStreamService.getCachedMediaSourceId`'s
|
||
cache-hit/cache-miss/dropped-on-failure behavior. `EmbyClient.fetch`'s
|
||
optional body-encoding capability stays generic plumbing (its own test
|
||
no longer claims PlaystateService as the reason it exists) - no
|
||
current call site needs it any more, but it costs nothing to keep for
|
||
whatever future caller does. No client changes at all - this is a pure
|
||
server-side background process with nothing for the browser to render
|
||
or react to.
|
||
|
||
## One shared Emby stream per party (not one per viewer)
|
||
|
||
Asked for directly: exactly one stream should ever be open between this
|
||
app and Emby per party, no matter how many members it has - before
|
||
this, every viewer's own `<video>`/`<audio>` independently negotiated
|
||
its own `PlaybackInfo` and got its own `PlaySessionId`, so a party of 5
|
||
meant Emby transcoding the same title 5 separate times. This is a
|
||
bigger, higher-regression-risk change than most of this session's other
|
||
work: it replaces the core playback path every existing party already
|
||
relies on (`CatalogService.proxyStream` and its route are gone
|
||
entirely) rather than adding something new alongside it - there was no
|
||
other consumer of the per-viewer path to preserve for, and the ask was
|
||
explicit that there should only ever be one mechanism, not two.
|
||
|
||
**A new cache, not new state on `CatalogService`** (that class is
|
||
deliberately stateless - its own comment says so): `server/src/services/
|
||
PartyStreamService.ts` owns `Map<partyId, {itemId, audioStreamIndex,
|
||
maxStreamingBitrate, targetPath, lastAccessUtc}>` - the same shape as
|
||
`TranscodeService`'s own session map, for the same reason (one entry
|
||
per party, replaced when the desired parameters change, reaped when
|
||
idle). `getOrNegotiate(party, session)` reads what the party *currently*
|
||
wants to stream (video: `party.itemId`; music: `party.music.current`'s
|
||
track - there's no host to read a position from the way video does,
|
||
but there's also only ever one shared position/track for the whole
|
||
party already, see "Music" above) and either reuses the cached target
|
||
or renegotiates.
|
||
|
||
**The negotiation itself was extracted, not duplicated**:
|
||
`CatalogService.proxyStream`'s old body (PlaybackInfo, then Emby's own
|
||
`TranscodingUrl` or a hand-built fallback, with the audio/quality
|
||
override applied twice - once to the negotiation, once to whatever URL
|
||
results, so a pick can't silently no-op regardless of which path Emby's
|
||
response takes) is now `negotiateStreamTarget(session, itemId, isAudio,
|
||
audioStreamIndex, maxStreamingBitrate): Promise<string>` - identical
|
||
logic, just returning the target instead of proxying it directly, so
|
||
`PartyStreamService` can cache what comes back.
|
||
|
||
**Recovering from a dead upstream session** needed no new plumbing:
|
||
Emby's own transcode isn't guaranteed to live forever, and a cached
|
||
target could start failing if Emby ends it on its own side.
|
||
`EmbyClient.proxyResponse` already calls `res.status(...)` before
|
||
writing the body, so `res.statusCode` is readable right after it
|
||
resolves - `PartyStreamService` checks that and drops the cache entry
|
||
on any failure, so the *next* request (a manual retry via the player's
|
||
own "Réessayer" button, or simply the next segment fetch) renegotiates
|
||
fresh instead of repeating the same dead target forever. This is the
|
||
same recovery a single failed per-viewer session already relied on
|
||
before - just applied once per party instead of once per viewer.
|
||
|
||
**Routes**, keyed by `partyId` (membership-checked, same pattern as the
|
||
burn-in routes above): `GET /api/catalog/party-stream/:partyId/
|
||
master.m3u8` and `.../raw?u=...` replace `/api/catalog/stream/:itemId/
|
||
...` entirely. `EmbyClient.rewritePlaylist`/`proxyResponse`'s trailing
|
||
param (`itemId` before this) is genericized to `routeKey` - it's only
|
||
ever read back out to build that second URL, so it's a real itemId for
|
||
`proxyImage`/`proxySubtitle` (which never actually reach the
|
||
playlist-rewrite branch anyway - wrong content-type/extension, so this
|
||
was always inert for them) and `party.id` for the new stream routes.
|
||
|
||
**Forced consequence, not a free design choice**: with only one stream,
|
||
there's only room for one set of encoding parameters behind it - a
|
||
video party's audio track and quality became a **host-only, party-wide**
|
||
choice (`Party.streamAudioIndex`/`streamQuality`, `setStreamOptions`/
|
||
`streamOptionsChanged`), byte-for-byte the same shape
|
||
`setBurnInSubtitle`/`burnInChanged` already established for the
|
||
image-subtitle case. `Player.ts`'s audio/quality `<select>`s are
|
||
disabled for a follower (`applyStreamOptionsGating`, mirroring
|
||
`renderSubtitleOptions`'s own gating) and restore the party's *current*
|
||
choice on load or broadcast instead of defaulting to the first option;
|
||
a host's change calls `setStreamOptions` instead of reloading locally,
|
||
and - like every other party-wide change this app makes - every member
|
||
(host included) only ever reacts to the resulting broadcast, never
|
||
applies a pick optimistically. `streamAudioIndex` resets on an item
|
||
change (an audio *track* index is exactly as item-specific as a
|
||
burn-in subtitle index is); `streamQuality` deliberately doesn't - a
|
||
bandwidth preference isn't tied to any one item, and wasn't reset
|
||
across an item change even before streams were shared either
|
||
(`qualitySelect`'s options were never rebuilt per-item).
|
||
|
||
**Music needed no picker at all** - `MusicBar.ts` never had an
|
||
audio-track/quality `<select>` in the first place, so sharing its
|
||
stream (`partyStreamUrl(activePartyId)`, same function the video player
|
||
uses) was the caching mechanism alone, no permission model to design.
|
||
A track change (`MusicQueueService.playNext`) ends the party's cached
|
||
session the same way `PartyService.advanceQueue` already does for
|
||
video - whatever was negotiated was for the outgoing track.
|
||
|
||
Verified: full typecheck/test/build on both projects. Unit tests cover
|
||
`negotiateStreamTarget` (moved from `proxyStream`'s old coverage) and
|
||
`PartyStreamService`'s cache hit/miss/param-change/item-change behavior,
|
||
including the failure-invalidation path, plus `setStreamOptions`'s
|
||
host-only gating and broadcast (mirroring the existing `setBurnInSubtitle`
|
||
tests). A real-browser check (mocked-fetch + real bundle, this
|
||
session's standing harness) confirmed a follower's audio/quality
|
||
selects render disabled and a host's render enabled, from the same
|
||
party data with only `youAreHost` flipped.
|
||
|
||
**TODO(verify)**: this is a different kind of unverified than this
|
||
session's other Emby-API guesses (the ffmpeg direct-stream URL, the
|
||
Playlists endpoint, the playstate-sync endpoint) - the actual endpoint
|
||
shape here (`PlaybackInfo`, `TranscodingUrl`) was already live,
|
||
confirmed code, just newly shared instead of per-viewer. What's
|
||
genuinely unverified is behavior under real multi-viewer load: whether
|
||
a real dead Emby session actually surfaces as an HTTP failure the way
|
||
`invalidateOnFailure` assumes, and how a real transcode's own seek
|
||
handling behaves once several viewers depend on the exact same one
|
||
instead of each having their own. Worth trying with two real accounts,
|
||
same standing item already on this project's own roadmap.
|
||
|
||
## Text subtitles join burn-in as a party-wide, host-only pick
|
||
|
||
Reported directly, after the shared-stream work above shipped: watch
|
||
party members still didn't have "synced viewing settings" for
|
||
subtitles. The report was accurate but the shared-stream work only
|
||
half-closed the gap - it made audio/quality/burn-in host-only and
|
||
party-wide, but a plain *text* subtitle pick (the common case; burn-in
|
||
only ever applies to bitmap PGS/VobSub/DVB tracks) was still each
|
||
viewer's own local `<track>` choice, exactly as it always had been
|
||
since "Player track selection" above. Once people actually watched
|
||
together, "everyone but me changed the subtitle" read as broken sync,
|
||
not as a deliberate per-viewer feature.
|
||
|
||
**A parallel mechanism, not a merge into `burnInSubtitleIndex`** - the
|
||
two picks are similar in shape but stay separate fields:
|
||
`Party.textSubtitleIndex: string | null`, a new `setTextSubtitle`/
|
||
`textSubtitleChanged` socket pair, byte-for-byte the same host-only,
|
||
video-only gating and request/broadcast/react shape
|
||
`setBurnInSubtitle`/`burnInChanged` already used. Keeping them separate
|
||
(instead of reusing `burnInSubtitleIndex` for both) avoided touching
|
||
the already-proven burn-in path at all - `SocketGateway`'s existing
|
||
`setBurnInSubtitle` handler gained exactly one new line to clear
|
||
`textSubtitleIndex` when it sets a real value, and `setTextSubtitle` is
|
||
new code entirely. **Only one subtitle can be showing at once**, so the
|
||
two are mutually exclusive - enforced on the server, in both
|
||
directions: a real (non-null) pick on either handler clears the other's
|
||
`burnInSubtitleIndex`/`textSubtitleIndex`. Clearing one to null/"Aucun"
|
||
is deliberately *not* treated as a pick for this purpose - it doesn't
|
||
touch the other field, so picking a burn-in and then explicitly
|
||
clearing it back to "Aucun" doesn't also silently wipe out an unrelated
|
||
text choice that was never actually in the way.
|
||
|
||
**Mechanically, the two still work nothing alike underneath** - the
|
||
whole reason this wasn't just a rename of `burnInSubtitleIndex`: a
|
||
burn-in choice changes the actual encoded stream (ffmpeg's overlay
|
||
filter), so every member reloads through `loadVideo` when it changes.
|
||
A text choice only ever selects which native `<track>` each member
|
||
already renders locally (`applySubtitleTrack`) - no transcode, no
|
||
reload, no re-buffering, the entire point of rendering text subtitles
|
||
client-side in the first place. `Player.ts`'s `onTextSubtitleChanged`
|
||
reflects this: it's normally just a `<track>` swap, *except* when the
|
||
text pick is what clears an active burn-in (the select allows picking
|
||
a text option, or "Aucun", at any time now - see below - even while
|
||
burn-in is active) - that one transition really does change the
|
||
underlying stream back to `PartyStreamService`'s normal one, so it
|
||
still needs the same reload `onBurnInChanged` already uses.
|
||
|
||
**Player UI**: `renderSubtitleOptions`'s gating stopped being
|
||
conditional on burn-in - `subtitleSelect.disabled` is now simply
|
||
`!isHost`, unconditionally, matching `applyStreamOptionsGating`'s own
|
||
reasoning for audio/quality. The select's value on load or broadcast
|
||
restores from `burnInSubtitleIndex || textSubtitleIndex || <previous
|
||
value>` - whichever kind of pick, if any, the party currently has.
|
||
Changing the select now always goes to the server - an image option
|
||
calls `setBurnInSubtitle`; everything else (a text option, or "Aucun")
|
||
calls `setTextSubtitle` - never applies anything locally first, same
|
||
discipline every other party-wide control in this app already follows.
|
||
|
||
**A second, unrelated bug in the same report**: "the language selector
|
||
shows an empty choice at the start." Root cause was in
|
||
`audioSelect.value = streamAudioIndex || '';` (both in
|
||
`loadTrackOptions` and `onStreamOptionsChanged`) - `streamAudioIndex`
|
||
is `null` by default (Emby's own default, nothing picked yet), and a
|
||
real audio-track `<option>` never has `value=""` (only the
|
||
no-tracks-at-all fallback branch does), so assigning `''` matched
|
||
nothing and left the select's `selectedIndex` at `-1` - a genuinely
|
||
empty-looking choice, not merely "showing the default option". Fixed
|
||
at both call sites by only assigning `.value` when `streamAudioIndex`
|
||
is truthy, otherwise leaving the select to fall back to its first real
|
||
option the way an untouched `<select>` already does. Unrelated to the
|
||
subtitle-sync fix above beyond living in the same file and the same
|
||
bug report - `qualitySelect` never had this problem, since its preset
|
||
list (`QUALITY_PRESETS`) already has an explicit `value=""` "Auto"
|
||
option to match against.
|
||
|
||
Verified: full typecheck/test/build on both projects. `SocketGateway`
|
||
tests cover `setTextSubtitle`'s host-only gating, its broadcast, its
|
||
no-op on a music party, and the mutual-exclusivity behavior in both
|
||
directions (mirroring the existing `setBurnInSubtitle` tests);
|
||
`PartyService` tests cover `textSubtitleIndex` through `create`/
|
||
`advanceQueue`/`toPublicState`. A real-browser check (mocked-fetch +
|
||
real bundle, this session's standing harness) confirmed: a follower's
|
||
subtitle select renders disabled regardless of burn-in state; a host's
|
||
select restores the party's current text or burn-in pick correctly on
|
||
load; and the audio select now lands on a real first option instead of
|
||
an empty one when `streamAudioIndex` is `null`.
|
||
|
||
## Known limitations
|
||
|
||
- Playback always goes through `hls.js` against whatever Emby's
|
||
`PlaybackInfo` suggests. If that ever comes back as a direct-play source
|
||
with no `TranscodingUrl` (no transcoding needed/possible), the client has
|
||
no fallback path yet - it'll try to hand hls.js something that isn't HLS.
|
||
- One fixed item/episode-queue per **video** party (a music party's queue
|
||
is the live, participative one - see "Music" above).
|
||
- A video party member needs a browser tab open and connected for sync to
|
||
apply - there's no "catch up automatically when you come back" beyond
|
||
whatever the next heartbeat corrects. Music has no equivalent heartbeat
|
||
at all (a deliberate simplification, not an oversight - see "Music"
|
||
above): position only resyncs on a discrete event (add/skip/pause/
|
||
resume), so two listeners' `<audio>` elements could drift apart slightly
|
||
during a long stretch with no such event - accepted for v1 given music's
|
||
more forgiving tolerance for exact sync compared to video+subtitles.
|
||
- No persistence: server restart or long idle time ends every party.
|
||
- No free seeking within a music track (only vote-skip to the *next* one)
|
||
- not asked for, and would need its own protocol addition
|
||
(`MusicQueueService` has no notion of "jump to position X" today, only
|
||
play/pause and advance).
|
||
- The self-hosted burn-in stream (see "Self-hosted ffmpeg" above)
|
||
transcodes forward from the start in real time - seeking far ahead of
|
||
what ffmpeg has produced so far will stall until it catches up; there's
|
||
no "restart ffmpeg at the seek target" logic yet, unlike Emby's own
|
||
transcode for every other stream. Its direct-source URL is also the
|
||
one Emby-shaped assumption in this whole app that's never been
|
||
confirmed against a real server.
|
||
- The Emby-playstate sync (see its own section above) only ever reports
|
||
*progress*, never an explicit start/stop - Emby's "now playing"
|
||
session list won't show this app's streams, and "mark fully watched"
|
||
isn't guaranteed to fire the way a clean Stop near 100% would trigger
|
||
it. A video follower's reported position is the *host's* tracked
|
||
position, not their own exact one (there's no per-follower tracking
|
||
today) - a reasonable approximation given the player's own drift
|
||
correction, not a precise per-viewer number.
|
||
- Now that every party shares one Emby stream (see "One shared Emby
|
||
stream per party" above), a rough patch in Emby's own transcode/seek
|
||
handling is felt by the whole party at once instead of just the one
|
||
viewer who caused it - not a new failure mode (Emby's own engine is
|
||
unchanged), just a wider blast radius for whatever it already was.
|
||
Audio track/quality changing from a per-viewer to a host-only,
|
||
party-wide choice is the explicit, intended trade-off for one shared
|
||
stream, not a limitation to fix - see that section for why.
|