mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-01 07:21:17 +00:00
fix(jellyfin): hydrate artwork on the song cursor
Jellyfin's listSongs streamed media files via GetCursor, which never hydrates artwork, so songs emitted entity-id image tags and no blurhash. media_file now uses the same id pre-pass as the other three cursors (album/artist/playlist), for consistency, but on a separate method, GetCursorWithArtwork: GetCursor itself must stay untouched, since it's also the scanner's hot path and the scanner never reads artwork. Measured on 1,000,000 tracks, the pre-pass over all ids costs +41.8 MB heap and +298 ms versus GetCursor's bounded +0.0 MB. The Jellyfin path is paginated, though, so in practice it only ever pre-passes a page's worth of ids, not the full library, and doesn't pay that cost.
This commit is contained in:
parent
1cc0df80b1
commit
8545cc4762
@ -549,6 +549,11 @@ type MediaFileRepository interface {
|
||||
GetRandom(options ...QueryOptions) (MediaFiles, error)
|
||||
GetAllByTags(tag TagName, values []string, options ...QueryOptions) (MediaFiles, error)
|
||||
GetCursor(options ...QueryOptions) (MediaFileCursor, error)
|
||||
// GetAllIDs returns just the media_file IDs for the same row set as GetAll.
|
||||
GetAllIDs(options ...QueryOptions) ([]string, error)
|
||||
// GetCursorWithArtwork streams like GetCursor, hydrated, so callers that render images don't
|
||||
// pay the scanner's per-row cost; it uses the same id pre-pass as the other cursors.
|
||||
GetCursorWithArtwork(options ...QueryOptions) (MediaFileCursor, error)
|
||||
Delete(id string) error
|
||||
DeleteMissing(ids []string) error
|
||||
DeleteAllMissing() (int64, error)
|
||||
|
||||
@ -568,6 +568,84 @@ var _ = Describe("Artwork hydration", func() {
|
||||
})
|
||||
})
|
||||
|
||||
Describe("GetCursorWithArtwork", func() {
|
||||
var mfRepo model.MediaFileRepository
|
||||
var onlySongs squirrel.Eq
|
||||
|
||||
BeforeEach(func() {
|
||||
mfRepo = NewMediaFileRepository(ctx, GetDBXBuilder())
|
||||
putInfo("al", albumSgtPeppers.ID, "curhash11111111")
|
||||
// Distinct titles keep the ordering/paging specs tie-free; other fixture songs share
|
||||
// titles (e.g. "Antenna") or albums, which would make row order ambiguous.
|
||||
onlySongs = squirrel.Eq{"media_file.id": []string{songDayInALife.ID, songComeTogether.ID,
|
||||
songRadioactivity.ID, songAntenna.ID, songDisc1Track01.ID, songCJK.ID, songPunctuation.ID}}
|
||||
})
|
||||
|
||||
It("hydrates artwork onto every streamed track, unlike GetCursor", func() {
|
||||
opts := model.QueryOptions{Sort: "title"}
|
||||
want, err := mfRepo.GetAll(opts)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(want).ToNot(BeEmpty())
|
||||
|
||||
cursor, err := mfRepo.GetCursorWithArtwork(opts)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
var got model.MediaFiles
|
||||
cursor(func(mf model.MediaFile, err error) bool {
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
got = append(got, mf)
|
||||
return true
|
||||
})
|
||||
|
||||
Expect(got).To(HaveLen(len(want)))
|
||||
for i := range want {
|
||||
Expect(got[i].ID).To(Equal(want[i].ID))
|
||||
Expect(got[i].ImageHash).To(Equal(want[i].ImageHash))
|
||||
Expect(got[i].ImageAbsent).To(Equal(want[i].ImageAbsent))
|
||||
Expect(got[i].AlbumImage.ImageHash).To(Equal(want[i].AlbumImage.ImageHash))
|
||||
Expect(got[i].AlbumImage.BlurHash).To(Equal(want[i].AlbumImage.BlurHash))
|
||||
}
|
||||
Expect(want).To(ContainElement(HaveField("AlbumImage.ImageHash", Not(BeEmpty()))),
|
||||
"fixture must include at least one track with album artwork, or this proves nothing")
|
||||
})
|
||||
|
||||
It("leaves the scanner's GetCursor unhydrated", func() {
|
||||
cursor, err := mfRepo.GetCursor(model.QueryOptions{Sort: "title"})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
var seen int
|
||||
cursor(func(mf model.MediaFile, err error) bool {
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(mf.AlbumImage.ImageHash).To(BeEmpty(), "GetCursor must stay unhydrated for the scanner")
|
||||
seen++
|
||||
return true
|
||||
})
|
||||
Expect(seen).To(BeNumerically(">", 0))
|
||||
})
|
||||
|
||||
It("streams the same ids in the same order as GetAll", func() {
|
||||
opts := model.QueryOptions{Sort: "title", Filters: onlySongs}
|
||||
want, err := mfRepo.GetAll(opts)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(want).ToNot(BeEmpty())
|
||||
|
||||
got := collectCursor(mfRepo.GetCursorWithArtwork(opts))
|
||||
|
||||
Expect(slice.Map(got, func(mf model.MediaFile) string { return mf.ID })).
|
||||
To(Equal(slice.Map(want, func(mf model.MediaFile) string { return mf.ID })))
|
||||
})
|
||||
|
||||
It("honors Max and Offset exactly once", func() {
|
||||
opts := model.QueryOptions{Sort: "title", Filters: onlySongs, Max: 2, Offset: 1}
|
||||
want, err := mfRepo.GetAll(opts)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(want).To(HaveLen(2))
|
||||
|
||||
got := collectCursor(mfRepo.GetCursorWithArtwork(opts))
|
||||
|
||||
Expect(slice.Map(got, func(mf model.MediaFile) string { return mf.ID })).
|
||||
To(Equal(slice.Map(want, func(mf model.MediaFile) string { return mf.ID })))
|
||||
})
|
||||
})
|
||||
|
||||
Describe("chunkOptions", func() {
|
||||
It("carries Sort, Order and the caller's filters, but never Max/Offset", func() {
|
||||
base := model.QueryOptions{Sort: "name", Order: "desc", Max: 10, Offset: 20,
|
||||
|
||||
@ -343,6 +343,31 @@ func (r *mediaFileRepository) GetCursor(options ...model.QueryOptions) (model.Me
|
||||
return wrapMediaFileCursor(cursor), nil
|
||||
}
|
||||
|
||||
// GetAllIDs returns just the media_file IDs for the same row set as GetAll, skipping the heavy
|
||||
// column projection. Used as GetCursorWithArtwork's id pre-pass.
|
||||
func (r *mediaFileRepository) GetAllIDs(options ...model.QueryOptions) ([]string, error) {
|
||||
sq := r.applyLibraryFilter(r.newSelect(options...).Columns("media_file.id"))
|
||||
if filtersNeedAnnotation(sq) {
|
||||
sq = r.withAnnotation(sq, "media_file.id")
|
||||
}
|
||||
ids := []string{}
|
||||
err := r.queryAllSlice(sq, &ids)
|
||||
return ids, err
|
||||
}
|
||||
|
||||
// GetCursorWithArtwork streams the same rows as GetCursor, hydrated, via the id pre-pass used by
|
||||
// the other cursors, rather than the scanner's bounded, unhydrated GetCursor itself.
|
||||
func (r *mediaFileRepository) GetCursorWithArtwork(options ...model.QueryOptions) (model.MediaFileCursor, error) {
|
||||
ids, err := r.GetAllIDs(options...)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
opts := chunkOptions(options, "media_file.id")
|
||||
return model.MediaFileCursor(streamByIDs(ids, func(chunk []string) (model.MediaFiles, error) {
|
||||
return r.GetAll(opts(chunk))
|
||||
})), nil
|
||||
}
|
||||
|
||||
// FindByPaths finds media files by their paths.
|
||||
// The paths can be library-qualified (format: "libraryID:path") or unqualified ("path").
|
||||
// Library-qualified paths search within the specified library, while unqualified paths
|
||||
|
||||
@ -588,7 +588,7 @@ func (api *Router) listSongs(ctx context.Context, opts model.QueryOptions, q ite
|
||||
// A full-library request (Finamp's sync, with MediaSources) is tens of thousands of fat rows.
|
||||
total, _ := repo.CountAll(model.QueryOptions{Filters: opts.Filters})
|
||||
open := streamCursor(func() (func(func(model.MediaFile, error) bool), error) {
|
||||
return repo.GetCursor(opts)
|
||||
return repo.GetCursorWithArtwork(opts)
|
||||
}, toItem)
|
||||
return streamed(open, int(total), opts.Offset), nil
|
||||
}
|
||||
|
||||
@ -123,6 +123,22 @@ func (m *MockMediaFileRepo) GetCursor(qo ...model.QueryOptions) (model.MediaFile
|
||||
}, nil
|
||||
}
|
||||
|
||||
func (m *MockMediaFileRepo) GetCursorWithArtwork(qo ...model.QueryOptions) (model.MediaFileCursor, error) {
|
||||
return m.GetCursor(qo...)
|
||||
}
|
||||
|
||||
func (m *MockMediaFileRepo) GetAllIDs(qo ...model.QueryOptions) ([]string, error) {
|
||||
all, err := m.GetAll(qo...)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
ids := make([]string, len(all))
|
||||
for i, mf := range all {
|
||||
ids[i] = mf.ID
|
||||
}
|
||||
return ids, nil
|
||||
}
|
||||
|
||||
func (m *MockMediaFileRepo) Put(mf *model.MediaFile) error {
|
||||
if m.Err {
|
||||
return errors.New("error")
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user