diff --git a/src/audio/player.test.ts b/src/audio/player.test.ts index ef020ec..9033ff4 100644 --- a/src/audio/player.test.ts +++ b/src/audio/player.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect } from "vitest"; +import { describe, it, expect, vi } from "vitest"; import { mkdtempSync, writeFileSync, existsSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -461,3 +461,128 @@ describe("AudioPlayer external-PCM mode (playPcmStream)", () => { player.stop(); }); }); + +// R3-4: the 20ms frame loop keeps running while paused (so a live-but-silent +// stream can refill on resume). But the stall/EOF end-detection branches MUST +// only run while state==="playing" — otherwise pausing a stalled or +// unknown-duration stream still accumulates emptyFrameAttempts and auto-emits +// trackEnd (~5s later), making the controller skip the paused track. +// +// These tests drive the real url-path frame loop (this.ffmpeg !== null, NOT +// external mode), which cannot be exercised via playPcmStream (that sets +// externalMode and suppresses both branches). We inject a fake live ffmpeg + +// an empty pcmBuffer (a stream that stays alive but never yields a full PCM +// frame) and run the actual startFrameLoop() under fake timers. `performance` +// is faked in lockstep with the timer clock so each tick advances a real 20ms, +// letting us cheaply cross MAX_EMPTY_ATTEMPTS (250 ticks ≈ 5s) deterministically. +describe("AudioPlayer stall/EOF end-detection is gated on playing state (R3-4)", () => { + // Fake `performance` in lockstep with the timer clock so each advanced 20ms is + // a real frame tick (the loop computes its delay from performance.now()). + const FAKE_TIMER_OPTS: Parameters[0] = { + toFake: ["setTimeout", "clearTimeout", "setInterval", "clearInterval", "Date", "performance"], + }; + + // A player primed as "playing" with a LIVE ffmpeg that never produces a full + // PCM frame (unknown duration -> isNearEnd forced true). startFrameLoop() runs + // the genuine loop; no real process is spawned (fake ffmpeg has no pid, so the + // end path never touches forceCleanup/process.kill). + function makeStalledPlaying(): AudioPlayer { + const player = new AudioPlayer(silentLogger); + const p = player as unknown as { + ffmpeg: unknown; + currentSongDuration: number; + pcmBuffer: Buffer; + emptyFrameAttempts: number; + framesPlayed: number; + state: string; + startFrameLoop(): void; + }; + p.ffmpeg = { pid: undefined }; // live ffmpeg, but delivers no PCM + p.currentSongDuration = 0; // unknown duration -> isNearEnd === true + p.pcmBuffer = Buffer.alloc(0); // always < one PCM frame + p.emptyFrameAttempts = 0; + p.framesPlayed = 0; + p.state = "playing"; + p.startFrameLoop(); + return player; + } + + it("does NOT emit trackEnd (and stays paused) when a stalled unknown-duration stream is paused past the stall threshold", () => { + vi.useFakeTimers(FAKE_TIMER_OPTS); + try { + const player = makeStalledPlaying(); + let ended = 0; + player.on("trackEnd", () => ended++); + + player.pause(); + expect(player.getState()).toBe("paused"); + + // Advance well past MAX_EMPTY_ATTEMPTS (250 ticks ≈ 5s): ~300 ticks. + vi.advanceTimersByTime(20 * 300); + + expect(ended).toBe(0); + expect(player.getState()).toBe("paused"); + player.stop(); + } finally { + vi.useRealTimers(); + } + }); + + it("STILL emits trackEnd when the SAME stalled unknown-duration stream is left playing (dead-stream recovery #89 preserved)", () => { + vi.useFakeTimers(FAKE_TIMER_OPTS); + try { + const player = makeStalledPlaying(); // stays "playing" + let ended = 0; + player.on("trackEnd", () => ended++); + + vi.advanceTimersByTime(20 * 300); // cross the 250-tick stall threshold + + expect(ended).toBe(1); + expect(player.getState()).toBe("idle"); + player.stop(); + } finally { + vi.useRealTimers(); + } + }); + + it("does not spuriously end after a brief pause+resume on a healthy stream", () => { + vi.useFakeTimers(FAKE_TIMER_OPTS); + try { + const player = new AudioPlayer(silentLogger); + const p = player as unknown as { + ffmpeg: unknown; + currentSongDuration: number; + pcmBuffer: Buffer; + emptyFrameAttempts: number; + framesPlayed: number; + state: string; + startFrameLoop(): void; + }; + // Healthy: a live ffmpeg with a large buffered runway that never drains + // empty across the ticks below, so no underrun is ever seen. + p.ffmpeg = { pid: undefined }; + p.currentSongDuration = 0; + p.pcmBuffer = Buffer.alloc(FRAME_BYTES * 400); + p.emptyFrameAttempts = 0; + p.framesPlayed = 0; + p.state = "playing"; + p.startFrameLoop(); + + let ended = 0; + player.on("trackEnd", () => ended++); + + vi.advanceTimersByTime(20 * 5); // play a few frames + player.pause(); + vi.advanceTimersByTime(20 * 100); // brief pause (buffer NOT drained while paused) + player.resume(); + expect(player.getState()).toBe("playing"); + vi.advanceTimersByTime(20 * 100); // resume; still plenty of runway + + expect(ended).toBe(0); + expect(player.getState()).toBe("playing"); + player.stop(); + } finally { + vi.useRealTimers(); + } + }); +}); diff --git a/src/audio/player.ts b/src/audio/player.ts index b2e713d..57d780f 100644 --- a/src/audio/player.ts +++ b/src/audio/player.ts @@ -616,7 +616,14 @@ export class AudioPlayer extends EventEmitter { // External mode: the sidecar PCM stream is continuous and never EOFs per // song; a transient underrun must NOT end the track (advance is driven by // the controller). Skip BOTH drain/stall branches while externalMode. - if (!this.externalMode && this.ffmpeg !== null && this.pcmBuffer.length < PCM_FRAME_BYTES) { + // + // R3-4: gate BOTH end-detection branches on state==="playing". While paused + // the loop still ticks (so a resumed stream can refill), but it must NOT + // accumulate stall attempts or emit trackEnd — otherwise pausing a stalled/ + // unknown-duration stream would auto-advance ~5s later. Because the if is + // now false while paused, the else resets emptyFrameAttempts to 0, so a + // resumed healthy stream starts fresh and never ends instantly. + if (this.state === "playing" && !this.externalMode && this.ffmpeg !== null && this.pcmBuffer.length < PCM_FRAME_BYTES) { this.emptyFrameAttempts++; // End the track when FFmpeg has gone silent: quickly if we're near the @@ -641,20 +648,20 @@ export class AudioPlayer extends EventEmitter { nearEnd: isNearEnd, }, "FFmpeg stopped outputting data, ending track"); this.frameLoopRunning = false; - if (this.state !== "idle") { - this.state = "idle"; - // 清理FFmpeg进程 - if (this.ffmpeg) { - const procToKill = this.ffmpeg; - const pidToKill = procToKill.pid; - this.ffmpeg = null; - if (pidToKill) { - this.forceCleanup(procToKill, pidToKill); - } + // The outer gate guarantees state==="playing" here, so no !=="idle" + // guard is needed: end the track directly. + this.state = "idle"; + // 清理FFmpeg进程 + if (this.ffmpeg) { + const procToKill = this.ffmpeg; + const pidToKill = procToKill.pid; + this.ffmpeg = null; + if (pidToKill) { + this.forceCleanup(procToKill, pidToKill); } - this.consecutiveFailures = 0; - this.emit("trackEnd"); } + this.consecutiveFailures = 0; + this.emit("trackEnd"); return; } } else { @@ -662,14 +669,15 @@ export class AudioPlayer extends EventEmitter { this.emptyFrameAttempts = 0; } - if (!this.externalMode && !this.ffmpeg && this.pcmBuffer.length < PCM_FRAME_BYTES) { + // R3-4: likewise gated on state==="playing" — a drained/EOF'd stream must + // not emit trackEnd while paused; end-detection resumes on resume(). + if (this.state === "playing" && !this.externalMode && !this.ffmpeg && this.pcmBuffer.length < PCM_FRAME_BYTES) { this.frameLoopRunning = false; - if (this.state !== "idle") { - this.state = "idle"; - if (!this.spawnFailed) { - this.consecutiveFailures = 0; - this.emit("trackEnd"); - } + // Outer gate guarantees state==="playing"; end directly (no !=="idle" guard). + this.state = "idle"; + if (!this.spawnFailed) { + this.consecutiveFailures = 0; + this.emit("trackEnd"); } return; }