diff --git a/.changes/playback-embedded-mpv-slow-shell-selection.md b/.changes/playback-embedded-mpv-slow-shell-selection.md new file mode 100644 index 000000000..61d0b1f1f --- /dev/null +++ b/.changes/playback-embedded-mpv-slow-shell-selection.md @@ -0,0 +1,8 @@ +--- +type: fix +area: playback +--- + +On Linux, the app no longer switches a saved Embedded MPV player back to the +default player when your login shell is slow to start. It now keeps your +choice until it knows for certain whether mpv is installed. diff --git a/apps/electron-backend/src/app/events/embedded-mpv.events.login-shell.spec.ts b/apps/electron-backend/src/app/events/embedded-mpv.events.login-shell.spec.ts new file mode 100644 index 000000000..8fab35178 --- /dev/null +++ b/apps/electron-backend/src/app/events/embedded-mpv.events.login-shell.spec.ts @@ -0,0 +1,198 @@ +/** + * The Linux native-view support check end to end in the main process: the + * real IPC handlers, native service and login shell PATH lookup. Only the + * process boundary is faked: the shell (`readPath`), `mpv --version` + * (`spawnSync`) and Electron. `process.platform` is forced here, so the + * Linux branch runs on every host. + */ +import { + EMBEDDED_MPV_PREPARE, + EMBEDDED_MPV_SUPPORT, + type EmbeddedMpvSupport, +} from '@iptvnator/shared/interfaces'; + +const mockSpawnSync = jest.fn(); +const mockIpcHandle = jest.fn(); + +jest.mock('child_process', () => ({ spawnSync: mockSpawnSync })); +jest.mock('electron', () => ({ + app: { + isPackaged: true, + getAppPath: () => '/mock/app.asar', + commandLine: { getSwitchValue: () => '' }, + }, + ipcMain: { handle: mockIpcHandle }, + powerSaveBlocker: { + start: jest.fn(), + stop: jest.fn(), + isStarted: jest.fn(), + }, + screen: { getDisplayMatching: jest.fn() }, +})); +jest.mock('../app', () => ({ + __esModule: true, + default: { mainWindow: null }, +})); +jest.mock('../services/embedded-mpv-session-options', () => ({ + readEmbeddedMpvSessionOptions: () => ({ + extraOptions: [], + autoReconnect: true, + }), +})); +jest.mock('../services/embedded-mpv-frame-copy-platform.util', () => ({ + ...jest.requireActual('../services/embedded-mpv-frame-copy-platform.util'), + getFrameCopyRuntimeAvailability: () => ({ + usable: false, + reason: 'helper-probe-failed', + }), + isFrameCopyRuntimeUsable: () => false, +})); + +const INHERITED_PATH = '/usr/bin:/bin'; +const LOGIN_SHELL_ONLY_DIR = '/home/user/.local/bin'; +const LOGIN_SHELL_PATH = `${LOGIN_SHELL_ONLY_DIR}:${INHERITED_PATH}`; +/** Budget of the lookup; the shell in these tests never answers within it. */ +const LOOKUP_BUDGET_MS = 5; + +type SupportHandler = (event: unknown) => Promise; + +async function flushLookup(): Promise { + await new Promise((resolve) => setImmediate(resolve)); + await new Promise((resolve) => setImmediate(resolve)); +} + +describe('Embedded MPV support and a slow login shell (Linux native-view)', () => { + const originalPlatform = process.platform; + const originalEnv = { + PATH: process.env.PATH, + DISPLAY: process.env.DISPLAY, + WAYLAND_DISPLAY: process.env.WAYLAND_DISPLAY, + IPTVNATOR_ENABLE_EMBEDDED_MPV_FRAME_COPY: + process.env.IPTVNATOR_ENABLE_EMBEDDED_MPV_FRAME_COPY, + }; + let answerShell: (path: string) => void; + + function handlerFor(channel: string): SupportHandler { + const registration = mockIpcHandle.mock.calls.find( + ([registered]) => registered === channel + ); + if (!registration) { + throw new Error(`Missing ipcMain handler for ${channel}`); + } + return registration[1] as SupportHandler; + } + + beforeEach(async () => { + jest.resetModules(); + mockIpcHandle.mockReset(); + mockSpawnSync.mockReset(); + // mpv is installed where only the login shell PATH reaches it. + mockSpawnSync.mockImplementation(() => ({ + status: (process.env.PATH ?? '') + .split(':') + .includes(LOGIN_SHELL_ONLY_DIR) + ? 0 + : 1, + })); + Object.defineProperty(process, 'platform', { value: 'linux' }); + process.env.PATH = INHERITED_PATH; + process.env.DISPLAY = ':0'; + delete process.env.WAYLAND_DISPLAY; + delete process.env.IPTVNATOR_ENABLE_EMBEDDED_MPV_FRAME_COPY; + + const { scheduleDeferredFixPath } = + await import('../startup/login-shell-path'); + const { embeddedMpvNativeService } = + await import('../services/embedded-mpv-native.service'); + await import('./embedded-mpv.events'); + // The addon is normally a vendored .node file; with it in place, + // mpv on PATH is the only thing support depends on. + ( + embeddedMpvNativeService as unknown as { + addon: { isSupported(): boolean }; + } + ).addon = { isSupported: () => true }; + + scheduleDeferredFixPath( + () => + new Promise((resolve) => { + answerShell = resolve; + }), + LOOKUP_BUDGET_MS + ); + // The lookup starts on the next tick; only then can it be answered. + await flushLookup(); + }); + + afterEach(async () => { + // Let the lookup finish, so no test leaves a pending shell behind. + answerShell(INHERITED_PATH); + await flushLookup(); + }); + + afterAll(() => { + Object.defineProperty(process, 'platform', { + value: originalPlatform, + }); + for (const [key, value] of Object.entries(originalEnv)) { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } + }); + + it.each([EMBEDDED_MPV_SUPPORT, EMBEDDED_MPV_PREPARE])( + '%s reports a missing mpv as inconclusive until the shell answers', + async (channel) => { + const check = handlerFor(channel); + + // The lookup runs out of budget: this probe sees the inherited + // PATH, where mpv is missing. + await expect(check({})).resolves.toMatchObject({ + supported: false, + inconclusive: true, + }); + // Asked again meanwhile: the cached answer is still not final. + await expect(check({})).resolves.toMatchObject({ + supported: false, + inconclusive: true, + }); + expect(mockSpawnSync).toHaveBeenCalledTimes(1); + + answerShell(LOGIN_SHELL_PATH); + await flushLookup(); + + const settled = await check({}); + expect(settled.supported).toBe(true); + expect(settled.inconclusive).toBeUndefined(); + expect(mockSpawnSync).toHaveBeenCalledTimes(2); + } + ); + + it('reports a missing mpv as final once the shell answered without it', async () => { + const support = handlerFor(EMBEDDED_MPV_SUPPORT); + await expect(support({})).resolves.toMatchObject({ + supported: false, + inconclusive: true, + }); + + answerShell(INHERITED_PATH); + await flushLookup(); + + const settled = await support({}); + expect(settled.supported).toBe(false); + expect(settled.reason).toContain('mpv executable'); + expect(settled.inconclusive).toBeUndefined(); + }); + + it('reports a missing mpv as final when the shell answered in time', async () => { + answerShell(INHERITED_PATH); + await flushLookup(); + + const answer = await handlerFor(EMBEDDED_MPV_SUPPORT)({}); + expect(answer.supported).toBe(false); + expect(answer.inconclusive).toBeUndefined(); + }); +}); diff --git a/apps/electron-backend/src/app/events/embedded-mpv.events.spec.ts b/apps/electron-backend/src/app/events/embedded-mpv.events.spec.ts index b496fc0c2..1c34a63c1 100644 --- a/apps/electron-backend/src/app/events/embedded-mpv.events.spec.ts +++ b/apps/electron-backend/src/app/events/embedded-mpv.events.spec.ts @@ -10,6 +10,7 @@ const mockEmbeddedMpvService = { getSupport: jest.fn(), willProbeLinuxMpvExecutable: jest.fn(() => false), forgetLinuxMpvExecutableProbe: jest.fn(), + markLinuxMpvExecutableProbeProvisional: jest.fn(), setPaused: jest.fn(), }; const mockSessionOptions = { @@ -26,9 +27,7 @@ jest.mock('../services/embedded-mpv-session-options', () => ({ })); const mockWaitForLoginShellPath = jest.fn(() => Promise.resolve(true)); let settleLookup: () => void = () => undefined; -const mockLookupSettled = new Promise((resolve) => { - settleLookup = resolve; -}); +let mockLookupSettled = Promise.resolve(); jest.mock('../startup/login-shell-path', () => ({ waitForLoginShellPath: () => mockWaitForLoginShellPath(), whenLoginShellPathSettled: () => mockLookupSettled, @@ -69,6 +68,16 @@ describe('EmbeddedMpvEvents IPC handlers', () => { }); describe('support checks and the login shell PATH', () => { + beforeEach(() => { + // A lookup of its own per test: the pending re-probe of one test + // must not answer for the next. + mockLookupSettled = new Promise((resolve) => { + settleLookup = resolve; + }); + mockEmbeddedMpvService.forgetLinuxMpvExecutableProbe.mockClear(); + mockEmbeddedMpvService.markLinuxMpvExecutableProbeProvisional.mockClear(); + }); + afterEach(() => { mockWaitForLoginShellPath.mockClear(); mockEmbeddedMpvService.willProbeLinuxMpvExecutable.mockReset(); @@ -110,6 +119,10 @@ describe('EmbeddedMpvEvents IPC handlers', () => { expect( mockEmbeddedMpvService.forgetLinuxMpvExecutableProbe ).not.toHaveBeenCalled(); + // The probe saw the login shell PATH: its answer is final. + expect( + mockEmbeddedMpvService.markLinuxMpvExecutableProbeProvisional + ).not.toHaveBeenCalled(); } ); @@ -125,6 +138,17 @@ describe('EmbeddedMpvEvents IPC handlers', () => { await expect( getIpcMainHandler(EMBEDDED_MPV_SUPPORT)({}) ).resolves.toEqual({ supported: false }); + // The service is told before it probes, so the answer of this + // very check is already marked as not final. + const { markLinuxMpvExecutableProbeProvisional, getSupport } = + mockEmbeddedMpvService; + expect( + markLinuxMpvExecutableProbeProvisional + ).toHaveBeenCalledTimes(1); + expect( + markLinuxMpvExecutableProbeProvisional.mock + .invocationCallOrder[0] + ).toBeLessThan(getSupport.mock.invocationCallOrder[0]); expect( mockEmbeddedMpvService.forgetLinuxMpvExecutableProbe ).not.toHaveBeenCalled(); @@ -136,6 +160,33 @@ describe('EmbeddedMpvEvents IPC handlers', () => { ).toHaveBeenCalledTimes(1); }); + it('still re-probes when the check on the inherited PATH throws', async () => { + const consoleErrorSpy = jest + .spyOn(console, 'error') + .mockImplementation(); + mockEmbeddedMpvService.willProbeLinuxMpvExecutable.mockReturnValue( + true + ); + mockWaitForLoginShellPath.mockResolvedValueOnce(false); + mockEmbeddedMpvService.getSupport.mockImplementation(() => { + throw new Error('probe failed'); + }); + + try { + await expect( + getIpcMainHandler(EMBEDDED_MPV_SUPPORT)({}) + ).rejects.toThrow('probe failed'); + // Otherwise the provisional state would outlive the lookup. + settleLookup(); + await new Promise((resolve) => setImmediate(resolve)); + expect( + mockEmbeddedMpvService.forgetLinuxMpvExecutableProbe + ).toHaveBeenCalledTimes(1); + } finally { + consoleErrorSpy.mockRestore(); + } + }); + it('does not wait when no probe runs, nor for session calls', async () => { mockEmbeddedMpvService.getSupport.mockReturnValue({ supported: true, diff --git a/apps/electron-backend/src/app/events/embedded-mpv.events.ts b/apps/electron-backend/src/app/events/embedded-mpv.events.ts index 904bd9bb2..cd49c6c58 100644 --- a/apps/electron-backend/src/app/events/embedded-mpv.events.ts +++ b/apps/electron-backend/src/app/events/embedded-mpv.events.ts @@ -80,13 +80,14 @@ async function afterLoginShellPathIfProbing(check: () => T): Promise { ) { return check(); } - // The lookup ran out of budget, so this probe sees the inherited PATH. - // Once the shell does answer, the result is probed again. - const result = check(); + // The lookup ran out of budget, so this probe sees the inherited PATH: a + // missing mpv is answered as inconclusive, never as a verdict the + // renderer may persist. Once the shell does answer, it is probed again. + getService().markLinuxMpvExecutableProbeProvisional(); void whenLoginShellPathSettled().then(() => getService().forgetLinuxMpvExecutableProbe() ); - return result; + return check(); } handleEmbeddedMpv(EMBEDDED_MPV_SUPPORT, () => diff --git a/apps/electron-backend/src/app/services/embedded-mpv-native.service.spec.ts b/apps/electron-backend/src/app/services/embedded-mpv-native.service.spec.ts index 728af5580..9991436e8 100644 --- a/apps/electron-backend/src/app/services/embedded-mpv-native.service.spec.ts +++ b/apps/electron-backend/src/app/services/embedded-mpv-native.service.spec.ts @@ -414,6 +414,56 @@ describe('EmbeddedMpvNativeService power blocker', () => { expect(service.willProbeLinuxMpvExecutable()).toBe(true); }); + it('reports a missing mpv as inconclusive only while its probe is provisional', () => { + Object.defineProperty(process, 'platform', { value: 'linux' }); + process.env.DISPLAY = ':0'; + delete process.env.WAYLAND_DISPLAY; + mockSpawnSync.mockReturnValue({ status: 1 }); + mockRuntimeUsable(); + + // The login shell has not answered: mpv is looked up on the + // inherited PATH. + service.markLinuxMpvExecutableProbeProvisional(); + expect(service.getSupport()).toEqual( + expect.objectContaining({ + supported: false, + inconclusive: true, + }) + ); + expect(service.prepareAddon()).toEqual( + expect.objectContaining({ + supported: false, + inconclusive: true, + }) + ); + + // It answered: the next probe is a verdict again. + service.forgetLinuxMpvExecutableProbe(); + const settled = service.getSupport(); + expect(settled.supported).toBe(false); + expect(settled.reason).toContain('mpv executable'); + expect(settled.inconclusive).toBeUndefined(); + }); + + it('keeps every other answer final while the mpv probe is provisional', () => { + Object.defineProperty(process, 'platform', { value: 'linux' }); + process.env.DISPLAY = ':0'; + delete process.env.WAYLAND_DISPLAY; + mockSpawnSync.mockReturnValue({ status: 0 }); + mockRuntimeUsable(); + service.markLinuxMpvExecutableProbeProvisional(); + + const found = service.getSupport(); + expect(found.supported).toBe(true); + expect(found.inconclusive).toBeUndefined(); + + // mpv is there, the addon is not: the PATH cannot change that. + addon.isSupported.mockReturnValue(false); + const unsupported = service.getSupport(); + expect(unsupported.supported).toBe(false); + expect(unsupported.inconclusive).toBeUndefined(); + }); + it('predicts no probe for the frame-copy engine or native Wayland', () => { Object.defineProperty(process, 'platform', { value: 'linux' }); process.env.DISPLAY = ':0'; diff --git a/apps/electron-backend/src/app/services/embedded-mpv-native.service.ts b/apps/electron-backend/src/app/services/embedded-mpv-native.service.ts index 10f30a806..e929d240f 100644 --- a/apps/electron-backend/src/app/services/embedded-mpv-native.service.ts +++ b/apps/electron-backend/src/app/services/embedded-mpv-native.service.ts @@ -174,6 +174,12 @@ export class EmbeddedMpvNativeService { private powerBlockerId: number | null = null; private readonly loadAddonModule = createRequire(__filename); private cachedLinuxMpvExecutableReason: string | null | undefined; + /** + * True while `mpv --version` runs, or was cached, on the inherited PATH + * because the login shell had not answered: a missing mpv is then no + * verdict yet. + */ + private linuxMpvExecutableProbeIsProvisional = false; private frameCopyAdapter: EmbeddedMpvFrameCopyAdapter | null = null; private sessionOptionsDirectory: string | null = null; /** @@ -401,6 +407,16 @@ export class EmbeddedMpvNativeService { */ forgetLinuxMpvExecutableProbe(): void { this.cachedLinuxMpvExecutableReason = undefined; + this.linuxMpvExecutableProbeIsProvisional = false; + } + + /** + * Declares that the probe sees the inherited PATH, the login shell one + * not having arrived. Until `forgetLinuxMpvExecutableProbe()`, a missing + * mpv is reported as `inconclusive`, so no caller settles on it. + */ + markLinuxMpvExecutableProbeProvisional(): void { + this.linuxMpvExecutableProbeIsProvisional = true; } getSupport(): EmbeddedMpvSupport { @@ -458,6 +474,9 @@ export class EmbeddedMpvNativeService { supported: false, platform: process.platform, reason: missingLinuxMpvExecutableReason, + ...(this.linuxMpvExecutableProbeIsProvisional + ? { inconclusive: true } + : {}), ...this.getFrameCopySupportDetails(), }; } diff --git a/docs/architecture/embedded-mpv-native.md b/docs/architecture/embedded-mpv-native.md index 69f46a5d3..ae39402cf 100644 --- a/docs/architecture/embedded-mpv-native.md +++ b/docs/architecture/embedded-mpv-native.md @@ -185,6 +185,21 @@ support API from global workspace startup paths; use an explicit user action or idle preparation path when a renderer surface only needs to reveal optional Embedded MPV UI. +An unsupported answer can be `inconclusive`. The Linux native-view `mpv` +executable check runs `mpv --version` by bare name, so the support and prepare +handlers wait for the login shell PATH lookup (`startup/login-shell-path.ts`) +first. When that lookup runs out of its budget, the check runs on the +inherited PATH: `EmbeddedMpvNativeService` then reports a missing `mpv` as +`supported: false` with `inconclusive: true`, keeps doing so while the cached +result stands, and probes again once the shell answers. Every other answer, +including a missing `mpv` after the shell answered, is final. An inconclusive +answer is not a verdict on the machine: never persist a decision made from it. +The settings store resets a saved Embedded MPV selection only on a final +unsupported answer; on an inconclusive one the selection stays, and the player +asks again when playback starts. The command palette and the settings search +keep a final answer for the session, but probe again on their next use after +an inconclusive one. + When `embedded-mpv` is the saved player, the settings store schedules an idle `prepareEmbeddedMpv()` call. This intentionally moves the first native addon load away from the click-to-play path. It can still block the Electron main process briefly because Node native addon loading is synchronous, but doing it during idle is less visible than doing it when the user clicks a video. Actual MPV session creation still happens on playback because it needs the current Electron window handle and viewport bounds. For the native-view engine, the MPV video surface is a platform view/window, diff --git a/libs/services/src/lib/settings-store.embedded-mpv.spec.ts b/libs/services/src/lib/settings-store.embedded-mpv.spec.ts new file mode 100644 index 000000000..6724f1064 --- /dev/null +++ b/libs/services/src/lib/settings-store.embedded-mpv.spec.ts @@ -0,0 +1,98 @@ +import { Injector } from '@angular/core'; +import { StorageMap } from '@ngx-pwa/local-storage'; +import { of } from 'rxjs'; +import { + EmbeddedMpvSupport, + Settings, + STORE_KEY, + VideoPlayer, +} from '@iptvnator/shared/interfaces'; +import { EpgSourceSettingsService } from './epg-source-settings.service'; +import { SettingsStore } from './settings-store.service'; + +/** What the main process answers when the Linux `mpv` probe finds nothing. */ +const MPV_MISSING: EmbeddedMpvSupport = { + supported: false, + platform: 'linux', + reason: 'Embedded MPV on Linux requires the mpv executable on PATH.', + frameCopyAvailable: false, + frameCopyUnavailableReason: 'helper-probe-failed', +}; + +describe('SettingsStore saved Embedded MPV selection', () => { + const testWindow = window as unknown as { + electron?: { getEmbeddedMpvSupport: jest.Mock }; + }; + const originalElectron = testWindow.electron; + let storage: { get: jest.Mock; set: jest.Mock }; + let injector: Injector; + + /** Loads settings as on startup and lets the support check finish. */ + async function startWithSavedEmbeddedMpv( + support: EmbeddedMpvSupport + ): Promise> { + testWindow.electron = { + getEmbeddedMpvSupport: jest.fn().mockResolvedValue(support), + }; + const store = injector.get(SettingsStore); + await store.loadSettings(); + await new Promise((resolve) => setTimeout(resolve)); + expect(testWindow.electron.getEmbeddedMpvSupport).toHaveBeenCalled(); + return store; + } + + beforeEach(() => { + const saved: Partial = { player: VideoPlayer.EmbeddedMpv }; + storage = { + get: jest.fn(() => of(saved)), + set: jest.fn(() => of(undefined)), + }; + injector = Injector.create({ + providers: [ + SettingsStore, + EpgSourceSettingsService, + { provide: StorageMap, useValue: storage }, + ], + }); + jest.spyOn( + injector.get(EpgSourceSettingsService), + 'synchronize' + ).mockResolvedValue(undefined); + }); + + afterEach(() => { + testWindow.electron = originalElectron; + }); + + it('keeps the saved player when the support check is inconclusive', async () => { + // A slow login shell: mpv was looked up before its PATH arrived. + const store = await startWithSavedEmbeddedMpv({ + ...MPV_MISSING, + inconclusive: true, + }); + + expect(store.player()).toBe(VideoPlayer.EmbeddedMpv); + expect(storage.set).not.toHaveBeenCalled(); + }); + + it('falls back to the default player on a final unsupported answer', async () => { + const store = await startWithSavedEmbeddedMpv(MPV_MISSING); + + expect(store.player()).toBe(VideoPlayer.VideoJs); + expect(storage.set).toHaveBeenCalledWith( + STORE_KEY.Settings, + expect.objectContaining({ player: VideoPlayer.VideoJs }) + ); + }); + + it('keeps the saved player when Embedded MPV is supported', async () => { + const store = await startWithSavedEmbeddedMpv({ + supported: true, + platform: 'linux', + engine: 'native', + }); + + expect(store.player()).toBe(VideoPlayer.EmbeddedMpv); + expect(storage.set).not.toHaveBeenCalled(); + }); +}); diff --git a/libs/services/src/lib/settings-store.service.ts b/libs/services/src/lib/settings-store.service.ts index 71800a36e..63f863bb4 100644 --- a/libs/services/src/lib/settings-store.service.ts +++ b/libs/services/src/lib/settings-store.service.ts @@ -338,14 +338,15 @@ export const SettingsStore = signalStore( try { const support = await window.electron.getEmbeddedMpvSupport(); - if (!support.supported) { + if (support.supported) { + scheduleEmbeddedMpvPrepare(); + } else if (!support.inconclusive) { + // An inconclusive answer is no verdict on this + // machine: the saved player stays as it is. await this.updateSettings({ player: DEFAULT_SETTINGS.player, }); - return; } - - scheduleEmbeddedMpvPrepare(); } catch (error) { console.warn( 'Failed to verify embedded MPV support; reverting to the default inline player.', diff --git a/libs/shared/interfaces/src/lib/embedded-mpv-session.interface.ts b/libs/shared/interfaces/src/lib/embedded-mpv-session.interface.ts index f452966c5..0f90c67c0 100644 --- a/libs/shared/interfaces/src/lib/embedded-mpv-session.interface.ts +++ b/libs/shared/interfaces/src/lib/embedded-mpv-session.interface.ts @@ -39,6 +39,13 @@ export interface EmbeddedMpvSupport { supported: boolean; platform: string; reason?: string; + /** + * True when `supported: false` is not a verdict on this machine yet: the + * Linux native-view `mpv` executable was looked up before the login shell + * PATH arrived, and is looked up again once the shell answers. Never + * persist a decision made from such an answer; ask again later. + */ + inconclusive?: boolean; capabilities?: EmbeddedMpvCapabilities; /** * Rendering engine the main process will use for new sessions. diff --git a/libs/workspace/shell/feature/src/lib/workspace-player-commands/workspace-player-commands.contributor.spec.ts b/libs/workspace/shell/feature/src/lib/workspace-player-commands/workspace-player-commands.contributor.spec.ts index 6fd6a4a32..a52118bdc 100644 --- a/libs/workspace/shell/feature/src/lib/workspace-player-commands/workspace-player-commands.contributor.spec.ts +++ b/libs/workspace/shell/feature/src/lib/workspace-player-commands/workspace-player-commands.contributor.spec.ts @@ -53,7 +53,7 @@ describe('WorkspacePlayerCommandsContributor', () => { let electronStub: | { getEmbeddedMpvSupport: jest.Mock< - Promise<{ supported: boolean }>, + Promise<{ supported: boolean; inconclusive?: boolean }>, [] >; } @@ -63,7 +63,10 @@ describe('WorkspacePlayerCommandsContributor', () => { function bootstrap(options: { supportsManagedExternalPlayers: boolean; supportsEmbeddedMpv?: boolean; - embeddedMpvSupportResult?: { supported: boolean } | null; + embeddedMpvSupportResult?: { + supported: boolean; + inconclusive?: boolean; + } | null; }) { viewCommands = { registerCommand: jest.fn().mockReturnValue(() => undefined), @@ -200,6 +203,50 @@ describe('WorkspacePlayerCommandsContributor', () => { expect(resolveBoolean(embedded?.visible)).toBe(false); }); + it('keeps a final unsupported answer, but asks again after an inconclusive one', async () => { + const contributor = bootstrap({ + supportsManagedExternalPlayers: true, + supportsEmbeddedMpv: true, + // A slow login shell: mpv was looked up before its PATH arrived. + embeddedMpvSupportResult: { supported: false, inconclusive: true }, + }); + const embedded = getRegistered(viewCommands).find( + (c) => c.id === 'switch-player-embedded-mpv' + ); + + await contributor.ensureEmbeddedMpvSupportLoaded(); + expect(resolveBoolean(embedded?.visible)).toBe(false); + + // The shell answered without mpv: that answer is final. + electronStub?.getEmbeddedMpvSupport.mockResolvedValue({ + supported: false, + }); + await contributor.ensureEmbeddedMpvSupportLoaded(); + expect(resolveBoolean(embedded?.visible)).toBe(false); + expect(contributor.ensureEmbeddedMpvSupportLoaded()).toBeUndefined(); + expect(electronStub?.getEmbeddedMpvSupport).toHaveBeenCalledTimes(2); + }); + + it('shows embedded MPV once an inconclusive answer turns into supported', async () => { + const contributor = bootstrap({ + supportsManagedExternalPlayers: true, + supportsEmbeddedMpv: true, + embeddedMpvSupportResult: { supported: false, inconclusive: true }, + }); + const embedded = getRegistered(viewCommands).find( + (c) => c.id === 'switch-player-embedded-mpv' + ); + await contributor.ensureEmbeddedMpvSupportLoaded(); + + electronStub?.getEmbeddedMpvSupport.mockResolvedValue({ + supported: true, + }); + await contributor.ensureEmbeddedMpvSupportLoaded(); + + expect(resolveBoolean(embedded?.visible)).toBe(true); + expect(contributor.ensureEmbeddedMpvSupportLoaded()).toBeUndefined(); + }); + it('switches to embedded MPV on run', () => { bootstrap({ supportsManagedExternalPlayers: true, diff --git a/libs/workspace/shell/feature/src/lib/workspace-player-commands/workspace-player-commands.contributor.ts b/libs/workspace/shell/feature/src/lib/workspace-player-commands/workspace-player-commands.contributor.ts index 4d0893e39..15e80942c 100644 --- a/libs/workspace/shell/feature/src/lib/workspace-player-commands/workspace-player-commands.contributor.ts +++ b/libs/workspace/shell/feature/src/lib/workspace-player-commands/workspace-player-commands.contributor.ts @@ -120,16 +120,20 @@ export class WorkspacePlayerCommandsContributor { } private async loadEmbeddedMpvSupport(): Promise { + // An inconclusive answer hides the command for now, but is not kept: + // the next palette open asks again. + let final = true; try { const support = await window.electron?.getEmbeddedMpvSupport?.(); this.embeddedMpvSupported.set(!!support?.supported); + final = !support?.inconclusive; } catch (error) { console.warn( 'Failed to verify embedded MPV support for the command palette.', error ); } finally { - this.embeddedMpvSupportChecked = true; + this.embeddedMpvSupportChecked = final; this.embeddedMpvSupportLoad = null; } } diff --git a/libs/workspace/shell/util/src/lib/settings-search/settings-search.service.spec.ts b/libs/workspace/shell/util/src/lib/settings-search/settings-search.service.spec.ts index 355d5c911..b945969e8 100644 --- a/libs/workspace/shell/util/src/lib/settings-search/settings-search.service.spec.ts +++ b/libs/workspace/shell/util/src/lib/settings-search/settings-search.service.spec.ts @@ -142,6 +142,7 @@ describe('SettingsSearchService', () => { function probeWith(support: { supported: boolean; + inconclusive?: boolean; frameCopyAvailable?: boolean; }) { const getEmbeddedMpvSupport = jest @@ -186,6 +187,32 @@ describe('SettingsSearchService', () => { ); }); + it('asks again after an inconclusive answer and keeps the final one', async () => { + // A slow login shell: mpv was looked up before its PATH arrived. + const getEmbeddedMpvSupport = probeWith({ + supported: false, + inconclusive: true, + }); + const { service } = setup({ + ...DESKTOP, + supportsEmbeddedMpv: true, + }); + const entries = () => service.visibleEntries().map(({ id }) => id); + + await service.ensureEmbeddedMpvSupportLoaded(); + expect(entries()).not.toContain('embedded-mpv-extra-options'); + + // The shell answered meanwhile, and mpv is there. + getEmbeddedMpvSupport.mockResolvedValue({ + platform: 'linux', + supported: true, + }); + await service.ensureEmbeddedMpvSupportLoaded(); + expect(entries()).toContain('embedded-mpv-extra-options'); + expect(service.ensureEmbeddedMpvSupportLoaded()).toBeUndefined(); + expect(getEmbeddedMpvSupport).toHaveBeenCalledTimes(2); + }); + it('does not probe where the runtime has no embedded MPV bridge', () => { const getEmbeddedMpvSupport = probeWith({ supported: true }); const { service } = setup(DESKTOP); diff --git a/libs/workspace/shell/util/src/lib/settings-search/settings-search.service.ts b/libs/workspace/shell/util/src/lib/settings-search/settings-search.service.ts index 0dddc9e8d..aa256d8ea 100644 --- a/libs/workspace/shell/util/src/lib/settings-search/settings-search.service.ts +++ b/libs/workspace/shell/util/src/lib/settings-search/settings-search.service.ts @@ -66,7 +66,8 @@ export class SettingsSearchService { * searchable. Returns the pending probe, or `undefined` when there is * nothing to wait for. Call it lazily (palette open, settings page), * never from shell bootstrap: supported desktop builds may load the - * native addon while answering. + * native addon while answering. An inconclusive answer is used until + * the next call, which probes again. */ ensureEmbeddedMpvSupportLoaded(): Promise | undefined { if (this.embeddedMpvSupportChecked) { @@ -88,7 +89,9 @@ export class SettingsSearchService { .then((support) => this.embeddedMpvSupport.set(support)) .catch(() => this.embeddedMpvSupport.set(null)) .finally(() => { - this.embeddedMpvSupportChecked = true; + this.embeddedMpvSupportChecked = + !this.embeddedMpvSupport()?.inconclusive; + this.embeddedMpvSupportLoad = undefined; }); return this.embeddedMpvSupportLoad; }