refactor(artwork): compute blurhash from served bytes, drop proxy signals

The worker no longer infers whether the served bytes changed through a stack of
proxy signals (snapshot timestamp vs stored blur_hash_updated_at, freshness guard,
gone bit, idempotent-write skip, computeFromArtwork re-read). It now takes the exact
bytes captured from a serve and is a pure function of them: placeholder clears,
undecodable is left alone, otherwise encode and write with an in-memory last-hash
dedup. Deletion is a checkGone job that re-reads once and clears only if the source
is still gone, so a transient fetch failure can't clobber a valid hash.
This commit is contained in:
Deluan 2026-07-17 20:34:33 -04:00
parent 2169938b30
commit f6dd722119
2 changed files with 184 additions and 248 deletions

View File

@ -17,22 +17,21 @@ import (
"github.com/navidrome/navidrome/resources"
)
// enqueueRequest carries the fill-time snapshot (the reader's LastUpdated, which folds row
// timestamps and live file mtimes into one clock). gone marks a serve that failed with
// ErrUnavailable: the only change no passive signal witnesses, so it clears a stale hash.
type enqueueRequest struct {
snapshot time.Time
gone bool
// blurHashJob is a unit of work: either bytes to hash (data != nil) or a deletion check (checkGone).
type blurHashJob struct {
data []byte
version time.Time
checkGone bool
}
// blurHashUpdater keeps stored blurhashes in sync with the artwork entering the image cache.
// Computation is triggered by cache fills (every change), not serves, so the worker only re-derives
// the hash and skips idempotent writes: Enqueue is a cheap dedup insert, a single worker decodes
// and persists.
// blurHashUpdater keeps stored blurhashes in sync with the bytes actually served. The serve path tees
// the served image and hands it here; there is no change-detection proxy — the hash is a pure function
// of the captured bytes. A single worker decodes, encodes, and writes (dedup'd in memory).
type blurHashUpdater struct {
a *artwork
mutex sync.Mutex
buffer map[model.ArtworkID]enqueueRequest
buffer map[model.ArtworkID]blurHashJob
last map[model.ArtworkID]string // last hash written this process; avoids redundant writes
wake chan struct{}
done chan struct{}
runDone chan struct{}
@ -44,29 +43,35 @@ type blurHashUpdater struct {
func newBlurHashUpdater(a *artwork) *blurHashUpdater {
return &blurHashUpdater{
a: a,
buffer: make(map[model.ArtworkID]enqueueRequest),
buffer: make(map[model.ArtworkID]blurHashJob),
last: make(map[model.ArtworkID]string),
wake: make(chan struct{}, 1),
done: make(chan struct{}),
runDone: make(chan struct{}),
}
}
// Enqueue schedules a recompute for an original-size cache fill, using the reader's snapshot as the
// artwork version. Called on the miss that fills the cache — i.e. exactly when the served bytes change.
func (u *blurHashUpdater) Enqueue(artID model.ArtworkID, snapshot time.Time) {
u.enqueue(artID, enqueueRequest{snapshot: snapshot})
}
// EnqueueGone schedules a clear for a serve that failed with ErrUnavailable, so a stored hash stops
// describing artwork that no longer exists (deletion is invisible to every passive signal).
func (u *blurHashUpdater) EnqueueGone(artID model.ArtworkID) {
u.enqueue(artID, enqueueRequest{gone: true})
}
func (u *blurHashUpdater) enqueue(artID model.ArtworkID, req enqueueRequest) {
func eligibleKind(artID model.ArtworkID) bool {
switch artID.Kind {
case model.KindAlbumArtwork, model.KindArtistArtwork, model.KindPlaylistArtwork:
default:
return true
}
return false
}
// EnqueueBytes schedules a blurhash update computed from the exact bytes served for artID.
func (u *blurHashUpdater) EnqueueBytes(artID model.ArtworkID, data []byte, version time.Time) {
u.enqueue(artID, blurHashJob{data: data, version: version})
}
// EnqueueClearIfGone schedules a deletion check: the worker re-reads the source once and clears the
// stored hash only if it still fails/serves a placeholder, so a transient failure won't clobber it.
func (u *blurHashUpdater) EnqueueClearIfGone(artID model.ArtworkID, version time.Time) {
u.enqueue(artID, blurHashJob{checkGone: true, version: version})
}
func (u *blurHashUpdater) enqueue(artID model.ArtworkID, job blurHashJob) {
if !eligibleKind(artID) {
return
}
u.mutex.Lock()
@ -76,24 +81,18 @@ func (u *blurHashUpdater) enqueue(artID model.ArtworkID, req enqueueRequest) {
}
if !u.started {
u.started = true
// Admin context: playlist artwork readers require a user. Lazy-starting keeps idle Artwork
// instances goroutine-free; stop() ends the worker (tests must call it, the server never does).
// Admin context: playlist artwork readers require a user. Lazy start keeps idle Artwork
// instances goroutine-free; stop() ends the worker (tests call it, the server never does).
ctx, cancel := context.WithCancel(request.WithUser(context.Background(), model.User{IsAdmin: true}))
u.runCancel = cancel
go u.run(ctx)
}
prev := u.buffer[artID]
if !req.snapshot.IsZero() {
// A successful serve proves the artwork exists, so it supersedes any pending gone request for
// the same artwork (a cover restored right after a missing-art serve must still recompute).
prev.gone = false
if req.snapshot.After(prev.snapshot) {
prev.snapshot = req.snapshot
}
} else if req.gone {
prev.gone = true
// A bytes job supersedes a pending gone-check (a successful serve proves the artwork exists);
// otherwise keep whichever is newer.
prev, ok := u.buffer[artID]
if !ok || job.data != nil || (prev.checkGone && job.version.After(prev.version)) {
u.buffer[artID] = job
}
u.buffer[artID] = prev
u.mutex.Unlock()
select {
case u.wake <- struct{}{}:
@ -134,30 +133,30 @@ func (u *blurHashUpdater) run(ctx context.Context) {
return
default:
}
artID, req, ok := u.next()
artID, job, ok := u.next()
if !ok {
break
}
u.process(ctx, artID, req)
u.processJob(ctx, artID, job)
}
}
}
func (u *blurHashUpdater) next() (model.ArtworkID, enqueueRequest, bool) {
func (u *blurHashUpdater) next() (model.ArtworkID, blurHashJob, bool) {
u.mutex.Lock()
defer u.mutex.Unlock()
for artID, req := range u.buffer {
for artID, job := range u.buffer {
delete(u.buffer, artID)
return artID, req, true
return artID, job, true
}
return model.ArtworkID{}, enqueueRequest{}, false
return model.ArtworkID{}, blurHashJob{}, false
}
// processTimeout bounds one computation: readers can call external agents, and a hung call must not
// stall the worker forever.
const processTimeout = 30 * time.Second
func (u *blurHashUpdater) process(ctx context.Context, artID model.ArtworkID, req enqueueRequest) {
func (u *blurHashUpdater) processJob(ctx context.Context, artID model.ArtworkID, job blurHashJob) {
// Artwork readers can touch storage, agents and plugins; a panic here must not kill the server.
defer func() {
if r := recover(); r != nil {
@ -166,115 +165,77 @@ func (u *blurHashUpdater) process(ctx context.Context, artID model.ArtworkID, re
}()
ctx, cancel := context.WithTimeout(ctx, processTimeout)
defer cancel()
stored, storedAt, version, err := u.loadState(ctx, artID)
if err != nil {
log.Trace(ctx, "BlurHash: could not load entity", "artID", artID, err)
return
}
// A future-dated artwork file mtime (clock skew, a future-stamped file) would otherwise be
// persisted verbatim and, via the !Before checks here and in the DTO, pin the stored hash until
// wall time caught up. Cap the snapshot at now so a later real change always moves past it.
now := time.Now()
snapshot := req.snapshot
if snapshot.After(now) {
snapshot = now
}
if req.gone {
// Only the failed serve witnesses a deletion; clear a stored hash so the DTO falls back to
// the rotating fake. An empty hash means there is nothing to clear.
if stored != "" {
if err := u.persist(ctx, artID, "", version); err != nil {
log.Warn(ctx, "BlurHash: error clearing stale hash", "artID", artID, err)
}
}
return
}
// snapshot folds row timestamps and file mtimes into one clock; a stored hash at or after it is
// already current. This is the only freshness comparison the fill trigger needs.
if stored != "" && storedAt != nil && !storedAt.Before(snapshot) {
return
}
hash, err := u.computeFromArtwork(ctx, artID)
if err != nil {
// A transient failure (timeout, flaky cache/DB read) is not evidence the artwork changed;
// leave the stored hash intact and let a later fill retry, so clients don't churn on a fake.
log.Trace(ctx, "BlurHash: recompute failed, keeping stored hash", "artID", artID, err)
return
}
if hash == "" {
// An empty hash means the served bytes are a placeholder: the cover is gone. Clear a stored
// hash so the DTO stops describing artwork no longer served.
if stored != "" {
if err := u.persist(ctx, artID, "", snapshot); err != nil {
log.Warn(ctx, "BlurHash: error clearing stale hash", "artID", artID, err)
}
}
return
}
// Unchanged hash with an unmoved snapshot needs no write — keeps cache-disabled installs (which
// fill on every original serve) from hammering the DB.
if hash == stored && storedAt != nil && !snapshot.After(*storedAt) {
return
}
if err := u.persist(ctx, artID, hash, snapshot); err != nil {
log.Warn(ctx, "BlurHash: error persisting", "artID", artID, err)
}
}
func (u *blurHashUpdater) loadState(ctx context.Context, artID model.ArtworkID) (string, *time.Time, time.Time, error) {
switch artID.Kind {
case model.KindAlbumArtwork:
al, err := u.a.ds.Album(ctx).Get(artID.ID)
if err != nil {
return "", nil, time.Time{}, err
}
return al.BlurHash, al.BlurHashUpdatedAt, al.ArtworkUpdatedAt(), nil
case model.KindArtistArtwork:
ar, err := u.a.ds.Artist(ctx).Get(artID.ID)
if err != nil {
return "", nil, time.Time{}, err
}
return ar.BlurHash, ar.BlurHashUpdatedAt, ar.ArtworkUpdatedAt(), nil
case model.KindPlaylistArtwork:
pl, err := u.a.ds.Playlist(ctx).Get(artID.ID)
if err != nil {
return "", nil, time.Time{}, err
}
return pl.BlurHash, pl.BlurHashUpdatedAt, pl.ArtworkUpdatedAt(), nil
if job.checkGone {
u.processGone(ctx, artID, job.version)
return
}
return "", nil, time.Time{}, model.ErrNotFound
}
func (u *blurHashUpdater) computeFromArtwork(ctx context.Context, artID model.ArtworkID) (string, error) {
artReader, err := u.a.getArtworkReader(ctx, artID, 0, false)
if isPlaceholder(job.data) {
u.clear(ctx, artID, job.version)
return
}
img, _, err := image.Decode(bytes.NewReader(job.data))
if err != nil {
return "", err
}
// Reads via the cache (not artwork.Get, so no re-enqueue): generated playlist mosaics are
// random per generation, and the hash must describe the bytes clients actually download.
r, err := u.a.cache.Get(ctx, artReader)
if err != nil {
return "", err
}
defer r.Close()
data, err := io.ReadAll(r)
if err != nil {
return "", err
}
if isPlaceholder(data) {
return "", nil
}
img, _, err := image.Decode(bytes.NewReader(data))
if err != nil {
return "", err
// Undecodable served bytes are not proof of change; leave the stored hash intact.
log.Trace(ctx, "BlurHash: served bytes not decodable, keeping stored hash", "artID", artID, err)
return
}
b := img.Bounds()
x, y := blurhash.Components(b.Dx(), b.Dy())
return blurhash.Encode(img, x, y)
hash, err := blurhash.Encode(img, x, y)
if err != nil || hash == "" {
return
}
u.write(ctx, artID, hash, job.version)
}
// isPlaceholder byte-compares against the embedded placeholder assets: placeholder artwork must
// never be persisted as an entity's blurhash, and cached reads carry no source path to check.
// processGone re-reads the source once; if it still yields a placeholder or fails, the artwork is
// really gone and the stored hash is cleared. A transient failure recovers by now and is left alone.
func (u *blurHashUpdater) processGone(ctx context.Context, artID model.ArtworkID, version time.Time) {
artReader, err := u.a.getArtworkReader(ctx, artID, 0, false)
if err == nil {
r, gErr := u.a.cache.Get(ctx, artReader)
if gErr == nil {
data, rErr := io.ReadAll(r)
_ = r.Close()
if rErr == nil && !isPlaceholder(data) {
return // source came back (or never really failed): keep the hash
}
}
}
u.clear(ctx, artID, version)
}
func (u *blurHashUpdater) write(ctx context.Context, artID model.ArtworkID, hash string, version time.Time) {
u.mutex.Lock()
if u.last[artID] == hash {
u.mutex.Unlock()
return
}
u.mutex.Unlock()
if err := u.persist(ctx, artID, hash, version); err != nil {
log.Warn(ctx, "BlurHash: error persisting", "artID", artID, err)
return
}
u.mutex.Lock()
u.last[artID] = hash
u.mutex.Unlock()
}
func (u *blurHashUpdater) clear(ctx context.Context, artID model.ArtworkID, version time.Time) {
// No cold-map dedup: an empty u.last[artID] means "never written" as easily as "already cleared",
// so skipping would leave a previous process's DB hash describing gone artwork. Clears are rare.
if err := u.persist(ctx, artID, "", version); err != nil {
log.Warn(ctx, "BlurHash: error clearing hash", "artID", artID, err)
return
}
u.mutex.Lock()
u.last[artID] = ""
u.mutex.Unlock()
}
// isPlaceholder byte-compares against the embedded placeholder assets: placeholder artwork must never
// be persisted as an entity's blurhash, and captured bytes carry no source path to check.
func isPlaceholder(data []byte) bool {
for _, p := range placeholderImages() {
if bytes.Equal(data, p) {

View File

@ -1,7 +1,10 @@
package artwork
import (
"errors"
"bytes"
"image"
"image/color"
"image/png"
"time"
"github.com/navidrome/navidrome/model"
@ -10,131 +13,103 @@ import (
. "github.com/onsi/gomega"
)
// failingFolderRepo makes the artwork reader chain fail with a clean (transient-style) error.
type failingFolderRepo struct{ model.FolderRepository }
// pngImage builds a deterministic 2x2 PNG image for a label (color derived from label bytes).
func pngImage(label string) *image.RGBA {
img := image.NewRGBA(image.Rect(0, 0, 2, 2))
var seed byte
for i := range len(label) {
seed += label[i]
}
c := color.RGBA{R: seed, G: seed * 3, B: seed * 7, A: 255}
for y := range 2 {
for x := range 2 {
img.Set(x, y, c)
}
}
return img
}
func (failingFolderRepo) GetAll(...model.QueryOptions) ([]model.Folder, error) {
return nil, errors.New("boom")
func realPNGBytes(label string) []byte {
var buf bytes.Buffer
Expect(png.Encode(&buf, pngImage(label))).To(Succeed())
return buf.Bytes()
}
var _ = Describe("blurHashUpdater", func() {
var u *blurHashUpdater
var ds *tests.MockDataStore
var version time.Time
BeforeEach(func() {
ds = &tests.MockDataStore{}
// started is pre-set so Enqueue never spawns run(): tests drive next()/process() directly.
u = &blurHashUpdater{
a: &artwork{ds: ds},
buffer: make(map[model.ArtworkID]enqueueRequest),
buffer: make(map[model.ArtworkID]blurHashJob),
wake: make(chan struct{}, 1),
last: make(map[model.ArtworkID]string),
started: true,
}
version = time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)
})
Describe("Enqueue", func() {
It("accepts album, artist and playlist artwork and dedups, keeping the newest snapshot", func() {
id := model.Album{ID: "al-1"}.CoverArtID()
t1 := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)
t2 := t1.Add(time.Hour)
u.Enqueue(id, t1)
u.Enqueue(id, t2)
u.Enqueue(model.Artist{ID: "ar-1"}.CoverArtID(), t1)
Expect(u.buffer).To(HaveLen(2))
Expect(u.buffer[id].snapshot).To(Equal(t2))
Expect(u.buffer[id].gone).To(BeFalse())
})
It("persists a hash computed from the given bytes", func() {
al := model.Album{ID: "al-1", UpdatedAt: version}
repo := tests.CreateMockAlbumRepo()
repo.SetData(model.Albums{al})
ds.MockedAlbum = repo
It("keeps a gone flag when it follows a pending fill (cover then vanished)", func() {
id := model.Album{ID: "al-1"}.CoverArtID()
t1 := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)
u.Enqueue(id, t1)
u.EnqueueGone(id)
Expect(u.buffer[id].snapshot).To(Equal(t1))
Expect(u.buffer[id].gone).To(BeTrue())
})
It("lets a successful fill supersede a pending gone (cover restored)", func() {
id := model.Album{ID: "al-1"}.CoverArtID()
t1 := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)
u.EnqueueGone(id)
u.Enqueue(id, t1)
Expect(u.buffer[id].snapshot).To(Equal(t1))
Expect(u.buffer[id].gone).To(BeFalse())
})
It("ignores other artwork kinds", func() {
u.Enqueue(model.ArtworkID{Kind: model.KindMediaFileArtwork, ID: "mf-1"}, time.Time{})
u.EnqueueGone(model.ArtworkID{Kind: model.KindRadioArtwork, ID: "ra-1"})
Expect(u.buffer).To(BeEmpty())
})
u.processJob(GinkgoT().Context(), al.CoverArtID(), blurHashJob{data: realPNGBytes("x"), version: version})
stored, err := ds.Album(GinkgoT().Context()).Get("al-1")
Expect(err).ToNot(HaveOccurred())
Expect(stored.BlurHash).ToNot(BeEmpty())
})
Describe("process", func() {
var version time.Time
It("clears the hash when the bytes are a placeholder", func() {
al := model.Album{ID: "al-1", UpdatedAt: version, BlurHash: "OLD"}
repo := tests.CreateMockAlbumRepo()
repo.SetData(model.Albums{al})
ds.MockedAlbum = repo
BeforeEach(func() {
version = time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)
})
u.processJob(GinkgoT().Context(), al.CoverArtID(), blurHashJob{data: placeholderImages()[0], version: version})
stored, _ := ds.Album(GinkgoT().Context()).Get("al-1")
Expect(stored.BlurHash).To(BeEmpty()) // cleared: placeholder means gone
})
It("skips entities whose stored hash is at or after the snapshot", func() {
al := model.Album{ID: "al-1", UpdatedAt: version, BlurHash: "LEHV6nWB2yk8", BlurHashUpdatedAt: &version}
repo := tests.CreateMockAlbumRepo()
repo.SetData(model.Albums{al})
ds.MockedAlbum = repo
It("leaves the hash untouched on undecodable bytes", func() {
al := model.Album{ID: "al-1", UpdatedAt: version, BlurHash: "KEEP"}
repo := tests.CreateMockAlbumRepo()
repo.SetData(model.Albums{al})
ds.MockedAlbum = repo
u.process(GinkgoT().Context(), al.CoverArtID(), enqueueRequest{snapshot: version})
stored, err := ds.Album(GinkgoT().Context()).Get("al-1")
Expect(err).ToNot(HaveOccurred())
Expect(stored.BlurHash).To(Equal("LEHV6nWB2yk8"))
})
u.processJob(GinkgoT().Context(), al.CoverArtID(), blurHashJob{data: []byte("not an image"), version: version})
stored, _ := ds.Album(GinkgoT().Context()).Get("al-1")
Expect(stored.BlurHash).To(Equal("KEEP"))
})
It("keeps the stored hash when a recompute fails transiently", func() {
al := model.Album{ID: "al-1", UpdatedAt: version, BlurHash: "LEHV6nWB2yk8", BlurHashUpdatedAt: nil}
repo := tests.CreateMockAlbumRepo()
repo.SetData(model.Albums{al})
ds.MockedAlbum = repo
ds.MockedFolder = failingFolderRepo{}
It("skips a redundant write when the hash is unchanged (in-memory dedup)", func() {
al := model.Album{ID: "al-1", UpdatedAt: version}
repo := tests.CreateMockAlbumRepo()
repo.SetData(model.Albums{al})
ds.MockedAlbum = repo
data := realPNGBytes("dedup")
// A newer snapshot forces a recompute, but the reader chain errors (transient): the stored
// hash must survive, so clients don't churn on a fake until a later fill succeeds.
newer := version.Add(time.Hour)
u.process(GinkgoT().Context(), al.CoverArtID(), enqueueRequest{snapshot: newer})
stored, err := ds.Album(GinkgoT().Context()).Get("al-1")
Expect(err).ToNot(HaveOccurred())
Expect(stored.BlurHash).To(Equal("LEHV6nWB2yk8"))
})
u.processJob(GinkgoT().Context(), al.CoverArtID(), blurHashJob{data: data, version: version})
first, _ := ds.Album(GinkgoT().Context()).Get("al-1")
u.processJob(GinkgoT().Context(), al.CoverArtID(), blurHashJob{data: data, version: version.Add(time.Hour)})
second, _ := ds.Album(GinkgoT().Context()).Get("al-1")
Expect(second.BlurHash).To(Equal(first.BlurHash))
})
It("clears a stored hash when the source is gone", func() {
al := model.Album{ID: "al-1", UpdatedAt: version, BlurHash: "LEHV6nWB2yk8", BlurHashUpdatedAt: &version}
repo := tests.CreateMockAlbumRepo()
repo.SetData(model.Albums{al})
ds.MockedAlbum = repo
It("ignores non-eligible artwork kinds on enqueue", func() {
u.EnqueueBytes(model.ArtworkID{Kind: model.KindMediaFileArtwork, ID: "mf-1"}, realPNGBytes("x"), version)
Expect(u.buffer).To(BeEmpty())
})
u.process(GinkgoT().Context(), al.CoverArtID(), enqueueRequest{gone: true})
stored, err := ds.Album(GinkgoT().Context()).Get("al-1")
Expect(err).ToNot(HaveOccurred())
Expect(stored.BlurHash).To(BeEmpty())
})
It("does nothing for a gone serve with no stored hash", func() {
al := model.Album{ID: "al-1", UpdatedAt: version}
repo := tests.CreateMockAlbumRepo()
repo.SetData(model.Albums{al})
ds.MockedAlbum = repo
Expect(func() {
u.process(GinkgoT().Context(), al.CoverArtID(), enqueueRequest{gone: true})
}).ToNot(Panic())
stored, _ := ds.Album(GinkgoT().Context()).Get("al-1")
Expect(stored.BlurHash).To(BeEmpty())
})
It("does nothing when the entity is gone", func() {
ds.MockedAlbum = tests.CreateMockAlbumRepo()
Expect(func() {
u.process(GinkgoT().Context(), model.Album{ID: "missing"}.CoverArtID(), enqueueRequest{})
}).ToNot(Panic())
})
It("supersedes a pending gone-check with a bytes job", func() {
id := model.Album{ID: "al-1"}.CoverArtID()
u.EnqueueClearIfGone(id, version)
u.EnqueueBytes(id, realPNGBytes("x"), version.Add(time.Hour))
Expect(u.buffer[id].checkGone).To(BeFalse())
Expect(u.buffer[id].data).ToNot(BeNil())
})
})