fix(subtitles): keep native secondary subtitles hidden (#232)

- Reapply hidden visibility after secondary track changes
- Relinquish suppression only after successful restoration

changes/secondary-subtitle-track-visibility.md

@@ -0,0 +1,4 @@
+type: fixed
+area: overlay
+
+- Native mpv secondary subtitles stay hidden when switching secondary subtitle tracks during playback.

src/core/services/mpv-protocol.test.ts

@@ -83,6 +83,7 @@ function createDeps(overrides: Partial<MpvProtocolHandleMessageDeps> = {}): {
         state.secondarySubText = text;
       },
       resolvePendingRequest: () => false,
+      shouldEnforceSecondarySubVisibilityHidden: () => true,
       setSecondarySubVisibility: () => {},
       syncCurrentAudioStreamIndex: () => {},
       setCurrentAudioTrackId: () => {},
@@ -198,6 +199,21 @@ test('dispatchMpvProtocolMessage rejects decimal subtitle track IDs', async () =
   assert.deepEqual(state.events, [{ sid: null }, { sid: null }, { sid: null }, { sid: null }]);
 });
 
+test('dispatchMpvProtocolMessage hides native secondary subtitles after a track change', async () => {
+  const visibilityChanges: boolean[] = [];
+  const { deps, state } = createDeps({
+    setSecondarySubVisibility: (visible) => visibilityChanges.push(visible),
+  });
+
+  await dispatchMpvProtocolMessage(
+    { event: 'property-change', name: 'secondary-sid', data: '4' },
+    deps,
+  );
+
+  assert.deepEqual(visibilityChanges, [false]);
+  assert.deepEqual(state.events, [{ sid: 4 }]);
+});
+
 test('dispatchMpvProtocolMessage enforces sub-visibility hidden when overlay suppression is enabled', async () => {
   const { deps, state } = createDeps({
     isVisibleOverlayVisible: () => true,
@@ -239,6 +255,24 @@ test('dispatchMpvProtocolMessage skips sub-visibility suppression when overlay i
   assert.equal(state.commands.length, 0);
 });
 
+test('dispatchMpvProtocolMessage corrects native secondary subtitle visibility', async () => {
+  const visibilityChanges: boolean[] = [];
+  const { deps } = createDeps({
+    setSecondarySubVisibility: (visible) => visibilityChanges.push(visible),
+  });
+
+  await dispatchMpvProtocolMessage(
+    { event: 'property-change', name: 'secondary-sub-visibility', data: 'yes' },
+    deps,
+  );
+  await dispatchMpvProtocolMessage(
+    { event: 'property-change', name: 'secondary-sub-visibility', data: 'no' },
+    deps,
+  );
+
+  assert.deepEqual(visibilityChanges, [false]);
+});
+
 test('dispatchMpvProtocolMessage sets secondary subtitle track based on track list response', async () => {
   const { deps, state } = createDeps();
 

src/core/services/mpv-protocol.ts

@@ -72,6 +72,7 @@ export interface MpvProtocolHandleMessageDeps {
   emitSubtitleMetricsChange: (payload: Partial<MpvSubtitleRenderMetrics>) => void;
   setCurrentSecondarySubText: (text: string) => void;
   resolvePendingRequest: (requestId: number, message: MpvMessage) => boolean;
+  shouldEnforceSecondarySubVisibilityHidden: () => boolean;
   setSecondarySubVisibility: (visible: boolean) => void;
   syncCurrentAudioStreamIndex: () => void;
   setCurrentAudioTrackId: (value: number | null) => void;
@@ -285,6 +286,9 @@ export async function dispatchMpvProtocolMessage(
             : null;
       deps.emitSubtitleTrackChange({ sid: sid !== null && Number.isInteger(sid) ? sid : null });
     } else if (msg.name === 'secondary-sid') {
+      if (deps.shouldEnforceSecondarySubVisibilityHidden()) {
+        deps.setSecondarySubVisibility(false);
+      }
       const sid =
         typeof msg.data === 'number'
           ? msg.data
@@ -375,6 +379,11 @@ export async function dispatchMpvProtocolMessage(
       if (deps.isVisibleOverlayVisible() && asBoolean(msg.data, false)) {
         deps.sendCommand({ command: ['set_property', 'sub-visibility', false] });
       }
+    } else if (msg.name === 'secondary-sub-visibility') {
+      const visible = parseVisibilityProperty(msg.data);
+      if (deps.shouldEnforceSecondarySubVisibilityHidden() && visible === true) {
+        deps.setSecondarySubVisibility(false);
+      }
     } else if (msg.name === 'sub-use-margins') {
       deps.emitSubtitleMetricsChange({
         subUseMargins: asBoolean(msg.data, deps.getSubtitleMetrics().subUseMargins),

src/core/services/mpv.test.ts

@@ -652,7 +652,7 @@ test('MpvIpcClient captures and disables secondary subtitle visibility on reques
   ]);
 });
 
-test('MpvIpcClient restorePreviousSecondarySubVisibility restores and clears tracked value', async () => {
+test('MpvIpcClient restores secondary subtitle visibility and relinquishes suppression', async () => {
   const commands: unknown[] = [];
   const client = new MpvIpcClient('/tmp/mpv.sock', makeDeps());
   const previous: boolean[] = [];
@@ -671,6 +671,12 @@ test('MpvIpcClient restorePreviousSecondarySubVisibility restores and clears tra
   });
   client.restorePreviousSecondarySubVisibility();
 
+  await invokeHandleMessage(client, {
+    event: 'property-change',
+    name: 'secondary-sub-visibility',
+    data: 'yes',
+  });
+
   assert.equal(previous[0], true);
   assert.equal(previous.length, 1);
   assert.deepEqual(commands, [
@@ -682,8 +688,53 @@ test('MpvIpcClient restorePreviousSecondarySubVisibility restores and clears tra
     },
   ]);
 
+  await invokeHandleMessage(client, {
+    event: 'property-change',
+    name: 'secondary-sub-visibility',
+    data: 'yes',
+  });
+  assert.equal(commands.length, 2);
+
   client.restorePreviousSecondarySubVisibility();
   assert.equal(commands.length, 2);
+
+  const callbacks = (client as any).transport.callbacks;
+  callbacks.onConnect();
+  commands.length = 0;
+
+  await invokeHandleMessage(client, {
+    event: 'property-change',
+    name: 'secondary-sub-visibility',
+    data: 'yes',
+  });
+  assert.deepEqual(commands, [{ command: ['set_property', 'secondary-sub-visibility', 'no'] }]);
+});
+
+test('MpvIpcClient keeps secondary subtitle suppression when restoration send fails', async () => {
+  const commands: unknown[] = [];
+  const client = new MpvIpcClient('/tmp/mpv.sock', makeDeps());
+
+  (client as any).send = (payload: unknown) => {
+    commands.push(payload);
+    return false;
+  };
+
+  await invokeHandleMessage(client, {
+    request_id: MPV_REQUEST_ID_SECONDARY_SUB_VISIBILITY,
+    data: 'yes',
+  });
+  client.restorePreviousSecondarySubVisibility();
+  await invokeHandleMessage(client, {
+    event: 'property-change',
+    name: 'secondary-sid',
+    data: 4,
+  });
+
+  assert.deepEqual(commands, [
+    { command: ['set_property', 'secondary-sub-visibility', 'no'] },
+    { command: ['set_property', 'secondary-sub-visibility', 'yes'] },
+    { command: ['set_property', 'secondary-sub-visibility', 'no'] },
+  ]);
 });
 
 test('MpvIpcClient updates current audio stream index from track list', async () => {

src/core/services/mpv.ts

@@ -184,6 +184,7 @@ export class MpvIpcClient implements MpvClient {
     osdDimensions: null,
   };
   private previousSecondarySubVisibility: boolean | null = null;
+  private enforceSecondarySubVisibilityHidden = true;
   private playbackPaused: boolean | null = null;
   private pauseAtTime: number | null = null;
   private pendingPauseAtSubEnd = false;
@@ -199,6 +200,7 @@ export class MpvIpcClient implements MpvClient {
       socketFactory: deps.socketFactory,
       connectTimeoutMs: deps.connectTimeoutMs,
       onConnect: () => {
+        this.enforceSecondarySubVisibilityHidden = true;
         this.connected = true;
         this.connecting = false;
         this.socket = this.transport.getSocket();
@@ -476,6 +478,7 @@ export class MpvIpcClient implements MpvClient {
       },
       resolvePendingRequest: (requestId: number, message: MpvMessage) =>
         this.tryResolvePendingRequest(requestId, message),
+      shouldEnforceSecondarySubVisibilityHidden: () => this.enforceSecondarySubVisibilityHidden,
       setSecondarySubVisibility: (visible: boolean) => this.setSecondarySubVisibility(visible),
       syncCurrentAudioStreamIndex: () => {
         this.syncCurrentAudioStreamIndex();
@@ -647,9 +650,11 @@ export class MpvIpcClient implements MpvClient {
   restorePreviousSecondarySubVisibility(): void {
     const previous = this.previousSecondarySubVisibility;
     if (previous === null) return;
-    this.send({
+    const restored = this.send({
       command: ['set_property', 'secondary-sub-visibility', previous ? 'yes' : 'no'],
     });
+    if (!restored) return;
+    this.enforceSecondarySubVisibilityHidden = false;
     this.previousSecondarySubVisibility = null;
   }