fix: do not offer files that are not images as cover art
A text file called cover.jpg was offered as the only folder image, chosen automatically because folder art wins, and then failed the entire album when embedding it produced "unable to guess MIME type". A file that cannot be identified as an image cannot be a cover, so it is no longer a candidate. The album now files with a placeholder instead of dying.
diff --git a/internal/artwork/artwork_test.go b/internal/artwork/artwork_test.go
@@ -1,6 +1,12 @@
package artwork
-import "testing"
+import (
+ "context"
+ "os"
+ "os/exec"
+ "path/filepath"
+ "testing"
+)
func TestUpscaleArtwork(t *testing.T) {
cases := []struct{ in, want string }{
@@ -112,3 +118,26 @@ func TestSortByYearMatch(t *testing.T) {
t.Errorf("the other year match should be second, got %s", releases[1].ID)
}
}
+
+// A junk file named cover.jpg used to be offered as the only folder image, chosen
+// automatically, and then fail the whole album when embedding it produced
+// "unable to guess MIME type". Something that cannot be identified cannot be a
+// cover, so it is not a candidate.
+func TestFolderCandidatesSkipFilesThatAreNotImages(t *testing.T) {
+ if _, err := exec.LookPath("magick"); err != nil {
+ t.Skip("magick is what decides whether a file is an image")
+ }
+ dir := t.TempDir()
+ if err := os.WriteFile(filepath.Join(dir, "cover.jpg"), []byte("not an image at all"), 0o600); err != nil {
+ t.Fatal(err)
+ }
+ savePNG(t, filepath.Join(dir, "front.png"), picture(200, 1))
+
+ got := folderCandidates(context.Background(), dir, dir)
+ if len(got) != 1 {
+ t.Fatalf("got %d candidates, want only the real image", len(got))
+ }
+ if filepath.Base(got[0].Path) != "front.png" {
+ t.Errorf("kept %q, which is not an image", filepath.Base(got[0].Path))
+ }
+}
diff --git a/internal/artwork/local.go b/internal/artwork/local.go
@@ -74,7 +74,9 @@ func embeddedCandidates(ctx context.Context, tracks []audio.Track) []Candidate {
EmbeddedCount: 1,
MatchScore: 3,
}
- fillDimensions(ctx, &c)
+ if !fillDimensions(ctx, &c) {
+ continue
+ }
seen[key] = &entry{candidate: c, count: 1}
order = append(order, key)
}
@@ -145,7 +147,9 @@ func folderCandidates(ctx context.Context, dir, albumRoot string) []Candidate {
Bytes: info.Size(),
MatchScore: 3,
}
- fillDimensions(ctx, &c)
+ if !fillDimensions(ctx, &c) {
+ continue
+ }
out = append(out, c)
}
sortFolderCandidates(out, filepath.Base(dir))
@@ -207,7 +211,9 @@ func localCandidates(ctx context.Context, folder string, album AlbumInfo) []Cand
Bytes: info.Size(),
MatchScore: 3,
}
- fillDimensions(ctx, &c)
+ if !fillDimensions(ctx, &c) {
+ continue
+ }
out = append(out, c)
}
return out
@@ -226,24 +232,39 @@ func sanitizeForMatch(s string) string {
return strings.Trim(b.String(), "-")
}
-func fillDimensions(ctx context.Context, c *Candidate) {
+// fillDimensions records an image's size, and reports whether the file is an
+// image that can be used at all.
+//
+// This is the only thing standing between a junk file and a broken album: a text
+// file called cover.jpg used to be offered as a candidate, chosen because it was
+// the only folder image, and then fail the album when embedding it produced
+// "unable to guess MIME type". Something magick cannot identify cannot be a
+// cover.
+func fillDimensions(ctx context.Context, c *Candidate) bool {
if info, err := os.Stat(c.Path); err == nil {
c.Bytes = info.Size()
}
+ if _, err := exec.LookPath("magick"); err != nil {
+ // Without it nothing can be measured, so the file is given the benefit
+ // of the doubt and the optimiser reports the real problem instead.
+ return true
+ }
cmd := exec.CommandContext(ctx, "magick", "identify", "-format", "%w %h", c.Path)
var out bytes.Buffer
cmd.Stdout = &out
cmd.Stderr = &bytes.Buffer{}
if err := cmd.Run(); err != nil {
- return
+ return false
}
fields := strings.Fields(out.String())
if len(fields) != 2 {
- return
+ return false
}
w, err1 := strconv.Atoi(fields[0])
h, err2 := strconv.Atoi(fields[1])
- if err1 == nil && err2 == nil {
- c.Width, c.Height = w, h
+ if err1 != nil || err2 != nil {
+ return false
}
+ c.Width, c.Height = w, h
+ return true
}
diff --git a/internal/artwork/providers.go b/internal/artwork/providers.go
@@ -166,7 +166,9 @@ func coverArtArchive(ctx context.Context, album AlbumInfo, opts Options) ([]Cand
URL: u,
Bytes: sizeBytes,
}
- fillDimensions(ctx, &c)
+ if !fillDimensions(ctx, &c) {
+ continue
+ }
out = append(out, c)
break
}
@@ -260,7 +262,9 @@ func deezer(ctx context.Context, album AlbumInfo, opts Options) ([]Candidate, er
URL: cover,
Bytes: size,
}
- fillDimensions(ctx, &c)
+ if !fillDimensions(ctx, &c) {
+ continue
+ }
out = append(out, c)
if len(out) >= 2 {
break
@@ -317,7 +321,9 @@ func itunes(ctx context.Context, album AlbumInfo, opts Options) ([]Candidate, er
URL: big,
Bytes: size,
}
- fillDimensions(ctx, &c)
+ if !fillDimensions(ctx, &c) {
+ continue
+ }
out = append(out, c)
if len(out) >= 2 {
break