diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 7bd83bd..97de43e 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -26,10 +26,6 @@ jobs: run: | pip install pytest - # mkvmerge + mediainfo are AudioCleaner's own runtime dependencies - # (bundled with the Windows build). Installing them here lets the - # integration suite (tests/test_integration_fixtures.py) exercise - # the real tools instead of only mocks. - name: Install mkvtoolnix & mediainfo run: | sudo apt-get update @@ -48,12 +44,8 @@ jobs: pytest tests/ -v windows-build: - name: PyInstaller build test (Windows) + name: PyInstaller build & launch smoke test (Windows) runs-on: windows-latest - # Build-only smoke test: confirms the app still packages into a - # working .exe with PySide6 + the bundled MediaInfo/libcurl binaries. - # It does not run the packaged exe (no display/media tools available - # on the runner) -- that's still a manual release-checklist step. steps: - uses: actions/checkout@v4 @@ -79,3 +71,22 @@ jobs: exit 1 } Write-Host "Build OK: $((Get-Item dist/AudioCleaner.exe).Length) bytes" + + - name: Launch executable smoke test + shell: pwsh + run: | + $exe = (Resolve-Path "dist/AudioCleaner.exe").Path + $proc = Start-Process -FilePath $exe -PassThru + try { + Start-Sleep -Seconds 8 + if ($proc.HasExited) { + Write-Error "AudioCleaner.exe exited during the launch smoke test with code $($proc.ExitCode)" + exit 1 + } + Write-Host "Launch OK: process remained running for 8 seconds" + } + finally { + if (-not $proc.HasExited) { + Stop-Process -Id $proc.Id -Force -ErrorAction SilentlyContinue + } + } diff --git a/audiocleaner/config.py b/audiocleaner/config.py index 8cf183c..40170ef 100644 --- a/audiocleaner/config.py +++ b/audiocleaner/config.py @@ -7,10 +7,11 @@ # --- Cache / rules versioning --- # Bump SCANNER_VERSION whenever probe.py's extraction logic changes in a way # that could change what a cached FileProbeResult contains (new fields, new -# matching logic, etc). Bump RULES_VERSION whenever track-selection logic -# changes (codec_rank.py) in a way that could change *decisions* made from -# an unchanged probe result. Either bump invalidates old cache entries, so a -# stale cache never silently keeps applying pre-change decisions. +# matching logic, etc). This is the version ProbeCache uses to invalidate +# stale probe data. RULES_VERSION identifies track-selection rules and can be +# recorded by callers/history when decision semantics change; changing it does +# not invalidate probe data because codec-ranking changes do not alter the +# underlying MediaInfo/mkvmerge facts stored in the cache. SCANNER_VERSION = "2" RULES_VERSION = "2" @@ -30,22 +31,21 @@ # Ordered best -> worst. Keys are internal codec identifiers produced by # codec_rank.classify_audio_track(). Lower index = higher priority. CODEC_PRIORITY = [ - "truehd_atmos", # Dolby TrueHD + Atmos - "dtsx", # DTS:X - "truehd", # Dolby TrueHD - "dts_hd_ma", # DTS-HD Master Audio - "lpcm", # LPCM - "flac", # FLAC - "eac3", # Dolby Digital Plus (E-AC3) - "dts", # DTS (core) - "ac3", # Dolby Digital (AC3) - "aac", # AAC - "mp3", # MP3 + "truehd_atmos", + "dtsx", + "truehd", + "dts_hd_ma", + "lpcm", + "flac", + "eac3", + "dts", + "ac3", + "aac", + "mp3", ] CODEC_RANK = {codec: i for i, codec in enumerate(CODEC_PRIORITY)} -# Human-readable labels for logs / UI. CODEC_LABELS = { "truehd_atmos": "Dolby TrueHD + Atmos", "dtsx": "DTS:X", @@ -61,41 +61,15 @@ "unknown": "Unknown", } -# English language tags as they appear in MKV/mkvmerge/mediainfo output. ENGLISH_LANG_CODES = {"eng", "en", "en-us", "en-gb"} -# --- Commentary tracks --- -# Off by default: commentary tracks are treated like any other extra audio -# track and stripped. When on, the single best non-commentary track is -# still chosen as "the" kept track, but commentary track(s) are kept -# alongside it rather than being removed. DEFAULT_KEEP_COMMENTARY = False - -# --- Subtitle language filtering --- -# Off by default, so existing behaviour (subtitles are never touched) is -# unchanged unless a user explicitly opts in. When on, only subtitle -# tracks matching a language in DEFAULT_SUBTITLE_LANGUAGES are kept, -# *plus* any track flagged Forced regardless of language (these are -# usually foreign-dialogue captions within an otherwise-English film that -# people expect kept even when everything else in that language is -# stripped). DEFAULT_SUBTITLE_FILTER_ENABLED = False DEFAULT_SUBTITLE_LANGUAGES = {"eng"} -# Cache file name written into the target root folder. CACHE_FILENAME = ".audiocleaner_cache.json" - -# Log file name written into the target root folder. LOG_FILENAME = "audiocleaner_log.txt" - MKV_EXTENSIONS = {".mkv"} -# --- Watch mode --- -# How often the watcher re-scans the folder tree for new/changed files. WATCH_POLL_INTERVAL_SECONDS = 15 - -# How long a file's size must stay unchanged before it's considered fully -# copied and safe to process. User-adjustable in the GUI; this is just the -# default (matches the "wait a minute or two" guidance for Radarr/Sonarr -# moving files into the library folder). WATCH_DEFAULT_SETTLE_SECONDS = 120 diff --git a/audiocleaner/processor.py b/audiocleaner/processor.py index d29b7c0..6c2ea32 100644 --- a/audiocleaner/processor.py +++ b/audiocleaner/processor.py @@ -7,6 +7,7 @@ import os import shutil import subprocess +from collections import Counter from dataclasses import dataclass, field from pathlib import Path from typing import Optional @@ -17,21 +18,8 @@ explain_audio_selection, explain_subtitle_selection, ) -# Tolerance for container-level duration drift between input and output -# (remuxing can shift the reported duration by a tiny amount even with no -# content change). Anything beyond this is treated as a real mismatch. _DURATION_TOLERANCE_SECONDS = 2.0 - -# Suppress console window creation for subprocess calls in a windowed -# (console=False) build -- otherwise Windows pops a new console per call. _NO_WINDOW = subprocess.CREATE_NO_WINDOW if os.name == "nt" else 0 - -# PROCESS_MODE_BACKGROUND_BEGIN: tells Windows this process is doing -# background work, which lowers its CPU, memory, *and* I/O priority -# together (stronger than just CPU priority - it's specifically what -# stops a big disk-heavy job like this from starving other programs, -# e.g. Plex, that are reading from the same physical drive at the -# same time). No effect on non-Windows platforms. _BACKGROUND_MODE = 0x00100000 if os.name == "nt" else 0 _REMUX_CREATE_FLAGS = _NO_WINDOW | _BACKGROUND_MODE @@ -48,25 +36,21 @@ class BeforeAfterInfo: @dataclass class ProcessResult: path: str - # "cleaned" | "skipped_single_track" | "no_english" | "unknown_codec" | "error" status: str kept_codec: Optional[str] = None - removed_track_count: int = 0 # audio tracks removed + removed_track_count: int = 0 removed_subtitle_count: int = 0 bytes_saved: int = 0 message: str = "" - preview: bool = False # True if this describes what *would* happen; nothing was written - restored_from_backup: bool = False # True if Max Safety Mode had to roll back - # Per-track KEEP/REMOVE explanations (spec sec 15). Empty when the file - # was skipped before track selection ran (error / no audio at all). - audio_decisions: list = field(default_factory=list) # list[AudioTrackDecision] - subtitle_decisions: list = field(default_factory=list) # list[SubtitleTrackDecision], only when filtering is on + preview: bool = False + restored_from_backup: bool = False + audio_decisions: list = field(default_factory=list) + subtitle_decisions: list = field(default_factory=list) before: Optional[BeforeAfterInfo] = None after: Optional[BeforeAfterInfo] = None def _atomic_replace(temp_path: Path, original_path: Path): - # os.replace is atomic on the same filesystem on both Windows and POSIX. os.replace(temp_path, original_path) @@ -95,11 +79,6 @@ def process_file( return ProcessResult(path=str(path), status="no_english", message="No English audio track found; file skipped.") - # Safety principle: if AudioCleaner can't confidently classify the - # audio it would keep, it must not touch the file. An "unknown" codec - # here means every candidate English track (or the only one available) - # didn't match any known codec signature -- guessing at how to rank or - # verify it is exactly the unsafe behaviour this app must avoid. if codec_key == "unknown": return ProcessResult( path=str(path), status="unknown_codec", @@ -110,21 +89,22 @@ def process_file( keep_audio_ids = [best_track.track_id] + [t.track_id for t in extra_audio_tracks] original_audio_ids = {t.track_id for t in result.audio_tracks} audio_unchanged = original_audio_ids == set(keep_audio_ids) - + expected_audio_tracks = [best_track] + list(extra_audio_tracks) audio_decisions = explain_audio_selection(result, keep_commentary=keep_commentary) - # Subtitle selection is only evaluated when the feature is switched on; - # otherwise subtitles are left completely alone, same as before this - # feature existed. subtitle_ids_to_keep = None subtitle_unchanged = True subtitle_decisions = [] if subtitle_filter_enabled and result.subtitle_tracks: kept_subs = select_subtitle_tracks_to_keep(result, subtitle_languages) subtitle_ids_to_keep = [t.track_id for t in kept_subs] - original_sub_ids = {t.track_id for t in result.subtitle_tracks} - subtitle_unchanged = original_sub_ids == set(subtitle_ids_to_keep) + subtitle_unchanged = {t.track_id for t in result.subtitle_tracks} == set(subtitle_ids_to_keep) subtitle_decisions = explain_subtitle_selection(result, subtitle_languages) + kept_sub_ids = set(subtitle_ids_to_keep or []) + expected_subtitle_tracks = ( + list(result.subtitle_tracks) if subtitle_ids_to_keep is None + else [t for t in result.subtitle_tracks if t.track_id in kept_sub_ids] + ) before = BeforeAfterInfo( audio_tracks=len(result.audio_tracks), @@ -146,18 +126,12 @@ def process_file( ) if preview_only: - # Everything above (probing, track selection) is identical to a - # real run; this is exactly what would happen. Stop here, before - # any temp file, mkvmerge call, or disk write -- nothing on disk - # is touched in preview mode. bytes_saved/after.size_bytes are - # left at 0/unknown since actual output size is only knowable by - # actually remuxing and measuring the result. preview_after = BeforeAfterInfo( audio_tracks=len(keep_audio_ids), video_tracks=len(result.video_tracks), subtitle_tracks=(len(subtitle_ids_to_keep) if subtitle_ids_to_keep is not None else len(result.subtitle_tracks)), - size_bytes=0, # unknowable without actually remuxing + size_bytes=0, ) return ProcessResult( path=str(path), status="cleaned", kept_codec=codec_key, @@ -168,11 +142,9 @@ def process_file( before=before, after=preview_after, ) - original_size = path.stat().st_size temp_path = path.with_name(path.stem + ".ac_tmp" + path.suffix) - # Clean up any stale temp file from a previous crashed run. if temp_path.exists(): try: temp_path.unlink() @@ -180,6 +152,12 @@ def process_file( pass mkvmerge_path = find_tool("mkvmerge") + if mkvmerge_path is None: + try: + find_tool.cache_clear() + except AttributeError: + pass + mkvmerge_path = find_tool("mkvmerge") if mkvmerge_path is None: return ProcessResult(path=str(path), status="error", message="mkvmerge not found (checked bundled copy and PATH).") @@ -193,9 +171,6 @@ def process_file( if subtitle_ids_to_keep: cmd += ["--subtitle-tracks", ",".join(str(i) for i in subtitle_ids_to_keep)] else: - # Filtering is on and nothing matched (no kept language, no - # forced tracks) -- drop subtitles entirely rather than silently - # keeping everything, which would defeat the point of the filter. cmd += ["--no-subtitles"] cmd.append(str(path)) @@ -209,6 +184,10 @@ def process_file( creationflags=_REMUX_CREATE_FLAGS, ) except FileNotFoundError: + try: + find_tool.cache_clear() + except AttributeError: + pass return ProcessResult(path=str(path), status="error", message="mkvmerge not found on PATH.") except subprocess.TimeoutExpired: @@ -217,7 +196,6 @@ def process_file( message="mkvmerge timed out (file may be very large or hung).") if proc.returncode == 2 or not temp_path.exists(): - # mkvmerge exit code 2 = error; 0 = success; 1 = warnings (still usable). _cleanup_temp(temp_path) stderr_msg = (proc.stderr or "").strip() stdout_msg = (proc.stdout or "").strip() @@ -225,15 +203,12 @@ def process_file( message=f"mkvmerge failed: {stderr_msg or stdout_msg or '(no output from mkvmerge)'}", audio_decisions=audio_decisions, subtitle_decisions=subtitle_decisions, before=before) - # --- Verification (pre-replacement): compare temp output against the - # original probe, not just against expected counts, so we catch e.g. a - # wrong track surviving in place of the right one, or a video/chapter/ - # attachment side-effect of a bad mkvmerge run. --- ok, verify_msg = _verify_output( - temp_path, result, best_track, - expected_audio_ids=keep_audio_ids, - expected_subtitle_count=len(subtitle_ids_to_keep) if subtitle_ids_to_keep is not None else None, + temp_path, result, + expected_audio_tracks=expected_audio_tracks, + expected_subtitle_tracks=expected_subtitle_tracks, subtitles_untouched=not (subtitle_filter_enabled and result.subtitle_tracks), + source_size=original_size, ) if not ok: _cleanup_temp(temp_path) @@ -247,10 +222,13 @@ def process_file( if max_safety_mode: backup_path = path.with_name(path.stem + ".ac_backup" + path.suffix) if backup_path.exists(): - try: - backup_path.unlink() - except OSError: - pass + _cleanup_temp(temp_path) + return ProcessResult( + path=str(path), status="error", + message=f"Maximum Safety Mode backup already exists: {backup_path}. " + "Refusing to overwrite the existing recovery copy.", + audio_decisions=audio_decisions, subtitle_decisions=subtitle_decisions, before=before, + ) try: shutil.copy2(path, backup_path) except OSError as e: @@ -263,27 +241,29 @@ def process_file( _atomic_replace(temp_path, path) except OSError as e: _cleanup_temp(temp_path) - if backup_path: + if backup_path and backup_path.exists() and not persistent_backup: _cleanup_temp(backup_path) return ProcessResult(path=str(path), status="error", message=f"Could not replace original file: {e}") - # --- Final post-replacement verification (ยง5): never assume - # os.replace() succeeding means the job is actually done -- reopen the - # file that's now sitting where the original was and probe it fresh. - final_check = probe_file(path, cache=None) - final_ok = ( - final_check.error is None - and len(final_check.audio_tracks) == len(keep_audio_ids) + final_ok, final_msg = _verify_output( + path, result, + expected_audio_tracks=expected_audio_tracks, + expected_subtitle_tracks=expected_subtitle_tracks, + subtitles_untouched=not (subtitle_filter_enabled and result.subtitle_tracks), + source_size=original_size, ) if not final_ok: if backup_path is not None and backup_path.exists(): try: - os.replace(backup_path, path) + shutil.copy2(backup_path, path) + if not persistent_backup: + _cleanup_temp(backup_path) return ProcessResult( path=str(path), status="error", restored_from_backup=True, message="Final verification failed after replacement; " - "original restored from Maximum Safety Mode backup.", + "original restored from Maximum Safety Mode backup. " + f"Verification detail: {final_msg}", ) except OSError as e: return ProcessResult( @@ -292,25 +272,21 @@ def process_file( f"backup could not be restored automatically: {e}. " f"Backup is at {backup_path}", ) - # No backup available (Maximum Safety Mode was off): the file at - # `path` is whatever survived the replace. We can't undo it, but we - # must not report success. return ProcessResult( path=str(path), status="error", message="Final verification failed after replacement. " "The replaced file may be inconsistent -- check it manually. " - "Enable Maximum Safety Mode to allow automatic recovery here.", + "Enable Maximum Safety Mode to allow automatic recovery here. " + f"Verification detail: {final_msg}", ) - if backup_path is not None and backup_path.exists() and not persistent_backup: - try: - backup_path.unlink() - except OSError: - pass # non-fatal: a leftover backup is safe, just extra disk usage - + final_check = probe_file(path, cache=None) if cache is not None: cache.put(final_check) + if backup_path is not None and backup_path.exists() and not persistent_backup: + _cleanup_temp(backup_path) + after = BeforeAfterInfo( audio_tracks=len(final_check.audio_tracks), video_tracks=len(final_check.video_tracks), @@ -336,88 +312,84 @@ def _cleanup_temp(temp_path: Path): pass +def _audio_signature(track) -> tuple: + return ( + (track.language or "").lower(), + (track.codec_id or "").upper(), + int(track.channels or 0), + ) + + +def _video_signature(track) -> tuple: + return ( + (track.codec_id or "").upper(), + int(track.width or 0), + int(track.height or 0), + ) + + +def _subtitle_signature(track) -> tuple: + return ( + (track.language or "").lower(), + (track.codec_id or "").upper(), + bool(track.forced), + bool(track.default), + ) + + def _verify_output( - temp_path: Path, + output_path: Path, source: FileProbeResult, - kept_track, - expected_audio_ids: list, - expected_subtitle_count: Optional[int], + expected_audio_tracks: list, + expected_subtitle_tracks: list, subtitles_untouched: bool, + source_size: int, ) -> tuple[bool, str]: - """Sanity-check the remuxed file before we let it replace the original. - - Compares the temp output against the *source* probe (not just bare - counts), covering video/audio/subtitle/chapter/attachment/duration/size - per the spec's verification requirements. Anything unexpected here means - the temp file is discarded and the original is never touched.""" + """Validate the exact artifact that is about to be or has been installed.""" try: - check = probe_file(temp_path, cache=None) - except ExternalToolError as e: + check = probe_file(output_path, cache=None) + except (ExternalToolError, OSError) as e: return False, str(e) if check.error: return False, check.error - # --- File size sanity --- - out_size = temp_path.stat().st_size + try: + out_size = output_path.stat().st_size + except OSError as e: + return False, f"could not stat output: {e}" if out_size <= 0: return False, "output file is empty" - src_size = Path(source.path).stat().st_size if Path(source.path).exists() else None - if src_size and out_size < src_size * 0.05: - # A remux that only removes some audio/subtitle streams should - # never shrink a file to a sliver of its original size -- that - # pattern indicates something went badly wrong (e.g. video got - # dropped), not a legitimate space saving. + if source_size and out_size < source_size * 0.05: return False, (f"output ({out_size} bytes) is implausibly small compared to " - f"source ({src_size} bytes) for an audio/subtitle-only change") + f"source ({source_size} bytes) for an audio/subtitle-only change") - # --- Video: must be completely unchanged (this app never touches video) --- if len(check.video_tracks) != len(source.video_tracks): return False, f"expected {len(source.video_tracks)} video track(s), found {len(check.video_tracks)}" - for src_v, out_v in zip(source.video_tracks, check.video_tracks): - if src_v.codec_id != out_v.codec_id or (src_v.width, src_v.height) != (out_v.width, out_v.height): - return False, "video track codec/resolution changed unexpectedly" - - # --- Audio: exact track count, and every kept track's key properties - # (language, codec, channels) must still be present in the output --- - if len(check.audio_tracks) != len(expected_audio_ids): - return False, f"expected {len(expected_audio_ids)} audio track(s) in output, found {len(check.audio_tracks)}" - - kept_langs = {t.language for t in check.audio_tracks} - if kept_track.language not in kept_langs: - return False, "kept track language mismatch after remux" - - out_codec_ids = {t.codec_id for t in check.audio_tracks} - if kept_track.codec_id and kept_track.codec_id not in out_codec_ids: - return False, "kept track codec changed unexpectedly after remux" - - out_channel_counts = {t.channels for t in check.audio_tracks} - if kept_track.channels and kept_track.channels not in out_channel_counts: - return False, "kept track channel count changed unexpectedly after remux" - - # --- Subtitles --- - if subtitles_untouched: - if len(check.subtitle_tracks) != len(source.subtitle_tracks): - return False, (f"subtitle filtering is off but subtitle count changed: " - f"{len(source.subtitle_tracks)} -> {len(check.subtitle_tracks)}") - src_sub_langs = sorted(t.language for t in source.subtitle_tracks) - out_sub_langs = sorted(t.language for t in check.subtitle_tracks) - if src_sub_langs != out_sub_langs: - return False, "subtitle languages changed unexpectedly with filtering off" - elif expected_subtitle_count is not None and len(check.subtitle_tracks) != expected_subtitle_count: - return False, f"expected {expected_subtitle_count} subtitle track(s) in output, found {len(check.subtitle_tracks)}" - - # --- Chapters & attachments: preserved unless a future feature says - # otherwise (none currently does) --- + if Counter(_video_signature(t) for t in check.video_tracks) != Counter(_video_signature(t) for t in source.video_tracks): + return False, "video track codec/resolution changed unexpectedly" + + if len(check.audio_tracks) != len(expected_audio_tracks): + return False, f"expected {len(expected_audio_tracks)} audio track(s) in output, found {len(check.audio_tracks)}" + expected_audio = Counter(_audio_signature(t) for t in expected_audio_tracks) + actual_audio = Counter(_audio_signature(t) for t in check.audio_tracks) + if actual_audio != expected_audio: + return False, f"audio track identities/properties changed unexpectedly: expected {sorted(expected_audio.elements())}, found {sorted(actual_audio.elements())}" + + expected_subtitles = source.subtitle_tracks if subtitles_untouched else expected_subtitle_tracks + if len(check.subtitle_tracks) != len(expected_subtitles): + return False, f"expected {len(expected_subtitles)} subtitle track(s), found {len(check.subtitle_tracks)}" + if Counter(_subtitle_signature(t) for t in check.subtitle_tracks) != Counter(_subtitle_signature(t) for t in expected_subtitles): + return False, "subtitle track properties changed unexpectedly" + if check.chapter_count != source.chapter_count: return False, f"chapter count changed: {source.chapter_count} -> {check.chapter_count}" if check.attachment_count != source.attachment_count: return False, f"attachment count changed: {source.attachment_count} -> {check.attachment_count}" - # --- Duration --- if source.duration_seconds and check.duration_seconds: if abs(source.duration_seconds - check.duration_seconds) > _DURATION_TOLERANCE_SECONDS: return False, (f"duration changed beyond tolerance: " - f"{source.duration_seconds:.1f}s -> {check.duration_seconds:.1f}s") + f"{source.duration_seconds:.1f}s -> {check.duration_seconds:.1f}s") return True, "ok" diff --git a/audiocleaner/scanner.py b/audiocleaner/scanner.py index 88b0d3f..383dbf1 100644 --- a/audiocleaner/scanner.py +++ b/audiocleaner/scanner.py @@ -24,6 +24,7 @@ class ScanSummary: total_scanned: int = 0 cleaned: int = 0 + preview: int = 0 skipped_single_track: int = 0 no_english: int = 0 unknown_codec: int = 0 @@ -32,7 +33,7 @@ class ScanSummary: total_removed_subtitle_tracks: int = 0 total_bytes_saved: int = 0 elapsed_seconds: float = 0.0 - results: list = field(default_factory=list) # list[ProcessResult] + results: list = field(default_factory=list) def find_mkv_files(root: Path) -> list[Path]: @@ -56,12 +57,6 @@ def run_pipeline( persistent_backup: bool = False, history: Optional[ProcessingHistory] = None, ) -> ScanSummary: - """ - on_progress(files_done, files_total, current_filename, phase) is called - repeatedly during both the scan and process phases. - on_file_done(ProcessResult) is called once per file after processing. - should_cancel() lets the caller request an early stop between files. - """ start = time.time() summary = ScanSummary() @@ -78,9 +73,8 @@ def run_pipeline( try: history = ProcessingHistory() except Exception: - history = None # history is a convenience feature; never block a scan on it + history = None - # --- Phase 1: parallel metadata scan (populates cache) --- probed = 0 with ThreadPoolExecutor(max_workers=probe_workers) as pool: futures = {pool.submit(probe_file, f, cache): f for f in files} @@ -92,10 +86,9 @@ def run_pipeline( try: future.result() except Exception: - pass # errors surface again during processing phase + pass cache.save() - # --- Phase 2: sequential processing (remux is disk I/O heavy) --- for i, f in enumerate(files, start=1): if should_cancel and should_cancel(): break @@ -114,7 +107,9 @@ def run_pipeline( summary.results.append(result) summary.total_scanned += 1 - if result.status == "cleaned": + if result.preview: + summary.preview += 1 + elif result.status == "cleaned": summary.cleaned += 1 summary.total_removed_tracks += result.removed_track_count summary.total_removed_subtitle_tracks += result.removed_subtitle_count @@ -135,7 +130,7 @@ def run_pipeline( try: history.record(str(root), result) except Exception: - pass # never let a history-logging failure abort the run + pass cache.save() if owns_history and history is not None: @@ -149,12 +144,7 @@ def scan_subtitle_languages( on_progress: Optional[Callable[[int, int, str, str], None]] = None, probe_workers: int = 4, ) -> set: - """ - Quick pass to discover which subtitle languages exist across every MKV - file under root, used to populate the GUI's language checklist. Shares - the same probe cache as run_pipeline, so this is near-instant on a - library that's already been scanned before. - """ + """Quick pass to discover subtitle languages across the MKV tree.""" files = find_mkv_files(root) if not files: return set() diff --git a/audiocleaner/watcher.py b/audiocleaner/watcher.py index 0c29623..80aee13 100644 --- a/audiocleaner/watcher.py +++ b/audiocleaner/watcher.py @@ -1,13 +1,7 @@ """ Continuous watch mode: polls a folder tree for new or still-copying MKV files and only hands a file to the processor once its size has stopped -changing for `settle_seconds` -- this is what keeps it from grabbing a -file while Radarr/Sonarr (or any download client) is still moving it into -place. - -State is kept in memory only (per watch session); restarting the watcher -re-evaluates the whole tree, which is cheap because probe.ProbeCache -still short-circuits anything that hasn't changed on disk. +changing for `settle_seconds`. """ import time @@ -21,29 +15,44 @@ from .history import ProcessingHistory +@dataclass(frozen=True) +class _FileIdentity: + size: int + mtime_ns: int + + @dataclass class _Sighting: - size: int + identity: _FileIdentity last_size_change: float class WatchState: - """Tracks per-file size-stability history across polling passes.""" + """Tracks per-file stability and successful processing across polls.""" def __init__(self): self._sightings: dict[str, _Sighting] = {} - self._processed: set[str] = set() + # Store the identity, not merely the path. If a processed file is + # deleted and a different file is later moved into the same path, it + # must be eligible for processing again. + self._processed: dict[str, _FileIdentity] = {} - def observe(self, path: Path) -> Optional[_Sighting]: + def _identity(self, path: Path) -> Optional[_FileIdentity]: try: - size = path.stat().st_size + stat = path.stat() except OSError: return None + return _FileIdentity(size=stat.st_size, mtime_ns=stat.st_mtime_ns) + + def observe(self, path: Path) -> Optional[_Sighting]: + identity = self._identity(path) + if identity is None: + return None key = str(path) now = time.time() prev = self._sightings.get(key) - if prev is None or prev.size != size: - sighting = _Sighting(size=size, last_size_change=now) + if prev is None or prev.identity != identity: + sighting = _Sighting(identity=identity, last_size_change=now) else: sighting = prev self._sightings[key] = sighting @@ -56,11 +65,22 @@ def is_settled(self, path: Path, settle_seconds: int) -> bool: return (time.time() - sighting.last_size_change) >= settle_seconds def mark_processed(self, path: Path): - self._processed.add(str(path)) + identity = self._identity(path) + if identity is not None: + self._processed[str(path)] = identity self._sightings.pop(str(path), None) + def mark_retryable(self, path: Path): + """Leave the current file eligible for a later poll after an error.""" + self._processed.pop(str(path), None) + # Keep the sighting so a transient error does not force another full + # settle interval when the file itself has not changed. + def already_processed(self, path: Path) -> bool: - return str(path) in self._processed + identity = self._identity(path) + if identity is None: + return False + return self._processed.get(str(path)) == identity def watch_iteration( @@ -75,11 +95,7 @@ def watch_iteration( persistent_backup: bool = False, history: Optional[ProcessingHistory] = None, ) -> list[ProcessResult]: - """ - One polling pass over the folder tree. Returns a ProcessResult for - every file that was stable-and-not-yet-processed this pass (usually - zero, since most passes find nothing new). - """ + """One polling pass over the folder tree.""" results = [] for path in sorted( p for p in root.rglob("*") @@ -90,10 +106,10 @@ def watch_iteration( sighting = state.observe(path) if sighting is None: - continue # unreadable / vanished mid-poll, try again next pass + continue if not state.is_settled(path, settle_seconds): - continue # still copying (or just arrived) -- wait + continue result = process_file( path, cache=cache, @@ -103,15 +119,21 @@ def watch_iteration( max_safety_mode=max_safety_mode, persistent_backup=persistent_backup, ) - # Mark processed regardless of outcome (including errors) so a - # broken file doesn't get retried forever on every poll. - state.mark_processed(path) results.append(result) + # Only successful/non-error outcomes become processed. A transient + # MediaInfo/mkvmerge failure, a locked file, or a temporary I/O issue + # should be retried instead of being permanently suppressed for the + # rest of the watch session. + if result.status == "error": + state.mark_retryable(path) + else: + state.mark_processed(path) + if history is not None: try: history.record(str(root), result) except Exception: - pass # never let a history-logging failure disrupt watching + pass return results diff --git a/tests/test_processor_recovery.py b/tests/test_processor_recovery.py new file mode 100644 index 0000000..095e71b --- /dev/null +++ b/tests/test_processor_recovery.py @@ -0,0 +1,88 @@ +from pathlib import Path +from types import SimpleNamespace + +from audiocleaner import processor + + +def _probe(path, cache=None): + return SimpleNamespace( + error=None, + path=str(path), + size=path.stat().st_size, + audio_tracks=[ + SimpleNamespace(track_id=1, language="eng", codec_id="A_AC3", channels=6), + SimpleNamespace(track_id=2, language="eng", codec_id="A_AAC", channels=2), + ], + subtitle_tracks=[], + video_tracks=[SimpleNamespace(track_id=0, codec_id="V_MPEG4/ISO/AVC", width=1920, height=1080)], + chapter_count=0, + attachment_count=0, + duration_seconds=10.0, + ) + + +def _selection(result, keep_commentary=False): + return result.audio_tracks[0], "ac3", [] + + +def test_existing_backup_is_never_overwritten(monkeypatch, tmp_path): + media = tmp_path / "movie.mkv" + media.write_bytes(b"original") + backup = tmp_path / "movie.ac_backup.mkv" + backup.write_bytes(b"precious-recovery-copy") + + class Proc: + returncode = 0 + stdout = "" + stderr = "" + + def fake_run(cmd, **kwargs): + out = Path(cmd[cmd.index("-o") + 1]) + out.write_bytes(b"cleaned") + return Proc() + + monkeypatch.setattr(processor, "probe_file", _probe) + monkeypatch.setattr(processor, "select_audio_tracks_to_keep", _selection) + monkeypatch.setattr(processor, "explain_audio_selection", lambda *args, **kwargs: []) + monkeypatch.setattr(processor, "find_tool", lambda name: "mkvmerge") + monkeypatch.setattr(processor.subprocess, "run", fake_run) + monkeypatch.setattr(processor, "_verify_output", lambda *args, **kwargs: (True, "ok")) + + result = processor.process_file(media, max_safety_mode=True) + assert result.status == "error" + assert "already exists" in result.message + assert backup.read_bytes() == b"precious-recovery-copy" + assert media.read_bytes() == b"original" + + +def test_failed_final_verification_restores_original(monkeypatch, tmp_path): + media = tmp_path / "movie.mkv" + media.write_bytes(b"original") + + calls = {"verify": 0} + + def verify(*args, **kwargs): + calls["verify"] += 1 + return (calls["verify"] == 1, "bad final artifact") + + class Proc: + returncode = 0 + stdout = "" + stderr = "" + + def fake_run(cmd, **kwargs): + out = Path(cmd[cmd.index("-o") + 1]) + out.write_bytes(b"cleaned") + return Proc() + + monkeypatch.setattr(processor, "probe_file", _probe) + monkeypatch.setattr(processor, "select_audio_tracks_to_keep", _selection) + monkeypatch.setattr(processor, "explain_audio_selection", lambda *args, **kwargs: []) + monkeypatch.setattr(processor, "find_tool", lambda name: "mkvmerge") + monkeypatch.setattr(processor.subprocess, "run", fake_run) + monkeypatch.setattr(processor, "_verify_output", verify) + + result = processor.process_file(media, max_safety_mode=True) + assert result.status == "error" + assert result.restored_from_backup is True + assert media.read_bytes() == b"original" diff --git a/tests/test_processor_verification.py b/tests/test_processor_verification.py new file mode 100644 index 0000000..967a063 --- /dev/null +++ b/tests/test_processor_verification.py @@ -0,0 +1,75 @@ +from types import SimpleNamespace + +from audiocleaner import processor + + +def track(language="eng", codec_id="A_TRUEHD", channels=8): + return SimpleNamespace(language=language, codec_id=codec_id, channels=channels) + + +def video(codec_id="V_MPEG4/ISO/AVC", width=1920, height=1080): + return SimpleNamespace(codec_id=codec_id, width=width, height=height) + + +def subtitle(language="eng", codec_id="S_TEXT/UTF8", forced=False, default=False): + return SimpleNamespace(language=language, codec_id=codec_id, forced=forced, default=default) + + +def probe_result(audio, subtitles=None): + return SimpleNamespace( + error=None, + video_tracks=[video()], + audio_tracks=audio, + subtitle_tracks=subtitles or [], + chapter_count=10, + attachment_count=1, + duration_seconds=100.0, + ) + + +def test_audio_properties_are_verified_per_track(monkeypatch, tmp_path): + output = tmp_path / "out.mkv" + output.write_bytes(b"x" * 100) + + source = probe_result([ + track(codec_id="A_TRUEHD", channels=6), + track(codec_id="A_AC3", channels=8), + ]) + + # These properties exist in the output, but on different tracks. The old + # independent-set checks would accept this combination incorrectly. + output_probe = probe_result([ + track(codec_id="A_TRUEHD", channels=8), + track(codec_id="A_AC3", channels=6), + ]) + monkeypatch.setattr(processor, "probe_file", lambda *args, **kwargs: output_probe) + + ok, message = processor._verify_output( + output, + source, + expected_audio_tracks=source.audio_tracks[:1], + expected_subtitle_tracks=[], + subtitles_untouched=True, + source_size=100, + ) + assert ok is False + assert "audio track" in message + + +def test_final_verification_uses_same_validation_rules(monkeypatch, tmp_path): + output = tmp_path / "out.mkv" + output.write_bytes(b"x" * 100) + source = probe_result([track()]) + + monkeypatch.setattr(processor, "probe_file", lambda *args, **kwargs: probe_result([track(codec_id="A_AC3")])) + + ok, message = processor._verify_output( + output, + source, + expected_audio_tracks=source.audio_tracks, + expected_subtitle_tracks=[], + subtitles_untouched=True, + source_size=100, + ) + assert ok is False + assert "audio track" in message diff --git a/tests/test_scanner_preview.py b/tests/test_scanner_preview.py new file mode 100644 index 0000000..4b766e9 --- /dev/null +++ b/tests/test_scanner_preview.py @@ -0,0 +1,26 @@ +from types import SimpleNamespace + +from audiocleaner import scanner + + +def test_preview_is_tracked_separately(monkeypatch, tmp_path): + media = tmp_path / "movie.mkv" + media.write_bytes(b"movie") + + monkeypatch.setattr(scanner, "probe_file", lambda *args, **kwargs: SimpleNamespace( + subtitle_tracks=[] + )) + monkeypatch.setattr(scanner, "ProcessingHistory", lambda: None) + monkeypatch.setattr(scanner, "process_file", lambda *args, **kwargs: SimpleNamespace( + status="cleaned", + preview=True, + removed_track_count=2, + removed_subtitle_count=1, + bytes_saved=123, + )) + + summary = scanner.run_pipeline(tmp_path, preview_only=True, probe_workers=1) + assert summary.preview == 1 + assert summary.cleaned == 0 + assert summary.total_removed_tracks == 0 + assert summary.total_bytes_saved == 0 diff --git a/tests/test_watcher.py b/tests/test_watcher.py new file mode 100644 index 0000000..abb82aa --- /dev/null +++ b/tests/test_watcher.py @@ -0,0 +1,44 @@ +from pathlib import Path +from types import SimpleNamespace + +from audiocleaner.watcher import WatchState + + +def test_processed_path_is_reprocessed_when_file_identity_changes(tmp_path, monkeypatch): + path = tmp_path / "movie.mkv" + path.write_bytes(b"first") + + state = WatchState() + assert state.already_processed(path) is False + state.mark_processed(path) + assert state.already_processed(path) is True + + path.write_bytes(b"replacement") + assert state.already_processed(path) is False + + +def test_watch_state_retries_errors(monkeypatch, tmp_path): + from audiocleaner import watcher + + path = tmp_path / "movie.mkv" + path.write_bytes(b"movie") + state = WatchState() + + monkeypatch.setattr(watcher.time, "time", lambda: 100.0) + sighting = state.observe(path) + assert sighting is not None + assert state.is_settled(path, 0) + + calls = [] + + def fake_process_file(*args, **kwargs): + calls.append(path) + return SimpleNamespace(status="error") + + monkeypatch.setattr(watcher, "process_file", fake_process_file) + cache = SimpleNamespace() + + results = watcher.watch_iteration(tmp_path, state, cache, settle_seconds=0) + assert len(results) == 1 + assert calls == [path] + assert state.already_processed(path) is False