diff --git a/PROGRESS.md b/PROGRESS.md index 8e9bafc..5cf28ae 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -37,9 +37,19 @@ the element a double tap was aimed at. The iOS audio graph is rebuilt only when the audio configuration changed. A lost connection unbinds the client but keeps the session, graph, and route alive, so a reconnect rebinds to a -live Bluetooth HFP link instead of renegotiating one; restoring voice reuses a running capture -tap of the same width; and foregrounding ensures the graph is running rather than rebuilding it. -Route changes, media-services resets, and the stall watchdog still force a full rebuild. +live Bluetooth HFP link instead of renegotiating one, and foregrounding ensures the graph is +running rather than rebuilding it. The input side — input node, voice processing, and capture tap +— is built once per session rather than added when voice is joined, so joining and leaving voice +name a stream instead of replacing the graph. The input is rendered through a silent mixer, +because the voice-processing IO unit only stays up while the input is in the render chain. Route +changes no longer force a rebuild; media-services resets still do, and the stall watchdog now +backs off and stops after four failed attempts instead of rebuilding without end. + +Voice-chat capture is not yet reliable on the first attempt: a graph with voice processing still +sometimes fails to start and is only recovered by a watchdog rebuild, which costs seconds before +audio appears. Speaker output is an explicit choice that no longer changes the preset. Every +rebuild logs `VC_REBUILD cause=`, and `VC_START`/`VC_START_CHECK` record whether the graph +survived its start; that instrumentation is what the remaining investigation needs. The media path now survives changing networks. A client whose source address changes proves possession of its media key from the new address with an authenticated `Rebind` frame and the @@ -63,6 +73,8 @@ cap the applied Opus hint at 30%, and feed it back over TLS. ## Release gates +- Find why an iOS graph with voice processing intermittently starts and then stops within a + second, so voice-chat capture comes up on the first attempt rather than after a watchdog rebuild. - Run real multi-person calls on Windows, macOS, and physical iOS hardware, including adaptive 20/40/60 ms buffering, duration-aware DRED/FEC, automatic packet-loss feedback, and mismatched input/output endpoints. diff --git a/clients/apple/VoiceCat.iOS/IosAudioEngine.cs b/clients/apple/VoiceCat.iOS/IosAudioEngine.cs index 89a6b77..8728e19 100644 --- a/clients/apple/VoiceCat.iOS/IosAudioEngine.cs +++ b/clients/apple/VoiceCat.iOS/IosAudioEngine.cs @@ -39,13 +39,42 @@ internal sealed class IosAudioEngine private AVAudioConverterInputHandler? inputProvider; private bool inputProvided; private bool tapInstalled; + private AVAudioMixerNode? captureSink; // The voice-processing state the live graph was actually built with, so Reconfigure can tell // a settings change apart from a re-check of an unchanged graph. private bool voiceProcessing; + // The input format the tap, the converter and the ring capacity were all built for. Asked of + // the input node rather than of AVAudioSession: the session's reported rate and channel count + // do not settle until after the graph has started, so comparing against them reports a change + // that has not happened and every rebuild reports it again. + private long lastRebuildAt; + // How long a freshly built graph is given to produce its first render callback. Enabling voice + // processing rebuilds both halves of the IO, which takes several hundred milliseconds, posts a + // configuration change and reports the engine as not running while it happens. Everything that + // judges a graph dead has to wait this out, or it tears down the graph it just built and the + // replacement reports exactly the same thing. + private const int SettlingMilliseconds = 2_000; + private double builtInputRate; + private uint builtInputChannels; + // The capture width the tap and converter were built for. The input side of the graph exists + // for the whole session, so this is what decides whether joining voice needs a rebuild at all. + private int builtCaptureChannels; private long captureCallbacks, capturedFrames, convertedFrames, rejectedFeeds, converterFailures, stereoFrames, stereoDifferentFrames; private long renderCallbacks, lastRenderTimestamp; internal bool IsConnected { get; private set; } internal bool IsRunning => engine?.Running == true; + // True while the last rebuild is still starting up, and so cannot be judged. + internal bool Settling => Environment.TickCount64 - Volatile.Read(ref lastRebuildAt) < SettlingMilliseconds; + // True when the graph's own input no longer has the format its tap and converter were built + // for, which is the only thing a route change can do that AVAudioEngine cannot absorb on its + // own. The engine's view is the one that is stable: it changes when the hardware actually + // changes, which is precisely what a configuration change reports. + internal bool HardwareChanged() + { + if (engine is not { } live || !tapInstalled) return false; + AVAudioFormat format = live.InputNode.GetBusOutputFormat(0); + return format.SampleRate != builtInputRate || format.ChannelCount != builtInputChannels; + } // The watchdog compares this across ticks: a graph that stops calling back while it still // reports Running leaves the whole device-clocked pipeline frozen until it is rebuilt. internal long RenderCallbacks => Interlocked.Read(ref renderCallbacks); @@ -78,29 +107,33 @@ internal sealed class IosAudioEngine internal void StartMicrophone(uint streamId, int channels) { - MicrophoneRoute? current = Volatile.Read(ref microphone); - // Restoring voice after a reconnect only changes which stream id the existing capture - // feeds. Reuse a running tap of the same width instead of rebuilding the graph around it. - if (current is { StreamId: 0 } && current.Channels == Math.Clamp(channels, 1, 2) && tapInstalled && engine?.Running == true) - { - current.Ring.Resynchronize(); - Volatile.Write(ref microphone, CreateMicrophoneRoute(streamId, channels)); return; - } - Volatile.Write(ref microphone, CreateMicrophoneRoute(streamId, channels)); Rebuild(); + // Capture is already running: the graph carries the tap for the whole session. Joining voice + // only names the stream the frames belong to, so it is a state change and not a rebuild. The + // width is the one exception, because the tap format and converter are built around it. + MicrophoneRoute next = CreateMicrophoneRoute(streamId, channels); + bool reusable = tapInstalled && engine?.Running == true && builtCaptureChannels == next.Channels; + Volatile.Write(ref microphone, next); + if (!reusable) Rebuild(); } - internal void StopMicrophone() { Volatile.Write(ref microphone, null); Rebuild(); } + // Leaving voice never rebuilds. Capture keeps running into a route nobody reads, exactly as it + // does between connecting and joining, which costs one discarded conversion per callback and + // saves tearing down the voice-processing IO only to build it again on the next join. + internal void StopMicrophone() { Volatile.Write(ref microphone, null); } // `force` rebuilds unconditionally, which is what a route change, a media-services reset and // the stall watchdog all need. Callers that are only re-checking a graph they expect to be // healthy — foregrounding, above all — pass false and get a no-op when nothing has changed. - internal void Reconfigure(bool force = true) + internal void Reconfigure(bool force = true, string cause = "reconfigure") { if (!IsConnected) return; + // A graph that has not finished starting is not a graph to replace. Only an explicit + // configuration change forces its way past this. + if (!force && Settling) return; MicrophoneRoute? current = Volatile.Read(ref microphone); int channels = IosAudioRouter.Shared.CaptureChannels; - if (!force && engine?.Running == true && tapInstalled == (current is not null) && - (current?.Channels ?? channels) == channels && voiceProcessing == IosAudioRouter.Shared.UsesVoiceProcessing) return; + if (!force && engine?.Running == true && tapInstalled && builtCaptureChannels == channels + && voiceProcessing == IosAudioRouter.Shared.UsesVoiceProcessing && !HardwareChanged()) return; if (current is not null) { // Stop the old tap before publishing a route with a different sample width. Otherwise @@ -109,7 +142,7 @@ internal sealed class IosAudioEngine if (current.Channels != channels && current.StreamId != 0) client?.Audio.SetCaptureChannels(current.StreamId, channels); Volatile.Write(ref microphone, CreateMicrophoneRoute(current.StreamId, channels)); } - Rebuild(); + Rebuild(cause); } private static MicrophoneRoute CreateMicrophoneRoute(uint streamId, int channels) @@ -119,16 +152,31 @@ internal sealed class IosAudioEngine internal bool EnsureRunning() { if (!IsConnected || engine?.Running == true) return true; + // Still starting: report it as running rather than replacing it, since that is what it is + // about to be, and a rebuild here would start the cycle over. + if (Settling) return true; Rebuild(); return engine?.Running == true; } - private void Rebuild() + private void Rebuild([System.Runtime.CompilerServices.CallerMemberName] string cause = "") { - DestroyGraph(); MicrophoneRoute? route = Volatile.Read(ref microphone); IosAudioRouter.Shared.Apply(route is not null); + Console.Error.WriteLine($"VC_REBUILD cause={cause} running={engine?.Running == true} callbacks={RenderCallbacks}"); + DestroyGraph(); MicrophoneRoute? route = Volatile.Read(ref microphone); + // The input node, voice processing and the tap are built once and kept for the session, not + // added when voice is joined. Adding them later means replacing a running graph that has no + // voice processing with one that has it, and that transition is what fails: the new IO unit + // starts and is torn down again within a second, non-deterministically, which is the + // rebuild storm the watchdog then chases. Steady-state voice processing is reliable; only + // the change into it is not. So capture always runs, and joining voice only decides which + // stream its frames belong to — Capture and PumpMicrophoneChunk both discard without one. + bool captures = IsConnected; + int captureChannels = route?.Channels ?? Math.Clamp(IosAudioRouter.Shared.CaptureChannels, 1, 2); + IosAudioRouter.Shared.Apply(captures); voiceProcessing = IosAudioRouter.Shared.UsesVoiceProcessing; + builtInputRate = 0; builtInputChannels = 0; var next = new AVAudioEngine(); AVAudioInputNode? input = null; - if (route is not null) + if (captures) { // Enabling voice processing rebuilds both sides of AVAudioEngine. Do it before any // formats are queried or nodes are connected so the graph is built from the final IO. @@ -144,12 +192,14 @@ internal sealed class IosAudioEngine if (OperatingSystem.IsIOSVersionAtLeast(27)) next.Connect(source, next.MainMixerNode, outputFormat, out connectionError); else next.Connect(source, next.MainMixerNode, outputFormat); if (connectionError is not null) throw new InvalidOperationException(connectionError.LocalizedDescription); - if (route is not null) + if (captures) { AVAudioFormat inputFormat = input!.GetBusOutputFormat(0); - Console.Error.WriteLine($"VC_GRAPH vpio={IosAudioRouter.Shared.UsesVoiceProcessing} requestedCh={route.Channels} " + + Console.Error.WriteLine($"VC_GRAPH vpio={IosAudioRouter.Shared.UsesVoiceProcessing} requestedCh={captureChannels} " + $"inputCh={inputFormat.ChannelCount} inputRate={inputFormat.SampleRate}"); - microphoneFormat = new(AVAudioCommonFormat.PCMInt16, 48_000, (uint)route.Channels, true); + builtInputRate = inputFormat.SampleRate; builtInputChannels = inputFormat.ChannelCount; + microphoneFormat = new(AVAudioCommonFormat.PCMInt16, 48_000, (uint)captureChannels, true); + builtCaptureChannels = captureChannels; microphoneConverter = new(inputFormat, microphoneFormat); uint capacity = checked((uint)Math.Ceiling(MaximumCaptureCallbackFrames * 48_000 / inputFormat.SampleRate) + 64); convertedMicrophone = new(microphoneFormat, capacity); @@ -159,17 +209,64 @@ internal sealed class IosAudioEngine else input.InstallTapOnBus(0, 960, inputFormat, Capture); if (tapError is not null) throw new InvalidOperationException(tapError.LocalizedDescription); tapInstalled = true; + // With voice processing the input and output run as one IO unit, and the unit only stays + // up while the input is part of the render chain. A tap alone does not put it there: the + // engine starts and the IO is torn down again within a second, which is why a graph with + // voice processing came up perhaps one time in four. Route the input through a silent + // mixer so it is genuinely rendered, contributing nothing audible. + captureSink = new AVAudioMixerNode(); + next.AttachNode(captureSink); + NSError? sinkError = null; + if (OperatingSystem.IsIOSVersionAtLeast(27)) + { + next.Connect(input, captureSink, inputFormat, out sinkError); + if (sinkError is null) next.Connect(captureSink, next.MainMixerNode, outputFormat, out sinkError); + } + else + { + next.Connect(input, captureSink, inputFormat); + next.Connect(captureSink, next.MainMixerNode, outputFormat); + } + if (sinkError is not null) throw new InvalidOperationException(sinkError.LocalizedDescription); + captureSink.OutputVolume = 0f; } next.Prepare(); if (!next.StartAndReturnError(out NSError? error)) { next.Dispose(); throw new InvalidOperationException(error.LocalizedDescription); } engine = next; + { + AVAudioSession live = AVAudioSession.SharedInstance(); + Console.Error.WriteLine($"VC_START running={next.Running} builtCh={builtInputChannels} sessionRate={live.SampleRate} " + + $"sessionInCh={live.InputNumberOfChannels} sessionOutCh={live.OutputNumberOfChannels} inputAvailable={live.InputAvailable} " + + $"other={live.OtherAudioPlaying} mode={live.Mode} options={live.CategoryOptions} io={live.IOBufferDuration:F4} " + + $"out={string.Join(',', live.CurrentRoute.Outputs.Select(value => value.PortType.ToString()))} " + + $"in={string.Join(',', live.CurrentRoute.Inputs.Select(value => value.PortType.ToString()))}"); + // A graph that starts and then stops on its own is the failure that matters, and it is + // invisible at start: check again once the IO has had time to come up. + AVAudioEngine started = next; + System.Threading.Tasks.Task.Delay(750).ContinueWith(_ => + UIApplication.SharedApplication.BeginInvokeOnMainThread(() => + { + if (!ReferenceEquals(engine, started)) return; + Console.Error.WriteLine($"VC_START_CHECK running={started.Running} render={RenderCallbacks} capture={Interlocked.Read(ref captureCallbacks)}"); + })); + } + Volatile.Write(ref lastRebuildAt, Environment.TickCount64); engineConfigurationObserver = Foundation.NSNotificationCenter.DefaultCenter.AddObserver( AVAudioEngine.ConfigurationChangeNotification, notification => { if (!ReferenceEquals(notification.Object, next)) return; UIApplication.SharedApplication.BeginInvokeOnMainThread(() => { - if (IsConnected && ReferenceEquals(engine, next) && !next.Running) Rebuild(); + if (!IsConnected || !ReferenceEquals(engine, next)) return; + // Building this graph is itself what posted most of these: enabling voice + // processing rebuilds the IO, and the engine reads as stopped until that + // finishes. Rebuilding then replaces a graph that was about to run with one + // that reports the same thing, without end. + if (Settling) return; + // A configuration change with the graph still running is usually one + // AVAudioEngine has already absorbed; it matters here only when the hardware + // the tap and converter were built around moved. + if (!next.Running || HardwareChanged()) Rebuild("configuration change"); }); }, next); } @@ -296,7 +393,9 @@ internal sealed class IosAudioEngine if (engine is { } old) { if (tapInstalled) old.InputNode.RemoveTapOnBus(0); - old.Stop(); if (source is not null) old.DetachNode(source); old.Dispose(); + old.Stop(); if (source is not null) old.DetachNode(source); + if (captureSink is not null) { old.DetachNode(captureSink); captureSink.Dispose(); captureSink = null; } + old.Dispose(); } tapInstalled = false; pendingInput = null; inputProvider = null; lastRenderTimestamp = 0; microphoneCredit = 0; convertedMicrophone?.Dispose(); convertedMicrophone = null; diff --git a/clients/apple/VoiceCat.iOS/IosAudioRouter.cs b/clients/apple/VoiceCat.iOS/IosAudioRouter.cs index 4fddf7c..40476cf 100644 --- a/clients/apple/VoiceCat.iOS/IosAudioRouter.cs +++ b/clients/apple/VoiceCat.iOS/IosAudioRouter.cs @@ -19,13 +19,24 @@ internal sealed class IosAudioRouter private readonly NSUserDefaults defaults = NSUserDefaults.StandardUserDefaults; private bool applying; private bool speakerIsExplicit; - private readonly VoiceCat.Core.AudioRouteWatcher route = new(); + // Whether this session actually has a stereo capsule configuration to undo. Clearing one that + // was never applied is not free: SetPreferredPolarPattern(Unknown) drops the built-in array out + // of the beamformed mono configuration VoiceChat mode selects and exposes its four raw + // channels, which the voice-processing IO cannot start against. + private bool stereoApplied; // 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; + private int watchdogAttempts; + private long nextWatchdogAttempt; + // A rebuild that does not restore the callbacks is not worth repeating at the same rate, and + // never worth repeating forever: an unbounded retry turns a graph that cannot start into a + // storm of session reconfiguration, which is both what the user hears and what hides the + // reason from the log. Back off, then stop and say so. + private const int MaximumWatchdogAttempts = 4; private bool watchdogTicking; private bool interrupted; internal event Action? Changed; @@ -151,27 +162,16 @@ internal sealed class IosAudioRouter ? 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); if (CaptureChannels == 2) port ??= session.AvailableInputs?.FirstOrDefault(value => value.PortType == AVAudioSession.PortBuiltInMic); - if (port is null) { if (CaptureChannels == 1) ClearStereoPolarPattern(session); return; } + if (port is null) { if (CaptureChannels == 1 && stereoApplied) ClearStereo(session); return; } if (CaptureChannels == 2) { // Polar-pattern discovery alone is insufficient on current iPhones: until a stereo @@ -181,11 +181,12 @@ internal sealed class IosAudioRouter // it also gives Core Audio an unambiguous left/right mapping before graph creation. if (!session.SetPreferredInputOrientation(AVAudioStereoOrientation.Portrait, out NSError? orientationError)) throw new InvalidOperationException(orientationError?.LocalizedDescription ?? "Could not set the stereo microphone orientation."); + stereoApplied = true; } AVAudioSessionDataSourceDescription? source = CaptureChannels == 2 ? port.DataSources?.FirstOrDefault(SupportsStereoPolarPattern) : port.DataSources?.FirstOrDefault(value => value.DataSourceID.ToString() == SelectedDataSourceId); - if (CaptureChannels == 1 && source is null) ClearStereoPolarPattern(session); + if (CaptureChannels == 1 && source is null && stereoApplied) ClearStereo(session); if (source is not null) { if (!port.SetPreferredDataSource(source, out NSError? portError)) throw new InvalidOperationException(portError.LocalizedDescription); @@ -236,6 +237,8 @@ internal sealed class IosAudioRouter internal static extern byte SetObject(NativeHandle receiver, NativeHandle selector, NativeHandle value, ref NativeHandle error); } + private void ClearStereo(AVAudioSession session) { stereoApplied = false; ClearStereoPolarPattern(session); } + private static void ClearStereoPolarPattern(AVAudioSession session) { session.SetPreferredInputOrientation(AVAudioStereoOrientation.None, out _); @@ -253,14 +256,15 @@ internal sealed class IosAudioRouter defaults.SetBool(speakerIsExplicit, "cat.voice.audio.speakerIsExplicit"); defaults.SetBool(VoiceProcessing, "cat.voice.audio.voiceProcessing"); defaults.SetBool(AutomaticGainControl, "cat.voice.audio.agc"); defaults.SetInt(CaptureChannels, "cat.voice.audio.captureChannels"); Set("cat.voice.audio.inputPortId", SelectedInputId); Set("cat.voice.audio.dataSourceId", SelectedDataSourceId); defaults.SetString(SelectedPolarPattern.ToString(), "cat.voice.audio.polarPattern"); defaults.Synchronize(); - if (IosAudioEngine.Shared.IsConnected) IosAudioEngine.Shared.Reconfigure(); Changed?.Invoke(); + ResetWatchdogAttempts(); + if (IosAudioEngine.Shared.IsConnected) IosAudioEngine.Shared.Reconfigure(true, "settings"); Changed?.Invoke(); } private void Set(string key, string? value) { if (value is null) defaults.RemoveObject(key); else defaults.SetString(value, key); } internal void Deactivate() { // A released session has no route the next graph can be compared against. - watchdog?.Dispose(); watchdog = null; ResetWatchdog(); route.Reset(); + watchdog?.Dispose(); watchdog = null; ResetWatchdog(); ResetWatchdogAttempts(); AVAudioSession.SharedInstance().SetActive(false, AVAudioSessionSetActiveOptions.NotifyOthersOnDeactivation, out _); } internal void EnsureAudio(string reason) @@ -276,7 +280,7 @@ internal sealed class IosAudioRouter if (!IosAudioEngine.Shared.IsConnected || interrupted) return; UIApplication.SharedApplication.BeginInvokeOnMainThread(() => { - try { IosAudioEngine.Shared.Reconfigure(force); } + try { IosAudioEngine.Shared.Reconfigure(force, reason); } catch (Exception exception) { System.Diagnostics.Debug.WriteLine($"Audio recovery ({reason}) failed: {exception}"); } }); } @@ -291,6 +295,7 @@ internal sealed class IosAudioRouter } private void ResetWatchdog() { lastRenderCallbacks = -1; watchdogMisses = 0; } + private void ResetWatchdogAttempts() { watchdogAttempts = 0; nextWatchdogAttempt = 0; } // An interruption that ends while the app is suspended never delivers its Ended notification, // so foregrounding still has to check. It must not rebuild unconditionally though: the `audio` @@ -310,15 +315,29 @@ internal sealed class IosAudioRouter watchdogTicking = true; try { - if (!IosAudioEngine.Shared.IsConnected || interrupted || applying) { ResetWatchdog(); return; } + // A graph still starting up reports no callbacks yet and reads as not running. Judging + // it there is how the watchdog ends up rebuilding a healthy graph on every tick. + if (!IosAudioEngine.Shared.IsConnected || interrupted || applying || IosAudioEngine.Shared.Settling) { ResetWatchdog(); return; } long callbacks = IosAudioEngine.Shared.RenderCallbacks; bool stalled = !IosAudioEngine.Shared.IsRunning || callbacks == lastRenderCallbacks; lastRenderCallbacks = callbacks; - if (!stalled) { watchdogMisses = 0; return; } + if (!stalled) { watchdogMisses = 0; ResetWatchdogAttempts(); return; } // One missed tick can be a route change already rebuilding the graph. if (++watchdogMisses < 2) return; + if (Environment.TickCount64 < nextWatchdogAttempt) return; + if (watchdogAttempts >= MaximumWatchdogAttempts) + { + if (watchdogAttempts == MaximumWatchdogAttempts) + { + watchdogAttempts++; + Console.Error.WriteLine("VC_WATCHDOG exhausted; the audio graph will not start and is no longer being rebuilt"); + } + return; + } ResetWatchdog(); - try { IosAudioEngine.Shared.Reconfigure(); } + watchdogAttempts++; + nextWatchdogAttempt = Environment.TickCount64 + Math.Min(2_000 * (1 << watchdogAttempts), 30_000); + try { IosAudioEngine.Shared.Reconfigure(true, $"stall watchdog {watchdogAttempts}"); } catch (Exception exception) { System.Diagnostics.Debug.WriteLine($"Audio watchdog rebuild failed: {exception}"); } } finally { watchdogTicking = false; } @@ -334,13 +353,18 @@ internal sealed class IosAudioRouter 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})"); + // Not a forced rebuild. Every reconfiguration moves the route, and moving the route + // notifies here, so answering a route change with an unconditional rebuild is a loop with + // one iteration per notification: forcing the speaker takes a headset out of the route as + // OldDeviceUnavailable, releasing it brings the headset back as NewDeviceAvailable, and + // even joining voice moves the route through SetPreferredInput. AVAudioEngine follows a + // route change on its own; what it cannot absorb is the hardware under the tap changing, + // and Reconfigure tests for that, rebuilding a stopped graph and leaving a healthy + // unchanged one alone. Comparing routes instead cannot work: CurrentRoute still names the + // previous route for a while after a reconfiguration. + Console.Error.WriteLine($"VC_ROUTE_CHANGE reason={reason} running={IosAudioEngine.Shared.IsRunning} " + + $"hardwareChanged={IosAudioEngine.Shared.HardwareChanged()}"); + Recover($"route change ({reason})", force: false); } private void HandleInterruption(NSNotification note) { diff --git a/src/VoiceCat.Core/AudioRouteWatcher.cs b/src/VoiceCat.Core/AudioRouteWatcher.cs deleted file mode 100644 index ff56394..0000000 --- a/src/VoiceCat.Core/AudioRouteWatcher.cs +++ /dev/null @@ -1,36 +0,0 @@ -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 deleted file mode 100644 index 26be969..0000000 --- a/tests/VoiceCat.Tests/AudioRouteWatcherTests.cs +++ /dev/null @@ -1,41 +0,0 @@ -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 78ae283..d3bb8be 100644 --- a/tests/VoiceCat.Tests/PublishServerScriptTests.cs +++ b/tests/VoiceCat.Tests/PublishServerScriptTests.cs @@ -180,7 +180,11 @@ public class PublishServerScriptTests Assert.Contains("private void TickWatchdog()", router); Assert.Contains("IosAudioEngine.Shared.RenderCallbacks", router); Assert.Contains("IosAudioEngine.Shared.IsRunning", router); - Assert.Contains("IosAudioEngine.Shared.Reconfigure()", router); + // Bounded: a rebuild that does not restore the callbacks backs off and then stops, so a + // graph that cannot start fails visibly instead of churning the session forever. + Assert.Contains("IosAudioEngine.Shared.Reconfigure(true, $\"stall watchdog {watchdogAttempts}\")", router); + Assert.Contains("watchdogAttempts >= MaximumWatchdogAttempts", router); + Assert.Contains("VC_WATCHDOG exhausted", router); // SetActive fails for as long as an interruption is in force, so Began suppresses // recovery and the watchdog retries the rebuild that Ended asks for. @@ -218,7 +222,15 @@ public class PublishServerScriptTests Assert.True(engine.IndexOf("SetVoiceProcessingEnabled", StringComparison.Ordinal) < engine.IndexOf("next.Connect(source", StringComparison.Ordinal)); Assert.Contains("AVAudioEngine.ConfigurationChangeNotification", engine); - Assert.Contains("ReferenceEquals(engine, next) && !next.Running", engine); + Assert.Contains("if (!IsConnected || !ReferenceEquals(engine, next)) return;", engine); + Assert.Contains("if (!next.Running || HardwareChanged()) Rebuild(\"configuration change\");", engine); + // With voice processing the input and output are one IO unit, and it only stays up while the + // input is rendered. A tap alone leaves it out of the chain and the unit dies seconds later. + Assert.Contains("next.Connect(input, captureSink, inputFormat", engine); + Assert.Contains("captureSink.OutputVolume = 0f;", engine); + // Capture exists for the session, so joining voice names a stream instead of rebuilding. + Assert.Contains("internal void StopMicrophone() { Volatile.Write(ref microphone, null); }", engine); + Assert.Contains("bool captures = IsConnected;", engine); } [Fact] @@ -239,11 +251,35 @@ public class PublishServerScriptTests // 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. + // Every reconfiguration moves the route and moving the route notifies the handler, so a + // route change must never force a rebuild: that is one rebuild per notification, without + // end. The graph is rebuilt only when it stopped or when the hardware it was built around + // moved, which is the one thing AVAudioEngine cannot absorb by itself. Assert.Contains("if (applying) return;", router); - Assert.Contains("!route.ShouldRebuild(RouteSignature(AVAudioSession.SharedInstance()))", router); - Assert.Contains("route.Built(RouteSignature(session))", router); + Assert.Contains("Recover($\"route change ({reason})\", force: false)", router); + Assert.DoesNotContain("Recover($\"route change ({reason})\");", router); + + string engine = await File.ReadAllTextAsync(Path.Combine( + FindRoot(), "clients", "apple", "VoiceCat.iOS", "IosAudioEngine.cs")); + // Asked of the graph's own input, not of AVAudioSession and not remembered from a route: + // the session's reported rate and channel count do not settle until after the graph has + // started, and CurrentRoute still names the previous route for a while, so either one + // reports a change that has not happened and every rebuild reports it again. + Assert.Contains("live.InputNode.GetBusOutputFormat(0)", engine); + Assert.Contains("format.SampleRate != builtInputRate || format.ChannelCount != builtInputChannels", engine); + Assert.DoesNotContain("AVAudioSession.SharedInstance().SampleRate", engine); + // Every rebuild names what asked for it, so a storm can be read from a log. + Assert.Contains("VC_REBUILD cause=", engine); + // Building a graph is itself what posts most configuration changes, and the engine reads as + // stopped until voice processing has rebuilt the IO. Nothing may judge a graph dead inside + // that window, or it replaces the graph it just built with one that reports the same thing. + Assert.Contains("internal bool Settling =>", engine); + Assert.Contains("if (!force && Settling) return;", engine); + Assert.Contains("if (Settling) return;", engine); + Assert.Contains("if (Settling) return true;", engine); + Assert.Contains("|| IosAudioEngine.Shared.Settling) { ResetWatchdog(); return; }", router); + Assert.Contains("&& !HardwareChanged()) return;", engine); + Assert.Contains("(!next.Running || HardwareChanged())", engine); Assert.Contains("if (Preset == IosAudioPreset.VoiceChat) { SelectedInputId = null; SelectedDataSourceId = null; }", router); Assert.DoesNotContain("SelectedInputId ??= session.PreferredInput", router); }