From 4b094aa3a510c766a3d328bc3a9ce893a7000be3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Padez?= <andre.padez@gmail.com>
Date: Tue, 4 Aug 2026 19:16:37 +0000
Subject: [PATCH] jellyfin: actually report playback progress to the server
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

nothing the player did ever reached jellyfin, so nothing ever appeared in continue watching
or in the currently-playing list. the proxy was fine — a progress POST through the sidecar
moves the resume point upstream and answers 204. the client was the problem.

useClient() rebuilds its verbs on every render, so `report` had a new identity every render,
so `sendReport` did, so the effect whose cleanup reports the final position re-ran on every
render — and that cleanup nulls planRef. planRef went null a few milliseconds after the
stream opened and every report after that returned early. the ten-second heartbeat never
fired either: its interval was cleared and restarted on every render, and timeupdate renders
about four times a second.

both are now held in refs, and the two effects have honest dependency lists. same treatment
for the negotiate mutation, which react query also rebuilds per render.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 .../src/apps/Jellyfin/VideoPlayer.tsx         | 49 ++++++++++++-------
 1 file changed, 31 insertions(+), 18 deletions(-)

diff --git a/src/workspaces/officerdev/src/apps/Jellyfin/VideoPlayer.tsx b/src/workspaces/officerdev/src/apps/Jellyfin/VideoPlayer.tsx
index f43de727..3b336de5 100644
--- a/src/workspaces/officerdev/src/apps/Jellyfin/VideoPlayer.tsx
+++ b/src/workspaces/officerdev/src/apps/Jellyfin/VideoPlayer.tsx
@@ -100,6 +100,12 @@ export const VideoPlayer = ({ id }: { id: string }) => {
   const planRef = useRef<PlaybackPlan | null>(null);
   const positionRef = useRef(0);
   const startedRef = useRef(false);
+  const idRef = useRef(id);
+  idRef.current = id;
+  // React Query hands back a new mutation object each render, so this is the same story as `report` below:
+  // keeping `open` stable is what keeps the start-once effect from re-running on every frame of playback.
+  const negotiateRef = useRef(negotiate);
+  negotiateRef.current = negotiate;
   // The chosen track and rung, kept in refs so `open` does not have to be rebuilt (and the start-once effect
   // re-run) every time one of them changes.
   const audioRef = useRef<number | null>(null);
@@ -110,21 +116,26 @@ export const VideoPlayer = ({ id }: { id: string }) => {
   const duration = plan?.runtimeSeconds ?? ticksToSeconds(item?.RunTimeTicks);
   const subtitle = plan?.subtitles.find((track) => track.index === subtitleIndex && track.url) ?? null;
 
-  const sendReport = useCallback(
-    (action: 'playing' | 'progress' | 'stopped', isPaused = false) => {
-      const current = planRef.current;
-      if (!current) return;
-      report(action, {
-        ItemId: id,
-        PlaySessionId: current.playSessionId,
-        MediaSourceId: current.mediaSourceId,
-        PositionTicks: Math.max(0, Math.round(positionRef.current * TICKS_PER_SECOND)),
-        IsPaused: isPaused,
-        CanSeek: current.seekable,
-      });
-    },
-    [id, report],
-  );
+  // `report` is a NEW function on every render — `useClient()` rebuilds its verbs each time — so it is held in
+  // a ref rather than named as a dependency. This is not a micro-optimisation: an unstable `sendReport` made
+  // the "report where playback stopped" cleanup below re-run on every render, which nulled `planRef` a few
+  // milliseconds after the stream opened and turned every later report into a no-op. Nothing ever reached
+  // Jellyfin, so nothing ever appeared in Continue Watching.
+  const reportRef = useRef(report);
+  reportRef.current = report;
+
+  const sendReport = useCallback((action: 'playing' | 'progress' | 'stopped', isPaused = false) => {
+    const current = planRef.current;
+    if (!current) return;
+    reportRef.current(action, {
+      ItemId: idRef.current,
+      PlaySessionId: current.playSessionId,
+      MediaSourceId: current.mediaSourceId,
+      PositionTicks: Math.max(0, Math.round(positionRef.current * TICKS_PER_SECOND)),
+      IsPaused: isPaused,
+      CanSeek: current.seekable,
+    });
+  }, []);
 
   /**
    * Open (or re-open) the stream. In progressive mode a seek is exactly this call; so is any audio-track or
@@ -137,8 +148,8 @@ export const VideoPlayer = ({ id }: { id: string }) => {
       if (planRef.current) sendReport('stopped');
       setError(null);
       try {
-        const next = await negotiate.mutateAsync({
-          id,
+        const next = await negotiateRef.current.mutateAsync({
+          id: idRef.current,
           startSeconds,
           audioStreamIndex: audioRef.current ?? undefined,
           maxStreamingBitrate: bitrateRef.current ?? undefined,
@@ -152,7 +163,7 @@ export const VideoPlayer = ({ id }: { id: string }) => {
         setError((err as { message?: string } | null)?.message ?? 'Playback could not be started');
       }
     },
-    [id, negotiate, sendReport],
+    [sendReport],
   );
 
   // Start once, at the stored resume point. Guarded by a ref rather than by the dependency list because
@@ -197,6 +208,8 @@ export const VideoPlayer = ({ id }: { id: string }) => {
   }, [mode, plan, token]);
 
   // The stop report, on the way out. Also the only place the server learns a transcode is no longer wanted.
+  // The dependency list is empty and must stay empty — this cleanup nulls `planRef`, so anything that made it
+  // re-run mid-playback would silently switch off every report that follows.
   useEffect(
     () => () => {
       sendReport('stopped');