From 2db63a8edd0613e9325a63fb56b7c3107569e818 Mon Sep 17 00:00:00 2001 From: Allamagoosa <36551512+dmatlock171@users.noreply.github.com> Date: Fri, 14 Aug 2026 11:12:41 -0700 Subject: [PATCH] Review #1885: one concurrency setting with a safe default and a numeric control Replaces the MultiThreadEnabled checkbox and the ProcessorCount default with a single MaxConcurrentDownloads value where 1 means serial - the behaviour Libation had before parallel downloads existed. The old pairing could not express 'off' at all: the bool was hardcoded true in the view model constructor and the int setter clamped to a minimum of 2, so there was no way back to one-at-a-time downloads. Collapsing both into one value makes that state unreachable rather than merely fixed, and removes the risk of the two settings disagreeing. Bounds live in Configuration as named constants: minimum 1, default 3, maximum 10. Audible throttles license requests, so the default is deliberately conservative and the cap keeps users from choosing a number that produces license denials instead of speed. WinForms swaps the 'Parallel downloads' checkbox for an 'At once:' spinner that reads its bounds from those constants. --- .../Configuration.PersistentSettings.cs | 24 ++++++++---- .../ProcessQueue/ProcessQueueViewModel.cs | 24 +++++++++--- .../ProcessQueueControl.Designer.cs | 37 ++++++++++++------- .../ProcessQueue/ProcessQueueControl.cs | 9 +++-- 4 files changed, 65 insertions(+), 29 deletions(-) diff --git a/Source/LibationFileManager/Configuration.PersistentSettings.cs b/Source/LibationFileManager/Configuration.PersistentSettings.cs index cffb3817..7fce064b 100644 --- a/Source/LibationFileManager/Configuration.PersistentSettings.cs +++ b/Source/LibationFileManager/Configuration.PersistentSettings.cs @@ -418,16 +418,24 @@ public partial class Configuration [Description("Auto-scroll the download queue to keep active downloads in view.")] public bool AutoScrollQueue { get => GetNonString(defaultValue: true); set => SetNonString(value); } - [Description("Maximum number of books to download and decrypt simultaneously. Defaults to the number of logical processors.")] + /// The lowest can go: one book at a time. + public const int MinConcurrentDownloads = 1; + + /// + /// The highest can go. Audible throttles license + /// requests, so more concurrency stops helping well before this and starts producing + /// license denials instead. + /// + public const int MaxAllowedConcurrentDownloads = 10; + + /// Deliberately conservative. See . + public const int DefaultConcurrentDownloads = 3; + + [Description("Maximum number of books to download and decrypt simultaneously. Set to 1 to download one book at a time.")] public int MaxConcurrentDownloads { - get - { - var value = GetNonString(defaultValue: 0); - // Treat 0 or 1 as "use default" — 1 may have been written by an earlier bug. - return value <= 1 ? Environment.ProcessorCount : value; - } - set => SetNonString(Math.Max(2, value)); + get => Math.Clamp(GetNonString(defaultValue: DefaultConcurrentDownloads), MinConcurrentDownloads, MaxAllowedConcurrentDownloads); + set => SetNonString(Math.Clamp(value, MinConcurrentDownloads, MaxAllowedConcurrentDownloads)); } [Description("Global download speed limit in bytes per second.")] diff --git a/Source/LibationUiBase/ProcessQueue/ProcessQueueViewModel.cs b/Source/LibationUiBase/ProcessQueue/ProcessQueueViewModel.cs index 1984db4f..f149790f 100644 --- a/Source/LibationUiBase/ProcessQueue/ProcessQueueViewModel.cs +++ b/Source/LibationUiBase/ProcessQueue/ProcessQueueViewModel.cs @@ -43,17 +43,31 @@ public class ProcessQueueViewModel : ReactiveObject SpeedLimit = Configuration.Instance.DownloadSpeedLimit / 1024m / 1024; MaxConcurrentDownloads = Configuration.Instance.MaxConcurrentDownloads; AutoScrollQueue = Configuration.Instance.AutoScrollQueue; - MultiThreadEnabled = true; } public int CompletedCount { get => field; private set { RaiseAndSetIfChanged(ref field, value); RaisePropertyChanged(nameof(AnyCompleted)); } } public int QueuedCount { get => field; private set { this.RaiseAndSetIfChanged(ref field, value); RaisePropertyChanged(nameof(AnyQueued)); } } public int ErrorCount { get => field; private set { RaiseAndSetIfChanged(ref field, value); RaisePropertyChanged(nameof(AnyErrors)); } } - public int MaxConcurrentDownloads { get => field; set { RaiseAndSetIfChanged(ref field, value); Configuration.Instance.MaxConcurrentDownloads = value; } } + /// + /// How many books download and decrypt at once. + /// means one at a time, which is how Libation behaved before parallel downloads existed - so this + /// single value is both the limit and the off switch, and the two can never disagree. + /// + public int MaxConcurrentDownloads + { + get => field; + set + { + var clamped = Math.Clamp(value, Configuration.MinConcurrentDownloads, Configuration.MaxAllowedConcurrentDownloads); + RaiseAndSetIfChanged(ref field, clamped); + Configuration.Instance.MaxConcurrentDownloads = clamped; + } + } public bool AutoScrollQueue { get => field; set { RaiseAndSetIfChanged(ref field, value); Configuration.Instance.AutoScrollQueue = value; } } - public bool MultiThreadEnabled { get => field; set => RaiseAndSetIfChanged(ref field, value); } - private int EffectiveConcurrentDownloads => MultiThreadEnabled ? Math.Max(2, MaxConcurrentDownloads) : 1; + /// Exposed so UI controls can bind their spinner bounds rather than hardcoding them. + public static int MinConcurrentDownloads => Configuration.MinConcurrentDownloads; + public static int MaxAllowedConcurrentDownloads => Configuration.MaxAllowedConcurrentDownloads; public string? RunningTime { get => field; set => RaiseAndSetIfChanged(ref field, value); } public bool ProgressBarVisible { get => field; set => RaiseAndSetIfChanged(ref field, value); } public bool AnyCompleted => CompletedCount > 0; @@ -662,7 +676,7 @@ public class ProcessQueueViewModel : ReactiveObject } // If at capacity, wait for a slot to open before trying to dequeue more - if (activeTasks.Count >= EffectiveConcurrentDownloads) + if (activeTasks.Count >= MaxConcurrentDownloads) { await Task.WhenAny(activeTasks); continue; diff --git a/Source/LibationWinForms/ProcessQueue/ProcessQueueControl.Designer.cs b/Source/LibationWinForms/ProcessQueue/ProcessQueueControl.Designer.cs index c72a37ae..788f1305 100644 --- a/Source/LibationWinForms/ProcessQueue/ProcessQueueControl.Designer.cs +++ b/Source/LibationWinForms/ProcessQueue/ProcessQueueControl.Designer.cs @@ -43,7 +43,8 @@ this.virtualFlowControl2 = new LibationWinForms.ProcessQueue.VirtualFlowControl(); this.panel1 = new System.Windows.Forms.Panel(); this.autoScrollChk = new System.Windows.Forms.CheckBox(); - this.multiThreadChk = new System.Windows.Forms.CheckBox(); + this.concurrencyLbl = new System.Windows.Forms.Label(); + this.concurrencyNum = new System.Windows.Forms.NumericUpDown(); this.label1 = new System.Windows.Forms.Label(); this.numericUpDown1 = new LibationWinForms.ProcessQueue.NumericUpDownSuffix(); this.btnCleanFinished = new System.Windows.Forms.Button(); @@ -61,6 +62,7 @@ this.tabPage1.SuspendLayout(); this.panel1.SuspendLayout(); ((System.ComponentModel.ISupportInitialize)(this.numericUpDown1)).BeginInit(); + ((System.ComponentModel.ISupportInitialize)(this.concurrencyNum)).BeginInit(); this.tabPage2.SuspendLayout(); ((System.ComponentModel.ISupportInitialize)(this.logDGV)).BeginInit(); this.panel2.SuspendLayout(); @@ -160,7 +162,8 @@ this.panel1.BackColor = System.Drawing.SystemColors.Control; this.panel1.BorderStyle = System.Windows.Forms.BorderStyle.FixedSingle; this.panel1.Controls.Add(this.autoScrollChk); - this.panel1.Controls.Add(this.multiThreadChk); + this.panel1.Controls.Add(this.concurrencyLbl); + this.panel1.Controls.Add(this.concurrencyNum); this.panel1.Controls.Add(this.label1); this.panel1.Controls.Add(this.numericUpDown1); this.panel1.Controls.Add(this.btnCleanFinished); @@ -183,17 +186,23 @@ this.autoScrollChk.Text = "Auto-scroll"; this.autoScrollChk.UseVisualStyleBackColor = true; // - // multiThreadChk + // concurrencyLbl // - this.multiThreadChk.Anchor = System.Windows.Forms.AnchorStyles.Left | System.Windows.Forms.AnchorStyles.Top; - this.multiThreadChk.AutoSize = true; - this.multiThreadChk.Checked = true; - this.multiThreadChk.CheckState = System.Windows.Forms.CheckState.Checked; - this.multiThreadChk.Location = new System.Drawing.Point(82, 28); - this.multiThreadChk.Name = "multiThreadChk"; - this.multiThreadChk.TabIndex = 7; - this.multiThreadChk.Text = "Parallel downloads"; - this.multiThreadChk.UseVisualStyleBackColor = true; + this.concurrencyLbl.Anchor = System.Windows.Forms.AnchorStyles.Left | System.Windows.Forms.AnchorStyles.Top; + this.concurrencyLbl.AutoSize = true; + this.concurrencyLbl.Location = new System.Drawing.Point(82, 30); + this.concurrencyLbl.Name = "concurrencyLbl"; + this.concurrencyLbl.Size = new System.Drawing.Size(54, 15); + this.concurrencyLbl.TabIndex = 7; + this.concurrencyLbl.Text = "At once:"; + // + // concurrencyNum + // + this.concurrencyNum.Anchor = System.Windows.Forms.AnchorStyles.Left | System.Windows.Forms.AnchorStyles.Top; + this.concurrencyNum.Location = new System.Drawing.Point(136, 27); + this.concurrencyNum.Name = "concurrencyNum"; + this.concurrencyNum.Size = new System.Drawing.Size(45, 23); + this.concurrencyNum.TabIndex = 8; // // label1 // @@ -361,6 +370,7 @@ this.panel1.ResumeLayout(false); this.panel1.PerformLayout(); ((System.ComponentModel.ISupportInitialize)(this.numericUpDown1)).EndInit(); + ((System.ComponentModel.ISupportInitialize)(this.concurrencyNum)).EndInit(); this.tabPage2.ResumeLayout(false); ((System.ComponentModel.ISupportInitialize)(this.logDGV)).EndInit(); this.panel2.ResumeLayout(false); @@ -393,6 +403,7 @@ private NumericUpDownSuffix numericUpDown1; private System.Windows.Forms.Label label1; private System.Windows.Forms.CheckBox autoScrollChk; - private System.Windows.Forms.CheckBox multiThreadChk; + private System.Windows.Forms.Label concurrencyLbl; + private System.Windows.Forms.NumericUpDown concurrencyNum; } } diff --git a/Source/LibationWinForms/ProcessQueue/ProcessQueueControl.cs b/Source/LibationWinForms/ProcessQueue/ProcessQueueControl.cs index 63ad0be1..0b717190 100644 --- a/Source/LibationWinForms/ProcessQueue/ProcessQueueControl.cs +++ b/Source/LibationWinForms/ProcessQueue/ProcessQueueControl.cs @@ -39,7 +39,10 @@ internal partial class ProcessQueueControl : UserControl ViewModel.LogEntries.CollectionChanged += LogEntries_CollectionChanged; ViewModel.ProcessStart += Book_ProcessStart; autoScrollChk.CheckedChanged += (s, e) => ViewModel.AutoScrollQueue = autoScrollChk.Checked; - multiThreadChk.CheckedChanged += (s, e) => ViewModel.MultiThreadEnabled = multiThreadChk.Checked; + + concurrencyNum.Minimum = ProcessQueueViewModel.MinConcurrentDownloads; + concurrencyNum.Maximum = ProcessQueueViewModel.MaxAllowedConcurrentDownloads; + concurrencyNum.ValueChanged += (s, e) => ViewModel.MaxConcurrentDownloads = (int)concurrencyNum.Value; ProcessQueue_PropertyChanged(this, new PropertyChangedEventArgs(null)); } @@ -131,8 +134,8 @@ internal partial class ProcessQueueControl : UserControl } if (e.PropertyName is null or nameof(ViewModel.AutoScrollQueue)) autoScrollChk.Checked = ViewModel.AutoScrollQueue; - if (e.PropertyName is null or nameof(ViewModel.MultiThreadEnabled)) - multiThreadChk.Checked = ViewModel.MultiThreadEnabled; + if (e.PropertyName is null or nameof(ViewModel.MaxConcurrentDownloads)) + concurrencyNum.Value = ViewModel.MaxConcurrentDownloads; } ///