fix: skip aniskip for url playback

2026-03-20 03:16:46 -07:00 · 2026-03-08 16:01:19 -07:00
parent 38034db1e4
commit 8e319a417d
6 changed files with 286 additions and 15 deletions
--- a/Skip-AniSkip-lookup-for-YouTube-and-URL-playback-targets.md
+++ b/Skip-AniSkip-lookup-for-YouTube-and-URL-playback-targets.md
@@ -0,0 +1,83 @@
+---
+id: TASK-127
+title: Skip AniSkip lookup for YouTube and URL playback targets
+status: Done
+assignee:
+  - '@codex'
+created_date: '2026-03-08 08:24'
+updated_date: '2026-03-08 10:12'
+labels:
+  - bug
+  - launcher
+  - youtube
+dependencies: []
+references:
+  - /Users/sudacode/projects/japanese/SubMiner/launcher/mpv.ts
+  - >-
+    /Users/sudacode/projects/japanese/SubMiner/launcher/commands/playback-command.ts
+  - /Users/sudacode/projects/japanese/SubMiner/launcher/mpv.test.ts
+---
+
+## Description
+
+<!-- SECTION:DESCRIPTION:BEGIN -->
+Prevent launcher playback from attempting AniSkip metadata resolution when the user is playing a YouTube target or any URL target. AniSkip only works for local anime files, so URL-driven playback and YouTube subtitle-generation flows should bypass it entirely.
+<!-- SECTION:DESCRIPTION:END -->
+
+## Acceptance Criteria
+<!-- AC:BEGIN -->
+- [x] #1 Launcher playback skips AniSkip metadata resolution for explicit URL targets, including YouTube URLs.
+- [x] #2 YouTube subtitle-generation playback does not invoke AniSkip lookup before mpv launch.
+- [x] #3 Automated launcher tests cover the URL/YouTube skip behavior.
+<!-- AC:END -->
+
+## Implementation Plan
+
+<!-- SECTION:PLAN:BEGIN -->
+1. Add a launcher mpv unit test that intercepts AniSkip resolution and proves URL/YouTube playback does not call it before spawning mpv.
+2. Run the focused launcher mpv test to confirm the new case fails or exposes the current gap.
+3. Patch launcher playback/AniSkip gating so URL and YouTube subtitle-generation paths always bypass AniSkip lookup.
+4. Re-run focused launcher tests and record the verification results in task notes.
+
+5. Add a Lua plugin regression test covering overlay-start on URL playback so AniSkip never runs after auto-start.
+
+6. Patch plugin/subminer/aniskip.lua to short-circuit all AniSkip lookup triggers for remote URL media paths.
+
+7. Re-run plugin regression plus touched launcher checks and update the task summary with the plugin-side fix.
+<!-- SECTION:PLAN:END -->
+
+## Implementation Notes
+
+<!-- SECTION:NOTES:BEGIN -->
+Added explicit AniSkip gating in launcher/mpv.ts via shouldResolveAniSkipMetadata(target, targetKind, preloadedSubtitles).
+
+URL targets now always bypass AniSkip. File targets with preloaded subtitles also bypass AniSkip, covering YouTube subtitle-preload playback.
+
+Added launcher/mpv.test.ts coverage for local-file vs URL vs preloaded-subtitle AniSkip gating.
+
+Verification: bun test launcher/mpv.test.ts passed.
+
+Verification: bun run typecheck passed.
+
+Verification: bunx prettier --check launcher/mpv.ts launcher/mpv.test.ts passed.
+
+Verification: bun run changelog:lint passed.
+
+Verification: bun run test:launcher:unit:src remains blocked by unrelated existing failure in launcher/aniskip-metadata.test.ts (`resolveAniSkipMetadataForFile resolves MAL id and intro payload`: expected malId 1234, got null).
+
+Added plugin regression in scripts/test-plugin-start-gate.lua for URL playback with auto-start/overlay-start; it now asserts no MAL or AniSkip curl requests occur.
+
+Patched plugin/subminer/aniskip.lua to short-circuit AniSkip lookup for remote media paths (`scheme://...`), which covers YouTube URL playback inside the mpv plugin lifecycle.
+
+Verification: lua scripts/test-plugin-start-gate.lua passed.
+
+Verification: bun run test:plugin:src passed.
+
+Verification: bun test launcher/mpv.test.ts passed after plugin-side fix.
+<!-- SECTION:NOTES:END -->
+
+## Final Summary
+
+<!-- SECTION:FINAL_SUMMARY:BEGIN -->
+Fixed AniSkip suppression end-to-end for URL playback. The launcher now skips AniSkip before mpv launch, and the mpv plugin now also refuses AniSkip lookups for remote URL media during file-loaded, overlay-start, or later refresh triggers. Added regression coverage in both launcher/mpv.test.ts and scripts/test-plugin-start-gate.lua, plus a changelog fragment. Wider `bun run test:launcher:unit:src` is still blocked by the unrelated existing launcher/aniskip-metadata.test.ts MAL-id failure.
+<!-- SECTION:FINAL_SUMMARY:END -->
--- a/changes/task-127.md
+++ b/changes/task-127.md
@@ -0,0 +1,4 @@
+type: fixed
+area: launcher
+
+- Skipped AniSkip lookup during URL playback and YouTube subtitle-preload playback, limiting AniSkip to local file targets where it can actually resolve anime metadata.
--- a/launcher/mpv.test.ts
+++ b/launcher/mpv.test.ts
@@ -5,7 +5,14 @@ import path from 'node:path';
 import net from 'node:net';
 import { EventEmitter } from 'node:events';
 import type { Args } from './types';
