Skip to content

refactor(embedded-mpv): split session controller into focused collaborators [2/7]#4

Closed
larsemig wants to merge 1 commit into
pr/01-player-controls-corefrom
pr/02-embedded-mpv-decompose
Closed

refactor(embedded-mpv): split session controller into focused collaborators [2/7]#4
larsemig wants to merge 1 commit into
pr/01-player-controls-corefrom
pr/02-embedded-mpv-decompose

Conversation

@larsemig

@larsemig larsemig commented Jul 7, 2026

Copy link
Copy Markdown
Owner

Note

Sidenote from the author: My review focus for this series was the architecture, not every implementation detail. If there are specific contribution guidelines I should follow (commit style, PR conventions, test expectations, etc.), please let me know and I will adapt the series accordingly.

Step 2 of the 7-PR series implementing the direction approved in 4gray#1105.

What

Mechanical decomposition of the monolithic embedded-MPV session controller into focused collaborators (503 → 304 lines), preparing it for the shared-controls adapter in step 3:

  • embedded-mpv-command-runner.ts — IPC command delegation with the exact same guard order and snapshot reconciliation
  • embedded-mpv-session-factory.ts — pure loading/error session snapshot factories
  • embedded-mpv-stalled-tracker.ts — stalled-playback timer, verbatim move
  • embedded-mpv-compositor.tsmeasureBounds (byte-identical move); embedded-mpv-format.utils keeps a re-export shim so existing imports don't break

Review notes

  • No change to existing behavior paths. The old player component and its spec are byte-identical to the base branch; the controller's public API is preserved via bound-field delegators.
  • One deliberate relocation: the error-session factory is now pure — sessionId.set(null) moved from inside the factory to the controller call site (same observable effect).
  • Net-new, not-yet-consumed API is intentionally landed here to spare step 3 from re-touching this file: attach(), createAttachingSession(), boundsTick. Nothing existing reads them yet; attach() has unit coverage.
  • Lint + full ui-playback suite green (182 tests, incl. the untouched old component spec).

🗺️ Series roadmap (proposal 4gray#1105)

Step Branch Purpose
1 pr/01-player-controls-core Shared engine-agnostic controls layer (lib only, flag OFF)
➡️ 2 pr/02-embedded-mpv-decompose Decompose embedded-MPV session controller (no behavior change)
3 pr/03-embedded-mpv-immersive Embedded MPV adopts shared controls + immersive overlay (macOS)
4 pr/04-embedded-mpv-native-fullscreen Real macOS native fullscreen for embedded MPV
5 pr/05-html5-shared-controls HTML5 player on shared controls (flag OFF)
6 pr/06-vjs-shared-controls Video.js on shared controls (flag OFF)
7 pr/07-art-player-shared-controls ArtPlayer on shared controls (flag OFF)

Stacked series: each PR is based on the previous one, so every diff shows only its own step.

🤖 Generated with Claude Code

@larsemig
larsemig force-pushed the pr/01-player-controls-core branch from 9cbf902 to 8010bc4 Compare July 7, 2026 19:00
@larsemig
larsemig force-pushed the pr/02-embedded-mpv-decompose branch from 2b1f5e5 to b73b54f Compare July 7, 2026 19:00
…rators

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>
@larsemig
larsemig force-pushed the pr/02-embedded-mpv-decompose branch from b73b54f to 2f9a144 Compare July 7, 2026 19:56
@larsemig
larsemig force-pushed the pr/01-player-controls-core branch from 8010bc4 to e3c8da6 Compare July 7, 2026 19:56
@larsemig

larsemig commented Jul 7, 2026

Copy link
Copy Markdown
Owner Author

Used as template only

@larsemig larsemig closed this Jul 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant