mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-08 10:27:08 +02:00
fix(lyrics): match external sidecar with alternate Unicode normalization - #4148
External lyrics files were resolved with a byte-exact path lookup, so a sidecar written next to a track in a different Unicode normalization form than the track's own filename was never found. macOS commonly stores names as NFD while Linux and Windows keep NFC, so a .lrc placed alongside a FLAC with non-ASCII characters (for example Japanese katakana with a dakuten) showed up as 'No lyrics' on the web. openSidecar now retries with the alternate NFC/NFD form when the exact path is missing, reusing the same approach the playlist importer already applies for cross-platform path matching. Closes #4148 Signed-off-by: Steve <stevejason639@gmail.com>
This commit is contained in:
parent
4ed7494a32
commit
f52cc623df
3 changed files with 115 additions and 1 deletions
|
|
@ -6,15 +6,19 @@ import (
|
|||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing/fstest"
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/conf"
|
||||
"github.com/navidrome/navidrome/conf/configtest"
|
||||
"github.com/navidrome/navidrome/core/lyrics"
|
||||
"github.com/navidrome/navidrome/core/storage/storagetest"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/tests"
|
||||
"github.com/navidrome/navidrome/utils"
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
"golang.org/x/text/unicode/norm"
|
||||
)
|
||||
|
||||
var _ = Describe("Lyrics", func() {
|
||||
|
|
@ -258,6 +262,38 @@ var _ = Describe("Lyrics", func() {
|
|||
}))
|
||||
})
|
||||
|
||||
It("resolves an external sidecar whose Unicode normalization differs from the track path (issue #4148)", func() {
|
||||
// Reproduces the reported case: a Japanese filename whose sidecar is stored
|
||||
// in a different normalization form than the track. An in-memory FS keeps
|
||||
// the mismatch byte-exact regardless of the host filesystem, which may
|
||||
// normalize names on lookup and hide the bug.
|
||||
const scheme = "fake-lyrics-e2e"
|
||||
fsys := &storagetest.FakeFS{}
|
||||
storagetest.Register(scheme, fsys)
|
||||
|
||||
// "ガ" is katakana KA plus a dakuten, so its NFC and NFD forms differ.
|
||||
const base = "03. 鬱P feat. 初音ミク - ガ"
|
||||
modTime := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)
|
||||
fsys.SetFiles(fstest.MapFS{
|
||||
norm.NFC.String(base) + ".flac": &fstest.MapFile{Data: []byte("audio"), ModTime: modTime},
|
||||
norm.NFD.String(base) + ".lrc": &fstest.MapFile{Data: []byte("[00:18.80]We're no strangers to love"), ModTime: modTime},
|
||||
})
|
||||
|
||||
// Default priority from the bug report: ".lrc,.txt,embedded".
|
||||
conf.Server.LyricsPriority = ".lrc,.txt,embedded"
|
||||
svc := lyrics.NewLyrics(nil, nil)
|
||||
list, err := svc.GetLyrics(ctx, &model.MediaFile{
|
||||
LibraryPath: scheme + ":///music",
|
||||
Path: norm.NFC.String(base) + ".flac",
|
||||
})
|
||||
|
||||
Expect(err).To(BeNil())
|
||||
Expect(list).To(HaveLen(1))
|
||||
Expect(list[0].Synced).To(BeTrue())
|
||||
Expect(list[0].Line).To(HaveLen(1))
|
||||
Expect(list[0].Line[0].Value).To(Equal("We're no strangers to love"))
|
||||
})
|
||||
|
||||
Context("Errors", func() {
|
||||
var RegularUserContext = XContext
|
||||
var isRegularUser = os.Getuid() != 0
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ import (
|
|||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/utils/ioutils"
|
||||
"golang.org/x/text/unicode/norm"
|
||||
)
|
||||
|
||||
func fromEmbedded(ctx context.Context, mf *model.MediaFile) (model.LyricList, error) {
|
||||
|
|
@ -39,7 +40,7 @@ func fromExternalFile(ctx context.Context, mf *model.MediaFile, suffix string) (
|
|||
return nil, fmt.Errorf("opening library filesystem: %w", err)
|
||||
}
|
||||
|
||||
f, err := fsys.Open(sidecarRelPath)
|
||||
f, err := openSidecar(fsys, sidecarRelPath)
|
||||
if errors.Is(err, fs.ErrNotExist) {
|
||||
log.Trace(ctx, "no lyrics found at path")
|
||||
return nil, nil
|
||||
|
|
@ -68,6 +69,27 @@ func fromExternalFile(ctx context.Context, mf *model.MediaFile, suffix string) (
|
|||
return list, nil
|
||||
}
|
||||
|
||||
// openSidecar opens the sidecar lyrics file, retrying with the alternate Unicode
|
||||
// normalization form when the exact path is missing. A sidecar written next to a
|
||||
// track can use a different form than the path stored for the media file (macOS
|
||||
// commonly yields NFD, while Linux and Windows typically keep NFC), so a
|
||||
// byte-exact lookup would miss a file that is really on disk. Issue #4148.
|
||||
func openSidecar(fsys fs.FS, relPath string) (fs.File, error) {
|
||||
f, err := fsys.Open(relPath)
|
||||
if !errors.Is(err, fs.ErrNotExist) {
|
||||
return f, err
|
||||
}
|
||||
|
||||
altPath := norm.NFD.String(relPath)
|
||||
if altPath == relPath {
|
||||
altPath = norm.NFC.String(relPath)
|
||||
}
|
||||
if altPath == relPath {
|
||||
return f, err
|
||||
}
|
||||
return fsys.Open(altPath)
|
||||
}
|
||||
|
||||
// fromPlugin attempts to load lyrics from a plugin with the given name.
|
||||
func (l *lyricsService) fromPlugin(ctx context.Context, mf *model.MediaFile, pluginName string) (model.LyricList, error) {
|
||||
if l.pluginLoader == nil {
|
||||
|
|
|
|||
|
|
@ -4,10 +4,14 @@ import (
|
|||
"context"
|
||||
"encoding/json"
|
||||
"path/filepath"
|
||||
"testing/fstest"
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/core/storage/storagetest"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
"golang.org/x/text/unicode/norm"
|
||||
)
|
||||
|
||||
var _ = Describe("sources", func() {
|
||||
|
|
@ -150,4 +154,56 @@ var _ = Describe("sources", func() {
|
|||
Expect(lyrics[0].Line[1].Value).To(Equal("UTF16 line two"))
|
||||
})
|
||||
})
|
||||
|
||||
// A sidecar written next to a track can use a different Unicode normalization
|
||||
// form than the track's own filename (macOS commonly stores names as NFD,
|
||||
// while Linux and Windows keep NFC). A byte-exact lookup then misses a sidecar
|
||||
// that is really on disk, so the retry has to try the other form. Issue #4148.
|
||||
Describe("fromExternalFile with mismatched Unicode normalization", func() {
|
||||
const scheme = "fake-lyrics-norm"
|
||||
// "ガ" is katakana KA plus a dakuten, so it has distinct NFC (a single
|
||||
// codepoint) and NFD (base plus combining mark) forms, unlike the plain
|
||||
// CJK ideographs around it. It mirrors the filename from the bug report.
|
||||
const base = "03. 鬱P feat. 初音ミク - ガ"
|
||||
|
||||
var fsys *storagetest.FakeFS
|
||||
|
||||
BeforeEach(func() {
|
||||
fsys = &storagetest.FakeFS{}
|
||||
storagetest.Register(scheme, fsys)
|
||||
})
|
||||
|
||||
lyricsFor := func(trackForm, sidecarForm norm.Form) (model.LyricList, error) {
|
||||
trackBase := trackForm.String(base)
|
||||
sidecarBase := sidecarForm.String(base)
|
||||
Expect(trackBase).ToNot(Equal(sidecarBase))
|
||||
|
||||
// A non-zero ModTime is required so the FakeFS directory-timestamp
|
||||
// bookkeeping settles instead of looping.
|
||||
modTime := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)
|
||||
fsys.SetFiles(fstest.MapFS{
|
||||
trackBase + ".flac": &fstest.MapFile{Data: []byte("audio"), ModTime: modTime},
|
||||
sidecarBase + ".lrc": &fstest.MapFile{Data: []byte("[00:18.80]We're no strangers to love"), ModTime: modTime},
|
||||
})
|
||||
|
||||
mf := &model.MediaFile{LibraryPath: scheme + ":///music", Path: trackBase + ".flac"}
|
||||
return fromExternalFile(ctx, mf, ".lrc")
|
||||
}
|
||||
|
||||
It("finds an NFD-encoded sidecar for an NFC track path", func() {
|
||||
lyrics, err := lyricsFor(norm.NFC, norm.NFD)
|
||||
|
||||
Expect(err).To(BeNil())
|
||||
Expect(lyrics).To(HaveLen(1))
|
||||
Expect(lyrics[0].Synced).To(BeTrue())
|
||||
})
|
||||
|
||||
It("finds an NFC-encoded sidecar for an NFD track path", func() {
|
||||
lyrics, err := lyricsFor(norm.NFD, norm.NFC)
|
||||
|
||||
Expect(err).To(BeNil())
|
||||
Expect(lyrics).To(HaveLen(1))
|
||||
Expect(lyrics[0].Synced).To(BeTrue())
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue