go-mxl-player/old_notes/REFACTORING_PLAN.md

# MXL Player refactoring plan

## Goal

Build a player with two persistent logical feed slots: video and audio. Each
slot may be configured, started, stopped, replaced, reconnected, or removed
while the program is running.

When both slots are configured, the user may enable or disable synchronization
at any time. Synchronization is a runtime relationship between the slots, not a
startup mode.

The design must leave a clean extension point for an `mxlfabrics` reader after
the local MXL player is stable. Playlist timing and selection must remain above
media readers and playback workers.

## Current status

- Stages 0 through 11 are complete.
- The reader-factory extension point from Stage 12 is complete; the
  `mxlfabrics` implementation remains future work.
- Stage 13 is complete, including manual/timed navigation, looping,
  pause/resume, failure policies, playlist-level retry configuration, and GUI
  diagnostics.
- Playlist lifecycle messages are explicit `PlaylistEvent` values produced by
  `PlaylistEventCoordinator`; runtime cancellation owns shutdown, so shared
  event channels are not closed by either endpoint.

## Required behaviour

### No configured feeds

- Start the normal GUI and renderer.
- Show a placeholder, simple shader, or empty player surface.
- Allow the user to enter either or both UUIDs.

### One configured feed

- An audio UUID starts audio playback.
- A video UUID starts video playback.
- A failed feed reconnects according to its retry policy.
- The user may stop and later resume the feed without clearing its UUID.
- The user may replace or remove the UUID at runtime.

### Two configured feeds, synchronization disabled

- Audio and video run as independent workers.
- Failure of one feed must not stop, close, or restart the other.
- Each feed has its own state, last error, and retry counter.
- The user may stop or resume either feed independently.
- Synchronization can be enabled at runtime.

### Two configured feeds, synchronization enabled

- Both feeds are read through one synchronization group.
- Failure of either member fails the group attempt.
- Reconnection recreates the complete group.
- Synchronization can be disabled at runtime. The two feeds then continue as
  independent workers.
- The user may stop both feeds together. Stopping only one member dismantles
  the group and lets the other member continue independently.

### Stop, resume, and remove semantics

- `StopVideo` stops video reading and retrying but retains the video UUID.
- `StopAudio` stops audio reading and retrying but retains the audio UUID.
- `StopAll` stops both feeds and all retry activity but retains both UUIDs.
- A stopped feed may be resumed without re-entering its UUID.
- Removing a feed stops it and clears its UUID.
- Stopping one detached feed has no effect on the other feed.
- Stopping one member of a synchronized group disables synchronization,
  stops the selected feed, and continues the other feed independently.
- Stopping both members of a synchronized group closes the group atomically.
- Stop commands cancel active reads and retry backoff promptly.

### Playlist behaviour

A playlist is an ordered list of playback entries. Each entry describes a
complete desired session state and may contain:

- synchronized video and audio UUIDs;
- independent video and audio UUIDs;
- video only;
- audio only;
- an optional per-entry playback duration.

Example:

```text
1. synchronized video A + audio A, 10 seconds
2. synchronized video B + audio B, 15 seconds
3. video C, 10 seconds
4. video D, 20 seconds
5. audio E, 30 seconds
```

The user may select any entry manually. Automatic playback advances after the
current entry's duration and loops from the final entry back to the first.

Playlist orchestration belongs above `SessionController`. Selecting an entry
must translate its desired state into the same controller operations used by
CLI and GUI controls. Source readers, workers, retry supervisors, renderer, and
audio output must not know that a playlist exists.

## Decisions to confirm before implementation

- `MaxAttempts == 0` means retry indefinitely.
- The initial connection counts as attempt 1.
- Retry counters reset only after useful media has been received for a defined
  stability period, not merely after a reader opens.
- Enabling or disabling synchronization starts a new lifecycle and resets the
  applicable retry counters.
- Explicitly removing either feed disables requested synchronization.
- Explicitly stopping one member of a synchronized group disables requested
  synchronization. Resuming that feed does not silently recreate the group;
  the user can enable synchronization again.
- Requesting synchronization with only one configured feed records the request,
  continues independent playback, and waits for the second feed.
- Exhausting attempts leaves the UUID configured and the unit in `Failed` until
  the user retries, replaces the UUID, changes synchronization, or removes it.

These are proposed defaults. Change them here before implementing the affected
stage if different behaviour is desired.

## Target architecture

