Skip to content

Commit 59dccb3

Browse files
authored
fix(player): clear progress on episode switch to stop end-of-playback glitch (#94) (#194)
* fix(player): clear progress on episode switch to stop end-of-playback glitch (#94) When an episode finishes and the queue auto-advances, currentEpisode.set swaps the player to the next episode and its title/artwork update immediately, but the $currentTime/$duration stores keep the finished episode's end values until the new audio fires loadedmetadata. The progress bar therefore stays pinned at 100% and the timestamps show the previous episode's end against the new episode for the whole (network-bound) metadata-fetch window — the visual glitch in #94. Reset $currentTime and $duration to 0 on a genuine in-player episode switch (in the currentEpisode subscription), guarded by a first-fire flag so the synchronous initial subscription does not wipe a restored resume position or flash 0 on mount/remount. onMetadataLoaded -> restorePlaybackTime sets the real position once the next episode loads, and the existing isLoading guard keeps this reset from being persisted. Also clamp non-finite and negative inputs to 0 in formatSeconds so the time display can never render "NaN:NaN:NaN" or negative components during the brief window where duration is unknown or currentTime exceeds a shorter next episode. Verified in real Obsidian: at the auto-advance transition the bar now shows 0% and 00:00:00 instead of the stale 100% / previous-episode timestamps. * fix(player): never mark an unknown-duration episode finished on switch (#94) Addresses Codex review on PR #194. The #94 progress reset sets currentTime and duration to 0 the instant the episode changes. If the user switches away again before the next episode's metadata loads (e.g. click B then C from the queue while B is still loading), currentEpisode.set reads ct === dur === 0 and its isFinished = (ct === dur) check persisted the skipped episode as finished at 0:00 — surfacing a never-played episode as played. Guard the finished computation against a zero/unknown duration in both persist paths: currentEpisode.set (store) and persistPlaybackPosition (player). A duration of 0 (or NaN) is never "finished". Genuine finishes (duration > 0 and currentTime === duration) are unaffected. Reproduced and verified fixed in real Obsidian: rapid A->B->C switch now leaves B as finished=false, while an episode played to its real end stays finished=true.
1 parent 33399f5 commit 59dccb3

6 files changed

Lines changed: 153 additions & 2 deletions

File tree

src/store/index.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,10 @@ import { LOCAL_FILES_SETTINGS, QUEUE_SETTINGS } from "src/constants";
99
import { QueueController } from "src/store_controllers/QueueController";
1010
import {
1111
currentEpisode,
12+
currentTime,
1213
dedupeEpisodesByTitle,
1314
downloadedEpisodes,
15+
duration,
1416
localFiles,
1517
playedEpisodes,
1618
plugin,
@@ -505,3 +507,42 @@ describe("queue automation toggle (issue #108)", () => {
505507
expect(queueTitles()).toEqual(["A", "B"]);
506508
});
507509
});
510+
511+
describe("currentEpisode.set finished guard (issue #94)", () => {
512+
beforeEach(() => {
513+
playedEpisodes.set({});
514+
currentTime.set(0);
515+
duration.set(0);
516+
currentEpisode.set(undefined as unknown as Episode, false);
517+
});
518+
519+
afterEach(() => {
520+
currentTime.set(0);
521+
duration.set(0);
522+
currentEpisode.set(undefined as unknown as Episode, false);
523+
});
524+
525+
test("does not persist a never-played episode as finished when switched away at unknown (0) duration", () => {
526+
// The player resets currentTime/duration to 0 the instant the episode
527+
// changes (#94). Rapidly switching A -> B -> C before B's metadata loads
528+
// reads ct === dur === 0; that must not mark the skipped episode finished.
529+
currentEpisode.set(ep("A"), false);
530+
currentTime.set(0);
531+
duration.set(0);
532+
currentEpisode.set(ep("B"), false);
533+
534+
const stored = get(playedEpisodes)["Pod::A"];
535+
expect(stored?.finished).toBe(false);
536+
});
537+
538+
test("still persists a genuinely finished episode as finished", () => {
539+
currentEpisode.set(ep("A"), false);
540+
currentTime.set(3600);
541+
duration.set(3600);
542+
currentEpisode.set(ep("B"), false);
543+
544+
const stored = get(playedEpisodes)["Pod::A"];
545+
expect(stored?.finished).toBe(true);
546+
expect(stored?.time).toBe(3600);
547+
});
548+
});

src/store/index.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,12 @@ export const currentEpisode = (() => {
4141

4242
const ct = get(currentTime);
4343
const dur = get(duration);
44-
const isFinished = ct === dur;
44+
// A zero/unknown duration is never a finished episode. The player
45+
// resets currentTime/duration to 0 the instant the episode changes
46+
// (issue #94); if the user switches away again before the next
47+
// episode's metadata loads, ct === dur === 0 would otherwise persist
48+
// a never-played episode as finished at 0:00 and surface it as played.
49+
const isFinished = dur > 0 && ct === dur;
4550
playedEpisodes.setEpisodeTime(previousEpisode, ct, dur, isFinished);
4651
}
4752

src/ui/PodcastView/EpisodePlayer.svelte

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@
5757
let isHoveringArtwork: boolean = false;
5858
let isLoading: boolean = true;
5959
let playerVolume: number = 1;
60+
// The currentEpisode subscription fires synchronously on subscribe with the
61+
// already-loaded episode; that first fire must not wipe a restored position
62+
// (or flash 0) on the initial mount. Only genuine in-player switches reset.
63+
let hasSeenFirstEpisodeFire: boolean = false;
6064
let chapters: Chapter[] = [];
6165
let lastChaptersUrl: string | undefined = undefined;
6266
@@ -189,7 +193,9 @@
189193
$currentEpisode,
190194
$currentTime,
191195
$duration,
192-
$currentTime === $duration,
196+
// A zero/unknown duration is never "finished" — guard against the brief
197+
// 0/0 window after an episode switch (issue #94) marking it played at 0:00.
198+
$duration > 0 && $currentTime === $duration,
193199
);
194200
}
195201
@@ -227,6 +233,23 @@
227233
isLoading = true;
228234
lastPositionSaveMs = Number.NEGATIVE_INFINITY;
229235
236+
// Clear the outgoing episode's progress the instant the episode changes
237+
// so the player never renders its full/last position against the
238+
// incoming episode while the new audio's metadata loads (issue #94).
239+
// Finishing an episode auto-advances by calling currentEpisode.set (see
240+
// queue.playNext), which swaps the title/artwork immediately — but
241+
// $currentTime/$duration keep the finished episode's end values until
242+
// the next episode's loadedmetadata fires. Without this reset the
243+
// progress bar stays pinned at 100% and the timestamps show the
244+
// previous episode's end for the whole (network-bound) metadata fetch.
245+
// onMetadataLoaded → restorePlaybackTime sets the real position, and
246+
// the isLoading guard above keeps this reset from being persisted.
247+
if (hasSeenFirstEpisodeFire) {
248+
currentTime.set(0);
249+
duration.set(0);
250+
}
251+
hasSeenFirstEpisodeFire = true;
252+
230253
srcPromise = getSrc($currentEpisode);
231254
232255
// Fetch chapters when episode changes

