Skip to content

Fix: batch cleanup of temporary audio files on exit - #11

Open
GinForGit wants to merge 1 commit into
qew21:masterfrom
GinForGit:fix/audio-temp-cleanup
Open

Fix: batch cleanup of temporary audio files on exit#11
GinForGit wants to merge 1 commit into
qew21:masterfrom
GinForGit:fix/audio-temp-cleanup

Conversation

@GinForGit

Copy link
Copy Markdown
Contributor

Problem

Previously, temporary audio files were deleted immediately after being downloaded, which could interfere with playback. Additionally, the session cleanup logic in MainWindow_Closing was not actually wired to the Closing event, so temp files were never cleaned up on exit.

Solution

  • Introduce AudioTempFileTracker to track downloaded audio temp files during the session.
  • Files are kept while playback may still need them.
  • On application exit, all tracked files are deleted in a batch via CleanupAll().
  • Fix MainWindow_Closing event subscription so cleanup actually runs.
  • Add defensive defaults for missing config values (Input, Output, Game, Size) discovered while testing the audio flow.

Changes

  • GI-Subtitles/Services/Audio/AudioTempFileTracker.cs (new)
  • GI-Subtitles/Views/MainWindow.xaml.cs
  • GI-Subtitles/Views/SettingsWindow.xaml.cs
  • GI-Test/AudioTempFileTrackerTests.cs (new)
  • .gitignore

Testing

  • Added 7 unit tests for AudioTempFileTracker; all pass.
  • Manually verified that the application starts and that a temp audio file created during playback is removed after normal exit.

@GinForGit
GinForGit force-pushed the fix/audio-temp-cleanup branch 2 times, most recently from 41c7163 to f7eac35 Compare July 11, 2026 10:51
@qew21

qew21 commented Jul 12, 2026

Copy link
Copy Markdown
Owner

Thank you for your submission.
After reassessing the actual disk footprint of temporary audio files, we found that roughly 1,00 such files take up less than 3 MB on average. There is therefore no disk pressure that requires real-time cleanup or immediate cleanup upon application exit.

The current master branch already includes a cleanup mechanism for historical audio files on startup. This approach also covers edge cases such as crashes and forced terminations where the Closing event cannot be triggered, and works sufficiently well as a low-frequency fallback solution. By contrast, this PR introduces the AudioTempFileTracker class, locking logic, PlaybackStopped callbacks, and bulk cleanup on exit — the added state management and lifecycle complexity outweigh the actual benefits.

The current implementation also carries additional risks:

  • The PlaybackStopped callback cleans up resources via shared waveOut and mediaReader fields, which means delayed callbacks from old player instances may incorrectly release resources belonging to new player instances.
  • There is no guarantee that the current reader has been released before exit cleanup, so actively playing files may fail to be deleted.
  • If the tracker fails to delete files, it still clears its record, preventing subsequent retry attempts.
  • The duplicate request issue (OpenRead followed by DownloadFile) remains unresolved.
  • Default configuration changes and excessive debug logging are tangential to the core purpose of temporary audio file cleanup in this PR.

I recommend not merging this session-level file tracking solution, and retaining the existing startup fallback cleanup in the master branch instead. If further improvements to the audio logic are needed, a separate, smaller PR can be created to address only the following issues:

  • Replace the dual OpenRead + DownloadFile pattern with a single download request
  • Ensure proper disposal of MediaFoundationReader and WaveOutEvent when switching audio tracks and closing windows
  • Delete incomplete temporary files created when downloads or initialization fail

Given the typical file size and growth rate of these temporary audio files, this streamlined implementation will be simpler and better aligned with real-world requirements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants