diff --git a/changes/secondary-subtitle-track-visibility.md b/changes/secondary-subtitle-track-visibility.md new file mode 100644 index 00000000..10ffaffd --- /dev/null +++ b/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. diff --git a/src/core/services/mpv-protocol.test.ts b/src/core/services/mpv-protocol.test.ts index b92d23ac..a1f0a791 100644 --- a/src/core/services/mpv-protocol.test.ts +++ b/src/core/services/mpv-protocol.test.ts @@ -83,6 +83,7 @@ function createDeps(overrides: Partial = {}): { 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(); diff --git a/src/core/services/mpv-protocol.ts b/src/core/services/mpv-protocol.ts index c892a577..3b9a7ccc 100644 --- a/src/core/services/mpv-protocol.ts +++ b/src/core/services/mpv-protocol.ts @@ -72,6 +72,7 @@ export interface MpvProtocolHandleMessageDeps { emitSubtitleMetricsChange: (payload: Partial) => 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), diff --git a/src/core/services/mpv.test.ts b/src/core/services/mpv.test.ts index 4afb556f..afe6a5bf 100644 --- a/src/core/services/mpv.test.ts +++ b/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 () => { diff --git a/src/core/services/mpv.ts b/src/core/services/mpv.ts index 271e9735..3345733d 100644 --- a/src/core/services/mpv.ts +++ b/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; }