diff --git a/clients/apple/VoiceCat.iOS/IosAudioRouter.cs b/clients/apple/VoiceCat.iOS/IosAudioRouter.cs index 6dc4d67..4fddf7c 100644 --- a/clients/apple/VoiceCat.iOS/IosAudioRouter.cs +++ b/clients/apple/VoiceCat.iOS/IosAudioRouter.cs @@ -19,6 +19,10 @@ internal sealed class IosAudioRouter private readonly NSUserDefaults defaults = NSUserDefaults.StandardUserDefaults; private bool applying; private bool speakerIsExplicit; + private readonly VoiceCat.Core.AudioRouteWatcher route = new(); + // Set by an explicit speaker choice and consumed by the next Apply. The override is a one-shot + // request, never steady-state configuration; see SetForceSpeaker. + private bool overridePending; private System.Threading.Timer? watchdog; private long lastRenderCallbacks = -1; private int watchdogMisses; @@ -86,7 +90,12 @@ internal sealed class IosAudioRouter // Deliberately does not move the preset to Advanced: the speaker is an output choice every // named preset supports, so a voice-chat user can take a call on the speaker without losing // the capture configuration the preset stands for. - internal void SetForceSpeaker(bool value) { ForceSpeaker = value; speakerIsExplicit = true; SaveAndReconfigure(); } + // The port override is issued once, on the toggle, and never re-issued by a later rebuild. + // Re-asserting it on every Apply means fighting iOS for the route: where the system wants to + // hand output back to a connected headset, each rebuild forces it back to the speaker, the + // route change that follows drives another rebuild, and the audio flips back and forth. The + // DefaultToSpeaker category option below is the part that does persist across rebuilds. + internal void SetForceSpeaker(bool value) { ForceSpeaker = value; speakerIsExplicit = overridePending = true; SaveAndReconfigure(); } internal void SetVoiceProcessing(bool value) { VoiceProcessing = value; SaveAndReconfigure(); } internal void SetAutomaticGainControl(bool value) { AutomaticGainControl = value; SaveAndReconfigure(); } internal void SetCaptureChannels(int value) { CaptureChannels = value == 2 ? 2 : 1; Preset = IosAudioPreset.Advanced; SaveAndReconfigure(); } @@ -134,16 +143,30 @@ internal sealed class IosAudioRouter $"pattern={session.InputDataSource?.SelectedPolarPattern.ToString() ?? "default"}"); // DefaultToSpeaker only decides where audio goes when nothing else is connected, so a // user who asks for the speaker with a headset attached needs the port override as - // well. Only an explicit request forces it: left alone, the route stays free to follow - // an HFP headset, which is what the voice-chat preset depends on. - session.OverrideOutputAudioPort(ForceSpeaker && BluetoothMode != IosBluetoothMode.BuiltInMicA2dp - ? AVAudioSessionPortOverride.Speaker : AVAudioSessionPortOverride.None, out _); + // well. Only the toggle itself issues it, and only once. + if (overridePending) + { + overridePending = false; + session.OverrideOutputAudioPort(ForceSpeaker && BluetoothMode != IosBluetoothMode.BuiltInMicA2dp + ? AVAudioSessionPortOverride.Speaker : AVAudioSessionPortOverride.None, out _); + } RefreshRoutes(); + // Last, so it describes the route this graph is being built on rather than the one it + // replaced. HandleRouteChange compares against it to recognise its own echo. + route.Built(RouteSignature(session)); ResetWatchdog(); EnsureWatchdog(); } finally { applying = false; } } + // Identifies the hardware carrying audio. A graph has to be rebuilt when this changes, because + // the input format changes with it; when it has not changed, there is nothing to rebuild for. + private static string RouteSignature(AVAudioSession session) + { + AVAudioSessionRouteDescription current = session.CurrentRoute; + return string.Join('|', current.Inputs.Select(value => value.UID).Concat(current.Outputs.Select(value => value.UID))); + } + private void ApplyInputSelection(AVAudioSession session) { AVAudioSessionPortDescription? port = session.AvailableInputs?.FirstOrDefault(value => value.UID == SelectedInputId); @@ -236,7 +259,8 @@ internal sealed class IosAudioRouter private void Set(string key, string? value) { if (value is null) defaults.RemoveObject(key); else defaults.SetString(value, key); } internal void Deactivate() { - watchdog?.Dispose(); watchdog = null; ResetWatchdog(); + // A released session has no route the next graph can be compared against. + watchdog?.Dispose(); watchdog = null; ResetWatchdog(); route.Reset(); AVAudioSession.SharedInstance().SetActive(false, AVAudioSessionSetActiveOptions.NotifyOthersOnDeactivation, out _); } internal void EnsureAudio(string reason) @@ -308,6 +332,14 @@ internal sealed class IosAudioRouter or AVAudioSessionRouteChangeReason.Override or AVAudioSessionRouteChangeReason.RouteConfigurationChange) return; + // Apply is mid-flight: this notification describes the change Apply is itself making. + if (applying) return; + // Every rebuild moves the route, and moving the route notifies here. Forcing the speaker + // takes a headset out of the route as OldDeviceUnavailable and releasing it brings the + // headset back as NewDeviceAvailable, neither of which is filtered above, so a rebuild + // that answered its own echo would rebuild again without end. The route the graph was + // built on is what decides: if it still carries audio, there is nothing to recover from. + if (!route.ShouldRebuild(RouteSignature(AVAudioSession.SharedInstance()))) return; Recover($"route change ({reason})"); } private void HandleInterruption(NSNotification note) diff --git a/src/VoiceCat.Core/AudioRouteWatcher.cs b/src/VoiceCat.Core/AudioRouteWatcher.cs new file mode 100644 index 0000000..ff56394 --- /dev/null +++ b/src/VoiceCat.Core/AudioRouteWatcher.cs @@ -0,0 +1,36 @@ +namespace VoiceCat.Core; + +/// Decides whether a reported audio-route change is a reason to rebuild the audio graph, or the +/// echo of the rebuild that produced it. +/// +/// The distinction matters because reconfiguring a session moves the route, and moving the route +/// is reported back as a change. Taking a headset out of the route to force the speaker is +/// reported as a device becoming unavailable, and releasing the speaker is reported as one +/// becoming available: act on either and the rebuild answers its own echo, which is an endless +/// flip between two routes rather than one reconfiguration. +/// +/// The rule is that only the hardware carrying audio decides. A graph must be rebuilt when that +/// changes, because the input format changes with it; when the route still matches the one the +/// live graph was built on, there is nothing to rebuild for. +/// +/// Platform-agnostic on purpose: the caller supplies whatever stable route identity its OS +/// reports (on iOS, the UIDs of the current route's ports). +public sealed class AudioRouteWatcher +{ + private readonly object gate = new(); + private string built = ""; + + /// Records the route a graph has just been built on. Called after the session is configured, + /// so it describes the new route rather than the one it replaced. + public void Built(string signature) { lock (gate) built = signature ?? ""; } + + /// Forgets the recorded route, for a session that has been released and has no route left. + public void Reset() { lock (gate) built = ""; } + + /// Returns true when the reported route differs from the one the live graph was built on. + /// A graph built before anything was recorded is rebuilt, since nothing is known about it. + public bool ShouldRebuild(string signature) + { + lock (gate) return built.Length == 0 || built != (signature ?? ""); + } +} diff --git a/tests/VoiceCat.Tests/AudioRouteWatcherTests.cs b/tests/VoiceCat.Tests/AudioRouteWatcherTests.cs new file mode 100644 index 0000000..26be969 --- /dev/null +++ b/tests/VoiceCat.Tests/AudioRouteWatcherTests.cs @@ -0,0 +1,41 @@ +using VoiceCat.Core; + +namespace VoiceCat.Tests; + +public class AudioRouteWatcherTests +{ + // Reconfiguring a session moves the route, and moving the route is reported back as a change. + // Acting on that echo rebuilds the graph that caused it, which is the audio flipping between + // two routes without end rather than one reconfiguration. + [Fact] + public void ARouteThatMatchesTheLiveGraphIsNotRebuiltFor() + { + var watcher = new AudioRouteWatcher(); + + // Nothing is known about a graph built before anything was recorded. + Assert.True(watcher.ShouldRebuild("builtInMic|builtInSpeaker")); + + watcher.Built("builtInMic|builtInSpeaker"); + // The echo of the Apply that produced this route, however often it is reported. + Assert.False(watcher.ShouldRebuild("builtInMic|builtInSpeaker")); + Assert.False(watcher.ShouldRebuild("builtInMic|builtInSpeaker")); + + // A headset arriving is hardware the graph is not built on: rebuild, then settle again. + Assert.True(watcher.ShouldRebuild("headsetMic|headset")); + watcher.Built("headsetMic|headset"); + Assert.False(watcher.ShouldRebuild("headsetMic|headset")); + + // Forcing the speaker takes the headset out of the route. The rebuild that does it records + // the result, so the change it is reported as does not drive a second rebuild. + watcher.Built("builtInMic|builtInSpeaker"); + Assert.False(watcher.ShouldRebuild("builtInMic|builtInSpeaker")); + // And releasing it, which brings the headset back, is one rebuild and no more. + Assert.True(watcher.ShouldRebuild("headsetMic|headset")); + watcher.Built("headsetMic|headset"); + Assert.False(watcher.ShouldRebuild("headsetMic|headset")); + + // A released session has no route left to compare against. + watcher.Reset(); + Assert.True(watcher.ShouldRebuild("headsetMic|headset")); + } +} diff --git a/tests/VoiceCat.Tests/PublishServerScriptTests.cs b/tests/VoiceCat.Tests/PublishServerScriptTests.cs index 35b2e5f..78ae283 100644 --- a/tests/VoiceCat.Tests/PublishServerScriptTests.cs +++ b/tests/VoiceCat.Tests/PublishServerScriptTests.cs @@ -222,16 +222,29 @@ public class PublishServerScriptTests } [Fact] - public async Task IosVoiceChatLeavesBluetoothHeadsetRouteToSystem() + public async Task IosSpeakerOutputIsAnOrthogonalOneShotChoice() { string router = await File.ReadAllTextAsync(Path.Combine( FindRoot(), "clients", "apple", "VoiceCat.iOS", "IosAudioRouter.cs")); - Assert.Contains("if (preset == IosAudioPreset.VoiceChat) ForceSpeaker = false", router); - Assert.Contains("if (Preset == IosAudioPreset.VoiceChat) ForceSpeaker = false", router); + // Speaker output decides where audio goes, not how it is captured, so it must not take the + // preset with it, and no preset may clear it back. + Assert.Contains("ForceSpeaker = value; speakerIsExplicit = overridePending = true", router); + Assert.DoesNotContain("ForceSpeaker = value; Preset = IosAudioPreset.Advanced", router); + Assert.DoesNotContain("IosAudioPreset.VoiceChat) ForceSpeaker = false", router); + // The stored flag is honoured only once the user has actually chosen, so the value older + // installs inherited from the voice-chat preset cannot pin a headset user to the speaker. + Assert.Contains("cat.voice.audio.speakerIsExplicit", router); + // The port override is the toggle's one-shot request. Re-asserting it on every rebuild + // means fighting the system for the route, which the user sees as audio flipping. + Assert.Contains("if (overridePending)", router); + Assert.Contains("overridePending = false;", router); + // A route change that reports the route the live graph was built on is the echo of the + // Apply that produced it, and answering it rebuilds without end. + Assert.Contains("if (applying) return;", router); + Assert.Contains("!route.ShouldRebuild(RouteSignature(AVAudioSession.SharedInstance()))", router); + Assert.Contains("route.Built(RouteSignature(session))", router); Assert.Contains("if (Preset == IosAudioPreset.VoiceChat) { SelectedInputId = null; SelectedDataSourceId = null; }", router); - Assert.Contains("ForceSpeaker = value; Preset = IosAudioPreset.Advanced", router); - Assert.DoesNotContain("OverrideOutputAudioPort(", router); Assert.DoesNotContain("SelectedInputId ??= session.PreferredInput", router); }