README.md

@@ -197,6 +197,16 @@ flight; only the two figures that needed the second walk are missing.
 Piped to a log it prints a plain line every thirty seconds instead, with no
 carriage returns, and a summary at the end either way.
 
+### Albums that changed are copied whole
+
+After the main pass, any album that gained or lost a track has the rest of its
+tracks copied again with `--ignore-times`. Their ReplayGain tags were rewritten
+in place when the album was re-levelled, which changes neither their size nor
+their mtime — the only two things rsync compares — so nothing else would ever
+send them. `touched_albums.py` works out the list from rsync's own report of
+what it moved, so this costs no extra traversal, and tracks the main pass has
+already copied are left out of it. See [Volume levelling](#volume-levelling).
+
 ### The Rockbox database
 
 Point `MUSIC_MIRROR_DATABASE_TOOL` at Rockbox's host-side builder and the sync
@@ -471,11 +481,16 @@ one.
 Turn it on at the player end under **Settings → Playback Settings →
 Replaygain**:
 
-| Setting          | Suggested            | Why                                                       |
-| ---------------- | -------------------- | --------------------------------------------------------- |
-| Replaygain type  | `Album Gain`, or `Track Gain if Shuffling` | Keeps a record's own dynamics; the second switches per mode |
-| Prevent clipping | `Yes`                | Uses the peak tags to back off rather than distort         |
-| Pre-amp          | `0 dB`               | The reference level is already −18 LUFS; move it only if the result is too quiet |
+| Setting          | Suggested                    | Why                                                        |
+| ---------------- | ---------------------------- | ---------------------------------------------------------- |
+| Replaygain type  | `Track Gain if Shuffling`    | Album gain while playing a record, track gain once shuffle is on — the only setting that is right in both cases |
+| Prevent clipping | `Yes`                        | Backs the gain off using the peak tags rather than distorting |
+| Pre-amp          | `0 dB`                       | The reference is already −18 LUFS; raise it only if everything ends up too quiet |
+
+`Track Gain if Shuffling` is not a compromise between the other two — Rockbox
+reads the shuffle setting and picks whole-hog album or track gain from it
+(`apps/misc.c`, `replaygain_setting_mode`). It is also Rockbox's default, so on
+a fresh install there may be nothing to change but `Prevent clipping`.
 
 Measuring costs a full decode of every track, so the first pass after enabling
 it takes roughly as long as the original encode did. After that only albums
@@ -488,6 +503,25 @@ Tagging rewrites the file, and staleness here is an mtime comparison, so
 look newer than its source and the next pass would re-encode the entire
 library.
 
+That has a consequence for the sync, and it is not obvious. The first time a
+track is levelled its tag grows by about a kilobyte, so its size changes and
+rsync copies it — the whole library goes across once, and there is no way
+around that; the tag sits at the head of the file and every byte after it
+moves. But `rsgain` leaves padding behind, so a *later* re-level fits inside it
+and changes neither the size nor the mtime:
+
+```
+before re-level: 277757 bytes, mtime 1577880000, album gain 3.75 dB
+after  re-level: 277757 bytes, mtime 1577880000, album gain 6.25 dB
+```
+
+Those are the two things rsync's quick check compares, so it would see nothing
+to do and the device would keep the old gains. `sync-to-ipod.sh` handles it: the
+track that arrived or left is always visible, so any album the main pass
+touched has the rest of its tracks copied again with `--ignore-times`. Albums
+whose file set has not changed are not touched, which is what keeps this from
+being a full re-copy.
+
 ## Getting the result onto an iPod
 
 The mirror is just a directory of MP3s, so any client will do:

tests/test_sync_to_ipod.py

@@ -5,6 +5,7 @@ protecting against emptying the wrong directory -- a mistake that does not
 announce itself.
 """
 
+import os
 import shutil
 import subprocess
 from pathlib import Path
@@ -346,3 +347,88 @@ def test_counting_can_be_asked_for(mirror, tmp_path):
 
     assert result.returncode == 0, result.stderr
     assert "files to copy" in result.stderr
+
+
+# A ReplayGain re-level rewrites a track's tags in the padding the previous
+# write left behind, so neither the size nor the mtime changes -- and those are
+# the two things rsync's quick check compares.
+
+
+def stale_copy(mirror, destination, relative, current, previous):
+    """Put a file on the device that differs only in content from the mirror's."""
+    source = mirror / relative
+    source.parent.mkdir(parents=True, exist_ok=True)
+    source.write_bytes(current)
+    device = destination / relative
+    device.parent.mkdir(parents=True, exist_ok=True)
+    device.write_bytes(previous)
+    os.utime(device, (source.stat().st_atime, source.stat().st_mtime))
+    return device
+
+
+def test_a_tag_only_change_reaches_the_device_with_the_track_that_caused_it(
+    mirror, tmp_path
+):
+    """The new track is visible to rsync; its re-levelled sibling is not, and
+    would otherwise keep the old album gain on the device forever."""
+    destination = tmp_path / "dest"
+    destination.mkdir()
+    sibling = stale_copy(mirror, destination, "Album/sibling.mp3", b"NEW", b"OLD")
+
+    result = run("-f", "-S", "-U", str(mirror), str(destination))
+
+    assert result.returncode == 0, result.stderr
+    assert sibling.read_bytes() == b"NEW"
+    assert (destination / "Album" / "track.mp3").is_file()
+
+
+def test_a_deletion_also_relevels_what_is_left_behind(mirror, tmp_path):
+    destination = tmp_path / "dest"
+    destination.mkdir()
+    sibling = stale_copy(mirror, destination, "Album/sibling.mp3", b"NEW", b"OLD")
+    (destination / "Album" / "gone.mp3").write_bytes(b"old")
+
+    result = run("-f", "-S", "-U", str(mirror), str(destination))
+
+    assert result.returncode == 0, result.stderr
+    assert sibling.read_bytes() == b"NEW"
+    assert not (destination / "Album" / "gone.mp3").exists()
+
+
+def test_an_untouched_album_is_not_copied_again(mirror, tmp_path):
+    """The second pass is scoped to albums that changed. An album whose file
+    set is the same is left where it is, which is the whole point of not
+    running --ignore-times over the library."""
+    destination = tmp_path / "dest"
+    destination.mkdir()
+    quiet = stale_copy(mirror, destination, "Quiet/only.mp3", b"NEW", b"OLD")
+    (destination / "Album").mkdir()
+    shutil.copy2(mirror / "Album" / "track.mp3", destination / "Album" / "track.mp3")
+
+    result = run("-f", "-S", "-U", str(mirror), str(destination))
+
+    assert result.returncode == 0, result.stderr
+    assert quiet.read_bytes() == b"OLD"
+
+
+def test_a_dry_run_says_how_many_extra_tracks_are_involved(mirror, tmp_path):
+    destination = tmp_path / "dest"
+    destination.mkdir()
+    stale_copy(mirror, destination, "Album/sibling.mp3", b"NEW", b"OLD")
+
+    result = run("-f", "-S", "-U", "-n", str(mirror), str(destination))
+
+    assert result.returncode == 0, result.stderr
+    assert "and 1 more in those albums" in result.stderr
+
+
+def test_a_first_sync_does_not_copy_anything_twice(mirror, tmp_path):
+    """Everything is transferred by the main pass, so there is nothing left for
+    the second one and it must not announce itself."""
+    destination = tmp_path / "dest"
+    destination.mkdir()
+
+    result = run("-f", "-S", "-U", str(mirror), str(destination))
+
+    assert result.returncode == 0, result.stderr
+    assert "re-levelled" not in result.stderr

tests/test_touched_albums.py

@@ -0,0 +1,128 @@
+"""A ReplayGain re-level rewrites a track's tags without changing its size or
+its mtime, which is precisely the pair rsync's quick check compares. These
+cover the list that is fed back to rsync to copy those tracks anyway."""
+
+import io
+import subprocess
+import sys
+from pathlib import Path
+
+sys.path.insert(0, str(Path(__file__).resolve().parent.parent / "tools"))
+
+import touched_albums  # noqa: E402
+
+
+def album(root, name, *tracks):
+    directory = root / name
+    directory.mkdir(parents=True, exist_ok=True)
+    for track in tracks:
+        (directory / track).write_bytes(b"x")
+    return directory
+
+
+def listing(mirror, *lines):
+    return touched_albums.remaining(mirror, *touched_albums.touched(list(lines)))
+
+
+def test_the_siblings_of_a_new_track_are_listed(tmp_path):
+    album(tmp_path, "Artist/Album", "01.mp3", "02.mp3", "03.mp3")
+
+    assert listing(tmp_path, "4096 Artist/Album/03.mp3") == [
+        "Artist/Album/01.mp3",
+        "Artist/Album/02.mp3",
+    ]
+
+
+def test_a_track_rsync_just_copied_is_not_copied_twice(tmp_path):
+    """A quality upgrade replaces every track on the record. Listing them again
+    would send the album across twice."""
+    album(tmp_path, "Artist/Album", "01.mp3", "02.mp3")
+
+    assert listing(tmp_path, "4096 Artist/Album/01.mp3", "4096 Artist/Album/02.mp3") == []
+
+
+def test_a_deletion_relevels_what_is_left(tmp_path):
+    album(tmp_path, "Artist/Album", "01.mp3", "02.mp3")
+
+    assert listing(tmp_path, "deleting Artist/Album/03.mp3") == [
+        "Artist/Album/01.mp3",
+        "Artist/Album/02.mp3",
+    ]
+
+
+def test_an_album_deleted_outright_lists_nothing(tmp_path):
+    assert listing(tmp_path, "deleting Artist/Gone/01.mp3") == []
+
+
+def test_untouched_albums_are_left_alone(tmp_path):
+    album(tmp_path, "Artist/Changed", "01.mp3", "02.mp3")
+    album(tmp_path, "Artist/Quiet", "01.mp3", "02.mp3")
+
+    assert listing(tmp_path, "4096 Artist/Changed/01.mp3") == ["Artist/Changed/02.mp3"]
+
+
+def test_a_replaced_cover_is_not_an_album_change(tmp_path):
+    """Only a track can change an album's gains, and covers are replaced often
+    enough that treating one as a re-level would copy records for nothing."""
+    album(tmp_path, "Artist/Album", "01.mp3", "02.mp3")
+
+    assert listing(tmp_path, "17408 Artist/Album/cover.jpg") == []
+
+
+def test_directories_are_not_mistaken_for_tracks(tmp_path):
+    album(tmp_path, "Artist/Album", "01.mp3")
+
+    assert listing(tmp_path, "4096 Artist/Album/", "deleting Artist/Old/") == []
+
+
+def test_rsync_talking_to_the_operator_is_not_a_path(tmp_path):
+    album(tmp_path, "Artist/Album", "01.mp3", "02.mp3")
+
+    assert (
+        listing(
+            tmp_path,
+            "sending incremental file list",
+            "",
+            "sent 1,234 bytes  received 56 bytes  2,580.00 bytes/sec",
+            "total size is 7,890  speedup is 6.12",
+        )
+        == []
+    )
+
+
+def test_a_track_at_the_mirror_root_does_not_pull_in_the_whole_tree(tmp_path):
+    """Nothing writes a mirror this way, but the directory of a root-level file
+    is the root, and recursing from there would be the whole library."""
+    (tmp_path / "loose.mp3").write_bytes(b"x")
+    (tmp_path / "other.mp3").write_bytes(b"x")
+    album(tmp_path, "Artist/Album", "01.mp3")
+
+    assert listing(tmp_path, "4096 loose.mp3") == ["other.mp3"]
+
+
+def test_the_paths_are_written_one_per_line(tmp_path):
+    """They are fed straight back to rsync as --files-from."""
+    album(tmp_path, "Artist/Album", "01.mp3", "02.mp3")
+    out = io.StringIO()
+
+    touched_albums.main(
+        ["--mirror", str(tmp_path)],
+        stream=["4096 Artist/Album/01.mp3"],
+        out=out,
+    )
+
+    assert out.getvalue() == "Artist/Album/02.mp3\n"
+
+
+def test_it_runs_as_a_script(tmp_path):
+    album(tmp_path, "Artist/Album", "01.mp3", "02.mp3")
+
+    completed = subprocess.run(
+        [sys.executable, touched_albums.__file__, "--mirror", str(tmp_path)],
+        input="4096 Artist/Album/01.mp3\n",
+        capture_output=True,
+        text=True,
+    )
+
+    assert completed.returncode == 0
+    assert completed.stdout == "Artist/Album/02.mp3\n"

tools/sync-to-ipod.sh

@@ -56,6 +56,10 @@ The destination must be on a mounted FAT filesystem. That check is also what
 catches an unmounted device: /media/IPOD/Music then resolves to the host's own
 root filesystem, and this refuses to empty that.
 
+An album that gained or lost a track is copied again in full afterwards. Its
+surviving tracks have had their ReplayGain tags rewritten in place, which
+changes neither their size nor their mtime, so the main pass cannot see them.
+
 Progress is one line that rewrites itself, showing the album currently going
 across, how far through the transfer is, the rate, and an estimate of what is
 left. Working the totals out first means a second pass over the tree, which is
@@ -193,8 +197,23 @@ done
 
 printf 'sync-to-ipod: %s -> %s\n' "$mirror" "$destination" >&2
 
+# Both passes below feed their file list through touched_albums.py, which reads
+# rsync's own report of what it moved. --files-from separates paths by newline,
+# so a filename containing one would be read as two -- which is a path FAT32
+# will not take either, and check_fat32.py above has already refused the run
+# unless -f was given to skip it.
+changed=$(mktemp)
+relevelled=$(mktemp)
+trap 'rm -f "$changed" "$relevelled"' EXIT
+
 if $dry_run; then
-    rsync "${options[@]}" --dry-run --verbose "$mirror/" "$destination/"
+    rsync "${options[@]}" --dry-run --verbose --out-format='%l %n' \
+        "$mirror/" "$destination/" | tee "$changed"
+    also=$(python3 "$here/touched_albums.py" --mirror "$mirror" <"$changed" | wc -l)
+    if [ "$also" -gt 0 ]; then
+        printf 'sync-to-ipod: and %s more in those albums, whose ReplayGain tags\n' "$also" >&2
+        printf 'sync-to-ipod: change without changing their size or their mtime\n' >&2
+    fi
     printf 'sync-to-ipod: dry run, nothing was written\n' >&2
     exit 0
 fi
@@ -256,10 +275,33 @@ interrupted() {
 trap interrupted INT TERM
 
 rsync "${options[@]}" --out-format='%l %n' "$mirror/" "$destination/" |
+    tee "$changed" |
     python3 "$here/rsync_progress.py" --total "$total" --bytes "$total_bytes"
 status=${PIPESTATUS[0]}
 [ "$status" -eq 0 ] || die "rsync exited $status"
 
+# A ReplayGain album gain belongs to the whole record, so a track arriving or
+# leaving rewrites the tags on all of its siblings. rsgain fits the new values
+# into the padding its previous write left behind, which changes neither the
+# size nor the mtime -- the only two things the pass above compares. Those
+# tracks are invisible to it, and the device would keep the old gains.
+#
+# The track that arrived or left is visible, though. So every album the pass
+# touched has the rest of its tracks copied again, with --ignore-times to
+# defeat the same quick check. No --delete: the pass above has already settled
+# what should be on the device, and --delete aimed at an explicit file list
+# does not mean what it looks like it means.
+python3 "$here/touched_albums.py" --mirror "$mirror" <"$changed" >"$relevelled"
+if [ -s "$relevelled" ]; then
+    also=$(wc -l <"$relevelled")
+    printf 'sync-to-ipod: re-copying %s tracks whose album was re-levelled\n' "$also" >&2
+    rsync --times --modify-window=2 --whole-file --omit-dir-times --ignore-times \
+        --files-from="$relevelled" --out-format='%l %n' "$mirror/" "$destination/" |
+        python3 "$here/rsync_progress.py" --total "$also"
+    status=${PIPESTATUS[0]}
+    [ "$status" -eq 0 ] || die "the re-levelled tracks failed to copy: rsync exited $status"
+fi
+
 # Rockbox reads its database from .tcd files in .rockbox. Building them here
 # rather than on the device is not just faster: the on-device commit sorts the
 # whole index in whatever memory it can scrape together, and on a large library

tools/touched_albums.py

@@ -0,0 +1,97 @@
+#!/usr/bin/env python3
+"""List the tracks a sync has to copy again because their album was re-levelled.
+
+Reads rsync's `--out-format='%l %n'` output on stdin and writes mirror-relative
+paths on stdout, one per line, for feeding straight back to rsync as
+`--files-from`.
+
+The problem it solves: a ReplayGain album gain is a property of every track on
+the record, so one track arriving or leaving changes the tags on all of its
+siblings. rsgain writes the new values into the padding its previous write left
+behind, which leaves both the file's size and its mtime untouched -- and size
+and mtime are exactly what rsync's quick check compares. It sees nothing to do,
+and the device keeps the old gains.
+
+What rsync always can see is the track that arrived or left. So any album it
+touched has the rest of its tracks copied again, and nothing else does.
+
+Tracks rsync has already dealt with are left out of the list. After a quality
+upgrade that replaces every track on a record, listing them again would send
+the whole album twice.
+"""
+
+import argparse
+import re
+import sys
+from pathlib import Path
+
+# Under --out-format='%l %n' a transfer is reported as the size, a space and
+# the path. A removal is reported as "deleting <path>" whatever the format is.
+# Everything else on the stream is rsync talking to the operator -- the file
+# list preamble, the byte totals -- and is not a path.
+TRANSFER = re.compile(r"^(\d+) (.+)$")
+DELETION = re.compile(r"^deleting (.+)$")
+
+# What the mirror is made of, and so the only thing worth copying again. A
+# cover is not rewritten by a re-level.
+MIRROR_SUFFIX = ".mp3"
+
+
+def touched(lines):
+    """Return the paths rsync transferred and the directories it changed."""
+    transferred = set()
+    directories = set()
+
+    for line in lines:
+        line = line.rstrip("\n")
+        deletion = DELETION.match(line)
+        transfer = None if deletion else TRANSFER.match(line)
+        if deletion:
+            path = deletion.group(1)
+        elif transfer:
+            path = transfer.group(2)
+        else:
+            continue
+        # Only a track can change an album's gains. rsync reports the
+        # directories it creates and removes too, and a cover replaced on its
+        # own is no reason to send the record again.
+        if not path.endswith(MIRROR_SUFFIX):
+            continue
+        if transfer:
+            transferred.add(path)
+        directories.add(path.rpartition("/")[0])
+
+    return transferred, directories
+
+
+def remaining(mirror, transferred, directories):
+    """Return the tracks in those directories that rsync has not just copied."""
+    paths = []
+    for directory in sorted(directories):
+        album = mirror / directory if directory else mirror
+        if not album.is_dir():
+            # Removed along with the last of its tracks. Nothing to copy.
+            continue
+        for track in sorted(album.glob(f"*{MIRROR_SUFFIX}")):
+            relative = f"{directory}/{track.name}" if directory else track.name
+            if relative not in transferred:
+                paths.append(relative)
+    return paths
+
+
+def main(argv=None, stream=None, out=None):
+    parser = argparse.ArgumentParser(
+        prog="touched_albums.py",
+        description="List the tracks to copy again after an album was re-levelled.",
+    )
+    parser.add_argument("--mirror", required=True, type=Path, help="root of the MP3 mirror")
+    args = parser.parse_args(argv)
+
+    transferred, directories = touched(stream if stream is not None else sys.stdin)
+    for path in remaining(args.mirror, transferred, directories):
+        print(path, file=out if out is not None else sys.stdout)
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())