"tsx --test test/**/*.test.ts" worked locally (Node v22) and failed on the CI runner (node:20-alpine, Node v20) with: Could not find '.../server/test/**/*.test.ts' node --test only gained native glob-pattern CLI arguments around Node 21/22; on Node 20 the literal glob string is treated as a single filename and, unsurprisingly, not found. Same script, two Node majors, different behavior - not a shell/quoting issue (both runs go through tsx's own CLI, no shell glob expansion involved either way). Fixed by listing the four test files explicitly instead of relying on a glob whose CLI support turned out to vary across the exact two Node installs this project runs on (this dev machine, and node:20-alpine - the CI container and server/Dockerfile's runtime image, deliberately matched). engines.node is >=18, so relying on a 21/22-only feature was already a latent bug even before it showed up. Verified locally (all 25 tests still pass with the explicit list) - the actual node:20-alpine behavior will confirm on the next CI run. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1267 lines
75 KiB
Markdown
1267 lines
75 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.
|
||
|
||
## 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.
|
||
|
||
## 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.
|
||
|
||
## 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.
|
||
|
||
## 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).
|
||
|
||
## 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).
|