src/ui/PodcastView/EpisodePlayer.test.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,52 @@ describe("EpisodePlayer — persists playback position during playback (issue #3
215215
expect(get(playedEpisodes)[keyB]?.time).toBe(1800);
216216
});
217217

218+
test("resets progress immediately on episode switch so the finished episode's bar is not shown against the next (issue #94)", async () => {
219+
const episodeB: Episode = {
220+
title: "Episode B",
221+
streamUrl: "https://pod.example.com/b.mp3",
222+
url: "https://pod.example.com/b",
223+
description: "",
224+
content: "",
225+
podcastName: "Test Podcast",
226+
};
227+
228+
const { container } = render(EpisodePlayer);
229+
await waitFor(() => {
230+
expect(container.querySelector("audio")).not.toBeNull();
231+
});
232+
const audio = container.querySelector("audio") as HTMLAudioElement;
233+
await fireEvent.loadedMetadata(audio);
234+
235+
// Simulate episode A finishing: progress pinned at the very end.
236+
currentTime.set(3600);
237+
duration.set(3600);
238+
await fireEvent.timeUpdate(audio);
239+
240+
// Auto-advance (queue.playNext) swaps the episode via currentEpisode.set.
241+
currentEpisode.set(episodeB);
242+
await waitFor(() => {
243+
const next = container.querySelector("audio");
244+
expect(next?.getAttribute("src")).toBe(episodeB.streamUrl);
245+
});
246+
247+
// Before B's loadedmetadata fires, the finished episode's playback position
248+
// must be cleared so the UI renders a zeroed loading state — not A's full
249+
// bar — for the duration of B's (network-bound) metadata fetch.
250+
expect(get(currentTime)).toBe(0);
251+
252+
// The user-visible outcome: the progress bar collapses to 0% and neither
253+
// timestamp shows A's end or a garbled "NaN" (the bar/text must not lag the
254+
// title/artwork switch).
255+
const bar = container.querySelector(".progress__bar") as HTMLElement;
256+
expect(bar.style.width).toBe("0%");
257+
const [elapsed, remaining] = Array.from(
258+
container.querySelectorAll(".status-container span"),
259+
).map((s) => s.textContent);
260+
expect(elapsed).toBe("00:00:00");
261+
expect(remaining).toBe("00:00:00");
262+
});
263+
218264
test("persists immediately when the app is backgrounded (visibilitychange → hidden)", async () => {
219265
await renderLoadedPlayer();
220266
currentTime.set(222);

src/utility/formatSeconds.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import { describe, expect, test } from "vitest";
2+
import { formatSeconds } from "./formatSeconds";
3+
4+
describe("formatSeconds", () => {
5+
test("formats whole seconds into HH:mm:ss", () => {
6+
expect(formatSeconds(0, "HH:mm:ss")).toBe("00:00:00");
7+
expect(formatSeconds(3661, "HH:mm:ss")).toBe("01:01:01");
8+
});
9+
10+
test("clamps NaN to 0 instead of rendering 'NaN:NaN:NaN' (issue #94)", () => {
11+
// Happens mid episode-switch when duration is briefly unknown and the
12+
// remaining-time expression ($duration - $currentTime) evaluates to NaN.
13+
expect(formatSeconds(Number.NaN, "HH:mm:ss")).toBe("00:00:00");
14+
});
15+
16+
test("clamps negative input to 0 instead of rendering negative components (issue #94)", () => {
17+
// Happens when currentTime momentarily exceeds a shorter next episode's
18+
// duration, making the remaining time negative.
19+
expect(formatSeconds(-42, "HH:mm:ss")).toBe("00:00:00");
20+
});
21+
22+
test("clamps non-finite Infinity to 0", () => {
23+
expect(formatSeconds(Number.POSITIVE_INFINITY, "HH:mm:ss")).toBe(
24+
"00:00:00",
25+
);
26+
});
27+
});

src/utility/formatSeconds.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,15 @@
88
* - A: AM/PM, a: am/pm
99
*/
1010
export function formatSeconds(totalSeconds: number, format: string): string {
11+
// Clamp non-finite (NaN/Infinity) and negative inputs to 0 so the player never
12+
// renders garbled times like "NaN:NaN:NaN" or "-1:-1:-10". These arise during
13+
// an episode switch, when duration is briefly unknown (NaN) before the new
14+
// audio's metadata loads, or when currentTime momentarily exceeds a shorter
15+
// next episode's duration (issue #94).
16+
if (!Number.isFinite(totalSeconds) || totalSeconds < 0) {
17+
totalSeconds = 0;
18+
}
19+
1120
const hours = Math.floor(totalSeconds / 3600);
1221
const minutes = Math.floor((totalSeconds % 3600) / 60);
1322
const secs = Math.floor(totalSeconds % 60);

0 commit comments

Comments
 (0)