mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-14 21:37:09 +02:00
refactor(artwork): simplify blurhash cleanup review findings
- detect placeholder artwork via the reader's source path instead of comparing against precomputed placeholder hashes (fragile fingerprint, dead artist-placeholder branch) - collapse the three identical UpdateBlurHash repo methods into a shared sqlRepository.updateBlurHash helper
This commit is contained in:
parent
76e1fba067
commit
0b365a0090
6 changed files with 20 additions and 65 deletions
|
|
@ -11,7 +11,6 @@ import (
|
|||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/model/request"
|
||||
"github.com/navidrome/navidrome/resources"
|
||||
)
|
||||
|
||||
// blurHashUpdater keeps stored blurhashes in sync with the artwork actually served. Enqueue is
|
||||
|
|
@ -127,26 +126,23 @@ func (u *blurHashUpdater) computeFromArtwork(ctx context.Context, artID model.Ar
|
|||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
// Bypasses artwork.Get so the worker's own fetch is never re-enqueued.
|
||||
r, err := u.a.cache.Get(ctx, artReader)
|
||||
// Reads straight from the source (not artwork.Get): no re-enqueue, and the returned path
|
||||
// identifies placeholder artwork, which must never be persisted as an entity's blurhash.
|
||||
r, path, err := artReader.Reader(ctx)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
defer r.Close()
|
||||
if path == consts.PlaceholderAlbumArt || path == consts.PlaceholderArtistArt {
|
||||
return "", nil
|
||||
}
|
||||
img, _, err := image.Decode(r)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
b := img.Bounds()
|
||||
x, y := blurhash.Components(b.Dx(), b.Dy())
|
||||
hash, err := blurhash.Encode(img, x, y)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
if _, isPlaceholder := placeholderHashes()[hash]; isPlaceholder {
|
||||
return "", nil
|
||||
}
|
||||
return hash, nil
|
||||
return blurhash.Encode(img, x, y)
|
||||
}
|
||||
|
||||
func (u *blurHashUpdater) persist(ctx context.Context, artID model.ArtworkID, hash string, version time.Time) error {
|
||||
|
|
@ -160,26 +156,3 @@ func (u *blurHashUpdater) persist(ctx context.Context, artID model.ArtworkID, ha
|
|||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// placeholderHashes identifies placeholder bytes by their hash, since cached reads lose the
|
||||
// source path; placeholder artwork must never be persisted as an entity's blurhash.
|
||||
var placeholderHashes = sync.OnceValue(func() map[string]struct{} {
|
||||
hashes := make(map[string]struct{})
|
||||
for _, name := range []string{consts.PlaceholderAlbumArt, consts.PlaceholderArtistArt} {
|
||||
f, err := resources.FS().Open(name)
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
img, _, err := image.Decode(f)
|
||||
_ = f.Close()
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
b := img.Bounds()
|
||||
x, y := blurhash.Components(b.Dx(), b.Dy())
|
||||
if hash, err := blurhash.Encode(img, x, y); err == nil {
|
||||
hashes[hash] = struct{}{}
|
||||
}
|
||||
}
|
||||
return hashes
|
||||
})
|
||||
|
|
|
|||
|
|
@ -59,14 +59,4 @@ var _ = Describe("blurHashUpdater", func() {
|
|||
Expect(func() { u.process(GinkgoT().Context(), model.Album{ID: "missing"}.CoverArtID()) }).ToNot(Panic())
|
||||
})
|
||||
})
|
||||
|
||||
Describe("placeholderHashes", func() {
|
||||
It("computes hashes for the embedded placeholder images", func() {
|
||||
hashes := placeholderHashes()
|
||||
Expect(hashes).To(HaveLen(2))
|
||||
for h := range hashes {
|
||||
Expect(len(h)).To(BeNumerically(">", 6))
|
||||
}
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -213,14 +213,8 @@ func (r *albumRepository) Put(al *model.Album) error {
|
|||
return nil
|
||||
}
|
||||
|
||||
// UpdateBlurHash is a targeted update: a full-row put would race with the scanner. Deliberately
|
||||
// a plain UPDATE with no insert fallback — updating a just-deleted row must be a silent no-op.
|
||||
func (r *albumRepository) UpdateBlurHash(id, blurHash string, artworkUpdatedAt time.Time) error {
|
||||
upd := Update(r.tableName).Where(Eq{"id": id}).
|
||||
Set("blur_hash", blurHash).
|
||||
Set("blur_hash_updated_at", artworkUpdatedAt)
|
||||
_, err := r.executeSQL(upd)
|
||||
return err
|
||||
return r.updateBlurHash(id, blurHash, artworkUpdatedAt)
|
||||
}
|
||||
|
||||
// TODO Move external metadata to a separated table
|
||||
|
|
|
|||
|
|
@ -232,14 +232,8 @@ func (r *artistRepository) Put(a *model.Artist, colsToUpdate ...string) error {
|
|||
return err
|
||||
}
|
||||
|
||||
// UpdateBlurHash is a targeted update: a full-row put would race with the scanner. Deliberately
|
||||
// a plain UPDATE with no insert fallback — updating a just-deleted row must be a silent no-op.
|
||||
func (r *artistRepository) UpdateBlurHash(id, blurHash string, artworkUpdatedAt time.Time) error {
|
||||
upd := Update(r.tableName).Where(Eq{"id": id}).
|
||||
Set("blur_hash", blurHash).
|
||||
Set("blur_hash_updated_at", artworkUpdatedAt)
|
||||
_, err := r.executeSQL(upd)
|
||||
return err
|
||||
return r.updateBlurHash(id, blurHash, artworkUpdatedAt)
|
||||
}
|
||||
|
||||
func (r *artistRepository) UpdateExternalInfo(a *model.Artist) error {
|
||||
|
|
|
|||
|
|
@ -155,14 +155,8 @@ func (r *playlistRepository) GetWithTracks(id string, refreshSmartPlaylist, incl
|
|||
return pls, nil
|
||||
}
|
||||
|
||||
// UpdateBlurHash is a targeted update: a full-row put would race with playlist sync. Deliberately
|
||||
// a plain UPDATE with no insert fallback — updating a just-deleted row must be a silent no-op.
|
||||
func (r *playlistRepository) UpdateBlurHash(id, blurHash string, artworkUpdatedAt time.Time) error {
|
||||
upd := Update(r.tableName).Where(Eq{"id": id}).
|
||||
Set("blur_hash", blurHash).
|
||||
Set("blur_hash_updated_at", artworkUpdatedAt)
|
||||
_, err := r.executeSQL(upd)
|
||||
return err
|
||||
return r.updateBlurHash(id, blurHash, artworkUpdatedAt)
|
||||
}
|
||||
|
||||
func (r *playlistRepository) FindByPath(path string) (*model.Playlist, error) {
|
||||
|
|
|
|||
|
|
@ -293,6 +293,16 @@ func (r sqlRepository) resetSeededRandom(options []model.QueryOptions) {
|
|||
}
|
||||
}
|
||||
|
||||
// updateBlurHash is a targeted update: a full-row put would race with the scanner. Deliberately
|
||||
// a plain UPDATE with no insert fallback — updating a just-deleted row must be a silent no-op.
|
||||
func (r sqlRepository) updateBlurHash(id, blurHash string, artworkUpdatedAt time.Time) error {
|
||||
upd := Update(r.tableName).Where(Eq{"id": id}).
|
||||
Set("blur_hash", blurHash).
|
||||
Set("blur_hash_updated_at", artworkUpdatedAt)
|
||||
_, err := r.executeSQL(upd)
|
||||
return err
|
||||
}
|
||||
|
||||
func (r sqlRepository) executeSQL(sq Sqlizer) (int64, error) {
|
||||
query, args, err := r.toSQL(sq)
|
||||
if err != nil {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue