fix(ndi): canonicalize 'public' -> 'Public' in discovery + sender group strings (the real bug)
Some checks failed
CI / build-and-test (push) Failing after 26s

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.
This commit is contained in:
Zac Gaetano 2026-05-16 18:33:49 -04:00
parent 1cdd4ebd04
commit d880941ad5
4 changed files with 86 additions and 9 deletions

View file

@ -35,6 +35,36 @@ public sealed class NdiInteropPInvoke : INdiInterop, IDisposable
return Marshal.PtrToStringAnsi(ptr) ?? string.Empty; return Marshal.PtrToStringAnsi(ptr) ?? string.Empty;
} }
/// <summary>
/// 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.
/// </summary>
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 ---- // ---- Discovery ----
public NdiFindHandle CreateFinder(string? groups = null) 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 // 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 // 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. // 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) if (trimmed is null)
{ {
var nativeDefault = NdiNative.FindCreateV2(IntPtr.Zero); var nativeDefault = NdiNative.FindCreateV2(IntPtr.Zero);
@ -247,7 +277,7 @@ public sealed class NdiInteropPInvoke : INdiInterop, IDisposable
public NdiSenderHandle CreateSender(string outputName, string? groups = null) 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 nameUtf8 = Marshal.StringToHGlobalAnsi(outputName);
var groupsUtf8 = trimmedGroups is null var groupsUtf8 = trimmedGroups is null
? IntPtr.Zero ? IntPtr.Zero

View file

@ -4,6 +4,15 @@
<ProjectReference Include="..\TeamsISO.Engine\TeamsISO.Engine.csproj" /> <ProjectReference Include="..\TeamsISO.Engine\TeamsISO.Engine.csproj" />
</ItemGroup> </ItemGroup>
<!-- Grant the engine test project visibility into internals (specifically
NdiInteropPInvoke.NormalizeGroups, which gates the "public" vs "Public"
NDI group case-folding fix). -->
<ItemGroup>
<AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleToAttribute">
<_Parameter1>TeamsISO.Engine.Tests</_Parameter1>
</AssemblyAttribute>
</ItemGroup>
<PropertyGroup> <PropertyGroup>
<TargetFramework>net8.0</TargetFramework> <TargetFramework>net8.0</TargetFramework>
<ImplicitUsings>enable</ImplicitUsings> <ImplicitUsings>enable</ImplicitUsings>

View file

@ -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);
}
}

View file

@ -26,6 +26,9 @@
<ItemGroup> <ItemGroup>
<ProjectReference Include="..\..\TeamsISO.Engine\TeamsISO.Engine.csproj" /> <ProjectReference Include="..\..\TeamsISO.Engine\TeamsISO.Engine.csproj" />
<!-- Needed by NdiInteropNormalizeGroupsTests to reach the internal
NormalizeGroups helper (the "public" → "Public" case-folding fix). -->
<ProjectReference Include="..\..\TeamsISO.Engine.NdiInterop\TeamsISO.Engine.NdiInterop.csproj" />
</ItemGroup> </ItemGroup>
</Project> </Project>