refactor: fix peer review findings in audio, encoder, renderer, and CLI#59
refactor: fix peer review findings in audio, encoder, renderer, and CLI#59flexiondotorg wants to merge 5 commits into
Conversation
- Audio: implement SlideFFTWindow helper to handle prefill and remove panic on high sample rates; write complete prefill to encoder instead of truncating to one frame - Encoder: fix inverted AVFrameMakeWritable checks in software and hardware paths; tag H.264 stream with full-range BT.601 JFIF colourspace; improve Close() to check flush/drain/trailer/avio results, join errors, prevent spin loops, ensure idempotence - Renderer: use clamped bar height in mirrors; add mirror-symmetry and bar-pixel tests; remove unfalsifiable assertion - CLI: route render-pass failures through RenderComplete.Err; print error after TUI closes, exit non-zero on failure - Module rename: github.com/linuxmatters/jivefire → github.com/linuxmatters/jive-visualiser across all Go imports, issue template, SECURITY.md - Tests: close five leaked C FFT contexts; add prefill-count tests; audit analyser fixtures; TestConversionEquivalence now asserts full-range swscale setup - Docs: credit FFmpeg av_tx RDFT and linear binning in ARCHITECTURE.md; update AGENTS.md with Go 1.26, linear binning, releases rule (no checksum generation); refresh benchmark figures 8.4x → 13.2x; add Harper dictionary terms Signed-off-by: Martin Wimpress <code@wimpress.io>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 34 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: This PR includes broad refactors with non-trivial logic changes across multiple modules, so it should receive human review even if no issues are reported.
Re-trigger cubic
…UI, and YUV packages - Correct 54 drifted comments to present-tense design language - Add rationale to 18 comments explaining non-obvious implementation choices - Remove 18 self-evident or process-narration comments - Fix American spellings to British, replace dashes with punctuation - Add missing doc comments to exported symbols Signed-off-by: Martin Wimpress <code@wimpress.io>
There was a problem hiding this comment.
0 issues found across 21 files (changes from recent commits).
Requires human review: This PR contains significant logic changes across audio processing, encoder, renderer, and CLI including new helper functions, modified frame conversion logic, and error handling paths. Even though the AI found no issues, the scope and risk warrant human review.
Re-trigger cubic
- Remove redundant wrappers, fields, and comments in audio, CLI,
encoder, and YUV code.
- Reuse shared helpers in the CLI, renderer, and encoder paths.
- Keep tests aligned with the shared code paths.
Signed-off-by: Martin Wimpress <code@wimpress.io>
There was a problem hiding this comment.
0 issues found across 10 files (changes from recent commits).
Requires human review: Changes affect encoder hardware/software paths, audio prefill logic, CLI error handling, and a module rename; these are high-impact and require human review.
Re-trigger cubic
- Move hardware encoder metadata into a registry and tighten CLI parsing. - Extract Pass 2 runner helpers for setup, asset loading, and progress updates. - Split encoder audio and video setup, and separate UI state from view helpers. - Add coverage for the refactor and refresh the architecture notes. Signed-off-by: Martin Wimpress <code@wimpress.io>
There was a problem hiding this comment.
3 issues found across 16 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
- Copy bar heights when building render progress messages so the UI
reads its own data.
- Check FFmpeg return values when making audio frames writable, sending
frames, and draining the encoder.
- Add a regression test for bar height copying.
Signed-off-by: Martin Wimpress <code@wimpress.io>
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Requires human review: Module rename and refactoring of audio, encoder, renderer, and CLI logic; high impact and risk.
Re-trigger cubic
1 participant
encoder instead of truncating to one frame
colourspace; improve Close() to check flush/drain/trailer/avio
results, join errors, prevent spin loops, ensure idempotence
template, SECURITY.md
asserts full-range swscale setup
rule (no checksum generation); refresh benchmark figures 8.4x → 13.2x;
add Harper dictionary terms