From 4d36f8a220a6cd5791590aa8a7421198a681e898 Mon Sep 17 00:00:00 2001 From: Yuto Terada Date: Mon, 27 Apr 2026 19:36:51 +0900 Subject: [PATCH 1/8] feat: add audio Compressor effect --- .../Audio/Effects/CompressorEffect.cs | 62 ++++++ .../Audio/Graph/Nodes/CompressorNode.cs | 203 ++++++++++++++++++ src/Beutl.Language/AudioStrings.ja.resx | 21 ++ src/Beutl.Language/AudioStrings.resx | 21 ++ src/Beutl/Services/LibraryRegistrar.cs | 1 + .../Engine/Audio/CompressorNodeTests.cs | 157 ++++++++++++++ 6 files changed, 465 insertions(+) create mode 100644 src/Beutl.Engine/Audio/Effects/CompressorEffect.cs create mode 100644 src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs create mode 100644 tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs diff --git a/src/Beutl.Engine/Audio/Effects/CompressorEffect.cs b/src/Beutl.Engine/Audio/Effects/CompressorEffect.cs new file mode 100644 index 000000000..23e35556a --- /dev/null +++ b/src/Beutl.Engine/Audio/Effects/CompressorEffect.cs @@ -0,0 +1,62 @@ +using System.ComponentModel.DataAnnotations; +using Beutl.Audio.Graph; +using Beutl.Audio.Graph.Nodes; +using Beutl.Engine; +using Beutl.Language; + +namespace Beutl.Audio.Effects; + +[Display(Name = nameof(AudioStrings.CompressorEffect), ResourceType = typeof(AudioStrings))] +public sealed partial class CompressorEffect : AudioEffect +{ + public CompressorEffect() + { + ScanProperties(); + } + + [Range(-60f, 0f)] + [Display(Name = nameof(AudioStrings.CompressorEffect_Threshold), ResourceType = typeof(AudioStrings))] + [SuppressResourceClassGeneration] + public IProperty Threshold { get; } = Property.CreateAnimatable(-20f); + + [Range(1f, 20f)] + [Display(Name = nameof(AudioStrings.CompressorEffect_Ratio), ResourceType = typeof(AudioStrings))] + [SuppressResourceClassGeneration] + public IProperty Ratio { get; } = Property.CreateAnimatable(4f); + + [Range(0.1f, 500f)] + [Display(Name = nameof(AudioStrings.CompressorEffect_Attack), ResourceType = typeof(AudioStrings))] + [SuppressResourceClassGeneration] + public IProperty Attack { get; } = Property.CreateAnimatable(10f); + + [Range(1f, 5000f)] + [Display(Name = nameof(AudioStrings.CompressorEffect_Release), ResourceType = typeof(AudioStrings))] + [SuppressResourceClassGeneration] + public IProperty Release { get; } = Property.CreateAnimatable(100f); + + [Range(0f, 24f)] + [Display(Name = nameof(AudioStrings.CompressorEffect_Knee), ResourceType = typeof(AudioStrings))] + [SuppressResourceClassGeneration] + public IProperty Knee { get; } = Property.CreateAnimatable(6f); + + [Range(-24f, 24f)] + [Display(Name = nameof(AudioStrings.CompressorEffect_MakeupGain), ResourceType = typeof(AudioStrings))] + [SuppressResourceClassGeneration] + public IProperty MakeupGain { get; } = Property.CreateAnimatable(0f); + + public override AudioNode CreateNode(AudioContext context, AudioNode inputNode) + { + var compressorNode = context.AddNode(new CompressorNode + { + Threshold = Threshold, + Ratio = Ratio, + Attack = Attack, + Release = Release, + Knee = Knee, + MakeupGain = MakeupGain + }); + + context.Connect(inputNode, compressorNode); + return compressorNode; + } +} diff --git a/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs b/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs new file mode 100644 index 000000000..ce36674a6 --- /dev/null +++ b/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs @@ -0,0 +1,203 @@ +using Beutl.Engine; +using Beutl.Media; + +namespace Beutl.Audio.Graph.Nodes; + +public sealed class CompressorNode : AudioNode +{ + private const float MinDb = -100f; + + private float _envelopeDb = MinDb; + private int _lastSampleRate; + private TimeSpan? _lastTimeRangeStart; + + public required IProperty Threshold { get; init; } + + public required IProperty Ratio { get; init; } + + public required IProperty Attack { get; init; } + + public required IProperty Release { get; init; } + + public required IProperty Knee { get; init; } + + public required IProperty MakeupGain { get; init; } + + public override AudioBuffer Process(AudioProcessContext context) + { + if (Inputs.Count != 1) + throw new InvalidOperationException("Compressor node requires exactly one input."); + + var input = Inputs[0].Process(context); + + if (_lastSampleRate != context.SampleRate) + { + Reset(); + _lastSampleRate = context.SampleRate; + } + + if (!_lastTimeRangeStart.HasValue || _lastTimeRangeStart.Value > context.TimeRange.Start) + { + Reset(); + } + _lastTimeRangeStart = context.TimeRange.Start + context.TimeRange.Duration; + + bool hasAnimation = Threshold.IsAnimatable || + Ratio.IsAnimatable || + Attack.IsAnimatable || + Release.IsAnimatable || + Knee.IsAnimatable || + MakeupGain.IsAnimatable; + + if (!hasAnimation) + { + return ProcessStatic(input, context); + } + + return ProcessAnimated(input, context); + } + + private AudioBuffer ProcessStatic(AudioBuffer input, AudioProcessContext context) + { + var output = new AudioBuffer(input.SampleRate, input.ChannelCount, input.SampleCount); + + float threshold = Threshold.CurrentValue; + float ratio = MathF.Max(1f, Ratio.CurrentValue); + float attackMs = MathF.Max(0.01f, Attack.CurrentValue); + float releaseMs = MathF.Max(0.01f, Release.CurrentValue); + float knee = MathF.Max(0f, Knee.CurrentValue); + float makeupGain = MakeupGain.CurrentValue; + + float attackCoeff = ComputeCoeff(attackMs, context.SampleRate); + float releaseCoeff = ComputeCoeff(releaseMs, context.SampleRate); + float slope = 1f - 1f / ratio; + float makeupLinear = AudioMath.ConvertDbToLinear(makeupGain); + + int channels = input.ChannelCount; + for (int i = 0; i < input.SampleCount; i++) + { + float peak = 0f; + for (int ch = 0; ch < channels; ch++) + { + float a = MathF.Abs(input.GetChannelData(ch)[i]); + if (a > peak) peak = a; + } + + float inputDb = peak > 0f ? 20f * MathF.Log10(peak) : MinDb; + float coeff = inputDb > _envelopeDb ? attackCoeff : releaseCoeff; + _envelopeDb = inputDb + coeff * (_envelopeDb - inputDb); + + float gainReductionDb = ComputeGainReductionDb(_envelopeDb, threshold, knee, slope); + float gainLinear = AudioMath.ConvertDbToLinear(-gainReductionDb) * makeupLinear; + + for (int ch = 0; ch < channels; ch++) + { + output.GetChannelData(ch)[i] = input.GetChannelData(ch)[i] * gainLinear; + } + } + + return output; + } + + private AudioBuffer ProcessAnimated(AudioBuffer input, AudioProcessContext context) + { + var output = new AudioBuffer(input.SampleRate, input.ChannelCount, input.SampleCount); + + const int maxChunkSize = 1024; + int bufferSize = Math.Min(maxChunkSize, input.SampleCount); + Span thresholds = stackalloc float[bufferSize]; + Span ratios = stackalloc float[bufferSize]; + Span attacks = stackalloc float[bufferSize]; + Span releases = stackalloc float[bufferSize]; + Span knees = stackalloc float[bufferSize]; + Span makeups = stackalloc float[bufferSize]; + + int channels = input.ChannelCount; + int processed = 0; + while (processed < input.SampleCount) + { + int chunkSize = Math.Min(bufferSize, input.SampleCount - processed); + + var chunkStart = context.GetTimeForSample(processed); + var chunkEnd = context.GetTimeForSample(processed + chunkSize); + var chunkRange = new TimeRange(chunkStart, chunkEnd - chunkStart); + + context.AnimationSampler.SampleBuffer(Threshold, chunkRange, context.SampleRate, thresholds[..chunkSize]); + context.AnimationSampler.SampleBuffer(Ratio, chunkRange, context.SampleRate, ratios[..chunkSize]); + context.AnimationSampler.SampleBuffer(Attack, chunkRange, context.SampleRate, attacks[..chunkSize]); + context.AnimationSampler.SampleBuffer(Release, chunkRange, context.SampleRate, releases[..chunkSize]); + context.AnimationSampler.SampleBuffer(Knee, chunkRange, context.SampleRate, knees[..chunkSize]); + context.AnimationSampler.SampleBuffer(MakeupGain, chunkRange, context.SampleRate, makeups[..chunkSize]); + + for (int i = 0; i < chunkSize; i++) + { + int idx = processed + i; + float peak = 0f; + for (int ch = 0; ch < channels; ch++) + { + float a = MathF.Abs(input.GetChannelData(ch)[idx]); + if (a > peak) peak = a; + } + + float inputDb = peak > 0f ? 20f * MathF.Log10(peak) : MinDb; + + float threshold = thresholds[i]; + float ratio = MathF.Max(1f, ratios[i]); + float attackMs = MathF.Max(0.01f, attacks[i]); + float releaseMs = MathF.Max(0.01f, releases[i]); + float knee = MathF.Max(0f, knees[i]); + float makeupDb = makeups[i]; + + float attackCoeff = ComputeCoeff(attackMs, context.SampleRate); + float releaseCoeff = ComputeCoeff(releaseMs, context.SampleRate); + float slope = 1f - 1f / ratio; + + float coeff = inputDb > _envelopeDb ? attackCoeff : releaseCoeff; + _envelopeDb = inputDb + coeff * (_envelopeDb - inputDb); + + float gainReductionDb = ComputeGainReductionDb(_envelopeDb, threshold, knee, slope); + float gainLinear = AudioMath.ConvertDbToLinear(makeupDb - gainReductionDb); + + for (int ch = 0; ch < channels; ch++) + { + output.GetChannelData(ch)[idx] = input.GetChannelData(ch)[idx] * gainLinear; + } + } + + processed += chunkSize; + } + + return output; + } + + private static float ComputeCoeff(float timeMs, int sampleRate) + { + return MathF.Exp(-1f / (timeMs * 0.001f * sampleRate)); + } + + private static float ComputeGainReductionDb(float envelopeDb, float thresholdDb, float kneeDb, float slope) + { + if (kneeDb > 0f) + { + float halfKnee = kneeDb * 0.5f; + float diff = envelopeDb - thresholdDb; + if (diff <= -halfKnee) + { + return 0f; + } + if (diff < halfKnee) + { + float x = diff + halfKnee; + return slope * x * x / (2f * kneeDb); + } + return slope * diff; + } + + return envelopeDb > thresholdDb ? slope * (envelopeDb - thresholdDb) : 0f; + } + + public void Reset() + { + _envelopeDb = MinDb; + } +} diff --git a/src/Beutl.Language/AudioStrings.ja.resx b/src/Beutl.Language/AudioStrings.ja.resx index 72a2676e9..1ea965867 100644 --- a/src/Beutl.Language/AudioStrings.ja.resx +++ b/src/Beutl.Language/AudioStrings.ja.resx @@ -82,4 +82,25 @@ ゲイン + + コンプレッサー + + + スレッショルド (dB) + + + レシオ + + + アタック (ms) + + + リリース (ms) + + + ニー (dB) + + + メイクアップゲイン (dB) + diff --git a/src/Beutl.Language/AudioStrings.resx b/src/Beutl.Language/AudioStrings.resx index 1e27e453b..a876de77d 100644 --- a/src/Beutl.Language/AudioStrings.resx +++ b/src/Beutl.Language/AudioStrings.resx @@ -87,4 +87,25 @@ Gain + + Compressor + + + Threshold (dB) + + + Ratio + + + Attack (ms) + + + Release (ms) + + + Knee (dB) + + + Makeup Gain (dB) + diff --git a/src/Beutl/Services/LibraryRegistrar.cs b/src/Beutl/Services/LibraryRegistrar.cs index a333b3b11..1c70fe1ca 100644 --- a/src/Beutl/Services/LibraryRegistrar.cs +++ b/src/Beutl/Services/LibraryRegistrar.cs @@ -210,6 +210,7 @@ public static void RegisterAll() .RegisterGroup(AudioStrings.AudioEffect, g => g .AddAudioEffect(AudioStrings.DelayEffect) .AddAudioEffect(AudioStrings.Equalizer) + .AddAudioEffect(AudioStrings.CompressorEffect) ); } } diff --git a/tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs b/tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs new file mode 100644 index 000000000..86bfdc734 --- /dev/null +++ b/tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs @@ -0,0 +1,157 @@ +using Beutl.Animation; +using Beutl.Audio; +using Beutl.Audio.Graph; +using Beutl.Audio.Graph.Nodes; +using Beutl.Engine; +using Beutl.Media; + +namespace Beutl.UnitTests.Engine.Audio; + +public class CompressorNodeTests +{ + private const int SampleRate = 48000; + + private sealed class StubSourceNode : AudioNode + { + public required AudioBuffer Buffer { get; init; } + + public override AudioBuffer Process(AudioProcessContext context) + { + var copy = new AudioBuffer(Buffer.SampleRate, Buffer.ChannelCount, Buffer.SampleCount); + Buffer.CopyTo(copy); + return copy; + } + } + + private static AudioBuffer CreateSineBuffer(float amplitude, float frequencyHz, int sampleCount, int channels = 2) + { + var buffer = new AudioBuffer(SampleRate, channels, sampleCount); + for (int ch = 0; ch < channels; ch++) + { + var data = buffer.GetChannelData(ch); + for (int i = 0; i < sampleCount; i++) + { + data[i] = amplitude * MathF.Sin(2f * MathF.PI * frequencyHz * i / SampleRate); + } + } + return buffer; + } + + private static float PeakDb(AudioBuffer buffer, int startSample) + { + float peak = 0f; + for (int ch = 0; ch < buffer.ChannelCount; ch++) + { + var data = buffer.GetChannelData(ch); + for (int i = startSample; i < buffer.SampleCount; i++) + { + float a = MathF.Abs(data[i]); + if (a > peak) peak = a; + } + } + return peak > 0f ? 20f * MathF.Log10(peak) : -100f; + } + + [Test] + public void Process_BelowThreshold_LeavesSignalUnchanged() + { + // Amplitude 0.05 ≈ -26 dB peak, well below the -20 dB threshold, so output should pass through. + const int sampleCount = SampleRate / 2; + var input = CreateSineBuffer(0.05f, 1000f, sampleCount); + var source = new StubSourceNode { Buffer = input }; + + var node = new CompressorNode + { + Threshold = Property.CreateAnimatable(-20f), + Ratio = Property.CreateAnimatable(4f), + Attack = Property.CreateAnimatable(10f), + Release = Property.CreateAnimatable(100f), + Knee = Property.CreateAnimatable(0f), + MakeupGain = Property.CreateAnimatable(0f) + }; + node.AddInput(source); + + var ctx = new AudioProcessContext( + new TimeRange(TimeSpan.Zero, TimeSpan.FromSeconds(0.5)), + SampleRate, + new AnimationSampler(), + null); + + var output = node.Process(ctx); + + float inputPeakDb = PeakDb(input, 0); + float outputPeakDb = PeakDb(output, sampleCount / 2); + + Assert.That(outputPeakDb, Is.EqualTo(inputPeakDb).Within(0.5f)); + } + + [Test] + public void Process_AboveThreshold_AppliesExpectedGainReduction() + { + // Steady 0.9 amplitude (~-0.92 dB) sine, threshold -20 dB, ratio 4:1, hard knee. + // Steady-state gain reduction ≈ (-0.92 - -20) * (1 - 1/4) ≈ 14.3 dB. + const int sampleCount = SampleRate; + var input = CreateSineBuffer(0.9f, 1000f, sampleCount); + var source = new StubSourceNode { Buffer = input }; + + var node = new CompressorNode + { + Threshold = Property.CreateAnimatable(-20f), + Ratio = Property.CreateAnimatable(4f), + Attack = Property.CreateAnimatable(5f), + Release = Property.CreateAnimatable(50f), + Knee = Property.CreateAnimatable(0f), + MakeupGain = Property.CreateAnimatable(0f) + }; + node.AddInput(source); + + var ctx = new AudioProcessContext( + new TimeRange(TimeSpan.Zero, TimeSpan.FromSeconds(1.0)), + SampleRate, + new AnimationSampler(), + null); + + var output = node.Process(ctx); + + // After the attack settles, the signal should sit around -15 dB. + float steadyStartSample = SampleRate / 2; + float outputPeakDb = PeakDb(output, (int)steadyStartSample); + + Assert.That(outputPeakDb, Is.LessThan(-12f)); + Assert.That(outputPeakDb, Is.GreaterThan(-18f)); + } + + [Test] + public void Process_MakeupGain_RaisesOutputAboveReducedLevel() + { + const int sampleCount = SampleRate; + var input = CreateSineBuffer(0.9f, 1000f, sampleCount); + var source = new StubSourceNode { Buffer = input }; + + var node = new CompressorNode + { + Threshold = Property.CreateAnimatable(-20f), + Ratio = Property.CreateAnimatable(4f), + Attack = Property.CreateAnimatable(5f), + Release = Property.CreateAnimatable(50f), + Knee = Property.CreateAnimatable(0f), + MakeupGain = Property.CreateAnimatable(6f) + }; + node.AddInput(source); + + var ctx = new AudioProcessContext( + new TimeRange(TimeSpan.Zero, TimeSpan.FromSeconds(1.0)), + SampleRate, + new AnimationSampler(), + null); + + var output = node.Process(ctx); + + float steadyStartSample = SampleRate / 2; + float outputPeakDb = PeakDb(output, (int)steadyStartSample); + + // 6 dB makeup applied to ~-15 dB → ~-9 dB. + Assert.That(outputPeakDb, Is.LessThan(-6f)); + Assert.That(outputPeakDb, Is.GreaterThan(-12f)); + } +} From 041a72e4bb2b5be277579d0b62c511350c7ad64e Mon Sep 17 00:00:00 2001 From: Yuto Terada Date: Mon, 27 Apr 2026 22:04:55 +0900 Subject: [PATCH 2/8] fix: correct compressor envelope state tracking and animation detection logic --- .../Audio/Graph/Nodes/CompressorNode.cs | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs b/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs index ce36674a6..63c671d4b 100644 --- a/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs +++ b/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs @@ -9,7 +9,7 @@ public sealed class CompressorNode : AudioNode private float _envelopeDb = MinDb; private int _lastSampleRate; - private TimeSpan? _lastTimeRangeStart; + private TimeSpan? _lastTimeRangeEnd; public required IProperty Threshold { get; init; } @@ -36,18 +36,21 @@ public override AudioBuffer Process(AudioProcessContext context) _lastSampleRate = context.SampleRate; } - if (!_lastTimeRangeStart.HasValue || _lastTimeRangeStart.Value > context.TimeRange.Start) + // Reset whenever the chunk does not continue directly from the previous one, because the + // node instance is cached across Compose() calls and stale envelope state would otherwise + // bleed into the first samples after a seek or stop/restart. + if (!_lastTimeRangeEnd.HasValue || _lastTimeRangeEnd.Value != context.TimeRange.Start) { Reset(); } - _lastTimeRangeStart = context.TimeRange.Start + context.TimeRange.Duration; - - bool hasAnimation = Threshold.IsAnimatable || - Ratio.IsAnimatable || - Attack.IsAnimatable || - Release.IsAnimatable || - Knee.IsAnimatable || - MakeupGain.IsAnimatable; + _lastTimeRangeEnd = context.TimeRange.Start + context.TimeRange.Duration; + + bool hasAnimation = Threshold.Animation != null || + Ratio.Animation != null || + Attack.Animation != null || + Release.Animation != null || + Knee.Animation != null || + MakeupGain.Animation != null; if (!hasAnimation) { From c96c7845cb1bfe16af4386eecba83a7c58421572 Mon Sep 17 00:00:00 2001 From: Yuto Terada Date: Mon, 27 Apr 2026 22:25:45 +0900 Subject: [PATCH 3/8] fix: normalize file encodings to UTF-8 with BOM --- src/Beutl.Engine/Audio/Effects/CompressorEffect.cs | 2 +- src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs | 2 +- tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Beutl.Engine/Audio/Effects/CompressorEffect.cs b/src/Beutl.Engine/Audio/Effects/CompressorEffect.cs index 23e35556a..ca7c3a597 100644 --- a/src/Beutl.Engine/Audio/Effects/CompressorEffect.cs +++ b/src/Beutl.Engine/Audio/Effects/CompressorEffect.cs @@ -1,4 +1,4 @@ -using System.ComponentModel.DataAnnotations; +using System.ComponentModel.DataAnnotations; using Beutl.Audio.Graph; using Beutl.Audio.Graph.Nodes; using Beutl.Engine; diff --git a/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs b/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs index 63c671d4b..23ac2a9b1 100644 --- a/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs +++ b/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs @@ -1,4 +1,4 @@ -using Beutl.Engine; +using Beutl.Engine; using Beutl.Media; namespace Beutl.Audio.Graph.Nodes; diff --git a/tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs b/tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs index 86bfdc734..34ffea78b 100644 --- a/tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs +++ b/tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs @@ -1,4 +1,4 @@ -using Beutl.Animation; +using Beutl.Animation; using Beutl.Audio; using Beutl.Audio.Graph; using Beutl.Audio.Graph.Nodes; From 9b3ad1b2356fb326cbd48c530c8e025f77cd3f75 Mon Sep 17 00:00:00 2001 From: Yuto Terada Date: Sun, 10 May 2026 16:06:25 +0900 Subject: [PATCH 4/8] fix: harden compressor node against non-finite values and expand tests Add layered defenses so corrupted upstream samples or malformed animation KeyFrames cannot silently mute the audio pipeline: the envelope state self- recovers, animation parameters fall back to their DefaultValue, and output samples are sanitized before reaching downstream nodes. Each non-finite event is logged once per node instance with the offending parameter name to aid debugging without flooding the audio thread. Align the per-parameter lower bounds with the [Range] attributes on CompressorEffect, cache attack/release coefficients across samples, and improve the wrong-input- count error message. Expand CompressorNodeTests from 3 to 25 cases covering ratio clamping, linked-stereo gain sharing, chunk-spanning envelope continuity, seek and sample-rate resets, soft-knee transition, animated paths (including a parameterized fallback test for all six animated parameters), chunk- boundary smoothness, mono buffers, and explicit invalid-input throws. --- .../Audio/Graph/Nodes/CompressorNode.cs | 122 ++- .../Engine/Audio/CompressorNodeTests.cs | 721 ++++++++++++++++-- 2 files changed, 773 insertions(+), 70 deletions(-) diff --git a/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs b/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs index 23ac2a9b1..89c8025a3 100644 --- a/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs +++ b/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs @@ -1,16 +1,33 @@ using Beutl.Engine; +using Beutl.Logging; using Beutl.Media; +using Microsoft.Extensions.Logging; namespace Beutl.Audio.Graph.Nodes; public sealed class CompressorNode : AudioNode { + private static readonly ILogger s_logger = Log.CreateLogger(); + private const float MinDb = -100f; + // Lower bounds match the [Range] attributes declared on CompressorEffect. Keeping them in sync + // avoids silent clamping when animation values overshoot the declared range. + private const float MinAttackMs = 0.1f; + private const float MinReleaseMs = 1f; + private const float MinRatio = 1f; + private const float MinKneeDb = 0f; + private float _envelopeDb = MinDb; private int _lastSampleRate; private TimeSpan? _lastTimeRangeEnd; + // Latched per node instance so the warning only fires once per non-finite event class, even + // when the corruption persists across thousands of samples. + private bool _loggedNonFiniteEnvelope; + private bool _loggedNonFiniteSample; + private bool _loggedNonFiniteParameter; + public required IProperty Threshold { get; init; } public required IProperty Ratio { get; init; } @@ -26,7 +43,8 @@ public sealed class CompressorNode : AudioNode public override AudioBuffer Process(AudioProcessContext context) { if (Inputs.Count != 1) - throw new InvalidOperationException("Compressor node requires exactly one input."); + throw new InvalidOperationException( + $"Compressor node requires exactly one input but got {Inputs.Count}."); var input = Inputs[0].Process(context); @@ -64,12 +82,12 @@ private AudioBuffer ProcessStatic(AudioBuffer input, AudioProcessContext context { var output = new AudioBuffer(input.SampleRate, input.ChannelCount, input.SampleCount); - float threshold = Threshold.CurrentValue; - float ratio = MathF.Max(1f, Ratio.CurrentValue); - float attackMs = MathF.Max(0.01f, Attack.CurrentValue); - float releaseMs = MathF.Max(0.01f, Release.CurrentValue); - float knee = MathF.Max(0f, Knee.CurrentValue); - float makeupGain = MakeupGain.CurrentValue; + float threshold = SafeParameter(Threshold.CurrentValue, Threshold.DefaultValue, nameof(Threshold)); + float ratio = MathF.Max(MinRatio, SafeParameter(Ratio.CurrentValue, Ratio.DefaultValue, nameof(Ratio))); + float attackMs = MathF.Max(MinAttackMs, SafeParameter(Attack.CurrentValue, Attack.DefaultValue, nameof(Attack))); + float releaseMs = MathF.Max(MinReleaseMs, SafeParameter(Release.CurrentValue, Release.DefaultValue, nameof(Release))); + float knee = MathF.Max(MinKneeDb, SafeParameter(Knee.CurrentValue, Knee.DefaultValue, nameof(Knee))); + float makeupGain = SafeParameter(MakeupGain.CurrentValue, MakeupGain.DefaultValue, nameof(MakeupGain)); float attackCoeff = ComputeCoeff(attackMs, context.SampleRate); float releaseCoeff = ComputeCoeff(releaseMs, context.SampleRate); @@ -89,13 +107,15 @@ private AudioBuffer ProcessStatic(AudioBuffer input, AudioProcessContext context float inputDb = peak > 0f ? 20f * MathF.Log10(peak) : MinDb; float coeff = inputDb > _envelopeDb ? attackCoeff : releaseCoeff; _envelopeDb = inputDb + coeff * (_envelopeDb - inputDb); + RecoverEnvelopeIfNonFinite(); float gainReductionDb = ComputeGainReductionDb(_envelopeDb, threshold, knee, slope); float gainLinear = AudioMath.ConvertDbToLinear(-gainReductionDb) * makeupLinear; for (int ch = 0; ch < channels; ch++) { - output.GetChannelData(ch)[i] = input.GetChannelData(ch)[i] * gainLinear; + float sample = input.GetChannelData(ch)[i] * gainLinear; + output.GetChannelData(ch)[i] = SanitizeOutput(sample); } } @@ -115,8 +135,27 @@ private AudioBuffer ProcessAnimated(AudioBuffer input, AudioProcessContext conte Span knees = stackalloc float[bufferSize]; Span makeups = stackalloc float[bufferSize]; + // Final fallbacks used when an animated parameter samples to NaN/Infinity (e.g. malformed + // KeyFrame). Without this guard, a single non-finite animated value would propagate into + // the gain calculation and cause the output sanitizer to silently zero out every sample. + float thresholdFallback = SafeParameter(Threshold.CurrentValue, Threshold.DefaultValue, nameof(Threshold)); + float ratioFallback = MathF.Max(MinRatio, SafeParameter(Ratio.CurrentValue, Ratio.DefaultValue, nameof(Ratio))); + float attackFallback = MathF.Max(MinAttackMs, SafeParameter(Attack.CurrentValue, Attack.DefaultValue, nameof(Attack))); + float releaseFallback = MathF.Max(MinReleaseMs, SafeParameter(Release.CurrentValue, Release.DefaultValue, nameof(Release))); + float kneeFallback = MathF.Max(MinKneeDb, SafeParameter(Knee.CurrentValue, Knee.DefaultValue, nameof(Knee))); + float makeupFallback = SafeParameter(MakeupGain.CurrentValue, MakeupGain.DefaultValue, nameof(MakeupGain)); + int channels = input.ChannelCount; int processed = 0; + + // lastAttackMs/lastReleaseMs are seeded with NaN so the first comparison is always + // unequal and the coefficients get computed on the very first sample. After that, Exp + // is only called when the animated ms value actually changes. + float lastAttackMs = float.NaN; + float lastReleaseMs = float.NaN; + float attackCoeff = 0f; + float releaseCoeff = 0f; + while (processed < input.SampleCount) { int chunkSize = Math.Min(bufferSize, input.SampleCount - processed); @@ -144,26 +183,36 @@ private AudioBuffer ProcessAnimated(AudioBuffer input, AudioProcessContext conte float inputDb = peak > 0f ? 20f * MathF.Log10(peak) : MinDb; - float threshold = thresholds[i]; - float ratio = MathF.Max(1f, ratios[i]); - float attackMs = MathF.Max(0.01f, attacks[i]); - float releaseMs = MathF.Max(0.01f, releases[i]); - float knee = MathF.Max(0f, knees[i]); - float makeupDb = makeups[i]; + float threshold = SafeParameter(thresholds[i], thresholdFallback, nameof(Threshold)); + float ratio = MathF.Max(MinRatio, SafeParameter(ratios[i], ratioFallback, nameof(Ratio))); + float attackMs = MathF.Max(MinAttackMs, SafeParameter(attacks[i], attackFallback, nameof(Attack))); + float releaseMs = MathF.Max(MinReleaseMs, SafeParameter(releases[i], releaseFallback, nameof(Release))); + float knee = MathF.Max(MinKneeDb, SafeParameter(knees[i], kneeFallback, nameof(Knee))); + float makeupDb = SafeParameter(makeups[i], makeupFallback, nameof(MakeupGain)); - float attackCoeff = ComputeCoeff(attackMs, context.SampleRate); - float releaseCoeff = ComputeCoeff(releaseMs, context.SampleRate); + if (attackMs != lastAttackMs) + { + attackCoeff = ComputeCoeff(attackMs, context.SampleRate); + lastAttackMs = attackMs; + } + if (releaseMs != lastReleaseMs) + { + releaseCoeff = ComputeCoeff(releaseMs, context.SampleRate); + lastReleaseMs = releaseMs; + } float slope = 1f - 1f / ratio; float coeff = inputDb > _envelopeDb ? attackCoeff : releaseCoeff; _envelopeDb = inputDb + coeff * (_envelopeDb - inputDb); + RecoverEnvelopeIfNonFinite(); float gainReductionDb = ComputeGainReductionDb(_envelopeDb, threshold, knee, slope); float gainLinear = AudioMath.ConvertDbToLinear(makeupDb - gainReductionDb); for (int ch = 0; ch < channels; ch++) { - output.GetChannelData(ch)[idx] = input.GetChannelData(ch)[idx] * gainLinear; + float sample = input.GetChannelData(ch)[idx] * gainLinear; + output.GetChannelData(ch)[idx] = SanitizeOutput(sample); } } @@ -173,6 +222,45 @@ private AudioBuffer ProcessAnimated(AudioBuffer input, AudioProcessContext conte return output; } + // Without this guard, a single non-finite envelope sample would permanently poison the state + // until the next Reset(). The first occurrence is logged; subsequent ones are suppressed so + // the audio thread is not spammed. + private void RecoverEnvelopeIfNonFinite() + { + if (float.IsFinite(_envelopeDb)) return; + _envelopeDb = MinDb; + if (_loggedNonFiniteEnvelope) return; + s_logger.LogWarning( + "Compressor envelope became non-finite (input sample produced inf/NaN); resetting to {MinDb} dB. Further occurrences will be suppressed.", + MinDb); + _loggedNonFiniteEnvelope = true; + } + + private float SafeParameter(float value, float fallback, string paramName) + { + if (float.IsFinite(value)) return value; + if (!_loggedNonFiniteParameter) + { + s_logger.LogWarning( + "Compressor parameter '{Param}' produced a non-finite value; falling back to {Fallback}. Further occurrences will be suppressed.", + paramName, fallback); + _loggedNonFiniteParameter = true; + } + return fallback; + } + + private float SanitizeOutput(float sample) + { + if (float.IsFinite(sample)) return sample; + if (!_loggedNonFiniteSample) + { + s_logger.LogWarning( + "Compressor produced a non-finite output sample; replacing with 0 to protect downstream nodes. Further occurrences will be suppressed."); + _loggedNonFiniteSample = true; + } + return 0f; + } + private static float ComputeCoeff(float timeMs, int sampleRate) { return MathF.Exp(-1f / (timeMs * 0.001f * sampleRate)); diff --git a/tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs b/tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs index 34ffea78b..f44151e84 100644 --- a/tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs +++ b/tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs @@ -1,4 +1,5 @@ using Beutl.Animation; +using Beutl.Animation.Easings; using Beutl.Audio; using Beutl.Audio.Graph; using Beutl.Audio.Graph.Nodes; @@ -23,20 +24,30 @@ public override AudioBuffer Process(AudioProcessContext context) } } - private static AudioBuffer CreateSineBuffer(float amplitude, float frequencyHz, int sampleCount, int channels = 2) + private static AudioBuffer CreateSineBuffer(float amplitude, float frequencyHz, int sampleCount, int channels = 2, int sampleRate = SampleRate) { - var buffer = new AudioBuffer(SampleRate, channels, sampleCount); + var buffer = new AudioBuffer(sampleRate, channels, sampleCount); for (int ch = 0; ch < channels; ch++) { var data = buffer.GetChannelData(ch); for (int i = 0; i < sampleCount; i++) { - data[i] = amplitude * MathF.Sin(2f * MathF.PI * frequencyHz * i / SampleRate); + data[i] = amplitude * MathF.Sin(2f * MathF.PI * frequencyHz * i / sampleRate); } } return buffer; } + private static AudioBuffer CreateConstantBuffer(float amplitude, int sampleCount, int channels = 2) + { + var buffer = new AudioBuffer(SampleRate, channels, sampleCount); + for (int ch = 0; ch < channels; ch++) + { + buffer.GetChannelData(ch).Fill(amplitude); + } + return buffer; + } + private static float PeakDb(AudioBuffer buffer, int startSample) { float peak = 0f; @@ -52,106 +63,710 @@ private static float PeakDb(AudioBuffer buffer, int startSample) return peak > 0f ? 20f * MathF.Log10(peak) : -100f; } + private static float ChannelPeakDb(AudioBuffer buffer, int channel, int startSample) + { + float peak = 0f; + var data = buffer.GetChannelData(channel); + for (int i = startSample; i < buffer.SampleCount; i++) + { + float a = MathF.Abs(data[i]); + if (a > peak) peak = a; + } + return peak > 0f ? 20f * MathF.Log10(peak) : -100f; + } + + private static float PeakDbInWindow(AudioBuffer buffer, int startSample, int width) + { + int end = Math.Min(buffer.SampleCount, startSample + width); + float peak = 0f; + for (int ch = 0; ch < buffer.ChannelCount; ch++) + { + var data = buffer.GetChannelData(ch); + for (int i = startSample; i < end; i++) + { + float a = MathF.Abs(data[i]); + if (a > peak) peak = a; + } + } + return peak > 0f ? 20f * MathF.Log10(peak) : -100f; + } + + private static CompressorNode CreateNode( + float threshold = -20f, + float ratio = 4f, + float attack = 5f, + float release = 50f, + float knee = 0f, + float makeup = 0f) + { + return new CompressorNode + { + Threshold = Property.CreateAnimatable(threshold), + Ratio = Property.CreateAnimatable(ratio), + Attack = Property.CreateAnimatable(attack), + Release = Property.CreateAnimatable(release), + Knee = Property.CreateAnimatable(knee), + MakeupGain = Property.CreateAnimatable(makeup) + }; + } + + private static AudioProcessContext CreateContext(TimeSpan start, TimeSpan duration, int sampleRate = SampleRate) + { + return new AudioProcessContext( + new TimeRange(start, duration), + sampleRate, + new AnimationSampler(), + null); + } + [Test] public void Process_BelowThreshold_LeavesSignalUnchanged() { - // Amplitude 0.05 ≈ -26 dB peak, well below the -20 dB threshold, so output should pass through. + // Amplitude 0.05 ≈ -26 dB peak, well below the -20 dB threshold, so output should be a + // bit-identical pass-through. We verify per-sample equality (not just peak) so that any + // unexpected residual gain reduction is caught immediately. + const int sampleCount = SampleRate / 2; + using var input = CreateSineBuffer(0.05f, 1000f, sampleCount); + var source = new StubSourceNode { Buffer = input }; + + var node = CreateNode(); + node.AddInput(source); + + var ctx = CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(0.5)); + + using var output = node.Process(ctx); + + for (int ch = 0; ch < input.ChannelCount; ch++) + { + var inData = input.GetChannelData(ch); + var outData = output.GetChannelData(ch); + for (int i = 0; i < sampleCount; i++) + { + Assert.That(outData[i], Is.EqualTo(inData[i]).Within(1e-5f)); + } + } + } + + [Test] + public void Process_AboveThreshold_AppliesExpectedGainReduction() + { + // Sine well above the threshold: the per-sample peak detector dips at every zero crossing + // so the steady-state reduction is somewhat below the textbook ratio formula. Tolerance + // is loose enough to absorb that envelope ripple but tight enough to catch a slope sign + // flip or a missing makeup application. + const int sampleCount = SampleRate; + using var input = CreateSineBuffer(0.9f, 1000f, sampleCount); + var source = new StubSourceNode { Buffer = input }; + + var node = CreateNode(); + node.AddInput(source); + + var ctx = CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(1.0)); + + using var output = node.Process(ctx); + + float steadyStartSample = SampleRate / 2; + float outputPeakDb = PeakDb(output, (int)steadyStartSample); + + Assert.That(outputPeakDb, Is.EqualTo(-13.5f).Within(1.5f)); + } + + [Test] + public void Process_MakeupGain_RaisesOutputAboveReducedLevel() + { + const int sampleCount = SampleRate; + using var input = CreateSineBuffer(0.9f, 1000f, sampleCount); + var source = new StubSourceNode { Buffer = input }; + + var node = CreateNode(makeup: 6f); + node.AddInput(source); + + var ctx = CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(1.0)); + + using var output = node.Process(ctx); + + float steadyStartSample = SampleRate / 2; + float outputPeakDb = PeakDb(output, (int)steadyStartSample); + + // Makeup gain should add directly on top of the compressed level. + Assert.That(outputPeakDb, Is.EqualTo(-7.5f).Within(1.5f)); + } + + [Test] + public void Process_RatioOne_PassesSignalThroughUnchanged() + { + const int sampleCount = SampleRate / 4; + using var input = CreateSineBuffer(0.9f, 1000f, sampleCount); + var source = new StubSourceNode { Buffer = input }; + + var node = CreateNode(ratio: 1f); + node.AddInput(source); + + var ctx = CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(0.25)); + + using var output = node.Process(ctx); + + for (int ch = 0; ch < input.ChannelCount; ch++) + { + var inData = input.GetChannelData(ch); + var outData = output.GetChannelData(ch); + for (int i = 0; i < sampleCount; i++) + { + Assert.That(outData[i], Is.EqualTo(inData[i]).Within(1e-6f)); + } + } + } + + [Test] + public void Process_RatioBelowOne_ClampsToPassthrough() + { + // Animation/programmatic assignment can push ratio below 1; the node must clamp it to 1 + // (passthrough) rather than amplify above the threshold. + const int sampleCount = SampleRate / 4; + using var input = CreateSineBuffer(0.9f, 1000f, sampleCount); + var source = new StubSourceNode { Buffer = input }; + + var node = CreateNode(ratio: 0.5f); + node.AddInput(source); + + var ctx = CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(0.25)); + + using var output = node.Process(ctx); + + float outputPeakDb = PeakDb(output, sampleCount / 2); + float inputPeakDb = PeakDb(input, 0); + Assert.That(outputPeakDb, Is.EqualTo(inputPeakDb).Within(0.5f)); + } + + [Test] + public void Process_LinkedStereo_AppliesSameGainToBothChannels() + { + // L = 0.9 sine drives compression; R = 0.05 sine sits below the threshold and would not + // compress on its own. Linked-stereo behaviour applies the L-derived gain reduction to R + // as well, so R's output should be R_input_peak attenuated by the same amount as L. + const int sampleCount = SampleRate; + using var input = new AudioBuffer(SampleRate, 2, sampleCount); + float leftInputPeakDb = 20f * MathF.Log10(0.9f); + float rightInputPeakDb = 20f * MathF.Log10(0.05f); + var lData = input.GetChannelData(0); + var rData = input.GetChannelData(1); + for (int i = 0; i < sampleCount; i++) + { + float t = 2f * MathF.PI * 1000f * i / SampleRate; + lData[i] = 0.9f * MathF.Sin(t); + rData[i] = 0.05f * MathF.Sin(t); + } + var source = new StubSourceNode { Buffer = input }; + + var node = CreateNode(); + node.AddInput(source); + + var ctx = CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(1.0)); + + using var output = node.Process(ctx); + + int steadyStart = SampleRate / 2; + float leftPeakDb = ChannelPeakDb(output, 0, steadyStart); + float rightPeakDb = ChannelPeakDb(output, 1, steadyStart); + + // L should be compressed to ~-13.5 dB (same as the steady-state test above). + Assert.That(leftPeakDb, Is.EqualTo(-13.5f).Within(1.5f), + "Sanity check: this test relies on L being compressed; if this fails the linked-gain expectation below is moot."); + + // The L-channel gain reduction (positive dB number) inferred from the measurement is + // applied to the R channel by linked-stereo design, so R should land at the same dB + // distance below its input peak. + float leftGainReductionDb = leftInputPeakDb - leftPeakDb; + float expectedRightDb = rightInputPeakDb - leftGainReductionDb; + Assert.That(rightPeakDb, Is.EqualTo(expectedRightDb).Within(1.5f)); + } + + [Test] + public void Process_EnvelopeStateContinuesAcrossChunks() + { + // A node warmed up by a previous loud chunk must NOT reset its envelope when the next + // chunk continues directly in time. We verify this by comparing the first sample of the + // second chunk against a fresh node processing the same loud input from scratch: + // the warmed-up node is already in compression so its first sample is quieter, while + // the fresh node still has to ramp through the attack phase. + const int chunkSamples = SampleRate / 10; + var chunkDuration = TimeSpan.FromSeconds(chunkSamples / (double)SampleRate); + var ctx1 = CreateContext(TimeSpan.Zero, chunkDuration); + var ctx2 = CreateContext(chunkDuration, chunkDuration); + + var nodeContinuing = CreateNode(release: 1000f); + using var warmupInput = CreateConstantBuffer(0.9f, chunkSamples); + nodeContinuing.AddInput(new StubSourceNode { Buffer = warmupInput }); + using var warmup = nodeContinuing.Process(ctx1); + nodeContinuing.ClearInputs(); + using var followInput = CreateConstantBuffer(0.9f, chunkSamples); + nodeContinuing.AddInput(new StubSourceNode { Buffer = followInput }); + using var followOutput = nodeContinuing.Process(ctx2); + + var nodeFresh = CreateNode(release: 1000f); + using var freshInput = CreateConstantBuffer(0.9f, chunkSamples); + nodeFresh.AddInput(new StubSourceNode { Buffer = freshInput }); + using var freshOutput = nodeFresh.Process(ctx1); + + float continuingFirst = MathF.Abs(followOutput.GetChannelData(0)[0]); + float freshFirst = MathF.Abs(freshOutput.GetChannelData(0)[0]); + Assert.That(continuingFirst, Is.LessThan(freshFirst)); + } + + [Test] + public void Process_NonContiguousTimeRange_ResetsEnvelope() + { + // First chunk drives compression; the second chunk starts at a non-contiguous time and + // must therefore reset the envelope so it begins fresh from MinDb. + const int chunkSamples = SampleRate / 10; + using var loud = CreateConstantBuffer(0.9f, chunkSamples); + + var node = CreateNode(release: 1000f); + node.AddInput(new StubSourceNode { Buffer = loud }); + var ctx1 = CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(chunkSamples / (double)SampleRate)); + using var firstOutput = node.Process(ctx1); + + node.ClearInputs(); + using var loud2 = CreateConstantBuffer(0.9f, chunkSamples); + node.AddInput(new StubSourceNode { Buffer = loud2 }); + // Start time jumps forward (seek), breaking contiguity. + var ctxSeek = CreateContext(TimeSpan.FromSeconds(5.0), TimeSpan.FromSeconds(chunkSamples / (double)SampleRate)); + using var seekedOutput = node.Process(ctxSeek); + + var nodeFresh = CreateNode(release: 1000f); + using var loud3 = CreateConstantBuffer(0.9f, chunkSamples); + nodeFresh.AddInput(new StubSourceNode { Buffer = loud3 }); + using var freshOutput = nodeFresh.Process( + CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(chunkSamples / (double)SampleRate))); + + // After a seek-style discontinuity, the envelope was reset, so the first sample should + // match a fresh node's first sample (within float tolerance). + float seekedFirst = MathF.Abs(seekedOutput.GetChannelData(0)[0]); + float freshFirst = MathF.Abs(freshOutput.GetChannelData(0)[0]); + Assert.That(seekedFirst, Is.EqualTo(freshFirst).Within(1e-4f)); + } + + [Test] + public void Process_SampleRateChange_ResetsEnvelope() + { + const int chunkSamples = SampleRate / 10; + using var loud = CreateConstantBuffer(0.9f, chunkSamples); + + var node = CreateNode(release: 1000f); + node.AddInput(new StubSourceNode { Buffer = loud }); + var ctx48 = CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(chunkSamples / (double)SampleRate)); + using var firstOutput = node.Process(ctx48); + + node.ClearInputs(); + const int altSampleRate = 44100; + using var loud44 = CreateSineBuffer(0.9f, 1000f, altSampleRate / 10, 2, altSampleRate); + node.AddInput(new StubSourceNode { Buffer = loud44 }); + // Time continues but sample rate changed → must reset envelope (and recompute coefficients + // for the new rate). + var ctx44 = new AudioProcessContext( + new TimeRange(TimeSpan.FromSeconds(chunkSamples / (double)SampleRate), TimeSpan.FromSeconds(0.1)), + altSampleRate, + new AnimationSampler(), + null); + using var secondOutput = node.Process(ctx44); + + // After a sample-rate switch the envelope is reset, so the first sample must match a + // fresh node running at the new rate. + var nodeFresh = CreateNode(release: 1000f); + using var freshInput = CreateSineBuffer(0.9f, 1000f, altSampleRate / 10, 2, altSampleRate); + nodeFresh.AddInput(new StubSourceNode { Buffer = freshInput }); + var ctxFresh = new AudioProcessContext( + new TimeRange(TimeSpan.Zero, TimeSpan.FromSeconds(0.1)), + altSampleRate, + new AnimationSampler(), + null); + using var freshOutput = nodeFresh.Process(ctxFresh); + + float secondFirst = MathF.Abs(secondOutput.GetChannelData(0)[0]); + float freshFirst = MathF.Abs(freshOutput.GetChannelData(0)[0]); + Assert.That(secondFirst, Is.EqualTo(freshFirst).Within(1e-4f)); + } + + [Test] + public void Reset_ClearsEnvelopeState() + { + // Process one chunk to drive the envelope into compression, then Reset() and process the + // next chunk at a *contiguous* time. Without Reset(), the time-range check would NOT + // trigger an automatic reset, so any difference from a fresh node must come from the + // explicit Reset() call. + const int chunkSamples = SampleRate / 10; + var chunkDuration = TimeSpan.FromSeconds(chunkSamples / (double)SampleRate); + var ctx1 = CreateContext(TimeSpan.Zero, chunkDuration); + var ctx2 = CreateContext(chunkDuration, chunkDuration); + + var node = CreateNode(release: 1000f); + using var warmupInput = CreateConstantBuffer(0.9f, chunkSamples); + node.AddInput(new StubSourceNode { Buffer = warmupInput }); + using var firstOutput = node.Process(ctx1); + + node.Reset(); + node.ClearInputs(); + using var followInput = CreateConstantBuffer(0.9f, chunkSamples); + node.AddInput(new StubSourceNode { Buffer = followInput }); + using var afterResetOutput = node.Process(ctx2); + + var nodeFresh = CreateNode(release: 1000f); + using var freshInput = CreateConstantBuffer(0.9f, chunkSamples); + nodeFresh.AddInput(new StubSourceNode { Buffer = freshInput }); + using var freshOutput = nodeFresh.Process(ctx1); + + Assert.That( + MathF.Abs(afterResetOutput.GetChannelData(0)[0]), + Is.EqualTo(MathF.Abs(freshOutput.GetChannelData(0)[0])).Within(1e-4f)); + } + + [Test] + public void Process_AnimatedThreshold_EngagesAnimatedPath() + { + // Threshold animates from -10 dB (no compression for 0.05 input) at t=0 to -40 dB + // (heavy compression for 0.05 input) at t=0.5s. The output should be louder near t=0 + // and quieter near t=0.5s, proving the animated path is exercised. const int sampleCount = SampleRate / 2; - var input = CreateSineBuffer(0.05f, 1000f, sampleCount); + using var input = CreateConstantBuffer(0.05f, sampleCount); var source = new StubSourceNode { Buffer = input }; + var thresholdAnim = new KeyFrameAnimation(); + thresholdAnim.KeyFrames.Add(new KeyFrame { Easing = new LinearEasing(), Value = -10f, KeyTime = TimeSpan.Zero }); + thresholdAnim.KeyFrames.Add(new KeyFrame { Easing = new LinearEasing(), Value = -40f, KeyTime = TimeSpan.FromSeconds(0.5) }); + + var thresholdProperty = Property.CreateAnimatable(-10f); + thresholdProperty.Animation = thresholdAnim; + var node = new CompressorNode { - Threshold = Property.CreateAnimatable(-20f), - Ratio = Property.CreateAnimatable(4f), - Attack = Property.CreateAnimatable(10f), - Release = Property.CreateAnimatable(100f), + Threshold = thresholdProperty, + Ratio = Property.CreateAnimatable(8f), + Attack = Property.CreateAnimatable(1f), + Release = Property.CreateAnimatable(50f), Knee = Property.CreateAnimatable(0f), MakeupGain = Property.CreateAnimatable(0f) }; node.AddInput(source); - var ctx = new AudioProcessContext( - new TimeRange(TimeSpan.Zero, TimeSpan.FromSeconds(0.5)), - SampleRate, - new AnimationSampler(), - null); + var ctx = CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(0.5)); - var output = node.Process(ctx); + using var output = node.Process(ctx); - float inputPeakDb = PeakDb(input, 0); - float outputPeakDb = PeakDb(output, sampleCount / 2); + int lastQuarterStart = sampleCount * 3 / 4; + float earlyPeakDb = PeakDb(output, 0); + float latePeakDb = PeakDb(output, lastQuarterStart); - Assert.That(outputPeakDb, Is.EqualTo(inputPeakDb).Within(0.5f)); + // Early threshold sits above the input (no compression); late threshold sits below it + // (compression engages). The late portion must therefore be measurably quieter. + Assert.That(latePeakDb, Is.LessThan(earlyPeakDb - 2f), + $"Animated threshold should attenuate the late portion (early≈{earlyPeakDb:F2} dB, late≈{latePeakDb:F2} dB)"); } [Test] - public void Process_AboveThreshold_AppliesExpectedGainReduction() + public void Process_InfinityInputSamples_RecoversAndDoesNotLeakNonFiniteOutput() + { + // First few samples on every channel are +Infinity, which (after MathF.Abs and Log10) + // produces inputDb = +Infinity, polluting the envelope state. Subsequent samples are a + // normal sine wave. The self-recovery clamp must reset the envelope and the output + // sanitizer must ensure no NaN/Infinity sample escapes downstream. + const int sampleCount = SampleRate / 4; + using var input = CreateSineBuffer(0.9f, 1000f, sampleCount); + for (int ch = 0; ch < input.ChannelCount; ch++) + { + var data = input.GetChannelData(ch); + data[0] = float.PositiveInfinity; + data[1] = float.PositiveInfinity; + } + + var source = new StubSourceNode { Buffer = input }; + var node = CreateNode(); + node.AddInput(source); + + var ctx = CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(0.25)); + using var output = node.Process(ctx); + + // No sample anywhere in the output may be NaN or Infinity. + for (int ch = 0; ch < output.ChannelCount; ch++) + { + var data = output.GetChannelData(ch); + for (int i = 0; i < output.SampleCount; i++) + { + Assert.That(float.IsFinite(data[i]), Is.True, + $"Output sample [{ch}][{i}] = {data[i]} is not finite"); + } + } + + // The steady-state region recovered to a sensible compressed level. + float steadyPeakDb = PeakDb(output, sampleCount / 2); + Assert.That(steadyPeakDb, Is.GreaterThan(-30f)); + Assert.That(steadyPeakDb, Is.LessThan(0f)); + } + + [Test] + public void Process_NaNInputSamples_ProducesFiniteOutput() + { + // A NaN input sample multiplied by any gain stays NaN. The output sanitizer must + // replace it with 0 so downstream consumers receive only finite samples. + const int sampleCount = SampleRate / 4; + using var input = CreateSineBuffer(0.9f, 1000f, sampleCount); + input.GetChannelData(0)[0] = float.NaN; + input.GetChannelData(1)[0] = float.NaN; + + var source = new StubSourceNode { Buffer = input }; + var node = CreateNode(); + node.AddInput(source); + + var ctx = CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(0.25)); + using var output = node.Process(ctx); + + Assert.That(output.GetChannelData(0)[0], Is.EqualTo(0f)); + Assert.That(output.GetChannelData(1)[0], Is.EqualTo(0f)); + // Subsequent samples remain finite. + for (int ch = 0; ch < output.ChannelCount; ch++) + { + var data = output.GetChannelData(ch); + for (int i = 1; i < output.SampleCount; i++) + { + Assert.That(float.IsFinite(data[i]), Is.True); + } + } + } + + [Test] + public void Process_SoftKnee_ProducesSmoothTransitionAroundThreshold() { - // Steady 0.9 amplitude (~-0.92 dB) sine, threshold -20 dB, ratio 4:1, hard knee. - // Steady-state gain reduction ≈ (-0.92 - -20) * (1 - 1/4) ≈ 14.3 dB. + // Soft knee starts attenuating before the input crosses the threshold; hard knee does + // not. We feed a sine right at the threshold and verify that soft-knee output is lower + // than hard-knee output, confirming the quadratic in-knee region engages. + const int sampleCount = SampleRate / 2; + // 0.1 amplitude → exactly -20 dB peak, matching the threshold. + using var input = CreateSineBuffer(0.1f, 1000f, sampleCount); + + var hardKneeNode = CreateNode(threshold: -20f, ratio: 4f, knee: 0f); + hardKneeNode.AddInput(new StubSourceNode { Buffer = input }); + using var hardOutput = hardKneeNode.Process(CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(0.5))); + + var softKneeNode = CreateNode(threshold: -20f, ratio: 4f, knee: 12f); + softKneeNode.AddInput(new StubSourceNode { Buffer = input }); + using var softOutput = softKneeNode.Process(CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(0.5))); + + int steadyStart = sampleCount / 2; + float hardPeakDb = PeakDb(hardOutput, steadyStart); + float softPeakDb = PeakDb(softOutput, steadyStart); + + // Soft knee already engages before the input crosses the threshold, so its output peak + // should be measurably lower than the hard-knee output. + Assert.That(softPeakDb, Is.LessThan(hardPeakDb - 0.3f), + $"Soft knee should attenuate near threshold (hard≈{hardPeakDb:F2} dB, soft≈{softPeakDb:F2} dB)"); + } + + [Test] + public void Process_MonoBuffer_ProducesExpectedGainReduction() + { + // The implementation iterates over the channel count; a single-channel buffer must work + // with no off-by-one and reach the same compression level as the stereo case. const int sampleCount = SampleRate; - var input = CreateSineBuffer(0.9f, 1000f, sampleCount); + using var input = CreateSineBuffer(0.9f, 1000f, sampleCount, channels: 1); var source = new StubSourceNode { Buffer = input }; + var node = CreateNode(); + node.AddInput(source); + + var ctx = CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(1.0)); + using var output = node.Process(ctx); + + Assert.That(output.ChannelCount, Is.EqualTo(1)); + float steadyPeakDb = PeakDb(output, sampleCount / 2); + Assert.That(steadyPeakDb, Is.EqualTo(-13.5f).Within(1.5f)); + } + + [Test] + public void Process_NoInputs_Throws() + { + var node = CreateNode(); + var ctx = CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(0.1)); + Assert.Throws(() => node.Process(ctx)); + } + + [Test] + public void Process_AnimatedAttackRelease_ExercisesCoefficientCache() + { + // Animate Attack from 1 ms (fast) to 200 ms (slow) over the buffer. Every new ms value + // invalidates the per-sample coefficient cache (`attackMs != lastAttackMs`), so + // ComputeCoeff is recomputed repeatedly. To verify the recomputed coefficients actually + // affect behaviour, we feed a step input (silence → loud) that arrives late in the + // buffer when the slow attack value is in effect. Right after the step the envelope + // hasn't clamped yet, so the transient peak must be louder than the eventually-settled + // tail. With attack stuck at 1 ms throughout, the transient would clamp instantly and + // this difference would not appear. + const int sampleCount = SampleRate; // 1 s buffer + int stepAt = SampleRate * 4 / 10; // 400 ms in: animated attack ≈ 80 ms + using var input = new AudioBuffer(SampleRate, 2, sampleCount); + for (int ch = 0; ch < 2; ch++) + { + var data = input.GetChannelData(ch); + for (int i = stepAt; i < sampleCount; i++) + { + data[i] = 0.9f * MathF.Sin(2f * MathF.PI * 1000f * i / SampleRate); + } + } + + var attackAnim = new KeyFrameAnimation(); + attackAnim.KeyFrames.Add(new KeyFrame { Easing = new LinearEasing(), Value = 1f, KeyTime = TimeSpan.Zero }); + attackAnim.KeyFrames.Add(new KeyFrame { Easing = new LinearEasing(), Value = 200f, KeyTime = TimeSpan.FromSeconds(1.0) }); + var attackProperty = Property.CreateAnimatable(1f); + attackProperty.Animation = attackAnim; + var node = new CompressorNode { Threshold = Property.CreateAnimatable(-20f), Ratio = Property.CreateAnimatable(4f), - Attack = Property.CreateAnimatable(5f), + Attack = attackProperty, Release = Property.CreateAnimatable(50f), Knee = Property.CreateAnimatable(0f), MakeupGain = Property.CreateAnimatable(0f) }; - node.AddInput(source); + node.AddInput(new StubSourceNode { Buffer = input }); - var ctx = new AudioProcessContext( - new TimeRange(TimeSpan.Zero, TimeSpan.FromSeconds(1.0)), - SampleRate, - new AnimationSampler(), - null); + var ctx = CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(1.0)); + using var output = node.Process(ctx); - var output = node.Process(ctx); + for (int ch = 0; ch < output.ChannelCount; ch++) + { + var data = output.GetChannelData(ch); + for (int i = 0; i < output.SampleCount; i++) + { + Assert.That(float.IsFinite(data[i]), Is.True); + } + } - // After the attack settles, the signal should sit around -15 dB. - float steadyStartSample = SampleRate / 2; - float outputPeakDb = PeakDb(output, (int)steadyStartSample); + int probeWidth = SampleRate / 200; // 5 ms + float transientPeakDb = PeakDbInWindow(output, stepAt, probeWidth); + float settledPeakDb = PeakDbInWindow(output, sampleCount - probeWidth, probeWidth); - Assert.That(outputPeakDb, Is.LessThan(-12f)); - Assert.That(outputPeakDb, Is.GreaterThan(-18f)); + Assert.That(transientPeakDb, Is.GreaterThan(settledPeakDb + 1.0f), + $"Slow attack should leave a louder transient than the settled tail (transient≈{transientPeakDb:F2} dB, settled≈{settledPeakDb:F2} dB)"); } [Test] - public void Process_MakeupGain_RaisesOutputAboveReducedLevel() + public void Process_AnimatedPath_SmoothAcrossChunkBoundary() { - const int sampleCount = SampleRate; - var input = CreateSineBuffer(0.9f, 1000f, sampleCount); - var source = new StubSourceNode { Buffer = input }; + // ProcessAnimated walks the input in fixed-size chunks. We send a buffer that straddles + // several chunk boundaries and verify that the envelope state survives them: a steady + // input must not show any visible discontinuity at sample indices that align with the + // chunk size, which would indicate the envelope was reset at the boundary. + const int chunkSize = 1024; + const int sampleCount = chunkSize * 3 + 137; // straddles several boundaries + using var input = CreateConstantBuffer(0.9f, sampleCount); + + // Animate threshold trivially so ProcessAnimated is taken; the value stays the same so + // the gain reduction itself should be smooth. + var thresholdAnim = new KeyFrameAnimation(); + thresholdAnim.KeyFrames.Add(new KeyFrame { Easing = new LinearEasing(), Value = -20f, KeyTime = TimeSpan.Zero }); + thresholdAnim.KeyFrames.Add(new KeyFrame { Easing = new LinearEasing(), Value = -20f, KeyTime = TimeSpan.FromSeconds(sampleCount / (double)SampleRate) }); + var thresholdProperty = Property.CreateAnimatable(-20f); + thresholdProperty.Animation = thresholdAnim; var node = new CompressorNode { - Threshold = Property.CreateAnimatable(-20f), + Threshold = thresholdProperty, Ratio = Property.CreateAnimatable(4f), Attack = Property.CreateAnimatable(5f), Release = Property.CreateAnimatable(50f), Knee = Property.CreateAnimatable(0f), - MakeupGain = Property.CreateAnimatable(6f) + MakeupGain = Property.CreateAnimatable(0f) }; - node.AddInput(source); + node.AddInput(new StubSourceNode { Buffer = input }); - var ctx = new AudioProcessContext( - new TimeRange(TimeSpan.Zero, TimeSpan.FromSeconds(1.0)), - SampleRate, - new AnimationSampler(), - null); + var ctx = CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(sampleCount / (double)SampleRate)); + using var output = node.Process(ctx); - var output = node.Process(ctx); + // Across each chunk boundary, the absolute change between adjacent samples must remain + // bounded by the change inside the previous window — i.e. no sudden jump caused by + // resetting state at a boundary. + var data = output.GetChannelData(0); + for (int boundary = chunkSize; boundary < sampleCount; boundary += chunkSize) + { + float prevDelta = MathF.Abs(data[boundary - 1] - data[boundary - 2]); + float boundaryDelta = MathF.Abs(data[boundary] - data[boundary - 1]); + // Tolerance allows for a tiny natural variation in the sine-like product but rejects + // an envelope reset (which would create a step of order 0.1 or larger here). + Assert.That(boundaryDelta, Is.LessThanOrEqualTo(prevDelta + 0.01f), + $"Discontinuity at chunk boundary {boundary}: prevDelta={prevDelta:F6}, boundaryDelta={boundaryDelta:F6}"); + } + } - float steadyStartSample = SampleRate / 2; - float outputPeakDb = PeakDb(output, (int)steadyStartSample); + public enum AnimatedParam { Threshold, Ratio, Attack, Release, Knee, MakeupGain } + + [TestCase(AnimatedParam.Threshold)] + [TestCase(AnimatedParam.Ratio)] + [TestCase(AnimatedParam.Attack)] + [TestCase(AnimatedParam.Release)] + [TestCase(AnimatedParam.Knee)] + [TestCase(AnimatedParam.MakeupGain)] + public void Process_AnimatedNonFiniteValue_FallsBackWithoutMutingOutput(AnimatedParam param) + { + // A KeyFrame with NaN or Infinity on any animated parameter must not propagate to the + // output sanitizer (which would silently mute the entire chunk). Instead each parameter + // must fall back to its DefaultValue. We test every animated parameter so the + // SafeParameter call cannot be silently dropped from any one of them. + const int sampleCount = SampleRate / 4; + using var input = CreateSineBuffer(0.9f, 1000f, sampleCount); + + var threshold = Property.CreateAnimatable(-20f); + var ratio = Property.CreateAnimatable(4f); + var attack = Property.CreateAnimatable(5f); + var release = Property.CreateAnimatable(50f); + var knee = Property.CreateAnimatable(0f); + var makeup = Property.CreateAnimatable(0f); + + IProperty target = param switch + { + AnimatedParam.Threshold => threshold, + AnimatedParam.Ratio => ratio, + AnimatedParam.Attack => attack, + AnimatedParam.Release => release, + AnimatedParam.Knee => knee, + AnimatedParam.MakeupGain => makeup, + _ => throw new ArgumentOutOfRangeException(nameof(param)) + }; + var anim = new KeyFrameAnimation(); + anim.KeyFrames.Add(new KeyFrame { Easing = new LinearEasing(), Value = float.NaN, KeyTime = TimeSpan.Zero }); + anim.KeyFrames.Add(new KeyFrame { Easing = new LinearEasing(), Value = float.NaN, KeyTime = TimeSpan.FromSeconds(0.25) }); + ((AnimatableProperty)target).Animation = anim; + + var node = new CompressorNode + { + Threshold = threshold, + Ratio = ratio, + Attack = attack, + Release = release, + Knee = knee, + MakeupGain = makeup + }; + node.AddInput(new StubSourceNode { Buffer = input }); + + var ctx = CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(0.25)); + using var output = node.Process(ctx); - // 6 dB makeup applied to ~-15 dB → ~-9 dB. - Assert.That(outputPeakDb, Is.LessThan(-6f)); - Assert.That(outputPeakDb, Is.GreaterThan(-12f)); + // If the NaN had reached the gain calc the output sanitizer would have zeroed every + // sample and the peak would be -100 dB. Anything well above that proves the fallback + // engaged for the parameter under test. + float steadyPeakDb = PeakDb(output, sampleCount / 2); + Assert.That(steadyPeakDb, Is.GreaterThan(-25f), + $"Fallback failed for {param}: output appears to have been zeroed by NaN propagation"); + } + + [Test] + public void Process_TooManyInputs_Throws() + { + const int sampleCount = SampleRate / 10; + using var bufA = CreateConstantBuffer(0.1f, sampleCount); + using var bufB = CreateConstantBuffer(0.1f, sampleCount); + var node = CreateNode(); + node.AddInput(new StubSourceNode { Buffer = bufA }); + node.AddInput(new StubSourceNode { Buffer = bufB }); + var ctx = CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(0.1)); + Assert.Throws(() => node.Process(ctx)); } } From d311f7b7d4912eedf759b1543711d2b10447082b Mon Sep 17 00:00:00 2001 From: Yuto Terada Date: Sun, 10 May 2026 16:41:07 +0900 Subject: [PATCH 5/8] fix: clamp animated compressor parameters to declared range Animated values bypass the [Range] coercion that runs on the static setter, so an animated Attack of 1e9 ms (or any other overshoot) would silently freeze the envelope. Centralize the bounds in a new CompressorParameters class so the effect's [Range] and the node's clamps share one source of truth, and sanitize each per-sample read through a Math.Clamp(SafeParameter(...)) helper. Also: track non-finite logging per parameter (was a single shared latch that hid follow-up errors on other parameters), make Reset() internal so external callers cannot zero the envelope mid-buffer, and early-return on zero-length input. Tests cover all of the above plus an animated MakeupGain sweep regression. --- .../Audio/Effects/CompressorEffect.cs | 26 +- .../Audio/Effects/CompressorParameters.cs | 31 +++ .../Audio/Graph/Nodes/CompressorNode.cs | 127 ++++++---- .../Engine/Audio/CompressorNodeTests.cs | 224 ++++++++++++++++++ 4 files changed, 351 insertions(+), 57 deletions(-) create mode 100644 src/Beutl.Engine/Audio/Effects/CompressorParameters.cs diff --git a/src/Beutl.Engine/Audio/Effects/CompressorEffect.cs b/src/Beutl.Engine/Audio/Effects/CompressorEffect.cs index ca7c3a597..463eeef2c 100644 --- a/src/Beutl.Engine/Audio/Effects/CompressorEffect.cs +++ b/src/Beutl.Engine/Audio/Effects/CompressorEffect.cs @@ -4,6 +4,8 @@ using Beutl.Engine; using Beutl.Language; +using static Beutl.Audio.Effects.CompressorParameters; + namespace Beutl.Audio.Effects; [Display(Name = nameof(AudioStrings.CompressorEffect), ResourceType = typeof(AudioStrings))] @@ -14,35 +16,35 @@ public CompressorEffect() ScanProperties(); } - [Range(-60f, 0f)] + [Range(MinThresholdDb, MaxThresholdDb)] [Display(Name = nameof(AudioStrings.CompressorEffect_Threshold), ResourceType = typeof(AudioStrings))] [SuppressResourceClassGeneration] - public IProperty Threshold { get; } = Property.CreateAnimatable(-20f); + public IProperty Threshold { get; } = Property.CreateAnimatable(DefaultThresholdDb); - [Range(1f, 20f)] + [Range(MinRatio, MaxRatio)] [Display(Name = nameof(AudioStrings.CompressorEffect_Ratio), ResourceType = typeof(AudioStrings))] [SuppressResourceClassGeneration] - public IProperty Ratio { get; } = Property.CreateAnimatable(4f); + public IProperty Ratio { get; } = Property.CreateAnimatable(DefaultRatio); - [Range(0.1f, 500f)] + [Range(MinAttackMs, MaxAttackMs)] [Display(Name = nameof(AudioStrings.CompressorEffect_Attack), ResourceType = typeof(AudioStrings))] [SuppressResourceClassGeneration] - public IProperty Attack { get; } = Property.CreateAnimatable(10f); + public IProperty Attack { get; } = Property.CreateAnimatable(DefaultAttackMs); - [Range(1f, 5000f)] + [Range(MinReleaseMs, MaxReleaseMs)] [Display(Name = nameof(AudioStrings.CompressorEffect_Release), ResourceType = typeof(AudioStrings))] [SuppressResourceClassGeneration] - public IProperty Release { get; } = Property.CreateAnimatable(100f); + public IProperty Release { get; } = Property.CreateAnimatable(DefaultReleaseMs); - [Range(0f, 24f)] + [Range(MinKneeDb, MaxKneeDb)] [Display(Name = nameof(AudioStrings.CompressorEffect_Knee), ResourceType = typeof(AudioStrings))] [SuppressResourceClassGeneration] - public IProperty Knee { get; } = Property.CreateAnimatable(6f); + public IProperty Knee { get; } = Property.CreateAnimatable(DefaultKneeDb); - [Range(-24f, 24f)] + [Range(MinMakeupGainDb, MaxMakeupGainDb)] [Display(Name = nameof(AudioStrings.CompressorEffect_MakeupGain), ResourceType = typeof(AudioStrings))] [SuppressResourceClassGeneration] - public IProperty MakeupGain { get; } = Property.CreateAnimatable(0f); + public IProperty MakeupGain { get; } = Property.CreateAnimatable(DefaultMakeupGainDb); public override AudioNode CreateNode(AudioContext context, AudioNode inputNode) { diff --git a/src/Beutl.Engine/Audio/Effects/CompressorParameters.cs b/src/Beutl.Engine/Audio/Effects/CompressorParameters.cs new file mode 100644 index 000000000..f8705d2cc --- /dev/null +++ b/src/Beutl.Engine/Audio/Effects/CompressorParameters.cs @@ -0,0 +1,31 @@ +namespace Beutl.Audio.Effects; + +// Single source of truth for the compressor's parameter ranges and defaults. CompressorEffect +// references these in its [Range] / Property.CreateAnimatable declarations and CompressorNode +// references the same values when clamping per-sample animated inputs, so the two cannot drift. +internal static class CompressorParameters +{ + public const float MinThresholdDb = -60f; + public const float MaxThresholdDb = 0f; + public const float DefaultThresholdDb = -20f; + + public const float MinRatio = 1f; + public const float MaxRatio = 20f; + public const float DefaultRatio = 4f; + + public const float MinAttackMs = 0.1f; + public const float MaxAttackMs = 500f; + public const float DefaultAttackMs = 10f; + + public const float MinReleaseMs = 1f; + public const float MaxReleaseMs = 5000f; + public const float DefaultReleaseMs = 100f; + + public const float MinKneeDb = 0f; + public const float MaxKneeDb = 24f; + public const float DefaultKneeDb = 6f; + + public const float MinMakeupGainDb = -24f; + public const float MaxMakeupGainDb = 24f; + public const float DefaultMakeupGainDb = 0f; +} diff --git a/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs b/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs index 89c8025a3..da35f5a3f 100644 --- a/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs +++ b/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs @@ -1,8 +1,11 @@ -using Beutl.Engine; +using Beutl.Audio.Effects; +using Beutl.Engine; using Beutl.Logging; using Beutl.Media; using Microsoft.Extensions.Logging; +using static Beutl.Audio.Effects.CompressorParameters; + namespace Beutl.Audio.Graph.Nodes; public sealed class CompressorNode : AudioNode @@ -11,13 +14,6 @@ public sealed class CompressorNode : AudioNode private const float MinDb = -100f; - // Lower bounds match the [Range] attributes declared on CompressorEffect. Keeping them in sync - // avoids silent clamping when animation values overshoot the declared range. - private const float MinAttackMs = 0.1f; - private const float MinReleaseMs = 1f; - private const float MinRatio = 1f; - private const float MinKneeDb = 0f; - private float _envelopeDb = MinDb; private int _lastSampleRate; private TimeSpan? _lastTimeRangeEnd; @@ -26,7 +22,9 @@ public sealed class CompressorNode : AudioNode // when the corruption persists across thousands of samples. private bool _loggedNonFiniteEnvelope; private bool _loggedNonFiniteSample; - private bool _loggedNonFiniteParameter; + // Per-parameter so a non-finite sample on one parameter (e.g. Attack) does not silently + // suppress diagnostics for an unrelated parameter (e.g. Threshold) later in the stream. + private readonly HashSet _loggedNonFiniteParameters = new(); public required IProperty Threshold { get; init; } @@ -63,6 +61,11 @@ public override AudioBuffer Process(AudioProcessContext context) } _lastTimeRangeEnd = context.TimeRange.Start + context.TimeRange.Duration; + if (input.SampleCount == 0) + { + return new AudioBuffer(input.SampleRate, input.ChannelCount, 0); + } + bool hasAnimation = Threshold.Animation != null || Ratio.Animation != null || Attack.Animation != null || @@ -82,17 +85,12 @@ private AudioBuffer ProcessStatic(AudioBuffer input, AudioProcessContext context { var output = new AudioBuffer(input.SampleRate, input.ChannelCount, input.SampleCount); - float threshold = SafeParameter(Threshold.CurrentValue, Threshold.DefaultValue, nameof(Threshold)); - float ratio = MathF.Max(MinRatio, SafeParameter(Ratio.CurrentValue, Ratio.DefaultValue, nameof(Ratio))); - float attackMs = MathF.Max(MinAttackMs, SafeParameter(Attack.CurrentValue, Attack.DefaultValue, nameof(Attack))); - float releaseMs = MathF.Max(MinReleaseMs, SafeParameter(Release.CurrentValue, Release.DefaultValue, nameof(Release))); - float knee = MathF.Max(MinKneeDb, SafeParameter(Knee.CurrentValue, Knee.DefaultValue, nameof(Knee))); - float makeupGain = SafeParameter(MakeupGain.CurrentValue, MakeupGain.DefaultValue, nameof(MakeupGain)); + EffectiveParameters p = ReadStaticParameters(); - float attackCoeff = ComputeCoeff(attackMs, context.SampleRate); - float releaseCoeff = ComputeCoeff(releaseMs, context.SampleRate); - float slope = 1f - 1f / ratio; - float makeupLinear = AudioMath.ConvertDbToLinear(makeupGain); + float attackCoeff = ComputeCoeff(p.Attack, context.SampleRate); + float releaseCoeff = ComputeCoeff(p.Release, context.SampleRate); + float slope = 1f - 1f / p.Ratio; + float makeupLinear = AudioMath.ConvertDbToLinear(p.MakeupGain); int channels = input.ChannelCount; for (int i = 0; i < input.SampleCount; i++) @@ -109,7 +107,7 @@ private AudioBuffer ProcessStatic(AudioBuffer input, AudioProcessContext context _envelopeDb = inputDb + coeff * (_envelopeDb - inputDb); RecoverEnvelopeIfNonFinite(); - float gainReductionDb = ComputeGainReductionDb(_envelopeDb, threshold, knee, slope); + float gainReductionDb = ComputeGainReductionDb(_envelopeDb, p.Threshold, p.Knee, slope); float gainLinear = AudioMath.ConvertDbToLinear(-gainReductionDb) * makeupLinear; for (int ch = 0; ch < channels; ch++) @@ -138,12 +136,7 @@ private AudioBuffer ProcessAnimated(AudioBuffer input, AudioProcessContext conte // Final fallbacks used when an animated parameter samples to NaN/Infinity (e.g. malformed // KeyFrame). Without this guard, a single non-finite animated value would propagate into // the gain calculation and cause the output sanitizer to silently zero out every sample. - float thresholdFallback = SafeParameter(Threshold.CurrentValue, Threshold.DefaultValue, nameof(Threshold)); - float ratioFallback = MathF.Max(MinRatio, SafeParameter(Ratio.CurrentValue, Ratio.DefaultValue, nameof(Ratio))); - float attackFallback = MathF.Max(MinAttackMs, SafeParameter(Attack.CurrentValue, Attack.DefaultValue, nameof(Attack))); - float releaseFallback = MathF.Max(MinReleaseMs, SafeParameter(Release.CurrentValue, Release.DefaultValue, nameof(Release))); - float kneeFallback = MathF.Max(MinKneeDb, SafeParameter(Knee.CurrentValue, Knee.DefaultValue, nameof(Knee))); - float makeupFallback = SafeParameter(MakeupGain.CurrentValue, MakeupGain.DefaultValue, nameof(MakeupGain)); + EffectiveParameters fallback = ReadStaticParameters(); int channels = input.ChannelCount; int processed = 0; @@ -183,31 +176,27 @@ private AudioBuffer ProcessAnimated(AudioBuffer input, AudioProcessContext conte float inputDb = peak > 0f ? 20f * MathF.Log10(peak) : MinDb; - float threshold = SafeParameter(thresholds[i], thresholdFallback, nameof(Threshold)); - float ratio = MathF.Max(MinRatio, SafeParameter(ratios[i], ratioFallback, nameof(Ratio))); - float attackMs = MathF.Max(MinAttackMs, SafeParameter(attacks[i], attackFallback, nameof(Attack))); - float releaseMs = MathF.Max(MinReleaseMs, SafeParameter(releases[i], releaseFallback, nameof(Release))); - float knee = MathF.Max(MinKneeDb, SafeParameter(knees[i], kneeFallback, nameof(Knee))); - float makeupDb = SafeParameter(makeups[i], makeupFallback, nameof(MakeupGain)); + EffectiveParameters p = SanitizeAnimated( + thresholds[i], ratios[i], attacks[i], releases[i], knees[i], makeups[i], fallback); - if (attackMs != lastAttackMs) + if (p.Attack != lastAttackMs) { - attackCoeff = ComputeCoeff(attackMs, context.SampleRate); - lastAttackMs = attackMs; + attackCoeff = ComputeCoeff(p.Attack, context.SampleRate); + lastAttackMs = p.Attack; } - if (releaseMs != lastReleaseMs) + if (p.Release != lastReleaseMs) { - releaseCoeff = ComputeCoeff(releaseMs, context.SampleRate); - lastReleaseMs = releaseMs; + releaseCoeff = ComputeCoeff(p.Release, context.SampleRate); + lastReleaseMs = p.Release; } - float slope = 1f - 1f / ratio; + float slope = 1f - 1f / p.Ratio; float coeff = inputDb > _envelopeDb ? attackCoeff : releaseCoeff; _envelopeDb = inputDb + coeff * (_envelopeDb - inputDb); RecoverEnvelopeIfNonFinite(); - float gainReductionDb = ComputeGainReductionDb(_envelopeDb, threshold, knee, slope); - float gainLinear = AudioMath.ConvertDbToLinear(makeupDb - gainReductionDb); + float gainReductionDb = ComputeGainReductionDb(_envelopeDb, p.Threshold, p.Knee, slope); + float gainLinear = AudioMath.ConvertDbToLinear(p.MakeupGain - gainReductionDb); for (int ch = 0; ch < channels; ch++) { @@ -222,6 +211,43 @@ private AudioBuffer ProcessAnimated(AudioBuffer input, AudioProcessContext conte return output; } + private EffectiveParameters ReadStaticParameters() + { + return new EffectiveParameters + { + Threshold = Sanitize(Threshold.CurrentValue, Threshold.DefaultValue, MinThresholdDb, MaxThresholdDb, nameof(Threshold)), + Ratio = Sanitize(Ratio.CurrentValue, Ratio.DefaultValue, MinRatio, MaxRatio, nameof(Ratio)), + Attack = Sanitize(Attack.CurrentValue, Attack.DefaultValue, MinAttackMs, MaxAttackMs, nameof(Attack)), + Release = Sanitize(Release.CurrentValue, Release.DefaultValue, MinReleaseMs, MaxReleaseMs, nameof(Release)), + Knee = Sanitize(Knee.CurrentValue, Knee.DefaultValue, MinKneeDb, MaxKneeDb, nameof(Knee)), + MakeupGain = Sanitize(MakeupGain.CurrentValue, MakeupGain.DefaultValue, MinMakeupGainDb, MaxMakeupGainDb, nameof(MakeupGain)), + }; + } + + private EffectiveParameters SanitizeAnimated( + float threshold, float ratio, float attack, float release, float knee, float makeup, + in EffectiveParameters fallback) + { + return new EffectiveParameters + { + Threshold = Sanitize(threshold, fallback.Threshold, MinThresholdDb, MaxThresholdDb, nameof(Threshold)), + Ratio = Sanitize(ratio, fallback.Ratio, MinRatio, MaxRatio, nameof(Ratio)), + Attack = Sanitize(attack, fallback.Attack, MinAttackMs, MaxAttackMs, nameof(Attack)), + Release = Sanitize(release, fallback.Release, MinReleaseMs, MaxReleaseMs, nameof(Release)), + Knee = Sanitize(knee, fallback.Knee, MinKneeDb, MaxKneeDb, nameof(Knee)), + MakeupGain = Sanitize(makeup, fallback.MakeupGain, MinMakeupGainDb, MaxMakeupGainDb, nameof(MakeupGain)), + }; + } + + // Substitute fallback for NaN/Infinity, then clamp to the parameter's declared [Range]. + // The clamp is what guards against an animated value silently bypassing the [Range] declaration + // on CompressorEffect — without it, e.g. an animated Attack of 1e9 ms would freeze the + // envelope without any diagnostic. + private float Sanitize(float value, float fallback, float min, float max, string paramName) + { + return Math.Clamp(SafeParameter(value, fallback, paramName), min, max); + } + // Without this guard, a single non-finite envelope sample would permanently poison the state // until the next Reset(). The first occurrence is logged; subsequent ones are suppressed so // the audio thread is not spammed. @@ -239,12 +265,11 @@ private void RecoverEnvelopeIfNonFinite() private float SafeParameter(float value, float fallback, string paramName) { if (float.IsFinite(value)) return value; - if (!_loggedNonFiniteParameter) + if (_loggedNonFiniteParameters.Add(paramName)) { s_logger.LogWarning( - "Compressor parameter '{Param}' produced a non-finite value; falling back to {Fallback}. Further occurrences will be suppressed.", + "Compressor parameter '{Param}' produced a non-finite value; falling back to {Fallback}. Further occurrences for this parameter will be suppressed.", paramName, fallback); - _loggedNonFiniteParameter = true; } return fallback; } @@ -287,8 +312,20 @@ private static float ComputeGainReductionDb(float envelopeDb, float thresholdDb, return envelopeDb > thresholdDb ? slope * (envelopeDb - thresholdDb) : 0f; } - public void Reset() + // Internal so tests can drive an explicit reset, but not part of the public API: external + // callers must not zero the envelope mid-buffer (it would produce an audible click). + internal void Reset() { _envelopeDb = MinDb; } + + private struct EffectiveParameters + { + public float Threshold; + public float Ratio; + public float Attack; + public float Release; + public float Knee; + public float MakeupGain; + } } diff --git a/tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs b/tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs index f44151e84..5f1376833 100644 --- a/tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs +++ b/tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs @@ -769,4 +769,228 @@ public void Process_TooManyInputs_Throws() var ctx = CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(0.1)); Assert.Throws(() => node.Process(ctx)); } + + [Test] + public void Process_ZeroLengthInput_Static_ReturnsEmptyBuffer() + { + // A zero-length chunk (silent gap, end-of-stream tail) must not divide by zero, allocate + // a stackalloc[0] for animation buffers, or otherwise misbehave. The static path is + // exercised because no parameters are animated. + using var input = new AudioBuffer(SampleRate, 2, 0); + var node = CreateNode(); + node.AddInput(new StubSourceNode { Buffer = input }); + var ctx = CreateContext(TimeSpan.Zero, TimeSpan.Zero); + + using var output = node.Process(ctx); + + Assert.That(output.SampleCount, Is.EqualTo(0)); + Assert.That(output.ChannelCount, Is.EqualTo(2)); + Assert.That(output.SampleRate, Is.EqualTo(SampleRate)); + } + + [Test] + public void Process_ZeroLengthInput_Animated_ReturnsEmptyBuffer() + { + // Same as above but on the animated path: a stackalloc[0] would otherwise be allocated + // and the chunk loop must handle SampleCount == 0 cleanly. + using var input = new AudioBuffer(SampleRate, 2, 0); + + var thresholdAnim = new KeyFrameAnimation(); + thresholdAnim.KeyFrames.Add(new KeyFrame { Easing = new LinearEasing(), Value = -20f, KeyTime = TimeSpan.Zero }); + thresholdAnim.KeyFrames.Add(new KeyFrame { Easing = new LinearEasing(), Value = -10f, KeyTime = TimeSpan.FromSeconds(1.0) }); + var thresholdProperty = Property.CreateAnimatable(-20f); + thresholdProperty.Animation = thresholdAnim; + + var node = new CompressorNode + { + Threshold = thresholdProperty, + Ratio = Property.CreateAnimatable(4f), + Attack = Property.CreateAnimatable(5f), + Release = Property.CreateAnimatable(50f), + Knee = Property.CreateAnimatable(0f), + MakeupGain = Property.CreateAnimatable(0f) + }; + node.AddInput(new StubSourceNode { Buffer = input }); + + var ctx = CreateContext(TimeSpan.Zero, TimeSpan.Zero); + using var output = node.Process(ctx); + + Assert.That(output.SampleCount, Is.EqualTo(0)); + Assert.That(output.ChannelCount, Is.EqualTo(2)); + } + + [Test] + public void Process_AnimatedMakeupGain_AppliesPerSampleGain() + { + // Sweep MakeupGain from 0 dB (start) to +12 dB (end) over a steady loud signal. The + // tail of the buffer must measure ~12 dB louder than the head — proving that the + // animated `makeupDb - gainReductionDb` path actually mixes the per-sample makeup value + // into the output (and that the sign is correct). + const int sampleCount = SampleRate; // 1 s buffer + using var input = CreateSineBuffer(0.9f, 1000f, sampleCount); + + var makeupAnim = new KeyFrameAnimation(); + makeupAnim.KeyFrames.Add(new KeyFrame { Easing = new LinearEasing(), Value = 0f, KeyTime = TimeSpan.Zero }); + makeupAnim.KeyFrames.Add(new KeyFrame { Easing = new LinearEasing(), Value = 12f, KeyTime = TimeSpan.FromSeconds(1.0) }); + var makeupProperty = Property.CreateAnimatable(0f); + makeupProperty.Animation = makeupAnim; + + var node = new CompressorNode + { + Threshold = Property.CreateAnimatable(-20f), + Ratio = Property.CreateAnimatable(4f), + Attack = Property.CreateAnimatable(5f), + Release = Property.CreateAnimatable(50f), + Knee = Property.CreateAnimatable(0f), + MakeupGain = makeupProperty + }; + node.AddInput(new StubSourceNode { Buffer = input }); + + var ctx = CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(1.0)); + using var output = node.Process(ctx); + + // Compare a steady-state window early in the buffer (makeup ≈ 0 dB) against one near + // the end (makeup ≈ +12 dB). Both are after the attack ramp so envelope state is steady. + int probeWidth = SampleRate / 100; // 10 ms + float earlyPeakDb = PeakDbInWindow(output, SampleRate / 4, probeWidth); + float latePeakDb = PeakDbInWindow(output, sampleCount - probeWidth, probeWidth); + + float observedRise = latePeakDb - earlyPeakDb; + // 12 dB nominal; tolerance allows for ~3 dB of drift due to envelope ripple and the + // 25%→100% sweep range covering 9 dB rather than the full 12 dB. + Assert.That(observedRise, Is.GreaterThan(6f), + $"Animated MakeupGain should raise output level (early≈{earlyPeakDb:F2} dB, late≈{latePeakDb:F2} dB, rise≈{observedRise:F2} dB)"); + Assert.That(observedRise, Is.LessThan(15f), + $"Animated MakeupGain rise is implausibly large (early≈{earlyPeakDb:F2} dB, late≈{latePeakDb:F2} dB)"); + } + + [Test] + public void Process_AnimatedRatio_BelowOne_ClampsToPassthrough() + { + // The animated path's clamp must mirror the static path: an animated ratio of 0.5 + // would otherwise produce slope = 1 - 1/0.5 = -1 which AMPLIFIES above the threshold. + // After clamping to MinRatio=1, slope = 0 → passthrough. + const int sampleCount = SampleRate / 4; + using var input = CreateSineBuffer(0.9f, 1000f, sampleCount); + + var ratioAnim = new KeyFrameAnimation(); + ratioAnim.KeyFrames.Add(new KeyFrame { Easing = new LinearEasing(), Value = 0.5f, KeyTime = TimeSpan.Zero }); + ratioAnim.KeyFrames.Add(new KeyFrame { Easing = new LinearEasing(), Value = 0.5f, KeyTime = TimeSpan.FromSeconds(0.25) }); + var ratioProperty = Property.CreateAnimatable(0.5f); + ratioProperty.Animation = ratioAnim; + + var node = new CompressorNode + { + Threshold = Property.CreateAnimatable(-20f), + Ratio = ratioProperty, + Attack = Property.CreateAnimatable(5f), + Release = Property.CreateAnimatable(50f), + Knee = Property.CreateAnimatable(0f), + MakeupGain = Property.CreateAnimatable(0f) + }; + node.AddInput(new StubSourceNode { Buffer = input }); + + var ctx = CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(0.25)); + using var output = node.Process(ctx); + + float outputPeakDb = PeakDb(output, sampleCount / 2); + float inputPeakDb = PeakDb(input, 0); + // After clamp, output must NOT exceed input peak (no amplification). + Assert.That(outputPeakDb, Is.LessThanOrEqualTo(inputPeakDb + 0.1f), + $"Animated ratio<1 must clamp to passthrough; output {outputPeakDb:F2} dB exceeded input {inputPeakDb:F2} dB"); + // And it must roughly equal the input (no compression either). + Assert.That(outputPeakDb, Is.EqualTo(inputPeakDb).Within(0.5f)); + } + + [Test] + public void Process_AnimatedAttack_AboveMaxClampsAndKeepsEnvelopeMoving() + { + // An animated Attack of 1e9 ms would collapse the coefficient to exactly 1.0, freezing + // the envelope so it never tracks the input — and therefore never crosses the threshold + // and never compresses. After clamping to MaxAttackMs the coefficient is < 1.0 so the + // envelope advances. We use a deep threshold (-50 dB) so the slow envelope reaches it + // within a 1 s buffer; the visible output reduction is then proof the clamp engaged. + const int sampleCount = SampleRate; + using var input = CreateSineBuffer(0.9f, 1000f, sampleCount); + + var attackAnim = new KeyFrameAnimation(); + attackAnim.KeyFrames.Add(new KeyFrame { Easing = new LinearEasing(), Value = 1e9f, KeyTime = TimeSpan.Zero }); + attackAnim.KeyFrames.Add(new KeyFrame { Easing = new LinearEasing(), Value = 1e9f, KeyTime = TimeSpan.FromSeconds(1.0) }); + var attackProperty = Property.CreateAnimatable(1e9f); + attackProperty.Animation = attackAnim; + + var node = new CompressorNode + { + Threshold = Property.CreateAnimatable(-50f), + Ratio = Property.CreateAnimatable(4f), + Attack = attackProperty, + Release = Property.CreateAnimatable(50f), + Knee = Property.CreateAnimatable(0f), + MakeupGain = Property.CreateAnimatable(0f) + }; + node.AddInput(new StubSourceNode { Buffer = input }); + + var ctx = CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(1.0)); + using var output = node.Process(ctx); + + // Probe in the last 50 ms window so the clamped 500 ms attack has had ~2 time constants + // to bring the envelope above -50 dB and trigger compression. + int probeWidth = SampleRate / 20; + float steadyPeakDb = PeakDbInWindow(output, sampleCount - probeWidth, probeWidth); + // Without clamping: envelope frozen at -100 dB → no compression → output ≈ input (-0.92 dB). + // With clamping: envelope advances, crosses -50 dB threshold, triggers heavy reduction. + Assert.That(steadyPeakDb, Is.LessThan(-10f), + $"Animated Attack overshoot must be clamped so the envelope can still track; got {steadyPeakDb:F2} dB"); + } + + [Test] + public void Process_MultipleAnimatedNonFiniteParameters_AllFallBackIndependently() + { + // Two animated parameters simultaneously produce NaN. With a single shared latch, only + // one parameter would log and the second's fallback could be skipped if the latch were + // also gating the substitution; the per-parameter HashSet ensures both still substitute + // their fallbacks. Observable: output is not muted (would be -100 dB if NaN propagated). + const int sampleCount = SampleRate / 4; + using var input = CreateSineBuffer(0.9f, 1000f, sampleCount); + + var attackAnim = new KeyFrameAnimation(); + attackAnim.KeyFrames.Add(new KeyFrame { Easing = new LinearEasing(), Value = float.NaN, KeyTime = TimeSpan.Zero }); + attackAnim.KeyFrames.Add(new KeyFrame { Easing = new LinearEasing(), Value = float.NaN, KeyTime = TimeSpan.FromSeconds(0.25) }); + var attackProperty = Property.CreateAnimatable(5f); + attackProperty.Animation = attackAnim; + + var thresholdAnim = new KeyFrameAnimation(); + thresholdAnim.KeyFrames.Add(new KeyFrame { Easing = new LinearEasing(), Value = float.NaN, KeyTime = TimeSpan.Zero }); + thresholdAnim.KeyFrames.Add(new KeyFrame { Easing = new LinearEasing(), Value = float.NaN, KeyTime = TimeSpan.FromSeconds(0.25) }); + var thresholdProperty = Property.CreateAnimatable(-20f); + thresholdProperty.Animation = thresholdAnim; + + var node = new CompressorNode + { + Threshold = thresholdProperty, + Ratio = Property.CreateAnimatable(4f), + Attack = attackProperty, + Release = Property.CreateAnimatable(50f), + Knee = Property.CreateAnimatable(0f), + MakeupGain = Property.CreateAnimatable(0f) + }; + node.AddInput(new StubSourceNode { Buffer = input }); + + var ctx = CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(0.25)); + using var output = node.Process(ctx); + + for (int ch = 0; ch < output.ChannelCount; ch++) + { + var data = output.GetChannelData(ch); + for (int i = 0; i < output.SampleCount; i++) + { + Assert.That(float.IsFinite(data[i]), Is.True, + $"Output sample [{ch}][{i}] = {data[i]} is not finite"); + } + } + + float steadyPeakDb = PeakDb(output, sampleCount / 2); + Assert.That(steadyPeakDb, Is.GreaterThan(-25f), + $"Both NaN parameters must fall back so output remains audible; got {steadyPeakDb:F2} dB"); + } } From baaf08f363a52f5e94acdd98435a25fb6af9d088 Mon Sep 17 00:00:00 2001 From: Yuto Terada Date: Sun, 10 May 2026 17:15:53 +0900 Subject: [PATCH 6/8] refactor: address compressor PR review feedback Tighten compressor diagnostics, unify the makeup-gain math, and add review-driven tests: - Reset() now clears the per-instance non-finite log latches so each new render session (sample-rate change, seek) gets a fresh diagnostic window instead of permanently silencing warnings after the first event. - Static and animated paths now share ComputeGainLinear so the makeup application cannot drift between branches; per-sample math is reused rather than duplicated. - Document the one-pole IIR coefficient, soft-knee quadratic, and the reset/discontinuity comment now also covers the first-call case. - CompressorParameters has a static ctor with Debug.Assert checks for Min < Default < Max and the MinRatio >= 1f slope invariant. - New CompressorEffectTests verify CreateNode wires every IProperty by reference, connects input -> compressor in order, and exposes the documented defaults. - CompressorNodeTests gains an exact-zero silence test (covers the peak > 0f / log10(0) guard) and an attack-time-constant test that back-solves the envelope to ~63% to catch regressions like a missing ms->s conversion in ComputeCoeff. - Note that Expression-backed properties intentionally stay on the static path until AnimationSampler evaluates expressions per-sample (mirrors EqualizerEffect's matching FIXME). --- .../Audio/Effects/CompressorParameters.cs | 26 +++++- .../Audio/Graph/Nodes/CompressorNode.cs | 52 ++++++++++-- .../Engine/Audio/CompressorEffectTests.cs | 80 +++++++++++++++++++ .../Engine/Audio/CompressorNodeTests.cs | 75 +++++++++++++++++ 4 files changed, 226 insertions(+), 7 deletions(-) create mode 100644 tests/Beutl.UnitTests/Engine/Audio/CompressorEffectTests.cs diff --git a/src/Beutl.Engine/Audio/Effects/CompressorParameters.cs b/src/Beutl.Engine/Audio/Effects/CompressorParameters.cs index f8705d2cc..a9c93ca96 100644 --- a/src/Beutl.Engine/Audio/Effects/CompressorParameters.cs +++ b/src/Beutl.Engine/Audio/Effects/CompressorParameters.cs @@ -1,4 +1,6 @@ -namespace Beutl.Audio.Effects; +using System.Diagnostics; + +namespace Beutl.Audio.Effects; // Single source of truth for the compressor's parameter ranges and defaults. CompressorEffect // references these in its [Range] / Property.CreateAnimatable declarations and CompressorNode @@ -9,6 +11,9 @@ internal static class CompressorParameters public const float MaxThresholdDb = 0f; public const float DefaultThresholdDb = -20f; + // MinRatio must stay >= 1f. The slope formula `1 - 1/Ratio` becomes negative below 1, which + // would amplify above-threshold signals instead of compressing them; CompressorNode.Sanitize + // relies on this invariant to make the slope safe without an additional guard. public const float MinRatio = 1f; public const float MaxRatio = 20f; public const float DefaultRatio = 4f; @@ -28,4 +33,23 @@ internal static class CompressorParameters public const float MinMakeupGainDb = -24f; public const float MaxMakeupGainDb = 24f; public const float DefaultMakeupGainDb = 0f; + + static CompressorParameters() + { + // Run once at module load so an inconsistent Min/Default/Max edit is caught immediately + // in debug builds rather than producing subtle audio glitches at runtime. + AssertRange(MinThresholdDb, DefaultThresholdDb, MaxThresholdDb, "Threshold"); + AssertRange(MinRatio, DefaultRatio, MaxRatio, "Ratio"); + AssertRange(MinAttackMs, DefaultAttackMs, MaxAttackMs, "Attack"); + AssertRange(MinReleaseMs, DefaultReleaseMs, MaxReleaseMs, "Release"); + AssertRange(MinKneeDb, DefaultKneeDb, MaxKneeDb, "Knee"); + AssertRange(MinMakeupGainDb, DefaultMakeupGainDb, MaxMakeupGainDb, "MakeupGain"); + Debug.Assert(MinRatio >= 1f, "MinRatio must stay >= 1f or the slope formula amplifies."); + } + + private static void AssertRange(float min, float def, float max, string name) + { + Debug.Assert(min < max, $"{name}: Min ({min}) must be strictly less than Max ({max})."); + Debug.Assert(min <= def && def <= max, $"{name}: Default ({def}) must lie in [{min}, {max}]."); + } } diff --git a/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs b/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs index da35f5a3f..88f88da15 100644 --- a/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs +++ b/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs @@ -52,9 +52,10 @@ public override AudioBuffer Process(AudioProcessContext context) _lastSampleRate = context.SampleRate; } - // Reset whenever the chunk does not continue directly from the previous one, because the - // node instance is cached across Compose() calls and stale envelope state would otherwise - // bleed into the first samples after a seek or stop/restart. + // Reset on the very first call (no prior end recorded) and whenever the new chunk does + // not start exactly where the previous one ended. The node instance is cached across + // Compose() calls, so without this guard stale envelope state would bleed into the first + // samples after a seek or stop/restart. if (!_lastTimeRangeEnd.HasValue || _lastTimeRangeEnd.Value != context.TimeRange.Start) { Reset(); @@ -66,6 +67,13 @@ public override AudioBuffer Process(AudioProcessContext context) return new AudioBuffer(input.SampleRate, input.ChannelCount, 0); } + // Expression-backed properties are intentionally not checked here: AnimationSampler does + // not currently evaluate expressions per-sample, so treating HasExpression as live would + // route to ProcessAnimated and read the same CurrentValue every iteration — strictly + // worse than ProcessStatic, which reads it once. Aligned with EqualizerEffect's matching + // FIXME: once AnimationSampler evaluates expressions per-sample, this guard must also + // treat HasExpression as live or expression-backed parameters will be silently frozen + // at their graph-build-time value. bool hasAnimation = Threshold.Animation != null || Ratio.Animation != null || Attack.Animation != null || @@ -90,7 +98,6 @@ private AudioBuffer ProcessStatic(AudioBuffer input, AudioProcessContext context float attackCoeff = ComputeCoeff(p.Attack, context.SampleRate); float releaseCoeff = ComputeCoeff(p.Release, context.SampleRate); float slope = 1f - 1f / p.Ratio; - float makeupLinear = AudioMath.ConvertDbToLinear(p.MakeupGain); int channels = input.ChannelCount; for (int i = 0; i < input.SampleCount; i++) @@ -102,13 +109,16 @@ private AudioBuffer ProcessStatic(AudioBuffer input, AudioProcessContext context if (a > peak) peak = a; } + // peak == 0 (digital silence) and peak == NaN both fail `peak > 0f`, so inputDb + // collapses to MinDb here; downstream RecoverEnvelopeIfNonFinite / SanitizeOutput + // handle the NaN case if it slipped through arithmetic above. float inputDb = peak > 0f ? 20f * MathF.Log10(peak) : MinDb; float coeff = inputDb > _envelopeDb ? attackCoeff : releaseCoeff; _envelopeDb = inputDb + coeff * (_envelopeDb - inputDb); RecoverEnvelopeIfNonFinite(); float gainReductionDb = ComputeGainReductionDb(_envelopeDb, p.Threshold, p.Knee, slope); - float gainLinear = AudioMath.ConvertDbToLinear(-gainReductionDb) * makeupLinear; + float gainLinear = ComputeGainLinear(gainReductionDb, p.MakeupGain); for (int ch = 0; ch < channels; ch++) { @@ -196,7 +206,7 @@ private AudioBuffer ProcessAnimated(AudioBuffer input, AudioProcessContext conte RecoverEnvelopeIfNonFinite(); float gainReductionDb = ComputeGainReductionDb(_envelopeDb, p.Threshold, p.Knee, slope); - float gainLinear = AudioMath.ConvertDbToLinear(p.MakeupGain - gainReductionDb); + float gainLinear = ComputeGainLinear(gainReductionDb, p.MakeupGain); for (int ch = 0; ch < channels; ch++) { @@ -286,11 +296,31 @@ private float SanitizeOutput(float sample) return 0f; } + // Combined linear gain factor: the dB-domain reduction is subtracted and the makeup gain is + // added before a single dB→linear conversion. Both ProcessStatic and ProcessAnimated share + // this helper so the static and animated paths cannot drift out of agreement (they used to + // apply makeup as a pre-computed `makeupLinear` multiplier vs. an in-formula addition, which + // is mathematically equivalent but a future refactor of one branch could silently break the + // other). + private static float ComputeGainLinear(float gainReductionDb, float makeupDb) + { + return AudioMath.ConvertDbToLinear(makeupDb - gainReductionDb); + } + + // Standard one-pole IIR smoothing coefficient for a 1/e settling time of `timeMs` at the + // given sample rate: y[n] = x[n] + coeff * (y[n-1] - x[n]) reaches (1 - 1/e) ≈ 63% of a step + // change after exactly `timeMs` milliseconds. Operates in dB-domain on `_envelopeDb` because + // dB-domain peak smoothing better matches how the human ear perceives compression action. private static float ComputeCoeff(float timeMs, int sampleRate) { return MathF.Exp(-1f / (timeMs * 0.001f * sampleRate)); } + // Soft-knee gain computer (Reece/Giannoulis formulation): when `kneeDb > 0`, the gain + // reduction curve transitions smoothly from "no compression" to the full `slope * diff` + // line over a `kneeDb`-wide region centred on the threshold, via a quadratic that is + // C¹-continuous at both knee boundaries. With `kneeDb == 0` this collapses to the standard + // hard-knee formula. private static float ComputeGainReductionDb(float envelopeDb, float thresholdDb, float kneeDb, float slope) { if (kneeDb > 0f) @@ -303,6 +333,9 @@ private static float ComputeGainReductionDb(float envelopeDb, float thresholdDb, } if (diff < halfKnee) { + // Quadratic interpolation across the knee: at diff = -halfKnee returns 0, at + // diff = +halfKnee returns slope * halfKnee, with matching derivatives at both + // ends (= 0 below, = slope above) so the overall curve is smooth. float x = diff + halfKnee; return slope * x * x / (2f * kneeDb); } @@ -314,9 +347,16 @@ private static float ComputeGainReductionDb(float envelopeDb, float thresholdDb, // Internal so tests can drive an explicit reset, but not part of the public API: external // callers must not zero the envelope mid-buffer (it would produce an audible click). + // Reset is also the natural "new render session" boundary (sample-rate change, seek), so we + // clear the diagnostic latches here. Otherwise, once a non-finite event fired in an earlier + // session, the operator would never see another warning for the entire node lifetime — even + // after fixing the offending keyframe and re-rendering. internal void Reset() { _envelopeDb = MinDb; + _loggedNonFiniteEnvelope = false; + _loggedNonFiniteSample = false; + _loggedNonFiniteParameters.Clear(); } private struct EffectiveParameters diff --git a/tests/Beutl.UnitTests/Engine/Audio/CompressorEffectTests.cs b/tests/Beutl.UnitTests/Engine/Audio/CompressorEffectTests.cs new file mode 100644 index 000000000..96c143f2d --- /dev/null +++ b/tests/Beutl.UnitTests/Engine/Audio/CompressorEffectTests.cs @@ -0,0 +1,80 @@ +using Beutl.Audio; +using Beutl.Audio.Effects; +using Beutl.Audio.Graph; +using Beutl.Audio.Graph.Nodes; +using Beutl.Engine; + +namespace Beutl.UnitTests.Engine.Audio; + +public class CompressorEffectTests +{ + private sealed class StubInputNode : AudioNode + { + public override AudioBuffer Process(AudioProcessContext context) + { + return new AudioBuffer(context.SampleRate, 2, 0); + } + } + + [Test] + public void CreateNode_WiresEveryPropertyToMatchingNodeSlot() + { + // Each IProperty on the effect must be forwarded by reference to the corresponding + // slot on CompressorNode. Reference equality (not just value equality) is critical because + // the node reads CurrentValue and Animation through these references at process time — + // copying the value would freeze it. A swap of any two properties (Threshold↔Ratio, etc.) + // would silently corrupt the compressor without this test. + var effect = new CompressorEffect(); + using var context = new AudioContext(48000, 2); + var inputNode = context.AddNode(new StubInputNode()); + + var node = effect.CreateNode(context, inputNode); + + Assert.That(node, Is.InstanceOf()); + var compressor = (CompressorNode)node; + + Assert.That(compressor.Threshold, Is.SameAs(effect.Threshold)); + Assert.That(compressor.Ratio, Is.SameAs(effect.Ratio)); + Assert.That(compressor.Attack, Is.SameAs(effect.Attack)); + Assert.That(compressor.Release, Is.SameAs(effect.Release)); + Assert.That(compressor.Knee, Is.SameAs(effect.Knee)); + Assert.That(compressor.MakeupGain, Is.SameAs(effect.MakeupGain)); + } + + [Test] + public void CreateNode_ConnectsInputAheadOfCompressor() + { + // The contract is "audio flows input → compressor". Connect must run with arguments in + // that order so the compressor's Inputs collection contains the upstream node, not the + // other way around (which would route the compressor's own output into the input node + // and produce silence at best, an infinite loop at worst). + var effect = new CompressorEffect(); + using var context = new AudioContext(48000, 2); + var inputNode = context.AddNode(new StubInputNode()); + + var node = effect.CreateNode(context, inputNode); + + Assert.That(node.Inputs, Has.Count.EqualTo(1)); + Assert.That(node.Inputs[0], Is.SameAs(inputNode)); + // The returned node is the compressor itself, not the input — downstream effects need to + // chain off the compressor. + Assert.That(node, Is.Not.SameAs(inputNode)); + } + + [Test] + public void CreateNode_DefaultPropertyValuesMatchCompressorParameters() + { + // The CompressorEffect's default values are sourced from CompressorParameters. If any + // default drifted (e.g. the file was edited but not the [Range]), the effect would still + // load but operators would see unexpected starting parameters. This guards against that + // drift by asserting each property's CurrentValue is the documented default. + var effect = new CompressorEffect(); + + Assert.That(effect.Threshold.CurrentValue, Is.EqualTo(-20f)); + Assert.That(effect.Ratio.CurrentValue, Is.EqualTo(4f)); + Assert.That(effect.Attack.CurrentValue, Is.EqualTo(10f)); + Assert.That(effect.Release.CurrentValue, Is.EqualTo(100f)); + Assert.That(effect.Knee.CurrentValue, Is.EqualTo(6f)); + Assert.That(effect.MakeupGain.CurrentValue, Is.EqualTo(0f)); + } +} diff --git a/tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs b/tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs index 5f1376833..400de1cbe 100644 --- a/tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs +++ b/tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs @@ -119,6 +119,81 @@ private static AudioProcessContext CreateContext(TimeSpan start, TimeSpan durati null); } + [Test] + public void Process_SilenceInput_ProducesExactSilenceAndKeepsEnvelopeAtFloor() + { + // peak == 0f must short-circuit to inputDb = MinDb at the `peak > 0f` guard. Without it, + // MathF.Log10(0) returns -Infinity, the envelope formula produces NaN, the recovery path + // would mask it, and any future change to that recovery path could leave the bug + // undetectable. We therefore feed an exactly-zero buffer and assert exact-zero output — + // any non-zero sample would prove the inputDb guard or makeup math is broken. + const int sampleCount = SampleRate / 4; + using var input = new AudioBuffer(SampleRate, 2, sampleCount); + // Default-constructed AudioBuffer is zeroed, so no fill needed. + var source = new StubSourceNode { Buffer = input }; + var node = CreateNode(); + node.AddInput(source); + + var ctx = CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(0.25)); + using var output = node.Process(ctx); + + for (int ch = 0; ch < output.ChannelCount; ch++) + { + var data = output.GetChannelData(ch); + for (int i = 0; i < output.SampleCount; i++) + { + Assert.That(data[i], Is.EqualTo(0f), + $"Silent input must produce exact-zero output, but [{ch}][{i}] = {data[i]}"); + } + } + } + + [Test] + public void Process_AttackTimeConstant_EnvelopeReachesAbout63PercentAfterAttackMs() + { + // Step input drives the envelope from MinDb toward inputDb_max. After exactly attackMs + // milliseconds, a one-pole IIR with time constant attackMs should have reached + // ~(1 - 1/e) ≈ 63% of the way to its target. This is the contract the ComputeCoeff + // formula encodes, and a future regression that, e.g., dropped the ms→s conversion + // (`timeMs * sampleRate` instead of `timeMs * 0.001f * sampleRate`) would leave the + // envelope barely moving — caught here. + const float attackMs = 50f; + const int sampleCount = SampleRate; // 1 s + const int stepAt = SampleRate / 10; // step at 100 ms; envelope below MinDb until then + using var input = new AudioBuffer(SampleRate, 1, sampleCount); + var data = input.GetChannelData(0); + for (int i = stepAt; i < sampleCount; i++) + { + data[i] = 1f; // exactly 0 dB peak after the step + } + + // Threshold well below the input so above-threshold compression engages immediately. + // Release is irrelevant here (envelope is rising). Knee=0 keeps the gain reduction a + // simple linear function of envelopeDb above threshold so we can back-solve the envelope. + var node = CreateNode(threshold: -40f, ratio: 4f, attack: attackMs, release: 100f, knee: 0f); + node.AddInput(new StubSourceNode { Buffer = input }); + var ctx = CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(1.0)); + using var output = node.Process(ctx); + + // Sample exactly attackMs after the step. At that point, in dB-domain: + // envelopeDb(t) = inputDb_max - (inputDb_max - inputDb_initial) * exp(-t / attackMs) + // With inputDb_max = 0 dB (peak 1.0) and inputDb_initial ≈ MinDb = -100 dB: + // envelopeDb ≈ 0 - 100 * (1/e) ≈ -36.79 dB at t = attackMs. + int probeIdx = stepAt + (int)(attackMs * 0.001f * SampleRate); + // Reconstruct envelopeDb from the observed gain reduction: + // gainLinear = 10^(-gainReductionDb / 20), envelope = output / input. + float gainLinear = MathF.Abs(output.GetChannelData(0)[probeIdx] / data[probeIdx]); + float gainReductionDb = -20f * MathF.Log10(gainLinear); + // gainReductionDb = slope * (envelopeDb - thresholdDb), slope = 1 - 1/4 = 0.75, threshold = -40: + // envelopeDb = thresholdDb + gainReductionDb / slope. + float reconstructedEnvelopeDb = -40f + gainReductionDb / 0.75f; + // 1 - 1/e ≈ 0.632; envelope should sit at ~-36.79 dB (i.e. 63.2% of the way from -100 to 0). + // Tolerance is generous (~±5 dB) because the dB-domain peak ripples slightly with the + // sine-like product, but a missing ms→s conversion would land near -100 dB — far outside. + Assert.That(reconstructedEnvelopeDb, Is.EqualTo(-36.79f).Within(5f), + $"After attackMs={attackMs} ms, envelope should reach ~63% (≈-36.79 dB) but got {reconstructedEnvelopeDb:F2} dB"); + } + [Test] public void Process_BelowThreshold_LeavesSignalUnchanged() { From e69a1346ea2cab1032c54e3ba3c6c63efc9f5604 Mon Sep 17 00:00:00 2001 From: Yuto Terada Date: Sun, 10 May 2026 17:37:04 +0900 Subject: [PATCH 7/8] fix: make compressor invariant checks actually run and tighten attack-time test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second-pass review found the prior commit's static-ctor validation never executed (const consumers don't trigger type init) and the attack-time- constant test could falsely pass under the very ms->s regression it was meant to catch. - Replace the unreachable static ctor in CompressorParameters with a [ModuleInitializer] method so the Min/Default/Max consistency asserts fire on every Debug-build run, regardless of whether any non-const member is touched. Drop the tautological MinRatio >= 1f assert. - Move the attack-time-constant test's load-bearing assertion onto gainReductionDb directly (≈9.91 dB ±2) and shift threshold to -50 dB so a buggy envelope stuck near -100 dB no longer reconstructs to a value inside the secondary tolerance window. Document the threshold choice as load-bearing. - Reframe the silence test's comment to describe what it actually catches (DC/noise injection on silent streams) rather than claiming to isolate the peak > 0f guard, which RecoverEnvelopeIfNonFinite trivially masks. - Point the Expression FIXME cross-reference at EqualizerEffect.IsNeutral specifically (build-time band elision) so a maintainer searching the equalizer node for the matching guard does not come up empty. - Split Reset() into ResetEnvelope/ResetDiagnostics private helpers so the broadened "new render session" semantics are visible at the call site instead of buried in a comment. - Trim the peak == 0 comment to drop the vestigial NaN clause; the abs/max loop above keeps `peak` finite. --- .../Audio/Effects/CompressorParameters.cs | 13 +++-- .../Audio/Graph/Nodes/CompressorNode.cs | 38 +++++++++---- .../Engine/Audio/CompressorNodeTests.cs | 56 ++++++++++++------- 3 files changed, 71 insertions(+), 36 deletions(-) diff --git a/src/Beutl.Engine/Audio/Effects/CompressorParameters.cs b/src/Beutl.Engine/Audio/Effects/CompressorParameters.cs index a9c93ca96..dff33bb3c 100644 --- a/src/Beutl.Engine/Audio/Effects/CompressorParameters.cs +++ b/src/Beutl.Engine/Audio/Effects/CompressorParameters.cs @@ -1,4 +1,5 @@ using System.Diagnostics; +using System.Runtime.CompilerServices; namespace Beutl.Audio.Effects; @@ -34,17 +35,21 @@ internal static class CompressorParameters public const float MaxMakeupGainDb = 24f; public const float DefaultMakeupGainDb = 0f; - static CompressorParameters() + // [ModuleInitializer] runs once at module load regardless of whether any non-const member + // is touched. A static constructor on this class would NOT run, because every consumer + // references our `const float` fields, and the C# compiler inlines const literals at the + // call site — neither `using static CompressorParameters;` nor `[Range(MinX, MaxX)]` triggers + // type initialization. The module initializer sidesteps that and guarantees the asserts + // below execute on every Debug-build run (production builds elide Debug.Assert anyway). + [ModuleInitializer] + internal static void Validate() { - // Run once at module load so an inconsistent Min/Default/Max edit is caught immediately - // in debug builds rather than producing subtle audio glitches at runtime. AssertRange(MinThresholdDb, DefaultThresholdDb, MaxThresholdDb, "Threshold"); AssertRange(MinRatio, DefaultRatio, MaxRatio, "Ratio"); AssertRange(MinAttackMs, DefaultAttackMs, MaxAttackMs, "Attack"); AssertRange(MinReleaseMs, DefaultReleaseMs, MaxReleaseMs, "Release"); AssertRange(MinKneeDb, DefaultKneeDb, MaxKneeDb, "Knee"); AssertRange(MinMakeupGainDb, DefaultMakeupGainDb, MaxMakeupGainDb, "MakeupGain"); - Debug.Assert(MinRatio >= 1f, "MinRatio must stay >= 1f or the slope formula amplifies."); } private static void AssertRange(float min, float def, float max, string name) diff --git a/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs b/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs index 88f88da15..136b4ab5c 100644 --- a/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs +++ b/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs @@ -70,10 +70,11 @@ public override AudioBuffer Process(AudioProcessContext context) // Expression-backed properties are intentionally not checked here: AnimationSampler does // not currently evaluate expressions per-sample, so treating HasExpression as live would // route to ProcessAnimated and read the same CurrentValue every iteration — strictly - // worse than ProcessStatic, which reads it once. Aligned with EqualizerEffect's matching - // FIXME: once AnimationSampler evaluates expressions per-sample, this guard must also - // treat HasExpression as live or expression-backed parameters will be silently frozen - // at their graph-build-time value. + // worse than ProcessStatic, which reads it once. The same root cause is documented at + // EqualizerEffect.IsNeutral (build-time band elision), where the FIXME notes that once + // AnimationSampler evaluates expressions per-sample, both that guard and this one must + // be updated to treat HasExpression as live; otherwise expression-backed parameters + // will be silently frozen at their graph-build-time value. bool hasAnimation = Threshold.Animation != null || Ratio.Animation != null || Attack.Animation != null || @@ -109,9 +110,10 @@ private AudioBuffer ProcessStatic(AudioBuffer input, AudioProcessContext context if (a > peak) peak = a; } - // peak == 0 (digital silence) and peak == NaN both fail `peak > 0f`, so inputDb - // collapses to MinDb here; downstream RecoverEnvelopeIfNonFinite / SanitizeOutput - // handle the NaN case if it slipped through arithmetic above. + // peak == 0 (digital silence) collapses inputDb to MinDb here. The local `peak` is + // always finite because the abs/max loop above stays at 0 when MathF.Abs(NaN) > 0 + // is false; any non-finite envelope state arriving from elsewhere is recovered by + // RecoverEnvelopeIfNonFinite below. float inputDb = peak > 0f ? 20f * MathF.Log10(peak) : MinDb; float coeff = inputDb > _envelopeDb ? attackCoeff : releaseCoeff; _envelopeDb = inputDb + coeff * (_envelopeDb - inputDb); @@ -347,13 +349,27 @@ private static float ComputeGainReductionDb(float envelopeDb, float thresholdDb, // Internal so tests can drive an explicit reset, but not part of the public API: external // callers must not zero the envelope mid-buffer (it would produce an audible click). - // Reset is also the natural "new render session" boundary (sample-rate change, seek), so we - // clear the diagnostic latches here. Otherwise, once a non-finite event fired in an earlier - // session, the operator would never see another warning for the entire node lifetime — even - // after fixing the offending keyframe and re-rendering. + // Reset() corresponds to a "new render session" boundary (sample-rate change, seek), and + // unifies two concerns: + // - DSP state (the envelope follower) — see ResetEnvelope + // - Diagnostic latches (one-shot warnings) — see ResetDiagnostics + // They are atomic in production callers because every session boundary is also a fresh + // diagnostic window: we want operators to see warnings re-fire after fixing a bad keyframe + // and re-rendering. Splitting them into named helpers makes the dual responsibility legible + // at the call site instead of buried in this comment. internal void Reset() + { + ResetEnvelope(); + ResetDiagnostics(); + } + + private void ResetEnvelope() { _envelopeDb = MinDb; + } + + private void ResetDiagnostics() + { _loggedNonFiniteEnvelope = false; _loggedNonFiniteSample = false; _loggedNonFiniteParameters.Clear(); diff --git a/tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs b/tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs index 400de1cbe..a51f30f07 100644 --- a/tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs +++ b/tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs @@ -120,13 +120,15 @@ private static AudioProcessContext CreateContext(TimeSpan start, TimeSpan durati } [Test] - public void Process_SilenceInput_ProducesExactSilenceAndKeepsEnvelopeAtFloor() + public void Process_SilenceInput_ProducesExactSilenceOutput() { - // peak == 0f must short-circuit to inputDb = MinDb at the `peak > 0f` guard. Without it, - // MathF.Log10(0) returns -Infinity, the envelope formula produces NaN, the recovery path - // would mask it, and any future change to that recovery path could leave the bug - // undetectable. We therefore feed an exactly-zero buffer and assert exact-zero output — - // any non-zero sample would prove the inputDb guard or makeup math is broken. + // End-to-end "silence in → silence out" smoke test. Note: this does NOT specifically + // isolate the `peak > 0f` guard, because RecoverEnvelopeIfNonFinite would mask a NaN + // envelope produced by Log10(0) — the gain calculation against a 0-amplitude sample + // still yields exactly 0. Genuinely isolating that guard would require log capture or + // exposing internal state. What this test does catch: any future bug that injects DC, + // noise, or non-zero offset into a silent stream (e.g., a stray makeup application that + // mishandles the additive identity, or a sanitizer that fails open). const int sampleCount = SampleRate / 4; using var input = new AudioBuffer(SampleRate, 2, sampleCount); // Default-constructed AudioBuffer is zeroed, so no fill needed. @@ -154,12 +156,11 @@ public void Process_AttackTimeConstant_EnvelopeReachesAbout63PercentAfterAttackM // Step input drives the envelope from MinDb toward inputDb_max. After exactly attackMs // milliseconds, a one-pole IIR with time constant attackMs should have reached // ~(1 - 1/e) ≈ 63% of the way to its target. This is the contract the ComputeCoeff - // formula encodes, and a future regression that, e.g., dropped the ms→s conversion - // (`timeMs * sampleRate` instead of `timeMs * 0.001f * sampleRate`) would leave the - // envelope barely moving — caught here. + // formula encodes; a regression like dropping the ms→s conversion (timeMs * sampleRate + // instead of timeMs * 0.001f * sampleRate) would leave the envelope still near -100 dB. const float attackMs = 50f; const int sampleCount = SampleRate; // 1 s - const int stepAt = SampleRate / 10; // step at 100 ms; envelope below MinDb until then + const int stepAt = SampleRate / 10; // step at 100 ms; envelope sits at MinDb until then using var input = new AudioBuffer(SampleRate, 1, sampleCount); var data = input.GetChannelData(0); for (int i = stepAt; i < sampleCount; i++) @@ -167,29 +168,42 @@ public void Process_AttackTimeConstant_EnvelopeReachesAbout63PercentAfterAttackM data[i] = 1f; // exactly 0 dB peak after the step } - // Threshold well below the input so above-threshold compression engages immediately. - // Release is irrelevant here (envelope is rising). Knee=0 keeps the gain reduction a - // simple linear function of envelopeDb above threshold so we can back-solve the envelope. - var node = CreateNode(threshold: -40f, ratio: 4f, attack: attackMs, release: 100f, knee: 0f); + // Threshold = -50 dB is the load-bearing choice. With the bug, envelope stays near + // -100 dB (below threshold) → gainReductionDb = 0 → output = input → reconstructed + // envelope clamps to thresholdDb = -50 dB. Setting threshold ABOVE the buggy envelope + // (at -50, far above -100) makes the buggy reconstruction 13.21 dB away from the target + // (-36.79 dB), well outside the ±5 dB tolerance. A threshold of -40 or lower would put + // the buggy reconstruction within tolerance and the test would falsely pass. + // Knee=0 keeps the gain formula linear so we can back-solve the envelope value. + const float thresholdDb = -50f; + const float ratio = 4f; + const float slope = 1f - 1f / ratio; // 0.75 + var node = CreateNode(threshold: thresholdDb, ratio: ratio, attack: attackMs, release: 100f, knee: 0f); node.AddInput(new StubSourceNode { Buffer = input }); var ctx = CreateContext(TimeSpan.Zero, TimeSpan.FromSeconds(1.0)); using var output = node.Process(ctx); // Sample exactly attackMs after the step. At that point, in dB-domain: // envelopeDb(t) = inputDb_max - (inputDb_max - inputDb_initial) * exp(-t / attackMs) - // With inputDb_max = 0 dB (peak 1.0) and inputDb_initial ≈ MinDb = -100 dB: + // With inputDb_max = 0 dB (peak 1.0) and inputDb_initial = MinDb = -100 dB: // envelopeDb ≈ 0 - 100 * (1/e) ≈ -36.79 dB at t = attackMs. int probeIdx = stepAt + (int)(attackMs * 0.001f * SampleRate); // Reconstruct envelopeDb from the observed gain reduction: // gainLinear = 10^(-gainReductionDb / 20), envelope = output / input. float gainLinear = MathF.Abs(output.GetChannelData(0)[probeIdx] / data[probeIdx]); float gainReductionDb = -20f * MathF.Log10(gainLinear); - // gainReductionDb = slope * (envelopeDb - thresholdDb), slope = 1 - 1/4 = 0.75, threshold = -40: - // envelopeDb = thresholdDb + gainReductionDb / slope. - float reconstructedEnvelopeDb = -40f + gainReductionDb / 0.75f; - // 1 - 1/e ≈ 0.632; envelope should sit at ~-36.79 dB (i.e. 63.2% of the way from -100 to 0). - // Tolerance is generous (~±5 dB) because the dB-domain peak ripples slightly with the - // sine-like product, but a missing ms→s conversion would land near -100 dB — far outside. + + // Direct assertion: the expected gain reduction at t = attackMs is + // slope * (-36.79 - thresholdDb) = 0.75 * (-36.79 - (-50)) = 0.75 * 13.21 ≈ 9.91 dB. + // Under the ms→s bug, envelope stays below threshold so gainReductionDb is 0 — the + // tolerance below excludes that. This is the load-bearing assertion. + Assert.That(gainReductionDb, Is.EqualTo(9.91f).Within(2f), + $"At t = attackMs ({attackMs} ms), expected ≈9.91 dB reduction but got {gainReductionDb:F2} dB. " + + $"Near 0 dB indicates ComputeCoeff lost its ms→s conversion."); + + // Secondary back-solve for human readability of the failure mode: + // gainReductionDb = slope * (envelopeDb - thresholdDb) for envelopeDb > thresholdDb. + float reconstructedEnvelopeDb = thresholdDb + gainReductionDb / slope; Assert.That(reconstructedEnvelopeDb, Is.EqualTo(-36.79f).Within(5f), $"After attackMs={attackMs} ms, envelope should reach ~63% (≈-36.79 dB) but got {reconstructedEnvelopeDb:F2} dB"); } From e0e82357d361ed55006f42dded123759d14cd59d Mon Sep 17 00:00:00 2001 From: Yuto Terada Date: Sun, 10 May 2026 17:53:23 +0900 Subject: [PATCH 8/8] test: guard compressor [ModuleInitializer] and tighten review-flagged comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add a reflection-based smoke test that asserts CompressorParameters.Validate exists and carries [ModuleInitializer]. Without that attribute the Min/Default/Max consistency asserts silently become dead code (a plain static ctor would not run because every consumer reads const fields, which the compiler inlines). The smoke test fails fast if a future refactor removes the attribute or renames the method. - Reword the Reset() doc comment from "atomic" to "always invoked together" to avoid the threading/transaction connotation; the meaning is purely "called as a pair from production sites." - Tighten the silence-test comment's causal chain: Log10(0) returns -Infinity, and NaN only appears later when the IIR formula evaluates (-∞) + coeff·(+∞). The previous wording skipped the IIR step. --- .../Audio/Graph/Nodes/CompressorNode.cs | 8 +++---- .../Engine/Audio/CompressorEffectTests.cs | 23 ++++++++++++++++++- .../Engine/Audio/CompressorNodeTests.cs | 10 ++++---- 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs b/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs index 136b4ab5c..93f21adb6 100644 --- a/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs +++ b/src/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cs @@ -353,10 +353,10 @@ private static float ComputeGainReductionDb(float envelopeDb, float thresholdDb, // unifies two concerns: // - DSP state (the envelope follower) — see ResetEnvelope // - Diagnostic latches (one-shot warnings) — see ResetDiagnostics - // They are atomic in production callers because every session boundary is also a fresh - // diagnostic window: we want operators to see warnings re-fire after fixing a bad keyframe - // and re-rendering. Splitting them into named helpers makes the dual responsibility legible - // at the call site instead of buried in this comment. + // They are always invoked together in production callers because every session boundary is + // also a fresh diagnostic window: we want operators to see warnings re-fire after fixing a + // bad keyframe and re-rendering. Splitting them into named helpers makes the dual + // responsibility legible at the call site instead of buried in this comment. internal void Reset() { ResetEnvelope(); diff --git a/tests/Beutl.UnitTests/Engine/Audio/CompressorEffectTests.cs b/tests/Beutl.UnitTests/Engine/Audio/CompressorEffectTests.cs index 96c143f2d..0904b82b4 100644 --- a/tests/Beutl.UnitTests/Engine/Audio/CompressorEffectTests.cs +++ b/tests/Beutl.UnitTests/Engine/Audio/CompressorEffectTests.cs @@ -1,4 +1,6 @@ -using Beutl.Audio; +using System.Reflection; +using System.Runtime.CompilerServices; +using Beutl.Audio; using Beutl.Audio.Effects; using Beutl.Audio.Graph; using Beutl.Audio.Graph.Nodes; @@ -77,4 +79,23 @@ public void CreateNode_DefaultPropertyValuesMatchCompressorParameters() Assert.That(effect.Knee.CurrentValue, Is.EqualTo(6f)); Assert.That(effect.MakeupGain.CurrentValue, Is.EqualTo(0f)); } + + [Test] + public void CompressorParameters_Validate_IsAnnotatedAsModuleInitializer() + { + // CompressorParameters.Validate is the only execution path for the Min/Default/Max + // consistency asserts. Because every consumer references const fields (which the C# + // compiler inlines), no `ldsfld` against CompressorParameters is ever emitted, so a + // plain static constructor would not run. The class instead relies on + // [ModuleInitializer] to invoke Validate at module load. If a future refactor removes + // the attribute or renames the method without updating the contract, the asserts + // become silent dead code. This reflection check fails fast if either invariant breaks. + var method = typeof(CompressorParameters).GetMethod( + "Validate", + BindingFlags.Static | BindingFlags.NonPublic); + Assert.That(method, Is.Not.Null, + "CompressorParameters.Validate must exist; it is the only carrier of the [ModuleInitializer] attribute."); + Assert.That(method!.GetCustomAttribute(), Is.Not.Null, + "CompressorParameters.Validate must be annotated [ModuleInitializer] so the Min/Default/Max asserts run at module load. Without it, the asserts become unreachable."); + } } diff --git a/tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs b/tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs index a51f30f07..b63e30266 100644 --- a/tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs +++ b/tests/Beutl.UnitTests/Engine/Audio/CompressorNodeTests.cs @@ -123,10 +123,12 @@ private static AudioProcessContext CreateContext(TimeSpan start, TimeSpan durati public void Process_SilenceInput_ProducesExactSilenceOutput() { // End-to-end "silence in → silence out" smoke test. Note: this does NOT specifically - // isolate the `peak > 0f` guard, because RecoverEnvelopeIfNonFinite would mask a NaN - // envelope produced by Log10(0) — the gain calculation against a 0-amplitude sample - // still yields exactly 0. Genuinely isolating that guard would require log capture or - // exposing internal state. What this test does catch: any future bug that injects DC, + // isolate the `peak > 0f` guard, because RecoverEnvelopeIfNonFinite would mask the + // non-finite envelope state produced when Log10(0) = -Infinity propagates through the + // IIR formula `inputDb + coeff * (_envelopeDb - inputDb)` (NaN appears at the + // (-∞) + coeff·(+∞) step). The gain calculation against a 0-amplitude sample still + // yields exactly 0 either way. Genuinely isolating that guard would require log capture + // or exposing internal state. What this test does catch: any future bug that injects DC, // noise, or non-zero offset into a silent stream (e.g., a stray makeup application that // mishandles the additive identity, or a sanitizer that fails open). const int sampleCount = SampleRate / 4;