# MXL Pattern Generator Architecture Review

Reviewed on 2026-09-16 against commit `f37f480`.

## Summary

The current stack is a good fit for a test-pattern generator:

- Go is well suited to CLI handling, MXL lifecycle management, timing, and synthetic audio generation.
- `go-mxl` provides the correct data model: discrete grains for video and continuous samples for audio.
- wgpu/WGSL is a reasonable portable GPU abstraction for producing v210 video.
- CPU-rendered text is appropriate because text changes infrequently and font rendering does not need to be implemented in a shader.

The project is still prototype-quality in parts of its orchestration and configuration layers, but flow-definition handling, lifecycle management, audio generation, and the previously failing rendering tests have now been addressed.

## Recommended audio architecture

Video and audio should run in separate goroutines. Each goroutine should own its own MXL writer and timing loop because video and audio have different units and rates:

```text
run
  |-- shared context, signal handling, and error propagation
  |-- video goroutine
  |     OpenGrain -> render -> overlay -> Commit
  `-- audio goroutine
        OpenSamples -> synthesize -> Commit
```

This is also consistent with `go-mxl`: a writer is not safe for concurrent use, so a writer should belong to exactly one goroutine.

The goroutines should not synchronize by sending a notification for every video frame. That would make audio timing depend on GPU latency and Go scheduler jitter. Instead, both loops should use the MXL clock independently:

```go
videoIndex := mxl.CurrentIndex(videoRate)
audioIndex := mxl.CurrentIndex(audioRate)
```

After committing data, each loop advances its own index and uses `mxl.NsUntilIndex` for pacing. Both loops should share a `context.Context`; an error in either flow should cancel the other flow and be returned to the caller.

For audio, start with CPU generation and approximately 10 ms batches (480 samples at 48 kHz). A sine wave is inexpensive to calculate, so GPU audio generation would add complexity without a useful performance benefit.

Audio phase should be derived from the absolute sample index:

```go
phase := 2 * math.Pi * frequency *
    float64(firstSample+i) / sampleRate
```

This preserves continuity across batches and after a timing resynchronization.

Video and audio should expose different interfaces because a video frame and a range of audio samples are fundamentally different units:

```go
type VideoGenerator interface {
    GenerateFrame(dst []byte, frameIndex uint64) error
    Close() error
}

type AudioGenerator interface {
    GenerateSamples(channels [][]byte, firstSample, sampleCount uint64) error
    Close() error
}
```

For `audio/float32`, the audio package may use `[]float32` internally and keep byte encoding at the MXL boundary.

## Issue status

| # | Issue | Status | Current observation |
|---|---|---|---|
| 1 | The validated `--domain` value was ignored in favor of `/dev/shm/mxl`. | **Fixed** | Logging, `mxl.NewInstance`, and reuse messages now use `args.domain`. |
| 2 | An external video flow definition does not populate runtime width, height, FPS, or UUID. | **Fixed** | External definitions are parsed and validated as `flowdef.Video`; `video.Config` supplies their dimensions, rate, and ID to the generator and writer. |
| 3 | `checkArgs` received `appArgs` by value, so generated UUIDs were discarded. | **Fixed** | Argument validation mutates the actual configuration, and the hard-coded video UUID has been removed. |
| 4 | The default pattern was `bars`, which did not exist. | **Fixed** | The default is now `ebu75`, which exists in the pattern registry. |
| 5 | `NewFlowDefJSON(TYPE_AUDIO, ...)` produces a video/v210 definition. | **Fixed** | The generic discriminator-based builder was removed. Video and audio have separate schema types and typed `NewV210Video` and `NewFloat32Audio` constructors. |
| 6 | The wgpu path was described as zero-copy although it performs GPU readback and a CPU copy. | **Deferred — fix after audio** | Every frame is copied from GPU storage to a mapped host buffer and then copied into the MXL payload. The path is synchronous and serial. This may require substantial benchmarking and architectural work, so audio implementation takes priority. Update the documentation now, but defer optimization or redesign until audio is complete. |
| 7 | The test suite had three failures. | **Fixed** | Text positioning tests now match the explicit-position API and reject invalid bounds/alignment. WGPU tests now verify the actual RP 219 geometry and dynamic overlay behavior. |
| 8 | The Makefile clean target uses `fm -f` instead of `rm -f`. | **Fixed** | The clean target now uses `rm -f`. |

## Additional implementation priorities

1. Add integration coverage for simultaneous video/audio startup, cancellation, and error propagation against an MXL instance.
2. Measure end-to-end frame time and missed deadlines at 1080p50/60 and UHD. The current wgpu path may be adequate, but it is neither zero-copy nor asynchronous. Treat GPU readback optimization as a separate, potentially large task.

## Suggested package layout

```text
cmd/mxl-pattern/
    main.go

internal/config/
    config.go
    flow.go

internal/video/
    generator.go
    wgpu.go
    overlay.go
    runner.go

internal/audio/
    generator.go
    sine.go
    runner.go

internal/app/
    run.go
```

The exact directory names are less important than keeping CLI parsing, typed configuration, media generation, and MXL writing as separate responsibilities.

## Verification

The following command was used:

```sh
GOCACHE=/tmp/go-mxl-gen-cache go test ./...
```

All packages pass. `go vet ./...`, the application build, and `git diff --check` also succeed.

## Conclusion

Keep the chosen Go + go-mxl + wgpu stack. Audio belongs on the CPU in its own goroutine, with video and audio using separate writers, indices, and pacing loops. Coordinate them through a shared context and the common MXL timebase, not through per-frame messages. Typed configuration, media runners, and rendering correctness coverage are now in place; GPU readback optimization remains deferred until after audio work.