```text
CLI initial values                 GUI runtime commands
        |                                  |
        +----------------+-----------------+
                         v
                SessionController
              desired vs actual state
                         |
             +-----------+-----------+
             |                       |
      sync disabled              sync enabled
     VideoWorker                 SyncGroupWorker
     AudioWorker                 (video + audio)
             |                       |
             +-----------+-----------+
                         v
                stable output layer
               video sink / audio sink
```

### Ownership rules

- `SessionController` is the only owner allowed to change playback topology.
- A worker exclusively owns its reader and closes it when the worker stops.
- Independent audio and video workers never close each other's resources.
- A synchronized worker owns and closes the entire group.
- GUI and renderer consume status or media; they never inspect live readers.
- Every topology change increments a generation number. Late events from an old
  generation are ignored.

### Package direction

```text
internal/source
    Local MXL reader adapters and media metadata.
    No retry, GUI, SDL, Vulkan, or application policy.

internal/playback
    Feed configuration, commands, state, retry supervisor, independent workers,
    synchronized worker, and session controller.

internal/output (optional once extraction is useful)
    Stable video and audio sink interfaces and their adapters.

cmd/mxl-player
    Process initialization, CLI translation, GUI event translation, and wiring.
```

Do not introduce a single broad media-source interface. Video, audio, and a
synchronized pair have different results and should use small explicit
interfaces.

## Runtime state

Each independently supervised playback unit uses:

```text
Idle -> Connecting -> Playing -> Reconnecting -> Failed
  ^         |            |            |
  +---------+------------+------------+-> Stopping -> Idle
```

`Idle` may still have a configured UUID. Configuration and running state are
separate: a configured slot can be stopped without being removed.

Synchronization additionally distinguishes desired and actual state:

```text
Disabled
TransitioningOn
Enabled
TransitioningOff
WaitingForSecondFeed
```

The GUI should receive immutable status snapshots containing at least:

- configured UUID;
- desired and actual synchronization state;
- playback state per active unit;
- current attempt and configured limit;
- last error;
- last successful media time;
- received and dropped media counters.

## Retry policy

Retry belongs above `internal/source`.

One attempt is:

```text
open reader(s) -> receive media -> terminal read error -> close reader(s)
```

The supervisor then either stops or waits using cancellable capped exponential
backoff with small jitter before opening again.

Requirements:

- Apply the same policy to initial connection and later disconnection.
- Allow finite and infinite attempts.
- Cancel immediately on shutdown, UUID replacement, or topology change.
- Cancel immediately when the user stops the supervised feed.
- Never run two retry loops for the same playback unit.
- Retain and publish the final error after attempt exhaustion.
- Classify configuration/format errors separately from transient availability
  errors. Whether permanent errors should retry is decided explicitly.

## Runtime synchronization transitions

### Enable synchronization

1. Record synchronization as desired and increment the topology generation.
2. Cancel both independent workers.
3. Wait until both workers close their readers.
4. Clear stale queued audio.
5. Create the synchronized worker with the existing UUIDs.
6. Establish a new shared timeline and publish `Enabled` after valid paired data.
7. If opening or reading fails, retry the group as one unit.

The renderer may retain the last video frame or show a placeholder during the
transition.

### Disable synchronization

1. Record synchronization as disabled and increment the topology generation.
2. Cancel and close the synchronized worker and group.
3. Clear audio belonging to the old synchronized timeline.
4. Start independent workers for both configured slots.
5. Let either worker begin playing without waiting for the other.

## Playlist model

The implemented model is conceptually:

```go
type PlaylistEntry struct {
    Name          string
    Video         PlaylistFeed // domain + UUID, optional
    Audio         PlaylistFeed // domain + UUID, optional
    SyncRequested bool
    Duration      time.Duration
}

type Playlist struct {
    Entries   []PlaylistEntry
    Loop      bool
    OnFailure PlaylistFailurePolicy // wait or next
    Retry     *RetryPolicy           // optional playlist-wide override
}
```

The playlist controller owns:

- current entry index;
- manual selection;
- automatic advance timer;
- loop behaviour;
- pause/resume of automatic advance;
- applying one entry atomically to the session controller.

Before implementing playlists, decide when an entry's duration begins. The
recommended rule is when its requested playback topology first reaches
`Playing`, so connection and retry time do not consume the viewing period.
Also decide whether an entry that exhausts retries waits for user action or
automatically advances; this should be configurable rather than implicit.

## Implementation stages

Only begin a stage after the previous stage's acceptance criteria pass. Keep
each stage small enough for one focused review and commit.

### Stage 0 — Behaviour contract and baseline

Work:

- Confirm or edit the decisions in this document.
- Record the current CLI examples and observable behaviour.
- Run the existing build/tests and record known failures.
- Identify commands used for a local audio feed, video feed, and paired feeds.

