jellyfin: actually report playback progress to the server
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>
This commit is contained in:
@@ -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');
|
||||
|
||||
Reference in New Issue
Block a user