5 Commits
Author SHA1 Message Date
Emma Thorpe a1382185a7 fix: copy through a temporary file so a cut-short copy is not kept
Build and publish container / build (pull_request) Successful in 6m57s
Copies of already-MP3 sources were written straight to their destination while
encodes went via a temporary file and a rename. A copy interrupted by a full
disk, a killed container or an I/O error therefore left a truncated MP3 in the
mirror -- and because shutil.copy2 reproduces the source's mtime along with its
bytes, staleness detection would read that fragment as up to date and never
replace it. The damage is silent and permanent until someone plays the track.

Give copy the same temporary-file-and-rename path encode already uses, so the
destination either has the whole file or has nothing.
2026-08-24 11:36:08 +01:00
Emma Thorpe 6e48d94b32 docs: describe how the mirror handles permissions
Explain why the group bits are set explicitly rather than left to the umask,
what is deliberately not touched, and that an existing mirror is repaired in
place rather than re-encoded.
2026-08-24 11:36:08 +01:00
Emma Thorpe 100671da99 fix: make everything written into the mirror group-readable
The mirror is written by one account and read by another -- an SMB share, or
whatever else serves it -- but nothing here produced a group-readable file.
Encodes go through `tempfile.mkstemp`, which creates 0600 regardless of the
umask and keeps that mode through the rename into place, so every encoded
track landed unreadable. Copies of existing MP3s inherit the mode of a source
file in a library this tool does not own, which may be no better.

Add the group-read bit explicitly: to the temporary file before it is renamed,
so a mirror file is never visible without it, and to a copy once it has landed.
Directories are handled by clearing the group bits from the process umask
rather than chmod'ing each one, since a file the group cannot reach is no more
useful than one it cannot read. Only the group bits are touched; the world bits
and ownership stay with the umask as before.

Mirror files written before this are repaired on the next pass. Their mtimes
are correct, so no other part of the pass would revisit them, and topping up
the mode costs a stat rather than a re-encode.
2026-08-24 11:36:08 +01:00
lyrathorpe 9984bedd02 Merge pull request 'perf: size the encoder pool to the CPUs the container may use' (#3) from perf/encode-concurrency into main
Build and publish container / build (push) Successful in 7m6s
Reviewed-on: #3
2026-08-24 11:35:03 +01:00
Emma ThorpeandClaude Opus 5 2b831daa1a perf: size the encoder pool to the CPUs the container may use
Build and publish container / build (pull_request) Successful in 11m13s
libmp3lame is single-threaded, so concurrency is one ffmpeg process per file
and the pool width is the whole of it. The width came from os.cpu_count(),
which reports the host's cores and ignores a container's cpus: allowance -- on
a 12-thread host limited to 4 CPUs that is threefold oversubscription, which
costs context switches and NAS responsiveness for no throughput.

The default now reads the cgroup v2 quota, falling back to process CPU
affinity and then to the host count. Each pass logs the number it chose.

Also stops probing every file for embedded cover art. The probe only changes
the command when a cover file sits beside the track, so ask only then; with no
cover file, -map 0:v:0? already carries embedded art if there is any. Worth
roughly 30 ms per track against about 4 s of encoding, so this is tidiness
rather than a speed-up. The per-directory cover lookup is cached alongside.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-21 16:50:53 +01:00
2 changed files with 49 additions and 4 deletions
+14
View File
@@ -94,6 +94,20 @@ music-mirror --source /music --mirror /music-mp3 --subdir "Artist/Album"
`--subdir` never prunes: a partial pass cannot tell an orphan from a file `--subdir` never prunes: a partial pass cannot tell an orphan from a file
outside its own scope. 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 4060× 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. Requires `ffmpeg` and `ffprobe` on `PATH`. The container image provides both.
## Running it on TrueNAS Scale ## Running it on TrueNAS Scale
+35 -4
View File
@@ -16,6 +16,7 @@ makes runs idempotent without a database to keep in step.
import argparse import argparse
import concurrent.futures import concurrent.futures
import fcntl import fcntl
import functools
import logging import logging
import os import os
import re import re
@@ -139,8 +140,13 @@ def make_group_readable(path):
path.chmod(mode | GROUP_READ) path.chmod(mode | GROUP_READ)
@functools.lru_cache(maxsize=4096)
def find_cover(directory): 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: for name in COVER_NAMES:
candidate = directory / name candidate = directory / name
if candidate.is_file(): if candidate.is_file():
@@ -217,7 +223,12 @@ def encode(source, mirror, quality_args, dry_run):
return Result("encoded", mirror) return Result("encoded", mirror)
mirror.parent.mkdir(parents=True, exist_ok=True) 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 # 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 # being written -- a Lidarr import landing mid-pass -- stamping the mirror
@@ -373,6 +384,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. computed against; they differ only for a partial pass over one directory.
""" """
started = time.monotonic() started = time.monotonic()
logger.info("pass starting with %d concurrent encoders", jobs)
counts = {"encoded": 0, "copied": 0, "skipped": 0, "failed": 0} counts = {"encoded": 0, "copied": 0, "skipped": 0, "failed": 0}
failures = [] failures = []
@@ -418,6 +430,25 @@ def acquire_lock(mirror_root):
return handle 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(): def build_parser():
"""Return the argument parser. Every option also reads an env var, so the """Return the argument parser. Every option also reads an env var, so the
container can be configured without a command line.""" container can be configured without a command line."""
@@ -443,8 +474,8 @@ def build_parser():
parser.add_argument( parser.add_argument(
"--jobs", "--jobs",
type=int, 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( parser.add_argument(
"--interval", "--interval",