Commit graph

3 commits

Author SHA1 Message Date
8718175bae Fix playstate 400 "Value cannot be null": missing PlaySessionId
All checks were successful
Publier l'image Docker / test (push) Successful in 23s
Publier l'image Docker / publish (push) Successful in 15s
Real request logged after the last fix (MediaSourceId in the query
string) shipped: still a 400, now Emby's own "Value cannot be null.
(Parameter 'key')" - a bare .NET Dictionary null-key exception, not
the earlier problem (MediaSourceId was present and well-formed in the
logged request).

This exact message already burned this app once before, in
CatalogService.negotiateStreamTarget's hand-built fallback URL (see
its own long-standing TODO(verify) comment) - diagnosed there as Emby
indexing some internal state by a parameter the request hadn't
supplied. PlaySessionId is the standout remaining candidate: it's how
Emby/Jellyfin clients normally correlate a progress report with the
specific playback/transcode session it belongs to, and evidently
Emby's real Progress handler keys a lookup on it even though the
OpenAPI spec doesn't mark it required.

Fixed the same way MediaSourceId was: CatalogService
.negotiateStreamTarget already reads info.PlaySessionId from the same
PlaybackInfo response (it already used it in the hand-built fallback
URL) - its return type just needed to surface it too. PartyStreamService
now caches it alongside mediaSourceId and exposes both together from
one accessor, getCachedPlaybackIds(partyId), replacing the
round-1 getCachedMediaSourceId. PlaystateService.reportProgress and
index.ts's sweep both thread it through.

Not independently confirmed against a real server yet (no safe way to
fire a real, state-mutating test call without one) - flagged in
docs/architecture.md the same way every other real-server-dependent
guess in this app is, but grounded in this app's own prior confirmed
diagnosis of the identical error message rather than a fresh guess.

Updated catalogService.test's negotiateStreamTarget assertions to
cover playSessionId in both return branches, partyStreamService.test's
mocks/assertions for the new {mediaSourceId, playSessionId} cache
shape, and playstateService.test's request-shape and logging
assertions for the extra PlaySessionId param. Full typecheck/test/build
green (96/96 tests). docs/architecture.md and README.md updated.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-14 22:56:54 +02:00
55e8b8b378 Fix Emby playstate sync: wrong param shape, confirmed against real API
All checks were successful
Publier l'image Docker / test (push) Successful in 21s
Publier l'image Docker / publish (push) Successful in 12s
Checked the app's Emby-API assumptions directly against Emby's own
real OpenAPI spec (swagger.emby.media, pointed at a real server) after
a "watch status per user seems broken" report.

PlaystateService.reportProgress was wrong: POST
/Users/{UserId}/PlayingItems/{Id}/Progress does exist at that exact
path, but the spec declares PositionTicks/IsPaused as *query string*
parameters, not body fields - and a third parameter, MediaSourceId, is
*required* and wasn't being sent at all. The request body schema this
endpoint does list (Api.OnPlaybackProgress) covers unrelated fields
(PlaylistIndex/Shuffle/SleepTimerMode/...), none of these. So the
original guess sent PositionTicks/IsPaused somewhere Emby never reads
them, while omitting a parameter required just to accept the call -
almost certainly meaning every progress report was failing outright,
silently (best-effort by design, so nothing broke loudly).

Fixed: reportProgress now sends MediaSourceId/PositionTicks/IsPaused
as query parameters, no body at all. MediaSourceId comes from
PartyStreamService's own cache - CatalogService.negotiateStreamTarget
already resolves a real one for every party's shared stream
negotiation, so PartyStreamService now caches it alongside the target
path (negotiateStreamTarget's return type changed from a bare string
to {targetPath, mediaSourceId}) and exposes it read-only via a new
getCachedMediaSourceId(partyId) - deliberately passive, never triggers
a negotiation of its own, so a party where no one has started
playback yet this session simply has nothing to report; index.ts's
sweep skips it rather than paying for an extra PlaybackInfo round trip
every tick just for this.

While checking this, the other two flagged 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, no
missing required parameter, consistent with them having been tried
for real successfully before.

Updated the 5 negotiateStreamTarget call sites in catalogService.test
to read the new {targetPath, mediaSourceId} shape, the 7
negotiateStreamTarget mocks in partyStreamService.test to return it
(plus 2 new tests: cache exposes mediaSourceId, a failed response
drops it too), rewrote playstateService.test's request-shape
assertions for query params instead of a body, and re-pointed
embyClient.test's generic body-encoding test at an arbitrary example
instead of implying it's still PlaystateService's real shape.

