From cc3c21bf1acab28edb02cc05b71e7af77795d3a2 Mon Sep 17 00:00:00 2001 From: Martin Wimpress Date: Fri, 20 Mar 2026 18:59:50 +0000 Subject: [PATCH] refactor: code smells remediation across audio and ui packages - Remove FrameAnalysis.BarMagnitudes and Profile.Frames; fields were populated but never read, wasting ~27MB heap for 30-min audio; progress callback bar display preserved via a reusable buffer - Extract decodeS16/decodeS32/decodeF32/sampleDecoder/unsafeByteSlice helpers from extractSamples, reducing it from ~189 to ~58 lines; add unit tests with edge cases for each decoder - Move fire colour palette from internal/cli to internal/theme as the single source of truth; remove re-aliased vars in ui/progress.go, eliminating ui's dependency on internal/cli - Note in AGENTS.md that convertRGBAToYUV/convertRGBAToNV12 duplication is intentional for hot-path performance Signed-off-by: Martin Wimpress --- AGENTS.md | 1 + internal/audio/analyzer.go | 37 ++-- internal/audio/analyzer_test.go | 82 +------- internal/audio/ffmpeg_decoder.go | 222 +++++++-------------- internal/audio/ffmpeg_decoder_test.go | 111 +++++++++++ internal/cli/help.go | 13 +- internal/{cli/colors.go => theme/theme.go} | 6 +- internal/ui/progress.go | 44 ++-- 8 files changed, 224 insertions(+), 292 deletions(-) create mode 100644 internal/audio/ffmpeg_decoder_test.go rename internal/{cli/colors.go => theme/theme.go} (90%) diff --git a/AGENTS.md b/AGENTS.md index d7cbdc4..8085f77 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -43,6 +43,7 @@ ## Performance Patterns - RGB→YUV conversion in `encoder/frame.go` parallelised across CPU cores (8.4× faster than swscale) +- `convertRGBAToYUV` (YUV420P) and `convertRGBAToNV12` (NV12) are intentionally kept as separate functions despite near-identical structure — the hot-path duplication avoids a callback/interface indirection that would hurt throughput; do not refactor into a shared helper - Frame rendering uses symmetric mirroring (draw 1/4 pixels, mirror 3×) - Pre-computed intensity/colour tables in `renderer/frame.go` - Bubbletea UI uses non-blocking goroutine channels diff --git a/internal/audio/analyzer.go b/internal/audio/analyzer.go index 54789af..13c2eef 100644 --- a/internal/audio/analyzer.go +++ b/internal/audio/analyzer.go @@ -18,9 +18,6 @@ type FrameAnalysis struct { // RMS level of audio chunk RMSLevel float64 - - // Average per-bar magnitudes (for future use) - BarMagnitudes [config.NumBars]float64 } // Profile holds complete audio analysis results. @@ -28,9 +25,6 @@ type Profile struct { // Total number of frames in audio NumFrames int - // Per-frame analysis data - Frames []FrameAnalysis - // Global statistics GlobalPeak float64 // Highest peak magnitude across all frames GlobalRMS float64 // Average RMS across all frames @@ -57,8 +51,7 @@ func AnalyzeAudio(filename string, progressCb ProgressCallback) (*Profile, error defer reader.Close() profile := &Profile{ - NumFrames: 0, // Will be set after we count actual samples - Frames: make([]FrameAnalysis, 0), // Will grow as we read + NumFrames: 0, // Will be set after we count actual samples SampleRate: reader.SampleRate(), Duration: 0, // Will be calculated from actual sample count } @@ -86,6 +79,9 @@ func AnalyzeAudio(filename string, progressCb ProgressCallback) (*Profile, error return nil, fmt.Errorf("no audio data in file") } + // Pre-allocate bar magnitudes buffer for progress callbacks + barHeights := make([]float64, config.NumBars) + startTime := time.Now() frameNum := 0 @@ -95,8 +91,7 @@ func AnalyzeAudio(filename string, progressCb ProgressCallback) (*Profile, error coeffs := processor.ProcessChunk(fftBuffer) // Analyze frequency bins - analysis := analyzeFrame(coeffs, fftBuffer) - profile.Frames = append(profile.Frames, analysis) + analysis := analyzeFrame(coeffs, fftBuffer, barHeights) // Track global statistics if analysis.PeakMagnitude > maxPeak { @@ -108,12 +103,6 @@ func AnalyzeAudio(filename string, progressCb ProgressCallback) (*Profile, error // Send progress update via callback (throttle to every 3 frames for performance) if progressCb != nil && frameNum%3 == 0 { - // Convert bar magnitudes to slice for progress update - barHeights := make([]float64, config.NumBars) - for i := range config.NumBars { - barHeights[i] = analysis.BarMagnitudes[i] - } - elapsed := time.Since(startTime) // No total frames estimate available during first pass progressCb(frameNum, 0, analysis.RMSLevel, analysis.PeakMagnitude, barHeights, elapsed) @@ -125,10 +114,6 @@ func AnalyzeAudio(filename string, progressCb ProgressCallback) (*Profile, error if errors.Is(err, io.EOF) { // Send final progress update if progressCb != nil { - barHeights := make([]float64, config.NumBars) - for i := range config.NumBars { - barHeights[i] = analysis.BarMagnitudes[i] - } elapsed := time.Since(startTime) progressCb(frameNum, frameNum, analysis.RMSLevel, analysis.PeakMagnitude, barHeights, elapsed) } @@ -176,8 +161,10 @@ func AnalyzeAudio(filename string, progressCb ProgressCallback) (*Profile, error return profile, nil } -// analyzeFrame extracts statistics from FFT coefficients and audio chunk -func analyzeFrame(coeffs []complex128, audioChunk []float64) FrameAnalysis { +// analyzeFrame extracts statistics from FFT coefficients and audio chunk. +// barMagnitudes is an optional buffer that receives per-bar average magnitudes +// for progress display; pass nil when bar magnitudes are not needed. +func analyzeFrame(coeffs []complex128, audioChunk []float64, barMagnitudes []float64) FrameAnalysis { analysis := FrameAnalysis{} // Calculate RMS of audio chunk @@ -187,7 +174,7 @@ func analyzeFrame(coeffs []complex128, audioChunk []float64) FrameAnalysis { } analysis.RMSLevel = math.Sqrt(sumSquares / float64(len(audioChunk))) - // Analyze frequency bins (same logic as BinFFT) + // Analyse frequency bins (same logic as BinFFT) // Use full spectrum up to Nyquist frequency for complete frequency coverage halfSize := len(coeffs) / 2 maxFreqBin := halfSize @@ -205,7 +192,9 @@ func analyzeFrame(coeffs []complex128, audioChunk []float64) FrameAnalysis { } avgMagnitude := sum / float64(binsPerBar) - analysis.BarMagnitudes[bar] = avgMagnitude + if barMagnitudes != nil { + barMagnitudes[bar] = avgMagnitude + } // Track peak if avgMagnitude > analysis.PeakMagnitude { diff --git a/internal/audio/analyzer_test.go b/internal/audio/analyzer_test.go index 16186ae..c560b68 100644 --- a/internal/audio/analyzer_test.go +++ b/internal/audio/analyzer_test.go @@ -50,11 +50,6 @@ func TestAnalyzeAudio(t *testing.T) { t.Errorf("Expected positive OptimalBaseScale, got %.6f", profile.OptimalBaseScale) } - // Validate frame analysis array - if len(profile.Frames) != profile.NumFrames { - t.Errorf("Frame count mismatch: expected %d, got %d", profile.NumFrames, len(profile.Frames)) - } - t.Logf("Analysis complete:") t.Logf(" Duration: %.1f seconds", profile.Duration) t.Logf(" Frames: %d", profile.NumFrames) @@ -71,32 +66,6 @@ func TestAnalyzeAudioInvalidFile(t *testing.T) { } } -func TestAnalyzeFrameStatistics(t *testing.T) { - profile := mustAnalyze(t) - - // Check first few frames have valid statistics - for i := 0; i < 10 && i < len(profile.Frames); i++ { - frame := profile.Frames[i] - - if frame.PeakMagnitude < 0 { - t.Errorf("Frame %d: negative PeakMagnitude: %.6f", i, frame.PeakMagnitude) - } - - if frame.RMSLevel < 0 { - t.Errorf("Frame %d: negative RMSLevel: %.6f", i, frame.RMSLevel) - } - - // Check bar magnitudes - for bar := range config.NumBars { - if frame.BarMagnitudes[bar] < 0 { - t.Errorf("Frame %d, Bar %d: negative magnitude: %.6f", i, bar, frame.BarMagnitudes[bar]) - } - } - } - - t.Logf("Frame statistics validated for %d frames", profile.NumFrames) -} - func TestOptimalBaseScaleCalculation(t *testing.T) { profile := mustAnalyze(t) @@ -119,55 +88,6 @@ func TestOptimalBaseScaleCalculation(t *testing.T) { profile.GlobalPeak, profile.OptimalBaseScale, testValue) } -func TestGlobalPeakIsMaximum(t *testing.T) { - profile := mustAnalyze(t) - - // GlobalPeak should be >= all frame peaks - for i, frame := range profile.Frames { - if frame.PeakMagnitude > profile.GlobalPeak { - t.Errorf("Frame %d peak (%.6f) exceeds GlobalPeak (%.6f)", - i, frame.PeakMagnitude, profile.GlobalPeak) - } - } - - // Find the actual maximum to verify it matches - var maxFound float64 - var maxFrameIdx int - for i, frame := range profile.Frames { - if frame.PeakMagnitude > maxFound { - maxFound = frame.PeakMagnitude - maxFrameIdx = i - } - } - - if maxFound != profile.GlobalPeak { - t.Errorf("GlobalPeak (%.6f) doesn't match actual maximum (%.6f) at frame %d", - profile.GlobalPeak, maxFound, maxFrameIdx) - } - - t.Logf("GlobalPeak correctly represents maximum: %.6f at frame %d", profile.GlobalPeak, maxFrameIdx) -} - -func TestGlobalRMSIsAverage(t *testing.T) { - profile := mustAnalyze(t) - - // Calculate average RMS manually - var sumRMS float64 - for _, frame := range profile.Frames { - sumRMS += frame.RMSLevel - } - expectedRMS := sumRMS / float64(len(profile.Frames)) - - // Allow small floating point error - diff := profile.GlobalRMS - expectedRMS - if diff < -0.000001 || diff > 0.000001 { - t.Errorf("GlobalRMS (%.6f) doesn't match calculated average (%.6f)", - profile.GlobalRMS, expectedRMS) - } - - t.Logf("GlobalRMS correctly calculated as average: %.6f", profile.GlobalRMS) -} - func TestDynamicRangeCalculation(t *testing.T) { profile := mustAnalyze(t) @@ -197,7 +117,7 @@ func TestAnalyzeFrameDirectly(t *testing.T) { coeffs := processor.ProcessChunk(testSamples) // Analyze - analysis := analyzeFrame(coeffs, testSamples) + analysis := analyzeFrame(coeffs, testSamples, nil) // Validate results if analysis.PeakMagnitude <= 0 { diff --git a/internal/audio/ffmpeg_decoder.go b/internal/audio/ffmpeg_decoder.go index e22c884..7415fe6 100644 --- a/internal/audio/ffmpeg_decoder.go +++ b/internal/audio/ffmpeg_decoder.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "math" + "unsafe" ffmpeg "github.com/linuxmatters/ffmpeg-statigo" ) @@ -186,6 +187,52 @@ func (d *FFmpegDecoder) ReadChunk(numSamples int) ([]float64, error) { return result, nil } +// unsafeByteSlice reinterprets a pointer as a byte slice of the given length. +func unsafeByteSlice(ptr unsafe.Pointer, length int) []byte { + return (*[1 << 30]byte)(ptr)[:length:length] +} + +// decodeS16 decodes a signed 16-bit little-endian sample at byte offset i, +// normalised to [-1.0, 1.0]. +func decodeS16(buf []byte, i int) float64 { + val := int16(buf[i]) | int16(buf[i+1])<<8 + return float64(val) / 32768.0 +} + +// decodeS32 decodes a signed 32-bit little-endian sample at byte offset i, +// normalised to [-1.0, 1.0]. +func decodeS32(buf []byte, i int) float64 { + val := int32(buf[i]) | + int32(buf[i+1])<<8 | + int32(buf[i+2])<<16 | + int32(buf[i+3])<<24 + return float64(val) / 2147483648.0 +} + +// decodeF32 decodes a 32-bit IEEE 754 float sample at byte offset i. +func decodeF32(buf []byte, i int) float64 { + bits := uint32(buf[i]) | + uint32(buf[i+1])<<8 | + uint32(buf[i+2])<<16 | + uint32(buf[i+3])<<24 + return float64(math.Float32frombits(bits)) +} + +// sampleDecoder selects the appropriate decode function and bytes-per-sample +// for the given sample format code. +func sampleDecoder(sampleFormat int) (func([]byte, int) float64, int, error) { + switch sampleFormat { + case 1, 6: // S16, S16P + return decodeS16, 2, nil + case 2, 7: // S32, S32P + return decodeS32, 4, nil + case 3, 8: // Flt, Fltp + return decodeF32, 4, nil + default: + return nil, 0, fmt.Errorf("unsupported sample format: %d", sampleFormat) + } +} + // extractSamples extracts float64 samples from the current frame. // Stereo is automatically downmixed to mono. func (d *FFmpegDecoder) extractSamples() ([]float64, error) { @@ -193,6 +240,11 @@ func (d *FFmpegDecoder) extractSamples() ([]float64, error) { sampleFormat := d.frame.Format() channels := d.channels + decode, bps, err := sampleDecoder(sampleFormat) + if err != nil { + return nil, err + } + samples := make([]float64, nbSamples) // Determine if format is planar (planar formats start at 5) @@ -200,179 +252,43 @@ func (d *FFmpegDecoder) extractSamples() ([]float64, error) { switch { case isPlanar && channels == 2: - // Planar stereo - separate buffers for left and right leftPtr := d.frame.Data().Get(0) rightPtr := d.frame.Data().Get(1) if leftPtr == nil || rightPtr == nil { return nil, fmt.Errorf("missing channel data") } - - switch sampleFormat { - case 6: // AVSampleFmtS16P - planar 16-bit signed - leftSlice := (*[1 << 30]byte)(leftPtr)[: nbSamples*2 : nbSamples*2] - rightSlice := (*[1 << 30]byte)(rightPtr)[: nbSamples*2 : nbSamples*2] - for i := range nbSamples { - leftVal := int16(leftSlice[i*2]) | int16(leftSlice[i*2+1])<<8 - rightVal := int16(rightSlice[i*2]) | int16(rightSlice[i*2+1])<<8 - samples[i] = (float64(leftVal) + float64(rightVal)) / (2 * 32768.0) - } - - case 7: // AVSampleFmtS32P - planar 32-bit signed (FLAC) - leftSlice := (*[1 << 30]byte)(leftPtr)[: nbSamples*4 : nbSamples*4] - rightSlice := (*[1 << 30]byte)(rightPtr)[: nbSamples*4 : nbSamples*4] - for i := range nbSamples { - leftVal := int32(leftSlice[i*4]) | - int32(leftSlice[i*4+1])<<8 | - int32(leftSlice[i*4+2])<<16 | - int32(leftSlice[i*4+3])<<24 - rightVal := int32(rightSlice[i*4]) | - int32(rightSlice[i*4+1])<<8 | - int32(rightSlice[i*4+2])<<16 | - int32(rightSlice[i*4+3])<<24 - samples[i] = (float64(leftVal) + float64(rightVal)) / (2 * 2147483648.0) - } - - case 8: // AVSampleFmtFltp - planar float - leftSlice := (*[1 << 30]byte)(leftPtr)[: nbSamples*4 : nbSamples*4] - rightSlice := (*[1 << 30]byte)(rightPtr)[: nbSamples*4 : nbSamples*4] - for i := range nbSamples { - leftBits := uint32(leftSlice[i*4]) | - uint32(leftSlice[i*4+1])<<8 | - uint32(leftSlice[i*4+2])<<16 | - uint32(leftSlice[i*4+3])<<24 - leftFloat := math.Float32frombits(leftBits) - - rightBits := uint32(rightSlice[i*4]) | - uint32(rightSlice[i*4+1])<<8 | - uint32(rightSlice[i*4+2])<<16 | - uint32(rightSlice[i*4+3])<<24 - rightFloat := math.Float32frombits(rightBits) - - samples[i] = float64(leftFloat+rightFloat) / 2 - } - - default: - return nil, fmt.Errorf("unsupported planar sample format: %d", sampleFormat) + leftBuf := unsafeByteSlice(leftPtr, nbSamples*bps) + rightBuf := unsafeByteSlice(rightPtr, nbSamples*bps) + for i := range nbSamples { + samples[i] = (decode(leftBuf, i*bps) + decode(rightBuf, i*bps)) / 2 } + case isPlanar && channels == 1: - // Planar mono - just one buffer dataPtr := d.frame.Data().Get(0) if dataPtr == nil { return nil, fmt.Errorf("no data in frame") } - - switch sampleFormat { - case 6: // AVSampleFmtS16P - dataSlice := (*[1 << 30]byte)(dataPtr)[: nbSamples*2 : nbSamples*2] - for i := range nbSamples { - val := int16(dataSlice[i*2]) | int16(dataSlice[i*2+1])<<8 - samples[i] = float64(val) / 32768.0 - } - - case 7: // AVSampleFmtS32P - dataSlice := (*[1 << 30]byte)(dataPtr)[: nbSamples*4 : nbSamples*4] - for i := range nbSamples { - val := int32(dataSlice[i*4]) | - int32(dataSlice[i*4+1])<<8 | - int32(dataSlice[i*4+2])<<16 | - int32(dataSlice[i*4+3])<<24 - samples[i] = float64(val) / 2147483648.0 - } - - case 8: // AVSampleFmtFltp - dataSlice := (*[1 << 30]byte)(dataPtr)[: nbSamples*4 : nbSamples*4] - for i := range nbSamples { - bits := uint32(dataSlice[i*4]) | - uint32(dataSlice[i*4+1])<<8 | - uint32(dataSlice[i*4+2])<<16 | - uint32(dataSlice[i*4+3])<<24 - samples[i] = float64(math.Float32frombits(bits)) - } - - default: - return nil, fmt.Errorf("unsupported planar mono format: %d", sampleFormat) + buf := unsafeByteSlice(dataPtr, nbSamples*bps) + for i := range nbSamples { + samples[i] = decode(buf, i*bps) } + default: // Interleaved formats dataPtr := d.frame.Data().Get(0) if dataPtr == nil { return nil, fmt.Errorf("no data in frame") } - - switch sampleFormat { - case 1: // AVSampleFmtS16 - interleaved 16-bit signed - if channels == 1 { - dataSlice := (*[1 << 30]byte)(dataPtr)[: nbSamples*2 : nbSamples*2] - for i := range nbSamples { - val := int16(dataSlice[i*2]) | int16(dataSlice[i*2+1])<<8 - samples[i] = float64(val) / 32768.0 - } - } else { - // Stereo interleaved: L R L R ... - dataSlice := (*[1 << 30]byte)(dataPtr)[: nbSamples*4 : nbSamples*4] - for i := range nbSamples { - leftVal := int16(dataSlice[i*4]) | int16(dataSlice[i*4+1])<<8 - rightVal := int16(dataSlice[i*4+2]) | int16(dataSlice[i*4+3])<<8 - samples[i] = (float64(leftVal) + float64(rightVal)) / (2 * 32768.0) - } - } - - case 2: // AVSampleFmtS32 - interleaved 32-bit signed (FLAC) - if channels == 1 { - dataSlice := (*[1 << 30]byte)(dataPtr)[: nbSamples*4 : nbSamples*4] - for i := range nbSamples { - val := int32(dataSlice[i*4]) | - int32(dataSlice[i*4+1])<<8 | - int32(dataSlice[i*4+2])<<16 | - int32(dataSlice[i*4+3])<<24 - samples[i] = float64(val) / 2147483648.0 - } - } else { - // Stereo interleaved - dataSlice := (*[1 << 30]byte)(dataPtr)[: nbSamples*8 : nbSamples*8] - for i := range nbSamples { - leftVal := int32(dataSlice[i*8]) | - int32(dataSlice[i*8+1])<<8 | - int32(dataSlice[i*8+2])<<16 | - int32(dataSlice[i*8+3])<<24 - rightVal := int32(dataSlice[i*8+4]) | - int32(dataSlice[i*8+5])<<8 | - int32(dataSlice[i*8+6])<<16 | - int32(dataSlice[i*8+7])<<24 - samples[i] = (float64(leftVal) + float64(rightVal)) / (2 * 2147483648.0) - } + stride := bps * channels + buf := unsafeByteSlice(dataPtr, nbSamples*stride) + if channels == 1 { + for i := range nbSamples { + samples[i] = decode(buf, i*stride) } - - case 3: // AVSampleFmtFlt - interleaved float - if channels == 1 { - dataSlice := (*[1 << 30]byte)(dataPtr)[: nbSamples*4 : nbSamples*4] - for i := range nbSamples { - bits := uint32(dataSlice[i*4]) | - uint32(dataSlice[i*4+1])<<8 | - uint32(dataSlice[i*4+2])<<16 | - uint32(dataSlice[i*4+3])<<24 - samples[i] = float64(math.Float32frombits(bits)) - } - } else { - // Stereo interleaved - dataSlice := (*[1 << 30]byte)(dataPtr)[: nbSamples*8 : nbSamples*8] - for i := range nbSamples { - leftBits := uint32(dataSlice[i*8]) | - uint32(dataSlice[i*8+1])<<8 | - uint32(dataSlice[i*8+2])<<16 | - uint32(dataSlice[i*8+3])<<24 - rightBits := uint32(dataSlice[i*8+4]) | - uint32(dataSlice[i*8+5])<<8 | - uint32(dataSlice[i*8+6])<<16 | - uint32(dataSlice[i*8+7])<<24 - leftFloat := math.Float32frombits(leftBits) - rightFloat := math.Float32frombits(rightBits) - samples[i] = float64(leftFloat+rightFloat) / 2 - } + } else { + for i := range nbSamples { + samples[i] = (decode(buf, i*stride) + decode(buf, i*stride+bps)) / 2 } - - default: - return nil, fmt.Errorf("unsupported interleaved sample format: %d", sampleFormat) } } diff --git a/internal/audio/ffmpeg_decoder_test.go b/internal/audio/ffmpeg_decoder_test.go new file mode 100644 index 0000000..2224885 --- /dev/null +++ b/internal/audio/ffmpeg_decoder_test.go @@ -0,0 +1,111 @@ +package audio + +import ( + "math" + "testing" +) + +func TestDecodeS16(t *testing.T) { + tests := []struct { + name string + buf []byte + want float64 + }{ + {"zero", []byte{0x00, 0x00}, 0.0}, + {"max positive", []byte{0xFF, 0x7F}, float64(math.MaxInt16) / 32768.0}, + {"max negative", []byte{0x00, 0x80}, -1.0}, + {"one", []byte{0x01, 0x00}, 1.0 / 32768.0}, + {"minus one", []byte{0xFF, 0xFF}, -1.0 / 32768.0}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := decodeS16(tt.buf, 0) + if math.Abs(got-tt.want) > 1e-10 { + t.Errorf("decodeS16() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestDecodeS16WithOffset(t *testing.T) { + // Sample at offset 2: little-endian 0x0100 = 256 + buf := []byte{0xAA, 0xBB, 0x00, 0x01} + got := decodeS16(buf, 2) + want := float64(256) / 32768.0 + if math.Abs(got-want) > 1e-10 { + t.Errorf("decodeS16(offset=2) = %v, want %v", got, want) + } +} + +func TestDecodeS32(t *testing.T) { + tests := []struct { + name string + buf []byte + want float64 + }{ + {"zero", []byte{0x00, 0x00, 0x00, 0x00}, 0.0}, + {"max positive", []byte{0xFF, 0xFF, 0xFF, 0x7F}, float64(math.MaxInt32) / 2147483648.0}, + {"max negative", []byte{0x00, 0x00, 0x00, 0x80}, -1.0}, + {"one", []byte{0x01, 0x00, 0x00, 0x00}, 1.0 / 2147483648.0}, + {"minus one", []byte{0xFF, 0xFF, 0xFF, 0xFF}, -1.0 / 2147483648.0}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := decodeS32(tt.buf, 0) + if math.Abs(got-tt.want) > 1e-10 { + t.Errorf("decodeS32() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestDecodeF32(t *testing.T) { + tests := []struct { + name string + buf []byte + want float64 + }{ + {"zero", []byte{0x00, 0x00, 0x00, 0x00}, 0.0}, + {"one", []byte{0x00, 0x00, 0x80, 0x3F}, 1.0}, + {"minus one", []byte{0x00, 0x00, 0x80, 0xBF}, -1.0}, + {"half", []byte{0x00, 0x00, 0x00, 0x3F}, 0.5}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := decodeF32(tt.buf, 0) + if math.Abs(got-tt.want) > 1e-6 { + t.Errorf("decodeF32() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestDecodeF32WithOffset(t *testing.T) { + // 1.0f at offset 4 + buf := []byte{0xDE, 0xAD, 0xBE, 0xEF, 0x00, 0x00, 0x80, 0x3F} + got := decodeF32(buf, 4) + if math.Abs(got-1.0) > 1e-6 { + t.Errorf("decodeF32(offset=4) = %v, want 1.0", got) + } +} + +func TestSampleDecoder(t *testing.T) { + validFormats := []int{1, 2, 3, 6, 7, 8} + for _, fmt := range validFormats { + fn, bps, err := sampleDecoder(fmt) + if err != nil { + t.Errorf("sampleDecoder(%d) returned error: %v", fmt, err) + } + if fn == nil { + t.Errorf("sampleDecoder(%d) returned nil function", fmt) + } + if bps == 0 { + t.Errorf("sampleDecoder(%d) returned 0 bps", fmt) + } + } + + _, _, err := sampleDecoder(99) + if err == nil { + t.Error("sampleDecoder(99) should return error") + } +} diff --git a/internal/cli/help.go b/internal/cli/help.go index 2d5109c..0289206 100644 --- a/internal/cli/help.go +++ b/internal/cli/help.go @@ -6,35 +6,36 @@ import ( "github.com/alecthomas/kong" "github.com/charmbracelet/lipgloss" + "github.com/linuxmatters/jivefire/internal/theme" ) // Custom help styles - fire theme var ( helpTitleStyle = lipgloss.NewStyle(). Bold(true). - Foreground(FireYellow). + Foreground(theme.FireYellow). MarginBottom(1) helpDescStyle = lipgloss.NewStyle(). - Foreground(FireOrange). + Foreground(theme.FireOrange). Italic(true). MarginBottom(1) helpSectionStyle = lipgloss.NewStyle(). Bold(true). - Foreground(FireOrange). + Foreground(theme.FireOrange). MarginTop(1) helpFlagStyle = lipgloss.NewStyle(). - Foreground(FireYellow). + Foreground(theme.FireYellow). Bold(true) helpArgStyle = lipgloss.NewStyle(). - Foreground(FireRed). + Foreground(theme.FireRed). Bold(true) helpDefaultStyle = lipgloss.NewStyle(). - Foreground(WarmGray). + Foreground(theme.WarmGray). Italic(true) ) diff --git a/internal/cli/colors.go b/internal/theme/theme.go similarity index 90% rename from internal/cli/colors.go rename to internal/theme/theme.go index b8ad4a4..de988a8 100644 --- a/internal/cli/colors.go +++ b/internal/theme/theme.go @@ -1,9 +1,9 @@ -package cli +package theme import "github.com/charmbracelet/lipgloss" -// Fire colour palette 🔥 -// Shared fire theme colours for consistent branding across CLI and TUI +// Fire colour palette +// Shared fire theme colours for consistent branding across CLI and TUI. var ( // Core fire colours (dark to bright) FireYellow = lipgloss.Color("#FFD700") // Bright yellow diff --git a/internal/ui/progress.go b/internal/ui/progress.go index 4e7562b..26b9c2a 100644 --- a/internal/ui/progress.go +++ b/internal/ui/progress.go @@ -10,16 +10,7 @@ import ( "github.com/charmbracelet/bubbles/progress" tea "github.com/charmbracelet/bubbletea" "github.com/charmbracelet/lipgloss" - "github.com/linuxmatters/jivefire/internal/cli" -) - -// Fire colour palette - import shared colours from cli package -var ( - fireYellow = cli.FireYellow - fireOrange = cli.FireOrange - fireRed = cli.FireRed - fireCrimson = cli.FireCrimson - warmGray = cli.WarmGray + "github.com/linuxmatters/jivefire/internal/theme" ) // Phase represents the current processing phase @@ -129,14 +120,14 @@ type Model struct { func NewModel(noPreview bool) *Model { // Fire gradient: deep red → orange → yellow p := progress.New( - progress.WithGradient(string(fireCrimson), string(fireYellow)), + progress.WithGradient(string(theme.FireCrimson), string(theme.FireYellow)), progress.WithWidth(40), progress.WithoutPercentage(), ) // Smaller progress bar for summary performance charts summaryBar := progress.New( - progress.WithGradient(string(fireCrimson), string(fireYellow)), + progress.WithGradient(string(theme.FireCrimson), string(theme.FireYellow)), progress.WithWidth(30), progress.WithoutPercentage(), ) @@ -238,12 +229,12 @@ func (m *Model) renderFinalProgress() string { // Title title := lipgloss.NewStyle(). Bold(true). - Foreground(fireYellow). + Foreground(theme.FireYellow). Render("Jivefire 🔥") s.WriteString(title) s.WriteString("\n") - s.WriteString(lipgloss.NewStyle().Foreground(fireOrange).Render("Pass 2: Rendering & Encoding")) + s.WriteString(lipgloss.NewStyle().Foreground(theme.FireOrange).Render("Pass 2: Rendering & Encoding")) s.WriteString("\n\n") // Progress bar at 100% @@ -284,7 +275,7 @@ func (m *Model) renderFinalProgress() string { return lipgloss.NewStyle(). BorderStyle(lipgloss.RoundedBorder()). - BorderForeground(fireOrange). + BorderForeground(theme.FireOrange). Padding(1, 2). Render(s.String()) } @@ -295,7 +286,7 @@ func (m *Model) renderProgress() string { // Title title := lipgloss.NewStyle(). Bold(true). - Foreground(fireYellow). + Foreground(theme.FireYellow). Render("Jivefire 🔥") s.WriteString(title) @@ -308,7 +299,7 @@ func (m *Model) renderProgress() string { } else { phaseLabel = "Pass 2: Rendering & Encoding" } - s.WriteString(lipgloss.NewStyle().Foreground(fireOrange).Render(phaseLabel)) + s.WriteString(lipgloss.NewStyle().Foreground(theme.FireOrange).Render(phaseLabel)) s.WriteString("\n\n") // Progress bar and timing @@ -330,7 +321,7 @@ func (m *Model) renderProgress() string { return lipgloss.NewStyle(). BorderStyle(lipgloss.RoundedBorder()). - BorderForeground(fireRed). + BorderForeground(theme.FireRed). Padding(1, 2). Render(s.String()) } @@ -440,7 +431,7 @@ func (m *Model) renderAudioProfile(s *strings.Builder) { } func (m *Model) renderSpectrumAndStats(s *strings.Builder) { - s.WriteString(lipgloss.NewStyle().Foreground(fireOrange).Render("Live Visualisation:")) + s.WriteString(lipgloss.NewStyle().Foreground(theme.FireOrange).Render("Live Visualisation:")) s.WriteString("\n") // Use full width for spectrum (64 bars matches the actual bar count) @@ -452,7 +443,7 @@ func (m *Model) renderSpectrumAndStats(s *strings.Builder) { var rightCol strings.Builder if m.renderState.FileSize > 0 || m.renderState.VideoCodec != "" || m.renderState.AudioCodec != "" { - labelStyle := lipgloss.NewStyle().Foreground(warmGray) + labelStyle := lipgloss.NewStyle().Foreground(theme.WarmGray) valueStyle := lipgloss.NewStyle().Bold(true) rightCol.WriteString(labelStyle.Render("File: ")) @@ -497,7 +488,7 @@ func (m *Model) renderComplete() string { title := lipgloss.NewStyle(). Bold(true). - Foreground(fireYellow). + Foreground(theme.FireYellow). Render("✓ Encoding Complete!") s.WriteString(title) @@ -527,10 +518,10 @@ func (m *Model) renderComplete() string { fmt.Fprintf(&s, "%s%s\n\n", dimLabel.Render("Size: "), formatBytes(m.complete.FileSize)) // Audio Profile section - headerStyle := lipgloss.NewStyle().Bold(true).Foreground(fireOrange) + headerStyle := lipgloss.NewStyle().Bold(true).Foreground(theme.FireOrange) labelStyle := lipgloss.NewStyle().Faint(true) valueStyle := lipgloss.NewStyle() - highlightValueStyle := lipgloss.NewStyle().Foreground(fireOrange) + highlightValueStyle := lipgloss.NewStyle().Foreground(theme.FireOrange) totalMs := m.complete.TotalTime.Milliseconds() if totalMs == 0 { @@ -603,7 +594,7 @@ func (m *Model) renderComplete() string { return lipgloss.NewStyle(). BorderStyle(lipgloss.RoundedBorder()). - BorderForeground(fireOrange). + BorderForeground(theme.FireOrange). Padding(1, 1). Render(s.String()) + "\n" } @@ -649,7 +640,10 @@ func renderSpectrum(barHeights []float64, width int) string { blocks := []rune{'▁', '▂', '▃', '▄', '▅', '▆', '▇', '█'} - // Fire gradient colours from low to high intensity + // Gradient expansion of the four base theme colours (FireCrimson, FireRed, + // FireOrange, FireYellow) into eight steps for smoother spectrum colouring. + // This is not duplication - these intermediate values provide visual fidelity + // that the four base colours alone cannot. fireColors := []lipgloss.Color{ lipgloss.Color("#8B0000"), // Dark red (ember) lipgloss.Color("#B22222"), // Firebrick