refactor: minor tweaks and improvements#49
Conversation
- Remove self-evident inline comments (e.g., "Create test RGB data") - Strip dev-process metadata and hardcoded line references - Rewrite past-tense regression guards as present-tense design language - Improve doc comments on exported symbols for clarity and accuracy - Net: 189 lines removed, 93 inserted; no code behaviour change Signed-off-by: Martin Wimpress <code@wimpress.io>
- Replace hand-rolled binary unpacking and stereo averaging with libswresample - Support any sample format, channel layout, and count via correct downmix - Eliminate unsafe.Slice (4 → 1 usage) and encoding/binary dependency - Remove ffmpeg_decoder_test.go (functionality subsumed by integration tests) All tests pass; behaviour verified identical. docs: add FFmpeg 8.1.1 A/V processing audit Audit of hand-rolled audio and video processing against FFmpeg 8.1.1 APIs, documenting the rationale for libswresample migration and identifying optimisation opportunities in the encoding pipeline. Signed-off-by: Martin Wimpress <code@wimpress.io>
- Render time and ETA in MM:SS or H:MM:SS clock format (eliminates sub-second flicker) - Use multiplication sign (×) for speed multiplier instead of ASCII x - Use square-kHz glyph (㎑) for audio codec frequency instead of text kHz - Use decibel glyph (㏈) for peak/RMS/dynamic range instead of text dB - Add formatClock helper for whole-second time formatting - Update test assertions to match new glyph rendering 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 18 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 a significant refactor of the audio streaming reader (internal/audio/reader.go) that replaces sample conversion logic with swresample and deletes the corresponding test file (ffmpeg_decoder_test.go), changes that carry high risk for core audio processing and warrant human review.
Re-trigger cubic
- Allocate a private preview buffer only when preview is enabled - Copy frame pixels on each throttled UI send instead of sharing live buffer - Prevent render loop from mutating a buffer the UI goroutine reads Signed-off-by: Martin Wimpress <code@wimpress.io>
Signed-off-by: Martin Wimpress <code@wimpress.io>
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
- Previous single-buffer approach still races: UI goroutine holds pointer from previous send while render loop overwrites that buffer on next tick - Alternate between two buffers so producer always fills the one the UI is not reading, eliminating the race with zero additional per-frame cost Signed-off-by: Martin Wimpress <code@wimpress.io>
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: The PR replaces the audio decoding pipeline with libswresample and removes the corresponding decoder test file, which is a high-risk change affecting core audio processing logic and data integrity.
Re-trigger cubic
1 participant
No description provided.