diff --git a/core/artwork/processor.go b/core/artwork/processor.go index fcdcaa44a..a62cf131c 100644 --- a/core/artwork/processor.go +++ b/core/artwork/processor.go @@ -9,15 +9,11 @@ import ( "image/draw" _ "image/gif" // the only artwork format with no other importer in this package "io" - "sync" "time" - "github.com/navidrome/navidrome/core/agents" "github.com/navidrome/navidrome/core/artwork/blurhash" - "github.com/navidrome/navidrome/core/ffmpeg" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" - "github.com/navidrome/navidrome/utils/cache" xdraw "golang.org/x/image/draw" ) @@ -45,19 +41,6 @@ const maxImageBytes = 20 << 20 // huge canvas that image.Decode would expand into gigabytes (decompression bomb). const maxImagePixels = 64 << 20 -// workerDeps are the collaborators processItem needs; gate and pruneLock are set by NewWorker -// in production and nil only in tests, where resolveItem falls back to a plain passthrough and -// nothing prunes. -type workerDeps struct { - ds model.DataStore - store *ImageStore - agents *agents.Agents - ffmpeg ffmpeg.FFmpeg - cache cache.FileCache - gate gateFunc - pruneLock sync.Locker -} - // acquired is what processItem persisted, handed back so the caller can warm the resize // cache without re-reading the rows and the file it just wrote. type acquired struct { @@ -71,7 +54,7 @@ type acquired struct { func processItem(ctx context.Context, deps *workerDeps, item model.ArtworkQueueItem) (outcome, *acquired) { repo := deps.ds.Artwork(ctx) - res, err := newResolver(deps.ds, deps.agents, deps.ffmpeg, deps.gate).resolve(ctx, item) + res, err := deps.resolver.resolve(ctx, item) if err != nil { log.Warn(ctx, "Artwork: Could not resolve item", "kind", item.ItemKind, "id", item.ItemID, err) return outcomeFailed, nil @@ -115,7 +98,7 @@ func processItem(ctx context.Context, deps *workerDeps, item model.ArtworkQueueI } art.SizeBytes = int64(len(data)) - ia, err := persist(deps, repo, item, hash, art, res, data) + ia, err := persist(deps, repo, item, art, res, data) if err != nil { log.Warn(ctx, "Artwork: Failed to persist resolved image", "kind", item.ItemKind, "id", item.ItemID, err) return outcomeFailed, nil @@ -130,7 +113,7 @@ func processItem(ctx context.Context, deps *workerDeps, item model.ArtworkQueueI // persist places the bytes and commits the rows referencing them. Only this window excludes // Prune, which reclaims store files no row points at; resolution stays outside so a slow fetch // cannot hold prune off. -func persist(deps *workerDeps, repo model.ArtworkRepository, item model.ArtworkQueueItem, hash string, +func persist(deps *workerDeps, repo model.ArtworkRepository, item model.ArtworkQueueItem, art *model.Artwork, res resolution, data []byte, ) (*model.ItemArtwork, error) { if deps.pruneLock != nil { @@ -148,7 +131,7 @@ func persist(deps *workerDeps, repo model.ArtworkRepository, item model.ArtworkQ ItemKind: item.ItemKind, ItemID: item.ItemID, ImageType: item.ImageType, - Hash: hash, + Hash: art.Hash, Source: res.source, SourcePath: sourcePath, RefMtime: refMtime, diff --git a/core/artwork/processor_test.go b/core/artwork/processor_test.go index c6fb3321d..d16179624 100644 --- a/core/artwork/processor_test.go +++ b/core/artwork/processor_test.go @@ -71,7 +71,7 @@ var _ = Describe("processItem", func() { } ds.MockedAlbum = tests.CreateMockAlbumRepo() store = NewImageStore(GinkgoT().TempDir()) - deps = &workerDeps{ds: ds, store: store, agents: ag, ffmpeg: ffm} + deps = &workerDeps{ds: ds, store: store, resolver: newResolver(ds, ag, ffm, nil)} conf.Server.CoverArtPriority = "cover.jpg, embedded" }) diff --git a/core/artwork/worker.go b/core/artwork/worker.go index 0fece4ed3..6881ac1a1 100644 --- a/core/artwork/worker.go +++ b/core/artwork/worker.go @@ -49,11 +49,22 @@ type drainPool struct { wake chan struct{} } +// workerDeps are the collaborators processItem needs. The resolver is built once by NewWorker +// rather than per item; pruneLock is nil only in tests, where nothing prunes. +type workerDeps struct { + ds model.DataStore + store *ImageStore + resolver *resolver + pruneLock sync.Locker +} + // Worker drains the artwork queue through processItem: each external agent is rate-limited // and circuit-broken independently, and prune is serialized against the store-write window // via pruneMu. type Worker struct { deps workerDeps + cache cache.FileCache + ffmpeg ffmpeg.FFmpeg broker events.Broker pruneMu sync.RWMutex pools []*drainPool @@ -65,13 +76,15 @@ type Worker struct { func NewWorker(ds model.DataStore, store *ImageStore, ag *agents.Agents, ffmpeg ffmpeg.FFmpeg, broker events.Broker, imgCache cache.FileCache) *Worker { w := &Worker{ - deps: workerDeps{ds: ds, store: store, agents: ag, ffmpeg: ffmpeg, cache: imgCache}, + deps: workerDeps{ds: ds, store: store}, + cache: imgCache, + ffmpeg: ffmpeg, broker: broker, pools: newDrainPools(), runCtx: context.Background(), gates: map[string]*extGate{}, } - w.deps.gate = w.gate + w.deps.resolver = newResolver(ds, ag, ffmpeg, w.gate) w.deps.pruneLock = w.pruneMu.RLocker() return w } @@ -301,7 +314,7 @@ func (w *Worker) hasResolvedArtwork(ctx context.Context, item model.ArtworkQueue // first UI request is a cache hit without re-reading the rows or the file. Skipped when // disabled; failures are debug-only. func (w *Worker) precache(ctx context.Context, got *acquired) { - if !conf.Server.EnableArtworkPrecache || w.deps.cache == nil || w.deps.cache.Disabled(ctx) { + if !conf.Server.EnableArtworkPrecache || w.cache == nil || w.cache.Disabled(ctx) { return } // Same key as the serving path (hash/size/square); only the source of the bytes differs. @@ -310,10 +323,10 @@ func (w *Worker) precache(ctx context.Context, got *acquired) { hash: got.ia.Hash, size: conf.Server.UICoverArtSize, square: true, - ffmpeg: w.deps.ffmpeg, + ffmpeg: w.ffmpeg, open: func() (io.ReadCloser, error) { return io.NopCloser(bytes.NewReader(got.data)), nil }, } - stream, err := w.deps.cache.Get(ctx, item) + stream, err := w.cache.Get(ctx, item) if err != nil { log.Debug(ctx, "Artwork: Precache failed", "kind", got.ia.ItemKind, "id", got.ia.ItemID, err) return diff --git a/core/artwork/worker_soak_test.go b/core/artwork/worker_soak_test.go index f728e6899..5d06a19a0 100644 --- a/core/artwork/worker_soak_test.go +++ b/core/artwork/worker_soak_test.go @@ -54,7 +54,7 @@ var _ = Describe("Worker soak", func() { MockedAlbum: albumRepo, } store := NewImageStore(GinkgoT().TempDir()) - deps := &workerDeps{ds: ds, store: store, agents: ag, ffmpeg: ffm} + deps := &workerDeps{ds: ds, store: store, resolver: newResolver(ds, ag, ffm, nil)} conf.Server.CoverArtPriority = "cover.jpg, embedded" // Dangling refs (al/ra ids the repos don't know about) mirror an entity