-import { runAppCommandCaptureOutput, startOverlay, state, waitForUnixSocketReady } from './mpv';
+import {
+  cleanupPlaybackSession,
+  runAppCommandCaptureOutput,
+  shouldResolveAniSkipMetadata,
+  startOverlay,
+  state,
+  waitForUnixSocketReady,
+} from './mpv';
 import * as mpvModule from './mpv';

 function createTempSocketPath(): { dir: string; socketPath: string } {
@@ -73,6 +80,20 @@ test('waitForUnixSocketReady returns true when socket becomes connectable before
  }
 });

+test('shouldResolveAniSkipMetadata skips URL and YouTube-preloaded playback', () => {
+  assert.equal(shouldResolveAniSkipMetadata('/media/show.mkv', 'file'), true);
+  assert.equal(
+    shouldResolveAniSkipMetadata('https://www.youtube.com/watch?v=test123', 'url'),
+    false,
+  );
+  assert.equal(
+    shouldResolveAniSkipMetadata('/tmp/video123.webm', 'file', {
+      primaryPath: '/tmp/video123.ja.srt',
+    }),
+    false,
+  );
+});
+
 function makeArgs(overrides: Partial<Args> = {}): Args {
  return {
    backend: 'x11',
@@ -80,16 +101,19 @@ function makeArgs(overrides: Partial<Args> = {}): Args {
    recursive: false,
    profile: '',
    startOverlay: false,
-    youtubeSubgenMode: 'off',
    whisperBin: '',
    whisperModel: '',
+    whisperVadModel: '',
+    whisperThreads: 4,
    youtubeSubgenOutDir: '',
    youtubeSubgenAudioFormat: 'wav',
    youtubeSubgenKeepTemp: false,
+    youtubeFixWithAi: false,
    youtubePrimarySubLangs: [],
    youtubeSecondarySubLangs: [],
    youtubeAudioLangs: [],
    youtubeWhisperSourceLanguage: 'ja',
+    aiConfig: {},
    useTexthooker: false,
    autoStartOverlay: false,
    texthookerOnly: false,
@@ -152,3 +176,59 @@ test('startOverlay resolves without fixed 2s sleep when readiness signals arrive
    fs.rmSync(dir, { recursive: true, force: true });
  }
 });
+
+test('cleanupPlaybackSession preserves background app while stopping mpv-owned children', async () => {
+  const { dir } = createTempSocketPath();
+  const appPath = path.join(dir, 'fake-subminer.sh');
+  const appInvocationsPath = path.join(dir, 'app-invocations.log');
+  fs.writeFileSync(
+    appPath,
+    `#!/bin/sh\necho \"$@\" >> ${JSON.stringify(appInvocationsPath)}\nexit 0\n`,
+  );
+  fs.chmodSync(appPath, 0o755);
+
+  const calls: string[] = [];
+  const overlayProc = {
+    killed: false,
+    kill: () => {
+      calls.push('overlay-kill');
+      return true;
+    },
+  } as unknown as NonNullable<typeof state.overlayProc>;
+  const mpvProc = {
+    killed: false,
+    kill: () => {
+      calls.push('mpv-kill');
+      return true;
+    },
+  } as unknown as NonNullable<typeof state.mpvProc>;
+  const helperProc = {
+    killed: false,
+    kill: () => {
+      calls.push('helper-kill');
+      return true;
+    },
+  } as unknown as NonNullable<typeof state.overlayProc>;
+
+  state.stopRequested = false;
+  state.appPath = appPath;
+  state.overlayManagedByLauncher = true;
+  state.overlayProc = overlayProc;
+  state.mpvProc = mpvProc;
+  state.youtubeSubgenChildren.add(helperProc);
+
+  try {
+    await cleanupPlaybackSession(makeArgs());
+
+    assert.deepEqual(calls, ['mpv-kill', 'helper-kill']);
+    assert.equal(fs.existsSync(appInvocationsPath), false);
+  } finally {
+    state.overlayProc = null;
+    state.mpvProc = null;
+    state.youtubeSubgenChildren.clear();
+    state.overlayManagedByLauncher = false;
+    state.appPath = '';
+    state.stopRequested = false;
+    fs.rmSync(dir, { recursive: true, force: true });
+  }
+});
--- a/launcher/mpv.ts
+++ b/launcher/mpv.ts
@@ -419,6 +419,20 @@ export async function loadSubtitleIntoMpv(
  }
 }

+export function shouldResolveAniSkipMetadata(
+  target: string,
+  targetKind: 'file' | 'url',
+  preloadedSubtitles?: { primaryPath?: string; secondaryPath?: string },
+): boolean {
+  if (targetKind !== 'file') {
+    return false;
+  }
+  if (preloadedSubtitles?.primaryPath || preloadedSubtitles?.secondaryPath) {
+    return false;
+  }
+  return !isYoutubeTarget(target);
+}
+
 export async function startMpv(
  target: string,
  targetKind: 'file' | 'url',
@@ -456,17 +470,13 @@ export async function startMpv(
      log('debug', args.logLevel, `YouTube subtitle langs: ${subtitleLangs}`);
      log('debug', args.logLevel, `YouTube audio langs: ${audioLangs}`);
      mpvArgs.push(`--ytdl-format=${DEFAULT_YOUTUBE_YTDL_FORMAT}`, `--alang=${audioLangs}`);
-
-      if (args.youtubeSubgenMode === 'off') {
-        mpvArgs.push(
-          '--sub-auto=fuzzy',
-          `--slang=${subtitleLangs}`,
-          '--ytdl-raw-options-append=write-auto-subs=',
-          '--ytdl-raw-options-append=write-subs=',
-          '--ytdl-raw-options-append=sub-format=vtt/best',
-          `--ytdl-raw-options-append=sub-langs=${subtitleLangs}`,
-        );
-      }
+      mpvArgs.push(
+        '--sub-auto=fuzzy',
+        `--slang=${subtitleLangs}`,
+        '--ytdl-raw-options-append=write-subs=',
+        '--ytdl-raw-options-append=sub-format=vtt/best',
+        `--ytdl-raw-options-append=sub-langs=${subtitleLangs}`,
+      );
    }
  }

@@ -479,8 +489,9 @@ export async function startMpv(
  if (options?.startPaused) {
    mpvArgs.push('--pause=yes');
  }
-  const aniSkipMetadata =
-    targetKind === 'file' ? await resolveAniSkipMetadataForFile(target) : null;
+  const aniSkipMetadata = shouldResolveAniSkipMetadata(target, targetKind, preloadedSubtitles)
+    ? await resolveAniSkipMetadataForFile(target)
+    : null;
  const scriptOpts = buildSubminerScriptOpts(appPath, socketPath, aniSkipMetadata);
  if (aniSkipMetadata) {
    log(
@@ -628,6 +639,29 @@ export function stopOverlay(args: Args): void {
  void terminateTrackedDetachedMpv(args.logLevel);
 }

+export async function cleanupPlaybackSession(args: Args): Promise<void> {
+  if (state.mpvProc && !state.mpvProc.killed) {
+    try {
+      state.mpvProc.kill('SIGTERM');
+    } catch {
+      // ignore
+    }
+  }
+
+  for (const child of state.youtubeSubgenChildren) {
+    if (!child.killed) {
+      try {
+        child.kill('SIGTERM');
+      } catch {
+        // ignore
+      }
+    }
+  }
+  state.youtubeSubgenChildren.clear();
+
+  await terminateTrackedDetachedMpv(args.logLevel);
+}
+
 function buildAppEnv(): NodeJS.ProcessEnv {
  const env: Record<string, string | undefined> = {
    ...process.env,
--- a/plugin/subminer/aniskip.lua
+++ b/plugin/subminer/aniskip.lua
@@ -31,6 +31,18 @@ function M.create(ctx)
 		return encoded:gsub(" ", "%%20")
 	end

+	local function is_remote_media_path()
+		local media_path = mp.get_property("path")
+		if type(media_path) ~= "string" then
+			return false
+		end
+		local trimmed = media_path:match("^%s*(.-)%s*$") or ""
+		if trimmed == "" then
+			return false
+		end
+		return trimmed:match("^%a[%w+.-]*://") ~= nil
+	end
+
 	local function parse_json_payload(text)
 		if type(text) ~= "string" then
 			return nil
@@ -523,6 +535,10 @@ function M.create(ctx)
 	end

 	local function should_fetch_aniskip_async(trigger_source, callback)
+		if is_remote_media_path() then
+			callback(false, "remote-url")
+			return
+		end
 		if trigger_source == "script-message" or trigger_source == "overlay-start" then
 			callback(true, trigger_source)
 			return
--- a/scripts/test-plugin-start-gate.lua
+++ b/scripts/test-plugin-start-gate.lua
@@ -461,6 +461,36 @@ do
 	)
 end

+do
+	local recorded, err = run_plugin_scenario({
+		process_list = "",
+		option_overrides = {
+			binary_path = binary_path,
+			auto_start = "yes",
+			auto_start_visible_overlay = "yes",
+			auto_start_pause_until_ready = "no",
+			socket_path = "/tmp/subminer-socket",
+		},
+		input_ipc_server = "/tmp/subminer-socket",
+		path = "https://www.youtube.com/watch?v=lJI7uL4JDkE",
+		media_title = "【文字起こし】マジで役立つ！！恋愛術！【告radio】",
+		files = {
+			[binary_path] = true,
+		},
+	})
+	assert_true(recorded ~= nil, "plugin failed to load for URL overlay-start AniSkip scenario: " .. tostring(err))
+	fire_event(recorded, "file-loaded")
+	assert_true(find_start_call(recorded.async_calls) ~= nil, "URL auto-start should still invoke --start command")
+	assert_true(
+		not has_async_curl_for(recorded.async_calls, "myanimelist.net/search/prefix.json"),
+		"URL playback should skip AniSkip MAL lookup even after overlay-start"
+	)
+	assert_true(
+		not has_async_curl_for(recorded.async_calls, "api.aniskip.com"),
+		"URL playback should skip AniSkip API lookup even after overlay-start"
+	)
+end
+
 do
 	local recorded, err = run_plugin_scenario({
 		process_list = "",
@@ -687,6 +717,30 @@ do
 	)
 end

+do
+	local recorded, err = run_plugin_scenario({
+		process_list = "",
+		option_overrides = {
+			binary_path = binary_path,
+			auto_start = "yes",
+			auto_start_visible_overlay = "yes",
+			socket_path = "/tmp/subminer-socket",
+		},
+		input_ipc_server = "/tmp/subminer-socket",
+		media_title = "Random Movie",
+		files = {
+			[binary_path] = true,
+		},
+	})
+	assert_true(recorded ~= nil, "plugin failed to load for shutdown-preserve-background scenario: " .. tostring(err))
+	fire_event(recorded, "file-loaded")
+	fire_event(recorded, "shutdown")
+	assert_true(
+		find_control_call(recorded.async_calls, "--stop") == nil,
+		"mpv shutdown should not stop the background SubMiner process"
+	)
+end
+
 do
 	local recorded, err = run_plugin_scenario({
 		process_list = "",