Commit Graph
7 Commits
Author SHA1 Message Date
Emma Thorpe 284069d9eb fix: report renames honestly in a dry run
--dry-run described a FAT32 rename as an encode. process() skipped the rename
whenever dry_run was set, then found no file at the target and fell through to
the encode path, so a preview of enabling --fat32-safe announced a full
re-encode of every track whose name held a reserved character. The real run
moves those files in a moment. A preview that inverts the cost of the thing
being previewed is worse than no preview at all.

Prune compounded it. With nothing renamed, the pre-sanitisation files are still
on disk, and they were reported as orphans due for deletion -- so the same dry
run claimed the library would be re-encoded and the originals thrown away,
neither of which is true.

Renames are now their own outcome: reported as "would rename" in a dry run,
counted separately from encodes in the pass summary, and excluded from the
orphan list when a dry run leaves them in place.
2026-08-25 11:26:24 +01:00
Emma Thorpe 3141f7ca87 feat: name the mirror so a FAT32 device will take it, and copy album art
Build and publish container / build (pull_request) Successful in 2m18s
Two changes for playing the mirror on a Rockbox iPod, where the device is FAT32
and Rockbox reads a plain directory tree rather than a database.

--fat32-safe names mirror files acceptably: the reserved characters and control
characters become underscores, trailing dots and spaces are stripped because
FAT eats them silently and the name then round-trips as a different one, and a
component left empty becomes an underscore. Names differing only in case are
detected as collisions, since two files here are one file there and the second
would silently overwrite the first. "Kick Out the Epic Motherf**ker" is a real
example from a real library, and without this it simply never arrives.

Off by default. It renames files, and that should be a decision rather than a
surprise on somebody's next pass.

Turning it on does not re-encode anything. Every track whose name held a
reserved character changes path, and encoding those again would be hours of
work producing files that already exist byte for byte, so the run moves them
instead and logs each one. Prune then finds nothing left behind.

Album art is now also copied into the mirror as cover.jpg beside the tracks.
Rockbox searches the filesystem for art -- cover.jpg, folder.jpg and the rest,
in the track's directory or its parent -- and that search never looks at the
picture embedded in the tag, so a mirror that only embeds art displays none of
it on the device. Embedding continues for the Apple firmware; both are now
satisfied. A cover whose tracks have all been pruned is removed too, or its
directory would never look empty and never go.

Adds tools/check_fat32.py, which reports unacceptable paths before a copy
rather than during one: rsync reports them too, but scattered through fifty
thousand files where they are easy to lose. It exits non-zero so it can gate a
script.

The README documents the rsync invocation, including why --modify-window=2 is
required against FAT and why Rhythmbox must be kept out of the transfer --
rb_ipod_helpers_is_ipod() reads access-protocols from media-player-info and
returns true on the USB id alone, without looking at the filesystem, so
removing iPod_Control changes nothing.
2026-08-25 10:29:55 +01:00
Emma Thorpe e9852e6c86 fix: keep the mirror readable under a umask that masks the owner's read bit
Build and publish container / build (pull_request) Successful in 15m16s
The umask handling added for group access cleared only the group bits and left
owner and other to the environment. A container whose umask carries 0400 then
produces mirror directories of mode 0300: writable and enterable, unreadable to
the very run that created them, and unreadable to anything serving the share.

Clear the owner read and execute bits from the umask as well. The `other` bits
stay where the environment puts them, because whether the mirror is
world-readable is a real policy question; being able to read a directory the
process itself just created is not.

