fix(embedded-mpv): address review findings on the frame-copy engine

- Stale pump attach can no longer win over a newer session: attach/detach
  bump a shared epoch and async attach waits re-check it after every await,
  so an attach for a replaced session aborts instead of installing itself
  (greptile P1).
- A failed frame-view attach (no canvas, no WebGL2, reader missing) now
  disposes the session and surfaces the error UI instead of leaving audio
  playing behind a black canvas (codex P2).
- A stale frame-copy opt-in without the helper binary falls back to the
  native engine instead of reporting embedded MPV unsupported, and the
  Settings checkbox stays visible while a saved opt-in exists so it can
  always be cleared (codex P2). Regression test covers the fallback.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
4grayandClaude Fable 5 committed 2026-07-15 18:20:45 +02:00
1 parent c6ad3469ab
commit 79f6955bcb
5 files changed
+57 -28

No files matched your search

@@ -83,6 +83,12 @@ interface PumpState {
let pump: PumpState | null = null;
let sourceListenerRegistered = false;
/**
* Monotonic attach epoch: bumped by every attach/detach. Async attach waits
* re-check it after each await so a stale attach (for a session that was
* replaced meanwhile) can never install itself over the active pump.
*/
let attachEpoch = 0;
function delay(ms: number): Promise<void> {
return new Promise((resolve) => setTimeout(resolve, ms));
@@ -226,14 +232,17 @@ export async function attachEmbeddedMpvFrameView(
sessionId: string
): Promise<boolean> {
detachEmbeddedMpvFrameView();
const epoch = ++attachEpoch;
ensureSourceListener();
const canvas = await waitForCanvas();
if (epoch !== attachEpoch) return false;
if (!canvas) {
console.error('[embedded-mpv-pump] frame canvas not found in DOM');
return false;
}
const source = await waitForFrameSource(sessionId);
if (epoch !== attachEpoch) return false;
if (!source) {
console.error('[embedded-mpv-pump] no frame source for', sessionId);
return false;
@@ -276,6 +285,7 @@ export async function attachEmbeddedMpvFrameView(
}
export function detachEmbeddedMpvFrameView(): void {
attachEpoch++; // aborts any attach still waiting on canvas/frame source
if (!pump) return;
cancelAnimationFrame(pump.rafHandle);
try {
@@ -206,6 +206,28 @@ describe('EmbeddedMpvNativeService power blocker', () => {
};
}
it('falls back to the native engine when frame-copy is requested without a helper binary', () => {
// A stale opt-in (cleaned native build) must not brick embedded MPV:
// no helper on disk => the engine env flag is ignored, native keeps
// working, and support does not advertise frame-copy.
process.env.IPTVNATOR_ENABLE_EMBEDDED_MPV_FRAME_COPY = '1';
jest.spyOn(
service as unknown as {
resolveFrameCopyHelperPath: () => string | null;
},
'resolveFrameCopyHelperPath'
).mockReturnValue(null);
try {
expect(service.getActiveEngine()).toBe('native');
const support = service.getSupport();
expect(support.engine).not.toBe('frame-copy');
startSession('s-fallback', snapshot('loading'));
expect(addon.createSession).toHaveBeenCalled();
} finally {
delete process.env.IPTVNATOR_ENABLE_EMBEDDED_MPV_FRAME_COPY;
}
});
it('does not acquire a blocker for a loading session', () => {
startSession('s1', snapshot('loading'));
expect(powerSaveBlockerMock.start).not.toHaveBeenCalled();
@@ -122,10 +122,14 @@ export class EmbeddedMpvNativeService {
}
private isFrameCopyEngineActive(): boolean {
// Requires the helper binary too: a stale opt-in (cleaned native
// build, bad install) must fall back to the native engine instead
// of leaving embedded MPV unsupported with no way to recover.
return (
this.isFrameCopyEngineRequested() &&
process.platform === 'darwin' &&
process.arch === 'arm64'
process.arch === 'arm64' &&
this.resolveFrameCopyHelperPath() !== null
);
}
@@ -241,33 +245,16 @@ export class EmbeddedMpvNativeService {
};
}
if (this.isFrameCopyEngineRequested()) {
if (!this.isFrameCopyEngineActive()) {
return {
supported: false,
platform: process.platform,
reason: 'The frame-copy embedded MPV engine is currently supported on Apple Silicon macOS only.',
};
}
if (!this.resolveFrameCopyHelperPath()) {
return {
supported: false,
platform: process.platform,
reason: 'The frame-copy embedded MPV helper binary was not found. Rebuild the native target (pnpm run serve:backend:embedded-mpv rebuilds it).',
};
}
// A requested-but-unavailable frame-copy engine (wrong platform or
// missing helper) intentionally falls through to the native path so
// embedded MPV keeps working and Settings can clear the opt-in.
if (this.isFrameCopyEngineActive()) {
return {
supported: true,
platform: process.platform,
engine: 'frame-copy',
frameCopyAvailable: true,
capabilities: {
subtitles: true,
playbackSpeed: true,
aspectOverride: true,
screenshot: false,
recording: true,
},
capabilities: this.detectCapabilities(),
};
}
@@ -114,7 +114,7 @@
</div>
</div>
@if (frameCopyAvailable()) {
@if (frameCopyAvailable() || form().value.embeddedMpvFrameCopy) {
<div
class="setting-item"
data-test-id="embedded-mpv-frame-copy-setting"
@@ -188,11 +188,21 @@ export class EmbeddedMpvSessionController {
await electron.loadEmbeddedMpvPlayback(created.id, playback);
if (untracked(() => this.isFrameCopyEngine())) {
// Frame-copy engine: start the preload frame pump that
// paints helper frames onto the component's canvas. Failure
// is non-fatal here — the session error/stall paths cover it.
void electron
// paints helper frames onto the component's canvas. A failed
// attach (no canvas, no WebGL2, reader missing) must surface
// as a session error — otherwise the helper keeps playing
// audio behind a black canvas with no recovery UI.
const attached = await electron
.attachEmbeddedMpvFrameView?.(created.id)
.catch(() => undefined);
.catch(() => false);
if (attached === false && !disposed) {
await electron
.disposeEmbeddedMpvSession(created.id)
.catch(() => undefined);
throw new Error(
'The embedded MPV frame view failed to initialize.'
);
}
}
scheduleBoundsSync();
};