fix(player): re-entering a video no longer opens the audio player (DR-100)
Leaving a video and returning to it rendered the movie/episode in AudioPlayer. Closing a webview-rendered video deliberately emits no "stopped" state (that would break the autoplay handoff), and the direct-play path does not stop the backend on unmount, so the Rust controller still reported that item as its loaded media. Re-entering the route therefore took the "already playing, just show the UI" shortcut, which returns before a stream URL is fetched, and the render fell through to the audio surface. Mostly visible on Android, where video direct-plays; Linux transcodes and stops the backend on unmount. Both decisions move into playerSurface.ts as pure functions: shouldReuseActivePlayback excludes video, so video always takes the full load path and gets its stream URL and resume position; resolvePlayerSurface maps video-without-a-stream-URL to "pending" (spinner) rather than falling through to audio.
This commit is contained in:
@@ -0,0 +1,107 @@
|
||||
/**
|
||||
* Regression tests for the `/player/[id]` surface decision.
|
||||
*
|
||||
* The bug these pin down: a video that was left and re-entered rendered in the
|
||||
* AUDIO player. Exiting a webview-rendered video does not stop the Rust
|
||||
* controller (`onReportStop` deliberately emits no `stopped` state, so the
|
||||
* autoplay handoff survives), so the backend still reports that episode/movie as
|
||||
* the loaded media. Re-entering the route therefore took the "already playing,
|
||||
* just show the UI" shortcut, which returns *before* a stream URL is fetched —
|
||||
* and the render then fell through to `<AudioPlayer>` because it treated
|
||||
* "video without a stream URL" as audio.
|
||||
*
|
||||
* TRACES: UR-005 | DR-100 | UT-092, UT-093
|
||||
*/
|
||||
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { shouldReuseActivePlayback, resolvePlayerSurface } from "./playerSurface";
|
||||
|
||||
describe("shouldReuseActivePlayback", () => {
|
||||
it("reuses playback when the same audio track is already loaded", () => {
|
||||
expect(
|
||||
shouldReuseActivePlayback({
|
||||
requestedId: "track-1",
|
||||
activeMediaId: "track-1",
|
||||
isVideo: false,
|
||||
forceRestart: false,
|
||||
})
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("does NOT reuse playback for video, even when the backend reports it loaded", () => {
|
||||
// Video needs a full load: the shortcut skips fetching the stream URL, and
|
||||
// <VideoPlayer> cannot render without one.
|
||||
expect(
|
||||
shouldReuseActivePlayback({
|
||||
requestedId: "episode-1",
|
||||
activeMediaId: "episode-1",
|
||||
isVideo: true,
|
||||
forceRestart: false,
|
||||
})
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("does not reuse playback for a different item", () => {
|
||||
expect(
|
||||
shouldReuseActivePlayback({
|
||||
requestedId: "track-2",
|
||||
activeMediaId: "track-1",
|
||||
isVideo: false,
|
||||
forceRestart: false,
|
||||
})
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("does not reuse playback when nothing is loaded", () => {
|
||||
expect(
|
||||
shouldReuseActivePlayback({
|
||||
requestedId: "track-1",
|
||||
activeMediaId: null,
|
||||
isVideo: false,
|
||||
forceRestart: false,
|
||||
})
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("does not reuse playback when an explicit start position is requested", () => {
|
||||
expect(
|
||||
shouldReuseActivePlayback({
|
||||
requestedId: "track-1",
|
||||
activeMediaId: "track-1",
|
||||
isVideo: false,
|
||||
startPosition: 42,
|
||||
forceRestart: false,
|
||||
})
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("does not reuse playback when restarting (next-episode advance)", () => {
|
||||
expect(
|
||||
shouldReuseActivePlayback({
|
||||
requestedId: "episode-2",
|
||||
activeMediaId: "episode-2",
|
||||
isVideo: true,
|
||||
forceRestart: true,
|
||||
})
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolvePlayerSurface", () => {
|
||||
it("renders the video surface for video with a stream URL", () => {
|
||||
expect(resolvePlayerSurface({ isVideo: true, streamUrl: "http://s/master.m3u8" })).toBe(
|
||||
"video"
|
||||
);
|
||||
});
|
||||
|
||||
it("renders the audio surface for audio content", () => {
|
||||
expect(resolvePlayerSurface({ isVideo: false, streamUrl: null })).toBe("audio");
|
||||
});
|
||||
|
||||
it("never renders video content in the audio surface when the stream URL is missing", () => {
|
||||
// A video whose stream URL has not resolved yet is pending, not audio —
|
||||
// otherwise the movie/episode shows up in the audio player.
|
||||
expect(resolvePlayerSurface({ isVideo: true, streamUrl: null })).toBe("pending");
|
||||
expect(resolvePlayerSurface({ isVideo: true, streamUrl: "" })).toBe("pending");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,63 @@
|
||||
/**
|
||||
* Pure decisions for the `/player/[id]` route: which player surface to render,
|
||||
* and whether a load can be skipped because the backend is already playing the
|
||||
* requested item.
|
||||
*
|
||||
* Kept free of Svelte so both can be unit-tested without mounting the route.
|
||||
*
|
||||
* TRACES: UR-005 | DR-100 | UT-092, UT-093
|
||||
*/
|
||||
|
||||
/** Which player component the route should render. */
|
||||
export type PlayerSurface = "video" | "audio" | "pending";
|
||||
|
||||
export interface ReuseActivePlaybackInput {
|
||||
/** Item id the route was asked to play. */
|
||||
requestedId: string;
|
||||
/** Id of the media the backend currently reports as loaded, if any. */
|
||||
activeMediaId: string | null | undefined;
|
||||
/** Whether the requested item is video content. */
|
||||
isVideo: boolean;
|
||||
/** Explicit start position, if the caller asked for one. */
|
||||
startPosition?: number;
|
||||
/** Advancing to a next episode always restarts from the beginning. */
|
||||
forceRestart: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the route can show its UI over the backend's existing playback
|
||||
* instead of reloading the item (e.g. expanding the audio mini player).
|
||||
*
|
||||
* Never for video. The shortcut returns before a stream URL is fetched, which
|
||||
* is fine for audio (the backend owns the stream and the UI only mirrors it)
|
||||
* but leaves `<VideoPlayer>` with nothing to render. Leaving a webview-rendered
|
||||
* video does not clear the Rust controller's media — closing the route emits no
|
||||
* `stopped` state by design — so re-entering the same movie/episode hit this
|
||||
* shortcut and rendered the audio player instead.
|
||||
*/
|
||||
export function shouldReuseActivePlayback(input: ReuseActivePlaybackInput): boolean {
|
||||
return (
|
||||
!input.isVideo &&
|
||||
input.activeMediaId === input.requestedId &&
|
||||
!input.startPosition &&
|
||||
!input.forceRestart
|
||||
);
|
||||
}
|
||||
|
||||
export interface PlayerSurfaceInput {
|
||||
isVideo: boolean;
|
||||
streamUrl: string | null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Which surface to render for the loaded item.
|
||||
*
|
||||
* Video without a stream URL is `pending`, never `audio` — falling through to
|
||||
* the audio player is how a movie/episode ended up in it.
|
||||
*/
|
||||
export function resolvePlayerSurface(input: PlayerSurfaceInput): PlayerSurface {
|
||||
if (input.isVideo) {
|
||||
return input.streamUrl ? "video" : "pending";
|
||||
}
|
||||
return "audio";
|
||||
}
|
||||
@@ -14,6 +14,7 @@
|
||||
import { get } from "svelte/store";
|
||||
import AudioPlayer from "$lib/components/player/AudioPlayer.svelte";
|
||||
import VideoPlayer from "$lib/components/player/VideoPlayer.svelte";
|
||||
import { shouldReuseActivePlayback, resolvePlayerSurface } from "$lib/components/player/playerSurface";
|
||||
import NextEpisodePopup from "$lib/components/player/NextEpisodePopup.svelte";
|
||||
import {
|
||||
reportPlaybackStart,
|
||||
@@ -72,6 +73,10 @@
|
||||
let pollInterval: ReturnType<typeof setInterval> | null = null;
|
||||
let loadedItemId: string | null = null;
|
||||
|
||||
// Which player component to render. Video without a stream URL is "pending"
|
||||
// (still resolving), never audio — see playerSurface.ts.
|
||||
const surface = $derived(resolvePlayerSurface({ isVideo, streamUrl }));
|
||||
|
||||
onMount(() => {
|
||||
// Start position polling (only for audio via MPV backend)
|
||||
pollInterval = setInterval(updateStatus, 1000);
|
||||
@@ -137,30 +142,33 @@
|
||||
return;
|
||||
}
|
||||
|
||||
// If this track is already playing in the backend, just show the UI
|
||||
// without restarting playback (e.g., when expanding from MiniPlayer).
|
||||
// forceRestart bypasses this so advancing to the next episode always
|
||||
// restarts from the beginning even if it were already loaded.
|
||||
const alreadyPlayingMedia = get(storeCurrentMedia);
|
||||
if (alreadyPlayingMedia?.id === id && !startPosition && !forceRestart) {
|
||||
console.log("loadAndPlay: Track already playing, showing UI without restarting");
|
||||
isLive = item.kind === "liveChannel";
|
||||
isVideo = item.kind === "movie" || item.kind === "episode" || isLive || isVideoChannelItem(item);
|
||||
isPlaying = true;
|
||||
loading = false;
|
||||
// hasNext/hasPrevious come from the event-driven queue store.
|
||||
// Fetch next episode for video skip button
|
||||
if (isVideo) {
|
||||
fetchNextEpisode(item);
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
// Determine if this is video content (Movie, Episode, live TV channels, and
|
||||
// channel leaf items that carry a video stream).
|
||||
isLive = item.kind === "liveChannel";
|
||||
isVideo = item.kind === "movie" || item.kind === "episode" || isLive || isVideoChannelItem(item);
|
||||
|
||||
// If this track is already playing in the backend, just show the UI
|
||||
// without restarting playback (e.g., when expanding from MiniPlayer).
|
||||
// Audio only, and forceRestart bypasses it so advancing to the next
|
||||
// episode always restarts from the beginning — see playerSurface.ts for
|
||||
// why video must never take this shortcut.
|
||||
const alreadyPlayingMedia = get(storeCurrentMedia);
|
||||
if (
|
||||
shouldReuseActivePlayback({
|
||||
requestedId: id,
|
||||
activeMediaId: alreadyPlayingMedia?.id,
|
||||
isVideo,
|
||||
startPosition,
|
||||
forceRestart,
|
||||
})
|
||||
) {
|
||||
console.log("loadAndPlay: Track already playing, showing UI without restarting");
|
||||
isPlaying = true;
|
||||
loading = false;
|
||||
// hasNext/hasPrevious come from the event-driven queue store.
|
||||
return;
|
||||
}
|
||||
|
||||
// When switching to video, stop audio playback and clear the queue
|
||||
// This prevents audio from continuing in the background and clears stale state
|
||||
if (isVideo) {
|
||||
@@ -650,10 +658,6 @@
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
{:else if loading}
|
||||
<div class="fixed inset-0 bg-[var(--color-background)] flex items-center justify-center z-50">
|
||||
<div class="w-8 h-8 border-2 border-[var(--color-jellyfin)] border-t-transparent rounded-full animate-spin"></div>
|
||||
</div>
|
||||
{:else if error}
|
||||
<div class="fixed inset-0 bg-[var(--color-background)] flex items-center justify-center z-50 p-4">
|
||||
<div class="text-center max-w-lg">
|
||||
@@ -667,7 +671,13 @@
|
||||
</button>
|
||||
</div>
|
||||
</div>
|
||||
{:else if isVideo && streamUrl}
|
||||
{:else if loading || surface === "pending"}
|
||||
<!-- "pending" = video whose stream URL has not resolved yet. Showing the
|
||||
spinner keeps it out of the audio player. -->
|
||||
<div class="fixed inset-0 bg-[var(--color-background)] flex items-center justify-center z-50">
|
||||
<div class="w-8 h-8 border-2 border-[var(--color-jellyfin)] border-t-transparent rounded-full animate-spin"></div>
|
||||
</div>
|
||||
{:else if surface === "video" && streamUrl}
|
||||
<VideoPlayer
|
||||
media={currentMedia}
|
||||
{streamUrl}
|
||||
|
||||
Reference in New Issue
Block a user