Files were never exposed to this: mkstemp sets 0600 outright and copy2 takes
the source file's mode, both ignoring the umask.
2026-08-24 13:21:07 +01:00
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 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
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
lyrathorpeandEmma Thorpe d4ccff3b75 feat: mirror a lossless library to MP3 for iPod sync (#1)
Build and publish container / build (push) Failing after 51s
## What

A path-for-path MP3 mirror of a lossless library. FLAC in, MP3 out, same relative layout, tags and cover art carried across. Already-MP3 sources are copied rather than re-encoded. Mirror files whose source has gone are deleted, along with any directory they emptied. The source library is never written to — mounted read-only in the compose file, and never opened for writing in code.

| Situation                        | Action                               |
| -------------------------------- | ------------------------------------ |
| No mirror file                   | encode                               |
| Source modified since the mirror | re-encode (a Lidarr quality upgrade) |
| Mirror up to date                | skip                                 |
| Source is already MP3            | copy verbatim                        |
| Source gone                      | delete, prune empty dirs             |

## Design notes

- **No database.** Freshness is mtime: an encode is stamped with its source's mtime, so a file is stale exactly when the two differ. Lidarr owns the library; a second tool with its own index would only fall out of step with it. This is also why it is not beets.
- **Atomic writes.** Encode to a temp file, rename into place. An interrupted run cannot leave a truncated MP3 that the next run treats as finished.
- **A lock file** in the mirror root stops two passes overlapping.
- **Refuses a mirror inside the source tree**, which would otherwise recurse.
- `--subdir` never prunes: a partial pass cannot distinguish an orphan from a file outside its scope.

## Shipping

- Python package with a `music-mirror` console script, no runtime dependencies beyond ffmpeg.
- `Dockerfile` plus `compose.yaml` as a TrueNAS Scale Custom App: source dataset read-only, mirror dataset writable, `MUSIC_MIRROR_INTERVAL=6h`.
- CI runs the tests **inside the image**, against the ffmpeg that ships, on every pull request, and on merge publishes multi-arch (amd64 + arm64) to this Gitea's registry using the `PACKAGES_SECRET` repository secret. Versioning follows the same conventional-commit scheme as `legacy-email-proxy`, including writing the released version back into `pyproject.toml` so the packaging metadata cannot drift behind the tag.

## Behaviour under 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, though it re-encodes rather than moving |
| Deletes an album or artist               | Every orphaned mirror file goes, and the directories they emptied with them        |

Three defects were found and fixed while writing those tests, each verified to
fail against the previous code:

- Pruning probed the source tree for a mirror file's original name, lowercase
  extensions only. A `.FLAC` source was never found, so its mirror file was
  deleted as an orphan and rebuilt on the next pass, for ever. Pruning now works
  from the set of paths the pass actually accounted for.
- Two sources could claim one mirror path — `01 Song.flac` beside a leftover
  `01 Song.mp3`. Both encoded to the same destination and each pass found the
  loser stale. The better format now wins, ties break on path.
- The source mtime was read after encoding rather than before, so a file still
  being written when the pass reached it could be stamped current while holding
  truncated audio.

## Why there is no flake

The deployment target is a container. A flake here would sit on no path between
the source and the NAS, and `nix flake check` would test against nixpkgs' ffmpeg
while the artefact ships Debian's — precisely the layer these tests exercise. It
was removed in favour of running the suite inside the image. The sibling
`legacy-email-proxy` keeps its flake because there the flake *is* the deployment
mechanism.

## Verification

- 21 tests, all real ffmpeg round-trips: layout, tag survival, MP3 output, skip-when-current, re-encode-on-change, orphan pruning, empty-dir removal, `--no-prune`, MP3 passthrough, `--dry-run`, `--subdir`, external cover art embedding, both refusal paths, and the Lidarr lifecycle cases below.
- The suite also runs in the multi-stage Docker `test` stage: 21 passed. The published `runtime` stage carries neither the tests nor pytest, verified by inspecting the image.
- Container built and run locally against a sample library: correct output path, Cyrillic tags intact.
- The release step was extracted from the workflow and run against a scratch repository: it commits and tags when the version changes, and skips the commit but still tags when `pyproject.toml` already carries it.
- **Not tested against the real library** — the first run there should be `--dry-run`.

## Follow-ups, not in this PR

- Lidarr imports are picked up on the next scheduled pass rather than instantly. A webhook trigger is the obvious next step if six hours feels slow.
- `PACKAGES_SECRET` must exist as a repository secret before the first merge, or the login step fails.

---------

Co-authored-by: Emma Thorpe <emma.thorpe@citrix.com>
Reviewed-on: #1
2026-08-21 15:30:58 +01:00