mirror of
https://github.com/4gray/iptvnator.git
synced 2026-10-14 05:06:16 -08:00
refactor(embedded-mpv): split session controller into focused collaborators [2/7] (#1149)
* refactor(embedded-mpv): split session controller into focused collaborators Mechanical decomposition of the embedded-MPV session controller into focused collaborators under embedded-mpv-player/: - embedded-mpv-command-runner.ts: transport/track/recording IPC delegators with guarded snapshot reconciliation - embedded-mpv-session-factory.ts: pure placeholder-session factories (loading/attaching/error) and the startup-paint wait - embedded-mpv-stalled-tracker.ts: loading-stall timer and stalled flag - embedded-mpv-compositor.ts: host bounds measurement (measureBounds), re-exported from embedded-mpv-format.utils for existing imports No behavior change. The existing embedded-mpv-player component is kept untouched and keeps working against the controller's unchanged public API (commands are now bound fields delegating to the runner). Test coverage extended per Codecov patch report. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(embedded-mpv): drop superseded overlay hooks * fix(embedded-mpv): guard async session races * docs(embedded-mpv): document renderer collaborators * fix(embedded-mpv): abort stale recording startup * docs(embedded-mpv): clarify renderer safety details --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: 4gray <serega05@gmail.com>
This commit is contained in:
11 files changed
+1179
-285
No files matched your search
@@ -306,8 +306,11 @@ The Angular side of the embedded MPV player is intentionally split so the player
|
||||
- `embedded-mpv-shortcuts.ts` — `EmbeddedMpvShortcuts` class with `attach(handlers)` / `detach()`. Owns the document keydown listener and routes through a callback interface; the component supplies the callbacks. Listens for Space/K (toggle), F (fullscreen), arrow keys (seek/volume), M (mute), Escape (close popovers).
|
||||
- `embedded-mpv-overlay-visibility.service.ts` — singleton service that exposes `overlayActive: signal<boolean>`. Tracks `MatDialog.afterOpened`/`afterAllClosed` for dialog-shaped overlays and falls back to a `MutationObserver` on the CDK overlay container for any remaining backdrop-bearing CDK overlays. The native MPV video host is hidden off-screen while a modal is open so DOM dialogs can paint above it.
|
||||
- `embedded-mpv-ui-state.ts` — `EmbeddedMpvMenuState` (single-open popover state machine with `volumeOpen`, `audioOpen`, `subtitleOpen`, `speedOpen`, `aspectOpen` signals plus `anyOpen` computed; `toggle`/`open`/`close`/`closeAll` helpers) and `EmbeddedMpvFeedback` (transient overlay that auto-clears after a configurable delay; used for keypress feedback).
|
||||
- `embedded-mpv-session-controller.ts` — component-scoped `Injectable` service that owns the `support`, `session`, `sessionId`, `stalled`, and `retryToken` signals. Subscribes to `onEmbeddedMpvSessionUpdate`, runs the polling-driven `stalled` timer, owns bounds-sync (resize, scroll, overlay state), and exposes the imperative IPC surface (`startSession`, `togglePaused`, `seekBy`/`seekTo`, `applyVolume`, `setAudioTrack`, `setSubtitleTrack`, `setSpeed`, `setAspect`, `startRecording`, `stopRecording`, `retry`).
|
||||
- `embedded-mpv-player.component.ts` — view-only shell. Holds view children, derived `computed` signals, DOM event listeners (pointermove, pointerdown, fullscreenchange, dblclick), and three `effect()`s.
|
||||
- `embedded-mpv-command-runner.ts` — transport/track/recording IPC delegation; contains addon-side throws; reconciles a returned snapshot only when the current canonical session id and returned snapshot id both match the captured command session id.
|
||||
- `embedded-mpv-session-factory.ts` — side-effect-free loading/error placeholder factories plus `waitForStartupPaint`.
|
||||
- `embedded-mpv-stalled-tracker.ts` — owns the 30-second loading timer and `stalled` signal.
|
||||
- `embedded-mpv-session-controller.ts` — component-scoped lifecycle coordinator. It exposes `support`, `session`, `sessionId`, `stalled`, and `retryToken`; subscribes to native updates; coordinates prepare/create/load/dispose, frame-copy attachment, and bounds sync; and delegates commands, placeholders, and stalled timing.
|
||||
- `embedded-mpv-player.component.ts` — view-only shell. Holds view children, derived `computed` signals, DOM event listeners (pointermove, pointerdown, fullscreenchange, dblclick), and effects for session lifecycle, overlay/menu-driven bounds sync, session fan-out, playback-ended emission, and recording elapsed-time ticks.
|
||||
|
||||
### Bounds compositing strategy
|
||||
|
||||
@@ -332,15 +335,18 @@ Concrete bugs from the audit, recorded so they don't get reintroduced:
|
||||
- **Infinite session-create loop.** `EmbeddedMpvSessionController.startSession` once wrote `this.support.set(prepared)` after the `prepareEmbeddedMpv` round-trip. The component's session-creation effect tracks `this.support()`, so the write fired the effect → cleanup disposed the session → new session was created → prepare ran again → support was set again. Symptom: endless "Loading stream…" spinner. Fix: do not write `support` inside `startSession`; the constructor's `loadSupport()` already populates it including capabilities.
|
||||
- **Stream restart on volume change.** The session-creation effect once read `this.volume()` directly to pass to `startSession`'s `initialVolume`. Each volume tick re-ran the effect, disposing and recreating the session — for VOD/series this restarted playback from the beginning. Fix: read it via `untracked(() => this.volume())`. Subsequent volume changes flow through `controller.applyVolume()`, never through the effect graph.
|
||||
- **Spurious `timeUpdate` re-emits and `volume.set` calls.** The session-fan-out effect calls `scheduleControlsHide()`, which reads `isPlaying`, `menus.anyOpen`, `statusLabel`, and `controlsVisible`. Those reads became tracked deps, so opening any popover, pausing, or hovering re-ran the body. No loop in isolation, but a parent that wires `timeUpdate` back into `playback.startTime` would have hit the volume-restart bug class. Fix: wrap the side-effect block in `untracked()` so the effect listens only to session changes.
|
||||
- **2 Hz no-op stalled-tracker re-runs.** The controller's stalled effect tracked the full `session` signal, which updates on every position-poll snapshot. `handleStalledTracking` is a no-op for non-loading status, so the re-runs cost nothing useful. Fix: track a `sessionStatus = computed(() => this.session()?.status ?? null)` instead so the effect fires only on real status transitions.
|
||||
- **2 Hz no-op stalled-tracker re-runs.** Position polling updates `session` around 2 Hz. Tracking the full session would re-run stalled logic for snapshots with unchanged status, so the controller tracks only `sessionStatus` and invokes `EmbeddedMpvStalledTracker.track` inside `untracked()`, avoiding full-session reruns.
|
||||
|
||||
When adding a new effect, audit it the same way: list every tracked signal read explicitly, justify each one as a _re-trigger source_, and wrap everything else in `untracked()`. When extending an existing helper that is called from inside an effect, treat the helper's signal reads as if they were inline in the effect.
|
||||
|
||||
### IPC safety
|
||||
|
||||
Renderer-side IPC methods on the controller use the canonical `sessionId()` signal as the gate, **not** `session()?.id`. The session payload during the loading window carries a placeholder id (`embedded-mpv-starting`) set by `createLoadingSession()`; pushing that placeholder to the addon would hit `getSessionOrThrow` for a session that does not exist. The native side throws `Napi::Error` rather than `std::runtime_error` so that misuse surfaces as a JS exception rather than a process abort, but the renderer should still gate properly so the addon never sees the placeholder.
|
||||
Renderer command IPC in `EmbeddedMpvCommandRunner` uses the canonical `sessionId()` signal as the gate, **not** `session()?.id`. The session payload during the loading window carries a placeholder id (`embedded-mpv-starting`) set by `createLoadingSession()`; pushing that placeholder to the addon would hit `getSessionOrThrow` for a session that does not exist. The native side throws `Napi::Error` rather than `std::runtime_error` so that misuse surfaces as a JS exception rather than a process abort, but the renderer should still gate properly so the addon never sees the placeholder.
|
||||
|
||||
Every IPC call goes through a `guardIpc` helper that swallows addon-side throws — sessions can be torn down while a call is in flight, and snapshot polling will resync state on the next tick.
|
||||
- **Playback-load teardown.** If teardown happens while `loadEmbeddedMpvPlayback` is in flight, the asynchronous startup task exits immediately after the load resolves, before frame attachment or bounds scheduling. The teardown path owns disposal of that session.
|
||||
- **Command identity.** Renderer commands capture the canonical `sessionId` before starting IPC. Default recording-folder resolution is asynchronous preflight before the recording-command IPC. The runner revalidates the captured id immediately after that preflight and skips IPC when it no longer matches, preventing a late recording command from being issued against the superseded session.
|
||||
- **Reply reconciliation.** `EmbeddedMpvCommandRunner` applies a returned snapshot only when both the current canonical `sessionId` and the returned snapshot id match the captured command session id. Late or mismatched replies are ignored. Its `guardIpc` helper contains addon-side errors, and the next broadcast session update resynchronizes state.
|
||||
- **Frame-attachment teardown.** If teardown happens during asynchronous frame attachment, the asynchronous startup task exits immediately after the attachment await, before bounds scheduling. The preload detach/attachment epochs abort pending frame setup, so the teardown's detach is not followed by a second late global detach.
|
||||
|
||||
### Power management
|
||||
|
||||
|
||||
Reference in new issue
Block a user