diff --git a/src/PepperDash.Essentials.Core/Routing/Extensions.cs b/src/PepperDash.Essentials.Core/Routing/Extensions.cs index aed888051..8513ad763 100644 --- a/src/PepperDash.Essentials.Core/Routing/Extensions.cs +++ b/src/PepperDash.Essentials.Core/Routing/Extensions.cs @@ -460,26 +460,30 @@ private static void ReleaseRouteInternal(IRoutingInputs destination, string inpu RouteRequests.Remove(destination.Key); - var current = RouteDescriptorCollection.DefaultCollection.RemoveRouteDescriptor(destination, inputPortKey); - if (current != null) + // An AudioVideo route is stored as separate Audio and Video descriptors. Release all of + // them: releasing only the first orphans the other, so later releases act on a stale + // route and its output ports never drop out of use. + var current = RouteDescriptorCollection.DefaultCollection.RemoveRouteDescriptors(destination, inputPortKey); + var releasedSignalTypes = default(eRoutingSignalType); + + foreach (var descriptor in current) { - Debug.LogMessage(LogEventLevel.Information, "Releasing current route: {0}", destination, current.Source.Key); - current.ReleaseRoutes(clearRoute); + Debug.LogMessage(LogEventLevel.Information, "Releasing current route: {source} ({signalType})", destination, descriptor.Source.Key, descriptor.SignalType); + descriptor.ReleaseRoutes(clearRoute); + releasedSignalTypes |= descriptor.SignalType; } // Clear ICurrentSources on the destination if clearing the route if (clearRoute && destination is ICurrentSources currentSourcesDevice) { - if (current != null) + if (current.Count > 0) { - var signalType = current.SignalType; - - if (signalType.HasFlag(eRoutingSignalType.Audio) || signalType.HasFlag(eRoutingSignalType.AudioVideo)) + if (releasedSignalTypes.HasFlag(eRoutingSignalType.Audio)) { currentSourcesDevice.SetCurrentSource(eRoutingSignalType.Audio, null); } - if (signalType.HasFlag(eRoutingSignalType.Video) || signalType.HasFlag(eRoutingSignalType.AudioVideo)) + if (releasedSignalTypes.HasFlag(eRoutingSignalType.Video)) { currentSourcesDevice.SetCurrentSource(eRoutingSignalType.Video, null); } diff --git a/src/PepperDash.Essentials.Core/Routing/RouteDescriptorCollection.cs b/src/PepperDash.Essentials.Core/Routing/RouteDescriptorCollection.cs index d17ca8fe0..87e03a414 100644 --- a/src/PepperDash.Essentials.Core/Routing/RouteDescriptorCollection.cs +++ b/src/PepperDash.Essentials.Core/Routing/RouteDescriptorCollection.cs @@ -107,6 +107,11 @@ public RouteDescriptor GetRouteDescriptorForDestinationAndInputPort(IRoutingInpu /// Returns the RouteDescriptor for a given destination AND removes it from collection. /// Returns null if no route with the provided destination exists. /// + /// + /// Removes only the first match. An AudioVideo route is stored as two descriptors (one Audio, + /// one Video), so to release everything routed to a destination use + /// instead. + /// /// The destination device /// The input port key (optional) /// The matching RouteDescriptor or null if not found @@ -127,4 +132,36 @@ public RouteDescriptor RemoveRouteDescriptor(IRoutingInputs destination, string return descr; } -} \ No newline at end of file + + /// + /// Removes and returns every RouteDescriptor for a destination, optionally limited to one input + /// port. An AudioVideo route is stored as separate Audio and Video descriptors, so "the route to + /// this destination" can be more than one; releasing only one of them orphans the other, along + /// with its output ports' in-use registrations. + /// + /// The destination device + /// The input port key (optional). When empty, every descriptor for the destination is removed. + /// The removed descriptors, oldest first. Empty if none matched. + public List RemoveRouteDescriptors(IRoutingInputs destination, string inputPortKey = "") + { + var removed = RouteDescriptors + .Where(rd => rd.Destination == destination && + (string.IsNullOrEmpty(inputPortKey) || (rd.InputPort != null && rd.InputPort.Key == inputPortKey))) + .ToList(); + + foreach (var descriptor in removed) + { + RouteDescriptors.Remove(descriptor); + } + + if (removed.Count > 0) + { + RouteDescriptorCollectionChanged?.Invoke(this, EventArgs.Empty); + } + + Debug.LogMessage(LogEventLevel.Information, "Removed {count} route descriptor(s) for '{destination}':'{inputPortKey}'", + removed.Count, destination?.Key, string.IsNullOrEmpty(inputPortKey) ? "auto" : inputPortKey); + + return removed; + } +} diff --git a/src/PepperDash.Essentials.Core/Routing/RoutingFeedbackManager.cs b/src/PepperDash.Essentials.Core/Routing/RoutingFeedbackManager.cs index 5e5168866..73c995a83 100644 --- a/src/PepperDash.Essentials.Core/Routing/RoutingFeedbackManager.cs +++ b/src/PepperDash.Essentials.Core/Routing/RoutingFeedbackManager.cs @@ -396,7 +396,7 @@ RoutingInputPort inputPort // here, not that it currently is - so it can't be used to report feedback. try { - while (RouteDescriptorCollection.DefaultCollection.RemoveRouteDescriptor(destination, inputPort.Key) != null) { } + RouteDescriptorCollection.DefaultCollection.RemoveRouteDescriptors(destination, inputPort.Key); foreach (var signalType in new[] { eRoutingSignalType.Audio, eRoutingSignalType.Video }) { diff --git a/src/PepperDash.Essentials.Tests/Routing/RouteDescriptorCollectionTests.cs b/src/PepperDash.Essentials.Tests/Routing/RouteDescriptorCollectionTests.cs new file mode 100644 index 000000000..a48fec727 --- /dev/null +++ b/src/PepperDash.Essentials.Tests/Routing/RouteDescriptorCollectionTests.cs @@ -0,0 +1,172 @@ +using FluentAssertions; +using PepperDash.Essentials.Core; +using Xunit; + +namespace PepperDash.Essentials.Tests.Routing; + +/// +/// An AudioVideo route is stored as two descriptors (Audio and Video). Releasing a destination must +/// release both, or the other one is orphaned along with its output port's in-use registration +/// (issue #1494). +/// +public class RouteDescriptorCollectionTests +{ + private sealed class FakeSource : IRoutingOutputs + { + public FakeSource(string key) + { + Key = key; + Out = new RoutingOutputPort("out", eRoutingSignalType.AudioVideo, eRoutingPortConnectionType.Hdmi, "out", this); + OutputPorts = new RoutingPortCollection { Out }; + } + + public string Key { get; } + public RoutingOutputPort Out { get; } + public RoutingPortCollection OutputPorts { get; } + } + + private sealed class FakeSink : IRoutingInputs + { + public FakeSink(string key) + { + Key = key; + HdmiIn1 = new RoutingInputPort("hdmiIn1", eRoutingSignalType.AudioVideo, eRoutingPortConnectionType.Hdmi, "hdmiIn1", this); + HdmiIn2 = new RoutingInputPort("hdmiIn2", eRoutingSignalType.AudioVideo, eRoutingPortConnectionType.Hdmi, "hdmiIn2", this); + InputPorts = new RoutingPortCollection { HdmiIn1, HdmiIn2 }; + } + + public string Key { get; } + public RoutingInputPort HdmiIn1 { get; } + public RoutingInputPort HdmiIn2 { get; } + public RoutingPortCollection InputPorts { get; } + } + + private sealed class FakeMatrix : IRoutingMidpointWithFeedback + { + public FakeMatrix() + { + In1 = new RoutingInputPort("in1", eRoutingSignalType.AudioVideo, eRoutingPortConnectionType.Hdmi, "in1", this); + Out1 = new RoutingOutputPort("out1", eRoutingSignalType.AudioVideo, eRoutingPortConnectionType.Hdmi, "out1", this); + InputPorts = new RoutingPortCollection { In1 }; + OutputPorts = new RoutingPortCollection { Out1 }; + } + + public string Key => "matrix"; + public RoutingInputPort In1 { get; } + public RoutingOutputPort Out1 { get; } + public RoutingPortCollection InputPorts { get; } + public RoutingPortCollection OutputPorts { get; } + public List CurrentRoutes { get; } = new List(); +#pragma warning disable CS0067 // Required by the interface; these tests never raise it. + public event RouteChangedEventHandler? RouteChanged; +#pragma warning restore CS0067 + + public void ExecuteSwitch(object inputSelector, object outputSelector, eRoutingSignalType signalType) { } + public void ClearRoute(object outputSelector, eRoutingSignalType signalType) { } + } + + /// What RunRouteRequest stores for an AudioVideo route: one descriptor per signal. + private static (RouteDescriptor audio, RouteDescriptor video) AudioVideoRoute( + FakeSource source, FakeSink sink, FakeMatrix? matrix = null) + { + RouteDescriptor Make(eRoutingSignalType type) + { + var descriptor = new RouteDescriptor(source, sink, sink.HdmiIn1, source.Out, type); + if (matrix != null) + descriptor.Routes.Add(new RouteSwitchDescriptor(matrix.Out1, matrix.In1)); + return descriptor; + } + + return (Make(eRoutingSignalType.Audio), Make(eRoutingSignalType.Video)); + } + + [Fact] + public void RemoveRouteDescriptors_RemovesBothHalvesOfAnAudioVideoRoute() + { + var collection = new RouteDescriptorCollection(); + var sink = new FakeSink("display"); + var other = new FakeSink("other-display"); + var (audio, video) = AudioVideoRoute(new FakeSource("laptop"), sink); + var (otherAudio, _) = AudioVideoRoute(new FakeSource("laptop"), other); + collection.AddRouteDescriptor(audio); + collection.AddRouteDescriptor(video); + collection.AddRouteDescriptor(otherAudio); + + var removed = collection.RemoveRouteDescriptors(sink); + + removed.Should().BeEquivalentTo(new[] { audio, video }); + collection.Descriptors.Should().ContainSingle().Which.Should().BeSameAs(otherAudio); + } + + [Fact] + public void RemoveRouteDescriptors_WithAnInputPort_LeavesOtherInputsAlone() + { + var collection = new RouteDescriptorCollection(); + var source = new FakeSource("laptop"); + var sink = new FakeSink("codec"); + var onIn1 = new RouteDescriptor(source, sink, sink.HdmiIn1, source.Out, eRoutingSignalType.Video); + var onIn2 = new RouteDescriptor(source, sink, sink.HdmiIn2, source.Out, eRoutingSignalType.Video); + collection.AddRouteDescriptor(onIn1); + collection.AddRouteDescriptor(onIn2); + + collection.RemoveRouteDescriptors(sink, "hdmiIn1").Should().ContainSingle().Which.Should().BeSameAs(onIn1); + collection.Descriptors.Should().ContainSingle().Which.Should().BeSameAs(onIn2); + } + + [Fact] + public void RemoveRouteDescriptors_WhenNothingMatches_ReturnsEmptyWithoutRaisingChanged() + { + var collection = new RouteDescriptorCollection(); + var changed = 0; + collection.RouteDescriptorCollectionChanged += (_, _) => changed++; + + collection.RemoveRouteDescriptors(new FakeSink("display")).Should().BeEmpty(); + changed.Should().Be(0); + } + + [Fact] + public void ReleasingEveryRemovedDescriptor_ReturnsTheOutputPortToUnused() + { + var collection = new RouteDescriptorCollection(); + var matrix = new FakeMatrix(); + var sink = new FakeSink("display"); + var (audio, video) = AudioVideoRoute(new FakeSource("laptop"), sink, matrix); + foreach (var descriptor in new[] { audio, video }) + { + collection.AddRouteDescriptor(descriptor); + descriptor.ExecuteRoutes(); + } + matrix.Out1.InUseTracker.InUseCountFeedback.IntValue.Should().Be(2); + + foreach (var descriptor in collection.RemoveRouteDescriptors(sink)) + descriptor.ReleaseRoutes(); + + matrix.Out1.InUseTracker.InUseCountFeedback.IntValue.Should().Be(0); + } + + [Fact] + public void ChangingSourceRepeatedly_NeverAccumulatesDescriptors() + { + // The issue's reproduction: cycle a display through sources, releasing before each route + // exactly as ReleaseAndMakeRoute does. Each release must act on the previous source. + var collection = new RouteDescriptorCollection(); + var sink = new FakeSink("display"); + var sources = new[] { "zoom", "appletv", "iptv", "signage", "zoom", "appletv" } + .Select(key => new FakeSource(key)) + .ToList(); + + string? previous = null; + foreach (var source in sources) + { + var released = collection.RemoveRouteDescriptors(sink); + released.Select(d => d.Source.Key).Distinct().Should().Equal( + previous == null ? Array.Empty() : new[] { previous }); + + var (audio, video) = AudioVideoRoute(source, sink); + collection.AddRouteDescriptor(audio); + collection.AddRouteDescriptor(video); + collection.Descriptors.Should().HaveCount(2); + previous = source.Key; + } + } +}