P2 leftovers: every inline dialog now audited + one shared sample clamp (review complete bar peer extraction)
Inline-built dialogs were invisible to the accessibility audit - which is exactly how mnemonic-less buttons kept slipping through (the single-instance dialog Andre caught, and now TWO more found by this very change: ManualPeerPrompt's OK/Cancel and ProfileSaveAsPrompt's Cancel had no mnemonics - both fixed). Each inline dialog's construction is split from its ShowDialog into a Build seam, and the audit now covers 15 dialog surfaces (was 8): manual peer prompt, quick profile switch, change password, password manager, profile name prompt, and the service Additional-options window join the eight Form dialogs. Behaviour unchanged. The encoder-boundary clamp is now ONE shared rule (Core SampleClamp) instead of three private copies in MixingEngine / AsioCaptureBackend / PushModeWasapiBackend - and the ASIO copy's up-to-four interlocked increments per frame became one batched add per buffer on the RT thread. Pinned by a new self-test (over-range clamps to exactly +/-1 and counts; +/-1 exactly passes untouched). Gate 61/61. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -4,7 +4,19 @@ internal static class ManualPeerPrompt
|
||||
{
|
||||
public static string? Show(IWin32Window owner)
|
||||
{
|
||||
using var dialog = new Form
|
||||
var (dialog, textBox) = Build();
|
||||
using (dialog)
|
||||
{
|
||||
return dialog.ShowDialog(owner) == DialogResult.OK ? textBox.Text.Trim() : null;
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>Construction split from ShowDialog so the accessibility audit can inspect the real
|
||||
/// dialog (inline-built dialogs used to be invisible to the audit — which is exactly how missing
|
||||
/// mnemonics slipped through).</summary>
|
||||
internal static (Form Dialog, TextBox Input) Build()
|
||||
{
|
||||
var dialog = new Form
|
||||
{
|
||||
Text = "Add manual peer",
|
||||
StartPosition = FormStartPosition.CenterParent,
|
||||
@@ -24,8 +36,8 @@ internal static class ManualPeerPrompt
|
||||
Width = 380,
|
||||
AccessibleName = "Peer IP address or hostname",
|
||||
};
|
||||
var okButton = new Button { Text = "OK", AutoSize = true, DialogResult = DialogResult.OK };
|
||||
var cancelButton = new Button { Text = "Cancel", AutoSize = true, DialogResult = DialogResult.Cancel };
|
||||
var okButton = new Button { Text = "&OK", AutoSize = true, DialogResult = DialogResult.OK };
|
||||
var cancelButton = new Button { Text = "&Cancel", AutoSize = true, DialogResult = DialogResult.Cancel };
|
||||
textBox.KeyDown += (_, args) =>
|
||||
{
|
||||
if (args.KeyCode == Keys.Enter)
|
||||
@@ -46,6 +58,6 @@ internal static class ManualPeerPrompt
|
||||
dialog.Controls.Add(panel);
|
||||
dialog.AcceptButton = okButton;
|
||||
dialog.CancelButton = cancelButton;
|
||||
return dialog.ShowDialog(owner) == DialogResult.OK ? textBox.Text.Trim() : null;
|
||||
return (dialog, textBox);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -15,7 +15,24 @@ internal static class ProfilePasswordDialog
|
||||
{
|
||||
public static string? Show(string profileTitle, string currentPassword, bool requireNonEmpty = false)
|
||||
{
|
||||
using var dialog = new Form
|
||||
var (dialog, textBox) = Build(profileTitle, currentPassword, requireNonEmpty);
|
||||
using (dialog)
|
||||
{
|
||||
// Run with a foreground 1×1 owner so the prompt jumps to the front even when RemSound is
|
||||
// sitting minimised in the tray — e.g. a quick profile switch to a passwordless-but-
|
||||
// streaming profile trips the password gate mid-switch, and the user must be able to read
|
||||
// and answer it there and then. Centres on screen, forces focus, then closes.
|
||||
return ForegroundDialog.Show(owner => dialog.ShowDialog(owner)) == DialogResult.OK
|
||||
? textBox.Text.Trim()
|
||||
: null;
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>Construction split from ShowDialog so the accessibility audit can inspect the real
|
||||
/// dialog (inline-built dialogs used to be invisible to the audit).</summary>
|
||||
internal static (Form Dialog, TextBox Input) Build(string profileTitle, string currentPassword, bool requireNonEmpty = false)
|
||||
{
|
||||
var dialog = new Form
|
||||
{
|
||||
Text = "Change profile password",
|
||||
StartPosition = FormStartPosition.CenterParent,
|
||||
@@ -100,13 +117,6 @@ internal static class ProfilePasswordDialog
|
||||
dialog.Controls.Add(panel);
|
||||
dialog.AcceptButton = okButton;
|
||||
dialog.CancelButton = cancelButton;
|
||||
|
||||
// Run with a foreground 1×1 owner so the prompt jumps to the front even when RemSound is
|
||||
// sitting minimised in the tray — e.g. a quick profile switch to a passwordless-but-
|
||||
// streaming profile trips the password gate mid-switch, and the user must be able to read
|
||||
// and answer it there and then. Centres on screen, forces focus, then closes.
|
||||
return ForegroundDialog.Show(owner => dialog.ShowDialog(owner)) == DialogResult.OK
|
||||
? textBox.Text.Trim()
|
||||
: null;
|
||||
return (dialog, textBox);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -14,10 +14,22 @@ namespace RemSound.App;
|
||||
internal static class ProfilePasswordManagerDialog
|
||||
{
|
||||
public static bool Show(IWin32Window owner, ProfileStore store)
|
||||
{
|
||||
var (dialog, rows) = Build(store);
|
||||
using (dialog)
|
||||
{
|
||||
if (dialog.ShowDialog(owner) != DialogResult.OK) return false;
|
||||
return SaveChanges(store, rows);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>Construction split from ShowDialog so the accessibility audit can inspect the real
|
||||
/// dialog (inline-built dialogs used to be invisible to the audit).</summary>
|
||||
internal static (Form Dialog, List<(string Title, string Original, TextBox Box)> Rows) Build(ProfileStore store)
|
||||
{
|
||||
var titles = store.ListProfileTitles();
|
||||
|
||||
using var dialog = new Form
|
||||
var dialog = new Form
|
||||
{
|
||||
Text = "Profile passwords",
|
||||
StartPosition = FormStartPosition.CenterParent,
|
||||
@@ -77,9 +89,11 @@ internal static class ProfilePasswordManagerDialog
|
||||
dialog.Controls.Add(intro);
|
||||
dialog.AcceptButton = okButton;
|
||||
dialog.CancelButton = cancelButton;
|
||||
return (dialog, rows);
|
||||
}
|
||||
|
||||
if (dialog.ShowDialog(owner) != DialogResult.OK) return false;
|
||||
|
||||
private static bool SaveChanges(ProfileStore store, List<(string Title, string Original, TextBox Box)> rows)
|
||||
{
|
||||
var changedAny = false;
|
||||
foreach (var (title, original, box) in rows)
|
||||
{
|
||||
|
||||
@@ -23,7 +23,38 @@ internal static class ProfileSaveAsPrompt
|
||||
string dialogTitle = "Save profile as",
|
||||
string promptLabel = "Profile name:")
|
||||
{
|
||||
using var dialog = new Form
|
||||
var (dialog, textBox) = Build(defaultName, dialogTitle, promptLabel);
|
||||
using (dialog)
|
||||
{
|
||||
while (true)
|
||||
{
|
||||
if (dialog.ShowDialog(owner) != DialogResult.OK) return null;
|
||||
var name = textBox.Text.Trim();
|
||||
if (string.IsNullOrWhiteSpace(name))
|
||||
{
|
||||
MessageBox.Show(owner, "Please enter a profile name.", "RemSound", MessageBoxButtons.OK, MessageBoxIcon.Warning);
|
||||
continue;
|
||||
}
|
||||
if (store is not null && store.Exists(name))
|
||||
{
|
||||
var overwrite = MessageBox.Show(owner,
|
||||
$"A profile named \"{name}\" already exists. Overwrite?",
|
||||
"Confirm overwrite", MessageBoxButtons.YesNo, MessageBoxIcon.Question, MessageBoxDefaultButton.Button2);
|
||||
if (overwrite != DialogResult.Yes) continue;
|
||||
}
|
||||
return name;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>Construction split from ShowDialog so the accessibility audit can inspect the real
|
||||
/// dialog (inline-built dialogs used to be invisible to the audit).</summary>
|
||||
internal static (Form Dialog, TextBox Input) Build(
|
||||
string? defaultName = null,
|
||||
string dialogTitle = "Save profile as",
|
||||
string promptLabel = "Profile name:")
|
||||
{
|
||||
var dialog = new Form
|
||||
{
|
||||
Text = dialogTitle,
|
||||
StartPosition = FormStartPosition.CenterParent,
|
||||
@@ -40,7 +71,7 @@ internal static class ProfileSaveAsPrompt
|
||||
AccessibleName = promptLabel.TrimEnd(':', ' '),
|
||||
};
|
||||
var okButton = new Button { Text = "&OK", AutoSize = true, DialogResult = DialogResult.OK };
|
||||
var cancelButton = new Button { Text = "Cancel", AutoSize = true, DialogResult = DialogResult.Cancel };
|
||||
var cancelButton = new Button { Text = "&Cancel", AutoSize = true, DialogResult = DialogResult.Cancel };
|
||||
textBox.KeyDown += (_, args) =>
|
||||
{
|
||||
if (args.KeyCode == Keys.Enter)
|
||||
@@ -72,24 +103,6 @@ internal static class ProfileSaveAsPrompt
|
||||
dialog.Controls.Add(panel);
|
||||
dialog.AcceptButton = okButton;
|
||||
dialog.CancelButton = cancelButton;
|
||||
|
||||
while (true)
|
||||
{
|
||||
if (dialog.ShowDialog(owner) != DialogResult.OK) return null;
|
||||
var name = textBox.Text.Trim();
|
||||
if (string.IsNullOrWhiteSpace(name))
|
||||
{
|
||||
MessageBox.Show(owner, "Please enter a profile name.", "RemSound", MessageBoxButtons.OK, MessageBoxIcon.Warning);
|
||||
continue;
|
||||
}
|
||||
if (store is not null && store.Exists(name))
|
||||
{
|
||||
var overwrite = MessageBox.Show(owner,
|
||||
$"A profile named \"{name}\" already exists. Overwrite?",
|
||||
"Confirm overwrite", MessageBoxButtons.YesNo, MessageBoxIcon.Question, MessageBoxDefaultButton.Button2);
|
||||
if (overwrite != DialogResult.Yes) continue;
|
||||
}
|
||||
return name;
|
||||
}
|
||||
return (dialog, textBox);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -19,8 +19,25 @@ internal sealed class QuickProfileSwitchDialog
|
||||
public static string? Show(IReadOnlyList<ProfileEntry> profiles)
|
||||
{
|
||||
if (profiles.Count == 0) return null;
|
||||
var (dialog, list, chosenPath) = Build(profiles);
|
||||
using (dialog)
|
||||
{
|
||||
dialog.Shown += (_, _) =>
|
||||
{
|
||||
BringToForeground(dialog);
|
||||
list.Focus();
|
||||
};
|
||||
return dialog.ShowDialog() == DialogResult.OK ? chosenPath() : null;
|
||||
}
|
||||
}
|
||||
|
||||
using var dialog = new Form
|
||||
/// <summary>Audit seam: the real dialog, built with a placeholder profile, never shown. Inline-built
|
||||
/// dialogs used to be invisible to the accessibility audit — how missing mnemonics slipped through.</summary>
|
||||
internal static Form BuildForAudit() => Build(new[] { new ProfileEntry("Audit profile", "audit", true) }).Dialog;
|
||||
|
||||
private static (Form Dialog, ListBox List, Func<string?> ChosenPath) Build(IReadOnlyList<ProfileEntry> profiles)
|
||||
{
|
||||
var dialog = new Form
|
||||
{
|
||||
Text = "Quick profile switch",
|
||||
AccessibleName = "Quick profile switch",
|
||||
@@ -123,14 +140,7 @@ internal sealed class QuickProfileSwitchDialog
|
||||
}
|
||||
};
|
||||
|
||||
dialog.Shown += (_, _) =>
|
||||
{
|
||||
BringToForeground(dialog);
|
||||
list.Focus();
|
||||
};
|
||||
|
||||
var result = dialog.ShowDialog();
|
||||
return result == DialogResult.OK ? chosenPath : null;
|
||||
return (dialog, list, () => chosenPath);
|
||||
}
|
||||
|
||||
private static void BringToForeground(Form form)
|
||||
|
||||
@@ -69,6 +69,7 @@ internal static class SelfTest
|
||||
RunStep(results, "Cue variant resolution (dedupe, order, chosen-default fallback)", CueVariantResolution);
|
||||
RunStep(results, "Updater swap + rollback (a failed update restores exactly)", UpdaterSwapRollback);
|
||||
RunStep(results, "ASIO per-driver tick memory (snapshot/restore round-trip)", AsioTickMemory);
|
||||
RunStep(results, "Encoder-boundary sample clamp (shared rule, exact count)", SampleClampRule);
|
||||
RunStep(results, "App settings save and reload", SettingsRoundTrip);
|
||||
RunStep(results, "Per-peer shaping DSP", PeerShapingDsp);
|
||||
RunStep(results, "Multi-output fan-out (both lanes)", FanOutToBothOutputs);
|
||||
@@ -964,6 +965,22 @@ internal static class SelfTest
|
||||
return "follower flagged + sentinel shared with the app; service resolves it to the live default render endpoint";
|
||||
}
|
||||
|
||||
/// <summary>The encoder-boundary clamp is now ONE shared rule (SampleClamp) used by all three
|
||||
/// capture paths — mix engine, ASIO backend, push-mode backend. A clamp bug here means encoder
|
||||
/// overflow when loud sources sum past unity, while packets still flow. Pins: over-range clamps to
|
||||
/// exactly ±1, in-range (including exactly ±1) is untouched and uncounted, count is exact.</summary>
|
||||
private static string? SampleClampRule()
|
||||
{
|
||||
var buf = new[] { 0.5f, 1f, 1.5f, -1f, -2.5f, 0f, -0.999f };
|
||||
var clipped = SampleClamp.ClampBuffer(buf);
|
||||
Check(clipped == 2, $"exactly the two over-range samples must count as clipped (got {clipped})");
|
||||
Check(buf[2] == 1f && buf[4] == -1f, "over-range samples must clamp to exactly ±1");
|
||||
Check(buf[0] == 0.5f && buf[1] == 1f && buf[3] == -1f && buf[5] == 0f && buf[6] == -0.999f,
|
||||
"in-range samples — including exactly ±1 — must pass through untouched");
|
||||
Check(SampleClamp.ClampBuffer(buf) == 0, "a clean buffer must count zero");
|
||||
return "over-range → ±1 and counted; ±1 exactly and below untouched; count exact";
|
||||
}
|
||||
|
||||
/// <summary>Per-driver ASIO tick memory (Ed's EVO→ReaRoute→EVO silence): switching drivers clears
|
||||
/// the pair ticks by design (pair N is a different physical channel on a different card), and the
|
||||
/// memory restores each driver's OWN ticks on return. Pins the snapshot/restore round-trip, that a
|
||||
@@ -1866,6 +1883,15 @@ internal static class SelfTest
|
||||
("Keyboard shortcut import", () => new KeyboardShortcutImportDialog(Array.Empty<string>())),
|
||||
("Update install notice", () => new UpdateInstallNoticeDialog(
|
||||
new UpdateInfo("v9.9", new Version(9, 9, 0), "https://example.invalid/x.zip", "notes", "https://example.invalid/rel"))),
|
||||
// The inline-built dialogs, via their Build seams — previously invisible to this audit,
|
||||
// which is exactly how mnemonic-less buttons slipped through (single-instance, manual peer).
|
||||
("Manual peer prompt", () => ManualPeerPrompt.Build().Dialog),
|
||||
("Quick profile switch", QuickProfileSwitchDialog.BuildForAudit),
|
||||
("Change profile password", () => ProfilePasswordDialog.Build("Audit", "pw").Dialog),
|
||||
("Profile passwords manager", () => ProfilePasswordManagerDialog.Build(new ProfileStore(
|
||||
Path.Combine(Path.GetTempPath(), "remsound-selftest-pwmgr-" + Guid.NewGuid().ToString("N")))).Dialog),
|
||||
("Profile name prompt", () => ProfileSaveAsPrompt.Build("Audit").Dialog),
|
||||
("Service additional options", () => ServiceProfileDialog.BuildAdditionalOptions(false).Dialog),
|
||||
("Profile selection", () => new ProfileSelectionDialog(new ProfileStore(
|
||||
Path.Combine(Path.GetTempPath(), "remsound-selftest-picker-" + Guid.NewGuid().ToString("N"))))),
|
||||
};
|
||||
|
||||
@@ -334,7 +334,23 @@ internal sealed class ServiceProfileDialog : Form
|
||||
|
||||
private void ShowAdditionalOptions()
|
||||
{
|
||||
using var dlg = new Form
|
||||
var (dlg, logging) = BuildAdditionalOptions(ServiceLoggingEnabled);
|
||||
using (dlg)
|
||||
{
|
||||
if (ForegroundDialog.Show(owner => dlg.ShowDialog(owner)) == DialogResult.OK)
|
||||
{
|
||||
ServiceLoggingEnabled = logging.Checked;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>Construction split from ShowDialog so the accessibility audit can inspect this inner
|
||||
/// dialog too. No connect/disconnect cue checkboxes here (removed 2026-07-19, review sweep): the
|
||||
/// headless service NEVER plays cues — nothing in the service host touches CuePlayer, and a
|
||||
/// logged-out session couldn't render them anyway. The cue fields stay on Profile for the app.</summary>
|
||||
internal static (Form Dialog, AccessibleCheckBox Logging) BuildAdditionalOptions(bool loggingEnabled)
|
||||
{
|
||||
var dlg = new Form
|
||||
{
|
||||
Text = "Additional service options",
|
||||
FormBorderStyle = FormBorderStyle.FixedDialog,
|
||||
@@ -345,22 +361,14 @@ internal sealed class ServiceProfileDialog : Form
|
||||
ClientSize = new Size(460, 110),
|
||||
AccessibleName = "Additional service options",
|
||||
};
|
||||
// No connect/disconnect cue checkboxes here (removed 2026-07-19, review sweep): the dialog used
|
||||
// to offer them, but the headless service NEVER plays cues — nothing in the service host touches
|
||||
// CuePlayer, and a logged-out session couldn't render them anyway. Offering a switch that does
|
||||
// nothing is worse than not offering it. The cue fields stay on Profile for the app's own use.
|
||||
var logging = new AccessibleCheckBox { Text = "Enable service &logging (Alt+L)", AccessibleName = "Enable service logging", AutoSize = true, Checked = ServiceLoggingEnabled };
|
||||
var logging = new AccessibleCheckBox { Text = "Enable service &logging (Alt+L)", AccessibleName = "Enable service logging", AutoSize = true, Checked = loggingEnabled };
|
||||
var ok = new Button { Text = "&OK", AutoSize = true, DialogResult = DialogResult.OK };
|
||||
|
||||
var layout = new TableLayoutPanel { Dock = DockStyle.Fill, ColumnCount = 1, Padding = new Padding(12), AutoSize = true };
|
||||
foreach (var c in new Control[] { logging, ok }) { var w = new FlowLayoutPanel { AutoSize = true, Dock = DockStyle.Fill }; w.Controls.Add(c); layout.Controls.Add(w); }
|
||||
dlg.Controls.Add(layout);
|
||||
dlg.AcceptButton = ok;
|
||||
|
||||
if (ForegroundDialog.Show(owner => dlg.ShowDialog(owner)) == DialogResult.OK)
|
||||
{
|
||||
ServiceLoggingEnabled = logging.Checked;
|
||||
}
|
||||
return (dlg, logging);
|
||||
}
|
||||
|
||||
private static Profile CloneProfile(Profile p) =>
|
||||
|
||||
@@ -350,15 +350,15 @@ internal sealed class AsioCaptureBackend : ICaptureBackend
|
||||
if (lCh < recordChannelCount) l += interleaved[srcBase + lCh];
|
||||
if (rCh < recordChannelCount) r += interleaved[srcBase + rCh];
|
||||
}
|
||||
// Soft-limit-ish clamp at the encoder boundary; matches MixingEngine.
|
||||
if (l > 1f) { l = 1f; Interlocked.Increment(ref clippedSampleCount); }
|
||||
else if (l < -1f) { l = -1f; Interlocked.Increment(ref clippedSampleCount); }
|
||||
if (r > 1f) { r = 1f; Interlocked.Increment(ref clippedSampleCount); }
|
||||
else if (r < -1f) { r = -1f; Interlocked.Increment(ref clippedSampleCount); }
|
||||
mixScratch[dstBase] = l;
|
||||
mixScratch[dstBase + 1] = r;
|
||||
}
|
||||
|
||||
// Encoder-boundary clamp — the shared rule (SampleClamp), with ONE batched counter add per
|
||||
// buffer instead of the old up-to-four interlocked increments per frame on this RT thread.
|
||||
var clipped = SampleClamp.ClampBuffer(mixScratch.AsSpan(0, stereoFloats));
|
||||
if (clipped > 0) Interlocked.Add(ref clippedSampleCount, clipped);
|
||||
|
||||
onMixedSamples(new ReadOnlyMemory<float>(mixScratch, 0, stereoFloats));
|
||||
// Capture-thread CPU instrumentation (item 2 of RemSoundefficiency.md). Records
|
||||
// the time the WHOLE callback spent — including the synchronous downstream
|
||||
|
||||
@@ -382,14 +382,8 @@ internal sealed class MixingEngine : ICaptureBackend
|
||||
}
|
||||
|
||||
// Hard-clamp mixed sum to [-1, 1] to prevent encoder clipping when multiple loud
|
||||
// sources sum past unity. Counts clipped samples for diagnostics.
|
||||
long clipped = 0;
|
||||
for (var i = 0; i < read; i++)
|
||||
{
|
||||
var v = mixScratch[i];
|
||||
if (v > 1f) { mixScratch[i] = 1f; clipped++; }
|
||||
else if (v < -1f) { mixScratch[i] = -1f; clipped++; }
|
||||
}
|
||||
// sources sum past unity. Shared rule (SampleClamp); one batched counter add.
|
||||
var clipped = SampleClamp.ClampBuffer(mixScratch.AsSpan(0, read));
|
||||
if (clipped > 0) Interlocked.Add(ref clippedSampleCount, clipped);
|
||||
Interlocked.Increment(ref mixTickCount);
|
||||
|
||||
|
||||
@@ -369,14 +369,10 @@ internal sealed class PushModeWasapiBackend : ICaptureBackend
|
||||
stereo = stereoScratch;
|
||||
}
|
||||
|
||||
// 4. Soft clamp at the encoder boundary (matches MixingEngine / AsioCaptureBackend).
|
||||
// 4. Encoder-boundary clamp — the shared rule (SampleClamp), one batched counter add.
|
||||
var stereoFloatCount = workingFrames * MixChannels;
|
||||
for (var i = 0; i < stereoFloatCount; i++)
|
||||
{
|
||||
var v = stereo[i];
|
||||
if (v > 1f) { stereo[i] = 1f; Interlocked.Increment(ref clippedSampleCount); }
|
||||
else if (v < -1f) { stereo[i] = -1f; Interlocked.Increment(ref clippedSampleCount); }
|
||||
}
|
||||
var clipped = SampleClamp.ClampBuffer(stereo.AsSpan(0, stereoFloatCount));
|
||||
if (clipped > 0) Interlocked.Add(ref clippedSampleCount, clipped);
|
||||
|
||||
// 5. Hand off to the encoder/UDP-send pipeline. Synchronous on the capture thread.
|
||||
onMixedSamples(new ReadOnlyMemory<float>(stereo, 0, stereoFloatCount));
|
||||
|
||||
Reference in New Issue
Block a user