From 3cda9a7f26d58d759f523d6aa906bfb3955fed59 Mon Sep 17 00:00:00 2001 From: Kentaro Hakase Date: Wed, 12 Aug 2026 00:37:49 +0200 Subject: [PATCH 1/3] Make the long running steps cancellable and stop progress ticks from stalling the UI Adding files never raised IsBusy, so while a dropped folder was analyzed Cancel stayed disabled, Start stayed enabled and further drops were accepted. The analysis now runs as a busy phase with its own cancellation source, and so does the processed preview render, which is a full FFmpeg pass and was equally unstoppable. Both report a cancellation as a normal outcome instead of letting it escape into the drop handler, which has no guard of its own. The two startup tasks were fire and forget. They are held now and cancelled on shutdown, and they check the token before writing view model state again, so a late answer can no longer raise change notifications into a torn down view. Their catch-all no longer hides a genuine defect: a cancellation stays silent, anything else is logged. Waiting for them in Dispose would deadlock the thread that has to run their continuations, so only the cancellation is issued. Finally, every property change of every queue item rebuilt the summary and re-raised all 24 command states. Progress is written on every FFmpeg tick and feeds neither, so the refresh is now limited to the properties that do. --- ViewModels/MainViewModel.Analysis.cs | 24 +++++++++++++++++-- ViewModels/MainViewModel.Batch.cs | 33 +++++++++++++++++++++++--- ViewModels/MainViewModel.Download.cs | 16 +++++++++---- ViewModels/MainViewModel.Preview.cs | 16 ++++++++++++- ViewModels/MainViewModel.Processing.cs | 2 ++ ViewModels/MainViewModel.Update.cs | 23 ++++++++++++++---- ViewModels/MainViewModel.cs | 18 ++++++++++++-- 7 files changed, 115 insertions(+), 17 deletions(-) diff --git a/ViewModels/MainViewModel.Analysis.cs b/ViewModels/MainViewModel.Analysis.cs index 09e65fe..4db2dac 100644 --- a/ViewModels/MainViewModel.Analysis.cs +++ b/ViewModels/MainViewModel.Analysis.cs @@ -63,9 +63,29 @@ public async Task LoadInputFilesAsync(IEnumerable paths) } _logService.Info(LocalizationService.Instance.Format("Log_BatchAddedFilesFormat", addResult.AddedItems.Count)); - await AnalyzeBatchItemsAsync(addResult.AddedItems, CancellationToken.None); - SetStatus("Status_BatchReadyFormat", _batchQueueService.GetProcessableItems(BatchItems).Count); + // The analysis of a dropped folder can take a while, so it runs as a busy phase: + // that keeps Cancel enabled, Start disabled and further drops out until it ends. + _analysisCancellation?.Dispose(); + _analysisCancellation = new CancellationTokenSource(); + IsBusy = true; + + try + { + await AnalyzeBatchItemsAsync(addResult.AddedItems, _analysisCancellation.Token); + SetStatus("Status_BatchReadyFormat", _batchQueueService.GetProcessableItems(BatchItems).Count); + } + catch (OperationCanceledException) + { + SetProcessingPhase("Phase_Cancel"); + SetStatus("Error_AnalysisCancelled"); + } + finally + { + IsBusy = false; + _analysisCancellation.Dispose(); + _analysisCancellation = null; + } } private async Task SelectFileAsync() diff --git a/ViewModels/MainViewModel.Batch.cs b/ViewModels/MainViewModel.Batch.cs index 26725e4..99cfb06 100644 --- a/ViewModels/MainViewModel.Batch.cs +++ b/ViewModels/MainViewModel.Batch.cs @@ -117,14 +117,24 @@ private async Task PrepareRetryItemsAsync(IReadOnlyList item.AudioInfo is null).ToArray(); if (needsAnalysis.Length > 0) { + _analysisCancellation?.Dispose(); + _analysisCancellation = new CancellationTokenSource(); IsBusy = true; try { - await AnalyzeBatchItemsAsync(needsAnalysis, CancellationToken.None); + await AnalyzeBatchItemsAsync(needsAnalysis, _analysisCancellation.Token); + } + catch (OperationCanceledException) + { + SetProcessingPhase("Phase_Cancel"); + SetStatus("Error_AnalysisCancelled"); + return 0; } finally { IsBusy = false; + _analysisCancellation.Dispose(); + _analysisCancellation = null; } } @@ -212,8 +222,11 @@ private void OnBatchItemPropertyChanged(object? sender, PropertyChangedEventArgs } } - UpdateBatchSummary(); - RaiseCommandStates(); + if (BatchQueueStateChanged(e.PropertyName)) + { + UpdateBatchSummary(); + RaiseCommandStates(); + } if (ReferenceEquals(sender, SelectedBatchItem)) { @@ -235,6 +248,20 @@ private static bool BatchViewNeedsRefresh(string? propertyName) nameof(BatchProcessingItem.ComparisonReport); } + // Progress is written on every FFmpeg progress tick of every running item. It feeds + // neither the summary counts (they only look at Status and ComparisonReport) nor any + // command predicate, so it must not trigger the expensive refresh: RaiseCommandStates + // makes WPF re-evaluate 24 predicates on the UI thread, several of which touch the + // file system or build a filter plan. + private static bool BatchQueueStateChanged(string? propertyName) + { + return propertyName is null or + nameof(BatchProcessingItem.Status) or + nameof(BatchProcessingItem.HasComparisonWarnings) or + nameof(BatchProcessingItem.ComparisonReport) or + nameof(BatchProcessingItem.OutputPath); + } + private int GetVisibleIndex(BatchProcessingItem item) { var visibleItems = BatchItemsView.Cast().ToArray(); diff --git a/ViewModels/MainViewModel.Download.cs b/ViewModels/MainViewModel.Download.cs index dbefea4..308db57 100644 --- a/ViewModels/MainViewModel.Download.cs +++ b/ViewModels/MainViewModel.Download.cs @@ -84,7 +84,7 @@ private bool CanDownloadFromUrl() return !IsBusy && YtDlpDownloadService.IsLikelyValidUrl(YouTubeUrl); } - private async Task PrepareYtDlpAsync() + private async Task PrepareYtDlpAsync(CancellationToken cancellationToken) { try { @@ -100,16 +100,22 @@ private async Task PrepareYtDlpAsync() _ytDlpAutoUpdate, lastCheck, _logService.Info, - CancellationToken.None); + cancellationToken); - if (newCheck.HasValue) + if (newCheck.HasValue && !cancellationToken.IsCancellationRequested) { _ytDlpLastUpdateCheckUtc = newCheck.Value.ToString("o", CultureInfo.InvariantCulture); } } - catch + catch (OperationCanceledException) { - // Startup preparation of the downloader is best effort and never blocks the app. + // The app is closing. + } + catch (Exception exception) + { + // Startup preparation of the downloader is best effort and never blocks the + // app, but a genuine defect in this path should not stay invisible either. + _logService.Warning($"{exception.GetType().Name}: {exception.Message}"); } } } diff --git a/ViewModels/MainViewModel.Preview.cs b/ViewModels/MainViewModel.Preview.cs index a27efdb..36bd080 100644 --- a/ViewModels/MainViewModel.Preview.cs +++ b/ViewModels/MainViewModel.Preview.cs @@ -32,6 +32,12 @@ private async Task RenderProcessedPreviewAsync() InvalidateProcessedPreview(); _isProcessedPreviewRendering = true; + + // The render is a full FFmpeg pass over the loudest section, so it runs as a busy + // phase like the deep analysis does. That is what makes Cancel reach it at all. + _previewRenderCancellation?.Dispose(); + _previewRenderCancellation = new CancellationTokenSource(); + IsBusy = true; RaiseCommandStates(); try @@ -41,7 +47,7 @@ private async Task RenderProcessedPreviewAsync() options, _logService.Info, null, - CancellationToken.None); + _previewRenderCancellation.Token); if (result.IsFailure || result.Value is null) { @@ -61,9 +67,17 @@ private async Task RenderProcessedPreviewAsync() SetStatus("Status_ProcessedPreviewReadyFormat", Path.GetFileName(_processedPreviewPath)); _logService.Info(LocalizationService.Instance.Format("Log_ProcessedPreviewReadyFormat", _processedPreviewPath)); } + catch (OperationCanceledException) + { + SetProcessingPhase("Phase_Cancel"); + SetStatus("Error_ProcessingCancelled"); + } finally { _isProcessedPreviewRendering = false; + IsBusy = false; + _previewRenderCancellation.Dispose(); + _previewRenderCancellation = null; RaiseCommandStates(); } } diff --git a/ViewModels/MainViewModel.Processing.cs b/ViewModels/MainViewModel.Processing.cs index 89418f2..925e073 100644 --- a/ViewModels/MainViewModel.Processing.cs +++ b/ViewModels/MainViewModel.Processing.cs @@ -136,6 +136,8 @@ private void CancelProcessing() SetProcessingPhase("Phase_Cancel"); _processingCancellation?.Cancel(); _diagnosticsCancellation?.Cancel(); + _analysisCancellation?.Cancel(); + _previewRenderCancellation?.Cancel(); } private async Task ValidateProcessedItemAsync(BatchProcessingItem item, CancellationToken cancellationToken) diff --git a/ViewModels/MainViewModel.Update.cs b/ViewModels/MainViewModel.Update.cs index e331f16..29ece35 100644 --- a/ViewModels/MainViewModel.Update.cs +++ b/ViewModels/MainViewModel.Update.cs @@ -6,7 +6,7 @@ namespace AudioQualityEnhancer.ViewModels; // App update check: notify (and link to the download) when a newer release exists. public sealed partial class MainViewModel { - private async Task CheckForAppUpdateAsync() + private async Task CheckForAppUpdateAsync(CancellationToken cancellationToken) { try { @@ -26,7 +26,16 @@ private async Task CheckForAppUpdateAsync() } var current = System.Reflection.Assembly.GetExecutingAssembly().GetName().Version ?? new Version(0, 0, 0); - var update = await _appUpdateService.CheckAsync(current, CancellationToken.None); + var update = await _appUpdateService.CheckAsync(current, cancellationToken); + + // The window may have closed while the request was in flight; writing view + // model state after that would only raise change notifications into a + // torn-down view. + if (cancellationToken.IsCancellationRequested) + { + return; + } + _appUpdateLastCheckUtc = now.ToString("o", CultureInfo.InvariantCulture); if (update is not null) @@ -36,9 +45,15 @@ private async Task CheckForAppUpdateAsync() IsUpdateAvailable = true; } } - catch + catch (OperationCanceledException) + { + // The app is closing. + } + catch (Exception exception) { - // An update check must never disrupt startup (offline, rate limit, ...). + // An update check must never disrupt startup (offline, rate limit, ...), + // but a genuine defect in this path should not stay invisible either. + _logService.Warning($"{exception.GetType().Name}: {exception.Message}"); } } diff --git a/ViewModels/MainViewModel.cs b/ViewModels/MainViewModel.cs index 87574fb..964992f 100644 --- a/ViewModels/MainViewModel.cs +++ b/ViewModels/MainViewModel.cs @@ -109,6 +109,11 @@ public sealed partial class MainViewModel : INotifyPropertyChanged, IDisposable private bool _syncingSelectedBatchItem; private CancellationTokenSource? _processingCancellation; private CancellationTokenSource? _diagnosticsCancellation; + private CancellationTokenSource? _analysisCancellation; + private CancellationTokenSource? _previewRenderCancellation; + private readonly CancellationTokenSource _shutdownCancellation = new(); + private Task _ytDlpPreparation = Task.CompletedTask; + private Task _appUpdateCheck = Task.CompletedTask; private bool _hasAnalysisWarnings; private bool _updatingPositionFromTimer; @@ -304,8 +309,8 @@ public async Task InitializeAsync() _logService.Warning(_ffprobeStatus.ErrorMessage ?? LocalizationService.Instance["Log_FFprobeUnavailable"]); } - _ = PrepareYtDlpAsync(); - _ = CheckForAppUpdateAsync(); + _ytDlpPreparation = PrepareYtDlpAsync(_shutdownCancellation.Token); + _appUpdateCheck = CheckForAppUpdateAsync(_shutdownCancellation.Token); } private void OnLogAdded(object? sender, string line) @@ -520,6 +525,13 @@ private void OnPropertyChanged([CallerMemberName] string? propertyName = null) public void Dispose() { + // The startup tasks resume on the dispatcher, so waiting for them here would + // deadlock the very thread that has to run their continuations. Cancelling is + // enough: both check the token before they touch view model state again. The + // source itself is not disposed because those tasks may still hold its token. + _shutdownCancellation.Cancel(); + + _logService.LogAdded -= OnLogAdded; _audioPreviewController.Tick -= OnPreviewTimerTick; _audioPreviewController.PlaybackFailed -= OnPlaybackFailed; _audioPreviewController.PlaybackEnded -= OnPlaybackEnded; @@ -527,6 +539,8 @@ public void Dispose() LocalizationService.Instance.PropertyChanged -= OnLocalizationChanged; _processingCancellation?.Dispose(); _diagnosticsCancellation?.Dispose(); + _analysisCancellation?.Dispose(); + _previewRenderCancellation?.Dispose(); _audioPreviewController.Dispose(); InvalidateProcessedPreview(); foreach (var item in BatchItems) From 5b4a1aa7e376540e6bbc25ab66e031daf14da55b Mon Sep 17 00:00:00 2001 From: Kentaro Hakase Date: Wed, 12 Aug 2026 00:37:59 +0200 Subject: [PATCH 2/3] Localize the tool source and stop re-probing a broken tool every time The place a tool was found in was built as a German literal and handed straight to the app bar, so an English user read Benutzer-Tools next to an otherwise translated interface. The location now carries a resource key that is resolved on display. A failed probe was never cached. Since the probe is bounded by a timeout, a missing or unresponsive binary cost the full twenty seconds on every call. The failure is cached as well now, but only for half a minute, so a tool that is installed while the app runs is still picked up. --- Models/ToolStatus.cs | 9 ++++--- Resources/Strings.en.resx | 5 ++++ Resources/Strings.resx | 5 ++++ Services/ToolDiscoveryService.cs | 45 +++++++++++++++++++++++--------- 4 files changed, 48 insertions(+), 16 deletions(-) diff --git a/Models/ToolStatus.cs b/Models/ToolStatus.cs index 53f3c71..0561e31 100644 --- a/Models/ToolStatus.cs +++ b/Models/ToolStatus.cs @@ -7,14 +7,14 @@ public sealed class ToolStatus public ToolStatus( string name, string executablePath, - string source, + string sourceKey, bool isAvailable, string? versionLine, string? errorMessage) { Name = name; ExecutablePath = executablePath; - Source = source; + SourceKey = sourceKey; IsAvailable = isAvailable; VersionLine = versionLine; ErrorMessage = errorMessage; @@ -24,7 +24,8 @@ public ToolStatus( public string ExecutablePath { get; } - public string Source { get; } + /// Resource key of the place the tool was found in, resolved on display. + public string SourceKey { get; } public bool IsAvailable { get; } @@ -33,6 +34,6 @@ public ToolStatus( public string? ErrorMessage { get; } public string DisplayText => IsAvailable - ? LocalizationService.Instance.Format("ToolStatus_Found", Name, Source) + ? LocalizationService.Instance.Format("ToolStatus_Found", Name, LocalizationService.Instance[SourceKey]) : LocalizationService.Instance.Format("ToolStatus_NotFound", Name); } diff --git a/Resources/Strings.en.resx b/Resources/Strings.en.resx index d83fddb..2390bbd 100644 --- a/Resources/Strings.en.resx +++ b/Resources/Strings.en.resx @@ -520,6 +520,11 @@ {0}: found ({1}) {0}: not found + user tools + app folder + tools folder + PATH + Select audio or video file Select output folder Audio and video files diff --git a/Resources/Strings.resx b/Resources/Strings.resx index 55c3383..47d3bec 100644 --- a/Resources/Strings.resx +++ b/Resources/Strings.resx @@ -520,6 +520,11 @@ {0}: gefunden ({1}) {0}: nicht gefunden + Benutzer-Tools + App-Ordner + Tools-Ordner + PATH + Audio- oder Videodatei auswählen Ausgabeordner auswählen Audio- und Videodateien diff --git a/Services/ToolDiscoveryService.cs b/Services/ToolDiscoveryService.cs index 681936b..ac65d9c 100644 --- a/Services/ToolDiscoveryService.cs +++ b/Services/ToolDiscoveryService.cs @@ -10,9 +10,15 @@ public sealed class ToolDiscoveryService // forever, so it is bounded and reported as unavailable instead. internal static readonly TimeSpan ProbeTimeout = TimeSpan.FromSeconds(20); + // A failed probe used to be repeated on every call. Together with the bounded probe + // that costs the full timeout each time, so failures are cached as well - but only + // briefly, so a tool that is installed while the app runs is still picked up. + internal static readonly TimeSpan FailedStatusCacheDuration = TimeSpan.FromSeconds(30); + private readonly object _cacheLock = new(); private readonly Dictionary _locationCache = new(StringComparer.OrdinalIgnoreCase); private readonly Dictionary _statusCache = new(StringComparer.OrdinalIgnoreCase); + private readonly Dictionary _failedStatusCache = new(StringComparer.OrdinalIgnoreCase); public string ResolveExecutable(string toolName) { @@ -30,14 +36,27 @@ public async Task GetStatusAsync(string toolName, CancellationToken { return cachedStatus; } + + if (_failedStatusCache.TryGetValue(toolName, out var cachedFailure) && + string.Equals(cachedFailure.Status.ExecutablePath, location.ExecutablePath, StringComparison.OrdinalIgnoreCase) && + Stopwatch.GetElapsedTime(cachedFailure.Timestamp) < FailedStatusCacheDuration) + { + return cachedFailure.Status; + } } var status = await ProbeToolAsync(toolName, location, versionArgument, cancellationToken); - if (status.IsAvailable) + + lock (_cacheLock) { - lock (_cacheLock) + if (status.IsAvailable) { _statusCache[toolName] = status; + _failedStatusCache.Remove(toolName); + } + else + { + _failedStatusCache[toolName] = new FailedStatus(status, Stopwatch.GetTimestamp()); } } @@ -73,13 +92,13 @@ private static async Task ProbeToolAsync(string toolName, ToolLocati if (process.ExitCode == 0) { - return new ToolStatus(toolName, location.ExecutablePath, location.Source, true, versionLine, null); + return new ToolStatus(toolName, location.ExecutablePath, location.SourceKey, true, versionLine, null); } return new ToolStatus( toolName, location.ExecutablePath, - location.Source, + location.SourceKey, false, versionLine, LocalizationService.Instance.Format("Error_ToolExitCodeFormat", toolName, process.ExitCode)); @@ -90,7 +109,7 @@ private static async Task ProbeToolAsync(string toolName, ToolLocati return new ToolStatus( toolName, location.ExecutablePath, - location.Source, + location.SourceKey, false, null, LocalizationService.Instance.Format("Error_ToolTimeoutFormat", toolName)); @@ -105,7 +124,7 @@ private static async Task ProbeToolAsync(string toolName, ToolLocati return new ToolStatus( toolName, location.ExecutablePath, - location.Source, + location.SourceKey, false, null, LocalizationService.Instance.Format("Error_ToolNotFoundFormat", toolName)); @@ -176,28 +195,28 @@ private static ToolLocation LocateTool(string toolName) var userToolPath = Path.Combine(GetUserToolsDirectory(), exeName); if (File.Exists(userToolPath)) { - return new ToolLocation(userToolPath, "Benutzer-Tools"); + return new ToolLocation(userToolPath, "ToolSource_UserTools"); } var appLocalPath = Path.Combine(AppContext.BaseDirectory, exeName); if (File.Exists(appLocalPath)) { - return new ToolLocation(appLocalPath, "App-Ordner"); + return new ToolLocation(appLocalPath, "ToolSource_AppFolder"); } var toolsPath = Path.Combine(AppContext.BaseDirectory, "Tools", exeName); if (File.Exists(toolsPath)) { - return new ToolLocation(toolsPath, "Tools-Ordner"); + return new ToolLocation(toolsPath, "ToolSource_ToolsFolder"); } var pathTool = FindInPath(exeName); if (pathTool is not null) { - return new ToolLocation(pathTool, "PATH"); + return new ToolLocation(pathTool, "ToolSource_Path"); } - return new ToolLocation(exeName, "PATH"); + return new ToolLocation(exeName, "ToolSource_Path"); } private static string? FindInPath(string exeName) @@ -241,5 +260,7 @@ private static ToolLocation LocateTool(string toolName) return null; } - private sealed record ToolLocation(string ExecutablePath, string Source); + private sealed record ToolLocation(string ExecutablePath, string SourceKey); + + private sealed record FailedStatus(ToolStatus Status, long Timestamp); } From b7f755d394ea3e870f446c18b3a226278010666a Mon Sep 17 00:00:00 2001 From: Kentaro Hakase Date: Wed, 12 Aug 2026 00:38:21 +0200 Subject: [PATCH 3/3] Document the hardening changes in the changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f375f31..0e9750b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,9 @@ - An unexpected error in an asynchronous command (start, download, analysis, retry, preview) is now logged and shown as status instead of shutting the app down; cancelling a running command is treated as a normal outcome. - A log file that cannot be written after a batch run (read-only or full output folder) is reported as a warning instead of ending the session through the crash handler. The exported files are already finished at that point. - The crash handler no longer fails when the crash log itself cannot be written and shows the error without a log path in that case. +- Adding files no longer leaves the window in a half-usable state: while a dropped folder is analyzed, Cancel is available, Start is disabled and further drops are rejected, and the analysis can now actually be stopped. +- The processed preview render can be cancelled as well; it is a full FFmpeg pass and previously had to run to completion. +- The tool source shown in the app bar is translated instead of always appearing in German. ### Changed @@ -18,6 +21,9 @@ - The tool check for FFmpeg, FFprobe and yt-dlp is bounded by a timeout: a binary that never answers is terminated and reported as unavailable instead of blocking the startup check indefinitely. - Reading the remaining tool output after a process exits is bounded as well, so a pipe that a child process keeps open cannot hang a run. - The update check accepts only an https link to the project host from the release API and otherwise falls back to the known releases page, because the link is handed to the shell when it is opened. +- The queue no longer re-evaluates every button and rebuilds the summary on each FFmpeg progress tick, so the window stays responsive during a batch export. +- The startup checks for yt-dlp and for a new release are stopped when the window closes and no longer write into a closed window; an unexpected failure in either is logged instead of being swallowed. +- A tool that is missing or does not answer is remembered for half a minute instead of being probed again on every call, where each attempt cost the full probe timeout. ## 0.17.0 - 2026-06-13