mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-01 07:21:17 +00:00
fix(artwork): honor disabled per-track art at serve time; use nanosecond mtime provenance
Two serving-correctness fixes from review: - serveMediaFile served a persisted mf embedded image even after EnableMediaFileCoverArt was turned off (the setting isn't in the config fingerprint, so found rows aren't reprocessed). Direct mf- URLs now honor the setting at serve time and fall back to disc/album art. - The file-backed staleness check compared whole-second mtimes, so a same-second content replacement (two writes in one second, or timestamp-preserving tools) could serve different bytes under the old hash + immutable policy. RefMtime is now unix-nanoseconds (no schema change; int64 column), detecting sub-second changes where the filesystem records them.
This commit is contained in:
parent
aa6c0b1f17
commit
aba7ed925c
@ -257,7 +257,7 @@ var _ = Describe("processItem", func() {
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(iaA.Source).To(Equal("folder"))
|
||||
Expect(filepath.ToSlash(iaA.SourcePath)).To(HaveSuffix("album-a/cover.jpg"))
|
||||
Expect(iaA.RefMtime).To(Equal(int64(1000)))
|
||||
Expect(iaA.RefMtime).To(Equal(time.Unix(1000, 0).UnixNano()))
|
||||
|
||||
// Poison the shared row's blurhash: the second item must dedup on hash, not re-decode.
|
||||
poisoned := artRepo.Data[iaA.Hash]
|
||||
@ -270,13 +270,13 @@ var _ = Describe("processItem", func() {
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(iaB.Hash).To(Equal(iaA.Hash))
|
||||
Expect(filepath.ToSlash(iaB.SourcePath)).To(HaveSuffix("album-b/cover.jpg"))
|
||||
Expect(iaB.RefMtime).To(Equal(int64(2000)))
|
||||
Expect(iaB.RefMtime).To(Equal(time.Unix(2000, 0).UnixNano()))
|
||||
|
||||
// The first item's provenance survives the second item processing identical bytes.
|
||||
iaAafter, err := artRepo.GetItemArtwork("al", "alA", model.ImageTypePrimary)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(filepath.ToSlash(iaAafter.SourcePath)).To(HaveSuffix("album-a/cover.jpg"))
|
||||
Expect(iaAafter.RefMtime).To(Equal(int64(1000)))
|
||||
Expect(iaAafter.RefMtime).To(Equal(time.Unix(1000, 0).UnixNano()))
|
||||
|
||||
// One shared artwork row, and dedup preserved it untouched.
|
||||
Expect(artRepo.Data).To(HaveLen(1))
|
||||
|
||||
@ -29,7 +29,7 @@ type resolution struct {
|
||||
reader io.ReadCloser // nil when no source yielded an image
|
||||
source string // model.ItemArtwork.Source value: "folder", "embedded", "external", "upload", "generated"
|
||||
sourcePath string // backing library/upload file (folder/upload: the image; embedded: the audio file); "" otherwise
|
||||
refMtime int64 // mtime of sourcePath at resolution time; 0 when no sourcePath
|
||||
refMtime int64 // sourcePath mtime (unix-nanoseconds) at resolution; 0 when no sourcePath
|
||||
// external source errored/timed out. With no reader: forces failed (never absent).
|
||||
// On a hit: a higher-priority external step failed—serve this, but retry later.
|
||||
extError bool
|
||||
@ -410,7 +410,7 @@ func mtimeOf(path string) int64 {
|
||||
if err != nil {
|
||||
return 0
|
||||
}
|
||||
return info.ModTime().Unix()
|
||||
return info.ModTime().UnixNano()
|
||||
}
|
||||
|
||||
// mtimeViaFS stats through the library FS instead of a joined absolute path,
|
||||
@ -423,7 +423,7 @@ func mtimeViaFS(fsys fs.FS, name string) int64 {
|
||||
if err != nil {
|
||||
return 0
|
||||
}
|
||||
return info.ModTime().Unix()
|
||||
return info.ModTime().UnixNano()
|
||||
}
|
||||
|
||||
// decodeTile and assembleTiles mirror playlistArtworkReader's createTile/
|
||||
|
||||
@ -145,7 +145,7 @@ func openOriginal(ia *model.ItemArtwork, mime string, store *ImageStore) (io.Rea
|
||||
f.Close()
|
||||
return nil, err
|
||||
}
|
||||
if ia.RefMtime != 0 && info.ModTime().Unix() != ia.RefMtime {
|
||||
if ia.RefMtime != 0 && info.ModTime().UnixNano() != ia.RefMtime {
|
||||
f.Close()
|
||||
return nil, errStaleSource
|
||||
}
|
||||
@ -158,7 +158,7 @@ func openOriginal(ia *model.ItemArtwork, mime string, store *ImageStore) (io.Rea
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if info.ModTime().Unix() != ia.RefMtime {
|
||||
if info.ModTime().UnixNano() != ia.RefMtime {
|
||||
return nil, errStaleSource
|
||||
}
|
||||
}
|
||||
@ -232,6 +232,16 @@ func (s *service) serveBytes(ctx context.Context, hash string, data []byte, last
|
||||
// serveMediaFile serves a track: own found art wins; an absent row delegates to the album;
|
||||
// a missing row extracts embedded art (if eligible, enqueuing) else delegates without enqueue.
|
||||
func (s *service) serveMediaFile(ctx context.Context, artID model.ArtworkID, size int, square bool) (*Image, error) {
|
||||
// Per-track art can be disabled after mf rows were resolved (the setting is not in the
|
||||
// config fingerprint). Honor it at serve time so a direct mf- URL falls 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("mf", artID.ID, model.ImageTypePrimary)
|
||||
switch {
|
||||
case err == nil && ia.Hash != "":
|
||||
@ -362,7 +372,7 @@ func unixMtime(mtime int64) time.Time {
|
||||
if mtime <= 0 {
|
||||
return time.Time{}
|
||||
}
|
||||
return time.Unix(mtime, 0)
|
||||
return time.Unix(0, mtime) // RefMtime is unix-nanoseconds
|
||||
}
|
||||
|
||||
// resizedItem is an artworkReader that resizes bytes opened by open() and caches the
|
||||
|
||||
@ -219,6 +219,18 @@ var _ = Describe("Service", func() {
|
||||
Expect(readAll(img)).To(Equal(coverBytes))
|
||||
})
|
||||
|
||||
It("ignores a resolved mf row and delegates to the album when per-track art is disabled", func() {
|
||||
conf.Server.EnableMediaFileCoverArt = false
|
||||
// A resolved mf row exists (from when the setting was on) but must not be served.
|
||||
seedFoundStore("mf", "mf7", []byte("stale embedded track art"))
|
||||
seedFoundStore("al", "albz", coverBytes)
|
||||
mfRepo.SetData(model.MediaFiles{{ID: "mf7", AlbumID: "albz"}})
|
||||
|
||||
img, err := svc.Get(ctx, model.MustParseArtworkID("mf-mf7"), 0, false)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(readAll(img)).To(Equal(coverBytes), "album art, not the persisted embedded art")
|
||||
})
|
||||
|
||||
It("delegates to the album when the track's state is absent", func() {
|
||||
seedFoundStore("al", "albm", coverBytes)
|
||||
Expect(artRepo.PutItemArtwork(&model.ItemArtwork{ItemKind: "mf", ItemID: "mf2"})).To(Succeed())
|
||||
@ -348,5 +360,5 @@ func fileMtime(path string) int64 {
|
||||
GinkgoHelper()
|
||||
info, err := os.Stat(path)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
return info.ModTime().Unix()
|
||||
return info.ModTime().UnixNano()
|
||||
}
|
||||
|
||||
@ -31,7 +31,7 @@ type ItemArtwork struct {
|
||||
Source string `structs:"source"`
|
||||
// SourcePath is the backing file (folder/upload: the image; embedded: the audio file); "" otherwise.
|
||||
SourcePath string `structs:"source_path"`
|
||||
// RefMtime is SourcePath's mtime at resolution; 0 when there is no SourcePath.
|
||||
// RefMtime is SourcePath's mtime (unix-nanoseconds) at resolution; 0 when there is no SourcePath.
|
||||
RefMtime int64 `structs:"ref_mtime"`
|
||||
// attempted_at/updated_at are nullable in the schema but always set by PutItemArtwork;
|
||||
// raw inserts must set them too, since these non-pointer time.Time fields fail to scan NULL.
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user