Fix YouTube playback PR review issues

This commit is contained in:
2026-03-25 16:25:57 -07:00
committed by sudacode
parent c95518e94a
commit 58304757aa
7 changed files with 158 additions and 23 deletions

View File

@@ -7,6 +7,11 @@ test('normalizeOverlayWindowBoundsForPlatform returns original geometry outside
assert.deepEqual(normalizeOverlayWindowBoundsForPlatform(geometry, 'linux', null), geometry);
});
test('normalizeOverlayWindowBoundsForPlatform returns original geometry on Windows when screen is unavailable', () => {
const geometry = { x: 150, y: 90, width: 1200, height: 675 };
assert.deepEqual(normalizeOverlayWindowBoundsForPlatform(geometry, 'win32', null), geometry);
});
test('normalizeOverlayWindowBoundsForPlatform converts Windows physical pixels to DIP', () => {
assert.deepEqual(
normalizeOverlayWindowBoundsForPlatform(

View File

@@ -65,3 +65,12 @@ test('resolveYoutubePlaybackUrl rejects when yt-dlp returns no URL', async () =>
);
});
});
test('resolveYoutubePlaybackUrl rejects when yt-dlp output exceeds capture limit', async () => {
await withFakeYtDlp(`${'x'.repeat(1024 * 1024 + 1)}\n`, async () => {
await assert.rejects(
resolveYoutubePlaybackUrl('https://www.youtube.com/watch?v=abc123'),
/exceeded 1048576 bytes/,
);
});
});

View File

@@ -2,6 +2,18 @@ import { spawn } from 'node:child_process';
const YOUTUBE_PLAYBACK_RESOLVE_TIMEOUT_MS = 15_000;
const DEFAULT_PLAYBACK_FORMAT = 'b';
const MAX_CAPTURE_BYTES = 1024 * 1024;
function terminateCaptureProcess(proc: ReturnType<typeof spawn>): void {
if (proc.killed) {
return;
}
try {
proc.kill('SIGKILL');
} catch {
proc.kill();
}
}
function runCapture(
command: string,
@@ -12,29 +24,62 @@ function runCapture(
const proc = spawn(command, args, { stdio: ['ignore', 'pipe', 'pipe'] });
let stdout = '';
let stderr = '';
let settled = false;
const cleanup = (): void => {
clearTimeout(timer);
proc.stdout.removeAllListeners('data');
proc.stderr.removeAllListeners('data');
proc.removeAllListeners('error');
proc.removeAllListeners('close');
};
const rejectOnce = (error: Error): void => {
if (settled) return;
settled = true;
cleanup();
reject(error);
};
const resolveOnce = (result: { stdout: string; stderr: string }): void => {
if (settled) return;
settled = true;
cleanup();
resolve(result);
};
const appendChunk = (
current: string,
chunk: unknown,
streamName: 'stdout' | 'stderr',
): string => {
const next = current + String(chunk);
if (Buffer.byteLength(next, 'utf8') > MAX_CAPTURE_BYTES) {
terminateCaptureProcess(proc);
rejectOnce(new Error(`yt-dlp ${streamName} exceeded ${MAX_CAPTURE_BYTES} bytes`));
}
return next;
};
const timer = setTimeout(() => {
proc.kill();
reject(new Error(`yt-dlp timed out after ${timeoutMs}ms`));
terminateCaptureProcess(proc);
rejectOnce(new Error(`yt-dlp timed out after ${timeoutMs}ms`));
}, timeoutMs);
proc.stdout.setEncoding('utf8');
proc.stderr.setEncoding('utf8');
proc.stdout.on('data', (chunk) => {
stdout += String(chunk);
stdout = appendChunk(stdout, chunk, 'stdout');
});
proc.stderr.on('data', (chunk) => {
stderr += String(chunk);
stderr = appendChunk(stderr, chunk, 'stderr');
});
proc.once('error', (error) => {
clearTimeout(timer);
reject(error);
rejectOnce(error);
});
proc.once('close', (code) => {
clearTimeout(timer);
if (code === 0) {
resolve({ stdout, stderr });
if (settled) {
return;
}
reject(new Error(stderr.trim() || `yt-dlp exited with status ${code ?? 'unknown'}`));
if (code === 0) {
resolveOnce({ stdout, stderr });
return;
}
rejectOnce(new Error(stderr.trim() || `yt-dlp exited with status ${code ?? 'unknown'}`));
});
});
}

View File

