From 0cda5b6dd485f891a830eb8fc66a9a1a762e7766 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Wed, 7 Oct 2026 01:48:10 +0200 Subject: [PATCH] fix(player): only the credits that end an item move on to the next one An item can carry several credits blocks with mid- and post-credit scenes between them, and Plex flags the one that really ends it. The player left at the first block. Now the Plex-final block moves on (by the auto-skip or the Skip button), an earlier block is only skipped to its end by the button, and with no final flag the last block ends the item by the button alone. --- backend/app/models.py | 3 +- backend/app/routes/plex/detail.py | 4 ++ backend/tests/test_plex_detail.py | 35 ++++++++++++ .../PlexPlayer.controls.dom.test.tsx | 2 +- .../PlexPlayer.session.dom.test.tsx | 54 ++++++++++++++++++- frontend/src/lib/api/plex.ts | 2 + frontend/src/lib/plexMarkers.test.ts | 47 ++++++++++++++++ frontend/src/lib/plexMarkers.ts | 33 ++++++++++++ frontend/src/lib/plexPlayerHooks.ts | 19 ++++--- 9 files changed, 189 insertions(+), 10 deletions(-) create mode 100644 frontend/src/lib/plexMarkers.test.ts create mode 100644 frontend/src/lib/plexMarkers.ts diff --git a/backend/app/models.py b/backend/app/models.py index 163026d4..b8b1a4fb 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -1274,7 +1274,8 @@ class PlexItem(Base, TimestampMixin, UpdatedAtMixin): # bit-depth/HDR. NULL = never (the backfill's work-list). Only HW-decodable transcode items are # probed (the rest are SW-only by codec and need no bit-depth); see `_probe_media_batch`. media_probed_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) - # Intro/credit markers: [{"type": "intro"|"credits", "start_s": int, "end_s": int}, ...] + # Intro/credit markers: [{"type": "intro"|"credits", "start_s": int, "end_s": int}, ...] — unused: + # the player reads them live from Plex (`routes/plex/detail.item_detail`, with Plex's `final`). markers: Mapped[list | None] = mapped_column(JSON) added_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), index=True) # Filterable / orderable metadata mirrored from the Plex section listing (no extra API calls; diff --git a/backend/app/routes/plex/detail.py b/backend/app/routes/plex/detail.py index 94318d1d..99de5974 100644 --- a/backend/app/routes/plex/detail.py +++ b/backend/app/routes/plex/detail.py @@ -404,6 +404,10 @@ def item_detail( "type": m.get("type"), "start_s": round((m.get("startTimeOffset") or 0) / 1000, 1), "end_s": round((m.get("endTimeOffset") or 0) / 1000, 1), + # An item can carry several credits blocks with scenes between them; Plex + # flags the one that really ends it. Which credits may leave the item is the + # player's rule (`lib/plexMarkers`), so the flag goes over as Plex sent it. + "final": bool(m.get("final")), } ) # Audio + subtitle tracks (ordinals among their stream type — used to -map the selection). diff --git a/backend/tests/test_plex_detail.py b/backend/tests/test_plex_detail.py index cca81afb..b9bbf62b 100644 --- a/backend/tests/test_plex_detail.py +++ b/backend/tests/test_plex_detail.py @@ -401,3 +401,38 @@ def test_item_detail_names_tracks_from_their_language_not_plexs_title( ("magyar Forced", "hu", True, False, None), ("English SDH", "en", False, True, None), ] + + +def test_item_detail_markers_carry_plex_final_flag(db, monkeypatch, override_dep): + """An item can carry several credits markers — mid- and post-credit scenes sit between them — + and only the one Plex flags `final` is the real end; the player needs that flag to know which + credits may move on to the next item.""" + lib = PlexLibrary(plex_key="1", title="Movies", kind="movie", enabled=True) + db.add(lib) + db.flush() + db.add(PlexItem(rating_key="m2", library_id=lib.id, kind="movie", title="Iron Man")) + db.commit() + user = _user(db, "markers@example.com") + db.commit() + + meta = { + "Marker": [ + {"type": "intro", "startTimeOffset": 1000, "endTimeOffset": 60000}, + {"type": "credits", "startTimeOffset": 6811000, "endTimeOffset": 6831000}, + { + "type": "credits", + "startTimeOffset": 7549000, + "endTimeOffset": 7559000, + "final": True, + }, + {"type": "commercial", "startTimeOffset": 1, "endTimeOffset": 2}, + ] + } + monkeypatch.setattr(detail_routes, "PlexClient", lambda db: _StubClient(meta)) + resp = _get(db, user, monkeypatch, override_dep, "/api/plex/item/m2") + assert resp.status_code == 200, resp.text + assert resp.json()["markers"] == [ + {"type": "intro", "start_s": 1.0, "end_s": 60.0, "final": False}, + {"type": "credits", "start_s": 6811.0, "end_s": 6831.0, "final": False}, + {"type": "credits", "start_s": 7549.0, "end_s": 7559.0, "final": True}, + ] diff --git a/frontend/src/components/PlexPlayer.controls.dom.test.tsx b/frontend/src/components/PlexPlayer.controls.dom.test.tsx index f02d71cf..1db2467d 100644 --- a/frontend/src/components/PlexPlayer.controls.dom.test.tsx +++ b/frontend/src/components/PlexPlayer.controls.dom.test.tsx @@ -358,7 +358,7 @@ describe("keyboard focus in the bands", () => { it("a Space or Enter on Skip intro, outside the faded bands, stays Skip intro's", async () => { const intro = { ...item("A"), - markers: [{ type: "intro", start_s: 50, end_s: 200 }], + markers: [{ type: "intro", start_s: 50, end_s: 200, final: false }], } as unknown as PlexItemDetail; mount({ details: { A: intro } }); await playingAt("/media/A", 100); diff --git a/frontend/src/components/PlexPlayer.session.dom.test.tsx b/frontend/src/components/PlexPlayer.session.dom.test.tsx index 4cf57eb6..2a2b3603 100644 --- a/frontend/src/components/PlexPlayer.session.dom.test.tsx +++ b/frontend/src/components/PlexPlayer.session.dom.test.tsx @@ -334,7 +334,7 @@ describe("leaving an item that was played through", () => { prefs({ autoSkipCredits: true, autoSkipDelay: 0 }); const withCredits = { ...item("A"), - markers: [{ type: "credits", start_s: 2900, end_s: 2995 }], + markers: [{ type: "credits", start_s: 2900, end_s: 2995, final: true }], } as unknown as PlexItemDetail; const { qc, progress } = mount({ details: { A: withCredits, B: item("B") }, @@ -446,7 +446,7 @@ describe("leaving an item that was played through", () => { prefs({ upNext: false, autoSkipCredits: true, autoSkipDelay: 0 }); const withCredits = { ...item("A"), - markers: [{ type: "credits", start_s: 2000, end_s: 2990 }], + markers: [{ type: "credits", start_s: 2000, end_s: 2990, final: true }], } as unknown as PlexItemDetail; const { qc, progress } = mount({ details: { A: withCredits, B: item("B") }, @@ -463,6 +463,56 @@ describe("leaving an item that was played through", () => { expect(qc.getQueryData(qk.plexItem("A", "en"))?.status).not.toBe("watched"); }); + it("lets a credits block that does not end the item play on, its Skip only jumping past it", async () => { + // An item can carry several credits blocks with scenes between them: the first one moving on + // to the next item skipped a post-credit scene. + prefs({ autoSkipCredits: true, autoSkipDelay: 0 }); + const twoCredits = { + ...item("A"), + markers: [ + { type: "credits", start_s: 2000, end_s: 2100, final: false }, + { type: "credits", start_s: 2900, end_s: 2995, final: true }, + ], + } as unknown as PlexItemDetail; + const { qc } = mount({ + details: { A: twoCredits, B: item("B") }, + sessions: { A: () => Promise.resolve(direct("A")), B: () => new Promise(() => {}) }, + queue: ["A", "B"], + }); + const setState = vi.spyOn(api, "plexSetState").mockResolvedValue(undefined as never); + await playingAt("/media/A", 2010); + await tick(); + expect(screen.queryByText("Title B")).toBeNull(); + expect(qc.getQueryData(qk.plexItem("B", "en"))).toBeUndefined(); + fireEvent.click(screen.getByRole("button", { name: /plex\.player\.skipCredits/ })); + await waitFor(() => expect(startsAsked("A").slice(-1)).toEqual([2100])); // past the block, still A + expect(screen.queryByText("Title B")).toBeNull(); + expect(setState).not.toHaveBeenCalled(); + await playingAt("/media/A", 2910); // the final block: the auto-skip moves on + await screen.findByText("Title B"); + expect(setState).toHaveBeenCalledWith("A", "watched"); + }); + + it("does not leave on its own at credits Plex did not flag final, but its Skip does", async () => { + prefs({ autoSkipCredits: true, autoSkipDelay: 0 }); + const unflagged = { + ...item("A"), + markers: [{ type: "credits", start_s: 2900, end_s: 2995, final: false }], + } as unknown as PlexItemDetail; + mount({ + details: { A: unflagged, B: item("B") }, + sessions: { A: () => Promise.resolve(direct("A")), B: () => new Promise(() => {}) }, + queue: ["A", "B"], + }); + const setState = vi.spyOn(api, "plexSetState").mockResolvedValue(undefined as never); + await playingAt("/media/A", 2910); + await tick(); + expect(screen.queryByText("Title B")).toBeNull(); + fireEvent.click(screen.getByRole("button", { name: /plex\.player\.skipCredits/ })); + await screen.findByText("Title B"); + expect(setState).toHaveBeenCalledWith("A", "watched"); + }); + it("records a second viewing when the viewer plays on from the finished frame", async () => { // `finished` is a latch that makes every checkpoint a no-op; the only thing that lowers it is // the video's own 'play' event. Nothing tested that, so dropping the listener would have diff --git a/frontend/src/lib/api/plex.ts b/frontend/src/lib/api/plex.ts index cf511093..2a058a51 100644 --- a/frontend/src/lib/api/plex.ts +++ b/frontend/src/lib/api/plex.ts @@ -132,6 +132,8 @@ export interface PlexMarker { type: "intro" | "credits"; start_s: number; end_s: number; + /** Plex's own word that this credits block ends the item (see `lib/plexMarkers`). */ + final: boolean; } // What the player names an audio/subtitle track from (`lib/vocab` trackLabel). Plex's own title // (`label`) is in the Plex SERVER's language, so it is only the fallback for a track with no language tag. diff --git a/frontend/src/lib/plexMarkers.test.ts b/frontend/src/lib/plexMarkers.test.ts new file mode 100644 index 00000000..df61f1d1 --- /dev/null +++ b/frontend/src/lib/plexMarkers.test.ts @@ -0,0 +1,47 @@ +// Which credits block ends an item, and what skipping each marker does. An item can carry several +// credits blocks — mid- and post-credit scenes play between them — and only the last one may move +// on to the next item; only the one Plex itself flags as the end may do so on its own. +import { describe, it, expect } from "vitest"; + +import type { PlexMarker } from "./api"; +import { markerSkip } from "./plexMarkers"; + +const credits = (start_s: number, end_s: number, final = false): PlexMarker => ({ + type: "credits", + start_s, + end_s, + final, +}); +const intro: PlexMarker = { type: "intro", start_s: 10, end_s: 60, final: false }; + +describe("markerSkip", () => { + it("an intro jumps to its end, on its own when the viewer wants that", () => { + expect(markerSkip(intro, [intro])).toEqual({ leaves: false, auto: true }); + }); + + it("only the credits Plex flags final leave the item, and only they may do it on their own", () => { + const first = credits(6811, 6831); + const middle = credits(7039, 7507); + const end = credits(7549, 7559, true); + const all = [intro, first, middle, end]; + expect(markerSkip(first, all)).toEqual({ leaves: false, auto: false }); + expect(markerSkip(middle, all)).toEqual({ leaves: false, auto: false }); + expect(markerSkip(end, all)).toEqual({ leaves: true, auto: true }); + }); + + it("with no credits flagged final the last one is the end, but leaves only when asked", () => { + const first = credits(2000, 2100); + const last = credits(2900, 2995); + // listed out of order on purpose: "last" is by time, not by position in the list + expect(markerSkip(last, [last, first])).toEqual({ leaves: true, auto: false }); + expect(markerSkip(first, [last, first])).toEqual({ leaves: false, auto: false }); + expect(markerSkip(last, [last])).toEqual({ leaves: true, auto: false }); + }); + + it("a final flag on an earlier block wins over a later block", () => { + const end = credits(2000, 2100, true); + const later = credits(2900, 2995); + expect(markerSkip(end, [end, later])).toEqual({ leaves: true, auto: true }); + expect(markerSkip(later, [end, later])).toEqual({ leaves: false, auto: false }); + }); +}); diff --git a/frontend/src/lib/plexMarkers.ts b/frontend/src/lib/plexMarkers.ts new file mode 100644 index 00000000..b38033d0 --- /dev/null +++ b/frontend/src/lib/plexMarkers.ts @@ -0,0 +1,33 @@ +// What skipping an intro/credits marker does. An item can carry several credits blocks — mid- and +// post-credit scenes play between them — so only the block that ends the item moves on to the next +// one: the block Plex flags `final`, else the last one. Every other credits block is skipped like an +// intro, to its own end. And only an end Plex itself vouches for is left on its own (the credits +// auto-skip); an end inferred from the list waits for the viewer's Skip — the stretch after it may +// still be the show. +import type { PlexMarker } from "./api"; + +/** `leaves`: skipping it moves on to the next item (else: jumps to its end). `auto`: the auto-skip + * setting may act on it without the viewer. */ +export interface MarkerSkip { + leaves: boolean; + auto: boolean; +} + +function endCredits(markers: readonly PlexMarker[]): PlexMarker | undefined { + const credits = markers.filter((m) => m.type === "credits"); + return ( + credits.find((m) => m.final) ?? + credits.reduce( + (last, m) => (last && last.start_s > m.start_s ? last : m), + undefined, + ) + ); +} + +export function markerSkip(marker: PlexMarker, markers: readonly PlexMarker[]): MarkerSkip { + if (marker.type === "intro") return { leaves: false, auto: true }; + // By offset, not identity: a refetched list hands back equal-but-new objects. + const end = endCredits(markers); + const leaves = end != null && end.start_s === marker.start_s && end.end_s === marker.end_s; + return { leaves, auto: leaves && marker.final }; +} diff --git a/frontend/src/lib/plexPlayerHooks.ts b/frontend/src/lib/plexPlayerHooks.ts index e01e5d7b..1a99c5ee 100644 --- a/frontend/src/lib/plexPlayerHooks.ts +++ b/frontend/src/lib/plexPlayerHooks.ts @@ -17,6 +17,7 @@ import { api, type PlexItemDetail, type PlexMarker } from "./api"; import { qk } from "./queryKeys"; import { useLatest } from "./useLatest"; import i18n, { plexLang } from "../i18n"; +import { markerSkip } from "./plexMarkers"; import { type PlexPlayerPrefs } from "./plexPlayerPrefs"; import { PLEX_SCALE, normalizeVolumeIn, stepVolumeIn } from "./playerVolume"; @@ -411,9 +412,10 @@ export function usePlayerVolume(opts: { } // Auto-skip intro/credits. Entering a marker whose auto-skip is on runs a countdown (skipProgress -// 0→1 over autoSkipDelay s, shown on the Skip button); on completion it skips — intro → jump to the -// marker end, credits → next item (binge). The returned refs (skipProgressRef / cancelAutoSkipRef) -// are the forward-declaration bridge the earlier-defined keyboard/back handlers read to cancel a +// 0→1 over autoSkipDelay s, shown on the Skip button); on completion it skips — to the marker end, or +// for the credits that end the item to the next item (binge). Which credits those are, and which the +// auto-skip may act on alone, is `lib/plexMarkers`. The returned refs (skipProgressRef / +// cancelAutoSkipRef) are the forward-declaration bridge the earlier-defined keyboard/back handlers read to cancel a // running countdown (Esc/Backspace during the countdown cancels it and must NOT navigate back). export function useAutoSkip(opts: { id: string; @@ -453,6 +455,10 @@ export function useAutoSkip(opts: { (m) => abs >= m.start_s && abs < m.end_s - 1, ); const activeMarkerRef = useLatest(activeMarker); + // A boolean, not the list, goes into the countdown's deps: a refetch hands back an equal-but-new + // list and must not restart a countdown under way (see below); doSkip reads the list from a ref. + const activeAuto = activeMarker != null && markerSkip(activeMarker, markers ?? []).auto; + const markersRef = useLatest(markers); const [skipProgress, setSkipProgress] = useState(null); const skipProgressRef = useLatest(skipProgress); @@ -470,7 +476,7 @@ export function useAutoSkip(opts: { const doSkip = useCallback( (m: PlexMarker) => { setSkipProgress(null); - if (m.type === "credits" && nextId) { + if (nextId && markerSkip(m, markersRef.current ?? []).leaves) { // Binge auto-advance: the episode we're leaving was watched through the credits, so mark IT // (the current id, captured here — not the reactive id, which go() is about to change). The // write is handed to go() so a refusal takes the finish back: this path has no near-end @@ -481,7 +487,7 @@ export function useAutoSkip(opts: { go(nextId, watchedWrite); } else seekTo(m.end_s); }, - [id, go, nextId, seekTo, reportWatchedErrorRef], + [id, go, nextId, seekTo, markersRef, reportWatchedErrorRef], ); const cancelAutoSkip = useCallback(() => { const m = activeMarkerRef.current; @@ -503,7 +509,7 @@ export function useAutoSkip(opts: { const m = activeMarkerRef.current; if (!m) return setSkipProgress(null); const key = `${m.type}-${m.start_s}`; - const enabled = m.type === "intro" ? autoSkipIntro : autoSkipCredits; + const enabled = (m.type === "intro" ? autoSkipIntro : autoSkipCredits) && activeAuto; if (!enabled || cancelledMarkerRef.current === key) return setSkipProgress(null); if (autoSkipDelay <= 0) return doSkip(m); // immediate let elapsed = 0; @@ -535,6 +541,7 @@ export function useAutoSkip(opts: { doSkip, videoRef, activeMarkerRef, + activeAuto, ]); return { activeMarker, skipProgress, skipProgressRef, cancelAutoSkipRef, doSkip };