mirror of
https://github.com/navidrome/navidrome.git
synced 2026-11-12 13:18:40 +01:00
feat(artwork): advertise album art for tracks whose embedded cover is the album's
Most albums embed the same picture in every track, so clients requesting per-track cover art (mf- ids) download and cache the same image once per song. This change hashes the embedded picture at scan time (XXH3 of the bytes, the same digest the artwork pipeline uses) and stores it on media_file.embed_art_hash. ToAlbum stores the hash of the track it already picks as the album's embedded cover (EmbedArtPath) on album.embed_art_hash. A new MediaFile.HasOwnCoverArt replaces the scattered "HasCoverArt && EnableMediaFileCoverArt" checks: a track whose hash equals its album's now returns the disc/album artwork id from CoverArtID, so the Subsonic, Jellyfin and native responses point every matching song at one cached image. Serving a stale mf- URL follows the same rule, so what is advertised and what is served agree. Tracks with a different embedded picture (compilations, per-track art) keep their own ids. Rows scanned before this change have an empty hash and keep the previous behavior until a full scan; no config option is added. The album hash is hydrated onto tracks with one batched PK lookup per page, only when the page has hashed tracks. BestImageIndex is exported from core/artwork so the scanner hashes the same picture the resolver serves.
This commit is contained in:
parent
1ceb25c6c1
commit
af984bc817
21 changed files with 280 additions and 82 deletions
|
|
@ -256,37 +256,27 @@ func (s *service) serveResolution(ctx context.Context, res resolution, size int,
|
|||
}
|
||||
|
||||
func (s *service) serveMediaFile(ctx context.Context, artID model.ArtworkID, size int, square bool) (*Image, error) {
|
||||
// The setting is not in the config fingerprint, so honor it at serve time: a direct mf- URL
|
||||
// must fall back to disc/album instead of serving stale persisted embedded art.
|
||||
if !conf.Server.EnableMediaFileCoverArt {
|
||||
mf, err := s.ds.MediaFile(ctx).Get(artID.ID)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return s.Get(ctx, mf.DiscCoverArtID(), size, square)
|
||||
}
|
||||
ia, err := s.ds.Artwork(ctx).GetItemArtwork(model.KindMediaFileArtwork, artID.ID, model.ImageTypePrimary)
|
||||
switch {
|
||||
case err == nil && ia.Hash != "":
|
||||
return s.serveHash(ctx, artID, ia, size, square)
|
||||
case err == nil:
|
||||
// absent row: fall through
|
||||
case errors.Is(err, model.ErrNotFound):
|
||||
// no row: fall through
|
||||
default:
|
||||
return nil, err
|
||||
}
|
||||
noRow := errors.Is(err, model.ErrNotFound)
|
||||
|
||||
mf, err := s.ds.MediaFile(ctx).Get(artID.ID)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if noRow && conf.Server.EnableMediaFileCoverArt && mf.HasCoverArt {
|
||||
return s.provisionalEmbedded(ctx, artID, *mf, size, square)
|
||||
// Decided at serve time so a stale mf- URL answers with what the track advertises now, not with
|
||||
// persisted embedded art (EnableMediaFileCoverArt is not in the config fingerprint).
|
||||
if !mf.HasOwnCoverArt() {
|
||||
return s.Get(ctx, mf.DiscCoverArtID(), size, square)
|
||||
}
|
||||
ia, err := s.ds.Artwork(ctx).GetItemArtwork(model.KindMediaFileArtwork, artID.ID, model.ImageTypePrimary)
|
||||
switch {
|
||||
case errors.Is(err, model.ErrNotFound):
|
||||
return s.provisionalEmbedded(ctx, artID, *mf, size, square)
|
||||
case err != nil:
|
||||
return nil, err
|
||||
case ia.Hash == "":
|
||||
// Settled absent: mirror CoverArtID's fallback rather than serving a placeholder.
|
||||
return s.Get(ctx, mf.DiscCoverArtID(), size, square)
|
||||
default:
|
||||
return s.serveHash(ctx, artID, ia, size, square)
|
||||
}
|
||||
// Mirror MediaFile.CoverArtID: a track defers to its disc art, which falls back to the album.
|
||||
return s.Get(ctx, mf.DiscCoverArtID(), size, square)
|
||||
}
|
||||
|
||||
// provisionalEmbedded serves a track's embedded art immediately, leaving the state row to the worker.
|
||||
|
|
|
|||
|
|
@ -244,12 +244,24 @@ var _ = Describe("Artwork", func() {
|
|||
Describe("media file", func() {
|
||||
It("serves a track's own found art", func() {
|
||||
seedFoundStore("mf", "mf1", coverBytes)
|
||||
mfRepo.SetData(model.MediaFiles{{ID: "mf1", AlbumID: "alba", HasCoverArt: true}})
|
||||
|
||||
img, err := svc.Get(ctx, model.MustParseArtworkID("mf-mf1"), 0, false)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(readAll(img)).To(Equal(coverBytes))
|
||||
})
|
||||
|
||||
It("ignores a resolved mf row and delegates to the album when the track's picture is the album's cover", func() {
|
||||
seedFoundStore("mf", "mf8", []byte("duplicate embedded track art"))
|
||||
seedFoundStore("al", "alby", coverBytes)
|
||||
mfRepo.SetData(model.MediaFiles{{ID: "mf8", AlbumID: "alby", HasCoverArt: true,
|
||||
EmbedArtHash: "samepicxxxxxxxxx", AlbumEmbedArtHash: "samepicxxxxxxxxx"}})
|
||||
|
||||
img, err := svc.Get(ctx, model.MustParseArtworkID("mf-mf8"), 0, false)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(readAll(img)).To(Equal(coverBytes), "album art, not the persisted embedded art")
|
||||
})
|
||||
|
||||
It("ignores a resolved mf row and delegates to the album when per-track art is disabled", func() {
|
||||
conf.Server.EnableMediaFileCoverArt = false
|
||||
seedFoundStore("mf", "mf7", []byte("stale embedded track art"))
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ import (
|
|||
"github.com/navidrome/navidrome/conf"
|
||||
"github.com/navidrome/navidrome/consts"
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/zeebo/xxh3"
|
||||
)
|
||||
|
||||
|
|
@ -50,7 +51,7 @@ func hashImage(r io.Reader) (string, error) {
|
|||
if _, err := io.Copy(d, r); err != nil {
|
||||
return "", err
|
||||
}
|
||||
return fmt.Sprintf("%016x", d.Sum64()), nil
|
||||
return model.FormatImageHash(d.Sum64()), nil
|
||||
}
|
||||
|
||||
// validHash guards path sharding: a malformed hash would slice-panic or inject path separators.
|
||||
|
|
|
|||
|
|
@ -55,13 +55,6 @@ func fromExternalFile(ctx context.Context, libFS fs.FS, files []string, pattern
|
|||
}
|
||||
}
|
||||
|
||||
// These regexes are used to match the picture type in the file, in the order they are listed.
|
||||
var picTypeRegexes = []*regexp.Regexp{
|
||||
regexp.MustCompile(`(?i).*cover.*front.*|.*front.*cover.*`),
|
||||
regexp.MustCompile(`(?i).*front.*`),
|
||||
regexp.MustCompile(`(?i).*cover.*`),
|
||||
}
|
||||
|
||||
func fromTag(ctx context.Context, libFS fs.FS, relPath string) sourceFunc {
|
||||
return func() (io.ReadCloser, string, error) {
|
||||
if relPath == "" {
|
||||
|
|
@ -93,7 +86,8 @@ func fromTag(ctx context.Context, libFS fs.FS, relPath string) sourceFunc {
|
|||
return nil, "", fmt.Errorf("no embedded image found in %s", relPath)
|
||||
}
|
||||
|
||||
imageIndex := findBestImageIndex(ctx, images, relPath)
|
||||
imageIndex := BestImageIndex(images)
|
||||
log.Trace(ctx, "Artwork: Using embedded image", "type", images[imageIndex].Type, "path", relPath)
|
||||
data, err := tf.Image(imageIndex)
|
||||
if err != nil || len(data) == 0 {
|
||||
return nil, "", fmt.Errorf("could not load embedded image from %s", relPath)
|
||||
|
|
@ -102,16 +96,23 @@ func fromTag(ctx context.Context, libFS fs.FS, relPath string) sourceFunc {
|
|||
}
|
||||
}
|
||||
|
||||
func findBestImageIndex(ctx context.Context, images []taglib.ImageDesc, path string) int {
|
||||
// Ranks embedded picture types, front covers first.
|
||||
var picTypeRegexes = []*regexp.Regexp{
|
||||
regexp.MustCompile(`(?i).*cover.*front.*|.*front.*cover.*`),
|
||||
regexp.MustCompile(`(?i).*front.*`),
|
||||
regexp.MustCompile(`(?i).*cover.*`),
|
||||
}
|
||||
|
||||
// BestImageIndex returns the index of the embedded picture to serve as cover art, defaulting to the
|
||||
// first. The scanner hashes the same pick, so a track's embed_art_hash matches what is served.
|
||||
func BestImageIndex(images []taglib.ImageDesc) int {
|
||||
for _, regex := range picTypeRegexes {
|
||||
for i, img := range images {
|
||||
if regex.MatchString(img.Type) {
|
||||
log.Trace(ctx, "Artwork: Found embedded image", "type", img.Type, "path", path)
|
||||
return i
|
||||
}
|
||||
}
|
||||
}
|
||||
log.Trace(ctx, "Artwork: Could not find a front image. Getting the first one", "type", images[0].Type, "path", path)
|
||||
return 0
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ import (
|
|||
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
"go.senan.xyz/taglib"
|
||||
)
|
||||
|
||||
var _ = Describe("fromExternalFile", func() {
|
||||
|
|
@ -90,3 +91,14 @@ type nonSeekableFile struct{ r *bytes.Reader }
|
|||
func (n *nonSeekableFile) Read(p []byte) (int, error) { return n.r.Read(p) }
|
||||
func (n *nonSeekableFile) Close() error { return nil }
|
||||
func (n *nonSeekableFile) Stat() (fs.FileInfo, error) { return nil, errors.New("not implemented") }
|
||||
|
||||
var _ = Describe("BestImageIndex", func() {
|
||||
It("prefers the front cover over an earlier image", func() {
|
||||
images := []taglib.ImageDesc{{Type: "Back Cover"}, {Type: "Cover (front)"}}
|
||||
Expect(BestImageIndex(images)).To(Equal(1))
|
||||
})
|
||||
It("falls back to the first image", func() {
|
||||
images := []taglib.ImageDesc{{Type: "Artist"}, {Type: "Band"}}
|
||||
Expect(BestImageIndex(images)).To(Equal(0))
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -271,10 +271,12 @@ func (ffs *FakeFS) parseFile(filePath string) (*metadata.Info, error) {
|
|||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
pictureHash, _ := data["picture_hash"].(string)
|
||||
p := metadata.Info{
|
||||
Tags: map[string][]string{},
|
||||
AudioProperties: metadata.AudioProperties{},
|
||||
HasPicture: data["has_picture"] == "true",
|
||||
HasPicture: data["has_picture"] == "true" || pictureHash != "",
|
||||
PictureHash: pictureHash,
|
||||
}
|
||||
if d, ok := data["duration"].(float64); ok {
|
||||
p.AudioProperties.Duration = time.Duration(d) * time.Second
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue