From 7b12c41fb144cb541baa64ecde22fe4cb6f2a31f Mon Sep 17 00:00:00 2001 From: Deluan Date: Mon, 7 Sep 2026 00:59:32 -0400 Subject: [PATCH] perf(scanner): take the picture fingerprint from go-taglib instead of hashing in Go Reading the embedded picture out of the WASM module to hash it in Go cost +28% in tag extraction and doubled allocations (93 real MP3s: 32.5ms -> 41.7ms, 91MiB -> 188MiB per pass), because the fork instantiates a module per file and the image read grows its memory and copies the bytes twice. go-taglib now fingerprints each embedded picture inside the module during the existing properties call (length, head, tail and 64 sampled stripes) and exposes it as ImageDesc.Hash. The extractor stores that value, so no picture bytes cross the boundary. Extraction is back to master speed (32.0ms, p=0.24) and the scanner benchmark is unchanged (10.88ms vs 10.89ms). Also adds tests for albums whose only art is an external file: no fingerprint is stored, tracks keep the album id, and hydration skips the album lookup. --- adapters/gotaglib/gotaglib.go | 6 +----- adapters/gotaglib/gotaglib_test.go | 7 +++--- core/artwork/image_store.go | 3 +-- go.mod | 2 +- go.sum | 4 ++-- model/artwork_id.go | 5 ----- persistence/artwork_hydration_test.go | 8 +++++++ scanner/scanner_test.go | 31 +++++++++++++++++++++++++++ 8 files changed, 47 insertions(+), 19 deletions(-) diff --git a/adapters/gotaglib/gotaglib.go b/adapters/gotaglib/gotaglib.go index 412f1a968..ab0b18bfb 100644 --- a/adapters/gotaglib/gotaglib.go +++ b/adapters/gotaglib/gotaglib.go @@ -24,9 +24,7 @@ import ( "github.com/navidrome/navidrome/core/artwork" "github.com/navidrome/navidrome/core/storage/local" "github.com/navidrome/navidrome/log" - "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/metadata" - "github.com/zeebo/xxh3" "go.senan.xyz/taglib" ) @@ -117,9 +115,7 @@ func (e extractor) extractMetadata(filePath string) (info *metadata.Info, err er var pictureHash string if len(props.Images) > 0 { - if data, err := f.Image(artwork.BestImageIndex(props.Images)); err == nil && len(data) > 0 { - pictureHash = model.FormatImageHash(xxh3.Hash(data)) - } + pictureHash = props.Images[artwork.BestImageIndex(props.Images)].Hash } return &metadata.Info{ diff --git a/adapters/gotaglib/gotaglib_test.go b/adapters/gotaglib/gotaglib_test.go index 4a31d8a58..4f1d7cc39 100644 --- a/adapters/gotaglib/gotaglib_test.go +++ b/adapters/gotaglib/gotaglib_test.go @@ -1,7 +1,6 @@ package gotaglib import ( - "fmt" "io/fs" "os" "strings" @@ -10,7 +9,6 @@ import ( "github.com/navidrome/navidrome/utils" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "github.com/zeebo/xxh3" "go.senan.xyz/taglib" ) @@ -38,9 +36,10 @@ var _ = Describe("Extractor", func() { Expect(m.Tags).To(HaveKeyWithValue("albumartist", []string{"Album Artist"})) Expect(m.HasPicture).To(BeTrue()) - img, err := taglib.ReadImage("tests/fixtures/test.mp3") + props, err := taglib.ReadProperties("tests/fixtures/test.mp3") Expect(err).NotTo(HaveOccurred()) - Expect(m.PictureHash).To(Equal(fmt.Sprintf("%016x", xxh3.Hash(img)))) + Expect(m.PictureHash).To(MatchRegexp(`^[0-9a-f]{16}$`)) + Expect(m.PictureHash).To(Equal(props.Images[0].Hash)) Expect(m.AudioProperties.Duration.String()).To(Equal("1.02s")) Expect(m.AudioProperties.BitRate).To(Equal(192)) Expect(m.AudioProperties.Channels).To(Equal(2)) diff --git a/core/artwork/image_store.go b/core/artwork/image_store.go index a15cd1ce3..5dbe727e4 100644 --- a/core/artwork/image_store.go +++ b/core/artwork/image_store.go @@ -14,7 +14,6 @@ import ( "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/consts" "github.com/navidrome/navidrome/log" - "github.com/navidrome/navidrome/model" "github.com/zeebo/xxh3" ) @@ -51,7 +50,7 @@ func hashImage(r io.Reader) (string, error) { if _, err := io.Copy(d, r); err != nil { return "", err } - return model.FormatImageHash(d.Sum64()), nil + return fmt.Sprintf("%016x", d.Sum64()), nil } // validHash guards path sharding: a malformed hash would slice-panic or inject path separators. diff --git a/go.mod b/go.mod index 4339b9c55..57b623afa 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/navidrome/navidrome go 1.27 // Fork to implement raw tags support -replace go.senan.xyz/taglib => github.com/deluan/go-taglib v0.0.0-20260905051825-df1d035571df +replace go.senan.xyz/taglib => github.com/deluan/go-taglib v0.0.0-20260907044645-42bf076d332f require ( github.com/Masterminds/squirrel v1.5.4 diff --git a/go.sum b/go.sum index 71d9facfd..bfd5e1738 100644 --- a/go.sum +++ b/go.sum @@ -29,8 +29,8 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/decred/dcrd/dcrec/secp256k1/v4 v4.4.1 h1:5RVFMOWjMyRy8cARdy79nAmgYw3hK/4HUq48LQ6Wwqo= github.com/decred/dcrd/dcrec/secp256k1/v4 v4.4.1/go.mod h1:ZXNYxsqcloTdSy/rNShjYzMhyjf0LaoftYK0p+A3h40= -github.com/deluan/go-taglib v0.0.0-20260905051825-df1d035571df h1:LdLQVAWVc6hCzqnrfVIEXOhP+r0iSit+EvsXwZDyL70= -github.com/deluan/go-taglib v0.0.0-20260905051825-df1d035571df/go.mod h1:QGxQ4Z1IWyY9w56xNEFjYAaWE8uSxA/gneQ7RPcFJrY= +github.com/deluan/go-taglib v0.0.0-20260907044645-42bf076d332f h1:jepb57KAgNZ00WBo9+sEugxCejlJJCsJ+o+wzliXY1E= +github.com/deluan/go-taglib v0.0.0-20260907044645-42bf076d332f/go.mod h1:QGxQ4Z1IWyY9w56xNEFjYAaWE8uSxA/gneQ7RPcFJrY= github.com/deluan/rest v0.0.0-20211102003136-6260bc399cbf h1:tb246l2Zmpt/GpF9EcHCKTtwzrd0HGfEmoODFA/qnk4= github.com/deluan/rest v0.0.0-20211102003136-6260bc399cbf/go.mod h1:tSgDythFsl0QgS/PFWfIZqcJKnkADWneY80jaVRlqK8= github.com/deluan/sanitize v0.0.0-20241120162836-fdfd8fdfaa55 h1:wSCnggTs2f2ji6nFwQmfwgINcmSMj0xF0oHnoyRSPe4= diff --git a/model/artwork_id.go b/model/artwork_id.go index 0854e6a65..e827e935a 100644 --- a/model/artwork_id.go +++ b/model/artwork_id.go @@ -110,11 +110,6 @@ func ParseArtworkID(id string) (ArtworkID, error) { return parsedID, nil } -// FormatImageHash renders an XXH3-64 digest as the 16-hex image hash used in artwork ids and rows. -func FormatImageHash(sum uint64) string { - return fmt.Sprintf("%016x", sum) -} - // isImageHash reports whether s is a 16-char lowercase-hex XXH3-64 content hash. func isImageHash(s string) bool { if len(s) != 16 { diff --git a/persistence/artwork_hydration_test.go b/persistence/artwork_hydration_test.go index 9e06719b0..f510ed0b0 100644 --- a/persistence/artwork_hydration_test.go +++ b/persistence/artwork_hydration_test.go @@ -280,6 +280,14 @@ var _ = Describe("Artwork hydration", func() { Expect(byID["2002"].ImageAbsent).To(BeFalse()) }) + It("skips the album lookup when no track on the page embeds a picture", func() { + mfs := model.MediaFiles{{ID: "x1", AlbumID: "101"}, {ID: "x2", AlbumID: "102", HasCoverArt: true}} + // A nil builder panics on any query, so reaching the assertions proves no lookup ran. + Expect(func() { hydrateAlbumEmbedArtHashes(ctx, nil, mfs) }).ToNot(Panic()) + Expect(mfs[0].AlbumEmbedArtHash).To(BeEmpty()) + Expect(mfs[1].AlbumEmbedArtHash).To(BeEmpty()) + }) + It("defers a track to its album when its embedded picture is the album's cover", func() { setCover("1001", true) setCover("1002", true) diff --git a/scanner/scanner_test.go b/scanner/scanner_test.go index 3387a5a7f..1f9529d03 100644 --- a/scanner/scanner_test.go +++ b/scanner/scanner_test.go @@ -445,6 +445,37 @@ var _ = Describe("Scanner", Ordered, func() { }) }) + Context("Album art only in an external file", func() { + BeforeEach(func() { + revolver := template(_t{"albumartist": "The Beatles", "album": "Revolver", "year": 1966}) + createFS(fstest.MapFS{ + "The Beatles/Revolver/01 - Taxman.mp3": revolver(track(1, "Taxman")), + "The Beatles/Revolver/02 - Eleanor Rigby.mp3": revolver(track(2, "Eleanor Rigby")), + "The Beatles/Revolver/cover.jpg": &fstest.MapFile{Data: []byte("jpeg bytes")}, + }) + }) + + It("stores no picture hash and keeps every track on the album art", func() { + conf.Server.EnableMediaFileCoverArt = true + Expect(runScanner(ctx, true)).To(Succeed()) + + albums, err := ds.Album(ctx).GetAll() + Expect(err).ToNot(HaveOccurred()) + Expect(albums).To(HaveLen(1)) + Expect(albums[0].EmbedArtPath).To(BeEmpty()) + Expect(albums[0].EmbedArtHash).To(BeEmpty()) + + mfs, err := ds.MediaFile(ctx).GetAll() + Expect(err).ToNot(HaveOccurred()) + Expect(mfs).To(HaveLen(2)) + for _, mf := range mfs { + Expect(mf.HasCoverArt).To(BeFalse()) + Expect(mf.EmbedArtHash).To(BeEmpty()) + Expect(mf.CoverArtID().Kind).To(Equal(model.KindAlbumArtwork)) + } + }) + }) + Context("Tracks embedding the album cover", func() { BeforeEach(func() { revolver := template(_t{"albumartist": "The Beatles", "album": "Revolver", "year": 1966, "disc": 1})