Fix secondary subtitle style parity and MPV visibility restore lifecycle

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
+
+<!-- SECTION:DESCRIPTION:BEGIN -->
+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.
+<!-- SECTION:DESCRIPTION:END -->
+
+## Acceptance Criteria
+<!-- AC:BEGIN -->
+- [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.
+<!-- AC:END -->
+
+## Final Summary
+
+<!-- SECTION:FINAL_SUMMARY:BEGIN -->
+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`.
+<!-- SECTION:FINAL_SUMMARY:END -->

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<MpvIpcClientDeps> = {},
@@ -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);
+});

src/core/services/mpv-service.ts

@@ -65,6 +65,7 @@ export interface MpvIpcClientDeps {
     patch: Partial<MpvSubtitleRenderMetrics>,
   ) => 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);
+  }
 }

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<string | number>;
-  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();

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(),

src/renderer/renderer.ts

@@ -208,6 +208,8 @@ async function init(): Promise<void> {
 
   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<void> {
       await window.electronAPI.getSubtitlePosition(),
       "startup",
     );
-    subtitleRenderer.applySubtitleStyle(await window.electronAPI.getSubtitleStyle());
     measurementReporter.schedule();
   }