Add SyncPlay stats-for-nerds page, manual sync button, and release pipeline
Phase 0 scaffold plus everything since: quality negotiation, host-relay, and active drift correction were each built and reverted after real bugs in live use (see PLAN.md for the full account of each). What remains is read-only SyncPlay observability (live drift/playback stats page) and a one-shot manual "sync me to group" action, plus Phase 5 packaging/release tooling (build.yaml, manifest.json generator, Gitea Actions release workflow) to distribute it as a proper plugin repository. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -32,6 +32,19 @@ switching tools.
|
||||
|
||||
## Architecture -- three pillars
|
||||
|
||||
**Status note (2026-07-09):** all three pillars below were built and then
|
||||
reverted after real bugs/unwanted behavior in live use (see "Rough phase
|
||||
breakdown" for the full account of each). Pillars 1/2: forcing quality/path
|
||||
alignment through the `PlaybackInfo` request froze clients on quality
|
||||
switches and left them permanently desynced. Pillar 3: a scoped-down active
|
||||
seek-correction on top of stock Jellyfin's own (already quite good) client
|
||||
correction caused its own feedback-loop problems and, even fixed, wasn't
|
||||
what the user wanted in practice. The original rationale is kept below for
|
||||
context, but as of this note **the plugin has no active functionality** --
|
||||
it's back to the Phase 0 scaffold baseline. Read "Rough phase breakdown" for
|
||||
what's actually true today, and don't restart from these designs without
|
||||
addressing why each one broke.
|
||||
|
||||
### 1. Group quality negotiation (the easy piece)
|
||||
|
||||
On group formation, resolve each member's device capability/bitrate profile
|
||||
@@ -149,37 +162,181 @@ Two different loops -- do not conflate them:
|
||||
- **ABI/version pinning**: `meta.json`'s `targetAbi` must match the running
|
||||
server version. Already fine since the deployment is pinned to `10.11.6`,
|
||||
not floating -- build against that exact version's SDK.
|
||||
- **Reaches past the stable plugin API.** SyncPlay group internals and
|
||||
transcode-session state aren't part of Jellyfin's published plugin SDK
|
||||
surface. Realistically requires building against Jellyfin server source at
|
||||
the matching tag (`git` checkout/submodule of `jellyfin/jellyfin` @
|
||||
`v10.11.6`) rather than just NuGet packages. This is closer to
|
||||
"semi-fork with a plugin wrapper" than a typical plugin -- sizing decisions
|
||||
should account for that.
|
||||
- **Reaches past the stable plugin API -- narrower than originally assumed.**
|
||||
Verified during Phase 1: `ISyncPlayManager`, `IGroupStateContext`,
|
||||
`GroupInfoDto`, and all the `IGroupPlaybackRequest` DTOs *are* public in the
|
||||
`Jellyfin.Controller` NuGet package -- group internals aren't actually
|
||||
hidden. What *is* missing from the SDK is the concrete implementations:
|
||||
`Emby.Server.Implementations.SyncPlay.SyncPlayManager` (needed to decorate
|
||||
`ISyncPlayManager` without reimplementing group logic) and
|
||||
`Jellyfin.Api.Controllers.MediaInfoController` /
|
||||
`Jellyfin.Api.Models.MediaInfoDtos.PlaybackInfoDto` (needed to target the
|
||||
PlaybackInfo action with an MVC filter). Both are public classes but ship
|
||||
only inside the server image, not as NuGet packages. Working solution:
|
||||
extract the two DLLs straight from the pinned `linuxserver/jellyfin:10.11.6`
|
||||
image (`docker cp <container>:/usr/lib/jellyfin/bin/{Emby.Server.Implementations,Jellyfin.Api}.dll`)
|
||||
into `lib/jellyfin-10.11.6/` and reference them as `<Reference Private="false">`
|
||||
-- compile-time only, since they're already loaded in the host process at
|
||||
runtime. No source checkout/submodule needed after all; re-extract these two
|
||||
files if the pinned server version ever changes. See `src/JellyfinSyncPlus/SyncPlay/`
|
||||
for the resulting decorator (`SyncPlusSyncPlayManager`) and MVC filter
|
||||
(`SyncPlusPlaybackInfoFilter`).
|
||||
- Host-relay approach (pillar 2) means the "host" member is a single point of
|
||||
failure for the group's stream -- needs a defined handoff/reselection
|
||||
behavior if the host disconnects mid-session (not just late-joiners).
|
||||
|
||||
## Rough phase breakdown
|
||||
|
||||
- **Phase 0 -- scaffold & dev loop.** `dotnet` plugin project scaffold against
|
||||
`Jellyfin.Controller` (or a source checkout of `jellyfin/jellyfin@v10.11.6`
|
||||
if the public SDK isn't sufficient), local docker dev Jellyfin with
|
||||
bind-mounted plugins folder, confirm a trivial "hello world" plugin loads
|
||||
and shows up in Dashboard -> Plugins.
|
||||
- **Phase 1 -- group quality negotiation.** Hook group-join, resolve each
|
||||
member's device profile, compute the common lowest profile, verify against
|
||||
real device/browser combos.
|
||||
- **Phase 2 -- host-relay shared transcode.** Custom API route serving the
|
||||
host session's HLS playlist/segments to other members; handle host
|
||||
disconnect/reselection.
|
||||
- **Phase 3 -- active drift correction.** Virtual playhead, clock calibration,
|
||||
heartbeat + EMA, two-tier nudge/reseek correction, scheduled absolute-time
|
||||
commands.
|
||||
- **Phase 0 -- scaffold & dev loop. DONE.** `dotnet` plugin project scaffold
|
||||
against `Jellyfin.Controller`/`Jellyfin.Model` 10.11.6 (public SDK was
|
||||
sufficient for this trivial plugin), local docker dev Jellyfin
|
||||
(`linuxserver/jellyfin:10.11.6`) with bind-mounted plugins folder, confirmed
|
||||
via container logs: `Loaded plugin: JellyfinSyncPlus 0.1.0.0`. Gotcha
|
||||
discovered: in `linuxserver/jellyfin`, the real plugins directory is
|
||||
`/config/data/plugins`, **not** `/config/plugins` (the latter exists but is
|
||||
unused/dead) -- `dev/docker-compose.yml` bind-mounts to the correct path
|
||||
now, don't regress this on a future compose edit.
|
||||
- **Phase 1 -- group quality negotiation. REVERTED 2026-07-09, superseded by
|
||||
Phase 3's approach below.** Built and live-verified (bitrate cap negotiation
|
||||
via `GroupQualityStore`/`SyncPlusPlaybackInfoFilter`, working correctly
|
||||
against real multi-device sessions -- see git history for the full account
|
||||
if this needs resurrecting). Torn out after a real, serious bug surfaced in
|
||||
actual use: switching quality mid-playback forces Jellyfin to issue a new
|
||||
`PlaySessionId` and reinitialize the player; our filter mutating that
|
||||
request made the client freeze on the switch and then **never resync
|
||||
afterward** -- worse than the drift it was meant to prevent. Root problem
|
||||
wasn't the negotiation math, it was forcing anything through the
|
||||
`PlaybackInfo` request/response path at all, since Jellyfin has no seam
|
||||
there for a plugin to intervene without looking, to the client, exactly
|
||||
like a user-initiated quality change. Replaced by active, corrective
|
||||
drift-correction instead of preventive path-forcing (Phase 3).
|
||||
- **Phase 2 -- host-relay shared transcode. REVERTED 2026-07-09, same
|
||||
reason as Phase 1** (the relay redirect went through the same
|
||||
`PlaybackInfo` response mutation that caused the freeze/permanent-desync
|
||||
bug). The engineering was sound and fully live-verified end-to-end
|
||||
against real video (real bug found and fixed along the way: browsers
|
||||
fetch HLS playlist/segment URLs with no auth header, only Jellyfin's
|
||||
`ApiKey=` query-param convention, which the relay didn't originally
|
||||
include) -- but it's not worth resurrecting unless a future need
|
||||
specifically requires deduplicating transcode load across a group, since
|
||||
Phase 3's correction now handles the drift problem this was meant to
|
||||
solve, without the instability. See git history for the implementation if
|
||||
transcode load dedup becomes a real requirement later.
|
||||
- **Phase 3 -- active drift correction. TRIED, REVERTED 2026-07-09.** Built a
|
||||
scoped-down version of this section's original plan (server watches
|
||||
standard `PlaybackProgress` reports, seeks back any member who drifts more
|
||||
than 3 seconds ahead of the group's laggard, one-directional only, plus a
|
||||
toast notification -- all via existing Jellyfin mechanisms, no custom
|
||||
client). Confirmed along the way that stock Jellyfin's web client already
|
||||
ships real NTP-style clock sync and two-tier correction
|
||||
(`syncPlay-core-PlaybackCore.*.chunk.js`, `SpeedToSync`/`SkipToSync`, with
|
||||
a real settings UI) and keeps steady-state drift bounded to ~70-340ms on
|
||||
its own -- that finding still stands and is worth remembering if this gets
|
||||
revisited. But the *active* correction on top of it didn't hold up in
|
||||
real use: a manual timeline seek triggered a feedback loop (fixed once --
|
||||
needed a per-device cooldown and to stop trusting an optimistically-
|
||||
written position as the "laggard" reference -- see git history for the
|
||||
full bug writeup) and even after that fix the user's call was that the
|
||||
behavior still wasn't right in practice. Pulled entirely rather than
|
||||
chase it further: `SyncPlusDriftCorrector`, `SyncPlusSyncPlayManager`,
|
||||
`IGroupMembershipStore`/`GroupMembershipStore`, and the
|
||||
`Emby.Server.Implementations.dll` reference they needed are all gone. The
|
||||
plugin is back to the Phase 0 baseline: loads, does nothing. All three
|
||||
original pillars (quality negotiation, host-relay, active correction)
|
||||
have now been tried and reverted -- see git history for any of them if a
|
||||
future direction wants to resurrect pieces, but don't restart from these
|
||||
designs without addressing why each one broke in real use (see the
|
||||
Phase 1/2 entries above and this one).
|
||||
- **Phase 4 -- disconnects, late joiners, seek/pause polish.**
|
||||
- **Phase 5 -- packaging & release pipeline.** Gitea-hosted plugin repository
|
||||
manifest, versioned zip releases, mirroring MR-Discord's pipeline.
|
||||
|
||||
## Stats for nerds (added 2026-07-09, outside the original three-pillar scope)
|
||||
|
||||
**DONE, live-verified.** After all three active-intervention pillars got
|
||||
reverted, the next thing built was purely observational instead: a
|
||||
YouTube-"stats for nerds"-style live view of every SyncPlay group's members,
|
||||
their drift relative to each other, and their playback method/bitrate/codec.
|
||||
Deliberately has zero effect on playback -- it only reads
|
||||
`ISessionManager.Sessions` live on each request, no in-memory state, no
|
||||
corrective action -- so it can't have the feedback-loop or freeze failure
|
||||
modes the reverted features did.
|
||||
|
||||
- `SyncPlusStatsController` (`GET /SyncPlus/Stats`, admin-only via
|
||||
`MediaBrowser.Common.Api.Policies.RequiresElevation`) returns a live JSON
|
||||
snapshot: for each SyncPlay group (from the passive `IGroupMembershipStore`/
|
||||
`SyncPlusSyncPlayManager` tracking -- same decorator pattern as before,
|
||||
minus anything that mutates a request or issues a command), each member's
|
||||
device/user name, now-playing item, position, drift in ms versus the
|
||||
group's laggard, pause state, play method, and transcode
|
||||
bitrate/codec/container/reason.
|
||||
- **Real blocker found and worked around**: originally built as an
|
||||
interactive plugin config page (`configPage.html`, reachable from
|
||||
Dashboard -> Plugins), but its inline `<script>` never ran -- confirmed
|
||||
live by inspecting the actual DOM after injection: 0 `<script>` elements
|
||||
survived. Jellyfin's web client runs plugin config page HTML through
|
||||
DOMPurify before injecting it into the SPA, which strips script tags
|
||||
entirely (also confirmed the older `pageshow`/`pagehide` custom events
|
||||
legacy plugin pages relied on aren't dispatched by the current
|
||||
React-based client either -- tried that first, same silent no-op result).
|
||||
No script tag survives that DOMPurify pass, regardless of what would have
|
||||
triggered it.
|
||||
- **Fix**: serve the interactive page as a genuine standalone HTML response
|
||||
instead (`GET /SyncPlus/Stats/Page`, `[AllowAnonymous]` since the page
|
||||
shell itself has nothing sensitive in it) -- a real page navigation isn't
|
||||
run through the SPA's sanitizer, so inline `<script>` works normally
|
||||
there. The page reads the current user's token straight out of the same
|
||||
`jellyfin_credentials` localStorage entry the main web client itself uses
|
||||
(same origin, so no separate login needed as long as the admin is already
|
||||
logged into Jellyfin in that browser) and uses it to poll the real,
|
||||
auth-gated `/SyncPlus/Stats` endpoint every second. The plugin's actual
|
||||
Dashboard config page is now just a static link to this standalone page --
|
||||
static HTML links work fine even with scripts stripped.
|
||||
- **Live-verified 2026-07-09**: confirmed the DOMPurify script-stripping
|
||||
diagnosis directly (0 script tags in the post-injection DOM), confirmed
|
||||
the standalone page loads and polls correctly (timestamp advancing
|
||||
second-to-second, no console errors), and confirmed a real SyncPlay group
|
||||
join shows up correctly in the JSON response (device/user name populated
|
||||
immediately; position/bitrate fields are null until actual playback
|
||||
starts, which is correct -- there's nothing to report yet).
|
||||
- Not yet tested: real multi-device drift numbers (only one browser session
|
||||
was available this round) and the config-page link's `target="_blank"`
|
||||
actually opening a new tab in a real browser (vs. this session's
|
||||
single-tab preview environment, where it was verified by direct
|
||||
navigation instead).
|
||||
- **Real bug found and fixed same day**: the stats page's JS assumed
|
||||
camelCase JSON property names (`group.members`, `m.deviceId`), but
|
||||
Jellyfin's API serializes in PascalCase (`Members`, `DeviceId`) --
|
||||
confirmed against the actual response body, which had been sitting right
|
||||
there in an earlier curl test output the whole time. Threw `Cannot read
|
||||
properties of undefined (reading 'forEach')` as soon as a real group
|
||||
existed to render. Fixed by matching the JS property access to the DTOs
|
||||
exactly; worth double-checking actual response shapes against assumptions
|
||||
before shipping JS that consumes them, not just assuming a convention.
|
||||
- **Manual "sync me" button added 2026-07-09.** User-requested: a one-shot,
|
||||
human-triggered action to fix your own drift on demand, as an alternative
|
||||
to the automatic corrector that got reverted for looping. `POST
|
||||
/SyncPlus/Stats/Sync` -- any authenticated user (not admin-gated like
|
||||
`GetStats`, since it only ever touches the caller's own session) seeks
|
||||
their own device to match whoever else in their group is furthest behind,
|
||||
using the exact same single-session `SendSyncPlayCommand` seek mechanism
|
||||
the reverted auto-corrector used. The difference that matters: this only
|
||||
ever fires once, when a human clicks a button -- there's no "again" until
|
||||
someone clicks again, so it structurally can't produce the
|
||||
re-correct-faster-than-the-previous-seek-lands feedback loop that broke
|
||||
the automatic version. No-ops safely (with an explanatory message, not an
|
||||
error) if the caller isn't in a group, isn't playing anything, or is
|
||||
already at or behind the group's laggard -- never pulls
|
||||
anyone but the caller, and never moves the caller forward past the
|
||||
laggard's position (same spoiler-avoidance principle as before). Button
|
||||
added to the stats page (`/SyncPlus/Stats/Page`).
|
||||
Live-verified 2026-07-09: confirmed both no-op paths fire correctly and
|
||||
with the right message ("not in a group", "nothing playing on this
|
||||
device"). Not yet tested: the actual successful-correction path with a
|
||||
real drifted second device (only one browser session available this
|
||||
round, same recurring constraint as everything else that needs two real
|
||||
logins).
|
||||
|
||||
## Open questions (revisit before/at the relevant phase, not now)
|
||||
|
||||
- Exact client compatibility target for v1 -- web client only, or also
|
||||
|
||||
Reference in New Issue
Block a user