itten/go-mxl-pattern-generator
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

video config construction refactoring

Browse Source
This commit is contained in:
Dmitry Sergeev
2026-09-16 21:31:12 +03:00
parent 765aa8d3e1
commit c9bdfec3b0
7 changed files with 406 additions and 199 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ARCHITECTURE_REVIEW.md
+8 -11
View File
@@ -69,23 +69,20 @@ For `audio/float32`, the audio package may use `[]float32` internally and keep b
| # | 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. | **Open** | The JSON is read as an opaque string while `vi` remains zero-valued. `NewWGPUGenerator` therefore receives `0, 0`. Parse and validate the definition before generator creation, or introduce a typed configuration source. |
| 3 | `checkArgs` received `appArgs` by value, so generated UUIDs were discarded. | **Partially fixed** | `checkArgs` now receives `*appArgs`. However, the generated video UUID is still overwritten by a hard-coded UUID in `main`; remove that assignment before release. |
| 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. | **Open** | The function accepts `TYPE_AUDIO`, but all emitted format and media fields remain video-specific. Create separate typed video and audio definition builders. |
| 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 video uses the typed `NewV210Video` constructor. An audio constructor will be added with audio generation. |
| 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. | **Open** | `TestNewTextOverlay`, `TestWGPUMoveSquare`, and `TestWGPUGenerator` still fail. |
| 8 | The Makefile clean target uses `fm -f` instead of `rm -f`. | **Open** | The typo remains in the `clean` target. |
| 8 | The Makefile clean target uses `fm -f` instead of `rm -f`. | **Fixed** | The clean target now uses `rm -f`. |
## Additional implementation priorities
1. Fix the existing tests or update incorrect expectations after confirming the intended color values and overlay positioning.
2. Split video and audio flow-definition types. Avoid an integer `feedType` accepted by a function that has a video-only parameter list.
3. Parse external flow definitions into typed configuration before creating generators. Validate format, media type, dimensions, rate, and channel count.
4. Remove the hard-coded video UUID.
5. Introduce a `run(...) error` orchestration function using shared cancellation and error propagation instead of calling `log.Fatalf` throughout the media loop.
6. Add a CPU audio generator and continuous-flow writer loop using `OpenSamples`, `ChannelFragments`, and `Commit`.
7. After audio is complete, 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.
2. Introduce a `run(...) error` orchestration function using shared cancellation and error propagation instead of calling `log.Fatalf` throughout the media loop.
3. Add a typed audio flow-definition constructor, CPU audio generator, and continuous-flow writer loop using `OpenSamples`, `ChannelFragments`, and `Commit`.
4. After audio is complete, 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
@@ -130,4 +127,4 @@ Package compilation succeeds, but the generator package fails these tests:
## Conclusion
Keep the chosen Go + go-mxl + wgpu stack. Implement audio on the CPU in its own goroutine and give video and audio separate writers, indices, and pacing loops. Coordinate them through a shared context and the common MXL timebase, not through per-frame messages. The immediate blockers are external-flow configuration, audio flow-definition support, and the currently failing tests.
Keep the chosen Go + go-mxl + wgpu stack. Implement audio on the CPU in its own goroutine and give video and audio separate writers, indices, and pacing loops. Coordinate them through a shared context and the common MXL timebase, not through per-frame messages. Typed external-video configuration is now implemented; the next architectural work is extracting the video runner and adding typed audio construction and generation. The three rendering-test failures remain a separate correctness task.