mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-31 07:30:32 +00:00
fix: address lyrics parser review feedback
This commit is contained in:
parent
0f3042b996
commit
81c3597018
@ -120,6 +120,19 @@ Another subtitle line`
|
||||
}))
|
||||
})
|
||||
|
||||
It("should parse embedded SRT blocks separated by whitespace-only blank lines", func() {
|
||||
content := "1\n00:00:01,000 --> 00:00:02,000\nFirst subtitle\n \n2\n00:00:03,000 --> 00:00:04,000\nSecond subtitle"
|
||||
|
||||
list, err := ParseEmbedded("eng", content)
|
||||
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(list).To(HaveLen(1))
|
||||
Expect(list[0].Line).To(Equal([]model.Line{
|
||||
{Start: ptr(int64(1000)), End: ptr(int64(2000)), Value: "First subtitle"},
|
||||
{Start: ptr(int64(3000)), End: ptr(int64(4000)), Value: "Second subtitle"},
|
||||
}))
|
||||
})
|
||||
|
||||
It("should keep embedded enhanced LRC cues", func() {
|
||||
content := "[00:01.00]<00:01.00>Lead <00:01.50>words"
|
||||
|
||||
|
||||
@ -83,7 +83,7 @@ func (l *lyricsService) getLyricsFromSource(ctx context.Context, mf *model.Media
|
||||
case strings.EqualFold(pattern, "embedded"):
|
||||
return fromEmbedded(ctx, mf)
|
||||
case strings.HasPrefix(pattern, "."):
|
||||
return fromExternalFile(ctx, mf, strings.ToLower(pattern))
|
||||
return fromExternalFile(ctx, mf, pattern)
|
||||
default:
|
||||
return l.fromPlugin(ctx, mf, pattern)
|
||||
}
|
||||
|
||||
@ -5,6 +5,7 @@ import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
|
||||
"github.com/navidrome/navidrome/conf"
|
||||
"github.com/navidrome/navidrome/conf/configtest"
|
||||
@ -192,6 +193,37 @@ var _ = Describe("sources", func() {
|
||||
Expect(list).To(Equal(ttmlLyrics))
|
||||
})
|
||||
|
||||
It("preserves configured sidecar suffix casing on case-sensitive filesystems", func() {
|
||||
dir, err := os.MkdirTemp("", "lyrics-case-*")
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
DeferCleanup(func() {
|
||||
Expect(os.RemoveAll(dir)).To(Succeed())
|
||||
})
|
||||
|
||||
probe := filepath.Join(dir, "CASECHECK")
|
||||
Expect(os.WriteFile(probe, []byte("probe"), 0644)).To(Succeed())
|
||||
_, err = os.Stat(filepath.Join(dir, "casecheck"))
|
||||
if err == nil {
|
||||
Skip("filesystem is case-insensitive")
|
||||
}
|
||||
Expect(os.IsNotExist(err)).To(BeTrue())
|
||||
|
||||
conf.Server.LyricsPriority = ".LRC"
|
||||
Expect(os.WriteFile(filepath.Join(dir, "song.LRC"), []byte("[00:01.00]Upper suffix"), 0644)).To(Succeed())
|
||||
|
||||
svc := lyrics.NewLyrics(nil)
|
||||
list, err := svc.GetLyrics(ctx, &model.MediaFile{
|
||||
LibraryPath: dir,
|
||||
Path: "song.mp3",
|
||||
})
|
||||
|
||||
Expect(err).To(BeNil())
|
||||
Expect(list).To(HaveLen(1))
|
||||
Expect(list[0].Line).To(Equal([]model.Line{
|
||||
{Start: ptr(int64(1000)), Value: "Upper suffix"},
|
||||
}))
|
||||
})
|
||||
|
||||
Context("Errors", func() {
|
||||
var RegularUserContext = XContext
|
||||
var isRegularUser = os.Getuid() != 0
|
||||
|
||||
@ -10,7 +10,10 @@ import (
|
||||
"github.com/navidrome/navidrome/utils/str"
|
||||
)
|
||||
|
||||
var srtTimeRegex = regexp.MustCompile(`^\s*(\d{1,2}):(\d{2}):(\d{2})[,.](\d{1,3})\s*$`)
|
||||
var (
|
||||
srtTimeRegex = regexp.MustCompile(`^\s*(\d{1,2}):(\d{2}):(\d{2})[,.](\d{1,3})\s*$`)
|
||||
srtBlockSeparatorRegex = regexp.MustCompile(`\n\s*\n`)
|
||||
)
|
||||
|
||||
func parseSRT(contents []byte) (model.LyricList, error) {
|
||||
return parseSRTWithLanguage(contents, "xxx")
|
||||
@ -51,7 +54,7 @@ func splitSRTBlocks(raw string) []string {
|
||||
return nil
|
||||
}
|
||||
|
||||
parts := strings.Split(raw, "\n\n")
|
||||
parts := srtBlockSeparatorRegex.Split(raw, -1)
|
||||
blocks := make([]string, 0, len(parts))
|
||||
for _, part := range parts {
|
||||
part = strings.TrimSpace(part)
|
||||
|
||||
@ -816,6 +816,20 @@ func contextHasRole(roles string, role string) bool {
|
||||
return false
|
||||
}
|
||||
|
||||
func appendTTMLRoles(existing string, roles string) string {
|
||||
for _, role := range strings.Fields(roles) {
|
||||
if contextHasRole(existing, role) {
|
||||
continue
|
||||
}
|
||||
if existing == "" {
|
||||
existing = role
|
||||
} else {
|
||||
existing += " " + role
|
||||
}
|
||||
}
|
||||
return existing
|
||||
}
|
||||
|
||||
func (p *ttmlParser) addMainLine(lang string, lineKey string, line model.Line) {
|
||||
lang = normalizeTTMLLang(lang)
|
||||
if _, ok := p.mainLinesByLang[lang]; !ok {
|
||||
@ -866,11 +880,7 @@ func (p *ttmlParser) childContext(attrs []xml.Attr, parent ttmlTimingContext) tt
|
||||
if role, ok := attrValue(attrs, "role"); ok {
|
||||
role = strings.TrimSpace(role)
|
||||
if role != "" {
|
||||
if ctx.role == "" {
|
||||
ctx.role = role
|
||||
} else if !strings.Contains(ctx.role, role) {
|
||||
ctx.role = ctx.role + " " + role
|
||||
}
|
||||
ctx.role = appendTTMLRoles(ctx.role, role)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -145,6 +145,30 @@ var _ = Describe("parseTTML", func() {
|
||||
Expect(line.Cue[2]).To(Equal(model.Cue{Start: ptr(int64(2000)), End: ptr(int64(2500)), Value: "echo", ByteStart: 6, ByteEnd: 9, AgentID: "__nd_bg__|main"}))
|
||||
})
|
||||
|
||||
It("should append role tokens exactly instead of using substring matches", func() {
|
||||
content := []byte(`<?xml version="1.0" encoding="UTF-8"?>
|
||||
<tt xmlns="http://www.w3.org/ns/ttml" xmlns:ttm="http://www.w3.org/ns/ttml#metadata">
|
||||
<body xml:lang="eng">
|
||||
<div>
|
||||
<p begin="00:01.000" end="00:03.000" ttm:role="not-x-bg"><span begin="00:01.000" end="00:01.400">Lead</span><span ttm:role="x-bg"><span begin="00:02.000" end="00:02.500">Echo</span></span></p>
|
||||
</div>
|
||||
</body>
|
||||
</tt>`)
|
||||
|
||||
list, err := parseTTML(content)
|
||||
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(list).To(HaveLen(1))
|
||||
Expect(list[0].Agents).To(Equal([]model.Agent{
|
||||
{ID: "main", Role: "main"},
|
||||
{ID: "__nd_bg__|main", Role: "bg"},
|
||||
}))
|
||||
Expect(list[0].Line).To(HaveLen(1))
|
||||
Expect(list[0].Line[0].Cue).To(HaveLen(2))
|
||||
Expect(list[0].Line[0].Cue[0].AgentID).To(Equal("main"))
|
||||
Expect(list[0].Line[0].Cue[1].AgentID).To(Equal("__nd_bg__|main"))
|
||||
})
|
||||
|
||||
It("should parse named TTML agents into main, voice, and group roles", func() {
|
||||
content := []byte(`<?xml version="1.0" encoding="UTF-8"?>
|
||||
<tt xmlns="http://www.w3.org/ns/ttml" xmlns:ttm="http://www.w3.org/ns/ttml#metadata">
|
||||
|
||||
@ -46,7 +46,7 @@ type Lyrics struct {
|
||||
}
|
||||
|
||||
// support the standard [mm:ss.mm], as well as [hh:*] and [*.mmm]
|
||||
const timeRegexString = `\[([0-9]{1,2}:)?([0-9]{1,2}):([0-9]{1,2})(.[0-9]{1,3})?\]`
|
||||
const timeRegexString = `\[([0-9]{1,2}:)?([0-9]{1,2}):([0-9]{1,2})(\.[0-9]{1,3})?\]`
|
||||
|
||||
var (
|
||||
// Should either be at the beginning of file, or beginning of line
|
||||
@ -55,7 +55,7 @@ var (
|
||||
lrcIdRegex = regexp.MustCompile(`\[(ar|ti|offset|lang):([^]]+)]`)
|
||||
|
||||
// Enhanced LRC: inline word-level timing markers like <00:12.34>
|
||||
enhancedLRCTimeString = `<([0-9]{1,2}:)?([0-9]{1,2}):([0-9]{1,2})(.[0-9]{1,3})?>`
|
||||
enhancedLRCTimeString = `<([0-9]{1,2}:)?([0-9]{1,2}):([0-9]{1,2})(\.[0-9]{1,3})?>`
|
||||
enhancedLRCRegex = regexp.MustCompile(enhancedLRCTimeString)
|
||||
)
|
||||
|
||||
@ -378,6 +378,10 @@ func NormalizeCueLines(lines []Line) []Line {
|
||||
copy(normalized, lines)
|
||||
|
||||
for i := range normalized {
|
||||
if len(normalized[i].Cue) > 0 {
|
||||
normalized[i].Cue = slices.Clone(normalized[i].Cue)
|
||||
}
|
||||
|
||||
var fallbackEnd *int64
|
||||
if normalized[i].End != nil {
|
||||
v := *normalized[i].End
|
||||
|
||||
@ -139,6 +139,15 @@ var _ = Describe("ToLyrics", func() {
|
||||
Expect(line1.Cue[1].End).To(BeNil())
|
||||
})
|
||||
|
||||
It("should not parse malformed Enhanced LRC timing markers", func() {
|
||||
lyrics, err := ToLyrics("xxx", "[00:01.00]<00:01a50>Not a marker")
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(lyrics.Synced).To(BeTrue())
|
||||
Expect(lyrics.Line).To(Equal([]Line{
|
||||
{Start: new(int64(1000)), Value: "<00:01a50>Not a marker"},
|
||||
}))
|
||||
})
|
||||
|
||||
It("should ignore Enhanced LRC markers and return plain lines when no markers present", func() {
|
||||
a, b := int64(1000), int64(3000)
|
||||
lyrics, err := ToLyrics("xxx", "[00:01.00]Plain line\n[00:03.00]Another plain line")
|
||||
@ -190,3 +199,30 @@ var _ = Describe("ToLyrics", func() {
|
||||
}))
|
||||
})
|
||||
})
|
||||
|
||||
var _ = Describe("NormalizeCueLines", func() {
|
||||
It("should not mutate caller cue slices when filling missing cue end times", func() {
|
||||
start0, start1, nextLineStart := int64(1000), int64(1500), int64(3000)
|
||||
lines := []Line{
|
||||
{
|
||||
Start: &start0,
|
||||
Value: "Some lyrics",
|
||||
Cue: []Cue{
|
||||
{Start: &start0, Value: "Some ", ByteStart: 0, ByteEnd: 4},
|
||||
{Start: &start1, Value: "lyrics", ByteStart: 5, ByteEnd: 10},
|
||||
},
|
||||
},
|
||||
{
|
||||
Start: &nextLineStart,
|
||||
Value: "Next line",
|
||||
},
|
||||
}
|
||||
|
||||
normalized := NormalizeCueLines(lines)
|
||||
|
||||
Expect(normalized[0].Cue[0].End).To(Equal(&start1))
|
||||
Expect(normalized[0].Cue[1].End).To(Equal(&nextLineStart))
|
||||
Expect(lines[0].Cue[0].End).To(BeNil())
|
||||
Expect(lines[0].Cue[1].End).To(BeNil())
|
||||
})
|
||||
})
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user