fix: verify flac integrity, read tags from other formats, ignore appleDouble files
Three things the archive was getting wrong about the files themselves. Every flac is now tested with the reference decoder before it is promoted. ffmpeg exits zero on a truncated flac having decoded nothing at all, so a half-finished download passed the decode check and landed in the archive as a master; flac --test decodes the whole stream and checks it against the MD5 in the file's own header, and costs about a fifth of a second per track. Tags are now read for every format ffprobe understands, not only flac and Ogg. Every mp3 was scanned with no tags at all, so a mixed album looked like a set of untagged files and was refused, and an mp3 album could not resolve its own artist. Files named "\._something.flac" are no longer treated as audio. macOS writes one beside each file it touches on a foreign filesystem, it keeps the audio extension, and it was left behind in staging after everything else had been cleaned up.
diff --git a/internal/audio/audio.go b/internal/audio/audio.go
@@ -4,6 +4,7 @@ import (
"bytes"
"context"
"encoding/json"
+ "errors"
"fmt"
"os/exec"
"path/filepath"
@@ -28,6 +29,13 @@ var Exts = map[string]bool{
}
func IsAudio(name string) bool {
+ // macOS writes an AppleDouble sidecar next to each file it touches on a
+ // foreign filesystem, named "._" plus the original name. It keeps the audio
+ // extension, so by extension alone it looks like a track: it was left behind
+ // in staging after everything else was cleaned up, and it is not audio.
+ if strings.HasPrefix(name, "._") {
+ return false
+ }
return Exts[strings.ToLower(filepath.Ext(name))]
}
@@ -42,6 +50,10 @@ type Track struct {
Codec string
ArtCount int
DecodeError string
+ // DiscHint is the disc number taken from the folder the file sits in, for
+ // rips that put each disc in a CD1 / Disc 2 folder and never tag it. It is
+ // only a fallback: a DISCNUMBER tag or a disc number in the file name wins.
+ DiscHint int
}
func Probe(ctx context.Context, path string) (Track, error) {
@@ -66,6 +78,14 @@ func Probe(ctx context.Context, path string) (Track, error) {
}
t.Tags = notes
t.ArtCount = countPictureComments(notes)
+ default:
+ // mp3 and anything else ffprobe understands. Without this the tags of
+ // every mp3 were ignored entirely, so a mixed album looked like a set of
+ // untagged files and was refused, and a pure mp3 album could not resolve
+ // its own artist or title.
+ if t.Tags == nil {
+ t.Tags = []tags.Note{}
+ }
}
return t, nil
}
@@ -107,14 +127,15 @@ type ffprobeOut struct {
} `json:"disposition"`
} `json:"streams"`
Format struct {
- Duration string `json:"duration"`
+ Duration string `json:"duration"`
+ Tags map[string]string `json:"tags"`
} `json:"format"`
}
func probeStream(ctx context.Context, t *Track) error {
cmd := exec.CommandContext(ctx, "ffprobe",
"-v", "error",
- "-show_entries", "stream=codec_name,sample_rate,channels,bits_per_sample:stream_disposition=attached_pic:format=duration",
+ "-show_entries", "stream=codec_name,sample_rate,channels,bits_per_sample:stream_disposition=attached_pic:format=duration:format_tags",
"-of", "json", t.Path)
var out, errb bytes.Buffer
cmd.Stdout = &out
@@ -146,6 +167,15 @@ func probeStream(ctx context.Context, t *Track) error {
if d, err := strconv.ParseFloat(parsed.Format.Duration, 64); err == nil {
t.Duration = d
}
+ // Kept for formats that have no better reader below. FLAC and Ogg are read
+ // with their own tools because ffprobe does not see their comments reliably.
+ if len(parsed.Format.Tags) > 0 {
+ lines := make([]string, 0, len(parsed.Format.Tags))
+ for k, v := range parsed.Format.Tags {
+ lines = append(lines, k+"="+v)
+ }
+ t.Tags = tags.Parse(lines)
+ }
return nil
}
@@ -201,6 +231,36 @@ func WriteTags(ctx context.Context, path string, lines []string, stripPictures b
return nil
}
+// TestFLAC checks a FLAC file against the reference decoder.
+//
+// ffmpeg is not enough for this. A truncated FLAC, which is what a
+// half-finished download is, makes ffmpeg exit zero having decoded nothing at
+// all, so the file passes a decode check and gets promoted into the archive.
+// flac --test decodes the whole stream and checks it against the MD5 stored in
+// the file's own header, which is what actually catches a partial file. It runs
+// at roughly a thousand times realtime, so it costs about a fifth of a second
+// per track.
+//
+// Returns nil when no flac binary is installed; the caller's decode check still
+// runs, it just cannot see truncation in that case.
+func TestFLAC(ctx context.Context, path string) error {
+ if _, err := exec.LookPath("flac"); err != nil {
+ return nil
+ }
+ cmd := exec.CommandContext(ctx, "flac", "--test", "--silent", path)
+ var errb bytes.Buffer
+ cmd.Stderr = &errb
+ if err := cmd.Run(); err != nil {
+ // flac names the file in its own message, so this does not.
+ msg := strings.TrimSpace(errb.String())
+ if msg == "" {
+ msg = fmt.Sprintf("%s: %v", filepath.Base(path), err)
+ }
+ return errors.New(msg)
+ }
+ return nil
+}
+
func DecodeOK(ctx context.Context, path string) error {
cmd := exec.CommandContext(ctx, "ffmpeg", "-v", "error", "-i", path, "-f", "null", "-")
var errb bytes.Buffer
diff --git a/internal/audio/audio_test.go b/internal/audio/audio_test.go
@@ -0,0 +1,75 @@
+package audio
+
+import (
+ "context"
+ "os"
+ "os/exec"
+ "path/filepath"
+ "testing"
+)
+
+// makeFLAC writes a short tone, and a truncated copy of it. Truncation is the
+// shape a half-finished download arrives in.
+func makeFLAC(t *testing.T, dir string) (good, cut string) {
+ t.Helper()
+ if _, err := exec.LookPath("ffmpeg"); err != nil {
+ t.Skip("ffmpeg is needed to build the fixture")
+ }
+ good = filepath.Join(dir, "good.flac")
+ cmd := exec.Command("ffmpeg", "-hide_banner", "-loglevel", "error", "-y",
+ "-f", "lavfi", "-i", "sine=frequency=440:duration=2", good)
+ if out, err := cmd.CombinedOutput(); err != nil {
+ t.Skipf("could not build a flac: %v %s", err, out)
+ }
+ data, err := os.ReadFile(good)
+ if err != nil {
+ t.Fatal(err)
+ }
+ cut = filepath.Join(dir, "cut.flac")
+ if err := os.WriteFile(cut, data[:len(data)/2], 0o600); err != nil {
+ t.Fatal(err)
+ }
+ return good, cut
+}
+
+// ffmpeg decodes a truncated flac without complaint, so it cannot be what the
+// archive relies on: a partial file would be promoted and only discovered later.
+func TestTestFLACCatchesTruncation(t *testing.T) {
+ if _, err := exec.LookPath("flac"); err != nil {
+ t.Skip("flac is not installed")
+ }
+ good, cut := makeFLAC(t, t.TempDir())
+
+ if err := TestFLAC(context.Background(), good); err != nil {
+ t.Errorf("an intact flac should pass: %v", err)
+ }
+ if err := TestFLAC(context.Background(), cut); err == nil {
+ t.Error("a truncated flac passed the integrity test")
+ }
+
+ // The same file that ffmpeg is happy with.
+ if err := DecodeOK(context.Background(), cut); err != nil {
+ t.Logf("ffmpeg also rejected it: %v", err)
+ } else {
+ t.Log("ffmpeg accepted the truncated file, which is why TestFLAC exists")
+ }
+}
+
+// macOS writes an AppleDouble sidecar beside each file it touches on a foreign
+// filesystem, named "._" plus the original. It keeps the audio extension, so by
+// extension alone it looks like a track, and it was left behind in staging after
+// everything else had been cleaned up.
+func TestAppleDoubleFilesAreNotAudio(t *testing.T) {
+ if IsAudio("._01 track.flac") {
+ t.Error("an AppleDouble sidecar should not count as a track")
+ }
+ if IsAudio("._song.mp3") {
+ t.Error("an AppleDouble sidecar should not count as a track")
+ }
+ if !IsAudio("01 track.flac") {
+ t.Error("a real track should still count")
+ }
+ if !IsAudio("SONG.MP3") {
+ t.Error("the extension check should stay case insensitive")
+ }
+}
diff --git a/internal/execute/execute.go b/internal/execute/execute.go
@@ -186,6 +186,18 @@ func verify(ctx context.Context, p plan.Plan, written []string, mode string, opt
}
}
+ // Every flac is integrity-tested, not only the sampled ones: a partial
+ // download can sit anywhere in the album, and the check costs about a fifth
+ // of a second per track.
+ for _, f := range written {
+ if !strings.EqualFold(filepath.Ext(f), ".flac") {
+ continue
+ }
+ if err := audio.TestFLAC(ctx, f); err != nil {
+ return fmt.Errorf("verification failed: %w", err)
+ }
+ }
+
switch mode {
case "all":
for _, f := range written {