New mediaSession.ts wraps the standard Media Session API - shared by Player.ts and MusicBar.ts, which can never both be active at once so there is no risk of them fighting over this one global. Metadata/ position/playback-state hook into each players own existing single source of truth (updatePlayPauseBtn/updateProgressBar, setPlayIcon/ updatePositionDisplay) instead of adding parallel call sites. Play/ pause/seek are wired once and read isHost/conn fresh on every OS-triggered call, so they keep working through a live host handoff; a non-host play/pause routes through the same requestPause/ requestResume the request toolbar already uses, and seek is gated on isHost the same way the in-page seek bar already is. Next is handled separately since a registered nexttrack handler is what makes the OS show that control at all - its presence tracks whatever the in-page equivalent already tracks (isHost && hasNextQueued for video, a vote skip for music). Cleared entirely on leaving a party/page, or just to idle (metadata/state only) when a listen partys queue runs empty but the party itself is still open. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
4461 lines
263 KiB
Markdown
4461 lines
263 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 for v1** - the spec's every concrete reference
|
||
(the player, the party-stream/burn routes, the burn-in fallback) was
|
||
video-specific, and `MusicBar.ts` had no comparable control surface at
|
||
the time. Extended to music too shortly after, once asked for directly -
|
||
see "Casting a listen party" below for what that actually took (turned
|
||
out to be almost entirely client-side: every piece of server support
|
||
below - `CastTokenService`, the 4 streaming routes' `sessionForStream`
|
||
fallback - was already media-type-agnostic, scoped to a `partyId`, never
|
||
a video-specific assumption).
|
||
|
||
**`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.
|
||
|
||
## Casting a listen party
|
||
|
||
Asked for directly, after the video case above already shipped and was
|
||
confirmed working end to end: let a listen party cast too. The server
|
||
side needed nothing new at all - `CastTokenService`, the two
|
||
`cast-token` routes, and `sessionForStream`'s fallback on the 4 streaming
|
||
routes were all already keyed on nothing more specific than `partyId`;
|
||
`partyStreamUrl(partyId)` was already exactly what `musicBar/MusicBar
|
||
.ts`'s own `<audio>` pointed at. This turned into an almost entirely
|
||
client-side feature: `cast.ts`'s `localDriver` only needed its parameter
|
||
type widened from `HTMLVideoElement` to `HTMLMediaElement` (every member
|
||
it reads - `currentTime`/`paused`/`duration`/`play()`/`pause()` - is
|
||
already on the shared `HTMLMediaElement` base both elements implement),
|
||
and `MusicBar.ts` grew the same `activeDriver` indirection `Player.ts`
|
||
already has.
|
||
|
||
The one genuine design question was host-gating, and the answer is that
|
||
there isn't any to port: a listen party has no host at all (see "One
|
||
shared Emby stream per party" below) - any member's play/pause already
|
||
applies party-wide with no arbitration, the same symmetric model this
|
||
extends unchanged. Casting a track is exactly as symmetric: whoever
|
||
starts one is just routing their own copy of the already-shared audio
|
||
through a TV/speaker instead of this tab, same as plugging in
|
||
headphones would be, and the Chromecast's own on-screen controls feed
|
||
back into `setMusicPaused` exactly like the local `<audio>`'s native
|
||
play/pause events already do.
|
||
|
||
The other real difference from the video case: a video party's item
|
||
changes rarely enough that casting simply drops back to local playback
|
||
on any reload (see above) - "re-cast, one click" was an acceptable cost
|
||
there. A listen party's queue advances far more often, so dropping the
|
||
cast on every track would have made this a much worse feature than not
|
||
having it. Instead, a track change while casting calls the exact same
|
||
`startCast()` again, reusing the *already-connected* session -
|
||
`requestSession()` resolving immediately for a session that's still
|
||
open, no fresh device picker, is exactly what this needs and the video
|
||
path never had reason to exercise. The one wrinkle: `partyStreamUrl`
|
||
never encodes which track is playing (the server alone decides that),
|
||
so reusing the identical URL string for a new load gained a `_=Date.now()`
|
||
cache-buster - without it, a receiver that cached the previous track's
|
||
manifest would have had no reason to ever re-fetch it. If a re-cast
|
||
throws (the TV dropped mid-party, a network hiccup), it falls back to
|
||
local playback rather than stranding the party silently stuck - the same
|
||
recovery instinct `loadTrack`'s existing try/catch around hls.js already
|
||
has for the same reason.
|
||
|
||
Leaving the party while casting ends the Chromecast session outright
|
||
(`stopCast()`, not just a driver swap back to local) - unlike Player.ts's
|
||
route cleanup, which deliberately leaves a cast running when you merely
|
||
navigate away from the page, actually leaving a listen party means there
|
||
is no more shared state left to cast at all.
|
||
|
||
Verified: full `typecheck`/`build` on the client (this app's client has
|
||
no test runner - see `client/package.json` - so client-side logic here
|
||
is typecheck+build+code-review only, consistent with the rest of this
|
||
codebase's client/server verification split). **Not verified**: no real
|
||
Chromecast or Emby login reachable from this sandbox, same standing gap
|
||
as the video case above - the actual re-cast-on-track-change round trip
|
||
needs a real device to confirm end to end.
|
||
|
||
## The catalog API being slow, and the front freezing while it loads
|
||
|
||
Reported directly, still true after an earlier round already fixed
|
||
image caching (see `EmbyClient.proxyResponse`'s own comment on that).
|
||
Two separate, compounding causes, both traced by reading the actual
|
||
request shapes rather than guessing:
|
||
|
||
`CatalogService.getLibraryItems` - the one query behind a whole library
|
||
grid, up to 300 items - asked Emby for `Overview` on every item and
|
||
never used it: `views/library`'s tiles only ever render a poster, name,
|
||
and year (`tiles.ts`'s `tileHtml`). Emby happily serializes a full
|
||
synopsis for all 300 anyway, and the client happily `JSON.parse`s all of
|
||
it, purely to discard it unread - real, wasted latency on both the Emby
|
||
round trip and the response parse. Dropped from that query's `Fields`
|
||
(kept where it's actually shown - `getSeriesEpisodes`/
|
||
`getPlaylistItemsRaw`, read by `views/series`/`views/playlist`).
|
||
`getArtists` (a music library's own top-level grid) had no `Limit` at
|
||
all, unlike every sibling query - given a large enough library, an
|
||
unbounded artist list compounds the second cause below without even
|
||
Emby's own side being slow.
|
||
|
||
The literal "freeze" was separate from raw latency: `tiles.ts`'s shared
|
||
render path built one HTML string for an entire batch of items and
|
||
assigned it via a single `innerHTML` write - fine for a home carousel
|
||
(`Limit: 16`), but a real, felt hitch on slower devices for a library
|
||
grid's up-to-300 (or previously-unbounded artist list). Fixed with
|
||
`renderTilesInChunks` - the same tiles, the same `tileHtml`, just
|
||
inserted 40 at a time across `requestAnimationFrame` callbacks instead
|
||
of one synchronous pass, so no single task blocking the main thread
|
||
grows with the list size. The one thing this couldn't stay naive about:
|
||
`wireTileButtons` used to attach one `click` listener per button at
|
||
call time, which would have silently never covered a tile that hadn't
|
||
rendered yet - switched to one delegated listener on the container
|
||
instead, which correctly covers whatever's in the DOM at click time
|
||
regardless of when it was inserted.
|
||
|
||
Verified: full `typecheck`/`test`/`build` on both projects (server
|
||
tests unaffected - neither changed query has a test asserting its exact
|
||
`Fields` string). The chunked-render mechanism itself was verified
|
||
directly - a synthetic 300-item list rendered against a real DOM in the
|
||
browser confirms every item lands and that a button in the *last* chunk
|
||
is still clickable through the delegated listener (the exact failure
|
||
mode a naive per-button-at-call-time wiring would have hit). **Not
|
||
verified**: no Emby login reachable from this sandbox to measure the
|
||
actual before/after latency against a real, large library.
|
||
|
||
**Correction, reported immediately after this shipped**: "the catalogs
|
||
are broken, not all the data is there". The `Limit: 300` both queries
|
||
above carried (`getArtists` gained one here; `getLibraryItems` already
|
||
had one, predating this section) turned out not to be the safe,
|
||
theoretical headroom it was assumed to be - confirmed directly against
|
||
the real server that this app's own "Musique" library has **1229**
|
||
distinct artists and "Films" has **421** movies, so both were silently
|
||
serving an incomplete grid, no error, no visible sign anything was
|
||
missing. Both `Limit`s removed outright, not raised to some other
|
||
guessed number - confirmed empirically that Emby returns everything
|
||
when `Limit` is omitted at all (`TotalRecordCount` matched
|
||
`Items.length` exactly for both the 421- and 1229-item cases), and the
|
||
render-side concern that motivated capping in the first place is
|
||
already `renderTilesInChunks`'s job regardless of how large the list
|
||
gets. That fix's own scheduling primitive needed a second look too:
|
||
`requestAnimationFrame` is deliberately paused by the browser for a
|
||
backgrounded/hidden tab - an ordinary thing to do while a now-possibly-
|
||
1000+-item list is still loading, and would have stalled the render
|
||
indefinitely, looking exactly like the same "data missing" symptom for
|
||
an unrelated reason. Switched to `setTimeout(fn, 0)`, which keeps
|
||
running (throttled, never paused) regardless of tab visibility.
|
||
|
||
## 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.
|
||
|
||
**Follow-up, reported later**: the queue popup should span the bar's
|
||
full width, not sit in a fixed 480px column, and scrolling a long queue
|
||
scrolled its "File d'attente" header away with it. Both were the same
|
||
kind of oversight as the original layout bug above - CSS carried over
|
||
from when this was a small popup, not re-examined once the row shape
|
||
changed. The width cap (`.musicBarQueue`'s `max-width: 480px`) was
|
||
simply dropped; the scroll region was the real fix - `overflow-y`/
|
||
`max-height` sat on `.musicBarQueue` itself, which wraps *both*
|
||
`.pageHeader` and `#queueList`, so scrolling the list scrolled the
|
||
header with it. Moved the scroll region down to `#queueList` alone
|
||
(`.musicBarQueue` becomes a plain flex column, header first with
|
||
`flex-shrink: 0`), the standard sticky-header-over-scrollable-body
|
||
split - the header now stays exactly where clicking "File d'attente"
|
||
put it regardless of queue length.
|
||
|
||
## 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.
|
||
|
||
## Cast still didn't play at all: no CORS on the streaming routes
|
||
|
||
Reported again after the castToken-propagation fix above: "still
|
||
doesn't work, the stream doesn't play on the TV side." That fix was
|
||
real, but not the whole story - it addressed what would break *after*
|
||
the first segment; this was more fundamental, blocking things from the
|
||
very first request.
|
||
|
||
The Default Media Receiver plays this app's HLS through an MSE-based
|
||
player (Chromecast's own Chromium build has no native HLS support to
|
||
fall back on) - which fetches every segment via `fetch`/XHR from
|
||
whatever origin Google hosts that receiver at. Cross-origin, from this
|
||
app's own server. This app never sent a single CORS header anywhere -
|
||
never had to, every consumer until now was this app's own same-origin
|
||
page. Without one, the browser blocks every one of those cross-origin
|
||
reads outright, before any of this app's own cookie/castToken check
|
||
even gets a chance to run - the master playlist request itself included.
|
||
|
||
Fixed with a small `allowCastCors` middleware on the same 4 streaming
|
||
routes `sessionForStream` already covers (`party-stream/:partyId/
|
||
master.m3u8`/`raw`, `burn/:partyId/master.m3u8`/`:segment`) -
|
||
`Access-Control-Allow-Origin: *`, plus handling `OPTIONS` for the same
|
||
4 paths (a real cross-origin `Range` header - needed for byte-range/
|
||
segment fetches - isn't itself CORS-safelisted, so it triggers a real
|
||
preflight, which this app previously had no answer for at all). `*` is
|
||
safe specifically because these routes never need to support
|
||
*credentialed* cross-origin access: a real browser already reaches them
|
||
same-origin (no CORS involved), and the one genuine cross-origin caller
|
||
authenticates via the castToken query param, never a cookie -
|
||
`Access-Control-Allow-Origin: *` combined with
|
||
`Access-Control-Allow-Credentials` would be required before a cookie
|
||
could ever ride along cross-origin, and this deliberately never sends
|
||
that second header at all.
|
||
|
||
Verified: full `typecheck`/`test`/`build`. Confirmed against the real
|
||
running server that an `OPTIONS` preflight on these routes now answers
|
||
204 with the right three headers, that a real (still-401, no token
|
||
given) `GET` still carries `Access-Control-Allow-Origin` regardless of
|
||
outcome (so a cross-origin caller's own JS can actually read that
|
||
response rather than seeing an opaque failure), and that an unrelated
|
||
route (`/api/auth/me`) carries no such header at all - this stays
|
||
scoped to exactly the 4 routes a Chromecast needs. **Not verified**:
|
||
whether this is now sufficient for a real receiver to actually decode
|
||
and play the stream end to end - no device reachable from this
|
||
sandbox, same standing limitation as the rest of "Casting to a
|
||
Chromecast".
|
||
|
||
## A sticky search bar, and letting anyone seek within a playing track
|
||
|
||
Two small, unrelated requests handled together.
|
||
|
||
**The search bar** on `views/library`/`views/artist`/`views/album`
|
||
(`tiles.ts`'s shared `.searchBar`) scrolled away with the page - asked
|
||
to stay reachable while scrolling a long grid instead. `position:
|
||
sticky; top: 0` on the input itself, not a wrapper - `base.css`'s own
|
||
generic `input` rule already gives it an opaque `var(--panel)`
|
||
background, which is what actually keeps tiles scrolling underneath
|
||
from showing through its bounds once stuck; nothing else needed
|
||
changing.
|
||
|
||
**Seeking within the current music track** was explicit, deliberate
|
||
scope this session's own "participative playlist" work had cut - the
|
||
progress bar was marked read-only specifically because, at the time,
|
||
`MusicQueueService` had "no `seek` concept... just play/pause/advance".
|
||
Asked for directly now. The permission model was already decided,
|
||
though, by that same class's own documented reasoning for why *anyone*
|
||
(not a vote, not a host) can pause/resume: it's reversible and doesn't
|
||
change what's playing - a seek is exactly that same shape, just
|
||
continuous instead of binary, so it gets the identical treatment, not
|
||
`voteSkip`'s majority gate (which changes *what's* playing).
|
||
|
||
`MusicQueueService.seek(party, positionSeconds)` mirrors `setPaused`'s
|
||
own elapsed-time math (clamped into `[0, durationSeconds]` when known),
|
||
with one real difference `setPaused`'s resume branch never has to deal
|
||
with: `scheduleAdvance` only ever *sets* `advanceTimer`, it never clears
|
||
a prior one - resuming gets away with calling it directly only because
|
||
pausing already cleared the timer first. Seeking while still playing
|
||
has no such guarantee (the old timer, counting down against the *old*
|
||
position, is still live) - skipping the clear first would leave it
|
||
running alongside the new one, eventually firing `playNext` at the
|
||
wrong moment. New `musicSeek` socket event, no host check (same as
|
||
`musicPlaystate`). Client-side, `musicBar/MusicBar.ts`'s progress track
|
||
gained the same pointerdown/move/up drag-to-seek handlers the video
|
||
player's own seek bar already has - no host branch to port over this
|
||
time, since a listen party has none - and a local, optimistic
|
||
`activeDriver.currentTime` write before broadcasting, for the same
|
||
instant-feedback reason the video player's own `seekFromPointerEvent`
|
||
does that.
|
||
|
||
Verified: full `typecheck`/`test`/`build` on both projects (128/128
|
||
server tests - new coverage for `seek`'s clamping, its paused-vs-playing
|
||
branches, and specifically a real-timer test proving the old advance
|
||
timer is cleared rather than left running alongside a re-armed one, the
|
||
exact bug the naive "just call scheduleAdvance again" version would
|
||
have shipped). No server is needed to see the sticky search bar itself,
|
||
so that was checked by reading `.mainContent`'s own layout directly
|
||
(no nested scroll container between it and the document, confirming
|
||
`position: sticky` resolves against the page scroll as intended, not
|
||
some inner element).
|
||
|
||
## Skipping the intro/end credits, using Emby's own native detection
|
||
|
||
Asked for directly: propose skipping a TV episode's opening/end credits,
|
||
Netflix-style. Checked directly against the real server before building
|
||
anything, rather than assuming a data source existed - no plugin
|
||
provides this (the installed "Cinema Intros" plugin is a different
|
||
feature entirely - pre-show trailers, not per-episode skip points), and
|
||
the embedded `Chapters` Emby already returns turned out to be
|
||
auto-generated placeholders every 5 minutes ("Chapitre 1", "Chapitre
|
||
2", ...), not real markers.
|
||
|
||
What the server DOES have, confirmed from its own `/openapi.json`
|
||
rather than assumed: a native, per-library `EnableMarkerDetection`
|
||
option (`Library/VirtualFolders`'s `LibraryOptions`) backed by a
|
||
built-in "Detect Episode Intros" scheduled task - audio-fingerprint
|
||
detection, the same idea as Jellyfin's separate Intro Skipper plugin,
|
||
just already part of Emby itself. Once enabled and run, it adds real
|
||
entries to the exact same `Chapters` array, distinguished from the
|
||
auto-generated ones by `MarkerType`: `IntroStart`/`IntroEnd`/
|
||
`CreditsStart` (confirmed from the schema, not guessed - there is no
|
||
`CreditsEnd`, since credits are assumed to run to the end of the file).
|
||
|
||
Given that, this feature is almost entirely "read what Emby already
|
||
computed and expose it" - no fingerprinting, no new persistence, no
|
||
background jobs of this app's own. `CatalogService.getPlaybackMarkers`
|
||
fetches `Fields=Chapters` for an item and picks out just those three
|
||
marker types (nulls when detection hasn't reached this item, or isn't
|
||
enabled for its library - never an error), behind a new
|
||
`GET /api/catalog/item/:id/markers` route, mirroring
|
||
`getPlaybackOptions`'s own shape exactly.
|
||
|
||
**The player side** adds one new floating button, `#skipIntroBtn` -
|
||
explicit label ("Passer l'intro"), not an icon, matching this app's own
|
||
established convention for every request-shaped control. It's
|
||
positioned as a sibling of `#bottomBar`, not inside it, specifically so
|
||
it stays reachable through the bar's own auto-hide/`.minimal` collapse -
|
||
someone who hasn't touched the player still needs to see it the moment
|
||
the intro starts. `updateSkipButton` (ticked every 500ms alongside
|
||
`updateProgressBar`) compares `activeDriver.currentTime` against the
|
||
fetched markers and shows/hides it; clicking it seeks straight to
|
||
`introEndTicks` through the exact same `activeDriver`/`onDriverStateChanged`
|
||
path any other seek already uses, so it's automatically host-only (a
|
||
follower's seek never broadcasts - see `sendLocalState`'s own gate) and
|
||
automatically cast-aware (works identically whether `activeDriver` is
|
||
the local `<video>` or a Chromecast's `RemotePlayer`) with no new
|
||
special-casing for either.
|
||
|
||
End credits deliberately got no button of their own: `#nextBtn` already
|
||
does exactly "go to the next queued item" and was already shown
|
||
whenever `hasNextQueued` is true, for the whole episode - not something
|
||
this feature needed to duplicate. The only real gap it closes is
|
||
*visibility*: if nobody's touched the player in a while, `#bottomBar`
|
||
(and `#nextBtn` inside it) is collapsed to `.minimal` right when
|
||
credits start. `updateSkipButton` calls the existing `showControls()`
|
||
once (a `creditsControlsShown` flag keeps it one-shot, not a fight
|
||
against someone deliberately re-minimizing the bar during a long
|
||
credits scroll) the moment playback crosses `creditsStartTicks`, so the
|
||
already-existing button is actually reachable without requiring a mouse
|
||
nudge first.
|
||
|
||
Verified: full `typecheck`/`test`/`build` on both projects (125/125
|
||
server tests, including new coverage for `getPlaybackMarkers` - the
|
||
real `MarkerType` values mixed with auto-generated `Chapter` ones, and
|
||
both "no Chapters field" and "no markers among the Chapters that are
|
||
there" resolving to nulls rather than throwing). The button's on-screen
|
||
position was checked directly (`getBoundingClientRect` against the real
|
||
compiled CSS, not eyeballed) - it sits clear of `#bottomBar` with a
|
||
10px gap at both a 960×540 and a phone-width viewport, since both use
|
||
fixed pixel offsets off the same `.playerWrap`. **Not verified**: the
|
||
detection task had only just been enabled on the real server (confirmed
|
||
via its own `EnableMarkerDetection`/`ScheduledTasks` state) and hadn't
|
||
finished analyzing any episode yet at the time this shipped, so the
|
||
actual end-to-end "does a detected marker make the button appear at
|
||
the right second" behavior is unconfirmed against real generated data -
|
||
worth a real check once the task has had time to run.
|
||
|
||
## Making Emby's own admin dashboard see watch-party streams
|
||
|
||
Asked for directly: Emby's admin "now playing" dashboard should see
|
||
this app's streams the same way it sees a real web/mobile/TV client's -
|
||
this is exactly the follow-up "Sync watch-party progress back to Emby"
|
||
(above) already anticipated and deliberately deferred, word for word:
|
||
"wiring clean start/stop into every transition... is real complexity
|
||
for a secondary payoff, left as a documented follow-up rather than
|
||
built speculatively." That section also already explains why: this app
|
||
switched entirely to direct `UserData` writes for resume-position/
|
||
watched-history, abandoning Emby's session-shaped `PlayingItems`
|
||
endpoints, precisely because they need a real session lifecycle to mean
|
||
anything - and given that section's own history (two separate rounds of
|
||
a wrong-but-schema-plausible request shape for that *other*,
|
||
now-abandoned endpoint family), this one was built carefully, checking
|
||
the real API before writing anything rather than assuming.
|
||
|
||
**Confirmed against the real server's own `/openapi.json`** (not
|
||
assumed) that the actual session-lifecycle family is `POST
|
||
/Sessions/Playing` (start), `/Sessions/Playing/Progress`, and
|
||
`/Sessions/Playing/Stopped` - distinct from the `PlayingItems` family
|
||
`PlaystateService` already walked away from. None of the three schemas
|
||
declare a `required` list at all, which - given this exact neighboring
|
||
history - was read as "don't trust that," not "everything's optional
|
||
in practice": `ItemId`/`MediaSourceId`/`PlaySessionId`/`PositionTicks`
|
||
are sent on every call anyway, matching what a real client obviously
|
||
would, rather than risking a repeat of the earlier "technically matches
|
||
the schema, still 400s" experience.
|
||
|
||
**One real Emby session per PARTY MEMBER, not one per party** - a
|
||
faithful copy of how a real client behaves (every device/account gets
|
||
its own), and the actual point of "let admin see who's watching what"
|
||
would be lost by collapsing every member onto one shared entry. New
|
||
`server/src/services/EmbySessionService.ts` (`start`/`progress`/`stop`)
|
||
mirrors `PlaystateService`'s own shape closely - same best-effort
|
||
philosophy (every failure caught and logged, never thrown), same
|
||
`Logger.playstate` category (broadened to describe both) - but is kept
|
||
as its own class rather than folded into `PlaystateService`, since that
|
||
class's entire opening comment is specifically about why it *doesn't*
|
||
use a session-shaped endpoint; mixing the two would muddy exactly the
|
||
distinction that comment goes out of its way to draw.
|
||
|
||
**Getting a `MediaSourceId`/`PlaySessionId` per member** needed a new
|
||
`CatalogService.negotiatePlaybackSession(session, itemId)` - a fresh
|
||
`PlaybackInfo` negotiation under that specific member's own account.
|
||
This can't reuse `PartyStreamService`'s existing shared negotiation:
|
||
that one is negotiated once, under whichever member happened to
|
||
trigger it first, and Emby scopes `PlaybackInfo` per user - never
|
||
guaranteed valid for reporting under everyone else's own account too.
|
||
Returns `null` on any failure rather than throwing (an expired token,
|
||
Emby unreachable, no source) - unlike `getMediaSource`'s own hard
|
||
failure (which feeds a real ffmpeg source and has to fail loudly), this
|
||
one only ever feeds an admin-dashboard nicety.
|
||
|
||
**Driven entirely from index.ts's existing periodic sweep**, not new
|
||
hooks scattered across every queue-advance/track-skip call site the way
|
||
`PlaystateService.markPlayed` needed. That sweep already visits every
|
||
connected member on every ~20s tick - exactly positioned to notice
|
||
"this member's current item no longer matches `PartyMember
|
||
.embySession.itemId`" and react: stop the old session if there was
|
||
one, negotiate fresh ids, start the new one. The same item as last tick
|
||
just sends a Progress report reusing the already-cached ids - no
|
||
renegotiation needed for the common case. `PartyMember` gained the
|
||
nullable `embySession` field this reads/writes (same null-until-known
|
||
shape as `embyToken`/`deviceId`). Explicit `stop` calls also happen at
|
||
the two places a member's session should end promptly rather than
|
||
waiting on the next sweep tick to just stop reporting: `PartyService
|
||
.leave()` (alongside the existing `CastTokenService.revokeForMember`)
|
||
and `removeIdleParties` (a defensive backstop for a member whose own
|
||
disconnect-driven `leave()` never fired for some reason - a real gap
|
||
would otherwise leave a stale dashboard entry indefinitely, not just
|
||
until the next tick).
|
||
|
||
Verified: full `typecheck`/`test`/`build` (140/140 server tests - new
|
||
coverage for `negotiatePlaybackSession`'s null-on-any-failure
|
||
contract, every `EmbySessionService` method's exact request shape and
|
||
that a non-ok response/thrown error is swallowed not propagated, and
|
||
that `leave()` calls `stop` with a member's real cached session info
|
||
but only when they actually had one open). **Not verified**: an actual
|
||
live write against the real server - deliberately not attempted from
|
||
here, since doing so would mean using a real account's Emby token for
|
||
a genuine state-changing call, beyond the read-only diagnostic use
|
||
this session's shared API key was ever meant for. So while the request
|
||
shape is checked against the real schema and the per-member logic is
|
||
unit-tested in full, whether Emby's dashboard actually displays these
|
||
sessions the way a native client's would - correct device/client
|
||
labeling in particular, since this app never sends the `X-Emby-
|
||
Authorization` Device/Client/Version header a real client would, only
|
||
`X-Emby-Token` - is genuinely unconfirmed. Worth watching the real
|
||
dashboard the first time a party runs after this ships.
|
||
|
||
## Picture-in-picture
|
||
|
||
Asked for directly, mobile in particular: a phone has no second window
|
||
to put this app in the way a desktop might, so the standard browser
|
||
Picture-in-Picture API - popping the `<video>` out into a small,
|
||
OS-managed floating window - is the one way to keep watching while
|
||
switching to another app. Standard `HTMLVideoElement
|
||
.requestPictureInPicture()`/`document.exitPictureInPicture()`, not the
|
||
Chromecast-adjacent `disableremoteplayback` trick or anything WebKit-
|
||
prefixed - confirmed directly in a real Chromium build that
|
||
`document.pictureInPictureEnabled`/`requestPictureInPicture`/
|
||
`exitPictureInPicture` are all present, which is what actually matters
|
||
for Android Chrome, the mobile browser this was asked for.
|
||
|
||
New `#pipBtn`, right next to `#fullscreenBtn` (`.bottomBarZone.right`)
|
||
- both are "how is this displayed" toggles, so they share the same
|
||
`.fsBtn` sizing, and a new `.fsBtn[aria-pressed="true"]` rule gives PiP
|
||
the identical "clearly on" orange treatment `#castBtn` already has
|
||
(kept as its own rule rather than combined onto `.iconBtn` - that class
|
||
and `.fsBtn` disagree on padding/border-radius/background, so
|
||
combining them risked an inconsistent size instead of a clean shared
|
||
look). Feature-detected once at mount (`document.pictureInPictureEnabled
|
||
&& !video.disablePictureInPicture`), not live the way `#castBtn`'s own
|
||
`onCastAvailabilityChanged` is - browser support for this doesn't
|
||
appear/disappear mid-session the way a Chromecast on the network does,
|
||
so there's nothing to react to after the first check. No new attribute
|
||
needed on `<video>` itself either - unlike `disableremoteplayback`
|
||
(added specifically to suppress Chrome's own auto-injected cast icon),
|
||
there's no equivalent native PiP affordance this app needs to
|
||
suppress.
|
||
|
||
State (`aria-pressed`) is kept in sync via the video's own
|
||
`enterpictureinpicture`/`leavepictureinpicture` events rather than the
|
||
button's own click handler alone - covers the browser's native PiP
|
||
window being closed directly (its own close control, or the OS), not
|
||
just a second click back in this app. Requires no cleanup on route
|
||
navigation: like every other per-mount element here, `<video>` itself
|
||
is destroyed and replaced by the next `root.innerHTML = html` (router.ts
|
||
runs this unconditionally on any navigation) - browsers already close
|
||
an active PiP window automatically once its source element leaves the
|
||
document, matching this app's very deliberate opposite choice for
|
||
casting (leaving an active Chromecast session running after
|
||
navigating away - see "Casting to a Chromecast" above): PiP has no
|
||
independent existence apart from the `<video>` element it mirrors, a
|
||
Chromecast session does.
|
||
|
||
Verified: full `typecheck`/`build` (client only - this is a pure
|
||
client-side feature, no server involved). Confirmed directly in a real
|
||
browser, not just type-checked: `document.pictureInPictureEnabled` is
|
||
`true` here, and running the exact show/hide + click-handling logic
|
||
against a real (sourceless) `<video>` correctly shows the button and
|
||
catches the resulting `InvalidStateError` from `requestPictureInPicture()`
|
||
without it becoming an uncaught exception - the same rejection a real
|
||
video would throw for a different reason (already in PiP, or the
|
||
document not focused), confirming the error handling path itself
|
||
rather than just assuming it. **Not verified**: an actual floating PiP
|
||
window on a real mobile browser, playing a real Emby stream - no
|
||
device/login reachable from this sandbox, same standing gap as this
|
||
project's other browser-native integrations (casting, the intro-skip
|
||
buttons).
|
||
|
||
## Queuing a whole album, or an artist's entire discography, in one action
|
||
|
||
Asked for directly: `views/album`'s and `views/artist`'s per-track/
|
||
per-album "Écouter ensemble"/"Ajouter à la file" buttons had no
|
||
whole-album or whole-discography equivalent - queuing an album meant
|
||
clicking every track in it one at a time, and an artist page had no
|
||
listen-party action at all beyond drilling into a specific album.
|
||
|
||
**Server-side resolution, not a client-side loop** - the same choice
|
||
already made for playlists (`createPartyFromPlaylist`), and for the
|
||
same reason: an artist's "entire discography" can genuinely be dozens
|
||
of albums and hundreds of tracks, and a client-side loop of individual
|
||
`addTrackToQueue` calls would mean that many sequential round trips
|
||
(slow, and non-atomic - a network hiccup partway through leaves a
|
||
partial queue) instead of one. New `resolveMusicTracksForItem(session,
|
||
item)` (`index.ts`) takes a raw Emby item already known to be
|
||
`Audio`/`MusicAlbum`/`MusicArtist` and resolves it to a flat, ordered
|
||
`MusicTrackInput[]` - a single track for `Audio`, every track in Emby's
|
||
own `IndexNumber` order for a `MusicAlbum`
|
||
(`CatalogService.getAlbumTracks`), every album's tracks (same order)
|
||
for a `MusicArtist`'s own albums (`getArtistAlbums`, itself already
|
||
sorted `ProductionYear,SortName` - i.e. chronological), fetched with
|
||
one `Promise.all` rather than sequentially. `POST /api/parties` gained
|
||
a `MusicAlbum`/`MusicArtist` branch, right alongside the existing
|
||
`Playlist` one, calling this and a new shared `createMusicPartyFromTracks`
|
||
- itself just `MusicQueueService.init` for the first track plus the new
|
||
`addTracks` (below) for the rest, extracted so `createPartyFromPlaylist`'s
|
||
own music branch could be simplified onto the exact same two calls
|
||
rather than keep its own separate per-track loop.
|
||
|
||
**`MusicQueueService.addTracks`** is the batch sibling of the existing
|
||
`addTrack` (both now share a `buildTrackEntry` helper for the actual
|
||
per-track shape) - same auto-starts-if-the-queue-was-empty behavior,
|
||
but one `queueUpdated` broadcast for the whole batch rather than one
|
||
per track. That distinction is what a naive "just call `addTrack` in a
|
||
loop" version would have gotten wrong: for a hundred-track discography,
|
||
that's a hundred intermediate broadcasts to everyone in the party for
|
||
what is, from a user's perspective, a single action.
|
||
|
||
**Adding to an already-live party's queue** needed its own new route,
|
||
`POST /api/parties/:partyId/queue` - given just an `itemId`, it resolves
|
||
it the same way (`resolveMusicTracksForItem`) and calls `addTracks`.
|
||
Kept separate from the existing `POST .../tracks`, which takes an
|
||
already-built track object a caller already has in hand (`views/album`'s
|
||
own per-track buttons, which never needed this resolution step at
|
||
all) - overloading that route to guess whether it received a full
|
||
track or a bare id to resolve would have been more surprising than a
|
||
second, clearly-named one.
|
||
|
||
**The buttons themselves** mirror `views/playlist`'s own "Lancer cette
|
||
playlist" exactly - same label-reflects-current-state-and-re-checks-on-
|
||
click discipline `views/album`'s per-track buttons already established,
|
||
same "attach the bar and stay on the page" behavior when starting fresh.
|
||
`views/album` gained one page-level button (`.playlistActions`, reused
|
||
from `Playlist.css` the same way `Album.css` already reuses `.seriesInfo`
|
||
from `Series.css` - one CSS bundle regardless of which view's file a
|
||
rule was first written in, so nothing needed duplicating). `views/artist`
|
||
gained its first listen-party action ever, sitting in `.pageHeader`
|
||
itself (its own `justify-content: space-between` already puts a second
|
||
child on the right of the title with no new CSS needed).
|
||
|
||
Verified: full `typecheck`/`test`/`build` on both projects (147/147
|
||
server tests - new coverage for `addTracks`: batch order preserved,
|
||
auto-start only fires for the batch's own first track when the queue
|
||
was empty, exactly one broadcast regardless of batch size, and a
|
||
harmless no-op for an empty batch). `resolveMusicTracksForItem`/
|
||
`createMusicPartyFromTracks` themselves are `index.ts` route glue, not
|
||
independently unit-tested - consistent with this project's existing
|
||
boundary that route-level orchestration isn't, only the services it
|
||
calls into are (same reasoning `createPartyFromPlaylist` was never
|
||
tested directly either). The two new buttons' layout was checked
|
||
visually against the real compiled CSS (a static render, real theme
|
||
tokens) rather than eyeballed. **Not verified**: an actual large
|
||
discography queuing correctly end to end against a real server - no
|
||
Emby login reachable from this sandbox.
|
||
|
||
## Progressive Web App
|
||
|
||
Asked for directly, as a follow-up to Picture-in-picture above: since
|
||
that feature's automatic-on-backgrounding variant turned out not to be
|
||
achievable (browsers require a *fresh* user gesture for
|
||
`requestPictureInPicture()` - confirmed empirically, not just from
|
||
docs, by calling it from a real `visibilitychange` listener in a real
|
||
Chromium tab and getting `NotAllowedError` even with
|
||
`navigator.userActivation.hasBeenActive === true` from an earlier real
|
||
click), a PWA install was asked for instead. **Worth being explicit
|
||
about, since it'd be easy to assume otherwise**: installing this app
|
||
does not change that outcome - the transient-activation requirement is
|
||
part of the Picture-in-Picture spec itself, not something tied to
|
||
being loaded in a browser tab versus an installed, `display: standalone`
|
||
window. The one Chrome feature that skips that requirement
|
||
(media-session-handler-based automatic PiP) stays desktop-only and
|
||
getUserMedia-gated regardless of install state, and nothing above
|
||
touches that gate. So the manual `#pipBtn` toggle remains the way to
|
||
enter PiP either way - a PWA install is its own, independently useful
|
||
feature here (an icon on the home screen, no browser chrome/URL bar
|
||
around the player, an OS-level app-switcher entry), not a workaround
|
||
for the previous section's limitation.
|
||
|
||
Three pieces, all standard and Chromium/Android-focused since that's
|
||
this app's stated mobile target (see Picture-in-picture above):
|
||
|
||
- **`client/src/public/manifest.webmanifest`** - `name`/`short_name`,
|
||
`display: "standalone"`, and `background_color`/`theme_color` copied
|
||
by hand from `base.css`'s `--bg`/`--bg-header` (a manifest can't read
|
||
CSS custom properties, so these two are duplicated on purpose - if
|
||
that palette ever changes, update both places). `start_url: "/"`,
|
||
`lang: "fr"` matching `index.html`'s own `<html lang="fr">`. No
|
||
`service worker` - confirmed against MDN's own installability
|
||
requirements that Chrome does not require one for the install
|
||
prompt, and this app has no real use for one: every page is either
|
||
live party/socket state or a proxied Emby catalog, and caching either
|
||
offline would mean showing stale library data or a stale party that's
|
||
already gone, not a working offline mode - so skipping it avoids a
|
||
whole stale-cache-invalidation problem this app doesn't need to
|
||
solve.
|
||
- **Icons** - hand-authored SVG (this project has no existing
|
||
logo/brand asset), rasterized to PNG at the sizes each consumer
|
||
needs (192/512 for the manifest, 180 for `apple-touch-icon`, 32 for
|
||
the browser-tab favicon) via a `<canvas>` in a real browser rather
|
||
than an image-editing tool, since that's what was available here.
|
||
Two source SVGs, not one: `icon.svg` fills the canvas (used for the
|
||
manifest's `"any"` purpose, the favicon, and `apple-touch-icon` -
|
||
none of those get OS-mask-cropped) and `icon-maskable.svg` scales
|
||
the same design down (~75%) so its farthest content point sits
|
||
inside Android's adaptive-icon safe zone (a circle of ~30.5% of the
|
||
icon's radius from center) - checked by computing the actual
|
||
farthest-point distance for both versions, not just eyeballed,
|
||
since a maskable icon that ignores that zone gets silently clipped
|
||
by circular-mask launchers.
|
||
- **`index.html`** - `<link rel="manifest">`, a `theme-color` meta
|
||
(Chrome/Android chrome), and the separate iOS-only tags Safari needs
|
||
instead of reading the manifest for the equivalent behavior
|
||
(`apple-touch-icon` link, `apple-mobile-web-app-capable`,
|
||
`apple-mobile-web-app-status-bar-style`, `apple-mobile-web-app-title`).
|
||
|
||
`build.mjs` gained one `cpSync(src/public, dist, { recursive: true })`
|
||
call alongside its existing `index.html` copy, rather than naming the
|
||
manifest and every icon individually - adding another icon later needs
|
||
no build.mjs change. `express.static` (already serving all of
|
||
`client/dist` unconditionally) picks up `manifest.webmanifest` and
|
||
`icons/*` with no server-side change at all.
|
||
|
||
Verified: full `typecheck`/`build` (client only). Confirmed directly
|
||
against a real running instance of this server, not just assumed:
|
||
`manifest.webmanifest` and all three manifest icons resolve `200` with
|
||
the correct `application/manifest+json`/`image/png` content types
|
||
(express's static middleware infers both from the file extension
|
||
correctly on its own), the `apple-touch-icon` link resolves `200`, the
|
||
generated PNGs are genuinely the declared dimensions (32/180/192/512),
|
||
and the page's own console shows no manifest-related warnings after
|
||
loading it. **Not verified**: an actual "Add to Home Screen"/install
|
||
prompt, or the installed app's standalone window, on a real device -
|
||
no device reachable from this sandbox, same standing gap as every
|
||
other browser-native integration in this project (casting, PiP, the
|
||
intro-skip buttons).
|
||
|
||
## Mobile navigation: a bottom sheet, not a reshaped sidebar
|
||
|
||
Asked for directly: the sidebar's old mobile treatment just reshaped
|
||
itself into a horizontally-scrolling top bar under 720px (a single
|
||
media query in Shell.css) - functional, but easy to miss (nothing
|
||
signals there's more to scroll to) and it buried "Se déconnecter" at
|
||
the far end of that scroll. Explored as a set of mockups first (four
|
||
patterns compared side by side, including the tradeoffs of each) before
|
||
writing any real code - the bottom sheet won out over a bottom tab bar
|
||
(too few slots for an unbounded number of Emby libraries), a hamburger
|
||
drawer (still the smallest code delta, but two taps for "Accueil" too),
|
||
and a merely-polished version of the existing horizontal bar.
|
||
|
||
**Structure mirrors the real data shape, not a flat list**: `getLibraries()`
|
||
already returns each library's Emby `CollectionType` as `collectionType`
|
||
on `LibraryDto` (`server/src/services/CatalogService.ts`) - confirmed by
|
||
reading that service directly rather than assumed. The sheet uses it to
|
||
render three zones instead of one flat list:
|
||
|
||
- **Accueil** - one fixed row, always first.
|
||
- **Bibliothèques** - a section label over a 2-column grid of every
|
||
library *except* the one whose `collectionType` is `'playlists'` -
|
||
this is the part whose count and contents genuinely depend on what
|
||
the connected Emby server has (2 libraries or 10, movies-only or
|
||
every type at once).
|
||
- **Listes de lecture** - pulled out of that grid into its own fixed
|
||
row, same visual weight as Accueil, bookending the group. Only
|
||
rendered at all when a `'playlists'`-type library is actually
|
||
present - same conditional-presence rule the sidebar already applies
|
||
to every library type, just applied one level deeper here.
|
||
|
||
Zero server-side work for this split - `collectionType` was already
|
||
being sent, just never read on the client before now.
|
||
|
||
**One `createLibraryLink()` builds both trees.** The desktop sidebar's
|
||
existing plain-text links and the mobile sheet's icon+text links are
|
||
different enough (icon or not, grid cell or plain row) that unifying
|
||
them into one shared DOM subtree styled two ways would need more CSS
|
||
trickery than it saves - so `Shell.ts` keeps two small render call
|
||
sites, both reading the one `getLibraries()` result, sharing the one
|
||
link-building function. The two logout buttons (sidebar footer, sheet
|
||
footer) are separate DOM nodes for the same reason - same label, same
|
||
`click` handler, wired once via `.logoutBtn` rather than `#logout`.
|
||
|
||
**Icons are new** - the sidebar has never had any. Small inline SVGs,
|
||
same "constant string, `currentColor`" convention `views/player/Player.ts`
|
||
already uses for its own icons, one per Emby `CollectionType`
|
||
(`movies`/`tvshows`/`music`/`musicvideos`/`playlists`) plus a generic
|
||
fallback for any future type this app hasn't mapped yet.
|
||
|
||
**The music bar's stacking, made structural instead of a magic number.**
|
||
`MusicBar.css`'s `.musicBar` no longer positions itself - a new
|
||
`.bottomStack` (`position: fixed; bottom: 0`, plain flex column) wraps
|
||
it together with the new mobile nav trigger button, so the trigger
|
||
always sits flush above the bar's *actual* rendered height. That
|
||
height isn't constant - `.musicBarRow` wraps onto a second line on a
|
||
narrow screen with a long track/artist name plus presence avatars -
|
||
so a hardcoded pixel offset would have drifted; normal flow inside one
|
||
shared fixed wrapper can't. Transparent to desktop: the trigger is
|
||
`display: none` there, so `.bottomStack` just becomes a plain wrapper
|
||
around the bar, unchanged from before.
|
||
|
||
The sheet itself (`.mobileNavSheet`) and its scrim are independent
|
||
`position: fixed` overlays above `.bottomStack`'s z-index, sliding up
|
||
via `transform: translateY` - closes on: tapping the scrim, tapping any
|
||
of the three zones' links (one delegated click listener, not one per
|
||
link), or any `hashchange` (covers browser back/forward too, alongside
|
||
the existing `highlight()` listener that already ran on that event for
|
||
the active-link state - now widened from `#sidebarNav .navItem` to
|
||
every `.navItem` on the page, so both trees highlight consistently).
|
||
|
||
Verified: full `typecheck`/`build` (client only). The interaction logic
|
||
- open/close, closing on a link tap, active-state highlighting, the
|
||
grouped Accueil/Bibliothèques/Listes-de-lecture split, the conditional
|
||
playlists row - was proven first in the mockup artifact (identical
|
||
logic), then re-verified here directly against the real shipped
|
||
`Shell.ts`/`Shell.html`/compiled `Shell.css`, wired up with fake library
|
||
data (no Emby login reachable from this sandbox) since the real Shell
|
||
only ever mounts after a real login: the desktop tree still renders
|
||
correctly untouched at desktop width, the compiled CSS's media query
|
||
and base mobile rules match what was written byte-for-byte, and the
|
||
mobile sheet's open/close/navigate/logout interactions were driven
|
||
programmatically end to end (`element.click()` still fires listeners on
|
||
a `display: none` element, so this didn't need a real narrow viewport
|
||
to exercise the logic itself). The music-bar/trigger stacking was
|
||
confirmed visually with a synthetic long-track-title music bar sitting
|
||
flush above the trigger, no gap or overlap. **Not verified**: the
|
||
actual rendered layout at a genuinely narrow viewport - this sandbox's
|
||
browser tool's viewport-emulation stopped taking effect partway through
|
||
this session (`screen.width` reflected the requested size, `innerWidth`
|
||
and `matchMedia` did not - a tooling issue hit repeatedly this session,
|
||
not something in this app's code), so the mobile-width visual check
|
||
above was done by forcing the same CSS rules through an injected
|
||
override instead of a real ≤720px viewport. Worth an early real-phone
|
||
check once deployed.
|
||
|
||
## Music UI rework: circles and squares instead of movie posters, a real now-playing screen
|
||
|
||
Asked for directly, "inspired by other apps": the music side of this
|
||
app reused `tiles.ts`'s poster tile (2:3, hover-revealed actions) for
|
||
artists and albums too, since it was already there for movies/shows -
|
||
functional, but a 2:3 box stretches an artist photo or a square album
|
||
cover oddly, and doesn't read as "this is music" at a glance the way a
|
||
circle/a square immediately does. Explored as mockups first (an
|
||
interactive artifact covering all four screens, mobile and desktop)
|
||
before writing any real code.
|
||
|
||
**Nothing invented that doesn't have a real data source behind it** -
|
||
no "popular tracks" (Emby has no listening-stats endpoint this app
|
||
uses), no "previous track" (there's no playback history, just the
|
||
shared queue), no favorites (no such concept server-side). Every
|
||
control below does exactly what its equivalent already did.
|
||
|
||
- **`tiles.ts`** - `tileHtml()` now dispatches on the item's Emby type:
|
||
`MusicArtist` gets a new `artistTileHtml()` (a circle, no hover
|
||
overlay - there's only ever the one action, open the artist page, so
|
||
the whole tile is just that link) and `MusicAlbum` gets
|
||
`albumTileHtml()` (a square, same "whole tile is the link"
|
||
reasoning). Dispatching inside the one shared function - not adding
|
||
a renderer parameter every call site has to pass - is what keeps the
|
||
home carousels, the library grid, and the artist page's own album
|
||
grid all rendering identically with zero changes to any of those
|
||
three call sites.
|
||
- **`views/artist`** - a real hero: circular photo, name, album count,
|
||
and a pill-shaped play button (a circular accent-colored icon badge
|
||
+ the existing text label, not icon-only - see the note below on
|
||
why). New `Artist.css` (this page had none before).
|
||
- **`views/album`** - the existing header already had the right shape
|
||
(a square cover, a flex row) - it keeps its own class names, just
|
||
gains the same pill play button and a `heroKicker` ("Album · 2023")
|
||
pulled out of the title instead of appended inline. Each track row's
|
||
wide text button ("Écouter ensemble"/"Ajouter à la file") becomes a
|
||
small circular icon (play, or a plus once a party's already active -
|
||
the same two states, just an icon swap instead of a label swap) -
|
||
with a whole album's worth of rows stacked, one word-length pill per
|
||
row read as noisier than the plain icon every real track list uses.
|
||
- **Explicit labels stay, deliberately, on the two hero play buttons**
|
||
(`heroPlayBtn`/`heroPlayIcon`, shared between the artist and album
|
||
pages) - a circular icon badge sits inside the button next to real
|
||
text, rather than the button being icon-only. This app already
|
||
standardized on icon-only for pure transport controls (Player.ts's
|
||
own play/pause/mute/fullscreen/cast) but explicit text for anything
|
||
request- or action-shaped (see the player-rework section above) -
|
||
starting or queueing a whole discography/album reads as the latter,
|
||
so the label stayed. The per-track buttons above are the one
|
||
deliberate exception - not because the convention changed, but
|
||
because a whole album's worth of them repeats the exact same action
|
||
as the hero button one row above, where the icon alone is
|
||
unambiguous in context.
|
||
- **`musicBar/MusicBar`** - the biggest change, restructured without
|
||
touching any of its existing sync/cast/HLS logic:
|
||
- `#playPauseBtn`/`#voteSkipBtn`/`#toggleQueueBtn`/`#leaveBtn` all
|
||
move from `.linklike` text (`"Passer (vote) - 2/3"`,
|
||
`"File d'attente"`, `"Quitter"`) to icon buttons, matching
|
||
`#castBtn`'s own pre-existing icon treatment instead of standing
|
||
out as the only text links in the bar. The vote tally becomes a
|
||
small badge (`#voteBadge`) on the skip icon instead of appended
|
||
text.
|
||
- New `#mobileMiniBar` - a collapsed preview (art, name/artist, a big
|
||
play button, a thin passive progress line) that's `display: none`
|
||
on desktop, where `#musicBarPanel` below is already always
|
||
visible - nothing to collapse into a preview of there.
|
||
- `#musicBarPanel` now wraps everything that used to be
|
||
`.musicBarRow` - on desktop it's the same always-visible bar,
|
||
restyled as a 3-column grid (track info / centered transport /
|
||
presence+cast+leave) instead of one wrapping flex row. On mobile
|
||
it becomes a full-screen "now playing" overlay (`position: fixed;
|
||
inset: 0`, sliding up via `transform`), opened by tapping
|
||
`#mobileMiniBar` and closed via a new `#panelCloseBtn` - the actual
|
||
mobile screen this app was missing entirely, not just a resized
|
||
version of the bar.
|
||
- `#trackProgress` (the existing thin top-edge seek strip) didn't
|
||
move or change shape at all - it was already the "thin bar along
|
||
the top" treatment asked for, just needed to not be reinvented as
|
||
a different, boxed, labeled scrub bar for desktop specifically
|
||
(an earlier mockup draft did exactly that before this same
|
||
feedback landed and got fixed there first).
|
||
- `MusicBar.css`'s `.musicBar` no longer positions itself at all -
|
||
unchanged from the mobile-nav work above, `.bottomStack` owns the
|
||
`position: fixed` wrapping so `#mobileMiniBar` always sits flush
|
||
above whatever height the (now-hidden-behind-it-when-collapsed)
|
||
panel would otherwise occupy.
|
||
|
||
**A real bug caught by verifying computed styles, not just reading the
|
||
CSS**: `.musicBarPlayBig` (the big circular play button, shared between
|
||
`#playPauseBtn` and `#miniPlayPauseBtn`) was silently losing its own
|
||
background/border-radius to `.iconBtn`'s (`Player.css`) at equal
|
||
selector specificity - which one wins depends on which file esbuild
|
||
happens to bundle later, not on anything either file's author controls.
|
||
Fixed by qualifying the selector as `.iconBtn.musicBarPlayBig` (both
|
||
classes are always present together on these two buttons), whose
|
||
higher specificity wins regardless of bundle order. Caught by checking
|
||
`getComputedStyle` in a real browser against the real compiled
|
||
`app.css`, not by re-reading the source and assuming the declared rule
|
||
wins - it visibly didn't, rendering as a plain 6px-radius transparent
|
||
button instead of a filled circle.
|
||
|
||
Verified: full `typecheck`/`build` (client only). Real compiled
|
||
`app.css` + the real `Shell.html`/`MusicBar.html` markup, driven with
|
||
fake artist/album/track data (no Emby login reachable from this
|
||
sandbox): circular vs square tiles and their exact aspect-ratio/
|
||
border-radius confirmed via `getComputedStyle`, the per-track icon
|
||
swap (play vs plus) confirmed per row, and `#mobileMiniBar`'s open/
|
||
close/guard-against-tapping-the-play-button behavior all driven
|
||
end to end and confirmed via real click events - not just read back
|
||
as source. This sandbox's browser tool's own viewport emulation
|
||
stayed unreliable for the rest of this session (`innerWidth` didn't
|
||
track a requested resize), so the mobile-width visual checks were
|
||
done the same way as the mobile-nav work above: forcing the mobile
|
||
media query's declarations through an injected override rather than a
|
||
genuinely narrow viewport. **Not verified**: any of this against a
|
||
real Emby server's actual artist/album images and names, or on a real
|
||
phone.
|
||
|
||
## Mobile navigation, take two: swipe-to-close was fighting pull-to-refresh
|
||
|
||
Reported directly, against a real phone: the bottom sheet (see "Mobile
|
||
navigation" above) had exactly one way to close it - tap the dimmed
|
||
scrim - since the grabber was purely decorative. Swiping the sheet
|
||
down, the instinctive gesture every real bottom sheet supports, fell
|
||
through to the browser's own default touch handling instead: with
|
||
nothing telling it otherwise, a downward drag starting on the scrim or
|
||
the sheet reads to the browser as "the user wants to pull the page
|
||
down," which on a phone is the built-in pull-to-refresh gesture - not
|
||
a bug in this app's own logic, but a real gap in it (nothing was
|
||
`preventDefault`-ing or claiming that gesture for the sheet instead).
|
||
|
||
**Two independent parts, not one fix**: containing the browser's own
|
||
gesture, and giving the swipe something to actually do.
|
||
|
||
- `touch-action: none` on `.mobileNavScrim` and the new
|
||
`.sheetGrabberHandle` - tells the browser not to apply *any* of its
|
||
own default touch behavior (panning, pull-to-refresh) to a touch
|
||
starting on either, since this app's own pointer handlers own that
|
||
gesture now. `overscroll-behavior: contain` on `.mobileNavSheet`
|
||
itself covers the other half of the same problem: its own library
|
||
grid can be taller than the sheet's `75vh` cap, so a drag that hits
|
||
*that* scroll boundary needs to stop there too, not chain past the
|
||
sheet to the page underneath.
|
||
- **The grabber is now a real drag handle**, not just a visual hint.
|
||
`#sheetGrabberHandle` wraps the small visible pill in a much bigger
|
||
invisible touch target (a real touch target, not a 36x4px sliver)
|
||
and drives the sheet with the same pointerdown/move/up(/cancel)
|
||
pattern `views/player/Player.css`'s own seek bar already uses:
|
||
1:1 tracking under the finger while dragging (`.dragging` suspends
|
||
the CSS transition for this), then either snapping back open (a
|
||
drag under 90px) or calling the exact same `closeMobileNav()` the
|
||
scrim tap already used (past it) - one close path, two ways to
|
||
reach it, not a second parallel implementation. Deliberately scoped
|
||
to the grabber alone, not the whole sheet: the sheet's own content
|
||
(the library grid, the logout row) needs normal taps and its own
|
||
scroll to keep working, which a whole-sheet drag-to-close would
|
||
fight.
|
||
|
||
Verified: full `typecheck`/`build` (client only). The CSS properties
|
||
(`touch-action`/`overscroll-behavior`) confirmed applied via
|
||
`getComputedStyle` against the real compiled `app.css`. The drag logic
|
||
itself confirmed with dispatched `PointerEvent`s carrying real
|
||
`clientY`/`pointerId` values against the real handlers copied from
|
||
`Shell.ts`: a 40px drag snaps back (stays open, `transform` cleared),
|
||
a 120px drag closes, and the live `transform` mid-drag tracks the
|
||
pointer 1:1 (confirmed at an intermediate point, not just the start/end
|
||
state) - not just asserted from reading the threshold constant.
|
||
Existing behavior re-confirmed unchanged in the same pass: scrim-tap-
|
||
to-close still closes. **Not verified**: actual touch input (not a
|
||
mouse or dispatched `PointerEvent`) on a real phone - no device
|
||
reachable from this sandbox, same standing gap as this project's other
|
||
touch-specific work (the video/music seek bars' own drag handling).
|
||
|
||
## Toast notifications, replacing every blocking `alert()`
|
||
|
||
Asked for directly, prompted by the host-handoff message specifically
|
||
(`"L'hôte précédent a quitté la party - tu es maintenant l'hôte."`) -
|
||
a blocking `alert()` freezes the whole tab and demands a click to
|
||
dismiss, for what's genuinely just a heads-up. Once looking at that
|
||
one, every OTHER `alert()` in the client turned out to be the exact
|
||
same pattern (`alert((err as Error).message)` after a failed
|
||
create/join, or a bare `onError: (message) => alert(message)` on a
|
||
socket connection) - ten call sites total, across `Player.ts`,
|
||
`MusicBar.ts`, `tiles.ts`'s shared `wireTileButtons`, and every one of
|
||
`Home`/`Series`/`Playlist`/`Artist`/`Album`'s own "start a party"
|
||
buttons. All ten now go through one new module instead.
|
||
|
||
**New `toast.ts`/`toast.css`** - a plain module-level singleton, not
|
||
something Shell.ts mounts like `musicBar/MusicBar` (there's no
|
||
per-party state here, just somewhere to drop text), so it lazily
|
||
creates its own fixed-position container on first use and works from
|
||
literally any file that imports `showToast()`. Visually the same
|
||
light pill/auto-dismiss language `views/player/Player.css`'s own
|
||
`.requestBanner` already used for pause/resume requests, generalized
|
||
to `position: fixed` against the viewport (not `.playerWrap`
|
||
specifically) so it works from any page, stacked (a flex column with
|
||
`gap`, so an error and a moment-later status update don't clobber each
|
||
other) and layered above everything else in the app (`z-index: 100`,
|
||
comfortably clear of the highest anything else currently uses).
|
||
|
||
**Two variants, one restrained look** - `variant: 'info'` (default,
|
||
the host-handoff message, every status-only case) and `variant:
|
||
'error'` (every migrated `alert()` above) differ by exactly one thing,
|
||
a plain `border-left` accent in `var(--danger)`. Deliberately not a
|
||
red banner/fill even for errors: a failed "start a party" click is
|
||
still just a message, not something this app should suddenly shout
|
||
about - matching the restraint the rest of this app's UI already
|
||
favors (subtle background tints, not loud fills, for e.g.
|
||
`.avatarActionRequested`). No manual dismiss button, same as
|
||
`.requestBanner` before it - every toast fades itself out after
|
||
~4.5s, respecting `prefers-reduced-motion` (an instant show/hide
|
||
instead of the opacity/transform fade, same guard `.spinner` already
|
||
uses elsewhere).
|
||
|
||
Verified: full `typecheck`/`build` (client only). Confirmed there are
|
||
no `alert(` calls left in `client/src` beyond comments that merely
|
||
mention the word. The fade-in double-`requestAnimationFrame` trick
|
||
(needed because adding the "visible" class in the same frame as the
|
||
insert can get coalesced with it, silently skipping the transition)
|
||
was checked directly against a real element in a real browser rather
|
||
than assumed from the general pattern.
|
||
|
||
## Library/artist grid: the tile minimum was too wide for a real phone
|
||
|
||
Reported directly, against a real device: a phone in the
|
||
~360-393px-wide range (an ordinary width, not a rare one) was showing
|
||
exactly one tile per row in every library/artist grid. The grid's
|
||
`minmax(160px, 1fr))` floor was never wrong on its own terms - it's
|
||
just wider than what's left once `.mainContent`'s own side padding is
|
||
subtracted from a real phone's width, and there was no narrower-screen
|
||
override for it at all (unlike the sidebar/nav or the music bar,
|
||
`tiles.css`'s `.grid` had never gotten one).
|
||
|
||
Fixed with `minmax(clamp(100px, 28vw, 160px), 1fr)` - a fluid minimum,
|
||
not a `max-width` breakpoint. `clamp()` scales continuously with the
|
||
real viewport instead of jumping at one arbitrary width, so every size
|
||
in between "phone" and "desktop" (a tablet, a half-width browser
|
||
window) already gets a reasonable column count too, not just the two
|
||
sizes a breakpoint would have explicitly targeted - and it reduces to
|
||
today's exact `160px` past whatever width `28vw` alone would already
|
||
exceed it, so desktop is provably unchanged rather than just "probably
|
||
fine."
|
||
|
||
Verified: full `typecheck`/`build` (client only). Confirmed in a real
|
||
browser, not just computed by hand: an `<iframe>` sized to each of
|
||
320/360/375/393/430/768/1200px (an iframe gets its own independent
|
||
viewport for `vw` units, which this sandbox's own unreliable
|
||
`resize_window` emulation can't provide - see the mobile-nav/music-UI
|
||
sections above) with the real compiled `tiles.css` loaded inside it,
|
||
checking the actual rendered column count at each. Every phone width
|
||
from 320 to 393px now gets 2 columns (previously 1), 430px gets 3, and
|
||
768/1200px match what the old fixed `160px` floor already gave -
|
||
confirmed empirically, not assumed from the formula alone.
|
||
|
||
## The library header wasn't fully sticky: the title still scrolled away
|
||
|
||
"A sticky search bar" (above) only ever made the `<input>` itself
|
||
sticky - asked for directly again, this time to also keep the page
|
||
title (`.pageHeader`'s `<h1>`) pinned alongside it on `views/library`,
|
||
so both stay visible together while a long grid scrolls underneath
|
||
instead of just the search box.
|
||
|
||
New `.libraryHeader` wrapper (`views/library/Library.css` - the one
|
||
view of this shape that had no dedicated stylesheet yet, everything
|
||
borrowed from `tiles.css`/`base.css`) around the existing `.pageHeader`
|
||
+ `.searchBar` pair, `position: sticky; top: 0` on the wrapper itself
|
||
with its own opaque `var(--bg)` background - the `<h1>` has none of its
|
||
own, so without this, tiles scrolling underneath would show through the
|
||
title text once stuck. `.searchBar` keeps its own pre-existing
|
||
`position: sticky; top: 0` from `tiles.css` unchanged (still needed as-is
|
||
on `views/artist`/`views/album`, which don't get this wrapper); nested
|
||
inside an already-stuck ancestor that isn't itself scrolling, it's
|
||
inert rather than conflicting.
|
||
|
||
Scoped to `views/library` only, not `tiles.css`'s shared `.pageHeader` -
|
||
`views/home` uses that same class for a plain, non-sticky title, and
|
||
`views/artist`/`views/album` have since moved to their own hero headers
|
||
(see "Music UI rework" above), not `.pageHeader` at all.
|
||
|
||
One real collision worth catching: below 720px, `shell/Shell.css`'s own
|
||
`.mobileTopBar` is *also* `position: sticky; top: 0` (the app's brand
|
||
bar, sitting above `.mainContent` as a sibling, not an ancestor) - the
|
||
pre-existing sticky `.searchBar` already raced it for the same spot,
|
||
just narrowly enough (one slim input) not to get reported. Stacking a
|
||
whole title+search block at `top: 0` too would have properly covered
|
||
it, later in paint order. `.mobileTopBar` got an explicit
|
||
`height: var(--mobile-topbar-h)` (new `base.css` token, `49px` -
|
||
replacing its previous padding-derived auto height with a fixed one
|
||
anything else can reliably stack against), and `.libraryHeader` sticks
|
||
to `top: var(--mobile-topbar-h)` instead of `0` under that same 720px
|
||
breakpoint - directly below the brand bar instead of underneath it.
|
||
|
||
Verified: full `typecheck`/`build` (client only). No Emby session is
|
||
needed to see either state, so checked directly: real compiled
|
||
`app.css` loaded (same-origin, from the running dev server) into two
|
||
`<iframe>`s - 1000px and 375px wide, each its own independent viewport
|
||
for the 720px media query, the same workaround this sandbox's own
|
||
broken `resize_window` emulation has needed all session - reproducing
|
||
`Shell.html`'s `.mobileTopBar`/`.mainContent` plus `Library.html`'s new
|
||
markup and a couple dozen dummy tiles, then scrolled deep into the grid
|
||
in both and read `getBoundingClientRect()` back: on the 1000px frame
|
||
the title and search bar both stayed at the viewport top, tiles
|
||
scrolling underneath, opaque background confirmed; on the 375px frame
|
||
`.libraryHeader`'s top edge landed exactly at `.mobileTopBar`'s bottom
|
||
edge (49px) with zero overlap, both bars' text still visible together.
|
||
|
||
## Alphabet index: jump straight to a letter in a library
|
||
|
||
Asked for directly, alongside the sticky header above: a way to jump to
|
||
a letter instead of scrolling a long library by hand. Proposed two
|
||
mockups first (a right-edge A-Z rail either bare or paired with sticky
|
||
per-letter section headings, both with the drag-to-scrub-plus-big-letter-
|
||
bubble feel of iOS's own Contacts index) - the section-headings version
|
||
was picked.
|
||
|
||
New `alphaIndex.ts`, deliberately not folded into `tiles.ts`: it's a
|
||
`views/library`-only concern (the ask was specifically "dans les
|
||
bibliothèques"), and `tiles.ts` stays exactly what every other page
|
||
still wants - one flat, ungrouped grid.
|
||
|
||
**Grouping** buckets each item by its first letter, accents folded to
|
||
their base (`normalize('NFD')` + stripping the combining marks) so
|
||
"Écho" lands under "E" like a French speaker would expect, not off in
|
||
its own accent-specific bucket. No client-side re-sort needed -
|
||
`embyApi`'s `getLibraryItems`/`getArtists` both already ask Emby for
|
||
`SortBy: SortName`, so a run of same-bucket items is already contiguous.
|
||
|
||
**Layout**: a `.letterHeading` (`grid-column: 1 / -1`) inserted before
|
||
each new bucket, directly inside the *same* `.grid` tiles.ts's own tiles
|
||
already populate - not a separate mini-grid per letter, which would have
|
||
needed its own column-count logic duplicated 20+ times over. Its `top`
|
||
(both for its own `position: sticky` and as every jump target's
|
||
`scroll-margin-top`, so a jump doesn't land a heading half-hidden behind
|
||
the still-stuck header above it) is measured off `.libraryHeader` at
|
||
runtime - `getComputedStyle(libraryHeader).top` picks up whichever of
|
||
Library.css's two breakpoint values (`0` desktop, `--mobile-topbar-h`
|
||
mobile - see the sticky-header section above) is currently active
|
||
without this module needing to know that breakpoint exists at all.
|
||
|
||
`renderGroupedTilesInChunks` mirrors `renderTilesInChunks`'s own chunked
|
||
`insertAdjacentHTML` (see that function's comment - a real library can
|
||
run past a thousand items, and one synchronous `innerHTML` write for all
|
||
of them is exactly the "front freezes while it loads" failure this app
|
||
already fixed once elsewhere); a chunk boundary landing mid-bucket is
|
||
harmless, headings and tiles just interleave in whatever order they're
|
||
inserted.
|
||
|
||
**The rail** (`.alphaRail`, fixed to the right edge) is 27 buckets
|
||
(`#` + A-Z), each dimmed and inert if `items` has nothing in it. Tap or
|
||
drag it - `pointerdown`/`move`/`up` on one dedicated element with
|
||
`touch-action: none` and `setPointerCapture`, the identical shape
|
||
`shell/Shell.ts`'s own nav-sheet drag handle already uses - and a
|
||
center-screen bubble shows the current letter while dragging, the same
|
||
"big transient label during a drag" idea as that handle's own gesture,
|
||
applied here to a very different one. Scrolling normally (not
|
||
dragging) instead runs a small scroll-spy on `window`'s scroll event,
|
||
highlighting whichever bucket's heading is currently at the top.
|
||
|
||
Both the rail and every heading's sticky offset track two elements'
|
||
*real* rendered size rather than a guessed constant - a `ResizeObserver`
|
||
on `.libraryHeader` (a very long title could wrap to two lines) and on
|
||
`shell/Shell.html`'s `.bottomStack` (the music bar floats there whenever
|
||
a listen party is active, on desktop as well as mobile, and can
|
||
start/stop while this page stays open) - so the rail's own `bottom`
|
||
always stops exactly above whichever of those is currently showing,
|
||
instead of a listen party's music bar quietly covering "X"-"Z".
|
||
|
||
**Search** still uses tiles.ts's existing `wireSearchFilter` unchanged
|
||
(`.tile`-level show/hide) - `wireSectionSearchSync` just walks each
|
||
`.letterHeading`'s following siblings up to the next one and hides it
|
||
too if every tile in that run is now `.hide`'d, registered on the same
|
||
input *after* `wireSearchFilter` so it always sees the up-to-date
|
||
`.hide` state (same-element listener order = registration order).
|
||
Without it, searching down to matches in one bucket would leave every
|
||
other bucket's heading floating above nothing.
|
||
|
||
**Cleanup**: `renderLibrary` itself had to change shape - from a plain
|
||
`async function` to the synchronous-wrapper-around-an-async-IIFE
|
||
`views/player/Player.ts`'s `renderPartyPlayer` already uses, returning
|
||
a real `CleanupFn` (see router.ts) instead of the `Promise<void>` an
|
||
`async function` can't help but return. Both the rail's two
|
||
`ResizeObserver`s and its `window` scroll/resize listeners live past
|
||
whatever DOM `router.ts`'s next `innerHTML = ''` clears, so without an
|
||
explicit teardown they'd keep running (and, for the scroll listener,
|
||
keep doing real work) after leaving the page - the exact leak class
|
||
"Realtime transport" up above already covers this app getting bitten by
|
||
once.
|
||
|
||
Verified: full `typecheck`/`build` (client only). No Emby session
|
||
available here, so the interaction itself was checked by bundling
|
||
`alphaIndex.ts` alone (a throwaway esbuild entry point, deleted after)
|
||
into a synthetic page reproducing the real Shell/Library DOM and real
|
||
compiled CSS, then driven for real rather than assumed: a genuine
|
||
browser click on a rail letter (not a script-dispatched one - dispatched
|
||
`PointerEvent`s never register as an active pointer, so `setPointerCapture`
|
||
itself only ever proved out under real input) jumped and highlighted
|
||
correctly with zero console errors; dragging across several letters
|
||
updated the active one and the bubble live at each step; tapping an
|
||
empty letter highlighted it but left scroll position untouched; typing
|
||
into the search box hid exactly the buckets left with no visible tiles
|
||
and none of the others; growing `#musicBar`'s height live moved the
|
||
rail's bottom edge with it and back; and calling the returned cleanup
|
||
stopped that last reaction cold, confirming the observer was actually
|
||
disconnected rather than just idle.
|
||
|
||
## The alphabet index's headers were in the wrong order
|
||
|
||
Reported directly, against a real library: header letters coming out
|
||
as e.g. `A, B, T, B, L` instead of one run per letter in order. Not a
|
||
sorting bug in Emby, and not a bug in `renderGroupedTilesInChunks`'s own
|
||
"new bucket = new heading" logic either - both were already correct.
|
||
The actual mismatch: `getLibraryItems`/`getArtists` ask Emby to *order*
|
||
the array by `SortBy: SortName`, but never asked for each item's
|
||
`SortName` *value* back, so `alphaIndex.ts`'s `bucketFor` had nothing to
|
||
bucket by except the raw display `name` - and Emby's `SortName` strips
|
||
leading articles ("The", "Le", "L'", ...) for exactly the kind of
|
||
natural alphabetical browsing this index is for. An item like "The
|
||
Bunker" sorts (correctly) among the B's by Emby's own SortName, but
|
||
bucketing it by its literal display name instead computed "T" - a
|
||
spurious heading mid-run, immediately followed by a second, duplicate
|
||
"B" heading once the next real B-titled item came right back around.
|
||
Every other letter, with no such item, looked completely fine, which is
|
||
why this wasn't caught by the earlier synthetic verification (its fake
|
||
titles never happened to include an article-stripped one at all).
|
||
|
||
Fixed at the source instead of trying to reimplement Emby's own
|
||
article-stripping rules client-side (which language's articles, and
|
||
which are configurable server-side, isn't this app's business to know):
|
||
`getLibraryItems`/`getArtists` now also request `Fields: SortName`, new
|
||
`ItemDto.sortName` carries it through (falling back to `name` when a
|
||
caller doesn't request it - same "populated where it matters, harmless
|
||
default elsewhere" shape `durationTicks`/`indexNumber` already use), and
|
||
`alphaIndex.ts` buckets by *that*, not `name`. Bucketing by the exact
|
||
same key Emby used to order the array is what guarantees a bucket is
|
||
always one contiguous run - not bucketing by something that's usually
|
||
close enough.
|
||
|
||
Verified: full `typecheck`/`test`/`build` on both projects (147/147
|
||
server tests). Reproduced the exact reported shape first - a small
|
||
fake item list in Emby's own SortName order, one entry's display name
|
||
starting with a different letter than its `sortName` (`"The Bunker"`,
|
||
`sortName: "Bunker"`, positioned between two other B titles) - confirmed
|
||
it produced the same `A, B, T, B, L` heading sequence against the old
|
||
bucket-by-`name` code, then confirmed the fix collapses it to the
|
||
correct `A, B, L` against the exact same input.
|
||
|
||
## The music bar's queue popup: broken on desktop, unclosable on mobile
|
||
|
||
Two reports that turned out to be the same missing `position` rule.
|
||
`.musicBarQueue`'s own comment already said it should "pop up ABOVE the
|
||
bar" (like `Player.css`'s `.settingsPanel` above its gear button), but
|
||
the CSS itself never actually gave it a `position` on desktop at all -
|
||
a plain normal-flow block, first child of `.musicBar`. Toggling it
|
||
visible didn't float anything; it pushed `#musicBarPanel` (the actual
|
||
transport controls) out of the way, growing `.bottomStack`'s own
|
||
fixed-to-viewport box from the bottom up - reported directly as the
|
||
player looking broken the moment the queue was opened.
|
||
|
||
Mobile had a different-looking but related bug. `#musicBarPanel`
|
||
becomes a full-screen `position: fixed` overlay under 720px; a normal-
|
||
flow `.musicBarQueue` sibling would have rendered *behind* it, so it
|
||
had already been given `position: relative; z-index: 26` specifically
|
||
to still show on top once opened from inside that full-screen panel
|
||
(see the git history for that rule's own reasoning). That's a real fix
|
||
for visibility, but not for layout: `position: relative` leaves an
|
||
element sized and placed by normal flow, which put the queue somewhere
|
||
in the middle of the screen rather than covering it - directly
|
||
overlapping the very `#toggleQueueBtn` used to open it, the only way
|
||
that existed to close it again. Reported directly as "can't be closed."
|
||
|
||
Fixed with a `position` on both sides, matching each context's actual
|
||
shape - `.musicBar` gets `position: relative` (a containing block),
|
||
desktop's `.musicBarQueue` becomes `position: absolute; left: 0;
|
||
right: 0; bottom: 100%` (floats directly above the bar, out of normal
|
||
flow, without touching `#musicBarPanel`'s own layout at all), and
|
||
mobile's becomes `position: fixed; inset: 0` (a proper full-screen
|
||
overlay of its own, `max-height: none` overriding the base rule's
|
||
`50vh` so it isn't clipped) - the same "size for what this context
|
||
actually needs" as `#musicBarPanel`'s own fixed/inset:0 treatment right
|
||
next to it.
|
||
|
||
Also added, so closing it never again depends on precisely where the
|
||
toggle button happens to render underneath: a dedicated `#queueCloseBtn`
|
||
inside the queue's own header, plus a click-outside-closes listener on
|
||
`document` (`views/player/Player.ts`'s own `settingsPanel` pattern) for
|
||
the desktop popover case.
|
||
|
||
Verified: full `typecheck`/`build` (client only). No Emby session here,
|
||
so checked directly - `initMusicBar` called against a synthetic
|
||
`.bottomStack`/`#musicBar` reproducing the real Shell markup, real
|
||
compiled CSS, a fake queue list: on the (real, 1280px) desktop viewport,
|
||
opening the queue landed its bottom edge exactly on the bar's own top
|
||
edge (zero gap, zero overlap) without moving the bar at all, and both
|
||
the close button and a click on unrelated page content behind it closed
|
||
it correctly. On a 375px-wide `<iframe>` (this sandbox's own
|
||
`resize_window` viewport emulation is unreliable - see the mobile-nav/
|
||
music-UI/library-grid sections above), with the now-playing panel
|
||
opened first: the queue rendered as a genuine full-viewport overlay,
|
||
`elementFromPoint` at the close button's own center hit the close button
|
||
itself (not something on top of it), and closing the queue correctly
|
||
returned to the still-open now-playing panel rather than closing
|
||
everything.
|
||
|
||
## Library tile buttons rendering on top of the sticky header
|
||
|
||
Reported directly: scrolling a library, a tile's own action buttons
|
||
("Lancer une watch party" / "Voir sur Emby") would briefly render
|
||
*above* the sticky title+search header instead of scrolling behind it.
|
||
|
||
Both `.libraryHeader` and the alphabet index's `.letterHeading` were
|
||
`z-index: 2` - copied from `tiles.css`'s own pre-existing `.searchBar`,
|
||
sticky at that same value long before either of them existed. What
|
||
that copy missed: a tile's own `.tileOverlay` (its hover-revealed
|
||
action buttons - always-visible rather than hover-revealed on a touch
|
||
device, see `tiles.css`'s `(hover: none)` block) is *also* `z-index: 2`.
|
||
Equal z-index is a tie, and a tie breaks toward whichever renders later
|
||
in the DOM - a tile, inside `#grid`, always after the header. The
|
||
original slim `.searchBar` never showed this in practice (a single
|
||
input has little vertical range to actually overlap a scrolling tile
|
||
during the transition); wrapping it into the much taller
|
||
`.libraryHeader` gave a scrolling tile far more room to pass directly
|
||
underneath before fully clearing it, is what actually surfaced it.
|
||
|
||
Fixed by raising `.libraryHeader` and `.letterHeading` to `z-index: 3`
|
||
- comfortably clear of `.tileOverlay`'s 2, and matching
|
||
`shell/Shell.css`'s own `.mobileTopBar` (the other sticky header
|
||
`.libraryHeader` already has to stack correctly under on mobile).
|
||
|
||
Verified: full `typecheck`/`build` (client only). Confirmed as a real
|
||
regression, not just reasoned about - real compiled CSS loaded into a
|
||
page reproducing `.libraryHeader` + a real `.grid` of tiles (their
|
||
`.tileOverlay` forced visible, simulating a touch device), scrolled so
|
||
a tile genuinely overlapped the header's own rect, then
|
||
`elementsFromPoint` read the actual paint order at that point: with
|
||
the header's z-index forced back to the old `2`, `.tileOverlay` topped
|
||
the stack exactly as reported; restored to `3`, the header (and the
|
||
search input inside it) topped it instead, `.tileOverlay` three layers
|
||
down, unreachable.
|
||
|
||
## The music bar's compact layout: the progress bar was never actually full-width
|
||
|
||
Reported directly, again, after the queue-popup fix above - the desktop
|
||
bar still didn't look right: the progress bar needed to span the full
|
||
width, with everything else in one row underneath.
|
||
|
||
`.musicBarProgressTrack`'s own comment already described it as "a thin
|
||
strip along the bar's own top edge" - the intent was always a full-width
|
||
strip above a row of controls, mirroring Spotify/YouTube Music's own
|
||
compact players. But `#trackProgress` (that element) is `.musicBarPanel`'s
|
||
*first* child inside a 3-column grid (`.musicBarPanelTrack`/
|
||
`.musicBarPanelTransport`/`.musicBarPanelSide`), with no `grid-column`
|
||
of its own - so grid auto-placement dropped it into column 1 of row 1
|
||
like any other item, squeezing it to that column's own width and
|
||
wrapping `.musicBarPanelSide` onto a second row underneath it instead.
|
||
Easy to miss at a glance: with no track loaded, `.musicBarPanelTrack`'s
|
||
own content (a small image + short placeholder text) still left column
|
||
1 wide enough that the squeeze didn't look obviously broken - see the
|
||
queue-popup section above for the *other* half of "cassé en desktop",
|
||
found and fixed first without this one.
|
||
|
||
Fixed with one line: `.musicBarProgressTrack { grid-column: 1 / -1; }`.
|
||
Spanning all 3 columns in row 1 is what pushes every sibling after it
|
||
(auto-placed, no changes needed to any of them) into row 2, across the
|
||
full width - track/transport/side exactly where the 3-column rule
|
||
already intended them, just no longer sharing row 1 with the progress
|
||
bar.
|
||
|
||
Verified: full `typecheck`/`build` (client only). Checked directly - a
|
||
synthetic page with real compiled CSS, `initMusicBar` called against a
|
||
reproduction of the real Shell markup, a fake track's name/artist/
|
||
position set so `.musicBarPanelTrack` has realistic content width. Read
|
||
back with `getBoundingClientRect()`: the progress bar's own width
|
||
matched `.musicBarPanel`'s full content width exactly (not the viewport,
|
||
which include its own outer padding); its bottom edge sat at or above
|
||
every other row's top edge; and the other three elements' vertical
|
||
ranges genuinely overlapped each other (not just similar `top` values,
|
||
which `align-items: center` legitimately varies by each one's own
|
||
content height) - confirming one shared row below, in the original
|
||
track/transport/side left-to-right order.
|
||
|
||
## An album's track rows collapsed to 3 lines on mobile for no reason
|
||
|
||
Reported directly: there was room to fit an album's track rows
|
||
(index/name/duration/a small play-or-queue button) on one line on
|
||
mobile, but `trackList.css`'s own `@media (max-width: 600px)` forced
|
||
`.trackRow` down to a 2-column grid and pinned both `.trackDuration`
|
||
and the button to that same second column - auto-placement then
|
||
stacked the two of them into their own rows underneath the track name,
|
||
three rows total instead of one.
|
||
|
||
Removed outright rather than tuned - the numbers don't support it
|
||
having ever been needed. `.trackRow`'s fixed overhead (28px index +
|
||
2×12px gaps + `auto`-sized duration/button + `12px` horizontal padding)
|
||
comes to roughly 155-165px, and even the narrowest common phone width
|
||
(320px) leaves comfortably more than that for `.mainContent`'s own
|
||
side padding plus a `1fr` name column - `.trackName`'s existing
|
||
`text-overflow: ellipsis` was already doing exactly the job this
|
||
breakpoint assumed it couldn't.
|
||
|
||
Verified: full `typecheck`/`build` (client only). Checked directly, not
|
||
assumed: real compiled CSS, a handful of `.trackRow`s (short and
|
||
deliberately overlong names) loaded into two `<iframe>`s at 320px and
|
||
375px - this sandbox's own `resize_window` viewport emulation is
|
||
unreliable, see the mobile-nav/music-UI sections above. At both widths
|
||
every row's index/name/duration/button sat on one line (their vertical
|
||
ranges genuinely overlapping, not just similar by coincidence), left to
|
||
right in that order, with the long names confirmed actually truncated
|
||
(`scrollWidth > clientWidth`) rather than merely happening to fit.
|
||
|
||
## A mobile install nudge for the PWA
|
||
|
||
Asked for directly: the PWA (see "Progressive Web App" above) existed,
|
||
but nothing ever told a mobile visitor it could be installed at all -
|
||
Chrome's own default mini-infobar is easy to miss/dismiss, and Safari
|
||
has no browser-driven prompt whatsoever.
|
||
|
||
New `pwaInstall.ts`, self-mounting like `toast.ts` (no per-route/
|
||
per-party state to attach) but with one real difference from that
|
||
module's own lazy-init pattern: `beforeinstallprompt` (Chrome/Edge/
|
||
Android WebView's event for this - not in `lib.dom.d.ts`, Safari/
|
||
Firefox never implement it) can fire before any UI exists to react to
|
||
it, and the browser never replays it for a listener that attached too
|
||
late. The listener itself is a top-level side effect of importing the
|
||
module - `shell/Shell.ts` does that (and calls the module's own
|
||
`initPwaInstallPrompt()`, after `initMusicBar`), and since ES module
|
||
imports all evaluate before an entry file's own top-level code runs,
|
||
that listener is live well before `app.ts`'s `boot()` even starts its
|
||
async login check, regardless of how soon the event actually fires.
|
||
|
||
Two real install paths exist, and the banner speaks to whichever one
|
||
applies:
|
||
- **Chrome/Edge/Android WebView**: `event.preventDefault()` stashes the
|
||
captured event (this banner replaces the browser's own default one);
|
||
clicking "Installer" calls the real `prompt()` and reads back
|
||
`userChoice` - only an outright 'accepted' removes the banner, since
|
||
the picker's own 'dismissed' just means "not right now", not "stop
|
||
asking" (that's what the banner's own close button is for).
|
||
- **iOS Safari**: no such event exists at all, ever - detected instead
|
||
by user-agent sniffing (there's no clean feature-detection for "is
|
||
this iOS Safari"), showing manual instructions ("appuie sur Partager
|
||
puis Sur l'écran d'accueil") with no button, since there's nothing to
|
||
programmatically trigger.
|
||
|
||
Mobile-only (`matchMedia('(max-width: 720px)')`, this app's own
|
||
existing breakpoint, re-checked on resize for a live rotate/unfold) and
|
||
suppressed entirely once already running installed
|
||
(`display-mode: standalone`, or `navigator.standalone` for iOS Safari's
|
||
own equivalent flag, which has no `display-mode` query at all). A
|
||
close click remembers "don't ask again" in `localStorage` - not
|
||
re-shown even against a fresh `beforeinstallprompt` on a later visit.
|
||
|
||
Rendered as a plain normal-flow banner at the very top of `<body>`,
|
||
before `#app` - not a fixed/sticky overlay - specifically to avoid
|
||
needing to coordinate yet another offset with `shell/Shell.css`'s own
|
||
already-stacked `.mobileTopBar`/`.libraryHeader` pair (see the sticky-
|
||
header sections above) or `alphaIndex.css`'s rail. It simply pushes the
|
||
app down by its own height and scrolls away with the page, the same as
|
||
a browser's own native install infobar would.
|
||
|
||
Verified: full `typecheck`/`build` (client only). No real device
|
||
install flow is triggerable in this sandbox (nothing can fabricate a
|
||
genuine browser installability signal), so checked everything around
|
||
that boundary directly instead: a synthetic `beforeinstallprompt`-
|
||
shaped event (real `Event`, stubbed `prompt`/`userChoice`) dispatched
|
||
into real compiled CSS/JS confirmed the banner appears with the
|
||
install button, that button calls the stub `prompt()` and removes the
|
||
banner only on an 'accepted' outcome, and that the close button both
|
||
removes it and blocks a *second* dispatched event from reopening it.
|
||
Overriding `navigator.userAgent` to an iPhone string confirmed the
|
||
iOS-flavored copy with no install button. A same-event dispatch into a
|
||
real ≥900px-wide `<iframe>` confirmed the banner never renders there
|
||
at all, regardless of the event firing.
|
||
|
||
## OS-level media controls (Windows' SMTC, macOS' Now Playing, ...)
|
||
|
||
Asked for directly: linking whatever's playing to Windows' own media
|
||
popup (the one media keys/the volume flyout show) - the standard
|
||
Media Session API (`navigator.mediaSession`), which the same code path
|
||
also surfaces to macOS' Now Playing widget and Android's notification
|
||
for free, on whichever OS/browser combination actually implements it
|
||
(Chromium family; Firefox has never implemented it at all).
|
||
|
||
New `mediaSession.ts`, a thin wrapper - every export is a one-line
|
||
`'mediaSession' in navigator` guard around the exact browser call, not
|
||
a new abstraction of its own - shared by `views/player/Player.ts`
|
||
(video) and `musicBar/MusicBar.ts` (a listen party), which can never
|
||
both be active at once (starting/joining either already leaves the
|
||
other first - see docs above), so there's no risk of the two fighting
|
||
over this one global.
|
||
|
||
**Metadata/position/playback state** hook into each player's own
|
||
existing single-source-of-truth functions rather than adding parallel
|
||
call sites everywhere: Player.ts's `updatePlayPauseBtn`/
|
||
`updateProgressBar` (already the one place `#playPauseBtn`'s icon and
|
||
the seek bar's fill/label get synced, from every driver-changed event,
|
||
the heartbeat, and the ticker alike) also call
|
||
`setMediaSessionPlaybackState`/`setMediaSessionPositionState` now;
|
||
MusicBar.ts's `setPlayIcon`/`updatePositionDisplay` do the same. Title/
|
||
artwork update at both places an item/track actually changes
|
||
(Player.ts's `init`/`onItemChanged`, MusicBar.ts's `loadTrack`) -
|
||
artist/album are left empty for video (meaningless there, same as this
|
||
app's own UI already treats them).
|
||
|
||
**Play/pause/seek** are wired once per party (not re-wired on state
|
||
changes) via `setMediaSessionActionHandlers` - the handler functions
|
||
themselves close over local state (`isHost`, `conn`, ...) and read it
|
||
fresh on every OS-triggered call, the same way any other closure in
|
||
either file already does, so a handler set at party-join time keeps
|
||
working correctly straight through a live host handoff without needing
|
||
to know that happened. Two real permission models, unchanged by this:
|
||
video's play/pause is host-only - a non-host's OS-level play/pause
|
||
routes to the exact same `requestPause`/`requestResume` (now factored
|
||
out so both the in-page buttons and this can call them) the request
|
||
toolbar already uses, never touching the video directly - and seeking
|
||
is gated on `isHost` the same way the in-page seek bar already is
|
||
(factored into a shared `seekTo`, reused by both the pointer handler
|
||
and this). Music has neither gate (no host concept at all - see
|
||
MusicQueueService's own reasoning), so its handlers act directly.
|
||
|
||
**"Next"** is handled differently from play/pause/seek because a
|
||
*registered* `nexttrack` handler is what makes the OS show that
|
||
control at all, regardless of whether clicking it would currently do
|
||
anything - so unlike the other three, its presence has to track
|
||
whatever the in-page equivalent's own visibility already tracks, via a
|
||
separate `setMediaSessionNextAction(fn | null)` called alongside it:
|
||
Player.ts's `#nextBtn` (`isHost && hasNextQueued`, in `becomeHost` and
|
||
`onItemChanged`) and MusicBar.ts's vote-skip button (`music.current`,
|
||
in `applyMusicState` - clicking the OS's own "next" casts one skip
|
||
vote, same as the in-page button, not an instant skip).
|
||
|
||
**Clearing**: `clearMediaSession()` (metadata, playback state, *and*
|
||
every action handler) runs on Player.ts's route cleanup and
|
||
MusicBar.ts's `leaveMusicParty` - an OS-level "now playing" card that
|
||
outlives the actual party would be actively wrong, not stale. A
|
||
lighter `setMediaSessionIdle()` (metadata/state only, action handlers
|
||
left alone) covers MusicBar.ts's own "queue ran empty but still in the
|
||
party" case instead - play/pause/seek stay meaningful the instant
|
||
something plays again, so there's nothing to re-wire there.
|
||
|
||
Verified: full `typecheck`/`build` (client only). `mediaSession.ts`
|
||
itself checked directly against the real API in a real Chromium
|
||
browser (this sandbox's own): metadata resolves to a genuine
|
||
`MediaMetadata` with the right fields and a resolved artwork URL (or
|
||
none for an empty one), playback state and position state both apply
|
||
without throwing, an invalid (non-positive/`NaN`) duration is silently
|
||
skipped rather than reaching the browser call that would throw on it,
|
||
action-handler registration doesn't throw with or without `onSeek`
|
||
supplied, and both `setMediaSessionIdle`/`clearMediaSession` reset
|
||
metadata to `null` and playback state to `'none'`. Player.ts's/
|
||
MusicBar.ts's own wiring is one-line calls into already-verified
|
||
functions at points each file already treats as its single source of
|
||
truth for that piece of state (see above) - not independently
|
||
exercised end to end against a live party (would need mocking the
|
||
entire socket/party layer for no more real signal than reading the
|
||
call sites already gives), and the actual OS-level popup itself - the
|
||
whole point - isn't something this sandbox can display or click
|
||
regardless. Worth a real Windows/Chrome check once deployed.
|
||
|
||
## 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.
|