chore(release): v0.1.1

Reviewed-on: #3

README.md

@@ -51,25 +51,9 @@ 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.
 
-Both encodes and copies are written to a temporary file and renamed into place,
+Encodes are written to a temporary file and renamed into place, so an
-so an interrupted run cannot leave a truncated MP3 that the next run mistakes
+interrupted run cannot leave a truncated MP3 that the next run mistakes for
-for finished work. Copies need it as much as encodes do: the mtime comes across
+finished work. A lock file in the mirror root stops two passes overlapping.
-with the bytes, so a half-written copy would look current for ever. A lock file
-in the mirror root stops two passes overlapping.
-
-### Permissions
-
-Everything written into the mirror is made group-readable, and its directories
-group-traversable, so the mirror can be read back by whatever serves it. Neither
-writer does that unaided: the temporary file an encode renames into place is
-created `0600` regardless of the umask, and a straight copy of an existing MP3
-inherits the mode of a source file in a library this tool does not own. Only the
-group bits are touched; whether the mirror is world-readable stays with the
-umask, as does the ownership.
-
-Mirror files written before this existed are topped up on the next pass. Their
-mtimes are correct, so nothing else would revisit them — and they are not
-re-encoded, only chmod'ed.
 
 ## Usage
 
@@ -94,6 +78,20 @@ music-mirror --source /music --mirror /music-mp3 --subdir "Artist/Album"
 `--subdir` never prunes: a partial pass cannot tell an orphan from a file
 outside its own scope.
 
+### Concurrency
+
+LAME is single-threaded — ffmpeg reports `Threading capabilities: none` for
+`libmp3lame` — so throughput comes entirely from running several encoders at
+once, one process per file. `--jobs` defaults to the CPUs the process may
+actually use, which inside a container means the `cpus:` allowance rather than
+the host's core count. Each pass logs the number it settled on.
+
+As a rough guide, a Zen 3 core encodes about 40–60× realtime at V0 depending on
+clock, so six cores clear roughly 250 hours of audio per hour of wall clock.
+The first full pass is the expensive one; after that only new and changed files
+are touched. Lower `MUSIC_MIRROR_JOBS` if you would rather the NAS stayed
+responsive than finished sooner.
+
 Requires `ffmpeg` and `ffprobe` on `PATH`. The container image provides both.
 
 ## Running it on TrueNAS Scale

music_mirror.py

@@ -16,6 +16,7 @@ makes runs idempotent without a database to keep in step.
 import argparse
 import concurrent.futures
 import fcntl
+import functools
 import logging
 import os
 import re
@@ -76,15 +77,6 @@ MIRROR_SUFFIX = ".mp3"
 # Filesystems disagree about mtime precision; SMB in particular rounds.
 MTIME_TOLERANCE_SECONDS = 2
 
-# The mirror exists to be read back by something else -- an SMB share, another
-# account on the box -- so everything written into it has to be group-readable.
-# Neither writer manages that unaided: tempfile.mkstemp forces 0600 whatever the
-# umask, and shutil.copy2 carries the source file's mode across from a library
-# that may be tighter still. Directories need the execute bit too, or the group
-# cannot enter them to reach the readable files inside.
-GROUP_READ = 0o040
-GROUP_ENTER = 0o050
-
 
 @dataclass
 class Result:
@@ -132,15 +124,13 @@ def is_current(source, mirror):
     return abs(source.stat().st_mtime - mirror.stat().st_mtime) <= MTIME_TOLERANCE_SECONDS
 
 
-def make_group_readable(path):
+@functools.lru_cache(maxsize=4096)
-    """Add the group-read bit to a mirror file, leaving the rest of the mode alone."""
-    mode = path.stat().st_mode
-    if not mode & GROUP_READ:
-        path.chmod(mode | GROUP_READ)
-
-
 def find_cover(directory):
-    """Return an external cover image for a directory, if one is present."""
+    """Return an external cover image for a directory, if one is present.
+
+    Cached because an album's tracks all ask the same question, and the answer
+    costs one stat per candidate name.
+    """
     for name in COVER_NAMES:
         candidate = directory / name
         if candidate.is_file():
@@ -217,7 +207,12 @@ def encode(source, mirror, quality_args, dry_run):
         return Result("encoded", mirror)
 
     mirror.parent.mkdir(parents=True, exist_ok=True)
-    cover = None if has_embedded_picture(source) else find_cover(source.parent)
+    # Probing costs an ffprobe process per file, so only ask when the answer
+    # can change the command. With no cover file beside the track, `-map
+    # 0:v:0?` carries embedded art if there is any and shrugs if there is not.
+    cover = find_cover(source.parent)
+    if cover is not None and has_embedded_picture(source):
+        cover = None
 
     # 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
@@ -239,9 +234,6 @@ def encode(source, mirror, quality_args, dry_run):
             lines = completed.stderr.strip().splitlines()
             return Result("failed", source, lines[-1] if lines else "ffmpeg failed")
         os.utime(temporary, (stat.st_atime, stat.st_mtime))
-        # Before the rename, so the file is never visible in the mirror without
-        # the bit.
-        make_group_readable(temporary)
         os.replace(temporary, mirror)
     except Exception as error:  # noqa: BLE001 - reported per file, run continues
         return Result("failed", source, str(error))
@@ -253,31 +245,16 @@ def encode(source, mirror, quality_args, dry_run):
 
 
 def copy(source, mirror, dry_run):
-    """Copy an already-MP3 source into the mirror, atomically."""
+    """Copy an already-MP3 source into the mirror."""
     if dry_run:
         logger.info("would copy %s", source)
         return Result("copied", mirror)
 
     mirror.parent.mkdir(parents=True, exist_ok=True)
-
-    # Through a temporary file and a rename, for the same reason encodes go
-    # that way, and a sharper one: copy2 reproduces the source's mtime as well
-    # as its bytes, so a copy cut short by a full disk or a killed container
-    # would leave a truncated MP3 that every later pass reads as current.
-    handle, temporary = tempfile.mkstemp(dir=mirror.parent, suffix=".mp3.part")
-    os.close(handle)
-    temporary = Path(temporary)
-
     try:
-        shutil.copy2(source, temporary)
+        shutil.copy2(source, mirror)
-        # copy2 brings the source's mode with it, and the source library is not
-        # ours to have permissions opinions about.
-        make_group_readable(temporary)
-        os.replace(temporary, mirror)
     except OSError as error:
         return Result("failed", source, str(error))
-    finally:
-        temporary.unlink(missing_ok=True)
 
     logger.info("copied %s", source)
     return Result("copied", mirror)
@@ -286,14 +263,6 @@ def copy(source, mirror, dry_run):
 def process(source, mirror, quality_args, dry_run):
     """Bring one source file's mirror entry up to date."""
     if is_current(source, mirror):
-        # A mirror written before this bit was set has a correct mtime, so
-        # nothing else in the pass would ever revisit it. Top it up here
-        # instead: one stat per file, and no chmod at all once it is right.
-        if not dry_run:
-            try:
-                make_group_readable(mirror)
-            except OSError as error:
-                return Result("failed", mirror, str(error))
         return Result("skipped", mirror)
     if source.suffix.lower() in COPY_EXTENSIONS:
         return copy(source, mirror, dry_run)
