diff --git a/core/artwork/resolve.go b/core/artwork/resolve.go index 6d41897e5..22ad3de52 100644 --- a/core/artwork/resolve.go +++ b/core/artwork/resolve.go @@ -49,6 +49,8 @@ func resolveItem(ctx context.Context, ds model.DataStore, ag *agents.Agents, ffm return resolvePlaylist(ctx, ds, ag, ffmpeg, item.ItemID, gate) case "ra": return resolveRadio(ctx, ds, item.ItemID) + case "mf": + return resolveMediaFile(ctx, ds, ffmpeg, item.ItemID) default: return resolution{}, fmt.Errorf("resolveItem: kind %q is not resolvable by the worker", item.ItemKind) } @@ -251,6 +253,24 @@ func resolveRadio(ctx context.Context, ds model.DataStore, radioID string) (reso return res, nil } +// resolveMediaFile resolves a track's own embedded art only; there is no folder or +// external fallback, so disabled/missing cover art is a definitive absent. +func resolveMediaFile(ctx context.Context, ds model.DataStore, ffm ffmpeg.FFmpeg, id string) (resolution, error) { + mf, err := ds.MediaFile(ctx).Get(id) + if err != nil { + return resolution{}, err + } + if !conf.Server.EnableMediaFileCoverArt || !mf.HasCoverArt { + return resolution{}, nil + } + lib, err := loadLibraryView(ctx, ds, mf.LibraryID) + if err != nil { + return resolution{}, err + } + res, _ := resolveEmbedded(ctx, lib, ffm, mf.Path) + return res, nil +} + // resolveExternalStep runs a single external sourceFunc through the named gate; used by // the playlist ExternalImageURL step. ok reports a hit; extErr reports a non-not-found // error (a not-found is a definitive "no", not a failure). diff --git a/core/artwork/resolve_test.go b/core/artwork/resolve_test.go index cce1554da..8309a6cc3 100644 --- a/core/artwork/resolve_test.go +++ b/core/artwork/resolve_test.go @@ -51,11 +51,61 @@ var _ = Describe("resolveItem", func() { Describe("kind dispatch", func() { It("returns an error for kinds the worker never enqueues", func() { - _, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "mf", ItemID: "x"}, nil) + _, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "zz", ItemID: "x"}, nil) Expect(err).To(HaveOccurred()) }) }) + Describe("media file", func() { + BeforeEach(func() { + conf.Server.EnableMediaFileCoverArt = true + ds.MockedMediaFile = tests.CreateMockMediaFileRepo() + }) + + It("resolves embedded art from the track file", func() { + ds.MockedMediaFile.(*tests.MockMediaFileRepo).SetData(model.MediaFiles{ + {ID: "mf1", LibraryID: 0, Path: "tests/fixtures/artist/an-album/test.mp3", HasCoverArt: true}, + }) + + res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "mf", ItemID: "mf1"}, nil) + Expect(err).ToNot(HaveOccurred()) + Expect(res.reader).ToNot(BeNil()) + defer res.reader.Close() + Expect(res.source).To(Equal("embedded")) + Expect(filepath.ToSlash(res.sourcePath)).To(HaveSuffix("tests/fixtures/artist/an-album/test.mp3")) + Expect(res.refMtime).To(BeNumerically(">", 0)) + Expect(res.extError).To(BeFalse()) + }) + + It("resolves absent when the track has no cover art", func() { + ds.MockedMediaFile.(*tests.MockMediaFileRepo).SetData(model.MediaFiles{ + {ID: "mf2", LibraryID: 0, Path: "tests/fixtures/artist/an-album/test.mp3", HasCoverArt: false}, + }) + + res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "mf", ItemID: "mf2"}, nil) + Expect(err).ToNot(HaveOccurred()) + Expect(res.reader).To(BeNil()) + Expect(res.extError).To(BeFalse()) + }) + + It("resolves absent when media file cover art is disabled", func() { + conf.Server.EnableMediaFileCoverArt = false + ds.MockedMediaFile.(*tests.MockMediaFileRepo).SetData(model.MediaFiles{ + {ID: "mf3", LibraryID: 0, Path: "tests/fixtures/artist/an-album/test.mp3", HasCoverArt: true}, + }) + + res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "mf", ItemID: "mf3"}, nil) + Expect(err).ToNot(HaveOccurred()) + Expect(res.reader).To(BeNil()) + }) + + It("returns the error when the track is not in the DB", func() { + res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "mf", ItemID: "missing"}, nil) + Expect(err).To(MatchError(model.ErrNotFound)) + Expect(res.reader).To(BeNil()) + }) + }) + Describe("album", func() { BeforeEach(func() { conf.Server.CoverArtPriority = "cover.jpg, embedded" diff --git a/core/artwork/worker_test.go b/core/artwork/worker_test.go index cf9b7cddf..cee64987f 100644 --- a/core/artwork/worker_test.go +++ b/core/artwork/worker_test.go @@ -115,6 +115,39 @@ var _ = Describe("Worker", func() { Expect(count).To(BeZero(), "a found item must be deleted from the queue") }) + It("processes an mf queue item, writing state and storing embedded bytes", func() { + conf.Server.EnableMediaFileCoverArt = true + ds.MockedMediaFile = tests.CreateMockMediaFileRepo() + ds.MockedMediaFile.(*tests.MockMediaFileRepo).SetData(model.MediaFiles{ + {ID: "mf1", LibraryID: 0, Path: "tests/fixtures/artist/an-album/test.mp3", HasCoverArt: true}, + }) + Expect(queueRepo.Enqueue(model.ArtworkQueueItem{ + ItemKind: "mf", ItemID: "mf1", Priority: model.ArtworkPriorityBump, + })).To(Succeed()) + + n, err := w.drain(ctx, 2) + Expect(err).ToNot(HaveOccurred()) + Expect(n).To(Equal(1)) + + ia, err := artRepo.GetItemArtwork("mf", "mf1", model.ImageTypePrimary) + Expect(err).ToNot(HaveOccurred()) + Expect(ia.Source).To(Equal("embedded")) + Expect(ia.Hash).ToNot(BeEmpty()) + + art, err := artRepo.GetImage(ia.Hash) + Expect(err).ToNot(HaveOccurred()) + r, err := store.Open(ia.Hash, art.Mime) + Expect(err).ToNot(HaveOccurred()) + defer r.Close() + data, err := io.ReadAll(r) + Expect(err).ToNot(HaveOccurred()) + Expect(data).ToNot(BeEmpty(), "embedded bytes must be written to the store") + + count, err := queueRepo.Count() + Expect(err).ToNot(HaveOccurred()) + Expect(count).To(BeZero()) + }) + It("reschedules a failed item via MarkFailed with a backed-off retry_at", func() { conf.Server.CoverArtPriority = "external" ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "al4", Name: "Album"}}) diff --git a/model/artwork.go b/model/artwork.go index a4f9e01e1..88b2d009d 100644 --- a/model/artwork.go +++ b/model/artwork.go @@ -80,6 +80,8 @@ type ArtworkRepository interface { GetItemArtwork(kind, id, imageType string) (*ItemArtwork, error) PutItemArtwork(ia *ItemArtwork) error DeleteForItem(kind, id string) error + // DeleteForItems removes state rows for the given ids of one kind, in chunks. + DeleteForItems(kind string, ids []string) error // GetInfoForItems hydrates a page: one batched query, item_artwork joined to artwork. GetInfoForItems(kind string, ids []string) (map[string]ItemArtworkInfo, error) // GetAllMimes returns hash -> current mime for every stored artwork, for sweep retention checks. diff --git a/persistence/artwork_queue_repository_test.go b/persistence/artwork_queue_repository_test.go index 7ccef59ef..9a4054cb9 100644 --- a/persistence/artwork_queue_repository_test.go +++ b/persistence/artwork_queue_repository_test.go @@ -127,18 +127,20 @@ var _ = Describe("ArtworkQueueRepository", func() { item("pl", "no-such-playlist", model.ArtworkPriorityScan), item("ra", radioWithHomePage.ID, model.ArtworkPriorityScan), item("ra", "no-such-radio", model.ArtworkPriorityScan), + item("mf", songDayInALife.ID, model.ArtworkPriorityScan), + item("mf", "no-such-mediafile", model.ArtworkPriorityScan), )).To(Succeed()) purged, err := repo.PurgeDangling() Expect(err).ToNot(HaveOccurred()) - Expect(purged).To(Equal(int64(4))) + Expect(purged).To(Equal(int64(5))) got, _ := repo.DequeueBatch(100) ids := make([]string, 0, len(got)) for _, it := range got { ids = append(ids, it.ItemID) } - Expect(ids).To(ConsistOf(albumSgtPeppers.ID, artistKraftwerk.ID, plsBest.ID, radioWithHomePage.ID)) + Expect(ids).To(ConsistOf(albumSgtPeppers.ID, artistKraftwerk.ID, plsBest.ID, radioWithHomePage.ID, songDayInALife.ID)) }) It("enqueues stale absent states for recheck", func() { diff --git a/persistence/artwork_repository.go b/persistence/artwork_repository.go index 9ac32b42e..e21a69c63 100644 --- a/persistence/artwork_repository.go +++ b/persistence/artwork_repository.go @@ -120,6 +120,7 @@ var danglingItemArtworkKinds = map[string]string{ "ar": "artist", "pl": "playlist", "ra": "radio", + "mf": "media_file", } // purgeDangling deletes rows in table whose owning entity is gone, one statement per kind. @@ -177,6 +178,15 @@ func (r *artworkRepository) DeleteForItem(kind, id string) error { return r.items.delete(Eq{"item_kind": kind, "item_id": id}) } +func (r *artworkRepository) DeleteForItems(kind string, ids []string) error { + for chunk := range slices.Chunk(ids, artworkBatchSize) { + if err := r.items.delete(Eq{"item_kind": kind, "item_id": chunk}); err != nil { + return err + } + } + return nil +} + func (r *artworkRepository) GetInfoForItems(kind string, ids []string) (map[string]model.ItemArtworkInfo, error) { res := map[string]model.ItemArtworkInfo{} for chunk := range slices.Chunk(ids, artworkBatchSize) { diff --git a/persistence/artwork_repository_test.go b/persistence/artwork_repository_test.go index ae9f15fda..c1f1ad741 100644 --- a/persistence/artwork_repository_test.go +++ b/persistence/artwork_repository_test.go @@ -147,16 +147,19 @@ var _ = Describe("ArtworkRepository", func() { Expect(repo.PutItemArtwork(&model.ItemArtwork{ItemKind: "pl", ItemID: "no-such-playlist", ImageType: model.ImageTypePrimary, Hash: "danglingPl"})).To(Succeed()) Expect(repo.PutItemArtwork(&model.ItemArtwork{ItemKind: "ra", ItemID: radioWithHomePage.ID, ImageType: model.ImageTypePrimary, Hash: "keepRa"})).To(Succeed()) Expect(repo.PutItemArtwork(&model.ItemArtwork{ItemKind: "ra", ItemID: "no-such-radio", ImageType: model.ImageTypePrimary, Hash: "danglingRa"})).To(Succeed()) + Expect(repo.PutItemArtwork(&model.ItemArtwork{ItemKind: "mf", ItemID: songDayInALife.ID, ImageType: model.ImageTypePrimary, Hash: "keepMf"})).To(Succeed()) + Expect(repo.PutItemArtwork(&model.ItemArtwork{ItemKind: "mf", ItemID: "no-such-mediafile", ImageType: model.ImageTypePrimary, Hash: "danglingMf"})).To(Succeed()) purged, err := repo.PurgeDanglingItemArtwork() Expect(err).ToNot(HaveOccurred()) - Expect(purged).To(Equal(int64(4))) + Expect(purged).To(Equal(int64(5))) for _, kept := range []model.ItemArtwork{ {ItemKind: "al", ItemID: albumSgtPeppers.ID}, {ItemKind: "ar", ItemID: artistKraftwerk.ID}, {ItemKind: "pl", ItemID: plsBest.ID}, {ItemKind: "ra", ItemID: radioWithHomePage.ID}, + {ItemKind: "mf", ItemID: songDayInALife.ID}, } { _, err := repo.GetItemArtwork(kept.ItemKind, kept.ItemID, model.ImageTypePrimary) Expect(err).ToNot(HaveOccurred()) @@ -166,6 +169,7 @@ var _ = Describe("ArtworkRepository", func() { {ItemKind: "ar", ItemID: "no-such-artist"}, {ItemKind: "pl", ItemID: "no-such-playlist"}, {ItemKind: "ra", ItemID: "no-such-radio"}, + {ItemKind: "mf", ItemID: "no-such-mediafile"}, } { _, err := repo.GetItemArtwork(gone.ItemKind, gone.ItemID, model.ImageTypePrimary) Expect(err).To(MatchError(model.ErrNotFound)) @@ -230,5 +234,26 @@ var _ = Describe("ArtworkRepository", func() { _, err := repo.GetItemArtwork("pl", "p1", model.ImageTypePrimary) Expect(err).To(MatchError(model.ErrNotFound)) }) + + It("deletes rows for many items in chunks, leaving others untouched", func() { + const n = artworkBatchSize + 5 + ids := make([]string, n) + for i := range n { + id := fmt.Sprintf("mf-%d", i) + ids[i] = id + Expect(repo.PutItemArtwork(&model.ItemArtwork{ItemKind: "mf", ItemID: id, ImageType: model.ImageTypePrimary, Hash: "h1"})).To(Succeed()) + } + Expect(repo.PutItemArtwork(&model.ItemArtwork{ItemKind: "mf", ItemID: "keep", ImageType: model.ImageTypePrimary, Hash: "h1"})).To(Succeed()) + + Expect(repo.DeleteForItems("mf", ids)).To(Succeed()) + + for _, id := range ids { + _, err := repo.GetItemArtwork("mf", id, model.ImageTypePrimary) + Expect(err).To(MatchError(model.ErrNotFound)) + } + kept, err := repo.GetItemArtwork("mf", "keep", model.ImageTypePrimary) + Expect(err).ToNot(HaveOccurred()) + Expect(kept.ItemID).To(Equal("keep")) + }) }) }) diff --git a/scanner/phase_1_folders.go b/scanner/phase_1_folders.go index 3a7265cc4..65f26b9d9 100644 --- a/scanner/phase_1_folders.go +++ b/scanner/phase_1_folders.go @@ -406,6 +406,17 @@ func (p *phaseFolders) persistChanges(entry *folderEntry) (*folderEntry, error) } } + // A re-imported track returns to unresolved so new embedded art is picked up lazily. + if len(entry.tracks) > 0 { + trackIDs := make([]string, len(entry.tracks)) + for i := range entry.tracks { + trackIDs[i] = entry.tracks[i].ID + } + if err := tx.Artwork(p.ctx).DeleteForItems("mf", trackIDs); err != nil { + log.Warn(p.ctx, "Scanner: could not invalidate media_file artwork", "folder", entry.path, err) + } + } + // Mark all missing tracks as not available if len(entry.missingTracks) > 0 { err = mfRepo.MarkMissing(true, entry.missingTracks...) diff --git a/scanner/scanner_test.go b/scanner/scanner_test.go index 0a7fb07b2..b8044c587 100644 --- a/scanner/scanner_test.go +++ b/scanner/scanner_test.go @@ -210,6 +210,26 @@ var _ = Describe("Scanner", Ordered, func() { Expect(albums[0].Participants.First(model.RoleProducer).Name).To(Equal("George Martin")) Expect(albums[0].SongCount).To(Equal(3)) }) + + It("invalidates the media_file artwork state so new embedded art is picked up lazily", func() { + Expect(runScanner(ctx, true)).To(Succeed()) + + mf, err := ds.MediaFile(ctx).GetAll(model.QueryOptions{Filters: squirrel.Eq{"title": "Help!"}}) + Expect(err).ToNot(HaveOccurred()) + Expect(mf).ToNot(BeEmpty()) + trackID := mf[0].ID + + Expect(ds.Artwork(ctx).PutItemArtwork(&model.ItemArtwork{ + ItemKind: "mf", ItemID: trackID, ImageType: model.ImageTypePrimary, + Source: "embedded", Hash: "stalehash", + })).To(Succeed()) + + fsys.UpdateTags("The Beatles/Help!/01 - Help!.mp3", _t{"comment": "reimport"}) + Expect(runScanner(ctx, true)).To(Succeed()) + + _, err = ds.Artwork(ctx).GetItemArtwork("mf", trackID, model.ImageTypePrimary) + Expect(err).To(MatchError(model.ErrNotFound)) + }) }) }) diff --git a/tests/mock_artwork_repo.go b/tests/mock_artwork_repo.go index db73af84d..9e9f84272 100644 --- a/tests/mock_artwork_repo.go +++ b/tests/mock_artwork_repo.go @@ -152,6 +152,22 @@ func (m *MockArtworkRepo) DeleteForItem(kind, id string) error { return nil } +func (m *MockArtworkRepo) DeleteForItems(kind string, ids []string) error { + if m.Err != nil { + return m.Err + } + idSet := make(map[string]bool, len(ids)) + for _, id := range ids { + idSet[id] = true + } + for k, ia := range m.ItemData { + if ia.ItemKind == kind && idSet[ia.ItemID] { + delete(m.ItemData, k) + } + } + return nil +} + func (m *MockArtworkRepo) GetInfoForItems(kind string, ids []string) (map[string]model.ItemArtworkInfo, error) { if m.Err != nil { return nil, m.Err