From 1a0ff957aebbdbaea8d8234ec8ad60c61690ab40 Mon Sep 17 00:00:00 2001 From: Talon Date: Fri, 25 Sep 2026 19:05:14 +0200 Subject: [PATCH] fix(ios): stop the speaker toggle rebuilding the audio graph in a loop Toggling speaker output flipped the route back and forth indefinitely. Two loops, both of which made a rebuild produce the condition for the next one. Reconfiguring the session moves the route, and moving the route is reported back through RouteChangeNotification. Forcing the speaker takes a headset out of the route, which arrives as OldDeviceUnavailable, and releasing it brings the headset back as NewDeviceAvailable; neither is among the reasons the handler filters, so each rebuild answered its own echo with another rebuild. Nothing compared the reported route against the route the live graph was actually built on. Record that route at the end of Apply, once the session is configured, and rebuild only when a reported change differs from it; notifications that arrive while Apply is still running describe the change Apply is itself making and are ignored outright. The decision is AudioRouteWatcher in VoiceCat.Core, which is platform-agnostic and tested, following ControlPathWatcher; the route identity it compares is supplied by the caller, on iOS the UIDs of the current route's ports. A graph whose route is unchanged but broken is still the stall watchdog's to catch. The port override was also re-asserted on every Apply, so where the system wanted to hand output back to a connected headset each rebuild forced it to the speaker again and the resulting route change drove the next rebuild. It is now the one-shot request it should always have been, issued by the toggle alone; the DefaultToSpeaker category option is the part that persists across rebuilds. Also updates the route test from 724f7e9, which asserted the voice-chat preset clearing the speaker flag and the absence of the port override. Both were deliberately removed when speaker output became orthogonal to the preset, and the test should have been updated with them. --- clients/apple/VoiceCat.iOS/IosAudioRouter.cs | 44 ++++++++++++++++--- src/VoiceCat.Core/AudioRouteWatcher.cs | 36 +++++++++++++++ .../VoiceCat.Tests/AudioRouteWatcherTests.cs | 41 +++++++++++++++++ .../PublishServerScriptTests.cs | 23 +++++++--- 4 files changed, 133 insertions(+), 11 deletions(-) create mode 100644 src/VoiceCat.Core/AudioRouteWatcher.cs create mode 100644 tests/VoiceCat.Tests/AudioRouteWatcherTests.cs 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); }