Remove current artwork implementation

model/album.go

@@ -7,8 +7,8 @@ type Album struct {
 
 	ID                   string    `structs:"id" json:"id"            orm:"column(id)"`
 	Name                 string    `structs:"name" json:"name"`
-	CoverArtPath         string    `structs:"cover_art_path" json:"coverArtPath"`
-	CoverArtId           string    `structs:"cover_art_id" json:"coverArtId"`
+	EmbedArtPath         string    `structs:"cover_art_path" json:"coverArtPath"`
+	EmbedArtId           string    `structs:"cover_art_id" json:"coverArtId"`
 	ArtistID             string    `structs:"artist_id" json:"artistId"      orm:"column(artist_id)"`
 	Artist               string    `structs:"artist" json:"artist"`
 	AlbumArtistID        string    `structs:"album_artist_id" json:"albumArtistId" orm:"column(album_artist_id)"`
@@ -39,6 +39,10 @@ type Album struct {
 	UpdatedAt            time.Time `structs:"updated_at" json:"updatedAt"`
 }
 
+func (a Album) CoverArtID() ArtworkID {
+	return artworkIDFromAlbum(a)
+}
+
 type (
 	Albums []Album
 	DiscID struct {

model/artwork_id.go

@@ -0,0 +1,61 @@
+package model
+
+import (
+	"errors"
+	"fmt"
+	"strconv"
+	"strings"
+	"time"
+)
+
+type Kind struct{ prefix string }
+
+var (
+	KindMediaFileArtwork = Kind{"mf"}
+	KindAlbumArtwork     = Kind{"al"}
+)
+
+type ArtworkID struct {
+	Kind       Kind
+	ID         string
+	LastAccess time.Time
+}
+
+func (id ArtworkID) String() string {
+	return fmt.Sprintf("%s-%s-%x", id.Kind.prefix, id.ID, id.LastAccess.Unix())
+}
+
+func ParseArtworkID(id string) (ArtworkID, error) {
+	parts := strings.Split(id, "-")
+	if len(parts) != 3 {
+		return ArtworkID{}, errors.New("invalid artwork id")
+	}
+	lastUpdate, err := strconv.ParseInt(parts[2], 16, 64)
+	if err != nil {
+		return ArtworkID{}, err
+	}
+	if parts[0] != KindAlbumArtwork.prefix && parts[0] != KindMediaFileArtwork.prefix {
+		return ArtworkID{}, errors.New("invalid artwork kind")
+	}
+	return ArtworkID{
+		Kind:       Kind{parts[0]},
+		ID:         parts[1],
+		LastAccess: time.Unix(lastUpdate, 0),
+	}, nil
+}
+
+func artworkIDFromAlbum(al Album) ArtworkID {
+	return ArtworkID{
+		Kind:       KindAlbumArtwork,
+		ID:         al.ID,
+		LastAccess: al.UpdatedAt,
+	}
+}
+
+func artworkIDFromMediaFile(mf MediaFile) ArtworkID {
+	return ArtworkID{
+		Kind:       KindMediaFileArtwork,
+		ID:         mf.ID,
+		LastAccess: mf.UpdatedAt,
+	}
+}

model/artwork_id_test.go

@@ -0,0 +1,34 @@
+package model_test
+
+import (
+	"time"
+
+	"github.com/navidrome/navidrome/model"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("ParseArtworkID()", func() {
+	It("parses album artwork ids", func() {
+		id, err := model.ParseArtworkID("al-1234-ff")
+		Expect(err).ToNot(HaveOccurred())
+		Expect(id.Kind).To(Equal(model.KindAlbumArtwork))
+		Expect(id.ID).To(Equal("1234"))
+		Expect(id.LastAccess).To(Equal(time.Unix(255, 0)))
+	})
+	It("parses media file artwork ids", func() {
+		id, err := model.ParseArtworkID("mf-a6f8d2b1-ffff")
+		Expect(err).ToNot(HaveOccurred())
+		Expect(id.Kind).To(Equal(model.KindMediaFileArtwork))
+		Expect(id.ID).To(Equal("a6f8d2b1"))
+		Expect(id.LastAccess).To(Equal(time.Unix(65535, 0)))
+	})
+	It("fails to parse malformed ids", func() {
+		_, err := model.ParseArtworkID("a6f8d2b1")
+		Expect(err).To(MatchError("invalid artwork id"))
+	})
+	It("fails to parse ids with invalid kind", func() {
+		_, err := model.ParseArtworkID("xx-a6f8d2b1-ff")
+		Expect(err).To(MatchError("invalid artwork kind"))
+	})
+})

model/mediafile.go

@@ -2,7 +2,6 @@ package model
 
 import (
 	"mime"
-	"os"
 	"path/filepath"
 	"strings"
 	"time"
@@ -65,10 +64,19 @@ type MediaFile struct {
 	UpdatedAt            time.Time `structs:"updated_at" json:"updatedAt"` // Time of file last update (mtime)
 }
 
-func (mf *MediaFile) ContentType() string {
+func (mf MediaFile) ContentType() string {
 	return mime.TypeByExtension("." + mf.Suffix)
 }
 
+func (mf MediaFile) CoverArtID() ArtworkID {
+	// If it is a mediaFile, and it has cover art, return it (if feature is disabled, skip)
+	if mf.HasCoverArt && !conf.Server.DevFastAccessCoverArt {
+		return artworkIDFromMediaFile(mf)
+	}
+	// if the mediaFile does not have a coverArt, fallback to the album cover
+	return artworkIDFromAlbum(Album{ID: mf.AlbumID, UpdatedAt: mf.UpdatedAt})
+}
+
 type MediaFiles []MediaFile
 
 func (mfs MediaFiles) Dirs() []string {
@@ -88,7 +96,6 @@ func (mfs MediaFiles) ToAlbum() Album {
 	var songArtistIds []string
 	var mbzAlbumIds []string
 	var comments []string
-	var firstPath string
 	for _, m := range mfs {
 		// We assume these attributes are all the same for all songs on an album
 		a.ID = m.AlbumID
@@ -128,12 +135,9 @@ func (mfs MediaFiles) ToAlbum() Album {
 			m.Album, m.AlbumArtist, m.Artist,
 			m.SortAlbumName, m.SortAlbumArtistName, m.SortArtistName,
 			m.DiscSubtitle)
-		if m.HasCoverArt {
-			a.CoverArtId = m.ID
-			a.CoverArtPath = m.Path
-		}
-		if firstPath == "" {
-			firstPath = m.Path
+		if m.HasCoverArt && a.EmbedArtId == "" {
+			a.EmbedArtId = m.ID
+			a.EmbedArtPath = m.Path
 		}
 	}
 	comments = slices.Compact(comments)
@@ -150,13 +154,6 @@ func (mfs MediaFiles) ToAlbum() Album {
 	a.AllArtistIDs = strings.Join(slices.Compact(songArtistIds), " ")
 	a.MbzAlbumID = slice.MostFrequent(mbzAlbumIds)
 
-	if a.CoverArtPath == "" || !strings.HasPrefix(conf.Server.CoverArtPriority, "embedded") {
-		if path := getCoverFromPath(firstPath, a.CoverArtPath); path != "" {
-			a.CoverArtId = "al-" + a.ID
-			a.CoverArtPath = path
-		}
-	}
-
 	return a
 }
 
@@ -194,44 +191,6 @@ func fixAlbumArtist(a Album, albumArtistIds []string) Album {
 	return a
 }
 
-// GetCoverFromPath accepts a path to a file, and returns a path to an eligible cover image from the
-// file's directory (as configured with CoverArtPriority). If no cover file is found, among
-// available choices, or an error occurs, an empty string is returned. If HasEmbeddedCover is true,
-// and 'embedded' is matched among eligible choices, GetCoverFromPath will return early with an
-// empty path.
-// TODO: Move to scanner (or at least out of here)
-func getCoverFromPath(mediaPath string, embeddedPath string) string {
-	n, err := os.Open(filepath.Dir(mediaPath))
-	if err != nil {
-		return ""
-	}
-
-	defer n.Close()
-	names, err := n.Readdirnames(-1)
-	if err != nil {
-		return ""
-	}
-
-	for _, p := range strings.Split(conf.Server.CoverArtPriority, ",") {
-		pat := strings.ToLower(strings.TrimSpace(p))
-		if pat == "embedded" {
-			if embeddedPath != "" {
-				return ""
-			}
-			continue
-		}
-
-		for _, name := range names {
-			match, _ := filepath.Match(pat, strings.ToLower(name))
-			if match && utils.IsImageFile(name) {
-				return filepath.Join(filepath.Dir(mediaPath), name)
-			}
-		}
-	}
-
-	return ""
-}
-
 type MediaFileRepository interface {
 	CountAll(options ...QueryOptions) (int64, error)
 	Exists(id string) (bool, error)

model/mediafile_internal_test.go

@@ -1,10 +1,6 @@
 package model
 
 import (
-	"os"
-	"path/filepath"
-
-	"github.com/navidrome/navidrome/conf"
 	"github.com/navidrome/navidrome/consts"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -55,57 +51,3 @@ var _ = Describe("fixAlbumArtist", func() {
 		})
 	})
 })
-
-var _ = Describe("getCoverFromPath", func() {
-	var testFolder, testPath, embeddedPath string
-	BeforeEach(func() {
-		testFolder, _ = os.MkdirTemp("", "album_persistence_tests")
-		if err := os.MkdirAll(testFolder, 0777); err != nil {
-			panic(err)
-		}
-		if _, err := os.Create(filepath.Join(testFolder, "Cover.jpeg")); err != nil {
-			panic(err)
-		}
-		if _, err := os.Create(filepath.Join(testFolder, "FRONT.PNG")); err != nil {
-			panic(err)
-		}
-		testPath = filepath.Join(testFolder, "somefile.test")
-		embeddedPath = filepath.Join(testFolder, "somefile.mp3")
-	})
-	AfterEach(func() {
-		_ = os.RemoveAll(testFolder)
-	})
-
-	It("returns audio file for embedded cover", func() {
-		conf.Server.CoverArtPriority = "embedded, cover.*, front.*"
-		Expect(getCoverFromPath(testPath, embeddedPath)).To(Equal(""))
-	})
-
-	It("returns external file when no embedded cover exists", func() {
-		conf.Server.CoverArtPriority = "embedded, cover.*, front.*"
-		Expect(getCoverFromPath(testPath, "")).To(Equal(filepath.Join(testFolder, "Cover.jpeg")))
-	})
-
-	It("returns embedded cover even if not first choice", func() {
-		conf.Server.CoverArtPriority = "something.png, embedded, cover.*, front.*"
-		Expect(getCoverFromPath(testPath, embeddedPath)).To(Equal(""))
-	})
-
-	It("returns first correct match case-insensitively", func() {
-		conf.Server.CoverArtPriority = "embedded, cover.jpg, front.svg, front.png"
-		Expect(getCoverFromPath(testPath, "")).To(Equal(filepath.Join(testFolder, "FRONT.PNG")))
-	})
-
-	It("returns match for embedded pattern", func() {
-		conf.Server.CoverArtPriority = "embedded, cover.jp?g, front.png"
-		Expect(getCoverFromPath(testPath, "")).To(Equal(filepath.Join(testFolder, "Cover.jpeg")))
-	})
-
-	It("returns empty string if no match was found", func() {
-		conf.Server.CoverArtPriority = "embedded, cover.jpg, front.apng"
-		Expect(getCoverFromPath(testPath, "")).To(Equal(""))
-	})
-
-	// Reset configuration to default.
-	conf.Server.CoverArtPriority = "embedded, cover.*, front.*"
-})

model/mediafile_test.go

@@ -3,6 +3,8 @@ package model_test
 import (
 	"time"
 
+	"github.com/navidrome/navidrome/conf"
+	"github.com/navidrome/navidrome/conf/configtest"
 	. "github.com/navidrome/navidrome/model"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -26,7 +28,7 @@ var _ = Describe("MediaFiles", func() {
 					SortAlbumName: "SortAlbumName", SortArtistName: "SortArtistName", SortAlbumArtistName: "SortAlbumArtistName",
 					OrderAlbumName: "OrderAlbumName", OrderArtistName: "OrderArtistName", OrderAlbumArtistName: "OrderAlbumArtistName",
 					MbzAlbumArtistID: "MbzAlbumArtistID", MbzAlbumType: "MbzAlbumType", MbzAlbumComment: "MbzAlbumComment",
-					Compilation: true, CatalogNum: "CatalogNum", HasCoverArt: true,
+					Compilation: true, CatalogNum: "CatalogNum", HasCoverArt: true, Path: "/music/file.mp3",
 				},
 			}
 		})
@@ -49,7 +51,8 @@ var _ = Describe("MediaFiles", func() {
 			Expect(album.MbzAlbumComment).To(Equal("MbzAlbumComment"))
 			Expect(album.CatalogNum).To(Equal("CatalogNum"))
 			Expect(album.Compilation).To(BeTrue())
-			Expect(album.CoverArtId).To(Equal("2"))
+			Expect(album.EmbedArtId).To(Equal("2"))
+			Expect(album.EmbedArtPath).To(Equal("/music/file.mp3"))
 		})
 	})
 	Context("Aggregated attributes", func() {
@@ -220,6 +223,34 @@ var _ = Describe("MediaFiles", func() {
 	})
 })
 
+var _ = Describe("MediaFile", func() {
+	BeforeEach(func() {
+		DeferCleanup(configtest.SetupConfig())
+		conf.Server.DevFastAccessCoverArt = false
+	})
+	Describe(".CoverArtId()", func() {
+		It("returns its own id if it HasCoverArt", func() {
+			mf := MediaFile{ID: "111", AlbumID: "1", HasCoverArt: true}
+			id := mf.CoverArtID()
+			Expect(id.Kind).To(Equal(KindMediaFileArtwork))
+			Expect(id.ID).To(Equal(mf.ID))
+		})
+		It("returns its album id if HasCoverArt is false", func() {
+			mf := MediaFile{ID: "111", AlbumID: "1", HasCoverArt: false}
+			id := mf.CoverArtID()
+			Expect(id.Kind).To(Equal(KindAlbumArtwork))
+			Expect(id.ID).To(Equal(mf.AlbumID))
+		})
+		It("returns its album id if DevFastAccessCoverArt is enabled", func() {
+			conf.Server.DevFastAccessCoverArt = true
+			mf := MediaFile{ID: "111", AlbumID: "1", HasCoverArt: true}
+			id := mf.CoverArtID()
+			Expect(id.Kind).To(Equal(KindAlbumArtwork))
+			Expect(id.ID).To(Equal(mf.AlbumID))
+		})
+	})
+})
+
 func t(v string) time.Time {
 	var timeFormats = []string{"2006-01-02", "2006-01-02 15:04", "2006-01-02 15:04:05", "2006-01-02T15:04:05", "2006-01-02T15:04", "2006-01-02 15:04:05.999999999 -0700 MST"}
 	for _, f := range timeFormats {