Two bugs reported together, both surfaced by actually trying this for real. Casting: "the .m3u8 needs to be passed instead of a raw [url]". The master playlist itself was fine - the initial master.m3u8?castToken=... request already carries the token. What broke immediately after: every URI line inside that playlist gets rewritten (EmbyClient.rewritePlaylist) to route back through this app's own party-stream/:partyId/raw?u=... proxy, and that rewrite never carried the castToken forward. A browser never noticed (it's still sending its cookie regardless of the URL), but a Chromecast has no cookie to fall back on, so every segment/variant- playlist fetch after the first one 401'd, breaking playback moments after the receiver appeared to connect. Fixed by threading the same castToken into every rewritten line (rewritePlaylist(text, routeKey, castToken?), sourced from req.query.castToken right inside proxyResponse). The self-hosted burn-in path had the identical gap for a different reason - ffmpeg's own bare relative segment filenames drop any query string when resolved against a URL that had one, per ordinary URL-resolution rules - fixed the same way with a new pure appendCastTokenToSegments(text, castToken), extracted specifically so it's unit-testable without a real ffmpeg session. Player toolbar: "still doesn't work, mobile and PC" - the auto-hide fix from two commits ago hadn't actually landed for anyone. Real bug, not a browser quirk this time: becomeHost's 3-second heartbeat got wired to the same onDriverStateChanged function real discrete play/pause/seeked events use, which also re-arms the auto-hide countdown - correct for a genuine event, wrong for a heartbeat that fires unconditionally every 3s regardless of any real change. With the hide delay also at 3000ms, every heartbeat tick cancelled and restarted the countdown on the exact cadence it needed to run uninterrupted to ever complete - so for a host, on any platform, the bar could never actually finish hiding. Fixed by giving the heartbeat its own function, reportHeartbeat - same state sync, without the scheduleControlsHide call. Verified: full typecheck/test/build (122/122 server tests, including new coverage for rewritePlaylist's token propagation - had none at all before this - and appendCastTokenToSegments). Confirmed the heartbeat fix directly: 3 simulated heartbeat ticks call the hide-timer's reset zero times, one simulated real event still calls it once. Not verified: a real Chromecast fetching real rewritten segment URLs - no device reachable from this sandbox. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2974 lines
177 KiB
Markdown
2974 lines
177 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.
|
||
|
||
**Much later still: moved the toolbar off the subtitle zone.** Asked
|
||
for directly - `.requestToolbar` shared `.bottomOverlay`'s centered
|
||
column with the banner/follower-notice above, which is also exactly
|
||
where a native `<track>` subtitle renders (see "Player track
|
||
selection" below): real, clickable buttons sitting on top of subtitle
|
||
text, for the whole session, is a worse problem than a transient
|
||
banner or static notice occasionally doing the same. Pulled `#request
|
||
Toolbar` out into its own sibling block in `Player.html`, positioned
|
||
independently in the bottom-right corner (`position: absolute; right:
|
||
16px; bottom: 20px`) and stacked as a column instead of a row (`align-
|
||
items: flex-end`) so two pill buttons never have to fight a narrow
|
||
phone screen for width side by side. `.bottomOverlay` itself is
|
||
untouched - still centered-bottom, still just the banner and the
|
||
notice, neither of which is something a viewer needs to click through
|
||
subtitle text to reach. Verified against the real bundle at both
|
||
desktop and phone width: the two buttons render bottom-right,
|
||
non-overlapping with the (still centered) follower notice, in both
|
||
widths.
|
||
|
||
## 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.
|
||
|
||
**Round 1, 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. Fixed: sent as query
|
||
parameters instead, `MediaSourceId` resolved from the same
|
||
`PlaybackInfo` negotiation `CatalogService.negotiateStreamTarget`
|
||
already does for the party's shared stream. While checking this, the
|
||
app's other two Emby-API assumptions (the ffmpeg direct-stream URL,
|
||
the `/Playlists/{Id}/Items` endpoint) were re-checked against the same
|
||
real spec too - both already correct.
|
||
|
||
**Round 2, caught immediately from new logging added specifically to
|
||
catch exactly this** (every outgoing request + its outcome, under
|
||
`[watchparty:playstate]` - see `Logger.playstate`'s own comment for why
|
||
this needed to be enough on its own to answer "is this actually
|
||
reaching Emby, and with what"): a real request logged after round 1
|
||
shipped still came back `400`, now `Value cannot be null. (Parameter
|
||
'key')` - a bare .NET `Dictionary` null-key exception, and *not* the
|
||
earlier problem (`MediaSourceId` was present and well-formed).
|
||
`PlaySessionId` was the standout candidate - how Emby/Jellyfin clients
|
||
normally correlate a report with the specific playback session it
|
||
belongs to - and sending it (resolved the same way `MediaSourceId`
|
||
was) fixed this particular error.
|
||
|
||
**Round 3: a further report questioned the whole approach, not just
|
||
its request shape** - "still don't see this in my Emby profile /
|
||
Continue Watching" - and that turned out to be the right instinct.
|
||
This app deliberately never calls the matching `POST .../PlayingItems
|
||
/{Id}` ("Start") or `DELETE .../PlayingItems/{Id}` ("Stop") - a
|
||
documented v1 scope cut (see the original framing of this section,
|
||
still true) - and a bare `Progress` report with no corresponding
|
||
"Start" may simply never reach the resume-position/watched-history
|
||
data real clients read, regardless of how correctly-shaped the request
|
||
is by that point. Confirmed directly against the real server that this
|
||
data lives somewhere else entirely, trivially reachable without any
|
||
session lifecycle at all: every item Emby returns already carries the
|
||
requesting user's own `UserData` embedded
|
||
(`PlaybackPositionTicks`/`Played`/`PlayCount`/`IsFavorite`/...) - the
|
||
exact thing "Continue Watching" and the watched checkmark actually
|
||
read - and a plain `POST /Users/{UserId}/Items/{ItemId}/UserData`
|
||
updates it directly.
|
||
|
||
**Switched to that entirely** rather than layering a third fix onto
|
||
`PlayingItems` - `PlaystateService.updatePosition` (periodic, ~20s,
|
||
replacing `reportProgress`) and a new `PlaystateService.markPlayed`
|
||
(see below) both go through one private `updateUserData` helper.
|
||
Simpler than the endpoint it replaces (one JSON body, no query-string
|
||
parameter shape to get wrong) and more direct (writes the exact data
|
||
model the UI reads, rather than a live-session side-channel whose
|
||
effect on that data model was the very thing in doubt). The
|
||
`MediaSourceId`/`PlaySessionId` plumbing rounds 1-2 added
|
||
(`CatalogService.negotiateStreamTarget` returning them,
|
||
`PartyStreamService` caching them) was removed along with it -
|
||
`negotiateStreamTarget` is back to returning a bare target path, since
|
||
nothing needs those two values for anything else.
|
||
|
||
**Fetch-merge-write, deliberately, never a bare partial POST.** Emby's
|
||
OpenAPI spec documents the `UserData` endpoint's body as the *complete*
|
||
`UserItemDataDto`, with nothing suggesting a partial object merges
|
||
rather than replaces - and this is the one write in the whole app where
|
||
guessing wrong risks actually *erasing* a user's own real data (a
|
||
favorite, a rating), not just silently doing nothing the way the
|
||
`Progress`-endpoint bugs did. So every call to `updateUserData` first
|
||
`GET`s the item (confirmed directly against the real server that Emby
|
||
always returns it with the requesting user's current `UserData`
|
||
embedded), applies just the one or two fields being changed to a copy
|
||
of exactly that, and `POST`s the whole result back - correct regardless
|
||
of which way the real merge-vs-replace semantics actually go, at the
|
||
cost of one extra read before every write.
|
||
|
||
**Marking items Played, addressing the other half of the same
|
||
report** ("les éléments visionnés marqué comme lu") - not attempted in
|
||
any earlier round. `PlaystateService.markPlayed` sets `Played: true`
|
||
and resets `PlaybackPositionTicks` to `0`, called once per currently-
|
||
connected member (with a token) at the exact moment the party moves
|
||
past an item for good: `PartyService.advanceQueue` (next episode, for
|
||
the *outgoing* item, captured before the party's own state moves on)
|
||
and `MusicQueueService.playNext` (a track naturally finishing, or a
|
||
majority vote-skip - both go through this one method, so both get the
|
||
same treatment; `music.current` captured as `outgoing` before it's
|
||
reassigned). Fire-and-forget, same as the periodic position sync - a
|
||
member's bad token or a transient Emby hiccup must never block the
|
||
party from actually advancing. This is a deliberate heuristic ("the
|
||
party moved on from it" = "done with it"), not a real "did they watch
|
||
the whole thing" check - this app has no reliable way to know that
|
||
beyond what it already tracks, and most real streaming clients treat a
|
||
skip as a play too. Precision beyond that (e.g. only marking Played
|
||
past some percentage watched) would need every queued item's own
|
||
duration tracked everywhere it's queued, which nothing currently does -
|
||
left as a documented gap, not built speculatively.
|
||
|
||
Verified: full typecheck/test/build on both projects. Unit tests cover
|
||
`getCurrentPlaybackState`, `setMemberEmbyToken`,
|
||
`PlaystateService.updatePosition`/`markPlayed`'s fetch-merge-write
|
||
behavior in detail - a favorite/rating/play-count this app never asked
|
||
to change survives a position update byte for byte, a `Played`-only
|
||
update via `markPlayed` doesn't disturb `IsFavorite`, a failed *read*
|
||
skips the write entirely rather than posting incomplete data, an item
|
||
with no `UserData` at all yet starts cleanly from an empty object - its
|
||
own request/outcome logging (success, a non-ok status on either call,
|
||
and a thrown error each produce a distinct, greppable line, and the
|
||
token itself never appears in one), and that
|
||
`PartyService.advanceQueue`/`MusicQueueService.playNext` call
|
||
`markPlayed` for the *outgoing* item only, only for members who are
|
||
both connected and have a known token, mirroring the exact same guard
|
||
`index.ts`'s periodic sweep already uses. `EmbyClient.fetch`'s optional
|
||
body-encoding capability (added for this feature's very first attempt)
|
||
stays generic plumbing - still exercised by this feature's own
|
||
`UserData` POST, so unlike after round 2 it's not just kept around for
|
||
a hypothetical future caller. No client changes at all - this is a pure
|
||
server-side background/event-driven 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`.
|
||
|
||
## Catalog browsing was slow: images were never cacheable
|
||
|
||
Reported directly ("l'api du contenu des catalogues est hyper lente").
|
||
`CatalogService`'s own listing methods (`getLibraryItems`, `getArtists`,
|
||
etc.) were already fine - each is exactly one Emby call, no per-item
|
||
follow-up requests, and timed directly against the real server a
|
||
300-item library listing (`getLibraryItems`'s own `Limit: 300`) came
|
||
back in well under half a second. The real cost was downstream of that:
|
||
every one of those up to 300 tiles needs its own thumbnail, each
|
||
proxied through `EmbyClient.proxyImage` → `proxyResponse`, and
|
||
`proxyResponse` was silently dropping every caching signal Emby sent -
|
||
confirmed directly against the real server that an image response
|
||
already carries `Cache-Control: public` and a real `ETag`, but this
|
||
method only ever forwarded `content-type`/`content-range`/
|
||
`accept-ranges`. With no validator and no cache policy ever reaching
|
||
the browser, it had no choice but to re-download every single thumbnail
|
||
in full, from scratch, on every visit to every library, home-page
|
||
carousel, series/season/album page - the same images, over and over,
|
||
compounding across however many tiles a page shows.
|
||
|
||
**Fixed both directions of the same problem.** `EmbyClient.fetch`
|
||
already accepted `range` to forward; it now also accepts
|
||
`ifNoneMatch`/`ifModifiedSince`, sent as `If-None-Match`/
|
||
`If-Modified-Since` - and a `304` (the entire point of sending those)
|
||
is explicitly excluded from the failure-logging path (`!res.ok` is true
|
||
for a 304, but it's Emby correctly saying "unchanged", not an error
|
||
worth `[watchparty:emby]`). `proxyResponse` now reads the browser's own
|
||
`If-None-Match`/`If-Modified-Since` request headers and forwards them
|
||
to Emby, and forwards `cache-control`/`etag`/`last-modified` back from
|
||
Emby's response, alongside the existing `content-range`/`accept-ranges`
|
||
pair. Confirmed end to end against the real server, not just reasoned
|
||
about: a plain image GET comes back `200` with `Cache-Control: public`
|
||
+ a real `ETag`; the *same* request repeated with that `ETag` as
|
||
`If-None-Match` comes back a real `304` with a zero-byte body. Harmless
|
||
for everything else this method proxies (video/audio segments, HLS
|
||
playlists, subtitles) - it only ever forwards whatever Emby itself
|
||
decided, never invents a policy, so something Emby doesn't mark
|
||
cacheable (a per-session transcode) simply never gets these headers set
|
||
at all, same as before.
|
||
|
||
Verified: full typecheck/test/build on both projects. New
|
||
`EmbyClient.proxyResponse` tests (this method had never been exercised
|
||
directly before - every existing test that touches it mocks it away
|
||
entirely to isolate whatever calls it) cover forwarding the cache
|
||
headers on a real bodied response, forwarding the browser's conditional
|
||
headers to Emby and relaying a real 304 with no body, and that a
|
||
response with neither header (most proxied paths) doesn't choke or
|
||
invent one; `EmbyClient.fetch` tests cover a 304 not hitting the
|
||
failure log and the two new headers being sent only when given.
|
||
|
||
## A spinner on every loading state
|
||
|
||
Asked for directly - every "Chargement..." spot across the app (route
|
||
entry for Album/Artist/Library/Playlist/Series, the sidebar's own
|
||
library list, Home's per-library carousels, a season switch reloading
|
||
its episode list, the player's video-buffering notice) was plain
|
||
`.hint` text with no visual motion, easy to mistake for a stuck page
|
||
rather than one still fetching. New `.spinner` in `styles/base.css` - a
|
||
plain CSS ring (`border` + `border-top-color`, no image/icon-font
|
||
asset), `display: inline-block` with a small negative `vertical-align`
|
||
so it drops into an existing text node instead of needing the element
|
||
restructured into a flex row; `@media (prefers-reduced-motion: reduce)`
|
||
turns off the spin for a viewer who's asked for less motion, matching
|
||
this app's existing carefulness about motion elsewhere.
|
||
|
||
New `client/src/loading.ts` exports one shared `LOADING_HINT` constant
|
||
(`<p class="hint"><span class="spinner"></span>Chargement...</p>`) so
|
||
every dynamic call site renders identical markup instead of each
|
||
hand-typing a slightly different one; the handful of *static* HTML
|
||
templates that had their own inert copy of this same placeholder text
|
||
(never actually paintable - immediately overwritten synchronously by
|
||
the same script that just set it, so a viewer could never see the
|
||
old, spinner-less version even for a frame) were left alone rather
|
||
than churned for a change nobody could ever see; `Library.html`'s own
|
||
`#grid` placeholder was the one exception - genuinely visible while
|
||
its library's items load - and got the same markup pasted directly
|
||
(a static template, no JS import of its own).
|
||
|
||
Not touched: the brief button-label loading states elsewhere ("Ajout...",
|
||
"Démarrage..." while a party/queue-add request is in flight) - a
|
||
different, smaller UI element than a block-level `.hint`, and typically
|
||
resolved within one REST round trip, low enough value to add a second
|
||
spinner variant for.
|
||
|
||
Verified: full typecheck/build on the client (no server changes at
|
||
all - this is pure client markup/CSS). Visually confirmed in a real
|
||
browser (the compiled bundle's own `app.css`) that the spinner renders
|
||
correctly inline with text in both a plain `.hint` and the player's
|
||
`.loadingNotice` context, and is genuinely animating (`getComputedStyle`
|
||
sampled twice, 300ms apart, showed a different `transform` matrix each
|
||
time).
|
||
|
||
## Casting to a Chromecast
|
||
|
||
Asked for directly, with a detailed spec naming the exact services/files
|
||
to touch: let a party member cast the party's video onto a Chromecast
|
||
(Google's Default Media Receiver - no custom receiver, so no Google Cast
|
||
Developer Console app registration needed at all) with play/pause/seek
|
||
shown as a remote on the sender's own screen. The spec's description of
|
||
this app's architecture was verified directly against the real code
|
||
before building anything: every party already streams through one
|
||
shared, party-keyed relay (`PartyStreamService`/`TranscodeService`,
|
||
proxied via `EmbyClient`, see "One shared Emby stream per party" above)
|
||
as standard HLS, and the server proxies 100% of Emby access - the
|
||
browser never holds an Emby token, only the `wp_session` cookie (see
|
||
"Goals"/`AuthService`). A Chromecast is a new *kind* of client for that
|
||
relay: it can't carry the sender's httpOnly cookie at all, so it needed
|
||
its own narrow way in without weakening "the server holds the only real
|
||
Emby token" that the rest of the app is built around.
|
||
|
||
**Scoped to video parties only** - the spec's every concrete reference
|
||
(the player, the party-stream/burn routes, the burn-in fallback) is
|
||
video-specific, and `MusicBar.ts` has no comparable control surface
|
||
today. Casting a listen-party would be a fast follow, not this.
|
||
|
||
**`CastTokenService`** (new, same shape as `AuthService`: an in-memory
|
||
`Map`, opaque `crypto.randomUUID()` keys, TTL-checked on read) issues a
|
||
short-lived token scoped to one `{partyId, userId}`, standing in for
|
||
that member's own already-authenticated `EmbySession` wherever a
|
||
Chromecast needs to reach the party's stream. Two new routes:
|
||
`POST /api/parties/:partyId/cast-token` (normal cookie auth, mints one)
|
||
and `DELETE /api/parties/:partyId/cast-token` (revokes it - called when
|
||
a cast session ends). The 4 routes a Chromecast actually needs to reach
|
||
- `party-stream/:partyId/master.m3u8`/`raw`, `burn/:partyId/master.m3u8`/
|
||
`:segment` - gained one small fallback (`sessionForStream` in `index.ts`):
|
||
try the session cookie first, same as ever, then a `castToken` query
|
||
param. Either way the result is the exact same `EmbySession` type every
|
||
other route already uses, so nothing downstream
|
||
(`PartyStreamService`/`TranscodeService`/`EmbyClient`) had to change at
|
||
all. The image/subtitle routes were deliberately left cookie-only -
|
||
outside the spec's own 4-route list, and a Chromecast's Default Receiver
|
||
has no use for either here.
|
||
|
||
A token's TTL (12h) is deliberately generous - it's only a backstop.
|
||
The real invalidation is explicit and unconditional: `PartyService
|
||
.leave()` already runs a few unconditional per-leaving-member cleanups
|
||
(`MusicQueueService.removeVote`, right after the member is removed from
|
||
the map) - `CastTokenService.revokeForMember` joined that list, so a
|
||
token dies the instant its owner actually leaves the party, whether or
|
||
not the client ever got a chance to call the explicit "stop casting"
|
||
route (a crashed tab, a Chromecast that just loses power). Even a token
|
||
that somehow outlived both of those is no more powerful than "this
|
||
party member's own streaming access" - every one of the 4 routes still
|
||
separately checks `party.members.has(session.userId)`, exactly like a
|
||
cookie session already does.
|
||
|
||
**The sender side** loads the CAF Sender SDK the same way `hls.js`/
|
||
`socket.io` already are - a pinned CDN `<script>` (Google only ever
|
||
serves this from its own `gstatic` host, not a versioned CDN like the
|
||
other two, and it signals readiness via a global callback rather than
|
||
being usable synchronously right after load). New `client/src/cast.ts`
|
||
is the only module that touches the SDK directly, and exposes a small
|
||
`PlaybackDriver` interface (`currentTime`, `paused`, `play()`,
|
||
`pause()`) with two implementations - one over the local `<video>`, one
|
||
over CAF's `RemotePlayer`/`RemotePlayerController` (which only ever
|
||
*pushes* state via events, so the wrapper caches it behind plain
|
||
getters). This turned out to be the **only** new abstraction the whole
|
||
feature needed, not a new sync mechanism: `Player.ts`'s existing
|
||
`sendLocalState`/`applyRemoteState` already read and write "the player"
|
||
in exactly one place each, so pointing them at whichever driver is
|
||
active - instead of `video` directly - carried over the host-only gate,
|
||
the drift-tolerance reconciliation, and the 3s heartbeat unchanged for
|
||
either one. Seek was already "the host's next state broadcast" with no
|
||
separate seek event, so seeking on the TV is already party-wide for
|
||
free. A non-host caster already only ever *received* `applyRemoteState`
|
||
(the host-only check in `sendLocalState` already blocks a follower from
|
||
pushing) - redirecting that to the cast driver instead of the local
|
||
`<video>` reproduces "still dependent on the host" automatically, with
|
||
zero cast-specific code for either of those two behaviors.
|
||
|
||
Starting a cast: `POST cast-token`, build an absolute
|
||
`partyStreamUrl`/`burnInStreamUrl` carrying it as `?castToken=...`,
|
||
`requestSession()` → `loadMedia()`, then tear down the local `hls.js`
|
||
instance (`hls.destroy()` - the same full teardown-and-rebuild this app
|
||
already uses for every other stream-parameter change, not a new
|
||
pause-in-place path) and swap the active driver. Ending one - the user
|
||
stopping it, or CAF's own `SESSION_ENDED` firing because the receiver
|
||
dropped off on its own - swaps back and calls the *existing*
|
||
`loadVideo(itemId, driver.currentTime, !driver.paused)` to resync,
|
||
reusing the exact path an audio/quality change or "Réessayer" already
|
||
uses rather than inventing a second one. A genuinely new reload for any
|
||
other reason (an item change, a burn-in/subtitle/stream-options change)
|
||
simply ends an active cast first rather than trying to keep a live
|
||
session in sync with every kind of party transition - judged not worth
|
||
the added complexity for v1; re-casting afterward is one click, and
|
||
CAF's `ORIGIN_SCOPED` auto-join policy means that click typically
|
||
reconnects to the same receiver instead of prompting a fresh device
|
||
picker. One related gap, also accepted for v1: reopening the same
|
||
party page after navigating away mid-cast (an SPA route change, not a
|
||
reload) doesn't try to detect and re-attach to a still-live session -
|
||
the new page mount just starts from local playback again.
|
||
|
||
Security, checked against the spec's own constraints: the real Emby
|
||
token never reaches the Chromecast (only the opaque `castToken` ever
|
||
appears in `contentId`, by construction - same as every other proxied
|
||
route); no second Emby stream is ever opened for a cast (it hits the
|
||
exact same party-keyed cache everything else does); a cast token is
|
||
only ever consulted by `sessionForStream`'s 4 call sites - `requireSession`,
|
||
used by every other route, is untouched.
|
||
|
||
Verified: full `typecheck`/`test`/`build` on both projects: unit tests
|
||
for `CastTokenService` (issue/resolve/reissue-invalidates-the-old-one/
|
||
revoke/wrong-party-rejection) and for `PartyService.leave()`'s new
|
||
call. Confirmed against the real running server (no login needed for
|
||
this part) that the 4 streaming routes still reject a request with no
|
||
auth at all exactly as before, and now also reject one with an invalid
|
||
`castToken` - both a plain `401`, no regression on the existing cookie
|
||
path. Confirmed in a real browser that the Cast SDK's `<script>` loads
|
||
and initializes correctly (`window.cast`/`window.chrome.cast` both
|
||
populated) with no console errors. **Not verified**: this sandbox has
|
||
no real Chromecast to finish a `requestSession()`/`loadMedia()` round
|
||
trip against, and no Emby login credentials to reach the player page at
|
||
all - so the actual on-TV playback, the cast button's live show/hide
|
||
behavior, and whether the Default Receiver truly renders this app's HLS
|
||
output cleanly are all unconfirmed, the same way this project already
|
||
flags its other can't-verify-without-the-real-thing assumptions. Worth
|
||
trying with a real device once deployed.
|
||
|
||
## Fixing the listen-party queue's layout and its missing album art
|
||
|
||
Reported directly: the music bar's "File d'attente" popup looked broken,
|
||
and the listen-party card on the home page never showed an image. A
|
||
mockup was put together first (before touching any code) to confirm the
|
||
direction, since the queue side was a genuine UI rework, not just a
|
||
one-line fix.
|
||
|
||
**The layout bug**: `MusicBar.ts`'s queue rows were reusing
|
||
`styles/trackList.css`'s shared `.trackRow` - a CSS grid with 4 fixed
|
||
columns (`28px 1fr auto auto`) sized for `views/album`'s own row shape
|
||
(track number, name, duration, an "Ajouter"/"Écouter ensemble" button).
|
||
The queue's rows never had a track number at all, and rendered
|
||
completely different content in that same slot - the track's own NAME
|
||
was the first child, so it landed in the 28px column meant for a
|
||
one-or-two-digit index, crushed down to its first couple of characters,
|
||
while the artist/album text spilled into the wide `1fr` column instead
|
||
meant for the name. Fixed with a dedicated `.queueRow` (flex, not grid -
|
||
image, then a truncating info column, then the duration), rather than
|
||
trying to force this list's different shape through Album's grid with
|
||
more special-casing.
|
||
|
||
**The missing images**: confirmed directly against the real Emby server
|
||
(three sampled tracks, `Fields=AlbumId,ImageTags`) that an individual
|
||
Audio item essentially never carries its own Primary image - every one
|
||
tested answered `500`, not even a clean `404` - while the same tracks'
|
||
parent albums mostly did (`200 image/jpeg` for two of three; the third
|
||
album genuinely has no artwork uploaded at all, a real data gap, not a
|
||
bug). Every place that built an image URL from a raw `EmbyItem`
|
||
independently reconstructed `/api/catalog/image/${item.Id}` by hand
|
||
(`CatalogService.toItemDto`, and three separate spots in `index.ts`'s
|
||
party-creation routes that skip `toItemDto` entirely) - all now funnel
|
||
through one new `CatalogService.imageUrlFor(item)`, which points a music
|
||
track at its `AlbumId` instead of its own `Id` when one's known.
|
||
`EmbyItem` gained the `AlbumId` field, and every `Fields` param that
|
||
lists Audio items among its results (`getAlbumTracks`,
|
||
`getPlaylistItemsRaw`, and defensively `getRecentItems`/
|
||
`getLibraryItems`) now requests it - a single-item detail fetch
|
||
(`getRawItem`, used by party creation) already includes it with no
|
||
`Fields` param needed, confirmed against the real server.
|
||
|
||
While fixing the home-page card's now-working image, its box turned out
|
||
to be the wrong shape for music too: `.partyCard img` was a fixed
|
||
48×72 portrait box - correct for a movie/show poster, but a hard crop
|
||
for an album's square cover. A video party's card is untouched; a
|
||
music party's now gets a 56×56 square box instead
|
||
(`.partyCardArt--square`, toggled from `LobbyPartyDto.mediaType` in
|
||
`Home.ts`).
|
||
|
||
Verified: full `typecheck`/`test`/`build` on both projects, including
|
||
new unit tests for `imageUrlFor` (a movie/episode unaffected, a track
|
||
with an `AlbumId` falls back to it, a track with none still falls back
|
||
to its own id rather than building a broken URL). No server is needed
|
||
to see the queue/card layout itself, so those were checked visually
|
||
against a mockup built from this app's own real theme tokens/typefaces
|
||
rather than a live instance.
|
||
|
||
## Reworking the player: a unified bottom bar, and a fullscreen that doesn't hide it
|
||
|
||
Asked for directly - a UI rework of the video player, "in both normal
|
||
and fullscreen mode", with one concrete requirement: a viewer in
|
||
fullscreen still needs to see the pause/resume request buttons. A
|
||
mockup was built first (this app's own theme tokens, not a generic
|
||
one) and iterated through a few rounds of feedback before any of this
|
||
landed - the mockup itself ended up covering three things, not the two
|
||
first asked for.
|
||
|
||
**The actual fullscreen bug, found while investigating the request.**
|
||
This app never had its own fullscreen button - the only way in was the
|
||
browser's native affordance on the bare `<video>` element itself
|
||
(double-click, or the host's own native `<video controls>` fullscreen
|
||
button - see `becomeHost`). Native per-element fullscreen only ever
|
||
renders that element's own subtree: every sibling overlay - the top
|
||
bar, presence avatars, and critically the request toolbar - is a
|
||
sibling of `<video>` in the DOM, so all of it disappeared the instant
|
||
fullscreen engaged. A follower in fullscreen had no way to reach the
|
||
host at all.
|
||
|
||
**The fix**: a new `#fullscreenBtn` that calls
|
||
`playerWrap.requestFullscreen()` - the whole player, video AND overlay
|
||
chrome - never `video.requestFullscreen()`. Since a native per-video
|
||
path is still reachable too (the same double-click/host-native-controls
|
||
gestures as before - nothing about those was removed, so the host keeps
|
||
native play/pause/seek/volume exactly as today), a document-level
|
||
`fullscreenchange` listener watches for `document.fullscreenElement ===
|
||
video` specifically and corrects it: exit, then immediately re-enter on
|
||
`playerWrap` instead. This runs regardless of which gesture triggered
|
||
it, so the overlay chrome is never actually lost for more than the
|
||
listener's own tick.
|
||
|
||
**The bottom bar rework.** What used to be three independent floating
|
||
pieces - a centered column (`.bottomOverlay`: the transient request
|
||
banner, a static "L'hôte contrôle la lecture" notice, a separate
|
||
Chromecast notice), a request toolbar cornered bottom-right, and an
|
||
"Épisode suivant" text link up in the top bar - are now one bar
|
||
(`#bottomBar`, three flex zones):
|
||
- **Left**: exactly one of `#nextBtn` (host, now a real accent-green
|
||
pill button with a skip-next icon, not a plain underlined link) or
|
||
`#requestToolbar` (follower, `#requestPauseBtn`/`#requestResumeBtn`,
|
||
each now labeled "Demander pause"/"Demander reprise" with a small
|
||
icon). The two are mutually exclusive by construction - only the host
|
||
ever has a queued item to advance, only a follower ever has someone
|
||
else's playback to request against - so there's never a conflict for
|
||
the same slot.
|
||
- **Center**: a single `#ambientNotice` line, replacing the two
|
||
separate static notices - `updateAmbientNotice()` shows "Diffusion en
|
||
cours sur le Chromecast" while casting (checked first - it's the more
|
||
useful thing to know in the moment), else "L'hôte contrôle la
|
||
lecture" for a follower, else nothing for a host who isn't casting.
|
||
- **Right**: `#fullscreenBtn`, for everyone, always.
|
||
|
||
The transient request banner stays independent, near the top rather
|
||
than folded into the persistent bar - it's a pop-in/fade-out toast
|
||
(`showRequestBanner`), and merging it into a bar that's otherwise always
|
||
present would have made it read as just another static line rather
|
||
than something that needs attention right now.
|
||
|
||
**A visible cue for casting.** `#castingScreen` now covers the video
|
||
area whenever `isCasting` is true - the local `<video>` is paused/torn
|
||
down while casting (see "Casting to a Chromecast" above), so leaving
|
||
its last frame sitting there with no explanation would just look
|
||
stuck. A plain icon + "Diffusion sur le Chromecast", not an attempt to
|
||
mirror the TV's own picture.
|
||
|
||
Verified: full `typecheck`/`build` on the client (no server changes at
|
||
all - this is pure client markup/CSS/TS). Checked in a real browser
|
||
against the actual compiled `app.css`/bundle (not just the mockup) -
|
||
injected the new markup with its real classes for the host view, the
|
||
follower view (with the toast), and the casting view, and confirmed
|
||
each renders as designed; caught and fixed one real sizing bug this
|
||
way (`.fsBtn` grew to 32px instead of its declared 30px - a flex item's
|
||
default `min-width: auto` was letting `button`'s own base 12px/16px
|
||
padding win over the smaller explicit width; fixed with an explicit
|
||
`padding: 0` on the icon-only button). **Not verified**: the Fullscreen
|
||
API itself needs a real user gesture and an actual browser fullscreen
|
||
transition, neither of which this sandbox can exercise - worth trying
|
||
for real once deployed, same as this project's other can't-verify-
|
||
without-the-real-thing items.
|
||
|
||
## The player's bottom bar was really two bars: replacing native `<video controls>`
|
||
|
||
A direct follow-up to the player rework above, reported right after
|
||
trying it for real: "two z-index layers, some elements duplicated,
|
||
only in fullscreen." The actual cause was never a bug in the new
|
||
bottom bar itself - it's that `video.controls = true` (set for the host
|
||
since the very first version of this player) has always rendered the
|
||
browser's own native control bar, a separate piece of UI that lives in
|
||
its own always-on-top compositing layer, entirely independent of this
|
||
app's DOM/CSS. That was never visually obvious before: fullscreen used
|
||
to show *only* the video (the bug the previous section fixed), so the
|
||
native bar was the only thing there was to look at. Once fullscreen
|
||
correctly started showing this app's own overlays too, the native bar
|
||
and the custom one both became visible at once - and specifically
|
||
*duplicated*, not just crowded: Chrome adds its own cast icon to a
|
||
`<video>`'s native controls automatically once it detects a Cast-
|
||
capable device, entirely separate from `#castBtn`'s own integration
|
||
(cast.ts) - "some elements duplicated, others not" was that icon
|
||
sitting right next to this app's own, while native play/pause/seek/
|
||
volume simply had no custom equivalent yet to *look* duplicated
|
||
against.
|
||
|
||
Two fixes, asked for as one:
|
||
|
||
- **`disableremoteplayback`** on `<video>` - the standard attribute
|
||
for exactly this situation (a page with its own Cast integration
|
||
telling the browser not to add its own). Should have been there from
|
||
the start of "Casting to a Chromecast" above; this is where it was
|
||
actually noticed missing.
|
||
- **Native `<video controls>` retired entirely**, replaced with a
|
||
custom transport bar - not just for fullscreen, everywhere, so there
|
||
is only ever one control surface instead of two that happen to
|
||
overlap less in the normal (windowed) case. `PlaybackDriver` (cast.ts)
|
||
gained `duration` alongside `currentTime`/`paused` - the one property
|
||
the existing abstraction didn't yet expose, needed for a seek bar to
|
||
compute a fraction or a target time regardless of which driver is
|
||
currently active.
|
||
|
||
**What `#bottomBar` gained**: a seek bar (`#progressBar`/`#progressFill`)
|
||
on its own row above the existing button row - column layout, so the
|
||
two stack in normal flow rather than two independently-guessed fixed
|
||
offsets (the same reasoning `.topOverlay`'s actionBar+presenceOverlay
|
||
grouping already used). Host: pointer events (not click/drag, so mouse
|
||
and touch share one implementation) seek by fraction of
|
||
`activeDriver.duration`; a follower's bar is the same element, just
|
||
read-only (`.readOnly` - a cursor/visual hint only, the actual pointerdown
|
||
handler checks `isHost` itself regardless). A `progressTicker`
|
||
(500ms) keeps the fill/time label moving between discrete events, since
|
||
`currentTime` changes continuously while playing with no event of its
|
||
own for that. `#playPauseBtn` (host-only, sharing the request buttons'
|
||
left zone with `#nextBtn` - never more than one of the three visible at
|
||
once) and `#muteBtn` (everyone, local-only - `video.muted` directly,
|
||
never synced, since unlike play/pause this has no party-wide meaning at
|
||
all - hidden while casting instead of controlling a local `<video>`
|
||
that's already silent then) round out the replacement.
|
||
|
||
One function, `onDriverStateChanged`, threads through every discrete
|
||
event that used to just call `sendLocalState` directly - the local
|
||
`<video>`'s own native play/pause/seeked events, and a cast session's
|
||
`RemotePlayerController` events (`startCast`'s own `onRemoteStateChanged`
|
||
callback) - so the button icon, the seek bar, and the party broadcast
|
||
stay in lockstep regardless of which driver or which gesture caused
|
||
the change, the same discipline every other party-wide state in this
|
||
app already follows.
|
||
|
||
Verified: full `typecheck`/`build` on the client. Checked in a real
|
||
browser against the compiled bundle - injected the new bottom bar
|
||
markup for both a "playing, unmuted" and a "paused, muted" state and
|
||
confirmed the icons/fill/time label/readOnly cursor all render as
|
||
designed. **Not verified**: an actual drag-to-seek gesture and real
|
||
play/pause/volume behavior against a real `<video>` with a real source -
|
||
this sandbox has no reachable Emby stream to attach one to, so the
|
||
event wiring is confirmed by reading, not by dragging a real seek bar.
|
||
Worth exercising for real once deployed.
|
||
|
||
## The bottom bar auto-hides down to just the seek bar
|
||
|
||
Reported right after the previous section shipped: the bottom bar,
|
||
now always visible, sits exactly where a native `<track>` subtitle
|
||
renders (`applySubtitleTrack`) - and was masking it. The standard fix
|
||
every real video player already uses: collapse to a minimal state
|
||
after a stretch of no activity, expand again on the slightest
|
||
interaction.
|
||
|
||
`#bottomBar` gets a `.minimal` class (`scheduleControlsHide`,
|
||
`CONTROLS_HIDE_DELAY_MS` = 3s of no activity) that transitions away its
|
||
padding/background/border and the buttons row's height/opacity -
|
||
`#progressBar` itself is untouched by any of this (including its own
|
||
generous invisible hit-area for seeking), so a thin seek line is always
|
||
there, just without the box around it once minimal. Expands again on
|
||
any `pointermove`/`pointerdown` anywhere in `.playerWrap`, or while the
|
||
pointer is sitting on the bar itself (`mouseenter`/`mouseleave` pause
|
||
the countdown entirely rather than just resetting it, so hovering the
|
||
bar can never lose a race with its own timer). Stays expanded outright
|
||
while genuinely paused - nothing playing to hide the bar *from* - and
|
||
every discrete playback event (`onDriverStateChanged`) re-arms the
|
||
countdown too, not just raw pointer activity, so resuming from a pause
|
||
correctly restarts it rather than leaving the bar stuck open forever.
|
||
Applies to everyone, host or follower alike - the bar (and the
|
||
subtitle-masking problem) exists for both.
|
||
|
||
Verified: full `typecheck`/`build`. Confirmed in a real browser against
|
||
the compiled bundle, side by side with an actual subtitle-shaped
|
||
overlay in the same position: the expanded bar visibly overlaps it,
|
||
the `.minimal` state (just the thin progress line) leaves it fully
|
||
readable. The *timer* itself (does it actually fire after 3s of real
|
||
inactivity, does hovering genuinely prevent it) wasn't driven through
|
||
real wall-clock waits here - the CSS transition and class-toggling
|
||
logic were checked directly instead.
|
||
|
||
**Reported broken on mobile, right after shipping**: it never actually
|
||
auto-hid there. Root cause - a classic mobile-web trap, not a typo:
|
||
`mouseenter`/`mouseleave` on the bar tracked "is the pointer resting on
|
||
the controls" so hovering could hold them open, but a touch pointer has
|
||
no real hover - tapping the bar fires a synthetic `pointerenter` with
|
||
no `pointerleave` ever coming after it (there's no cursor left to lift
|
||
away), so the very first tap anywhere on the bar latched
|
||
`isPointerOverBottomBar` permanently `true` and the bar could never
|
||
auto-hide again for the rest of that session. Fixed by checking
|
||
`event.pointerType === 'mouse'` in both handlers - touch/pen never even
|
||
attempt the "hovering holds it open" path at all, and correctly fall
|
||
back to the same plain activity-based wake-up (`pointerdown`/
|
||
`pointermove` on the whole player) every device already gets. Confirmed
|
||
by reproducing the exact failure directly - dispatched a synthetic
|
||
touch `pointerenter` with no matching `pointerleave` (the real-world
|
||
case) against the fixed logic and watched it still transition to
|
||
`.minimal` on schedule, where the original `mouseenter`-based version
|
||
would have latched open. A real touchscreen's own event behavior still
|
||
isn't independently reproducible here, so this is the strongest
|
||
confirmation available without one.
|
||
|
||
## Playlists: "Lire à partir d'ici"
|
||
|
||
Asked for directly: alongside "Lancer cette playlist" (always from the
|
||
first item), a way to start a playlist party from any chosen item.
|
||
`views/playlist/Playlist.ts`'s own top comment used to rule this out -
|
||
written back when the page was deliberately read-only, reasoning that
|
||
"start from item N, queue the rest" wasn't reconstructable there the
|
||
way `views/series` does it for episodes, since (unlike an Episode's own
|
||
`SeriesId`) a playlist item carries no back-reference to whichever
|
||
playlist it came from.
|
||
|
||
That reasoning was correct for a bare item id on its own - but doesn't
|
||
actually block this page specifically: `renderPlaylist` already fetches
|
||
the *entire, ordered* item list for `getPlaylistItems` to render the
|
||
list at all, so "item N of playlist P" only ever needed playlist P's
|
||
own already-in-hand list, never a back-reference on the item itself.
|
||
views/library's tiles and views/album's per-track rows still can't do
|
||
this (neither has that ordered list around) - only this exact page can,
|
||
which is why the feature lives here and nowhere else.
|
||
|
||
`createParty(itemId, startAtItemId?)` (`client/src/api/party.ts`) gained
|
||
the optional second argument, threaded through to `POST /api/parties`'s
|
||
body and on into `createPartyFromPlaylist` (`index.ts`) - meaningless
|
||
(silently ignored) for every other item type, so every other existing
|
||
call site keeps working unchanged with just one argument.
|
||
`createPartyFromPlaylist` resolves it against the playlist's own
|
||
(post-`splitPlaylistByMediaType`) item list and slices from there -
|
||
`allItems.findIndex(...)` then `.slice(startIndex)`, the exact same
|
||
"slice, don't wrap" shape `POST /api/parties`' own single-item path
|
||
already uses for `item.SeriesId`-based episode starts, just applied to
|
||
a playlist's item list instead of a series' episode list. Everything
|
||
before the chosen item is simply left out of this party, not queued
|
||
for later - true for both video (the fixed queue is built from the
|
||
sliced list) and music (the sliced list's own first item seeds the
|
||
queue, the rest is added after it, same as launching a playlist always
|
||
worked, just from a different starting point).
|
||
|
||
Client-side, each row in `#itemList` gets its own "Lire à partir d'ici"
|
||
button now (`.startFromBtn`, mirroring `views/series`' own
|
||
`.startEpisodeBtn` - reuses `.episodeRow`'s existing button styling/
|
||
responsive wrap with no new CSS at all), alongside the unchanged
|
||
"Lancer cette playlist" button at the top. Both funnel through one
|
||
shared `launch()` helper - only the button, its busy/idle label text,
|
||
and whether a starting item id is passed differ; create-then-dispatch-
|
||
on-mediaType-then-error-handling is identical either way.
|
||
|
||
Verified: full `typecheck`/`test`/`build` on both projects (118/118
|
||
server tests - no dedicated test added for `createPartyFromPlaylist`
|
||
itself, consistent with this project's existing boundary that route
|
||
glue in `index.ts` isn't unit-tested, only the services it calls into
|
||
are). Confirmed in a real browser against the compiled bundle that the
|
||
new per-row button renders correctly alongside the existing controls,
|
||
at both desktop and phone width (the phone-width wrap is the same
|
||
`@media` rule `views/series` already relies on, untouched here).
|
||
|
||
## Casting broke after the first segment: the rewritten playlist URLs never carried the castToken
|
||
|
||
Reported directly: "for cast, the .m3u8 needs to be passed instead of a
|
||
raw [url]". The master playlist itself was never the problem - `POST
|
||
.../cast-token` mints the token, and the initial `master.m3u8?castToken=...`
|
||
request already carries it. What broke immediately after: every URI
|
||
line inside that playlist gets rewritten (`EmbyClient.rewritePlaylist`)
|
||
to route back through this app's own `party-stream/:partyId/raw?u=...`
|
||
proxy instead of hitting Emby directly - and that rewrite never carried
|
||
the castToken forward at all. A browser doesn't notice, because it's
|
||
still sending its session cookie on every one of those follow-up
|
||
requests regardless of what's in the URL. A Chromecast has no cookie to
|
||
fall back on (see `CastTokenService`) - so its segment/variant-playlist
|
||
fetches hit `sessionForStream` with no auth of any kind and 401'd,
|
||
breaking playback moments after the receiver appeared to connect.
|
||
|
||
Fixed by threading the *same* castToken the master-playlist request
|
||
itself carried into every rewritten line - `rewritePlaylist(text,
|
||
routeKey, castToken?)`, sourced from `req.query.castToken` right inside
|
||
`proxyResponse` (which already receives `req`, so no new parameter
|
||
needed anywhere upstream of it). Omitted entirely when there wasn't one
|
||
(the normal browser/cookie case - unaffected either way, since a
|
||
browser was never relying on this).
|
||
|
||
The self-hosted burn-in path (`TranscodeService`) had the exact same
|
||
gap for a different reason: ffmpeg's own HLS muxer writes bare relative
|
||
segment filenames into its `master.m3u8`, which a browser resolves
|
||
against the request's own URL (keeping its cookie without even
|
||
noticing) but which - per ordinary URL-resolution rules, not a bug in
|
||
ffmpeg or this app - silently drops any query string when resolved
|
||
against a base URL that had one. Fixed the same way: a new pure
|
||
`appendCastTokenToSegments(text, castToken)` (extracted specifically so
|
||
this could be unit-tested directly, unlike `serveMasterPlaylist` itself
|
||
which needs a real ffmpeg session and on-disk files to exercise at all)
|
||
appends `?castToken=...` to each segment line when a request arrives
|
||
with one; the plain `fs.createReadStream(...).pipe(res)` fast path is
|
||
untouched when it doesn't.
|
||
|
||
Verified: full `typecheck`/`test`/`build` (122/122 server tests,
|
||
including new coverage for `rewritePlaylist`'s token propagation - had
|
||
no test at all before this, for the rewriting itself or the token fix -
|
||
and for `appendCastTokenToSegments`). **Not verified**: an actual
|
||
Chromecast fetching real rewritten segment URLs - no device reachable
|
||
from this sandbox, same standing limitation as the rest of "Casting to
|
||
a Chromecast" above.
|
||
|
||
## The player toolbar still wasn't auto-hiding: a heartbeat and its own hide-timer, both every 3s
|
||
|
||
Reported right after the mobile touch fix: "the toolbar still doesn't
|
||
work, mobile and PC" - meaning the *other* auto-hide fix hadn't
|
||
actually landed for anyone, not just touch devices. Root cause this
|
||
time: an actual bug in this session's own previous edit, not a browser
|
||
quirk. `becomeHost`'s 3-second heartbeat (`sendLocalState`, needed
|
||
regardless of any real activity, so a follower's periodic resync never
|
||
goes stale) got rewired to call the same `onDriverStateChanged` the
|
||
native `<video>`'s real play/pause/seeked events use - which also
|
||
re-arms the auto-hide countdown (`scheduleControlsHide`), on the
|
||
reasoning that resuming from pause needed to restart it. That reasoning
|
||
was correct for a genuine discrete event; the heartbeat is not one - it
|
||
fires unconditionally every 3s whether or not anything changed. With
|
||
`CONTROLS_HIDE_DELAY_MS` *also* 3000ms, every single heartbeat tick
|
||
cancelled and restarted the hide countdown from zero, on the same
|
||
cadence the countdown itself needed to run uninterrupted just to reach
|
||
its own deadline - so for a host, on any platform, the bar could get
|
||
re-armed indefinitely and effectively never complete a hide. A follower
|
||
(no heartbeat of their own) was never affected by this specific bug -
|
||
only by the earlier touch one.
|
||
|
||
Fixed by giving the heartbeat its own function, `reportHeartbeat` -
|
||
same state sync (button icon, seek bar, `sendLocalState`) as
|
||
`onDriverStateChanged`, deliberately *without* the
|
||
`scheduleControlsHide` call. Every genuine discrete event (native
|
||
video play/pause/seeked, a cast session's remote-state changes, a
|
||
manual seek) still goes through `onDriverStateChanged` unchanged and
|
||
still correctly re-arms the countdown.
|
||
|
||
Verified: full `typecheck`/build. Confirmed directly (not by racing two
|
||
real timers against each other, which only proves something over an
|
||
observation window long enough to be impractical here) that 3 simulated
|
||
heartbeat ticks call the hide-timer's reset function zero times, while
|
||
one simulated discrete event still calls it exactly once - the precise
|
||
behavioral difference the fix is supposed to produce.
|
||
|
||
## 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.
|