Verified: full typecheck/test/build (92/92 tests). docs/architecture.md
and README.md updated - the "Sync watch-party progress back to Emby"
section now documents the confirmed-wrong-then-fixed shape instead of
a standing TODO(verify), and README's "Not verified" paragraph moves
this (and the other two, now-confirmed) assumptions to "Resolved".

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-14 21:11:32 +02:00
f78b2f1408 One shared Emby stream per party, not one per viewer
All checks were successful
Publier l'image Docker / test (push) Successful in 21s
Publier l'image Docker / publish (push) Successful in 20s
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 - a party of 5 meant Emby
transcoding the same title 5 separate times.

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 alongside it - there's no other
consumer of the per-viewer path, and the ask was explicit that there
should only ever be one mechanism.

- server/src/services/CatalogService.ts: proxyStream's negotiate-then-
  build-a-target logic extracted into negotiateStreamTarget(session,
  itemId, isAudio, audioStreamIndex, maxStreamingBitrate) - identical
  logic, returns the target instead of proxying it directly so it can
  be cached. proxyStream/proxyRawStreamAsset removed.
- server/src/services/PartyStreamService.ts (new): one negotiated
  target per party.id, reused by every member; renegotiates when the
  desired item/audio/quality changes. Recovers from a dead upstream
  session with no new plumbing - EmbyClient.proxyResponse already sets
  res.statusCode before writing the body, so a failed proxied response
  is detected right after it resolves and the cache entry is dropped,
  so the next request (a manual retry, or the next segment fetch)
  renegotiates fresh instead of repeating the same dead target forever.
- server/src/emby/EmbyClient.ts: rewritePlaylist/proxyResponse's
  trailing itemId param genericized to routeKey - it's only ever read
  back to build the raw-asset URL, so it's party.id for the new stream
  routes and an inert real itemId for proxyImage/proxySubtitle (which
  never reach that branch anyway).
- server/src/index.ts: GET /api/catalog/party-stream/:partyId/
  master.m3u8 and .../raw replace /api/catalog/stream/:itemId/...
  entirely, membership-checked like the burn-in routes; a new idle-reap
  interval for PartyStreamService, same shape as TranscodeService's own.
- Forced consequence of "one stream", not a free design choice: audio
  track/quality become a host-only, party-wide choice for video -
  Party.streamAudioIndex/streamQuality, setStreamOptions/
  streamOptionsChanged, byte-for-byte the same shape
  setBurnInSubtitle/burnInChanged already established for burn-in
  subtitles. server/src/realtime/SocketGateway.ts gains the host-only,
  video-only handler; server/src/services/PartyService.ts resets
  streamAudioIndex (not streamQuality - a bandwidth preference isn't
  item-specific) on advanceQueue, same reasoning as burnIn* already
  gets; MusicQueueService.playNext ends the party's cached session on
  a track change.
- client/src/api/embyApi.ts: streamUrl/audioStreamUrl replaced by one
  partyStreamUrl(partyId) - no query params needed any more, the party
  itself is the source of truth for what it's streaming.
- client/src/api/party.ts: setStreamOptions/onStreamOptionsChanged.
- client/src/views/player/Player.ts: audio/quality <select>s disabled
  for a follower (applyStreamOptionsGating, mirroring
  renderSubtitleOptions' own gating), restore the party's current
  choice on load/broadcast, and a host's change calls setStreamOptions
  instead of reloading locally - every member (host included) only
  ever reacts to the resulting broadcast, same discipline the rest of
  this player already uses. Dead reloadWithCurrentOptions removed.
- client/src/musicBar/MusicBar.ts: loadTrack points at
  partyStreamUrl(activePartyId) - no picker to gate, MusicBar never had
  one.
- server/test/catalogService.test.ts: proxyStream's old cases moved to
  exercise negotiateStreamTarget instead, plus new quality-override and
  no-source-fails cases. server/test/partyStreamService.test.ts (new):
  cache hit/miss/param-change/item-change, the failure-invalidation
  path, endSession, idle reap. server/test/socketGateway.test.ts:
  setStreamOptions host-only gating + broadcast, mirroring the existing
  setBurnInSubtitle tests.
- docs/architecture.md, README.md: design, the forced audio/quality
  trade-off spelled out, and what's still unverified (a different kind
  than this session's other Emby-API guesses - the calls themselves
  were already live code, what's untested is real multi-viewer load).

Verified: full typecheck+test+build on both projects (85/85 server
tests, 12 new). 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.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-14 20:05:34 +02:00