@@ -501,6 +501,7 @@ const anilistAttemptedUpdateKeys = new Set<string>();
let anilistCachedAccessToken: string | null = null;
let jellyfinPlayQuitOnDisconnectArmed = false;
let youtubePlayQuitOnDisconnectArmed = false;
let youtubePlayQuitOnDisconnectArmTimer: ReturnType<typeof setTimeout> | null = null;
const JELLYFIN_LANG_PREF = 'ja,jp,jpn,japanese,en,eng,english,enUS,en-US';
const JELLYFIN_TICKS_PER_SECOND = 10_000_000;
const JELLYFIN_REMOTE_PROGRESS_INTERVAL_MS = 3000;
@@ -972,13 +973,23 @@ const waitForYoutubeMpvConnected = createWaitForMpvConnectedHandler({
sleep: (delayMs) => new Promise((resolve) => setTimeout(resolve, delayMs)),
});
function clearYoutubePlayQuitOnDisconnectArmTimer(): void {
if (youtubePlayQuitOnDisconnectArmTimer) {
clearTimeout(youtubePlayQuitOnDisconnectArmTimer);
youtubePlayQuitOnDisconnectArmTimer = null;
}
}
async function runYoutubePlaybackFlowMain(request: {
url: string;
mode: NonNullable<CliArgs['youtubeMode']>;
source: CliCommandSource;
}): Promise<void> {
youtubePrimarySubtitleNotificationRuntime.setAppOwnedFlowInFlight(true);
let flowCompleted = false;
try {
clearYoutubePlayQuitOnDisconnectArmTimer();
youtubePlayQuitOnDisconnectArmed = false;
let playbackUrl = request.url;
let launchedWindowsMpv = false;
if (process.platform === 'win32') {
@@ -1032,20 +1043,27 @@ async function runYoutubePlaybackFlowMain(request: {
: 'MPV not connected. Start mpv with the SubMiner profile or retry after mpv finishes starting.',
);
}
youtubePlayQuitOnDisconnectArmed = false;
setTimeout(() => {
youtubePlayQuitOnDisconnectArmed = true;
}, 3000);
if (request.source === 'initial') {
youtubePlayQuitOnDisconnectArmTimer = setTimeout(() => {
youtubePlayQuitOnDisconnectArmed = true;
youtubePlayQuitOnDisconnectArmTimer = null;
}, 3000);
}
const mediaReady = await prepareYoutubePlaybackInMpv({ url: playbackUrl });
if (!mediaReady) {
logger.warn('Timed out waiting for mpv to load requested YouTube URL; continuing anyway.');
throw new Error('Timed out waiting for mpv to load the requested YouTube URL.');
}
await youtubeFlowRuntime.runYoutubePlaybackFlow({
url: request.url,
mode: request.mode,
});
flowCompleted = true;
logger.info(`YouTube playback flow completed from ${request.source}.`);
} finally {
if (!flowCompleted) {
clearYoutubePlayQuitOnDisconnectArmTimer();
youtubePlayQuitOnDisconnectArmed = false;
}
youtubePrimarySubtitleNotificationRuntime.setAppOwnedFlowInFlight(false);
}
}

View File

@@ -76,6 +76,27 @@ test('mpv connection handler quits standalone youtube playback even after overla
assert.deepEqual(calls, ['presence-refresh', 'report-stop', 'schedule', 'quit']);
});
test('mpv connection handler keeps overlay-initialized non-youtube sessions alive', () => {
const calls: string[] = [];
const handler = createHandleMpvConnectionChangeHandler({
reportJellyfinRemoteStopped: () => calls.push('report-stop'),
refreshDiscordPresence: () => calls.push('presence-refresh'),
syncOverlayMpvSubtitleSuppression: () => calls.push('sync-overlay-mpv-sub'),
hasInitialPlaybackQuitOnDisconnectArg: () => true,
isOverlayRuntimeInitialized: () => true,
shouldQuitOnDisconnectWhenOverlayRuntimeInitialized: () => false,
isQuitOnDisconnectArmed: () => true,
scheduleQuitCheck: () => {
calls.push('schedule');
},
isMpvConnected: () => false,
quitApp: () => calls.push('quit'),
});
handler({ connected: false });
assert.deepEqual(calls, ['presence-refresh', 'report-stop']);
});
test('mpv subtitle timing handler ignores blank subtitle lines', () => {
const calls: string[] = [];
const handler = createHandleMpvSubtitleTimingHandler({

View File

@@ -36,6 +36,32 @@ test('prepare youtube playback treats matching video IDs as already loaded', asy
assert.deepEqual(commands, []);
});
test('prepare youtube playback does not mark matching target ready until tracks exist', async () => {
const commands: Array<Array<string>> = [];
let requestCount = 0;
const prepare = createPrepareYoutubePlaybackInMpvHandler({
requestPath: async () => {
requestCount += 1;
return 'https://www.youtube.com/watch?v=abc123';
},
requestProperty: async (name) => {
if (name !== 'track-list') return null;
return requestCount >= 3 ? [{ type: 'video', id: 1 }] : [];
},
sendMpvCommand: (command) => commands.push(command),
wait: createWaitStub(),
});
const ok = await prepare({
url: 'https://www.youtube.com/watch?v=abc123',
timeoutMs: 1500,
pollIntervalMs: 1,
});
assert.equal(ok, true);
assert.deepEqual(commands, []);
});
test('prepare youtube playback replaces media and waits for path switch', async () => {
const commands: Array<Array<string>> = [];
const observedPaths = [

View File

@@ -95,16 +95,27 @@ export function createPrepareYoutubePlaybackInMpvHandler(deps: YoutubePlaybackLa
// Ignore transient path request failures and continue with bootstrap commands.
}
if (pathMatchesYoutubeTarget(previousPath, targetUrl)) {
return true;
const alreadyTarget = pathMatchesYoutubeTarget(previousPath, targetUrl);
if (alreadyTarget) {
if (!deps.requestProperty) {
return true;
}
try {
const trackList = await deps.requestProperty('track-list');
if (hasPlayableMediaTracks(trackList)) {
return true;
}
} catch {
// Keep polling; mpv can report the target path before tracks are ready.
}
} else {
deps.sendMpvCommand(['set_property', 'pause', 'yes']);
deps.sendMpvCommand(['set_property', 'sub-auto', 'no']);
deps.sendMpvCommand(['set_property', 'sid', 'no']);
deps.sendMpvCommand(['set_property', 'secondary-sid', 'no']);
deps.sendMpvCommand(['loadfile', targetUrl, 'replace']);
}
deps.sendMpvCommand(['set_property', 'pause', 'yes']);
deps.sendMpvCommand(['set_property', 'sub-auto', 'no']);
deps.sendMpvCommand(['set_property', 'sid', 'no']);
deps.sendMpvCommand(['set_property', 'secondary-sid', 'no']);
deps.sendMpvCommand(['loadfile', targetUrl, 'replace']);
const deadline = now() + timeoutMs;
while (now() < deadline) {
await deps.wait(pollIntervalMs);