mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-08 10:27:08 +02:00
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.
This commit is contained in:
parent
af984bc817
commit
7b12c41fb1
8 changed files with 47 additions and 19 deletions
|
|
@ -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{
|
||||
|
|
|
|||
|
|
@ -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))
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
2
go.mod
2
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
|
||||
|
|
|
|||
4
go.sum
4
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=
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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})
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue