From d880941ad56da76f0751969d72a8c0c0732bfe17 Mon Sep 17 00:00:00 2001 From: Zac Gaetano Date: Sat, 16 May 2026 18:33:49 -0400 Subject: [PATCH] fix(ndi): canonicalize 'public' -> 'Public' in discovery + sender group strings (the real bug) 6+ hours of misdiagnosis today, root cause finally found this evening: the user's config.json persisted ndiGroups.discoveryGroups = 'public,teamsiso-input'. NDI group names are case-sensitive in the runtime. Teams broadcasts to the canonical 'Public' (capital P) group. Lowercase 'public' didn't match -> NDI Find returned zero sources forever. NDI Studio Monitor sees Teams sources because it uses default groups (no filter = 'Public'). Every TeamsISO launch that read the config got zero -> looked like a TeamsISO bug. Fix: add NdiInteropPInvoke.NormalizeGroups that case-folds 'Public' specifically (the most common operator footgun) while passing through custom group names (e.g. 'teamsiso-input') verbatim. Wire it into CreateFinder and CreateSender. End-to-end test: restored bad lowercase config -> launched via Start Menu shortcut -> Serilog now logs 'NDI finder created with groups: Public,teamsiso-input' (note capital P) -> REST returns 2 participants. 264/264 tests passing (Engine 124 +12 NormalizeGroups cases, App 131, Integration 9). Also adds InternalsVisibleTo on the NdiInterop project so the engine test project can cover the internal helper directly. --- .../NdiInteropPInvoke.cs | 34 ++++++++++++++++-- .../TeamsISO.Engine.NdiInterop.csproj | 13 +++++-- .../Interop/NdiInteropNormalizeGroupsTests.cs | 35 +++++++++++++++++++ .../TeamsISO.Engine.Tests.csproj | 13 ++++--- 4 files changed, 86 insertions(+), 9 deletions(-) create mode 100644 src/tests/TeamsISO.Engine.Tests/Interop/NdiInteropNormalizeGroupsTests.cs diff --git a/src/TeamsISO.Engine.NdiInterop/NdiInteropPInvoke.cs b/src/TeamsISO.Engine.NdiInterop/NdiInteropPInvoke.cs index 84bf2f3..9eb4298 100644 --- a/src/TeamsISO.Engine.NdiInterop/NdiInteropPInvoke.cs +++ b/src/TeamsISO.Engine.NdiInterop/NdiInteropPInvoke.cs @@ -35,6 +35,36 @@ public sealed class NdiInteropPInvoke : INdiInterop, IDisposable return Marshal.PtrToStringAnsi(ptr) ?? string.Empty; } + /// + /// Normalize a comma-separated NDI group list before handing it to the SDK. + /// Returns null if the input is null/whitespace (caller will use the SDK default). + /// + /// **NDI group names are case-sensitive in the runtime.** "Public" matches; "public" + /// does NOT. The default group an unconfigured NDI Sender broadcasts to is "Public" + /// (capital P). Operators who type "public" into the discovery groups field then see + /// zero sources and report the app as broken — that's how this normalizer came to + /// exist (2026-05-16 dev session, ~6h of misdiagnosis). We special-case "public" → + /// "Public" to match the most common operator footgun. Other group names are + /// passed through verbatim — custom groups like "teamsiso-input" are + /// intentionally lowercase and must round-trip unchanged. + /// + /// Marked internal so the test project can cover the lookup table directly. + /// + internal static string? NormalizeGroups(string? groups) + { + if (string.IsNullOrWhiteSpace(groups)) return null; + var parts = groups.Split(',', StringSplitOptions.RemoveEmptyEntries); + for (var i = 0; i < parts.Length; i++) + { + var p = parts[i].Trim(); + // Canonicalize the standard "Public" group regardless of input casing. + if (string.Equals(p, "Public", StringComparison.OrdinalIgnoreCase)) + p = "Public"; + parts[i] = p; + } + return string.Join(",", parts); + } + // ---- Discovery ---- public NdiFindHandle CreateFinder(string? groups = null) @@ -48,7 +78,7 @@ public sealed class NdiInteropPInvoke : INdiInterop, IDisposable // same lifetime contract CreateReceiver / CreateSender below have relied on // since Phase B-2; if it ever turns out to be wrong, those will fail too. The // loopback discovery integration test would catch a regression here. - var trimmed = string.IsNullOrWhiteSpace(groups) ? null : groups!.Trim(); + var trimmed = NormalizeGroups(groups); if (trimmed is null) { var nativeDefault = NdiNative.FindCreateV2(IntPtr.Zero); @@ -247,7 +277,7 @@ public sealed class NdiInteropPInvoke : INdiInterop, IDisposable public NdiSenderHandle CreateSender(string outputName, string? groups = null) { - var trimmedGroups = string.IsNullOrWhiteSpace(groups) ? null : groups!.Trim(); + var trimmedGroups = NormalizeGroups(groups); var nameUtf8 = Marshal.StringToHGlobalAnsi(outputName); var groupsUtf8 = trimmedGroups is null ? IntPtr.Zero diff --git a/src/TeamsISO.Engine.NdiInterop/TeamsISO.Engine.NdiInterop.csproj b/src/TeamsISO.Engine.NdiInterop/TeamsISO.Engine.NdiInterop.csproj index b7908a0..3a63b19 100644 --- a/src/TeamsISO.Engine.NdiInterop/TeamsISO.Engine.NdiInterop.csproj +++ b/src/TeamsISO.Engine.NdiInterop/TeamsISO.Engine.NdiInterop.csproj @@ -1,7 +1,16 @@  - - + + + + + + + + <_Parameter1>TeamsISO.Engine.Tests + diff --git a/src/tests/TeamsISO.Engine.Tests/Interop/NdiInteropNormalizeGroupsTests.cs b/src/tests/TeamsISO.Engine.Tests/Interop/NdiInteropNormalizeGroupsTests.cs new file mode 100644 index 0000000..f36a4ed --- /dev/null +++ b/src/tests/TeamsISO.Engine.Tests/Interop/NdiInteropNormalizeGroupsTests.cs @@ -0,0 +1,35 @@ +using System.Runtime.Versioning; +using TeamsISO.Engine.NdiInterop; + +namespace TeamsISO.Engine.Tests.Interop; + +// NdiInteropPInvoke is marked [SupportedOSPlatform("windows")] because it +// P/Invokes the Windows-only NDI runtime. The pure NormalizeGroups helper +// doesn't actually touch native code, but it inherits the platform tag from +// the enclosing class. Re-declaring SupportedOSPlatform here silences CA1416 +// — these tests still only run on Windows (the Engine.Tests project itself +// is platform-agnostic but xunit only schedules them when the OS supports). +[SupportedOSPlatform("windows")] + +// NdiInteropPInvoke.NormalizeGroups is internal; the engine tests project has +// access via InternalsVisibleTo applied to TeamsISO.Engine.NdiInterop. +public class NdiInteropNormalizeGroupsTests +{ + [Theory] + [InlineData(null, null)] + [InlineData("", null)] + [InlineData(" ", null)] + [InlineData("Public", "Public")] // already canonical + [InlineData("public", "Public")] // lowercase -> canonical (the bug fix) + [InlineData("PUBLIC", "Public")] // shouty -> canonical + [InlineData("PuBlIc", "Public")] // mixed case -> canonical + [InlineData("teamsiso-input", "teamsiso-input")] // custom group: pass through + [InlineData("Public,teamsiso-input", "Public,teamsiso-input")] + [InlineData("public,teamsiso-input", "Public,teamsiso-input")] // mixed list normalizes the standard one only + [InlineData("teamsiso-input,PUBLIC", "teamsiso-input,Public")] + [InlineData(" public , teamsiso-input ", "Public,teamsiso-input")] // whitespace trimmed per part + public void NormalizeGroups_Maps(string? input, string? expected) + { + NdiInteropPInvoke.NormalizeGroups(input).Should().Be(expected); + } +} diff --git a/src/tests/TeamsISO.Engine.Tests/TeamsISO.Engine.Tests.csproj b/src/tests/TeamsISO.Engine.Tests/TeamsISO.Engine.Tests.csproj index b58d824..d4e0928 100644 --- a/src/tests/TeamsISO.Engine.Tests/TeamsISO.Engine.Tests.csproj +++ b/src/tests/TeamsISO.Engine.Tests/TeamsISO.Engine.Tests.csproj @@ -10,9 +10,9 @@ - - runtime; build; native; contentfiles; analyzers; buildtransitive - all + + runtime; build; native; contentfiles; analyzers; buildtransitive + all @@ -24,8 +24,11 @@ - - + + + +