From 890ca64f51b51f663605f22a72e23d11deee0ab1 Mon Sep 17 00:00:00 2001 From: Deluan Date: Mon, 29 Jun 2020 10:50:38 -0400 Subject: [PATCH] Fix `cover.jpg` discovery --- engine/cover.go | 3 +++ persistence/album_repository.go | 14 ++++++++------ persistence/album_repository_test.go | 13 +++++++------ 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/engine/cover.go b/engine/cover.go index 055147495..180da8e8b 100644 --- a/engine/cover.go +++ b/engine/cover.go @@ -89,6 +89,7 @@ func (c *cover) Get(ctx context.Context, id string, size int, out io.Writer) err func (c *cover) getCoverPath(ctx context.Context, id string) (path string, lastUpdated time.Time, err error) { // If id is an album cover ID if strings.HasPrefix(id, "al-") { + log.Trace(ctx, "Looking for album art", "id", id) id = strings.TrimPrefix(id, "al-") var al *model.Album al, err = c.ds.Album(ctx).Get(id) @@ -101,6 +102,7 @@ func (c *cover) getCoverPath(ctx context.Context, id string) (path string, lastU return al.CoverArtPath, al.UpdatedAt, err } + log.Trace(ctx, "Looking for media file art", "id", id) // if id is a mediafile cover id var mf *model.MediaFile mf, err = c.ds.MediaFile(ctx).Get(id) @@ -112,6 +114,7 @@ func (c *cover) getCoverPath(ctx context.Context, id string) (path string, lastU } // if the mediafile does not have a coverArt, fallback to the album cover + log.Trace(ctx, "Media file does not contain art. Falling back to album art", "id", id, "albumId", "al-"+mf.AlbumID) return c.getCoverPath(ctx, "al-"+mf.AlbumID) } diff --git a/persistence/album_repository.go b/persistence/album_repository.go index fb4240ad9..3a4f3e59e 100644 --- a/persistence/album_repository.go +++ b/persistence/album_repository.go @@ -121,11 +121,12 @@ func (r *albumRepository) Refresh(ids ...string) error { SongArtists string Years string DiscSubtitles string + Path string } var albums []refreshAlbum sel := Select(`f.album_id as id, f.album as name, f.artist, f.album_artist, f.artist_id, f.album_artist_id, f.sort_album_name, f.sort_artist_name, f.sort_album_artist_name, - f.order_album_name, f.order_album_artist_name, + f.order_album_name, f.order_album_artist_name, f.path, f.compilation, f.genre, max(f.year) as max_year, sum(f.duration) as duration, count(*) as song_count, a.id as current_id, f2.id as cover_art_id, f2.path as cover_art_path, f2.has_cover_art, @@ -144,12 +145,13 @@ func (r *albumRepository) Refresh(ids ...string) error { toUpdate := 0 for _, al := range albums { if !al.HasCoverArt || !strings.HasPrefix(conf.Server.CoverArtPriority, "embedded") { - if path := getCoverFromPath(al.CoverArtPath, al.HasCoverArt); path != "" { + if path := getCoverFromPath(al.Path, al.CoverArtPath); path != "" { al.CoverArtId = "al-" + al.ID al.CoverArtPath = path } else if !al.HasCoverArt { al.CoverArtId = "" } + log.Trace(r.ctx, "Found album art", "id", al.ID, "name", al.Name, "coverArtPath", al.CoverArtPath, "coverArtId", al.CoverArtId, "hasCoverArt", al.HasCoverArt) } if al.Compilation { @@ -201,8 +203,8 @@ func getMinYear(years string) int { // 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. -func getCoverFromPath(path string, hasEmbeddedCover bool) string { - n, err := os.Open(filepath.Dir(path)) +func getCoverFromPath(mediaPath string, embeddedPath string) string { + n, err := os.Open(filepath.Dir(mediaPath)) if err != nil { return "" } @@ -216,7 +218,7 @@ func getCoverFromPath(path string, hasEmbeddedCover bool) string { for _, p := range strings.Split(conf.Server.CoverArtPriority, ",") { pat := strings.ToLower(strings.TrimSpace(p)) if pat == "embedded" { - if hasEmbeddedCover { + if embeddedPath != "" { return "" } continue @@ -225,7 +227,7 @@ func getCoverFromPath(path string, hasEmbeddedCover bool) string { for _, name := range names { match, _ := filepath.Match(pat, strings.ToLower(name)) if match && utils.IsImageFile(name) { - return filepath.Join(filepath.Dir(path), name) + return filepath.Join(filepath.Dir(mediaPath), name) } } } diff --git a/persistence/album_repository_test.go b/persistence/album_repository_test.go index d0d291745..3f8c3c616 100644 --- a/persistence/album_repository_test.go +++ b/persistence/album_repository_test.go @@ -104,34 +104,35 @@ var _ = Describe("AlbumRepository", func() { } testPath := filepath.Join(testFolder, "somefile.test") + embeddedPath := filepath.Join(testFolder, "somefile.mp3") It("returns audio file for embedded cover", func() { conf.Server.CoverArtPriority = "embedded, cover.*, front.*" - Expect(getCoverFromPath(testPath, true)).To(Equal("")) + 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, false)).To(Equal(filepath.Join(testFolder, "Cover.jpeg"))) + 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, true)).To(Equal("")) + 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, false)).To(Equal(filepath.Join(testFolder, "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, false)).To(Equal(filepath.Join(testFolder, "Cover.jpeg"))) + 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, false)).To(Equal("")) + Expect(getCoverFromPath(testPath, "")).To(Equal("")) }) // Reset configuration to default.