diff --git a/README.md b/README.md index f070bf5..1e33785 100644 --- a/README.md +++ b/README.md @@ -28,6 +28,29 @@ mtime, so a file is stale exactly when the two differ. There is no database to fall out of step with the library, which matters when something else — Lidarr, in this case — is the thing that owns and reorganises it. +### What that means for Lidarr + +| Lidarr does this | The mirror does this | +| --------------------------------------- | -------------------------------------------------------------------------------- | +| Replaces a file with a better rip | Re-encodes in place. Same path in, same path out, so no duplicate | +| Upgrades MP3 to FLAC | Both map to the same `.mp3` mirror path, so the old one is overwritten | +| Renames a track, album or artist folder | Old path pruned, new path encoded. Correct, but it re-encodes rather than moving | +| Deletes an album or artist | Every orphaned mirror file is deleted and the emptied directories go too | + +Pruning is driven by what the pass actually found, not by guessing source +filenames from mirror ones: a `.FLAC` source would not be found by a search for +`.flac`, and the mirror file would be deleted and rebuilt on alternate passes +for ever. + +If two sources want the same mirror path — a `01 Song.flac` next to a leftover +`01 Song.mp3`, which is what an interrupted upgrade leaves — the better format +wins, ties break on path, and the loser is logged. Without that rule both +encode to the same destination and every pass finds one of them stale. + +The mtime is read _before_ encoding rather than after. A file still being +written when the pass reaches it would otherwise be stamped with its final +mtime while holding truncated audio, and never be revisited. + Encodes are written to a temporary file and renamed into place, so an interrupted run cannot leave a truncated MP3 that the next run mistakes for finished work. A lock file in the mirror root stops two passes overlapping. diff --git a/music_mirror.py b/music_mirror.py index 1f838c3..97a57dd 100644 --- a/music_mirror.py +++ b/music_mirror.py @@ -50,6 +50,23 @@ SOURCE_EXTENSIONS = { # quality for nothing. COPY_EXTENSIONS = {".mp3"} +# Best first. Used only to settle which source wins when two of them want the +# same mirror path; see plan(). +SOURCE_PRIORITY = [ + ".flac", + ".wav", + ".aif", + ".aiff", + ".ape", + ".wv", + ".alac", + ".m4a", + ".ogg", + ".opus", + ".wma", + ".mp3", +] + # Looked for in the source directory when a file has no embedded picture. COVER_NAMES = ("cover.jpg", "folder.jpg", "front.jpg", "cover.png", "folder.png") @@ -186,6 +203,12 @@ def encode(source, mirror, quality_args, dry_run): mirror.parent.mkdir(parents=True, exist_ok=True) cover = None if has_embedded_picture(source) else find_cover(source.parent) + # Read the source's mtime before encoding, not after. If the file is still + # being written -- a Lidarr import landing mid-pass -- stamping the mirror + # with the later mtime would mark truncated output as current. Stamping the + # earlier one leaves the two mismatched, so the next pass re-encodes it. + stat = source.stat() + # Encode to a temporary file in the destination directory and rename it # into place, so an interrupted run cannot leave a truncated MP3 that the # next run would treat as complete. @@ -199,7 +222,6 @@ def encode(source, mirror, quality_args, dry_run): if completed.returncode != 0: lines = completed.stderr.strip().splitlines() return Result("failed", source, lines[-1] if lines else "ffmpeg failed") - stat = source.stat() os.utime(temporary, (stat.st_atime, stat.st_mtime)) os.replace(temporary, mirror) except Exception as error: # noqa: BLE001 - reported per file, run continues @@ -227,9 +249,8 @@ def copy(source, mirror, dry_run): return Result("copied", mirror) -def process(source, source_root, mirror_root, quality_args, dry_run): +def process(source, mirror, quality_args, dry_run): """Bring one source file's mirror entry up to date.""" - mirror = mirror_path_for(source, source_root, mirror_root) if is_current(source, mirror): return Result("skipped", mirror) if source.suffix.lower() in COPY_EXTENSIONS: @@ -245,15 +266,47 @@ def find_sources(root): yield path -def prune(source_root, mirror_root, dry_run): - """Delete mirror files whose source is gone, and any dirs left empty.""" - extensions = SOURCE_EXTENSIONS | COPY_EXTENSIONS +def plan(scan_root, source_root, mirror_root): + """Map each mirror path to the one source that should produce it. + + Two sources can want the same mirror path -- `01 Song.flac` alongside a + leftover `01 Song.mp3`, which is what an interrupted Lidarr upgrade leaves + behind. Without a decision here both would encode to the same destination, + each pass would find the loser stale, and the mirror would be rewritten + forever. Preferring the highest-quality source, ties broken by path, makes + the outcome stable and predictable instead. + """ + chosen = {} + for source in find_sources(scan_root): + mirror = mirror_path_for(source, source_root, mirror_root) + rival = chosen.get(mirror) + if rival is None: + chosen[mirror] = source + continue + winner, loser = sorted((source, rival), key=source_rank) + logger.warning("%s and %s both map to %s; using %s", rival, source, mirror, winner) + chosen[mirror] = winner + return chosen + + +def source_rank(source): + """Sort key preferring better source formats, then a stable path order.""" + suffix = source.suffix.lower() + position = SOURCE_PRIORITY.index(suffix) if suffix in SOURCE_PRIORITY else len(SOURCE_PRIORITY) + return (position, str(source)) + + +def prune(mirror_root, expected, dry_run): + """Delete mirror files this pass did not account for, and empty dirs. + + Driven by the set of paths the pass expects to exist rather than by + probing the source tree for names, which would disagree with it over + letter case and over any extension the walker does not collect. + """ removed = 0 for mirror in sorted(mirror_root.rglob(f"*{MIRROR_SUFFIX}")): - relative = mirror.relative_to(mirror_root) - stem = source_root / relative - if any((stem.with_suffix(extension)).exists() for extension in extensions): + if mirror in expected: continue removed += 1 if dry_run: @@ -281,10 +334,12 @@ def run_once(scan_root, source_root, mirror_root, quality_args, jobs, dry_run, d counts = {"encoded": 0, "copied": 0, "skipped": 0, "failed": 0} failures = [] + work = plan(scan_root, source_root, mirror_root) + with concurrent.futures.ThreadPoolExecutor(max_workers=jobs) as pool: futures = [ - pool.submit(process, source, source_root, mirror_root, quality_args, dry_run) - for source in find_sources(scan_root) + pool.submit(process, source, mirror, quality_args, dry_run) + for mirror, source in work.items() ] for future in concurrent.futures.as_completed(futures): result = future.result() @@ -292,7 +347,7 @@ def run_once(scan_root, source_root, mirror_root, quality_args, jobs, dry_run, d if result.action == "failed": failures.append(result) - removed = prune(source_root, mirror_root, dry_run) if do_prune else 0 + removed = prune(mirror_root, set(work), dry_run) if do_prune else 0 for failure in failures: logger.error("failed: %s: %s", failure.path, failure.error) diff --git a/tests/test_music_mirror.py b/tests/test_music_mirror.py index 70fb18d..825dfae 100644 --- a/tests/test_music_mirror.py +++ b/tests/test_music_mirror.py @@ -1,4 +1,5 @@ import os +import shutil import subprocess import time @@ -152,6 +153,100 @@ def test_existing_mp3_is_copied_not_re_encoded(tmp_path, make_flac): assert (mirror / "b.mp3").read_bytes() == (source / "b.mp3").read_bytes() +def test_format_upgrade_replaces_rather_than_duplicating(tmp_path, make_flac): + """Lidarr replacing an MP3 with a FLAC must not leave two mirror files.""" + source = tmp_path / "src" + mirror = tmp_path / "dst" + flac = make_flac(source / "Album" / "01 Song.flac") + subprocess.run( + ["ffmpeg", "-loglevel", "error", "-y", "-i", str(flac), str(source / "Album" / "01 Song.mp3")], + check=True, + capture_output=True, + ) + flac.unlink() + + run(source, mirror) + assert sorted(p.name for p in (mirror / "Album").iterdir()) == ["01 Song.mp3"] + + # The upgrade: the MP3 goes, a FLAC arrives at the same stem. + (source / "Album" / "01 Song.mp3").unlink() + make_flac(source / "Album" / "01 Song.flac", title="Upgraded") + run(source, mirror) + + assert sorted(p.name for p in (mirror / "Album").iterdir()) == ["01 Song.mp3"] + + +def test_renamed_album_leaves_nothing_behind(tmp_path, make_flac): + """A Lidarr rename is a delete plus an add; the old tree must not linger.""" + source = tmp_path / "src" + mirror = tmp_path / "dst" + make_flac(source / "Artist" / "Album (2019)" / "01 Song.flac") + + run(source, mirror) + (source / "Artist" / "Album (2019)").rename(source / "Artist" / "Album (2020)") + run(source, mirror) + + assert (mirror / "Artist" / "Album (2020)" / "01 Song.mp3").is_file() + assert not (mirror / "Artist" / "Album (2019)").exists() + + +def test_competing_sources_pick_the_lossless_one_and_stay_stable(tmp_path, make_flac, probe_tag): + """Both a FLAC and an MP3 at one stem: the FLAC wins, and stays won.""" + source = tmp_path / "src" + mirror = tmp_path / "dst" + make_flac(source / "a.flac", title="From FLAC") + other = make_flac(tmp_path / "scratch" / "other.flac", title="From MP3") + subprocess.run( + ["ffmpeg", "-loglevel", "error", "-y", "-i", str(other), str(source / "a.mp3")], + check=True, + capture_output=True, + ) + # Age the loser well beyond the mtime tolerance, so "is it current?" gives + # a definite answer for it rather than one that depends on the clock. + older = time.time() - 3600 + os.utime(source / "a.mp3", (older, older)) + + run(source, mirror) + assert probe_tag(mirror / "a.mp3", "title") == "From FLAC" + + # The loser must not make the mirror look stale on the next pass, or every + # run would re-encode for ever. + first = (mirror / "a.mp3").stat().st_mtime_ns + run(source, mirror) + assert (mirror / "a.mp3").stat().st_mtime_ns == first + + +def test_uppercase_extension_is_not_pruned_and_re_encoded(tmp_path, make_flac): + """A .FLAC source must not be treated as an orphan on the next pass.""" + source = tmp_path / "src" + mirror = tmp_path / "dst" + made = make_flac(source / "a.flac") + made.rename(source / "a.FLAC") + + run(source, mirror) + first = (mirror / "a.mp3").stat().st_mtime_ns + + run(source, mirror) + assert (mirror / "a.mp3").is_file() + assert (mirror / "a.mp3").stat().st_mtime_ns == first + + +def test_whole_library_deleted_empties_the_mirror(tmp_path, make_flac): + source = tmp_path / "src" + mirror = tmp_path / "dst" + make_flac(source / "A" / "one.flac") + make_flac(source / "B" / "two.flac") + + run(source, mirror) + shutil.rmtree(source / "A") + shutil.rmtree(source / "B") + run(source, mirror) + + assert list(mirror.rglob("*.mp3")) == [] + assert not (mirror / "A").exists() + assert not (mirror / "B").exists() + + def test_dry_run_writes_nothing(tmp_path, make_flac): source = tmp_path / "src" mirror = tmp_path / "dst"