diff --git a/backlog/tasks/task-21 - Persist-and-restore-MPV-secondary-subtitle-visibility-across-app-lifecycle.md b/backlog/tasks/task-21 - Persist-and-restore-MPV-secondary-subtitle-visibility-across-app-lifecycle.md new file mode 100644 index 0000000..819816a --- /dev/null +++ b/backlog/tasks/task-21 - Persist-and-restore-MPV-secondary-subtitle-visibility-across-app-lifecycle.md @@ -0,0 +1,38 @@ +--- +id: TASK-21 +title: Persist and restore MPV secondary subtitle visibility across app lifecycle +status: Done +assignee: [] +created_date: '2026-02-13 07:59' +updated_date: '2026-02-13 08:01' +labels: [] +dependencies: [] +priority: high +--- + +## Description + + +When SubMiner connects to MPV, capture the current MPV `secondary-sub-visibility` value and force it off. Keep it off during SubMiner runtime regardless of overlay visibility toggles. On app shutdown (and MPV shutdown event when possible), restore MPV `secondary-sub-visibility` to the captured pre-SubMiner value. + + +## Acceptance Criteria + +- [x] #1 Capture MPV `secondary-sub-visibility` once per MPV connection before overriding it. +- [x] #2 Set MPV `secondary-sub-visibility` to `no` after capture regardless of `bind_visible_overlay_to_mpv_sub_visibility`. +- [x] #3 Do not mutate/restore secondary MPV visibility as a side effect of visible overlay toggles. +- [x] #4 Restore captured secondary MPV visibility on app shutdown while MPV is connected. +- [x] #5 Attempt restore on MPV shutdown event before disconnect and clear captured state afterward. + + +## Final Summary + + +Implemented MPV secondary subtitle visibility lifecycle management: +- Moved secondary-sub visibility capture/disable to MPV connection initialization (`getInitialState` requests `secondary-sub-visibility`, then request handler stores prior value and forces `secondary-sub-visibility=no`). +- Removed secondary-sub visibility side effects from visible overlay visibility service so overlay toggles no longer capture/restore secondary MPV state. +- Added `restorePreviousSecondarySubVisibility()` to `MpvIpcClient`, invoked on MPV `shutdown` event and from app `onWillQuitCleanup` (best effort while connected). +- Wired new dependency getter/setter in main runtime bootstrap for tracked previous secondary visibility state. +- Added unit coverage in `mpv-service.test.ts` for capture/disable and restore/clear behavior. +- Verified with `pnpm run build` and `node --test dist/core/services/mpv-service.test.js`. + diff --git a/src/core/services/mpv-service.test.ts b/src/core/services/mpv-service.test.ts index 342719a..a9685cc 100644 --- a/src/core/services/mpv-service.test.ts +++ b/src/core/services/mpv-service.test.ts @@ -1,6 +1,10 @@ import test from "node:test"; import assert from "node:assert/strict"; -import { MpvIpcClient, MpvIpcClientDeps } from "./mpv-service"; +import { + MpvIpcClient, + MpvIpcClientDeps, + MPV_REQUEST_ID_SECONDARY_SUB_VISIBILITY, +} from "./mpv-service"; function makeDeps( overrides: Partial = {}, @@ -41,6 +45,7 @@ function makeDeps( osdHeight: 720, osdDimensions: null, }), + getPreviousSecondarySubVisibility: () => null, setPreviousSecondarySubVisibility: () => {}, showMpvOsd: () => {}, ...overrides, @@ -174,3 +179,62 @@ test("MpvIpcClient scheduleReconnect schedules timer and invokes connect", () => assert.equal(timers.length, 1); assert.equal(connectCalled, true); }); + +test("MpvIpcClient captures and disables secondary subtitle visibility on request", async () => { + const commands: unknown[] = []; + let previousSecondarySubVisibility: boolean | null = null; + const client = new MpvIpcClient( + "/tmp/mpv.sock", + makeDeps({ + getPreviousSecondarySubVisibility: () => previousSecondarySubVisibility, + setPreviousSecondarySubVisibility: (value) => { + previousSecondarySubVisibility = value; + }, + }), + ); + + (client as any).send = (payload: unknown) => { + commands.push(payload); + return true; + }; + + await (client as any).handleMessage({ + request_id: MPV_REQUEST_ID_SECONDARY_SUB_VISIBILITY, + data: "yes", + }); + + assert.equal(previousSecondarySubVisibility, true); + assert.deepEqual(commands, [ + { + command: ["set_property", "secondary-sub-visibility", "no"], + }, + ]); +}); + +test("MpvIpcClient restorePreviousSecondarySubVisibility restores and clears tracked value", () => { + const commands: unknown[] = []; + let previousSecondarySubVisibility: boolean | null = false; + const client = new MpvIpcClient( + "/tmp/mpv.sock", + makeDeps({ + getPreviousSecondarySubVisibility: () => previousSecondarySubVisibility, + setPreviousSecondarySubVisibility: (value) => { + previousSecondarySubVisibility = value; + }, + }), + ); + + (client as any).send = (payload: unknown) => { + commands.push(payload); + return true; + }; + + client.restorePreviousSecondarySubVisibility(); + + assert.deepEqual(commands, [ + { + command: ["set_property", "secondary-sub-visibility", "no"], + }, + ]); + assert.equal(previousSecondarySubVisibility, null); +}); diff --git a/src/core/services/mpv-service.ts b/src/core/services/mpv-service.ts index 5e52520..9daece4 100644 --- a/src/core/services/mpv-service.ts +++ b/src/core/services/mpv-service.ts @@ -65,6 +65,7 @@ export interface MpvIpcClientDeps { patch: Partial, ) => void; getMpvSubtitleRenderMetrics: () => MpvSubtitleRenderMetrics; + getPreviousSecondarySubVisibility: () => boolean | null; setPreviousSecondarySubVisibility: (value: boolean | null) => void; showMpvOsd: (text: string) => void; } @@ -369,6 +370,8 @@ export class MpvIpcClient implements MpvClient { }); } } + } else if (msg.event === "shutdown") { + this.restorePreviousSecondarySubVisibility(); } else if (msg.request_id) { const pending = this.pendingRequests.get(msg.request_id); if (pending) { @@ -440,10 +443,6 @@ export class MpvIpcClient implements MpvClient { this.currentSecondarySubText, ); } else if (msg.request_id === MPV_REQUEST_ID_SECONDARY_SUB_VISIBILITY) { - if (!this.deps.shouldBindVisibleOverlayToMpvSubVisibility()) { - this.deps.setPreviousSecondarySubVisibility(null); - return; - } this.deps.setPreviousSecondarySubVisibility( msg.data === true || msg.data === "yes", ); @@ -673,6 +672,10 @@ export class MpvIpcClient implements MpvClient { command: ["get_property", "secondary-sub-text"], request_id: MPV_REQUEST_ID_SECONDARY_SUBTEXT, }); + this.send({ + command: ["get_property", "secondary-sub-visibility"], + request_id: MPV_REQUEST_ID_SECONDARY_SUB_VISIBILITY, + }); this.send({ command: ["get_property", "aid"], request_id: MPV_REQUEST_ID_AID, @@ -758,4 +761,13 @@ export class MpvIpcClient implements MpvClient { this.pendingPauseAtSubEnd = true; this.send({ command: ["sub-seek", 1] }); } + + restorePreviousSecondarySubVisibility(): void { + const previous = this.deps.getPreviousSecondarySubVisibility(); + if (previous === null) return; + this.send({ + command: ["set_property", "secondary-sub-visibility", previous ? "yes" : "no"], + }); + this.deps.setPreviousSecondarySubVisibility(null); + } } diff --git a/src/core/services/overlay-visibility-service.ts b/src/core/services/overlay-visibility-service.ts index ac606c5..4074ea5 100644 --- a/src/core/services/overlay-visibility-service.ts +++ b/src/core/services/overlay-visibility-service.ts @@ -2,23 +2,12 @@ import { BrowserWindow, screen } from "electron"; import { BaseWindowTracker } from "../../window-trackers"; import { WindowGeometry } from "../../types"; -interface MpvCommandSender { - command: Array; - request_id?: number; -} - export function updateVisibleOverlayVisibilityService(args: { visibleOverlayVisible: boolean; mainWindow: BrowserWindow | null; windowTracker: BaseWindowTracker | null; trackerNotReadyWarningShown: boolean; setTrackerNotReadyWarningShown: (shown: boolean) => void; - shouldBindVisibleOverlayToMpvSubVisibility: boolean; - previousSecondarySubVisibility: boolean | null; - setPreviousSecondarySubVisibility: (value: boolean | null) => void; - mpvConnected: boolean; - mpvSend: (payload: MpvCommandSender) => void; - secondarySubVisibilityRequestId: number; updateVisibleOverlayBounds: (geometry: WindowGeometry) => void; ensureOverlayWindowLevel: (window: BrowserWindow) => void; enforceOverlayLayerOrder: () => void; @@ -30,34 +19,10 @@ export function updateVisibleOverlayVisibilityService(args: { if (!args.visibleOverlayVisible) { args.mainWindow.hide(); - - if ( - args.shouldBindVisibleOverlayToMpvSubVisibility && - args.previousSecondarySubVisibility !== null && - args.mpvConnected - ) { - args.mpvSend({ - command: [ - "set_property", - "secondary-sub-visibility", - args.previousSecondarySubVisibility ? "yes" : "no", - ], - }); - args.setPreviousSecondarySubVisibility(null); - } else if (!args.shouldBindVisibleOverlayToMpvSubVisibility) { - args.setPreviousSecondarySubVisibility(null); - } args.syncOverlayShortcuts(); return; } - if (args.shouldBindVisibleOverlayToMpvSubVisibility && args.mpvConnected) { - args.mpvSend({ - command: ["get_property", "secondary-sub-visibility"], - request_id: args.secondarySubVisibilityRequestId, - }); - } - if (args.windowTracker && args.windowTracker.isTracking()) { args.setTrackerNotReadyWarningShown(false); const geometry = args.windowTracker.getGeometry(); diff --git a/src/main.ts b/src/main.ts index 5035771..24a45a7 100644 --- a/src/main.ts +++ b/src/main.ts @@ -88,7 +88,6 @@ import { showDesktopNotification, } from "./core/utils"; import { - MPV_REQUEST_ID_SECONDARY_SUB_VISIBILITY, MpvIpcClient, SubtitleWebSocketService, TexthookerService, @@ -342,6 +341,11 @@ function getOverlayWindows(): BrowserWindow[] { return overlayManager.getOverlayWindows(); } +function restorePreviousSecondarySubVisibility(): void { + if (!mpvClient || !mpvClient.connected) return; + mpvClient.restorePreviousSecondarySubVisibility(); +} + function broadcastToOverlayWindows(channel: string, ...args: unknown[]): void { overlayManager.broadcastToOverlayWindows(channel, ...args); } @@ -552,6 +556,8 @@ const startupState = runStartupBootstrapRuntimeService({ updateMpvSubtitleRenderMetrics(patch); }, getMpvSubtitleRenderMetrics: () => mpvSubtitleRenderMetrics, + getPreviousSecondarySubVisibility: () => + previousSecondarySubVisibility, setPreviousSecondarySubVisibility: (value) => { previousSecondarySubVisibility = value; }, @@ -614,6 +620,7 @@ const startupState = runStartupBootstrapRuntimeService({ }); }, onWillQuitCleanup: () => { + restorePreviousSecondarySubVisibility(); globalShortcut.unregisterAll(); subtitleWsService.stop(); texthookerService.stop(); @@ -1182,18 +1189,6 @@ function updateVisibleOverlayVisibility(): void { setTrackerNotReadyWarningShown: (shown) => { trackerNotReadyWarningShown = shown; }, - shouldBindVisibleOverlayToMpvSubVisibility: - shouldBindVisibleOverlayToMpvSubVisibility(), - previousSecondarySubVisibility, - setPreviousSecondarySubVisibility: (value) => { - previousSecondarySubVisibility = value; - }, - mpvConnected: Boolean(mpvClient && mpvClient.connected), - mpvSend: (payload) => { - if (!mpvClient) return; - mpvClient.send(payload); - }, - secondarySubVisibilityRequestId: MPV_REQUEST_ID_SECONDARY_SUB_VISIBILITY, updateVisibleOverlayBounds: (geometry) => updateVisibleOverlayBounds(geometry), ensureOverlayWindowLevel: (window) => ensureOverlayWindowLevel(window), enforceOverlayLayerOrder: () => enforceOverlayLayerOrder(), diff --git a/src/renderer/renderer.ts b/src/renderer/renderer.ts index 20128ec..99f141f 100644 --- a/src/renderer/renderer.ts +++ b/src/renderer/renderer.ts @@ -208,6 +208,8 @@ async function init(): Promise { await keyboardHandlers.setupMpvInputForwarding(); + subtitleRenderer.applySubtitleStyle(await window.electronAPI.getSubtitleStyle()); + if (ctx.platform.isInvisibleLayer) { positioning.applyInvisibleStoredSubtitlePosition( await window.electronAPI.getSubtitlePosition(), @@ -222,7 +224,6 @@ async function init(): Promise { await window.electronAPI.getSubtitlePosition(), "startup", ); - subtitleRenderer.applySubtitleStyle(await window.electronAPI.getSubtitleStyle()); measurementReporter.schedule(); }