From 2fb9274a9577371a18a6248a5500a62388610650 Mon Sep 17 00:00:00 2001 From: Ednunp <29843396+Ednunp@users.noreply.github.com> Date: Thu, 9 Jul 2026 20:54:53 +0100 Subject: [PATCH] =?UTF-8?q?v5.2:=20stability=20and=20polish=20=E2=80=94=20?= =?UTF-8?q?deep-audit=20bug=20fixes=20+=20install-flow=20fixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Verified findings from a multi-dimension code audit, plus the two install-flow bugs: - Fix Opus encoder use-after-free on a codec/rate change while streaming (guard swap vs encode). - Fix "both" single-file recording dropping audio + drifting (drain both directions in lockstep). - Fix broken clip counter, UPnP teardown on exit, auto-update-restart foreground grant, and a malformed-Opus-format packet orphaning a playout session forever. - Post-install relaunch now respects start-minimised; uninstall is path-aware so it won't clear a different copy's run-at-startup. - Perf/hygiene: cache AppConfig off UI hot paths, fold per-peer EQ+gain into one pass, deterministic disposal (tray menu, timers, COM shortcut, Process handles, process meter), ring-buffer overflow guard, receiver session-lock fix, remote-control allow-list moved onto the UI thread. - Remove dead code (two IsAsioBackend, SessionPlayout.Reset, IsSameEndpoint, RemSoundUpdater IDisposable); several stale-doc fixes. Deferred (not in this release): drift-estimator tweak, peer-discovery pruning, uninstall retry-loop, encryption nonce. Wire format unchanged (interops v3.3-v5.1). Version -> 5.2. Co-Authored-By: Claude Opus 4.8 --- RELEASE_NOTES.md | 36 +++--- src/RemSound.App/AboutDialog.cs | 8 ++ src/RemSound.App/AppInstaller.cs | 31 ++++- src/RemSound.App/AudioRecorder.cs | 83 ++++++++++--- src/RemSound.App/CheckSoundService.cs | 12 +- src/RemSound.App/CueSounds.cs | 8 +- src/RemSound.App/MainForm.cs | 115 +++++++++++------- src/RemSound.App/MainFormTrayController.cs | 12 +- src/RemSound.App/ProcessSelfMeter.cs | 6 +- src/RemSound.App/RecordingController.cs | 11 ++ src/RemSound.App/RecordingSettingsDialog.cs | 4 + src/RemSound.App/RemSound.App.csproj | 2 +- src/RemSound.App/RemSoundUpdater.cs | 9 +- src/RemSound.App/RouterPortMapper.cs | 7 +- src/RemSound.App/StartupAutoStart.cs | 29 +++++ src/RemSound.App/TabSwitchSoundService.cs | 9 +- src/RemSound.App/UpdateApplier.cs | 14 ++- src/RemSound.Core/AudioRingBuffer.cs | 11 ++ src/RemSound.Core/RemSoundSettingsStore.cs | 4 +- src/RemSound.Receiver/AudioReceiver.cs | 28 ++++- src/RemSound.Receiver/NetworkListener.cs | 9 +- src/RemSound.Receiver/PeerDspChain.cs | 39 ++++-- src/RemSound.Receiver/PlayoutEngine.cs | 18 +-- src/RemSound.Receiver/SessionPlayout.cs | 30 ----- src/RemSound.Receiver/StreamSession.cs | 2 - src/RemSound.Sender/AudioSender.cs | 2 - .../CompositeCaptureBackend.cs | 9 +- src/RemSound.Sender/SenderLane.cs | 58 ++++++--- 28 files changed, 408 insertions(+), 198 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index a22f468..9eebaad 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -1,38 +1,36 @@ -# RemSound v5.1 +# RemSound v5.2 -Install RemSound as a proper Windows app — or keep running it portable, whichever you prefer. +A stability and polish release. No new features — this one is bug fixes and internal tidy-ups from a deep code review, so it should just feel a bit more solid. -## Install RemSound on this PC +## Fixes you might notice -A new **Options → Install RemSound on this PC** turns the copy you're running into a properly installed Windows app: +- **Changing the codec or send rate while streaming** could, in rare cases, crash RemSound with no warning. Fixed. +- **Recording "both sent and received" into a single file** could lose small amounts of audio and drift out of sync over a long session. It now stays sample-accurate for the whole recording. +- **After installing**, RemSound now honours your "start minimised" setting on the first relaunch (and still comes to the front when you're not starting minimised). +- **Uninstalling** one copy no longer switches off a different copy's "run at startup". +- Starting a split "received only" recording with **no peers connected** now tells you clearly, instead of quietly making an empty folder. +- The **clipping indicator** in the diagnostics works again. -- It sets RemSound up on the **Start menu** (with the program, the manual and an uninstall shortcut), adds a **desktop shortcut**, and lists it in Windows' **Installed apps**. -- It installs into your own user area, so it **never asks for administrator rights** and only affects your account. -- A tick-box dialog lets you choose what to set up — desktop and Start-menu shortcuts, run at startup — and bring your **profiles and settings, recordings and logs** across from the copy you're running. -- When it's done it reopens from the installed location; everything carries on exactly as before, and RemSound still updates itself in place as usual. +## Under the hood -Once it's installed, that same menu item becomes **Uninstall RemSound from this PC** (with matching entries in the Start menu and Windows' Installed apps). Uninstalling asks first, with two tick-boxes — **remove profiles, config and logs**, and **remove recordings** — both off by default, so your own files are kept unless you say otherwise. - -You never *have* to install: running RemSound straight from the unzipped folder works exactly as it always has. - -## Smaller download - -The built-in cue sounds have been slimmed down, so RemSound is a smaller download and takes up less space. +- The automatic-update restart now brings the window to the front, the same way the installer does. +- The router port opened by UPnP is properly removed when you close RemSound. +- Lighter on memory and disk — fewer background file reads while you work, and tidier release of resources when the window closes or switches profiles. ## Compatibility -The over-the-network format is unchanged, so **v5.1 talks to v3.3 through v5.0** with no trouble — you don't have to update both ends at once. (Everyone still needs **v3.3 or newer**, where end-to-end encryption came in.) +Nothing about the over-the-network format changed, so **v5.2 talks to v3.3 through v5.1** with no trouble — you don't have to update both ends at once. (Everyone still needs **v3.3 or newer**, where end-to-end encryption came in.) ## Install -1. Download `RemSound-v5.1.zip` from this release. +1. Download `RemSound-v5.2.zip` from this release. 2. Close RemSound. 3. Extract the zip **over your existing RemSound folder**, overwriting program files when prompted. The zip is program files only — it won't touch your settings, profiles, logs or recordings. -4. Run `RemSound.exe`. (If you'd like it set up as a proper Windows app, use **Options → Install RemSound on this PC**.) +4. Run `RemSound.exe`. ## Upgrading -**From v3.6 or newer:** Help → Check for updates installs v5.1 with the in-app updater — and if it can't finish, it puts your old version back exactly as it was. +**From v3.6 or newer:** Help → Check for updates installs v5.2 with the in-app updater — and if it can't finish, it puts your old version back exactly as it was. **From v1.9–v3.5:** Check for updates works, but uses your current version's older updater for this one hop. If auto-update has been failing on your machine, install by hand using the steps above. diff --git a/src/RemSound.App/AboutDialog.cs b/src/RemSound.App/AboutDialog.cs index 2b8c977..a8e4b6b 100644 --- a/src/RemSound.App/AboutDialog.cs +++ b/src/RemSound.App/AboutDialog.cs @@ -20,6 +20,14 @@ internal sealed class AboutDialog : Form /// updates" path. private const string ReleaseNotes = """ + RemSound v5.2 + + A stability and polish release — bug fixes and tidy-ups from a deep code review. No new features; it should just feel a bit more solid. + + Fixed: changing the codec or send rate while streaming could, in rare cases, crash RemSound. Fixed: recording both your sent and received audio into a single file could lose a little audio and drift out of sync over a long recording — it now stays accurate the whole way through. Fixed: after installing, RemSound now stays minimised if that's your setting (and still comes to the front otherwise), and uninstalling one copy no longer switches off another copy's run-at-startup. + + Also: the clipping indicator works again, the router port opened for incoming connections is tidied up when you close, and the app is a little lighter on memory and disk. Nothing about how you connect changed, so it still talks to older versions. + RemSound v5.1 Install RemSound as a proper Windows app. diff --git a/src/RemSound.App/AppInstaller.cs b/src/RemSound.App/AppInstaller.cs index e30dc50..6cc831d 100644 --- a/src/RemSound.App/AppInstaller.cs +++ b/src/RemSound.App/AppInstaller.cs @@ -103,6 +103,11 @@ internal static class AppInstaller return; } + // Degenerate case: the portable copy was extracted straight into the install location (no marker + // yet, so IsInstalledCopy is false). Copying the folder onto itself would throw an IOException + // (self-copy) and abort with a scary message. Detect it and skip the copy — we're already in + // place, so registration (marker + shortcuts) is all that's needed. + var sameLocation = string.Equals(source, target, StringComparison.OrdinalIgnoreCase); var updating = InstallExistsAtTarget; var options = ShowInstallOptionsDialog(owner, target, updating); if (options is null) return; // cancelled @@ -116,10 +121,14 @@ internal static class AppInstaller $"recordings={options.CopyRecordings}, logs={options.CopyLogs})"); // The file copy is the part that must succeed — a failure here (disk full, permissions) - // aborts with the copy untouched. Directory.CreateDirectory + copy. + // aborts with the copy untouched. Skipped entirely when we're already running from the + // target (self-copy would throw); registration below still runs. Directory.CreateDirectory(target); - CopyProgramFiles(source, target); - CopyUserData(source, target, options); + if (!sameLocation) + { + CopyProgramFiles(source, target); + CopyUserData(source, target, options); + } // Drop the marker that tells the installed copy it IS installed (so the Options menu shows // Uninstall). Part of the must-succeed path: without it the install wouldn't recognise @@ -258,7 +267,9 @@ internal static class AppInstaller { try { SetDesktopShortcut(false, "", ""); } catch { } try { SetStartMenuFolder(false, "", ""); } catch { } - try { StartupAutoStart.TryDisable(); } catch { } + // Only clear login-autostart if it points at THIS installed copy — never wipe a different + // copy's (e.g. a portable copy's) autostart entry that shares the "RemSound" value name. + try { StartupAutoStart.TryDisableIfPointsInto(AppContext.BaseDirectory); } catch { } try { UnregisterInstalledApp(); } catch { } } @@ -383,10 +394,12 @@ internal static class AppInstaller var shellType = Type.GetTypeFromProgID("WScript.Shell"); if (shellType is null) return; dynamic? shell = null; + object? linkObj = null; try { shell = Activator.CreateInstance(shellType); dynamic link = shell!.CreateShortcut(linkPath); + linkObj = link; // keep a handle so we can release this SECOND COM object too, not just shell link.TargetPath = targetPath; link.Arguments = arguments; link.WorkingDirectory = workingDir; @@ -396,6 +409,12 @@ internal static class AppInstaller } finally { + // Release both COM objects deterministically (the IWshShortcut from CreateShortcut AND the + // WScript.Shell), rather than leaving the shortcut object to the GC finalizer. + if (linkObj is not null) + { + try { System.Runtime.InteropServices.Marshal.FinalReleaseComObject(linkObj); } catch { } + } if (shell is not null) { try { System.Runtime.InteropServices.Marshal.FinalReleaseComObject(shell); } catch { } @@ -459,7 +478,7 @@ internal static class AppInstaller psi.ArgumentList.Add("--await-pid"); psi.ArgumentList.Add(Environment.ProcessId.ToString()); - var child = Process.Start(psi); + using var child = Process.Start(psi); // Grant the just-launched child the right to take the foreground. Given now, while we still // hold it, it survives our imminent exit and lets the child's SetForegroundWindow succeed. if (child is not null) @@ -524,7 +543,7 @@ internal static class AppInstaller { var path = Path.Combine(Path.GetTempPath(), namePrefix + Guid.NewGuid().ToString("N") + ".cmd"); File.WriteAllText(path, script, new UTF8Encoding(false)); - Process.Start(new ProcessStartInfo + using var proc = Process.Start(new ProcessStartInfo { FileName = "cmd.exe", Arguments = $"/c \"{path}\"", diff --git a/src/RemSound.App/AudioRecorder.cs b/src/RemSound.App/AudioRecorder.cs index 63b851d..83c907f 100644 --- a/src/RemSound.App/AudioRecorder.cs +++ b/src/RemSound.App/AudioRecorder.cs @@ -372,6 +372,24 @@ internal sealed class AudioRecorder : IDisposable return 0; } + /// How many frames would produce for one direction right + /// now, WITHOUT consuming anything — the lane-merge min (or the single active lane), capped at + /// . Used by the Both path to drain the sent and received directions by + /// the SAME amount so neither ring is over-consumed and the two stay sample-aligned. Read heads are + /// owned by this (writer) thread so a plain read is fine; the caller passes Volatile.Read snapshots + /// of the write heads, which the audio threads advance. + private static int DirectionAvailFrames(long wasapiWrite, long wasapiRead, long asioWrite, long asioRead, int maxFrames) + { + var w = (int)((wasapiWrite - wasapiRead) / MixChannels); + var a = (int)((asioWrite - asioRead) / MixChannels); + int avail; + if (w > 0 && a > 0) avail = Math.Min(w, a); + else if (w > 0) avail = w; + else if (a > 0) avail = a; + else avail = 0; + return Math.Min(avail, maxFrames); + } + private void Process() { int framesThisCall; @@ -410,18 +428,37 @@ internal sealed class AudioRecorder : IDisposable // received scratch we'll grow as needed. EnsureScratchSize(DrainChunkMaxFrames * MixChannels); EnsureSecondaryScratchSize(DrainChunkMaxFrames * MixChannels); - var sentFrames = DrainOneDirection( - sentWasapiRing, ref sentWasapiWriteHead, ref sentWasapiReadHead, - sentAsioRing, ref sentAsioWriteHead, ref sentAsioReadHead, - mixScratch, mixScratchAux, DrainChunkMaxFrames); EnsureRecvDirectionScratchSize(DrainChunkMaxFrames * MixChannels); - var recvFrames = DrainOneDirection( - receivedWasapiRing, ref receivedWasapiWriteHead, ref receivedWasapiReadHead, - receivedAsioRing, ref receivedAsioWriteHead, ref receivedAsioReadHead, - recvDirectionScratch, mixScratchAux, DrainChunkMaxFrames); - if (sentFrames > 0 && recvFrames > 0) + + // Peek how much each direction can supply WITHOUT consuming, so when both carry audio we + // drain them by the SAME amount and keep them sample-aligned. Draining each independently + // (the old code) advanced the faster direction's ring head past frames we then never + // wrote — a silent, continuous loss + progressive drift on any real two-way session, + // which is exactly what Both mode exists to capture. Now the surplus genuinely stays in + // its ring for the next pass. + var sentAvail = DirectionAvailFrames( + Volatile.Read(ref sentWasapiWriteHead), sentWasapiReadHead, + Volatile.Read(ref sentAsioWriteHead), sentAsioReadHead, DrainChunkMaxFrames); + var recvAvail = DirectionAvailFrames( + Volatile.Read(ref receivedWasapiWriteHead), receivedWasapiReadHead, + Volatile.Read(ref receivedAsioWriteHead), receivedAsioReadHead, DrainChunkMaxFrames); + + if (sentAvail > 0 && recvAvail > 0) { - framesThisCall = Math.Min(sentFrames, recvFrames); + // Both directions have data — take the same count from each. Because write heads only + // advance (producers add) and this is the sole consumer, each drain returns exactly + // `take`, so the two stay aligned and nothing is over-consumed. + var take = Math.Min(sentAvail, recvAvail); + var got1 = DrainOneDirection( + sentWasapiRing, ref sentWasapiWriteHead, ref sentWasapiReadHead, + sentAsioRing, ref sentAsioWriteHead, ref sentAsioReadHead, + mixScratch, mixScratchAux, take); + var got2 = DrainOneDirection( + receivedWasapiRing, ref receivedWasapiWriteHead, ref receivedWasapiReadHead, + receivedAsioRing, ref receivedAsioWriteHead, ref receivedAsioReadHead, + recvDirectionScratch, mixScratchAux, take); + framesThisCall = Math.Min(got1, got2); // defensive; both equal `take` in practice + if (framesThisCall <= 0) return; var dst = mixScratch.AsSpan(0, framesThisCall * MixChannels); var aux = recvDirectionScratch.AsSpan(0, framesThisCall * MixChannels); for (var i = 0; i < dst.Length; i++) @@ -431,20 +468,26 @@ internal sealed class AudioRecorder : IDisposable else if (s < -1f) s = -1f + MathF.Tanh(-1f - s); dst[i] = s; } - // Any leftover frames in the direction that produced MORE this tick stay - // in their rings for the next iteration — they're not lost, just deferred. - // We can't write them now without un-syncing the two directions. } - else if (sentFrames > 0) + else if (sentAvail > 0) { - framesThisCall = sentFrames; - // mixScratch already contains the sent direction's audio — emit as-is. + // Only the sent direction has audio right now — record it solo (no over-consume, + // nothing to align against). mixScratch already holds it. + framesThisCall = DrainOneDirection( + sentWasapiRing, ref sentWasapiWriteHead, ref sentWasapiReadHead, + sentAsioRing, ref sentAsioWriteHead, ref sentAsioReadHead, + mixScratch, mixScratchAux, DrainChunkMaxFrames); + if (framesThisCall <= 0) return; } - else if (recvFrames > 0) + else if (recvAvail > 0) { - framesThisCall = recvFrames; - // The recv-direction audio lives in recvDirectionScratch; copy into - // mixScratch so EmitMixBuffer (which reads from mixScratch) sees it. + framesThisCall = DrainOneDirection( + receivedWasapiRing, ref receivedWasapiWriteHead, ref receivedWasapiReadHead, + receivedAsioRing, ref receivedAsioWriteHead, ref receivedAsioReadHead, + recvDirectionScratch, mixScratchAux, DrainChunkMaxFrames); + if (framesThisCall <= 0) return; + // The recv-direction audio lives in recvDirectionScratch; copy into mixScratch so + // EmitMixBuffer (which reads from mixScratch) sees it. var len = framesThisCall * MixChannels; recvDirectionScratch.AsSpan(0, len).CopyTo(mixScratch.AsSpan(0, len)); } diff --git a/src/RemSound.App/CheckSoundService.cs b/src/RemSound.App/CheckSoundService.cs index 68e03f9..9cbfb81 100644 --- a/src/RemSound.App/CheckSoundService.cs +++ b/src/RemSound.App/CheckSoundService.cs @@ -16,6 +16,11 @@ internal static class CheckSoundService { private static CuePlayer? checkSound; private static CuePlayer? uncheckSound; + // Cached at Reload() so Play() (which fires on the UI thread for every genuine checkbox toggle) + // doesn't re-read + re-deserialize the whole config file from disk just to fetch one bool. Reload() + // is already the hook that runs whenever cue settings change, so these stay current. + private static bool enableCheckboxOn; + private static bool enableCheckboxOff; /// When true, is a no-op. MainForm sets this around bulk programmatic /// control updates (profile load, "uncheck all", device-list refresh). The per-call Focused gate @@ -29,6 +34,8 @@ internal static class CheckSoundService public static void Reload() { var cfg = AppConfig.Load(); + enableCheckboxOn = cfg.EnableCheckboxOnCue; + enableCheckboxOff = cfg.EnableCheckboxOffCue; checkSound = LoadCue(MainForm.CueId.CheckboxOn, "check.wav", cfg); uncheckSound = LoadCue(MainForm.CueId.CheckboxOff, "uncheck.wav", cfg); } @@ -36,9 +43,8 @@ internal static class CheckSoundService public static void Play(bool isChecked) { if (Suppressed) return; - var cfg = AppConfig.Load(); - if (isChecked) { if (cfg.EnableCheckboxOnCue) checkSound?.Play(); } - else { if (cfg.EnableCheckboxOffCue) uncheckSound?.Play(); } + if (isChecked) { if (enableCheckboxOn) checkSound?.Play(); } + else { if (enableCheckboxOff) uncheckSound?.Play(); } } private static CuePlayer? LoadCue(string cueId, string defaultFile, AppConfig cfg) diff --git a/src/RemSound.App/CueSounds.cs b/src/RemSound.App/CueSounds.cs index 288b56c..8dfaf44 100644 --- a/src/RemSound.App/CueSounds.cs +++ b/src/RemSound.App/CueSounds.cs @@ -51,7 +51,13 @@ internal static class CueSounds } } catch { return Array.Empty(); } - return matches.OrderBy(m => m.Order).Select(m => m.Name).ToList(); + var ordered = matches.OrderBy(m => m.Order).ToList(); + // If numbered variants exist, drop the bare unnumbered file: it labels as "Sound 1" (below), the + // same as " 1.wav", so the two would show as indistinguishable "Sound 1" rows a screen-reader + // user can't tell apart. The bare name is a legacy single-file fallback; numbered is what ships. + if (ordered.Exists(m => m.Order >= 1) && ordered.Exists(m => m.Order == 0)) + ordered.RemoveAll(m => m.Order == 0); + return ordered.Select(m => m.Name).ToList(); } /// The "Sound N" label for a variant filename, for the Preferences listbox. diff --git a/src/RemSound.App/MainForm.cs b/src/RemSound.App/MainForm.cs index 26bc5fb..95796e5 100644 --- a/src/RemSound.App/MainForm.cs +++ b/src/RemSound.App/MainForm.cs @@ -1331,11 +1331,16 @@ public sealed class MainForm : Form FormClosing += (_, _) => { - statusTimer.Stop(); - deviceRefreshTimer.Stop(); - continuousTuneTimer.Stop(); - updateCheckTimer.Stop(); - asioDriverChangeDebounce.Stop(); + // Stop AND dispose each timer. A WinForms Timer is a Component, not a Control, so base + // Form.Dispose never reaches it; Stop() only kills the WM_TIMER, leaving the timer's + // message-only window handle to be freed at GC finalization. MainForm is rebuilt on every + // profile switch, so disposing here releases those handles deterministically each time. + statusTimer.Stop(); statusTimer.Dispose(); + deviceRefreshTimer.Stop(); deviceRefreshTimer.Dispose(); + continuousTuneTimer.Stop(); continuousTuneTimer.Dispose(); + updateCheckTimer.Stop(); updateCheckTimer.Dispose(); + asioDriverChangeDebounce.Stop(); asioDriverChangeDebounce.Dispose(); + try { processSelfMeter.Dispose(); } catch { } try { deviceChangeNotifier?.Dispose(); } catch { } try { powerResumeHandler?.Dispose(); } catch { } try { routerPortMapper?.Dispose(); } catch { } @@ -1409,20 +1414,22 @@ public sealed class MainForm : Form // wired up before we hide the window. var coldStart = isFirstLaunch; isFirstLaunch = false; - // A post-install relaunch (--foreground) overrides any start-minimised preference — the - // user just ran an interactive install and expects to see the installed copy come up. - var minimizeThisInstance = !forceForegroundOnStart - && (startNextInstanceMinimized || (coldStart && AppConfig.Load().StartMinimised)); + var minimizeThisInstance = startNextInstanceMinimized || (coldStart && AppConfig.Load().StartMinimised); startNextInstanceMinimized = false; + // Consume the one-shot post-install foreground flag now, whichever branch we take below, so + // it can't leak into a later profile-switch relaunch. "Start minimised" WINS over it: a user + // who chose to boot into the tray wants the just-installed copy in the tray too — we only + // pull the window to the front when we're NOT minimising (else it can open behind others). + var forcePostInstallForeground = forceForegroundOnStart; + forceForegroundOnStart = false; if (minimizeThisInstance) { // playCue:false — starting up in the tray (StartMinimised / --minimized) isn't the // user choosing to minimise, so it must not sound the "minimise" cue. BeginInvoke(() => trayController.Minimize(playCue: false)); } - else if (forceForegroundOnStart) + else if (forcePostInstallForeground) { - forceForegroundOnStart = false; logFile.Event("installer: post-install relaunch — bringing the window to the foreground"); // Deferred so it runs after Shown settles, then yanks the window to the front so the // just-installed copy isn't left hiding behind other windows. Try again a moment later: @@ -3470,7 +3477,11 @@ public sealed class MainForm : Form loadingPanEqControls = true; try { - var s = GetOrCreateShaping(selectedShapingKey); + // Read-only for display: use the existing shaping if any, else a throwaway default. Do NOT + // GetOrCreateShaping here — merely selecting/scrolling a peer would then insert a no-op entry + // into the saved profile for every peer the user only glanced at. The actual edit handlers + // (pan/volume/mode/band) call GetOrCreateShaping, so an entry is created only on a real change. + var s = GetShaping(selectedShapingKey) ?? new PeerShaping(); volumeSlider.Value = Math.Clamp((int)Math.Round(s.Volume * 100f), 0, 100); UpdateVolumeAccessibleName(); panSlider.Value = Math.Clamp((int)Math.Round(s.Pan * 50f) + 50, 0, 100); @@ -8246,15 +8257,18 @@ public sealed class MainForm : Form /// Custom paths are per-profile (changed from machine-wide in v3.0.3 development) so /// each profile can carry its own cue palette. The settings cache mirrors the active /// profile's CustomCuePaths dictionary and is the runtime source of truth. - private void TryLoadCueSound(string cueId, string defaultFileName, out CuePlayer? player) + private void TryLoadCueSound(string cueId, string defaultFileName, out CuePlayer? player, AppConfig? cfg = null) { player = null; try { + // Load the config once per call (or reuse the caller's — ReloadAllCueSounds passes one shared + // instance for all 14 cues instead of each cue re-reading + re-parsing the file from disk). + cfg ??= AppConfig.Load(); string? path = null; var customPath = settings.LoadCustomCuePath(cueId); if (string.IsNullOrWhiteSpace(customPath) - && AppConfig.Load().MachineCueCustomPaths.TryGetValue(cueId, out var machinePath)) + && cfg.MachineCueCustomPaths.TryGetValue(cueId, out var machinePath)) { // Machine-wide cues (send/receive/hide/show) keep their custom override in AppConfig. customPath = machinePath; @@ -8268,7 +8282,7 @@ public sealed class MainForm : Form { // The cue ships as numbered variants ("connect 1.wav", "connect 2.wav", ...); // resolve the machine-wide chosen default (or the first variant) for this cue. - var defaultPath = CueSounds.ResolveDefaultPath(cueId, defaultFileName, AppConfig.Load()); + var defaultPath = CueSounds.ResolveDefaultPath(cueId, defaultFileName, cfg); if (defaultPath is not null && File.Exists(defaultPath)) { path = defaultPath; @@ -8295,20 +8309,23 @@ public sealed class MainForm : Form /// public void ReloadAllCueSounds() { - TryLoadCueSound(CueId.Connect, "connect.wav", out connectSound); - TryLoadCueSound(CueId.Disconnect, "disconnect.wav", out disconnectSound); - TryLoadCueSound(CueId.RecordStart, "record start.wav", out recordStartSound); - TryLoadCueSound(CueId.RecordStop, "record stop.wav", out recordStopSound); - TryLoadCueSound(CueId.Save, "save.wav", out saveSound); - TryLoadCueSound(CueId.ProfileSwitch, "profile.wav", out profileSwitchSound); - TryLoadCueSound(CueId.ProfileMenuOpen, "profile menu open.wav", out profileMenuOpenSound); - TryLoadCueSound(CueId.Update, "update.wav", out updateSound); - TryLoadCueSound(CueId.SendOn, "send on.wav", out sendOnSound); - TryLoadCueSound(CueId.SendOff, "send off.wav", out sendOffSound); - TryLoadCueSound(CueId.ReceiveOn, "recieve on.wav", out receiveOnSound); - TryLoadCueSound(CueId.ReceiveOff, "recieve off.wav", out receiveOffSound); - TryLoadCueSound(CueId.Hide, "minimise.wav", out hideSound); - TryLoadCueSound(CueId.Show, "maximise.wav", out showSound); + // Load the machine config ONCE and pass it to all 14 cues, instead of each cue (twice) re-reading + // and re-deserializing the config file — this runs on the UI thread on every Preferences close. + var cfg = AppConfig.Load(); + TryLoadCueSound(CueId.Connect, "connect.wav", out connectSound, cfg); + TryLoadCueSound(CueId.Disconnect, "disconnect.wav", out disconnectSound, cfg); + TryLoadCueSound(CueId.RecordStart, "record start.wav", out recordStartSound, cfg); + TryLoadCueSound(CueId.RecordStop, "record stop.wav", out recordStopSound, cfg); + TryLoadCueSound(CueId.Save, "save.wav", out saveSound, cfg); + TryLoadCueSound(CueId.ProfileSwitch, "profile.wav", out profileSwitchSound, cfg); + TryLoadCueSound(CueId.ProfileMenuOpen, "profile menu open.wav", out profileMenuOpenSound, cfg); + TryLoadCueSound(CueId.Update, "update.wav", out updateSound, cfg); + TryLoadCueSound(CueId.SendOn, "send on.wav", out sendOnSound, cfg); + TryLoadCueSound(CueId.SendOff, "send off.wav", out sendOffSound, cfg); + TryLoadCueSound(CueId.ReceiveOn, "recieve on.wav", out receiveOnSound, cfg); + TryLoadCueSound(CueId.ReceiveOff, "recieve off.wav", out receiveOffSound, cfg); + TryLoadCueSound(CueId.Hide, "minimise.wav", out hideSound, cfg); + TryLoadCueSound(CueId.Show, "maximise.wav", out showSound, cfg); // The app-wide checkbox tick/untick and tab-switch sounds live in their own services; keep // them in step. CheckSoundService.Reload(); @@ -8553,26 +8570,30 @@ public sealed class MainForm : Form /// private void HandleRemoteControlPacket(RemoteControlKind kind, sbyte delta, IPEndPoint remote) { - // Allow-list match by IP only — the sender's source port is their ephemeral outbound, - // not their announced audio port. - var allowed = false; - foreach (var ep in selectedPeerEndpoints.Values) - { - if (ep.Address.Equals(remote.Address)) { allowed = true; break; } - } - if (!allowed) - { - logFile.Event($"remote-control IGNORED (not in allow-list) kind={kind} delta={delta} from={remote}"); - return; - } - if (!settings.LoadAcceptRemoteVolumeCommands()) - { - logFile.Event($"remote-control IGNORED (Accept remote volume commands is off) kind={kind} delta={delta} from={remote}"); - return; - } - + // Marshal to the UI thread FIRST. The allow-list scan reads selectedPeerEndpoints — a plain + // Dictionary owned and mutated by the UI thread — so enumerating it here on the receiver's + // network thread races a concurrent peer tick/untick (a caught "collection was modified" that + // silently drops the remote command). Running the whole check on the UI thread removes the race. BeginInvoke(() => { + // Allow-list match by IP only — the sender's source port is their ephemeral outbound, + // not their announced audio port. + var allowed = false; + foreach (var ep in selectedPeerEndpoints.Values) + { + if (ep.Address.Equals(remote.Address)) { allowed = true; break; } + } + if (!allowed) + { + logFile.Event($"remote-control IGNORED (not in allow-list) kind={kind} delta={delta} from={remote}"); + return; + } + if (!settings.LoadAcceptRemoteVolumeCommands()) + { + logFile.Event($"remote-control IGNORED (Accept remote volume commands is off) kind={kind} delta={delta} from={remote}"); + return; + } + switch (kind) { case RemoteControlKind.VolumeUp: diff --git a/src/RemSound.App/MainFormTrayController.cs b/src/RemSound.App/MainFormTrayController.cs index 8cd6e32..93ab995 100644 --- a/src/RemSound.App/MainFormTrayController.cs +++ b/src/RemSound.App/MainFormTrayController.cs @@ -50,6 +50,10 @@ internal sealed class MainFormTrayController : IDisposable private readonly ToolStripMenuItem sendingItem; private readonly ToolStripMenuItem receivingItem; private readonly ToolStripMenuItem profilesItem; + // Retained so Dispose can free it — NotifyIcon.Dispose does NOT dispose an externally-assigned + // ContextMenuStrip, and this menu (a native-window-handle-owning Control once shown) would otherwise + // leak per profile-switch rebuild. Disposing it cascades to its ToolStripItems. + private readonly ContextMenuStrip menu; public MainFormTrayController( Form owner, @@ -87,7 +91,7 @@ internal sealed class MainFormTrayController : IDisposable trayIcon.Visible = false; trayIcon.DoubleClick += (_, _) => Restore(); - var menu = new ContextMenuStrip(); + menu = new ContextMenuStrip(); var showItem = new ToolStripMenuItem("Sho&w RemSound") { @@ -248,7 +252,11 @@ internal sealed class MainFormTrayController : IDisposable trayIcon.Visible = true; } - public void Dispose() => trayIcon.Dispose(); + public void Dispose() + { + trayIcon.Dispose(); + menu.Dispose(); // NotifyIcon.Dispose doesn't free the assigned ContextMenuStrip; do it ourselves. + } private void RefreshMenuState() { diff --git a/src/RemSound.App/ProcessSelfMeter.cs b/src/RemSound.App/ProcessSelfMeter.cs index 5eaf569..32897a1 100644 --- a/src/RemSound.App/ProcessSelfMeter.cs +++ b/src/RemSound.App/ProcessSelfMeter.cs @@ -18,7 +18,7 @@ namespace RemSound.App; /// Threading: is called from the App's status-tick handler on the UI /// thread. Snapshot fields are mutated by that same single thread; no locks needed. /// -internal sealed class ProcessSelfMeter +internal sealed class ProcessSelfMeter : IDisposable { private TimeSpan prevTotalCpu; private long prevAllocBytes; @@ -140,4 +140,8 @@ internal sealed class ProcessSelfMeter HandleCount: handleCount, ThreadCount: threadCount); } + + /// Release the cached Process handle deterministically rather than at GC finalization — + /// small, but consistent with the app's resource-lifecycle discipline. Called when MainForm closes. + public void Dispose() => selfProcess.Dispose(); } diff --git a/src/RemSound.App/RecordingController.cs b/src/RemSound.App/RecordingController.cs index f0d1033..1ef1c89 100644 --- a/src/RemSound.App/RecordingController.cs +++ b/src/RemSound.App/RecordingController.cs @@ -149,6 +149,17 @@ internal sealed class RecordingController private void StartMultiTrack(RecordingSettings s, DateTime now) { + // Refuse a split received-only recording with no peers connected: it would make an empty dated + // folder and capture nothing while still announcing "recording started". Fail clearly instead + // (Start()'s catch surfaces this message). Both / sent-only still record your own send, so they + // never hit this. Checked BEFORE creating the folder so no orphan folder is left behind. + if (s.Source == RecordingSource.ReceivedOnly && (ConnectedPeersProvider?.Invoke() ?? []).Count == 0) + { + throw new InvalidOperationException( + "No peers are connected, so a split received-only recording would capture nothing. " + + "Connect to a peer first, or set the recording source to include your own sent audio."); + } + var folder = MultiTrackFolder(s, now); Directory.CreateDirectory(folder); var ext = AudioRecorder.ExtensionFor(s.FileFormat); diff --git a/src/RemSound.App/RecordingSettingsDialog.cs b/src/RemSound.App/RecordingSettingsDialog.cs index 6b29cf8..75c67ca 100644 --- a/src/RemSound.App/RecordingSettingsDialog.cs +++ b/src/RemSound.App/RecordingSettingsDialog.cs @@ -612,5 +612,9 @@ internal sealed class RecordingSettingsDialog : Form && a.OggOpusBitrateKbps == b.OggOpusBitrateKbps && a.FlacBitsPerSample == b.FlacBitsPerSample && a.FlacCompressionLevel == b.FlacCompressionLevel + // Include the two v5 toggles the dialog can change, so toggling only these still marks the + // profile dirty (before, changing just split-tracks / bypass-shaping reported "no change"). + && a.SplitTracks == b.SplitTracks + && a.BypassShaping == b.BypassShaping && string.Equals(a.Folder ?? string.Empty, b.Folder ?? string.Empty, StringComparison.OrdinalIgnoreCase); } diff --git a/src/RemSound.App/RemSound.App.csproj b/src/RemSound.App/RemSound.App.csproj index 3d94874..8485fd2 100644 --- a/src/RemSound.App/RemSound.App.csproj +++ b/src/RemSound.App/RemSound.App.csproj @@ -18,7 +18,7 @@ tag_name on the latest GitHub release; bump it on every public release. The AssemblyVersion / FileVersion default to this value, and Assembly.GetName().Version is what the About dialog and the updater both read. --> - 5.1 + 5.2 diff --git a/src/RemSound.App/RemSoundUpdater.cs b/src/RemSound.App/RemSoundUpdater.cs index 4c5622b..848d55d 100644 --- a/src/RemSound.App/RemSoundUpdater.cs +++ b/src/RemSound.App/RemSoundUpdater.cs @@ -30,7 +30,7 @@ namespace RemSound.App; /// single canonical release stream, not to be re-pointed at a fork. If you need to publish /// from a different repo, change / . /// -internal sealed class RemSoundUpdater : IDisposable +internal sealed class RemSoundUpdater { public const string RepoOwner = "Ednunp"; public const string RepoName = "RemSound"; @@ -51,11 +51,6 @@ internal sealed class RemSoundUpdater : IDisposable public string CurrentVersion => Assembly.GetExecutingAssembly().GetName().Version?.ToString(3) ?? "0.0.0"; - public void Dispose() - { - // HttpClient is static and shared across the process; nothing to dispose here. - } - /// Hit the GitHub Releases API, parse the latest release, return a struct /// describing what was found. Returns null if the request fails (network down, rate /// limited, repo not found) or if the latest version is not newer than the running @@ -259,7 +254,7 @@ internal sealed class RemSoundUpdater : IDisposable } Log?.Invoke($"updater: launching in-app installer from {appRoot}, parent PID {pid}"); - System.Diagnostics.Process.Start(psi); + using (System.Diagnostics.Process.Start(psi)) { } return true; } catch (Exception ex) diff --git a/src/RemSound.App/RouterPortMapper.cs b/src/RemSound.App/RouterPortMapper.cs index 3d12b3f..f3f9d1f 100644 --- a/src/RemSound.App/RouterPortMapper.cs +++ b/src/RemSound.App/RouterPortMapper.cs @@ -226,9 +226,12 @@ internal sealed class RouterPortMapper : IDisposable public void Dispose() { - if (disposed) return; - disposed = true; + // Run the full Stop() teardown FIRST (remove the router mapping, stop discovery, unsubscribe the + // static NatUtility.DeviceFound handler) while disposed is still false — otherwise Stop()'s own + // `if (disposed) return;` guard would skip all of it, leaving the forwarded port open until its + // lease expires and the object subscribed to the process-wide event. THEN mark disposed. try { Stop(); } catch { /* shutting down */ } + lock (gate) { disposed = true; } } private void OnDeviceFound(object? sender, DeviceEventArgs args) diff --git a/src/RemSound.App/StartupAutoStart.cs b/src/RemSound.App/StartupAutoStart.cs index 9bf60b4..1e23ae9 100644 --- a/src/RemSound.App/StartupAutoStart.cs +++ b/src/RemSound.App/StartupAutoStart.cs @@ -97,4 +97,33 @@ internal static class StartupAutoStart return false; } } + + /// Remove the Run-key entry ONLY if it currently points at an exe inside + /// . Used by the uninstaller so removing an INSTALLED copy never wipes a + /// DIFFERENT copy's autostart entry (e.g. a portable copy the user still wants launching at login) + /// that happens to share the single "RemSound" value name. Returns true if the entry is gone + /// afterwards or was left alone because it points elsewhere; false only on registry error. + public static bool TryDisableIfPointsInto(string folder) + { + try + { + if (string.IsNullOrWhiteSpace(folder)) return false; + using var key = Registry.CurrentUser.OpenSubKey(RunKeyPath, writable: true); + if (key is null) return true; // No Run subkey → nothing to disable. + var value = (key.GetValue(ValueName) as string)?.Trim().Trim('"'); + if (string.IsNullOrWhiteSpace(value)) return true; // nothing set for us. + var target = System.IO.Path.GetFullPath(folder) + .TrimEnd(System.IO.Path.DirectorySeparatorChar, System.IO.Path.AltDirectorySeparatorChar); + // Only our own installed copy's entry (its exe lives inside the folder being removed). + if (value.StartsWith(target, StringComparison.OrdinalIgnoreCase)) + { + key.DeleteValue(ValueName, throwOnMissingValue: false); + } + return true; + } + catch + { + return false; + } + } } diff --git a/src/RemSound.App/TabSwitchSoundService.cs b/src/RemSound.App/TabSwitchSoundService.cs index 14d0742..0af72ff 100644 --- a/src/RemSound.App/TabSwitchSoundService.cs +++ b/src/RemSound.App/TabSwitchSoundService.cs @@ -15,6 +15,9 @@ namespace RemSound.App; internal static class TabSwitchSoundService { private static CuePlayer? switchSound; + // Cached at Reload() so Play() (fires on every focused tab change) doesn't re-read + re-deserialize + // the whole config file just to fetch one bool. Reload() runs whenever cue settings change. + private static bool enableTabSwitch; /// When true, is a no-op. Reserved for bulk programmatic tab changes /// the focus gate doesn't already cover; mirrors . @@ -24,13 +27,15 @@ internal static class TabSwitchSoundService /// settings change, alongside . public static void Reload() { - switchSound = LoadCue(MainForm.CueId.TabSwitch, "tab switch.wav", AppConfig.Load()); + var cfg = AppConfig.Load(); + enableTabSwitch = cfg.EnableTabSwitchCue; + switchSound = LoadCue(MainForm.CueId.TabSwitch, "tab switch.wav", cfg); } public static void Play() { if (Suppressed) return; - if (AppConfig.Load().EnableTabSwitchCue) switchSound?.Play(); + if (enableTabSwitch) switchSound?.Play(); } private static CuePlayer? LoadCue(string cueId, string defaultFile, AppConfig cfg) diff --git a/src/RemSound.App/UpdateApplier.cs b/src/RemSound.App/UpdateApplier.cs index 4e0ec2f..01b4188 100644 --- a/src/RemSound.App/UpdateApplier.cs +++ b/src/RemSound.App/UpdateApplier.cs @@ -204,12 +204,24 @@ internal static class UpdateApplier { var exe = Path.Combine(target, "RemSound.exe"); if (!File.Exists(exe)) { log($"cannot restart — {exe} missing"); return; } - Process.Start(new ProcessStartInfo { FileName = exe, WorkingDirectory = target, UseShellExecute = true }); + // Give the restarted copy the same foreground treatment as the post-install relaunch, so it + // doesn't reopen BEHIND other windows where a blind user wouldn't notice it came back (the + // old copy has already exited, so a fresh process has no foreground credit of its own). + // --foreground makes it pull itself forward; the AllowSetForegroundWindow grant is + // best-effort (this staged updater may not hold foreground rights to give away). + var psi = new ProcessStartInfo { FileName = exe, WorkingDirectory = target, UseShellExecute = true }; + psi.ArgumentList.Add("--foreground"); + using var child = Process.Start(psi); + if (child is not null) { try { AllowSetForegroundWindow(child.Id); } catch { } } log("RemSound restarted"); } catch (Exception ex) { log($"could not restart RemSound: {ex.Message}"); } } + [System.Runtime.InteropServices.DllImport("user32.dll")] + [return: System.Runtime.InteropServices.MarshalAs(System.Runtime.InteropServices.UnmanagedType.Bool)] + private static extern bool AllowSetForegroundWindow(int dwProcessId); + /// Best-effort temp cleanup. We're running FROM the stage, so we can't delete our own /// exe's folder here — the restarted app finishes that on startup (see Program.CleanUpUpdateStages). private static void CleanupStage(string? stageRoot, Action log) diff --git a/src/RemSound.Core/AudioRingBuffer.cs b/src/RemSound.Core/AudioRingBuffer.cs index 3945b9e..0b25478 100644 --- a/src/RemSound.Core/AudioRingBuffer.cs +++ b/src/RemSound.Core/AudioRingBuffer.cs @@ -49,6 +49,17 @@ public sealed class AudioRingBuffer /// Producer side. Writes the entire span; if the buffer is full, drops the oldest bytes to make room. public void Write(ReadOnlySpan source) { + if (source.Length > storage.Length) + { + // A single write bigger than the whole ring can't fit. Keep only the NEWEST storage.Length + // bytes and count the older excess as dropped, so an oversized write degrades to dropped + // audio instead of throwing (a CopyTo overflow) on this lock-free real-time producer path. + // Unreachable in normal use — writes are MTU-bounded and the ring is far larger — but this + // turns an undocumented precondition into a safe, real invariant. + Interlocked.Add(ref drops, source.Length - storage.Length); + source = source[^storage.Length..]; + } + var currentTail = tail; var currentHead = Volatile.Read(ref head); var available = storage.Length - ((currentTail - currentHead) & 0x7FFFFFFF); diff --git a/src/RemSound.Core/RemSoundSettingsStore.cs b/src/RemSound.Core/RemSoundSettingsStore.cs index d358346..4990096 100644 --- a/src/RemSound.Core/RemSoundSettingsStore.cs +++ b/src/RemSound.Core/RemSoundSettingsStore.cs @@ -102,7 +102,7 @@ public sealed class RemSoundSettingsStore public void SaveMaxLatencyMs(int value) { var s = Load() ?? new Settings(); - s.MaxLatencyMs = Math.Clamp(value, 1, 500); + s.MaxLatencyMs = Math.Clamp(value, 5, 500); // match LoadMaxLatencyMs's [5,500] so stored == effective Save(s); } @@ -121,7 +121,7 @@ public sealed class RemSoundSettingsStore public void SaveMaxLatencyMsAsio(int value) { var s = Load() ?? new Settings(); - s.MaxLatencyMsAsio = Math.Clamp(value, 1, 500); + s.MaxLatencyMsAsio = Math.Clamp(value, 5, 500); // match LoadMaxLatencyMsAsio's [5,500] Save(s); } diff --git a/src/RemSound.Receiver/AudioReceiver.cs b/src/RemSound.Receiver/AudioReceiver.cs index e55cd40..8fb67ae 100644 --- a/src/RemSound.Receiver/AudioReceiver.cs +++ b/src/RemSound.Receiver/AudioReceiver.cs @@ -176,8 +176,6 @@ public sealed class AudioReceiver : IDisposable if (wasRunning) multiOutput.Start(); } - public bool IsAsioBackend => multiOutput is CompositeRenderBackend; - /// Sets the Buffer-smoothness knob (1 = aggressive — clicks the buffer back /// to target on any drift, holds the user's latency tightly; 10 = smooth — no clicks /// but the queue can creep up under jitter or sustained clock drift). Knob drives a @@ -691,8 +689,14 @@ public sealed class AudioReceiver : IDisposable Interlocked.Exchange(ref bytesReceived, 0); Interlocked.Exchange(ref packetsDropped, 0); - // Tear down any sessions left over from a previous Start (in case Stop wasn't called). - DisposeAllSessionsLocked(); + // Tear down any sessions left over from a previous Start (in case Stop wasn't called). Under + // sessionsLock to match the method's "Locked" contract and its other two callers — the counter + // getters / prune run on the App's snapshot-tick thread and touch `sessions` under this lock, so + // clearing it bare would be an unsynchronised mutation of the non-thread-safe dictionary. + lock (sessionsLock) + { + DisposeAllSessionsLocked(); + } playoutEngine.ResetAll(); listener.Start(udpPort); @@ -1073,7 +1077,21 @@ public sealed class AudioReceiver : IDisposable isFormatChange = true; } - newSession = new StreamSession(remote, streamId, format, sp, diagnostics, _ => sp.NoteFramesQueued(playoutEngine.TargetLatencyMs), decryptor); + try + { + newSession = new StreamSession(remote, streamId, format, sp, diagnostics, _ => sp.NoteFramesQueued(playoutEngine.TargetLatencyMs), decryptor); + } + catch + { + // The StreamSession ctor failed — most realistically because a corrupt/hostile Format + // announced an Opus sample rate/channel count the decoder rejects. We already registered + // the SessionPlayout in PlayoutEngine via GetOrCreateSession above; nothing was added to + // `sessions`, so PruneIdleSessions would never reap it and it would linger forever, summed + // on every render callback and poisoning the auto-tune's underrun stats. Reap it here, + // then let the exception propagate so the packet handler still logs it. + playoutEngine.RemoveSession(remote, streamId); + throw; + } sessions[key] = newSession; // Same-lane streamId rotation: drop other sessions from this peer that share the diff --git a/src/RemSound.Receiver/NetworkListener.cs b/src/RemSound.Receiver/NetworkListener.cs index cca3561..0efd99f 100644 --- a/src/RemSound.Receiver/NetworkListener.cs +++ b/src/RemSound.Receiver/NetworkListener.cs @@ -10,9 +10,12 @@ namespace RemSound.Receiver; /// Hands raw packets (byte buffer + length + remote endpoint) up to a callback supplied by the /// owner — has no idea what's inside the packets. /// -/// Allocation-free in steady state: one fixed receive buffer reused across calls, -/// with avoids the per-call -/// IPEndPoint boxing that incurred. +/// The receive buffer is a single fixed array reused across calls. NOTE: the current +/// overload still +/// allocates a SocketAddress + a fresh IPEndPoint per datagram — tiny (tens of bytes) and dwarfed +/// by decode/mix work, but not literally allocation-free. A future optimisation could switch to the +/// Span/SocketAddress overload with a cached SocketAddress and only materialise an IPEndPoint when a +/// new session actually opens. /// internal sealed class NetworkListener : IDisposable { diff --git a/src/RemSound.Receiver/PeerDspChain.cs b/src/RemSound.Receiver/PeerDspChain.cs index c3913fe..db470bf 100644 --- a/src/RemSound.Receiver/PeerDspChain.cs +++ b/src/RemSound.Receiver/PeerDspChain.cs @@ -110,20 +110,41 @@ public sealed class PeerDspChain int n = left.Length; if (n > 0) { - for (int f = 0; f < frames; f++) + // Fold the post-EQ gain into the EQ loop's final store, so a peer with both EQ and non-unity + // gain walks the block ONCE on the render thread instead of twice (branch hoisted out of the + // per-frame loop). n==0 + gain-only keeps its own single pass below. + if (hasGain) { - float sl = output[2 * f]; - float sr = output[2 * f + 1]; - for (int b = 0; b < n; b++) + for (int f = 0; f < frames; f++) { - sl = left[b].Transform(sl); - sr = right[b].Transform(sr); + float sl = output[2 * f]; + float sr = output[2 * f + 1]; + for (int b = 0; b < n; b++) + { + sl = left[b].Transform(sl); + sr = right[b].Transform(sr); + } + output[2 * f] = sl * gainL; + output[2 * f + 1] = sr * gainR; + } + } + else + { + for (int f = 0; f < frames; f++) + { + float sl = output[2 * f]; + float sr = output[2 * f + 1]; + for (int b = 0; b < n; b++) + { + sl = left[b].Transform(sl); + sr = right[b].Transform(sr); + } + output[2 * f] = sl; + output[2 * f + 1] = sr; } - output[2 * f] = sl; - output[2 * f + 1] = sr; } } - if (hasGain) + else if (hasGain) { for (int f = 0; f < frames; f++) { diff --git a/src/RemSound.Receiver/PlayoutEngine.cs b/src/RemSound.Receiver/PlayoutEngine.cs index 5cf3efb..63a57c7 100644 --- a/src/RemSound.Receiver/PlayoutEngine.cs +++ b/src/RemSound.Receiver/PlayoutEngine.cs @@ -657,15 +657,15 @@ internal sealed class PlayoutEngine : IWaveProvider /// /// Render-side audio pull. Iterates every session regardless of lane tag and sums them - /// into one mixed bus. This is what every render backend (WasapiOnly, AsioOnly, classic - /// Both via the tee, and BothIndependent via the tee) reads from, so a user can pick any - /// output device for any received audio — independently of which capture technology the - /// sender used. Per-lane latency targets are still honoured: each session reads its own - /// route's TargetMs / MaxMs via , so the WASAPI-captured stream - /// can buffer at one latency and the ASIO-captured stream at another within the same - /// output mix. The lane-specific / - /// surfaces are kept around for future per-route routing options but are not used by the - /// default render path (see CompositeRenderBackend). 2026-05-11 revision: previous + /// into one mixed bus. This is what the WasapiOnly / AsioOnly render path reads from, so a + /// user can pick any output device for any received audio — independently of which capture + /// technology the sender used. Per-lane latency targets are still honoured: each session + /// reads its own route's TargetMs / MaxMs via , so the WASAPI-captured + /// stream can buffer at one latency and the ASIO-captured stream at another within the same + /// output mix. In BothIndependent mode this all-sessions pull is NOT used: each lane reads its + /// own filtered surface ( / ) directly + /// — see CompositeRenderBackend — so those surfaces are ACTIVE render sources, not + /// future-only. 2026-05-11 revision: previous /// implementation filtered by route, which made it impossible to route a WASAPI-captured /// stream onto an ASIO output (and vice versa) in BothIndependent mode — that broke a /// long-standing cross-backend send/receive flow. diff --git a/src/RemSound.Receiver/SessionPlayout.cs b/src/RemSound.Receiver/SessionPlayout.cs index 8820976..614d735 100644 --- a/src/RemSound.Receiver/SessionPlayout.cs +++ b/src/RemSound.Receiver/SessionPlayout.cs @@ -435,36 +435,6 @@ internal sealed class SessionPlayout : IDisposable drainRequested = true; } - /// Reset the buffer and per-session state. Used at start/stop. Arming will rebuild - /// from the next packets that arrive. - public void Reset() - { - playout.Reset(); - playbackArmed = false; - largestWriteMs = 0; - inUnderrunConcealment = false; - consecutiveEmptyReads = 0; - lastConcealSampleL = 0f; - lastConcealSampleR = 0f; - filteredErrorFrames = 0; - prevDriftSampleTicks = 0; - trimGlideTargetMs = 0; - prevTrimGlideTicks = 0; - // Phase-4 drift resampler state. Reset counters and window state. Reset() on the - // resampler clears its internal filter delay line so a fresh session doesn't - // inherit phase from a prior one. SetRates back to 1:1 — we'll re-measure drift - // from scratch. - bytesWrittenForDriftEst = 0; - bytesReadOutputForDriftEst = 0; - resamplerWindowStartTicks = 0; - resamplerWindowStartBytesWritten = 0; - resamplerWindowStartBytesOutput = 0; - smoothedRateRatio = 1.0; - resamplerActivelyTracking = false; - driftResampler.Reset(); - driftResampler.SetRates(MixSampleRate, MixSampleRate); - } - public void Dispose() { // AudioRingBuffer is managed; nothing to free explicitly. Method present for symmetry diff --git a/src/RemSound.Receiver/StreamSession.cs b/src/RemSound.Receiver/StreamSession.cs index 53458e1..9072a46 100644 --- a/src/RemSound.Receiver/StreamSession.cs +++ b/src/RemSound.Receiver/StreamSession.cs @@ -114,8 +114,6 @@ internal sealed class StreamSession : IDisposable && Format.Channels == format.Channels && Format.FrameSamplesPerChannel == format.FrameSamplesPerChannel; - public bool IsSameEndpoint(IPEndPoint endpoint) => Endpoint.Equals(endpoint); - public bool HandleAudioPayload(uint sequence, ReadOnlySpan payload) { diagnostics.RecordPacketArrived(); diff --git a/src/RemSound.Sender/AudioSender.cs b/src/RemSound.Sender/AudioSender.cs index a01fe83..8ec013f 100644 --- a/src/RemSound.Sender/AudioSender.cs +++ b/src/RemSound.Sender/AudioSender.cs @@ -421,8 +421,6 @@ public sealed class AudioSender : IDisposable } } - public bool IsAsioBackend => engine is CompositeCaptureBackend; - /// Updates the PCM frame size based on the user's "Send rate" choice. For Opus, /// frame size is set via 's opusFrameSamplesPerChannel /// parameter (the App halves it when SendRate is Tight). On a frame-size change, resets diff --git a/src/RemSound.Sender/CompositeCaptureBackend.cs b/src/RemSound.Sender/CompositeCaptureBackend.cs index 5c426b5..cc1d9e1 100644 --- a/src/RemSound.Sender/CompositeCaptureBackend.cs +++ b/src/RemSound.Sender/CompositeCaptureBackend.cs @@ -102,10 +102,11 @@ internal sealed class CompositeCaptureBackend : ICaptureBackend public long TotalCaptureBytes => (wasapi?.TotalCaptureBytes ?? 0) + (asio?.TotalCaptureBytes ?? 0); public string? FirstCaptureFormatDescription => asio?.FirstCaptureFormatDescription ?? wasapi?.FirstCaptureFormatDescription; public string? FirstCaptureLastError => asio?.FirstCaptureLastError ?? wasapi?.FirstCaptureLastError; - // ClippedSampleCount lived on the (now-removed) classic-Both mix loop; the per-lane - // BothIndependent pipeline has no shared mix bus to clip. Kept as 0 so any UI binding - // that still reads it doesn't NRE. - public long ClippedSampleCount => 0; + // Sum both inner backends' clip counters. Each per-lane backend (MixingEngine / PushModeWasapi / + // Asio) still clamps and counts clipped samples; the earlier "no shared mix bus, so always 0" was + // stale and silently zeroed the clip diagnostic that the SNAP log reports (clipΔ), leaving real + // clipping invisible — which matters given RemSound's click-vs-clip debugging history. + public long ClippedSampleCount => (wasapi?.ClippedSampleCount ?? 0) + (asio?.ClippedSampleCount ?? 0); /// Worst callback-gap across both inner backends. We have to take from BOTH (so /// each inner's counter resets), then return the larger — otherwise the unread inner diff --git a/src/RemSound.Sender/SenderLane.cs b/src/RemSound.Sender/SenderLane.cs index 4c7cfbc..afcec77 100644 --- a/src/RemSound.Sender/SenderLane.cs +++ b/src/RemSound.Sender/SenderLane.cs @@ -62,6 +62,15 @@ internal sealed class SenderLane private OpusEncoderState opusEncoder; private int opusFrameStereoSamples; + // Serialises the Opus encoder SWAP (OnCodecChanged, UI thread) against its USE (EmitOpusFrame, + // capture thread). Without it, changing codec or send-rate while streaming could Dispose the native + // libopus encoder mid-Encode on the capture thread — a native use-after-free / hard crash with no + // managed stack. Held only for the encode + copy-out (microseconds) and the rare swap, so hot-path + // contention is negligible. + private readonly object encoderGate = new(); + // Copy of the just-encoded Opus bytes, taken under encoderGate so encryption + send can run OUTSIDE + // the lock — LastEncoded returns a span into the encoder's own buffer, which the swap frees. + private readonly byte[] opusPlainScratch = new byte[4096]; // Per-lane pre-encode discontinuity probe. Moved here from AudioSender (2026-05-15) so // each lane has its OWN probe state and the cross-buffer step measurement (which carries @@ -154,13 +163,17 @@ internal sealed class SenderLane { if (newCodec == AudioTransportCodec.Opus) { - // Dispose the outgoing encoder before replacing it — its underlying - // NativeOpusEncoder owns native libopus state that doesn't get released until - // explicit Dispose under our SustainedLowLatency GC mode. Pre-2026-05-27 this - // overwrite leaked the old encoder's native state on every codec change. - opusEncoder.Dispose(); - opusEncoder = new OpusEncoderState(opusFrameSamplesPerChannel, opusBitrate); - opusFrameStereoSamples = opusEncoder.FrameSizePerChannel * MixChannels; + // Dispose the outgoing encoder before replacing it — its underlying NativeOpusEncoder owns + // native libopus state that doesn't get released until explicit Dispose under our + // SustainedLowLatency GC mode. Under encoderGate so the capture thread can't be mid-Encode on + // the old native encoder when we free it (that was a native use-after-free on a codec/rate + // change while streaming). + lock (encoderGate) + { + opusEncoder.Dispose(); + opusEncoder = new OpusEncoderState(opusFrameSamplesPerChannel, opusBitrate); + opusFrameStereoSamples = opusEncoder.FrameSizePerChannel * MixChannels; + } } streamId = NewStreamId(); lastFormatPacketUtc = DateTime.MinValue; @@ -319,23 +332,28 @@ internal sealed class SenderLane private void EmitOpusFrame(ReadOnlySpan stereoFloats) { - ReadOnlySpan opusBytes; - if (owner.IsMuted) + int encLen; + // Encode and copy the bytes out UNDER encoderGate, so a concurrent OnCodecChanged can't Dispose + // the encoder mid-Encode (native use-after-free) or free the LastEncoded buffer before we copy + // it. Crypto + send run outside the lock, off the copied bytes. + lock (encoderGate) { - Span silence = stackalloc float[opusFrameStereoSamples]; - silence.Clear(); - var muteLen = opusEncoder.Encode(silence); - opusBytes = opusEncoder.LastEncoded(muteLen); - } - else - { - var len = opusEncoder.Encode(stereoFloats); - if (len <= 0) return; - opusBytes = opusEncoder.LastEncoded(len); + if (owner.IsMuted) + { + Span silence = stackalloc float[opusFrameStereoSamples]; + silence.Clear(); + encLen = opusEncoder.Encode(silence); + } + else + { + encLen = opusEncoder.Encode(stereoFloats); + } + if (encLen <= 0) return; + opusEncoder.LastEncoded(encLen).CopyTo(opusPlainScratch); } EnsureCrypto(); if (cryptoGcm is null) return; // no password yet → never send audio in the clear - var ctLen = RemSoundCrypto.EncryptInto(cryptoGcm, opusBytes, cipherScratch); + var ctLen = RemSoundCrypto.EncryptInto(cryptoGcm, opusPlainScratch.AsSpan(0, encLen), cipherScratch); Interlocked.Increment(ref audioFramesSent); SendAudio(cipherScratch.AsSpan(0, ctLen)); }