Acceptance criteria:

- Behaviour choices are unambiguous.
- A repeatable baseline command is documented.
- No application code changes.

### Stage 1 — Pure configuration model

Work:

- Introduce configuration types for domain, two optional feed slots, desired
  synchronization, and retry policy.
- Make validation pure: return errors instead of printing or exiting.
- Permit no-feed startup.
- Require a domain only when at least one connection is requested.
- Translate CLI arguments into the same configuration later used by GUI actions.

Acceptance criteria:

- Unit tests cover zero, one, and two feeds plus invalid domains/policies.
- Existing playable CLI combinations still translate correctly.
- No playback lifecycle is moved yet.

### Stage 2 — Normalize low-level source APIs

Work:

- Use consistent `VideoSource`, `AudioSource`, and `SyncSource` naming.
- Make every blocking read context-aware and bounded.
- Remove duplicate read paths where safe.
- Centralize audio-fragment extraction and video-frame construction.
- Define classifiable source errors.
- Make MXL payload ownership and copying rules explicit.
- Make a synchronized read fail when either member cannot provide its part.

Acceptance criteria:

- Source tests cover cancellation and important MXL error classification.
- A read cannot hide forever inside an internal retry loop.
- Existing audio-only, video-only, and synchronized smoke tests still work.

### Stage 3 — Retry supervisor

Work:

- Implement a reusable supervisor around open, run, close, and retry.
- Add finite/infinite limits and cancellable capped backoff.
- Publish state transitions and attempt counts.
- Add deterministic timing hooks so tests do not sleep in real time.

Acceptance criteria:

- Tests cover initial failure, runtime failure, exhaustion, successful recovery,
  counter reset, and cancellation during backoff.
- The supervisor has no SDL, Vulkan, ImGui, or concrete MXL dependency.

### Stage 4 — Independent video worker

Work:

- Move video reader ownership and retry lifecycle into a video worker.
- Deliver owned frames through a bounded latest-frame channel or video sink.
- Stop writing directly into renderer staging memory from the source worker.

Acceptance criteria:

- Video reconnects without application restart.
- Replacing or removing its UUID cancels the old reader promptly.
- Slow rendering does not create an unbounded live-stream backlog.

### Stage 5 — Independent audio worker

Work:

- Move audio reader ownership and retry lifecycle into an audio worker.
- Centralize channel interleaving.
- Use bounded ordered buffering and explicit backpressure.
- Define when the SDL queue is cleared during reconnect/replacement.

Acceptance criteria:

- Audio reconnects without application restart.
- Audio is correct for mono, stereo, and more than two channels.
- Cancellation cannot leave stale samples playing indefinitely.

### Stage 6 — Session controller and independent dual playback

Work:

- Add typed controller commands rather than overloaded channel values.
- Own video and audio slot configuration in the controller.
- Track configured and running/desired-active state separately for each slot.
- Add topology generations and ignore stale worker events.
- Run both independent workers when both UUIDs are configured.

Acceptance criteria:

- Video failure does not interrupt audio.
- Audio failure does not interrupt video.
- Either UUID can be replaced while the other feed stays alive.
- Either feed can be stopped and resumed while the other stays alive.
- Stopping both feeds leaves the application and configured UUIDs intact.
- Race detector finds no shared-state races in controller/worker tests.

### Stage 7 — Synchronized worker

Work:

- Move sync-group ownership into one supervised worker.
- Treat missing video or audio data as failure of the group attempt.
- Recreate all synchronized resources on reconnect.
- Publish paired media from one established timeline.

Acceptance criteria:

- Failure of either member restarts the complete group.
- Retry exhaustion stops the group cleanly and retains both configured UUIDs.
- Cancellation releases the group and both readers promptly.

### Stage 8 — Runtime sync toggle

Work:

- Implement atomic enable and disable transitions described above.
- Preserve configured UUIDs across both transitions.
- Handle toggle commands during connection, playback, and retry backoff.
- Handle per-feed and stop-all commands in detached and synchronized playback.
- Define and implement behaviour when only one slot is configured.

Acceptance criteria:

- Detached feeds can be joined without restarting the application.
- A sync group can be detached into independent feeds without restarting.
- Rapid repeated toggles cannot leave duplicate workers or readers.
- Stopping one synchronized member detaches the group and preserves playback of
  the other member.
- Stopping both synchronized members closes the group without starting new
  independent workers.
- Old-generation errors cannot change the new topology.

### Stage 9 — Stable dynamic outputs and idle startup