@@ -373,6 +342,7 @@ def run_once(scan_root, source_root, mirror_root, quality_args, jobs, dry_run, d
     computed against; they differ only for a partial pass over one directory.
     """
     started = time.monotonic()
+    logger.info("pass starting with %d concurrent encoders", jobs)
     counts = {"encoded": 0, "copied": 0, "skipped": 0, "failed": 0}
     failures = []
 
@@ -418,6 +388,25 @@ def acquire_lock(mirror_root):
     return handle
 
 
+def default_jobs():
+    """Return the number of CPUs this process may actually use.
+
+    os.cpu_count() reports the host's total, which in a container with a `cpus:`
+    limit means starting several times more encoders than there is CPU to run
+    them. libmp3lame is single-threaded, so one process per available CPU is the
+    whole of the concurrency story.
+    """
+    try:
+        quota, period = Path("/sys/fs/cgroup/cpu.max").read_text().split()
+        if quota != "max":
+            return max(1, round(int(quota) / int(period)))
+    except (OSError, ValueError):
+        pass
+    if hasattr(os, "process_cpu_count"):  # 3.13+, respects CPU affinity
+        return os.process_cpu_count() or 4
+    return os.cpu_count() or 4
+
+
 def build_parser():
     """Return the argument parser. Every option also reads an env var, so the
     container can be configured without a command line."""
@@ -443,8 +432,8 @@ def build_parser():
     parser.add_argument(
         "--jobs",
         type=int,
-        default=int(os.getenv("MUSIC_MIRROR_JOBS", "0")) or (os.cpu_count() or 4),
+        default=int(os.getenv("MUSIC_MIRROR_JOBS", "0")) or default_jobs(),
-        help="concurrent encodes (env MUSIC_MIRROR_JOBS; default: CPU count)",
+        help="concurrent encodes (env MUSIC_MIRROR_JOBS; default: available CPUs)",
     )
     parser.add_argument(
         "--interval",
@@ -474,13 +463,6 @@ def main(argv=None):
     logging.basicConfig(format="%(asctime)s %(levelname)s %(message)s", level=logging.INFO)
     args = build_parser().parse_args(argv)
 
-    # Directories are created with 0o777 masked by the umask, so clear the group
-    # bits from it once here rather than chmod'ing every directory the walk
-    # creates. Files cannot be handled this way -- mkstemp and copy2 both set a
-    # mode outright -- so they get an explicit chmod instead.
-    inherited = os.umask(0o077)
-    os.umask(inherited & ~GROUP_ENTER)
-
     if not args.source or not args.mirror:
         logger.error("both --source and --mirror are required")
         return 2

pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "music-mirror"
-version = "0.1.0"
+version = "0.1.1"
 description = "Maintain a lossy MP3 mirror of a lossless music library"
 readme = "README.md"
 requires-python = ">=3.11"

tests/conftest.py

@@ -19,14 +19,6 @@ def require_ffmpeg():
             pytest.skip(f"{tool} is not on PATH", allow_module_level=True)
 
 
-@pytest.fixture
-def tight_umask():
-    """Run a test under a umask that would otherwise make the mirror private."""
-    previous = os.umask(0o077)
-    yield
-    os.umask(previous)
-
-
 @pytest.fixture
 def make_flac():
     """Return a factory writing a short tagged FLAC file."""

tests/test_music_mirror.py

@@ -1,9 +1,7 @@
 import os
 import shutil
-import stat
 import subprocess
 import time
-from pathlib import Path
 
 import pytest
 
@@ -155,105 +153,6 @@ 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_interrupted_copy_leaves_nothing_behind(tmp_path, make_flac, monkeypatch):
-    """copy2 reproduces the source mtime, so a truncated copy left in the mirror
-    would be read as current by every later pass."""
-    source = tmp_path / "src"
-    mirror = tmp_path / "dst"
-    flac = make_flac(source / "a.flac")
-    subprocess.run(
-        ["ffmpeg", "-loglevel", "error", "-y", "-i", str(flac), str(source / "b.mp3")],
-        check=True,
-        capture_output=True,
-    )
-    flac.unlink()
-
-    def truncated(src, destination, **kwargs):
-        Path(destination).write_bytes(Path(src).read_bytes()[:64])
-        raise OSError("no space left on device")
-
-    monkeypatch.setattr(music_mirror.shutil, "copy2", truncated)
-
-    assert run(source, mirror) == 1
-    assert not (mirror / "b.mp3").exists()
-    assert list(mirror.rglob("*.part")) == []
-
-
-def test_encoded_file_is_group_readable(tmp_path, make_flac, tight_umask):
-    """mkstemp creates 0600 whatever the umask, so the bit has to be added."""
-    source = tmp_path / "src"
-    mirror = tmp_path / "dst"
-    make_flac(source / "a.flac")
-
-    run(source, mirror)
-
-    assert (mirror / "a.mp3").stat().st_mode & stat.S_IRGRP
-
-
-def test_copied_file_is_group_readable(tmp_path, make_flac, tight_umask):
-    """copy2 carries the source's mode across, and the source may be private."""
-    source = tmp_path / "src"
-    mirror = tmp_path / "dst"
-    flac = make_flac(source / "a.flac")
-    subprocess.run(
-        ["ffmpeg", "-loglevel", "error", "-y", "-i", str(flac), str(source / "b.mp3")],
-        check=True,
-        capture_output=True,
-    )
-    flac.unlink()
-    (source / "b.mp3").chmod(0o600)
-
-    run(source, mirror)
-
-    assert (mirror / "b.mp3").stat().st_mode & stat.S_IRGRP
-
-
-def test_mirror_directories_are_group_traversable(tmp_path, make_flac, tight_umask):
-    """A readable file is unreachable if the group cannot enter its directory."""
-    source = tmp_path / "src"
-    mirror = tmp_path / "dst"
-    make_flac(source / "Artist" / "Album" / "a.flac")
-
-    run(source, mirror)
-
-    for directory in (mirror, mirror / "Artist", mirror / "Artist" / "Album"):
-        mode = directory.stat().st_mode
-        assert mode & stat.S_IRGRP, directory
-        assert mode & stat.S_IXGRP, directory
-
-
-def test_private_mirror_file_is_repaired_without_re_encoding(tmp_path, make_flac):
-    """A mirror written by an older version has a correct mtime, so nothing
-    else in the pass would revisit it."""
-    source = tmp_path / "src"
-    mirror = tmp_path / "dst"
-    make_flac(source / "a.flac")
-
-    run(source, mirror)
-    output = mirror / "a.mp3"
-    output.chmod(output.stat().st_mode & ~stat.S_IRGRP)
-    before = output.stat().st_mtime_ns
-
-    run(source, mirror)
-
-    assert output.stat().st_mode & stat.S_IRGRP
-    assert output.stat().st_mtime_ns == before
-
-
-def test_dry_run_does_not_change_permissions(tmp_path, make_flac):
-    source = tmp_path / "src"
-    mirror = tmp_path / "dst"
-    make_flac(source / "a.flac")
-
-    run(source, mirror)
-    output = mirror / "a.mp3"
-    output.chmod(output.stat().st_mode & ~stat.S_IRGRP)
-
-    run(source, mirror, "--dry-run")
-
-    assert not output.stat().st_mode & stat.S_IRGRP
-
-
 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"