From b351f14c68d1586f17cf1eb98e146ab9feb2883c Mon Sep 17 00:00:00 2001 From: Martin Wimpress Date: Mon, 16 Mar 2026 11:18:11 +0000 Subject: [PATCH 1/3] refactor(processor): consolidate spectral averaging with finalizeSpectral and writeSpectralTo methods - Add finalizeSpectral() method to calculate and return averaged spectral metrics - Add writeSpectralTo() method to map spectral metrics to BaseMeasurements fields - Replace two 16-line spectral averaging blocks with single method calls in AnalyzeAudio and finalizeOutputMeasurements - Add comprehensive test coverage for spectral finalization Signed-off-by: Martin Wimpress --- internal/processor/analyzer.go | 17 +-- internal/processor/analyzer_metrics.go | 57 +++++-- internal/processor/analyzer_metrics_test.go | 156 ++++++++++++++++++++ 3 files changed, 198 insertions(+), 32 deletions(-) create mode 100644 internal/processor/analyzer_metrics_test.go diff --git a/internal/processor/analyzer.go b/internal/processor/analyzer.go index a481291..809c75b 100644 --- a/internal/processor/analyzer.go +++ b/internal/processor/analyzer.go @@ -437,22 +437,7 @@ func AnalyzeAudio(filename string, config *FilterChainConfig, progressCallback f } // Calculate average spectral statistics from aspectralstats - if acc.spectralFrameCount > 0 { - spectralFrameCountF := float64(acc.spectralFrameCount) - measurements.SpectralMean = acc.spectralMeanSum / spectralFrameCountF - measurements.SpectralVariance = acc.spectralVarianceSum / spectralFrameCountF - measurements.SpectralCentroid = acc.spectralCentroidSum / spectralFrameCountF - measurements.SpectralSpread = acc.spectralSpreadSum / spectralFrameCountF - measurements.SpectralSkewness = acc.spectralSkewnessSum / spectralFrameCountF - measurements.SpectralKurtosis = acc.spectralKurtosisSum / spectralFrameCountF - measurements.SpectralEntropy = acc.spectralEntropySum / spectralFrameCountF - measurements.SpectralFlatness = acc.spectralFlatnessSum / spectralFrameCountF - measurements.SpectralCrest = acc.spectralCrestSum / spectralFrameCountF - measurements.SpectralFlux = acc.spectralFluxSum / spectralFrameCountF - measurements.SpectralSlope = acc.spectralSlopeSum / spectralFrameCountF - measurements.SpectralDecrease = acc.spectralDecreaseSum / spectralFrameCountF - measurements.SpectralRolloff = acc.spectralRolloffSum / spectralFrameCountF - } + acc.finalizeSpectral().writeSpectralTo(&measurements.BaseMeasurements) // Store astats measurements (if captured) if acc.astatsFound { diff --git a/internal/processor/analyzer_metrics.go b/internal/processor/analyzer_metrics.go index 25a4321..1cbb497 100644 --- a/internal/processor/analyzer_metrics.go +++ b/internal/processor/analyzer_metrics.go @@ -453,6 +453,29 @@ func (b *baseMetadataAccumulators) accumulateSpectral(spectral SpectralMetrics) b.spectralFrameCount++ } +// finalizeSpectral returns averaged spectral metrics from the accumulated sums. +// Returns zero-value SpectralMetrics when no spectral frames were accumulated. +func (b *baseMetadataAccumulators) finalizeSpectral() SpectralMetrics { + if b.spectralFrameCount == 0 { + return SpectralMetrics{} + } + return SpectralMetrics{ + Mean: b.spectralMeanSum, + Variance: b.spectralVarianceSum, + Centroid: b.spectralCentroidSum, + Spread: b.spectralSpreadSum, + Skewness: b.spectralSkewnessSum, + Kurtosis: b.spectralKurtosisSum, + Entropy: b.spectralEntropySum, + Flatness: b.spectralFlatnessSum, + Crest: b.spectralCrestSum, + Flux: b.spectralFluxSum, + Slope: b.spectralSlopeSum, + Decrease: b.spectralDecreaseSum, + Rolloff: b.spectralRolloffSum, + }.average(float64(b.spectralFrameCount)) +} + // extractAstatsMetadata extracts all astats measurements from FFmpeg metadata. // These are cumulative values, so we keep the latest from each frame. // Includes conversions: linearRatioToDB for CrestFactor, linearSampleToDBFS for MinLevel/MaxLevel. @@ -658,6 +681,23 @@ func (m SpectralMetrics) average(n float64) SpectralMetrics { } } +// writeSpectralTo maps all 13 spectral fields to the corresponding BaseMeasurements fields. +func (m SpectralMetrics) writeSpectralTo(b *BaseMeasurements) { + b.SpectralMean = m.Mean + b.SpectralVariance = m.Variance + b.SpectralCentroid = m.Centroid + b.SpectralSpread = m.Spread + b.SpectralSkewness = m.Skewness + b.SpectralKurtosis = m.Kurtosis + b.SpectralEntropy = m.Entropy + b.SpectralFlatness = m.Flatness + b.SpectralCrest = m.Crest + b.SpectralFlux = m.Flux + b.SpectralSlope = m.Slope + b.SpectralDecrease = m.Decrease + b.SpectralRolloff = m.Rolloff +} + // extractSpectralMetrics extracts all 13 aspectralstats measurements from FFmpeg metadata. // Returns a SpectralMetrics struct with Found=true if at least one metric was extracted. func extractSpectralMetrics(metadata *ffmpeg.AVDictionary) SpectralMetrics { @@ -916,22 +956,7 @@ func finalizeOutputMeasurements(acc *outputMetadataAccumulators) *OutputMeasurem } // Calculate average spectral statistics from aspectralstats - if acc.spectralFrameCount > 0 { - frameCount := float64(acc.spectralFrameCount) - m.SpectralMean = acc.spectralMeanSum / frameCount - m.SpectralVariance = acc.spectralVarianceSum / frameCount - m.SpectralCentroid = acc.spectralCentroidSum / frameCount - m.SpectralSpread = acc.spectralSpreadSum / frameCount - m.SpectralSkewness = acc.spectralSkewnessSum / frameCount - m.SpectralKurtosis = acc.spectralKurtosisSum / frameCount - m.SpectralEntropy = acc.spectralEntropySum / frameCount - m.SpectralFlatness = acc.spectralFlatnessSum / frameCount - m.SpectralCrest = acc.spectralCrestSum / frameCount - m.SpectralFlux = acc.spectralFluxSum / frameCount - m.SpectralSlope = acc.spectralSlopeSum / frameCount - m.SpectralDecrease = acc.spectralDecreaseSum / frameCount - m.SpectralRolloff = acc.spectralRolloffSum / frameCount - } + acc.finalizeSpectral().writeSpectralTo(&m.BaseMeasurements) return m } diff --git a/internal/processor/analyzer_metrics_test.go b/internal/processor/analyzer_metrics_test.go new file mode 100644 index 0000000..b8975cb --- /dev/null +++ b/internal/processor/analyzer_metrics_test.go @@ -0,0 +1,156 @@ +package processor + +import ( + "math" + "testing" +) + +const spectralTestEpsilon = 1e-9 + +func TestFinalizeSpectral_ZeroFrameCount(t *testing.T) { + acc := &baseMetadataAccumulators{} + result := acc.finalizeSpectral() + + if result != (SpectralMetrics{}) { + t.Errorf("expected zero-value SpectralMetrics, got %+v", result) + } +} + +func TestFinalizeSpectral_AveragesCorrectly(t *testing.T) { + acc := &baseMetadataAccumulators{ + spectralMeanSum: 10.0, + spectralVarianceSum: 20.0, + spectralCentroidSum: 3000.0, + spectralSpreadSum: 600.0, + spectralSkewnessSum: 4.0, + spectralKurtosisSum: 8.0, + spectralEntropySum: 1.5, + spectralFlatnessSum: 0.5, + spectralCrestSum: 6.0, + spectralFluxSum: 2.0, + spectralSlopeSum: -0.02, + spectralDecreaseSum: 0.4, + spectralRolloffSum: 8000.0, + spectralFrameCount: 2, + } + + result := acc.finalizeSpectral() + + checks := []struct { + name string + got float64 + want float64 + }{ + {"Mean", result.Mean, 5.0}, + {"Variance", result.Variance, 10.0}, + {"Centroid", result.Centroid, 1500.0}, + {"Spread", result.Spread, 300.0}, + {"Skewness", result.Skewness, 2.0}, + {"Kurtosis", result.Kurtosis, 4.0}, + {"Entropy", result.Entropy, 0.75}, + {"Flatness", result.Flatness, 0.25}, + {"Crest", result.Crest, 3.0}, + {"Flux", result.Flux, 1.0}, + {"Slope", result.Slope, -0.01}, + {"Decrease", result.Decrease, 0.2}, + {"Rolloff", result.Rolloff, 4000.0}, + } + for _, c := range checks { + if math.Abs(c.got-c.want) > spectralTestEpsilon { + t.Errorf("%s: got %v, want %v", c.name, c.got, c.want) + } + } +} + +func TestWriteSpectralTo_MapsAllFields(t *testing.T) { + sm := SpectralMetrics{ + Mean: 1.0, + Variance: 2.0, + Centroid: 3.0, + Spread: 4.0, + Skewness: 5.0, + Kurtosis: 6.0, + Entropy: 7.0, + Flatness: 8.0, + Crest: 9.0, + Flux: 10.0, + Slope: 11.0, + Decrease: 12.0, + Rolloff: 13.0, + } + + var bm BaseMeasurements + sm.writeSpectralTo(&bm) + + checks := []struct { + name string + got float64 + want float64 + }{ + {"SpectralMean", bm.SpectralMean, 1.0}, + {"SpectralVariance", bm.SpectralVariance, 2.0}, + {"SpectralCentroid", bm.SpectralCentroid, 3.0}, + {"SpectralSpread", bm.SpectralSpread, 4.0}, + {"SpectralSkewness", bm.SpectralSkewness, 5.0}, + {"SpectralKurtosis", bm.SpectralKurtosis, 6.0}, + {"SpectralEntropy", bm.SpectralEntropy, 7.0}, + {"SpectralFlatness", bm.SpectralFlatness, 8.0}, + {"SpectralCrest", bm.SpectralCrest, 9.0}, + {"SpectralFlux", bm.SpectralFlux, 10.0}, + {"SpectralSlope", bm.SpectralSlope, 11.0}, + {"SpectralDecrease", bm.SpectralDecrease, 12.0}, + {"SpectralRolloff", bm.SpectralRolloff, 13.0}, + } + for _, c := range checks { + if math.Abs(c.got-c.want) > spectralTestEpsilon { + t.Errorf("%s: got %v, want %v", c.name, c.got, c.want) + } + } +} + +func TestFinalizeSpectral_WriteSpectralTo_Chained(t *testing.T) { + acc := &baseMetadataAccumulators{ + spectralMeanSum: 30.0, + spectralVarianceSum: 60.0, + spectralCentroidSum: 9000.0, + spectralSpreadSum: 1500.0, + spectralSkewnessSum: 6.0, + spectralKurtosisSum: 12.0, + spectralEntropySum: 2.1, + spectralFlatnessSum: 0.9, + spectralCrestSum: 15.0, + spectralFluxSum: 3.0, + spectralSlopeSum: -0.06, + spectralDecreaseSum: 1.2, + spectralRolloffSum: 24000.0, + spectralFrameCount: 3, + } + + var bm BaseMeasurements + acc.finalizeSpectral().writeSpectralTo(&bm) + + checks := []struct { + name string + got float64 + want float64 + }{ + {"SpectralMean", bm.SpectralMean, 10.0}, + {"SpectralVariance", bm.SpectralVariance, 20.0}, + {"SpectralCentroid", bm.SpectralCentroid, 3000.0}, + {"SpectralSpread", bm.SpectralSpread, 500.0}, + {"SpectralSkewness", bm.SpectralSkewness, 2.0}, + {"SpectralKurtosis", bm.SpectralKurtosis, 4.0}, + {"SpectralEntropy", bm.SpectralEntropy, 0.7}, + {"SpectralFlatness", bm.SpectralFlatness, 0.3}, + {"SpectralCrest", bm.SpectralCrest, 5.0}, + {"SpectralFlux", bm.SpectralFlux, 1.0}, + {"SpectralSlope", bm.SpectralSlope, -0.02}, + {"SpectralDecrease", bm.SpectralDecrease, 0.4}, + {"SpectralRolloff", bm.SpectralRolloff, 8000.0}, + } + for _, c := range checks { + if math.Abs(c.got-c.want) > spectralTestEpsilon { + t.Errorf("%s: got %v, want %v", c.name, c.got, c.want) + } + } +} From 3234ba0552fd72eb96cc24c5d6558a450953ac90 Mon Sep 17 00:00:00 2001 From: Martin Wimpress Date: Mon, 16 Mar 2026 11:35:36 +0000 Subject: [PATCH 2/3] refactor(processor): extract region pair construction into reusable function - Add extractRegionPair function in analyzer_output.go to build optional SilenceRegion and SpeechRegion pointers from AudioMeasurements profiles - Replace duplicate 17-line region construction blocks in processor.go and normalise.go with single-line extractRegionPair calls - Add comprehensive test coverage for region pair extraction logic Signed-off-by: Martin Wimpress --- internal/processor/analyzer_output.go | 22 ++++ internal/processor/analyzer_output_test.go | 112 +++++++++++++++++++++ internal/processor/normalise.go | 17 +--- internal/processor/processor.go | 17 +--- 4 files changed, 136 insertions(+), 32 deletions(-) diff --git a/internal/processor/analyzer_output.go b/internal/processor/analyzer_output.go index b322b08..e77a873 100644 --- a/internal/processor/analyzer_output.go +++ b/internal/processor/analyzer_output.go @@ -182,6 +182,28 @@ func measureOutputSilenceRegionFromReader(reader *audio.Reader, region SilenceRe }, nil } +// extractRegionPair builds optional SilenceRegion and SpeechRegion pointers +// from AudioMeasurements profiles. Returns (nil, nil) when both profiles are absent. +func extractRegionPair(m *AudioMeasurements) (*SilenceRegion, *SpeechRegion) { + var silRegion *SilenceRegion + var spRegion *SpeechRegion + if m.NoiseProfile != nil { + silRegion = &SilenceRegion{ + Start: m.NoiseProfile.Start, + End: m.NoiseProfile.Start + m.NoiseProfile.Duration, + Duration: m.NoiseProfile.Duration, + } + } + if m.SpeechProfile != nil { + spRegion = &SpeechRegion{ + Start: m.SpeechProfile.Region.Start, + End: m.SpeechProfile.Region.End, + Duration: m.SpeechProfile.Region.Duration, + } + } + return silRegion, spRegion +} + // MeasureOutputRegions measures both silence and speech regions from the same // output file in a single open/close cycle. This avoids redundant file opens, // demuxing, and decoding that would occur if silence and speech regions were diff --git a/internal/processor/analyzer_output_test.go b/internal/processor/analyzer_output_test.go index 5c76c14..e4dbbea 100644 --- a/internal/processor/analyzer_output_test.go +++ b/internal/processor/analyzer_output_test.go @@ -2,6 +2,8 @@ package processor import ( "fmt" + "testing" + "time" "github.com/linuxmatters/jivetalking/internal/audio" ) @@ -37,3 +39,113 @@ func measureOutputSpeechRegion(outputPath string, region SpeechRegion) (*SpeechC return measureOutputSpeechRegionFromReader(reader, region) } + +func TestExtractRegionPair(t *testing.T) { + tests := []struct { + name string + measurements *AudioMeasurements + wantSilence bool + wantSpeech bool + wantSilEnd time.Duration // expected End = Start + Duration + }{ + { + name: "both profiles absent returns nil pair", + measurements: &AudioMeasurements{}, + wantSilence: false, + wantSpeech: false, + }, + { + name: "NoiseProfile only returns silence region", + measurements: &AudioMeasurements{ + NoiseProfile: &NoiseProfile{ + Start: 2 * time.Second, + Duration: 500 * time.Millisecond, + }, + }, + wantSilence: true, + wantSpeech: false, + wantSilEnd: 2*time.Second + 500*time.Millisecond, + }, + { + name: "SpeechProfile only returns speech region", + measurements: &AudioMeasurements{ + SpeechProfile: &SpeechCandidateMetrics{ + Region: SpeechRegion{ + Start: 5 * time.Second, + End: 8 * time.Second, + Duration: 3 * time.Second, + }, + }, + }, + wantSilence: false, + wantSpeech: true, + }, + { + name: "both present returns both non-nil", + measurements: &AudioMeasurements{ + NoiseProfile: &NoiseProfile{ + Start: 1 * time.Second, + Duration: 400 * time.Millisecond, + }, + SpeechProfile: &SpeechCandidateMetrics{ + Region: SpeechRegion{ + Start: 10 * time.Second, + End: 13 * time.Second, + Duration: 3 * time.Second, + }, + }, + }, + wantSilence: true, + wantSpeech: true, + wantSilEnd: 1*time.Second + 400*time.Millisecond, + }, + { + name: "End equals Start plus Duration for silence region", + measurements: &AudioMeasurements{ + NoiseProfile: &NoiseProfile{ + Start: 3 * time.Second, + Duration: 750 * time.Millisecond, + }, + }, + wantSilence: true, + wantSpeech: false, + wantSilEnd: 3*time.Second + 750*time.Millisecond, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + silRegion, spRegion := extractRegionPair(tt.measurements) + + if tt.wantSilence && silRegion == nil { + t.Fatal("expected non-nil SilenceRegion, got nil") + } + if !tt.wantSilence && silRegion != nil { + t.Fatalf("expected nil SilenceRegion, got %+v", silRegion) + } + if tt.wantSpeech && spRegion == nil { + t.Fatal("expected non-nil SpeechRegion, got nil") + } + if !tt.wantSpeech && spRegion != nil { + t.Fatalf("expected nil SpeechRegion, got %+v", spRegion) + } + + if silRegion != nil && tt.wantSilEnd != 0 { + if silRegion.End != tt.wantSilEnd { + t.Errorf("SilenceRegion.End = %v, want %v (Start + Duration)", silRegion.End, tt.wantSilEnd) + } + if silRegion.End != silRegion.Start+silRegion.Duration { + t.Errorf("SilenceRegion.End (%v) != Start (%v) + Duration (%v)", + silRegion.End, silRegion.Start, silRegion.Duration) + } + } + + if spRegion != nil { + want := tt.measurements.SpeechProfile.Region + if spRegion.Start != want.Start || spRegion.End != want.End || spRegion.Duration != want.Duration { + t.Errorf("SpeechRegion = %+v, want %+v", *spRegion, want) + } + } + }) + } +} diff --git a/internal/processor/normalise.go b/internal/processor/normalise.go index 4496d9c..0291feb 100644 --- a/internal/processor/normalise.go +++ b/internal/processor/normalise.go @@ -691,22 +691,7 @@ func applyLoudnormAndMeasure( // Measure silence and speech regions in final output (same regions as Pass 1 profiles) // NOTE: inputPath now contains the normalised output after os.Rename above if inputMeasurements != nil { - var silRegion *SilenceRegion - var spRegion *SpeechRegion - if inputMeasurements.NoiseProfile != nil { - silRegion = &SilenceRegion{ - Start: inputMeasurements.NoiseProfile.Start, - End: inputMeasurements.NoiseProfile.Start + inputMeasurements.NoiseProfile.Duration, - Duration: inputMeasurements.NoiseProfile.Duration, - } - } - if inputMeasurements.SpeechProfile != nil { - spRegion = &SpeechRegion{ - Start: inputMeasurements.SpeechProfile.Region.Start, - End: inputMeasurements.SpeechProfile.Region.End, - Duration: inputMeasurements.SpeechProfile.Region.Duration, - } - } + silRegion, spRegion := extractRegionPair(inputMeasurements) if silRegion != nil || spRegion != nil { silSample, spSample := MeasureOutputRegions(inputPath, silRegion, spRegion) finalMeasurements.SilenceSample = silSample diff --git a/internal/processor/processor.go b/internal/processor/processor.go index d174fd4..412d918 100644 --- a/internal/processor/processor.go +++ b/internal/processor/processor.go @@ -91,22 +91,7 @@ func ProcessAudio(inputPath string, config *FilterChainConfig, progressCallback // Measure silence and speech regions in Pass 2 output (before normalisation) for comparison if filteredMeasurements != nil { - var silRegion *SilenceRegion - var spRegion *SpeechRegion - if measurements.NoiseProfile != nil { - silRegion = &SilenceRegion{ - Start: measurements.NoiseProfile.Start, - End: measurements.NoiseProfile.Start + measurements.NoiseProfile.Duration, - Duration: measurements.NoiseProfile.Duration, - } - } - if measurements.SpeechProfile != nil { - spRegion = &SpeechRegion{ - Start: measurements.SpeechProfile.Region.Start, - End: measurements.SpeechProfile.Region.End, - Duration: measurements.SpeechProfile.Region.Duration, - } - } + silRegion, spRegion := extractRegionPair(measurements) if silRegion != nil || spRegion != nil { silSample, spSample := MeasureOutputRegions(outputPath, silRegion, spRegion) filteredMeasurements.SilenceSample = silSample From 9dd6d2893f3b72ce5d7dd658a3aa05205c5b60bb Mon Sep 17 00:00:00 2001 From: Martin Wimpress Date: Mon, 16 Mar 2026 12:33:27 +0000 Subject: [PATCH 3/3] refactor(processor): eliminate redundant interval accumulator reset call - Remove post-declaration reset() call from AnalyzeAudio; struct zero values suffice for all fields except truePeakMax and samplePeakMax - Collapse reset() body from 28 lines to 3 lines using struct literal initialization This reduces unnecessary initialization overhead and improves code clarity by relying on Go's zero-value semantics. Signed-off-by: Martin Wimpress --- internal/processor/analyzer.go | 1 - internal/processor/analyzer_metrics.go | 31 ++++---------------------- 2 files changed, 4 insertions(+), 28 deletions(-) diff --git a/internal/processor/analyzer.go b/internal/processor/analyzer.go index 809c75b..f3acc47 100644 --- a/internal/processor/analyzer.go +++ b/internal/processor/analyzer.go @@ -326,7 +326,6 @@ func AnalyzeAudio(filename string, config *FilterChainConfig, progressCallback f const intervalDuration = 250 * time.Millisecond var intervals []IntervalSample var intervalAcc intervalAccumulator - intervalAcc.reset() // Initialize with proper defaults var intervalStartTime time.Duration // Track input frame time (before filter graph, which upsamples to 192kHz) diff --git a/internal/processor/analyzer_metrics.go b/internal/processor/analyzer_metrics.go index 1cbb497..4644800 100644 --- a/internal/processor/analyzer_metrics.go +++ b/internal/processor/analyzer_metrics.go @@ -301,33 +301,10 @@ func (a *intervalAccumulator) finalize(timestamp time.Duration) IntervalSample { // reset clears the accumulator for the next interval. func (a *intervalAccumulator) reset() { - a.frameCount = 0 - - // Raw sample RMS and peak - a.rawSumSquares = 0 - a.rawSampleCount = 0 - a.rawPeakAbs = 0 - - // aspectralstats - a.spectralMeanSum = 0 - a.spectralVarianceSum = 0 - a.spectralCentroidSum = 0 - a.spectralSpreadSum = 0 - a.spectralSkewnessSum = 0 - a.spectralKurtosisSum = 0 - a.spectralEntropySum = 0 - a.spectralFlatnessSum = 0 - a.spectralCrestSum = 0 - a.spectralFluxSum = 0 - a.spectralSlopeSum = 0 - a.spectralDecreaseSum = 0 - a.spectralRolloffSum = 0 - - // ebur128 - a.momentaryLUFSSum = 0 - a.shortTermLUFSSum = 0 - a.truePeakMax = -120.0 - a.samplePeakMax = -120.0 + *a = intervalAccumulator{ + truePeakMax: -120.0, + samplePeakMax: -120.0, + } } // Cached metadata keys for frame extraction - avoids per-frame C string allocations