Work:

- Initialize GUI and rendering without requiring a successful source open.
- Show a placeholder when no video frame is available.
- Reconfigure video resources when dimensions/stride/format change.
- Reconfigure the audio stream when rate/channel/device properties change.

Acceptance criteria:

- `mxl-player` with no feed opens a usable GUI.
- The user can move among idle, audio-only, video-only, detached dual-feed, and
  synchronized playback without restarting.
- Output resources do not depend on the initial CLI topology.

### Stage 10 — GUI and CLI integration

Work:

- Convert GUI actions into controller commands.
- Add fields/actions for both UUIDs, reconnect, remove, retry, and sync toggle.
- Add independent start/stop controls and a stop-all action.
- Display desired/actual sync state and independent/group retry status.
- Add CLI options for retry limits and initial synchronization preference.

Acceptance criteria:

- CLI values only establish initial desired state.
- Every important runtime operation is available in the GUI.
- GUI never reads or closes source pointers directly.

### Stage 11 — Resilience verification

Test at least:

- missing producer on initial connection;
- producer disappearance during playback;
- independent audio-only and video-only failure;
- one failed detached feed while the other remains alive;
- failure of either synchronized member;
- finite exhaustion and infinite retry cancellation;
- UUID replacement during reads and backoff;
- stopping or resuming either feed during reads and retry backoff;
- stopping both feeds while detached and synchronized;
- synchronization toggles during reads and backoff;
- shutdown during blocked reads;
- resolution, rate, and channel-count changes;
- mono, stereo, and multichannel audio;
- long runtime and repeated connect/disconnect cycles;
- `go test -race` for testable non-GPU packages.

Acceptance criteria:

- The specified failure boundaries hold in every test.
- No known goroutine, MXL reader, sync group, SDL stream, or Vulkan resource leak.
- Player state remains understandable after every exhausted retry sequence.

### Stage 12 — Reader backend extension point

Work:

- Introduce small reader factories only after local MXL behaviour is solid.
- Keep playback workers dependent on reader interfaces/factories, not `go-mxl`.
- Implement local MXL as the first backend.
- Add `mxlfabrics` as a second backend without changing controller semantics.

Acceptance criteria:

- Backend selection does not alter retry or synchronization semantics.
- Fake readers can drive all controller and supervisor tests.
- Local MXL remains the reference implementation.

### Stage 13 — Simple playlist (complete)

Work:

- Add ordered playlist entries containing optional video/audio UUIDs,
  synchronization preference, and per-entry duration.
- Apply entry changes atomically through `SessionController`.
- Add manual previous, next, and direct-entry selection.
- Add timed automatic advance and optional looping.
- Add pause/resume for automatic playlist advancement.
- Expose the current entry and remaining time in the GUI.
- Define behaviour for stopped playback, retry exhaustion, invalid entries, and
  manual selection while an automatic timer is active.

Acceptance criteria:

- A playlist may freely mix synchronized pairs, independent pairs, video-only,
  and audio-only entries.
- Manual selection works regardless of the current playback topology.
- Timed entries advance in order and loop without leaking old workers/readers.
- Entry duration follows the documented start rule and uses cancellable timers.
- An old entry's timers and worker events cannot affect a newly selected entry.
- Playlist logic contains no direct MXL, SDL, Vulkan, or renderer operations.

## Suggested command API

Names may change, but commands must have one explicit meaning:

```go
SetVideo(uuid string)
SetAudio(uuid string)
StartVideo()
StartAudio()
StartAll()
StopVideo()
StopAudio()
StopAll()
RemoveVideo()
RemoveAudio()
EnableSync()
DisableSync()
ReconnectVideo()
ReconnectAudio()
RetryNow()
Stop()
```

In synchronized operation, reconnecting either slot means reconnecting the
group. The UI should make that consequence visible.

## Out of scope for the initial player refactoring

- Playlist implementation before the controller, dynamic topology, and output
  lifecycle are stable. The planned playlist work is Stage 13.
- Automatic multi-UUID failure-based failover selection.
- Snapshot, waveform, vectorscope, and advanced diagnostics.
- A large generic media framework.
- `mxlfabrics` implementation before the local player passes resilience tests.
- Unrelated renderer or GUI redesign.

## Working method

For every stage:

1. Agree on the narrow change and expected behaviour.
2. Make the change without pulling later-stage responsibilities forward.
3. Format and run focused tests.
4. Run the relevant manual smoke test.
5. Review ownership, cancellation, and error handling.
6. Record discoveries or revised decisions in this document.
7. Commit the stage separately when accepted.