diff --git a/src/TeamsISO.Engine/Pipeline/IsoPipeline.cs b/src/TeamsISO.Engine/Pipeline/IsoPipeline.cs index ac67a35..3bd064d 100644 --- a/src/TeamsISO.Engine/Pipeline/IsoPipeline.cs +++ b/src/TeamsISO.Engine/Pipeline/IsoPipeline.cs @@ -106,9 +106,11 @@ public sealed class IsoPipeline : IAsyncDisposable IncomingHeight: h) { State = State, - // Peak is published by NdiReceiver's audio loop; 0.0 means - // silence, no audio yet, or the sender is video-only. - PeakAudioLevel = receiver.LatestAudioPeak, + // Peak is the high-water mark since the last GetStats() call — + // ConsumeAudioPeak resets it to 0 atomically, so the next read + // reflects only what arrived in the next polling interval. + // 0.0 means silence, no audio yet, or the sender is video-only. + PeakAudioLevel = receiver.ConsumeAudioPeak(), }; } diff --git a/src/TeamsISO.Engine/Pipeline/NdiReceiver.cs b/src/TeamsISO.Engine/Pipeline/NdiReceiver.cs index 6f8404c..9997ded 100644 --- a/src/TeamsISO.Engine/Pipeline/NdiReceiver.cs +++ b/src/TeamsISO.Engine/Pipeline/NdiReceiver.cs @@ -18,18 +18,21 @@ public sealed class NdiReceiver : IDisposable private readonly NdiReceiverHandle _handle; private long _framesCaptured; - // Most recent audio peak, in [0, 1]. Updated by the audio capture loop; - // read by IsoPipeline.GetStats on the UI poll thread. We use a long - // holding the IEEE 754 double bits + Volatile read/write so reads are - // atomic across threads (a double on x86 can tear; long is always atomic - // when aligned, which the runtime guarantees for fields). + // Audio peak high-water mark since the last read, in [0, 1]. // - // Decay rationale: an audio frame arrives every ~20ms (~50Hz at 48kHz - // with 1024-sample blocks). The UI polls at 1Hz; without decay the bar - // would freeze at the loudest sample seen in the most recent buffer. - // We let the receiver keep the live max and let the UI apply visual - // decay on its end so the engine stays simple — see ParticipantViewModel. - private long _lastAudioPeakBits; + // Why a high-water mark and not a "latest peak": + // - Audio frames arrive at ~50Hz (1024-sample blocks @ 48kHz). + // - UI stats polling reads at 1Hz. + // If we only published the most recent buffer's peak, the UI would see + // exactly one of every ~50 audio frames per second — loud transients + // between reads would be invisible. By keeping the running max and + // consuming it on read, the UI sees the true peak across the entire + // polling interval, which is the actual behavior of a peak VU meter. + // + // Stored as the IEEE 754 long bits of a double so we can atomically + // CompareExchange-update without a struct lock. Volatile reads suffice + // for the consume side because we follow with an Exchange to 0. + private long _audioPeakBits; public NdiReceiver( INdiInterop interop, @@ -47,17 +50,32 @@ public sealed class NdiReceiver : IDisposable public long FramesCaptured => Interlocked.Read(ref _framesCaptured); /// - /// Most recent audio peak amplitude, in [0.0, 1.0]. Returns 0 when no - /// audio frame has been processed yet (silent source, video-only sender, - /// or audio loop hasn't started). Safe to call from any thread. + /// Reads the peak audio amplitude (in [0.0, 1.0]) seen since the last + /// call and resets the high-water mark to zero. The reset semantics are + /// what makes this a true peak meter — between two consecutive reads + /// the caller sees the loudest sample that crossed the receiver, not + /// just whatever the latest buffer happened to contain. + /// + /// Returns 0 if no audio has been received since the last read (silent + /// source, video-only sender, audio loop hasn't started, or the source + /// genuinely went quiet). Safe to call from any thread; reset is atomic. /// - public double LatestAudioPeak + public double ConsumeAudioPeak() { - get - { - var bits = Volatile.Read(ref _lastAudioPeakBits); - return BitConverter.Int64BitsToDouble(bits); - } + var bits = Interlocked.Exchange(ref _audioPeakBits, 0L); + return BitConverter.Int64BitsToDouble(bits); + } + + /// + /// Non-consuming peek at the current peak high-water mark. Useful for + /// observability paths that need to read the value without affecting the + /// max-since-last-read behavior — for example, an external diagnostics + /// dashboard that polls more often than the UI. + /// + public double PeekAudioPeak() + { + var bits = Volatile.Read(ref _audioPeakBits); + return BitConverter.Int64BitsToDouble(bits); } /// @@ -72,14 +90,28 @@ public sealed class NdiReceiver : IDisposable } /// - /// Captures one audio frame (or returns on timeout) and updates - /// . Test seam mirroring . + /// Captures one audio frame (or returns on timeout) and atomically + /// updates the high-water peak. Test seam mirroring . + /// The CAS loop is needed because two writers (this method, called from + /// the audio capture thread) and one reader (, + /// called from the UI poll thread) compete for the same field — a plain + /// Volatile.Write would lose updates if Consume fires between our read + /// and write. /// public void CaptureAudioOnce(int timeoutMs) { var peak = _interop.CaptureAudioPeak(_handle, timeoutMs); if (peak is null) return; - Volatile.Write(ref _lastAudioPeakBits, BitConverter.DoubleToInt64Bits(peak.Value)); + var newBits = BitConverter.DoubleToInt64Bits(peak.Value); + long current; + do + { + current = Volatile.Read(ref _audioPeakBits); + // If our peak is no louder than what's already there, leave it — + // somebody else (the audio thread itself, on a previous frame) + // already published a higher max. + if (peak.Value <= BitConverter.Int64BitsToDouble(current)) return; + } while (Interlocked.CompareExchange(ref _audioPeakBits, newBits, current) != current); } /// diff --git a/src/tests/TeamsISO.Engine.Tests/Fakes/FakeNdiInterop.cs b/src/tests/TeamsISO.Engine.Tests/Fakes/FakeNdiInterop.cs index c38e2e1..394745c 100644 --- a/src/tests/TeamsISO.Engine.Tests/Fakes/FakeNdiInterop.cs +++ b/src/tests/TeamsISO.Engine.Tests/Fakes/FakeNdiInterop.cs @@ -13,6 +13,8 @@ public sealed class FakeNdiInterop : INdiInterop public List Sources { get; } = new(); public ConcurrentDictionary> ReceiverFrames { get; } = new(); public ConcurrentDictionary> SentFrames { get; } = new(); + /// Optional per-source audio peak queue. Each dequeues one entry; null if empty (matches the production "timeout" behavior). + public ConcurrentDictionary> ReceiverAudioPeaks { get; } = new(); public string RuntimeVersion { get; set; } = "6.0.0"; public Dictionary ReceiverCreatedCount { get; } = new(); public Dictionary SenderCreatedCount { get; } = new(); @@ -44,6 +46,14 @@ public sealed class FakeNdiInterop : INdiInterop return null; // simulate timeout } + public double? CaptureAudioPeak(NdiReceiverHandle receiver, int timeoutMs) + { + var key = ((FakeReceiverHandle)receiver).Source; + if (ReceiverAudioPeaks.TryGetValue(key, out var q) && q.TryDequeue(out var peak)) + return peak; + return null; // no audio queued — simulate timeout + } + public NdiSenderHandle CreateSender(string outputName, string? groups = null) { SenderCreatedCount[outputName] = SenderCreatedCount.GetValueOrDefault(outputName) + 1; diff --git a/src/tests/TeamsISO.Engine.Tests/Pipeline/NdiReceiverTests.cs b/src/tests/TeamsISO.Engine.Tests/Pipeline/NdiReceiverTests.cs index 1d93422..3741280 100644 --- a/src/tests/TeamsISO.Engine.Tests/Pipeline/NdiReceiverTests.cs +++ b/src/tests/TeamsISO.Engine.Tests/Pipeline/NdiReceiverTests.cs @@ -59,4 +59,79 @@ public class NdiReceiverTests // Receiver was created exactly once interop.ReceiverCreatedCount[Source].Should().Be(1); } + + [Fact] + public void ConsumeAudioPeak_NoFramesProcessed_ReturnsZero() + { + var interop = new FakeNdiInterop(); + var output = Channel.CreateUnbounded(); + var receiver = new NdiReceiver(interop, Source, output.Writer, NullLogger.Instance); + + receiver.ConsumeAudioPeak().Should().Be(0.0); + } + + [Fact] + public void CaptureAudioOnce_PublishesPeak_ConsumeReturnsAndResets() + { + var interop = new FakeNdiInterop(); + interop.ReceiverAudioPeaks.GetOrAdd(Source, _ => new System.Collections.Concurrent.ConcurrentQueue()) + .Enqueue(0.42); + + var output = Channel.CreateUnbounded(); + var receiver = new NdiReceiver(interop, Source, output.Writer, NullLogger.Instance); + + receiver.CaptureAudioOnce(timeoutMs: 100); + + // Peek doesn't reset + receiver.PeekAudioPeak().Should().BeApproximately(0.42, precision: 0.0001); + + // First Consume returns the peak + receiver.ConsumeAudioPeak().Should().BeApproximately(0.42, precision: 0.0001); + + // Second Consume returns 0 — Consume has reset semantics + receiver.ConsumeAudioPeak().Should().Be(0.0); + } + + [Fact] + public void CaptureAudioOnce_KeepsHighWaterMarkAcrossMultipleFrames() + { + // Three frames with peaks 0.3, 0.8, 0.2. Without reset, the receiver + // must report the loudest (0.8) — that's the whole point of a peak + // meter. The "latest peak" naive overwrite would lose the 0.8 if a + // quieter 0.2 frame followed it. + var interop = new FakeNdiInterop(); + var q = interop.ReceiverAudioPeaks.GetOrAdd(Source, _ => new System.Collections.Concurrent.ConcurrentQueue()); + q.Enqueue(0.3); + q.Enqueue(0.8); + q.Enqueue(0.2); + + var output = Channel.CreateUnbounded(); + var receiver = new NdiReceiver(interop, Source, output.Writer, NullLogger.Instance); + + receiver.CaptureAudioOnce(timeoutMs: 100); + receiver.CaptureAudioOnce(timeoutMs: 100); + receiver.CaptureAudioOnce(timeoutMs: 100); + + receiver.ConsumeAudioPeak().Should().BeApproximately(0.8, precision: 0.0001); + } + + [Fact] + public void CaptureAudioOnce_NoFrameAvailable_LeavesHighWaterMarkUntouched() + { + // Establish a peak, then call CaptureAudioOnce when the fake has no + // queued frames. The high-water mark must NOT be reset just because + // a polling tick saw nothing — that would defeat the peak-hold + // semantic between consumer reads. + var interop = new FakeNdiInterop(); + interop.ReceiverAudioPeaks.GetOrAdd(Source, _ => new System.Collections.Concurrent.ConcurrentQueue()) + .Enqueue(0.5); + + var output = Channel.CreateUnbounded(); + var receiver = new NdiReceiver(interop, Source, output.Writer, NullLogger.Instance); + + receiver.CaptureAudioOnce(timeoutMs: 100); // peak now 0.5 + receiver.CaptureAudioOnce(timeoutMs: 100); // no frame; should be no-op + + receiver.PeekAudioPeak().Should().BeApproximately(0.5, precision: 0.0001); + } }