diff --git a/core/lyrics/lyrics_test.go b/core/lyrics/lyrics_test.go index b00bcd576..040815bfd 100644 --- a/core/lyrics/lyrics_test.go +++ b/core/lyrics/lyrics_test.go @@ -59,19 +59,16 @@ var _ = Describe("Lyrics", func() { Line: []model.Line{ { Start: new(int64(1000)), - End: new(int64(3000)), Value: "Lead words", Cue: []model.Cue{ { Start: new(int64(1000)), - End: new(int64(1500)), Value: "Lead ", ByteStart: 0, ByteEnd: 4, }, { Start: new(int64(1500)), - End: new(int64(3000)), Value: "words", ByteStart: 5, ByteEnd: 9, @@ -230,7 +227,7 @@ var _ = Describe("Lyrics", func() { })) }) - It("returns a non-Lyricsfile YAML sidecar as plain text, shadowing lower-priority sources", func() { + It("skips a non-Lyricsfile YAML sidecar and resolves the next source", func() { dir, err := os.MkdirTemp("", "lyrics-yaml-fallback-*") Expect(err).ToNot(HaveOccurred()) DeferCleanup(func() { @@ -247,17 +244,33 @@ var _ = Describe("Lyrics", func() { Path: "song.mp3", }) - // ParseLyrics falls back to plain text for any suffix when the content - // doesn't match the structured format, so the .yaml hit is non-empty and - // shadows the lower-priority .lrc entirely. Expect(err).To(BeNil()) Expect(list).To(HaveLen(1)) - Expect(list[0].Synced).To(BeFalse()) + Expect(list[0].Synced).To(BeTrue()) Expect(list[0].Line).To(Equal([]model.Line{ - {Value: "title: not lyricsfile"}, + {Start: new(int64(1000)), Value: "Fallback line"}, })) }) + It("skips a malformed claimed sidecar and resolves the next source", func() { + dir, err := os.MkdirTemp("", "lyrics-malformed-fallback-*") + Expect(err).ToNot(HaveOccurred()) + DeferCleanup(func() { + Expect(os.RemoveAll(dir)).To(Succeed()) + }) + + Expect(os.WriteFile(filepath.Join(dir, "song.ttml"), []byte(`

broken`), 0600)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(dir, "song.lrc"), []byte("[00:01.00]Fallback line"), 0600)).To(Succeed()) + + conf.Server.LyricsPriority = ".ttml,.lrc" + svc := lyrics.NewLyrics(nil, nil) + list, err := svc.GetLyrics(ctx, &model.MediaFile{LibraryPath: dir, Path: "song.mp3"}) + + Expect(err).ToNot(HaveOccurred()) + Expect(list).To(HaveLen(1)) + Expect(list[0].Line).To(Equal([]model.Line{{Start: new(int64(1000)), Value: "Fallback line"}})) + }) + Context("Errors", func() { var RegularUserContext = XContext var isRegularUser = os.Getuid() != 0 diff --git a/model/lyrics_lrc.go b/model/lyrics_lrc.go index 2cccb9d51..20a01b554 100644 --- a/model/lyrics_lrc.go +++ b/model/lyrics_lrc.go @@ -92,13 +92,15 @@ func parseLRC(language, text string) (*Lyrics, error) { } if validLine { - value, baseCues := parseEnhancedLine(priorLine) + value, baseCues, baseEnd := parseEnhancedLine(priorLine) for idx := range timestamps { startCopy := timestamps[idx] + shift := timestamps[idx] - timestamps[0] structuredLines = append(structuredLines, Line{ Start: &startCopy, + End: shiftELRCTime(baseEnd, shift), Value: value, - Cue: shiftELRCCues(baseCues, timestamps[idx]-timestamps[0]), + Cue: shiftELRCCues(baseCues, shift), }) } timestamps = nil @@ -143,13 +145,15 @@ func parseLRC(language, text string) (*Lyrics, error) { } if validLine { - value, baseCues := parseEnhancedLine(priorLine) + value, baseCues, baseEnd := parseEnhancedLine(priorLine) for idx := range timestamps { startCopy := timestamps[idx] + shift := timestamps[idx] - timestamps[0] structuredLines = append(structuredLines, Line{ Start: &startCopy, + End: shiftELRCTime(baseEnd, shift), Value: value, - Cue: shiftELRCCues(baseCues, timestamps[idx]-timestamps[0]), + Cue: shiftELRCCues(baseCues, shift), }) } } @@ -162,23 +166,23 @@ func parseLRC(language, text string) (*Lyrics, error) { }) } - lyrics := Lyrics{ + lyrics := NormalizeLyrics(Lyrics{ DisplayArtist: artist, DisplayTitle: title, Lang: language, - Line: normalizeCueLines(structuredLines), + Line: structuredLines, Offset: offset, Synced: synced, - } + }) return &lyrics, nil } // parseEnhancedLine extracts word-level timing cues from Enhanced LRC inline markers // and computes UTF-8 byte offsets against the final stripped line value. -func parseEnhancedLine(text string) (string, []Cue) { +func parseEnhancedLine(text string) (string, []Cue, *int64) { matches := enhancedLRCRegex.FindAllStringSubmatchIndex(text, -1) if len(matches) == 0 { - return strings.TrimSpace(text), nil + return strings.TrimSpace(text), nil, nil } type segment struct { @@ -189,6 +193,9 @@ func parseEnhancedLine(text string) (string, []Cue) { segments := make([]segment, 0, len(matches)) var rawValue strings.Builder + // Enhanced LRC permits text before the first inline timestamp. It is part of + // the visible line even though it has no word-level timing of its own. + rawValue.WriteString(text[:matches[0][0]]) var trailingEnd *int64 for i, match := range matches { timeMs, err := parseTime( @@ -234,7 +241,7 @@ func parseEnhancedLine(text string) (string, []Cue) { } if len(segments) == 0 { - return strings.TrimSpace(stripEnhancedMarkers(text)), nil + return strings.TrimSpace(stripEnhancedMarkers(text)), nil, trailingEnd } finalRaw := rawValue.String() @@ -266,7 +273,7 @@ func parseEnhancedLine(text string) (string, []Cue) { cues[len(cues)-1].End = trailingEnd } - return strings.TrimSpace(finalRaw), cues + return strings.TrimSpace(finalRaw), cues, trailingEnd } // adjustGroup remaps a capture group index from the original match to our rewritten "[...]" string. @@ -286,6 +293,14 @@ func stripEnhancedMarkers(text string) string { return enhancedLRCRegex.ReplaceAllString(text, "") } +func shiftELRCTime(value *int64, offsetMs int64) *int64 { + if value == nil { + return nil + } + shifted := *value + offsetMs + return &shifted +} + // shiftELRCCues returns a deep copy of baseCues with each cue's Start/End // timestamps shifted by offsetMs. Inline ELRC word markers parse to absolute // timestamps anchored at the line's first occurrence, so repeated-line LRC diff --git a/model/lyrics_lrc_test.go b/model/lyrics_lrc_test.go index 87514caf1..5d9dffb58 100644 --- a/model/lyrics_lrc_test.go +++ b/model/lyrics_lrc_test.go @@ -118,17 +118,17 @@ var _ = Describe("parseLRC", func() { line0 := lyrics.Line[0] Expect(line0.Start).To(Equal(&t1000)) - Expect(line0.End).To(Equal(&t3000)) + Expect(line0.End).To(BeNil()) Expect(line0.Value).To(Equal("Some lyrics here")) Expect(line0.Cue).To(Equal([]Cue{ - {Start: &t1000, End: &t1500, Value: "Some ", ByteStart: 0, ByteEnd: 4}, - {Start: &t1500, End: &t2000, Value: "lyrics ", ByteStart: 5, ByteEnd: 11}, - {Start: &t2000, End: &t3000, Value: "here", ByteStart: 12, ByteEnd: 15}, + {Start: &t1000, Value: "Some ", ByteStart: 0, ByteEnd: 4}, + {Start: &t1500, Value: "lyrics ", ByteStart: 5, ByteEnd: 11}, + {Start: &t2000, Value: "here", ByteStart: 12, ByteEnd: 15}, })) line1 := lyrics.Line[1] Expect(line1.Start).To(Equal(&t3000)) - Expect(line1.End).To(Equal(&t3500)) + Expect(line1.End).To(BeNil()) Expect(line1.Value).To(Equal("More words")) Expect(line1.Cue).To(Equal([]Cue{ {Start: &t3000, Value: "More ", ByteStart: 0, ByteEnd: 4}, @@ -153,14 +153,12 @@ var _ = Describe("parseLRC", func() { Expect(lyrics.Line).To(HaveLen(3)) t1000, t1500, t5000, t5500 := int64(1000), int64(1500), int64(5000), int64(5500) - t3000 := int64(3000) - Expect(lyrics.Line[0].Cue).To(Equal([]Cue{ - {Start: &t1000, End: &t1500, Value: "Some ", ByteStart: 0, ByteEnd: 4}, - {Start: &t1500, End: &t3000, Value: "lyrics", ByteStart: 5, ByteEnd: 10}, + {Start: &t1000, Value: "Some ", ByteStart: 0, ByteEnd: 4}, + {Start: &t1500, Value: "lyrics", ByteStart: 5, ByteEnd: 10}, })) Expect(lyrics.Line[0].Value).To(Equal("Some lyrics")) - Expect(lyrics.Line[0].End).To(Equal(&t3000)) + Expect(lyrics.Line[0].End).To(BeNil()) Expect(lyrics.Line[1].Cue).To(BeNil()) Expect(lyrics.Line[1].Value).To(Equal("Plain line")) @@ -188,6 +186,34 @@ var _ = Describe("parseLRC", func() { })) }) + It("preserves text before the first Enhanced LRC marker", func() { + lyrics, err := parseLRC("xxx", "[00:01.00]Prelude <00:01.50>timed") + Expect(err).ToNot(HaveOccurred()) + Expect(lyrics.Line).To(HaveLen(1)) + Expect(lyrics.Line[0].Value).To(Equal("Prelude timed")) + Expect(lyrics.Line[0].Cue).To(Equal([]Cue{{ + Start: new(int64(1500)), Value: "timed", ByteStart: 8, ByteEnd: 12, + }})) + }) + + It("preserves timestamp-only lines as explicit pauses", func() { + lyrics, err := parseLRC("xxx", "[00:01.00]Before\n[00:02.00]\n[00:03.00]After") + Expect(err).ToNot(HaveOccurred()) + Expect(lyrics.Line).To(Equal([]Line{ + {Start: new(int64(1000)), Value: "Before"}, + {Start: new(int64(2000)), Value: ""}, + {Start: new(int64(3000)), Value: "After"}, + })) + }) + + It("preserves a trailing end on prefix-only Enhanced LRC lines", func() { + lyrics, err := parseLRC("xxx", "[00:01.00]Prelude<00:02.00>") + Expect(err).ToNot(HaveOccurred()) + Expect(lyrics.Line).To(Equal([]Line{{ + Start: new(int64(1000)), End: new(int64(2000)), Value: "Prelude", + }})) + }) + It("should use a trailing Enhanced LRC marker as the end of the last word", func() { lyrics, err := parseLRC("xxx", "[00:01.00]<00:01.00>Some <00:01.50>lyrics<00:02.00>\n[00:30.00]Instrumental over") Expect(err).ToNot(HaveOccurred()) @@ -203,6 +229,15 @@ var _ = Describe("parseLRC", func() { })) }) + It("preserves an explicit trailing marker equal to the next line start", func() { + lyrics, err := parseLRC("xxx", "[00:01.00]<00:01.00>Some lyrics<00:02.00>\n[00:02.00]Next line") + Expect(err).ToNot(HaveOccurred()) + Expect(lyrics.Line).To(HaveLen(2)) + Expect(lyrics.Line[0].End).To(Equal(new(int64(2000)))) + Expect(lyrics.Line[0].Cue).To(HaveLen(1)) + Expect(lyrics.Line[0].Cue[0].End).To(Equal(new(int64(2000)))) + }) + It("should shift a trailing Enhanced LRC marker for repeated line occurrences", func() { lyrics, err := parseLRC("xxx", "[00:10.00][00:30.00]<00:10.10>Hello <00:10.50>world<00:10.90>") Expect(err).ToNot(HaveOccurred()) @@ -237,15 +272,15 @@ var _ = Describe("parseLRC", func() { t30500 := int64(30500) Expect(lyrics.Line[0].Start).To(Equal(&t10000)) - Expect(lyrics.Line[0].End).To(Equal(&t30000)) + Expect(lyrics.Line[0].End).To(BeNil()) Expect(lyrics.Line[0].Value).To(Equal("Hello world")) Expect(lyrics.Line[0].Cue).To(Equal([]Cue{ - {Start: &t10100, End: &t10500, Value: "Hello ", ByteStart: 0, ByteEnd: 5}, - {Start: &t10500, End: &t30000, Value: "world", ByteStart: 6, ByteEnd: 10}, + {Start: &t10100, Value: "Hello ", ByteStart: 0, ByteEnd: 5}, + {Start: &t10500, Value: "world", ByteStart: 6, ByteEnd: 10}, })) Expect(lyrics.Line[1].Start).To(Equal(&t30000)) - Expect(lyrics.Line[1].End).To(Equal(&t30500)) + Expect(lyrics.Line[1].End).To(BeNil()) Expect(lyrics.Line[1].Value).To(Equal("Hello world")) Expect(lyrics.Line[1].Cue).To(Equal([]Cue{ {Start: &t30100, Value: "Hello ", ByteStart: 0, ByteEnd: 5}, diff --git a/model/lyrics_lyricsfile.go b/model/lyrics_lyricsfile.go index 49d416f3f..5bfa849d4 100644 --- a/model/lyrics_lyricsfile.go +++ b/model/lyrics_lyricsfile.go @@ -3,6 +3,7 @@ package model import ( "bytes" "fmt" + "sort" "strings" "github.com/navidrome/navidrome/utils/str" @@ -50,7 +51,7 @@ func parseLyricsfile(lang string, contents []byte) (LyricList, error) { } if doc.Metadata.Instrumental { - return LyricList{normalizeLyrics(lyrics)}, nil + return LyricList{NormalizeLyrics(lyrics)}, nil } if len(doc.Lines) == 0 { @@ -59,18 +60,35 @@ func parseLyricsfile(lang string, contents []byte) (LyricList, error) { return nil, nil } lyrics.Line = lines - return LyricList{normalizeLyrics(lyrics)}, nil + return LyricList{NormalizeLyrics(lyrics)}, nil } + if err := validateLyricsfileLineStarts(doc.Lines); err != nil { + if lines := buildPlainLyricsfileLines(doc.Plain); len(lines) > 0 { + lyrics.Line = lines + return LyricList{NormalizeLyrics(lyrics)}, nil + } + return nil, err + } lines, agents := buildLyricsfileLines(doc.Lines) lyrics.Line = lines lyrics.Agents = agents lyrics.Synced = true - return LyricList{normalizeLyrics(lyrics)}, nil + return LyricList{NormalizeLyrics(lyrics)}, nil } const lyricsfileVersion = "1.0" +func hasLyricsfileVersion(contents []byte) bool { + var header struct { + Version string `yaml:"version"` + } + if err := yaml.Unmarshal(contents, &header); err != nil { + return false + } + return strings.TrimSpace(header.Version) == lyricsfileVersion +} + type lyricsfileDocument struct { Version string `yaml:"version"` Metadata lyricsfileMetadata `yaml:"metadata"` @@ -90,17 +108,26 @@ type lyricsfileMetadata struct { type lyricsfileLineEntry struct { Text string `yaml:"text"` - StartMs int64 `yaml:"start_ms"` + StartMs *int64 `yaml:"start_ms"` EndMs *int64 `yaml:"end_ms"` Words []lyricsfileWordEntry `yaml:"words"` } type lyricsfileWordEntry struct { Text string `yaml:"text"` - StartMs int64 `yaml:"start_ms"` + StartMs *int64 `yaml:"start_ms"` EndMs *int64 `yaml:"end_ms"` } +func validateLyricsfileLineStarts(entries []lyricsfileLineEntry) error { + for i, entry := range entries { + if entry.StartMs == nil || *entry.StartMs < 0 { + return fmt.Errorf("Lyricsfile line %d has a missing or invalid start_ms", i+1) + } + } + return nil +} + // buildLyricsfileLines converts YAML line entries to model.Line entries with // per-cue AgentIDs assigned by streaming overlap clustering (lowest-free // voice ID). The Agents slice is emitted only when at least one cue carries @@ -112,28 +139,30 @@ func buildLyricsfileLines(entries []lyricsfileLineEntry) ([]Line, []Agent) { return nil, nil } - // Resolved end timestamps per entry: explicit end_ms, final word end_ms, - // then the next entry's start. The last entry's end stays nil when no - // explicit or word-level end is available. + // Only explicit line ends and trustworthy final-word ends are exact. The + // next line's start remains a cue fallback and never becomes Line.End. ends := make([]*int64, len(entries)) for i := range entries { - var nextStart *int64 - if i+1 < len(entries) { - v := entries[i+1].StartMs - nextStart = &v - } - ends[i] = lyricsfileLineEnd(entries[i], nextStart) + ends[i] = lyricsfileLineEnd(entries[i]) } + // Allocate overlap voices chronologically, then write them back in source + // display order. Out-of-order documents therefore keep their intended layout. + order := make([]int, len(entries)) + for i := range order { + order[i] = i + } + sort.SliceStable(order, func(i, j int) bool { + return *entries[order[i]].StartMs < *entries[order[j]].StartMs + }) active := map[int]int64{} maxVoice := -1 - anyCues := false - lines := make([]Line, 0, len(entries)) - - for i, entry := range entries { - for vID, vEnd := range active { - if vEnd <= entry.StartMs { - delete(active, vID) + voiceByEntry := make([]int, len(entries)) + for _, entryIndex := range order { + start := *entries[entryIndex].StartMs + for voiceID, voiceEnd := range active { + if voiceEnd <= start { + delete(active, voiceID) } } @@ -144,17 +173,28 @@ func buildLyricsfileLines(entries []lyricsfileLineEntry) ([]Line, []Agent) { } voiceID++ } - if voiceID > maxVoice { - maxVoice = voiceID - } + voiceByEntry[entryIndex] = voiceID + maxVoice = max(maxVoice, voiceID) + end := start + if ends[entryIndex] != nil && *ends[entryIndex] >= start { + end = *ends[entryIndex] + } + active[voiceID] = end + } + + anyCues := false + lines := make([]Line, 0, len(entries)) + + for i, entry := range entries { + voiceID := voiceByEntry[i] agentID := fmt.Sprintf("voice-%d", voiceID) cues, value := wordsToLineCues(entry, agentID) if len(cues) > 0 { anyCues = true } - startMs := entry.StartMs + startMs := *entry.StartMs line := Line{ Start: &startMs, End: ends[i], @@ -162,14 +202,6 @@ func buildLyricsfileLines(entries []lyricsfileLineEntry) ([]Line, []Agent) { Cue: cues, } lines = append(lines, line) - - var endMs int64 - if ends[i] != nil { - endMs = *ends[i] - } else { - endMs = entry.StartMs - } - active[voiceID] = endMs } // Monophonic source, or attribution that has nowhere to land: emit no @@ -197,25 +229,32 @@ func buildLyricsfileLines(entries []lyricsfileLineEntry) ([]Line, []Agent) { return lines, agents } -func lyricsfileLineEnd(entry lyricsfileLineEntry, nextStart *int64) *int64 { +func lyricsfileLineEnd(entry lyricsfileLineEntry) *int64 { if entry.EndMs != nil { v := *entry.EndMs return &v } - if len(entry.Words) > 0 { + if lyricsfileWordTimingsValid(entry.Words) && len(entry.Words) > 0 { lastWord := entry.Words[len(entry.Words)-1] if lastWord.EndMs != nil { v := *lastWord.EndMs return &v } } - if nextStart != nil { - v := *nextStart - return &v - } return nil } +func lyricsfileWordTimingsValid(words []lyricsfileWordEntry) bool { + var previous int64 + for i, word := range words { + if word.StartMs == nil || *word.StartMs < 0 || (i > 0 && *word.StartMs < previous) { + return false + } + previous = *word.StartMs + } + return true +} + func buildPlainLyricsfileLines(plain string) []Line { plain = str.SanitizeText(plain) rawLines := strings.Split(plain, "\n") @@ -241,16 +280,25 @@ func wordsToLineCues(entry lyricsfileLineEntry, agentID string) ([]Cue, string) return nil, str.SanitizeText(entry.Text) } + values := make([]string, len(entry.Words)) var sb strings.Builder - for _, w := range entry.Words { - sb.WriteString(w.Text) + for i, word := range entry.Words { + values[i] = str.SanitizeText(word.Text) + sb.WriteString(values[i]) } lineValue := sb.String() + if !lyricsfileWordTimingsValid(entry.Words) { + return nil, lineValue + } cues := make([]Cue, len(entry.Words)) cursor := 0 for i, w := range entry.Words { - valueBytes := len(w.Text) + value := values[i] + if value == "" { + return nil, lineValue + } + valueBytes := len(value) bs := cursor be := bs if valueBytes > 0 { @@ -258,10 +306,10 @@ func wordsToLineCues(entry lyricsfileLineEntry, agentID string) ([]Cue, string) cursor = be + 1 } - s := w.StartMs + s := *w.StartMs cue := Cue{ Start: &s, - Value: w.Text, + Value: value, ByteStart: bs, ByteEnd: be, AgentID: agentID, diff --git a/model/lyrics_lyricsfile_test.go b/model/lyrics_lyricsfile_test.go index 45899cda7..b2d45500b 100644 --- a/model/lyrics_lyricsfile_test.go +++ b/model/lyrics_lyricsfile_test.go @@ -55,8 +55,7 @@ lines: Expect(l.Line).To(HaveLen(2)) Expect(*l.Line[0].Start).To(Equal(int64(18800))) - Expect(l.Line[0].End).ToNot(BeNil()) - Expect(*l.Line[0].End).To(Equal(int64(22801))) + Expect(l.Line[0].End).To(BeNil()) Expect(l.Line[0].Value).To(Equal("We're no strangers to love")) Expect(l.Line[0].Cue).To(BeNil()) @@ -297,4 +296,103 @@ lines: Expect(l.Line[0].Cue).To(BeNil()) Expect(l.Line[1].Cue).To(BeNil()) }) + + It("distinguishes an omitted line start from an explicit zero", func() { + explicitZero := `version: "1.0" +lines: + - text: "At zero" + start_ms: 0 +` + lyrics, err := parseLyricsfile("", []byte(explicitZero)) + Expect(err).ToNot(HaveOccurred()) + Expect(lyrics[0].Line[0].Start).To(Equal(new(int64(0)))) + + missing := `version: "1.0" +lines: + - text: "Missing start" +plain: | + Safe fallback +` + lyrics, err = parseLyricsfile("", []byte(missing)) + Expect(err).ToNot(HaveOccurred()) + Expect(lyrics[0].Synced).To(BeFalse()) + Expect(lyrics[0].Line).To(Equal([]Line{{Value: "Safe fallback"}})) + + _, err = parseLyricsfile("", []byte(`version: "1.0" +lines: + - text: "Missing start" +`)) + Expect(err).To(MatchError(ContainSubstring("missing or invalid start_ms"))) + }) + + It("sanitizes word text before computing UTF-8 offsets", func() { + input := `version: "1.0" +lines: + - text: "ignored" + start_ms: 0 + end_ms: 1000 + words: + - text: "Hi " + start_ms: 0 + end_ms: 500 + - text: "世" + start_ms: 500 + end_ms: 1000 +` + lyrics, err := parseLyricsfile("", []byte(input)) + Expect(err).ToNot(HaveOccurred()) + line := lyrics[0].Line[0] + Expect(line.Value).To(Equal("Hi 世")) + Expect(line.Cue).To(Equal([]Cue{ + {Start: new(int64(0)), End: new(int64(500)), Value: "Hi ", ByteStart: 0, ByteEnd: 2}, + {Start: new(int64(500)), End: new(int64(1000)), Value: "世", ByteStart: 3, ByteEnd: 5}, + })) + }) + + DescribeTable("retains reconstructed text but drops invalid word timing", + func(words string) { + input := `version: "1.0" +lines: + - text: "ignored" + start_ms: 1000 + words: +` + words + lyrics, err := parseLyricsfile("", []byte(input)) + Expect(err).ToNot(HaveOccurred()) + Expect(lyrics[0].Line[0].Value).To(Equal("hello")) + Expect(lyrics[0].Line[0].Cue).To(BeNil()) + }, + Entry("missing word start", " - text: \"hello\"\n"), + Entry("out-of-order word starts", " - text: \"hel\"\n start_ms: 1500\n - text: \"lo\"\n start_ms: 1400\n"), + ) + + It("assigns overlapping voices chronologically while retaining display order", func() { + input := `version: "1.0" +lines: + - text: "Later" + start_ms: 3000 + end_ms: 5000 + words: + - text: "Later" + start_ms: 3000 + end_ms: 5000 + - text: "Earlier" + start_ms: 1000 + end_ms: 4000 + words: + - text: "Earlier" + start_ms: 1000 + end_ms: 4000 +` + lyrics, err := parseLyricsfile("", []byte(input)) + Expect(err).ToNot(HaveOccurred()) + Expect(lyrics[0].Line[0].Value).To(Equal("Later")) + Expect(lyrics[0].Line[0].Cue[0].AgentID).To(Equal("voice-1")) + Expect(lyrics[0].Line[1].Value).To(Equal("Earlier")) + Expect(lyrics[0].Line[1].Cue[0].AgentID).To(Equal("voice-0")) + Expect(lyrics[0].Agents).To(Equal([]Agent{ + {ID: "voice-0", Role: "main"}, + {ID: "voice-1", Role: "voice"}, + })) + }) }) diff --git a/model/lyrics_normalize.go b/model/lyrics_normalize.go index a7d4b4e2a..c14522493 100644 --- a/model/lyrics_normalize.go +++ b/model/lyrics_normalize.go @@ -2,99 +2,158 @@ package model import ( "slices" + "strings" + "unicode/utf8" "github.com/navidrome/navidrome/utils/gg" ) -func normalizeLyrics(lyrics Lyrics) Lyrics { - lyrics.Line = normalizeCueLines(lyrics.Line) - if len(lyrics.Agents) == 0 { - lyrics.Agents = nil - } - return lyrics +// NormalizeLyrics returns a canonical, independent copy of lyrics. It keeps +// source line order and overlapping vocal timelines intact while repairing +// timing only when the source contains enough information to do so. +func NormalizeLyrics(lyrics Lyrics) Lyrics { + out := cloneLyrics(lyrics) + out.Line = normalizeCueLines(out.Line, out.Agents) + out.Agents = pruneLyricAgents(out.Line, out.Agents) + return out } -func normalizeCueLines(lines []Line) []Line { +func cloneLyrics(lyrics Lyrics) Lyrics { + out := lyrics + out.Offset = gg.Clone(lyrics.Offset) + out.Agents = slices.Clone(lyrics.Agents) + out.Line = make([]Line, len(lyrics.Line)) + for i := range lyrics.Line { + out.Line[i] = cloneLyricLine(lyrics.Line[i]) + } + return out +} + +func cloneLyricLine(line Line) Line { + out := line + out.Start = gg.Clone(line.Start) + out.End = gg.Clone(line.End) + out.Cue = make([]Cue, len(line.Cue)) + for i := range line.Cue { + out.Cue[i] = line.Cue[i] + out.Cue[i].Start = gg.Clone(line.Cue[i].Start) + out.Cue[i].End = gg.Clone(line.Cue[i].End) + } + if len(out.Cue) == 0 { + out.Cue = nil + } + return out +} + +func normalizeCueLines(lines []Line, agents []Agent) []Line { if len(lines) == 0 { return lines } - normalized := make([]Line, len(lines)) - copy(normalized, lines) - - for i := range normalized { - if len(normalized[i].Cue) > 0 { - normalized[i].Cue = slices.Clone(normalized[i].Cue) + knownAgents := make(map[string]struct{}, len(agents)) + for _, agent := range agents { + id := strings.TrimSpace(agent.ID) + if id != "" { + knownAgents[id] = struct{}{} } - - var fallbackEnd *int64 - if normalized[i].End != nil { - v := *normalized[i].End - fallbackEnd = &v - } else if i+1 < len(normalized) && normalized[i+1].Start != nil { - v := *normalized[i+1].Start - fallbackEnd = &v - } - - normalized[i] = normalizeCueLine(normalized[i], fallbackEnd) } - return normalized -} + for i := range lines { + line := lines[i] + if line.Start != nil && *line.Start < 0 { + line.Start = nil + line.End = nil + } + if line.Start != nil && line.End != nil && *line.End < *line.Start { + line.End = nil + } -func normalizeLineTiming(line Line) Line { - if len(line.Cue) == 0 { - return line - } + if len(line.Cue) == 0 { + if line.Start == nil { + line.End = nil + } + lines[i] = line + continue + } - var earliestStart *int64 - var latestEnd *int64 - for i := range line.Cue { - token := line.Cue[i] - if token.Start != nil { - if earliestStart == nil || *token.Start < *earliestStart { - v := *token.Start - earliestStart = &v + cues, valid := validateCueGeometry(line.Value, line.Cue, knownAgents) + if !valid { + line.Cue = nil + lines[i] = line + continue + } + + var nextTimedStart *int64 + for j := i + 1; j < len(lines); j++ { + if lines[j].Start != nil { + nextTimedStart = gg.Clone(lines[j].Start) + break } } - candidateEnd := token.End - if candidateEnd == nil { - candidateEnd = token.Start + line.Cue = normalizeCueEndsByAgent(cues, line.End, nextTimedStart) + line = widenLineTiming(line, cues, nextTimedStart) + lines[i] = line + } + + return lines +} + +func validateCueGeometry(lineValue string, cues []Cue, knownAgents map[string]struct{}) ([]Cue, bool) { + if !utf8.ValidString(lineValue) { + return nil, false + } + + out := make([]Cue, len(cues)) + lastByteEndByAgent := make(map[string]int) + lastStartByAgent := make(map[string]int64) + seenAgent := make(map[string]bool) + for i, cue := range cues { + out[i] = cue + out[i].Start = gg.Clone(cue.Start) + out[i].End = gg.Clone(cue.End) + + if cue.Start == nil || *cue.Start < 0 || !utf8.ValidString(cue.Value) { + return nil, false } - if candidateEnd != nil { - if latestEnd == nil || *candidateEnd > *latestEnd { - v := *candidateEnd - latestEnd = &v + if cue.ByteStart < 0 || cue.ByteEnd < cue.ByteStart || cue.ByteEnd >= len(lineValue) { + return nil, false + } + agentID := strings.TrimSpace(cue.AgentID) + out[i].AgentID = agentID + if agentID != "" { + if _, ok := knownAgents[agentID]; !ok { + return nil, false } } - } + if lastByteEnd, seen := lastByteEndByAgent[agentID]; seen && cue.ByteStart <= lastByteEnd { + return nil, false + } + if !isRuneBoundary(lineValue, cue.ByteStart) || !isRuneBoundary(lineValue, cue.ByteEnd+1) { + return nil, false + } + if lineValue[cue.ByteStart:cue.ByteEnd+1] != cue.Value { + return nil, false + } + if seenAgent[agentID] && *cue.Start < lastStartByAgent[agentID] { + return nil, false + } + seenAgent[agentID] = true + lastStartByAgent[agentID] = *cue.Start + lastByteEndByAgent[agentID] = cue.ByteEnd - if line.Start == nil && earliestStart != nil { - v := *earliestStart - line.Start = &v + if cue.End != nil && *cue.End < *cue.Start { + out[i].End = nil + } } - if line.End == nil && latestEnd != nil { - v := *latestEnd - line.End = &v - } - return line + return out, true } -func normalizeCueLine(line Line, fallbackEnd *int64) Line { - if len(line.Cue) == 0 { - return line - } - line.Cue = normalizeCueEndsByAgent(line.Cue, fallbackEnd) - return normalizeLineTiming(line) +func isRuneBoundary(value string, offset int) bool { + return offset == 0 || offset == len(value) || utf8.RuneStart(value[offset]) } -// normalizeCueEndsByAgent resolves cue end times independently per agent so that -// background (or other parallel) layers, whose cues interleave with the main -// timeline but are stored together in document order, do not clamp each other's -// ends. Each agent group is normalized in its own document order; results are -// reassembled into the original cue positions. -func normalizeCueEndsByAgent(cues []Cue, fallbackEnd *int64) []Cue { +func normalizeCueEndsByAgent(cues []Cue, lineEnd, nextTimedStart *int64) []Cue { groups := make(map[string][]int) order := make([]string, 0, 2) for i := range cues { @@ -105,36 +164,150 @@ func normalizeCueEndsByAgent(cues []Cue, fallbackEnd *int64) []Cue { groups[id] = append(groups[id], i) } - // Single agent: the document order already matches the timeline, so the - // straightforward normalization applies without regrouping. - if len(order) <= 1 { - return NormalizeCueEnds(cues, fallbackEnd) - } - out := slices.Clone(cues) for _, id := range order { idxs := groups[id] group := make([]Cue, len(idxs)) - for gi, pos := range idxs { - group[gi] = cues[pos] + for i, pos := range idxs { + group[i] = cues[pos] } - group = NormalizeCueEnds(group, fallbackEnd) - for gi, pos := range idxs { - out[pos] = group[gi] + group = normalizePartialCueEnds(group, lineEnd, nextTimedStart) + for i, pos := range idxs { + out[pos] = group[i] } } return out } -// NormalizeCueEnds resolves missing cue end times within a single ordered cue -// group: each end is filled from the next cue's start, then from fallbackEnd, -// and is clamped so it never precedes the cue's own start nor overruns the next -// cue. End times are all-or-none — if any cue still lacks an end afterwards, all -// ends in the group are cleared. The input slice is never mutated. -// -// Exported because the Subsonic enhanced-lyrics serializer resolves cue ends -// per agent group while building the response; all other normalization is -// package-internal. +func normalizePartialCueEnds(cues []Cue, lineEnd, nextTimedStart *int64) []Cue { + out := slices.Clone(cues) + hasEnd := false + for _, cue := range out { + if cue.End != nil { + hasEnd = true + break + } + } + if !hasEnd { + return out + } + + complete := true + for i := range out { + end := out[i].End + if end == nil && i+1 < len(out) { + end = out[i+1].Start + } + if end == nil { + end = lineEnd + } + if end == nil { + end = nextTimedStart + } + if end != nil && i+1 < len(out) && *end > *out[i+1].Start { + end = out[i+1].Start + } + if end == nil || *end < *out[i].Start { + complete = false + break + } + out[i].End = gg.Clone(end) + } + + if !complete { + for i := range out { + out[i].End = nil + } + } + return out +} + +func widenLineTiming(line Line, sourceCues []Cue, nextTimedStart *int64) Line { + var earliestStart *int64 + for _, cue := range line.Cue { + if cue.Start == nil { + continue + } + if earliestStart == nil || *cue.Start < *earliestStart { + earliestStart = cue.Start + } + } + if earliestStart != nil && (line.Start == nil || *earliestStart < *line.Start) { + line.Start = gg.Clone(earliestStart) + } + + // Only an explicit terminal cue end is exact enough to become Line.End. + groups := make(map[string][]Cue) + for _, cue := range sourceCues { + groups[cue.AgentID] = append(groups[cue.AgentID], cue) + } + var latestExactEnd *int64 + for _, group := range groups { + terminal := group[len(group)-1] + if terminal.Start == nil { + continue + } + isNextLineFallback := line.End == nil && nextTimedStart != nil && terminal.End != nil && *terminal.End == *nextTimedStart + if terminal.End != nil && !isNextLineFallback && *terminal.End >= *terminal.Start && (latestExactEnd == nil || *terminal.End > *latestExactEnd) { + latestExactEnd = terminal.End + } + } + if latestExactEnd != nil && (line.End == nil || *latestExactEnd > *line.End) { + line.End = gg.Clone(latestExactEnd) + } + if line.Start != nil && line.End != nil && *line.End < *line.Start { + line.End = nil + } + if line.Start == nil { + line.End = nil + } + return line +} + +// normalizeLineTiming is used while TTML is still assembling agent metadata. +// Full cue validation and agent pruning happen when the complete Lyrics value +// is passed through NormalizeLyrics. +func normalizeLineTiming(line Line) Line { + out := cloneLyricLine(line) + if out.Start != nil && out.End != nil && *out.End < *out.Start { + out.End = nil + } + return widenLineTiming(out, out.Cue, nil) +} + +func pruneLyricAgents(lines []Line, agents []Agent) []Agent { + used := make(map[string]struct{}) + for _, line := range lines { + for _, cue := range line.Cue { + if cue.AgentID != "" { + used[cue.AgentID] = struct{}{} + } + } + } + if len(used) == 0 { + return nil + } + + out := make([]Agent, 0, len(used)) + seen := make(map[string]struct{}, len(used)) + for _, agent := range agents { + id := strings.TrimSpace(agent.ID) + if _, ok := used[id]; !ok { + continue + } + if _, duplicate := seen[id]; duplicate { + continue + } + agent.ID = id + out = append(out, agent) + seen[id] = struct{}{} + } + return out +} + +// NormalizeCueEnds retains the v2 response helper contract: resolve missing +// ends from the next cue or the supplied exact/fallback end without mutating +// the caller. Model parsing should use NormalizeLyrics instead. func NormalizeCueEnds(cues []Cue, fallbackEnd *int64) []Cue { if len(cues) == 0 { return cues @@ -143,12 +316,11 @@ func NormalizeCueEnds(cues []Cue, fallbackEnd *int64) []Cue { out := slices.Clone(cues) for i := range out { end := out[i].End + if end == nil && i+1 < len(out) && out[i+1].Start != nil { + end = out[i+1].Start + } if end == nil { - if i+1 < len(out) && out[i+1].Start != nil { - end = out[i+1].Start - } else { - end = fallbackEnd - } + end = fallbackEnd } if end != nil && i+1 < len(out) && out[i+1].Start != nil && *end > *out[i+1].Start { end = out[i+1].Start @@ -160,12 +332,13 @@ func NormalizeCueEnds(cues []Cue, fallbackEnd *int64) []Cue { } for i := range out { - if out[i].End == nil { - for j := range out { - out[j].End = nil - } - break + if out[i].End != nil { + continue } + for j := range out { + out[j].End = nil + } + break } return out } diff --git a/model/lyrics_normalize_test.go b/model/lyrics_normalize_test.go index 24faffd54..f60ac0529 100644 --- a/model/lyrics_normalize_test.go +++ b/model/lyrics_normalize_test.go @@ -5,115 +5,231 @@ import ( . "github.com/onsi/gomega" ) -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}, +var _ = Describe("NormalizeLyrics", func() { + p := func(v int64) *int64 { return &v } + + It("is immutable and idempotent", func() { + input := Lyrics{ + Offset: p(25), + Line: []Line{ + { + Start: p(1000), Value: "Some lyrics", + Cue: []Cue{ + {Start: p(1000), End: p(1200), Value: "Some ", ByteStart: 0, ByteEnd: 4}, + {Start: p(1500), Value: "lyrics", ByteStart: 5, ByteEnd: 10}, + }, }, - }, - { - Start: &nextLineStart, - Value: "Next line", + {Start: p(3000), Value: "Next line"}, }, } + before := cloneLyrics(input) - normalized := normalizeCueLines(lines) + first := NormalizeLyrics(input) + second := NormalizeLyrics(first) - 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()) + Expect(first).To(Equal(second)) + Expect(input).To(Equal(before)) + Expect(first.Line[0].Cue[0].End).To(Equal(p(1200))) + Expect(first.Line[0].Cue[1].End).To(Equal(p(3000))) + Expect(first.Line[0].End).To(BeNil(), "next-line fallback must not become an exact line end") + }) + + It("preserves fully start-only cue groups", func() { + lyrics := Lyrics{Line: []Line{{ + Value: "hello world", + Cue: []Cue{ + {Start: p(1000), Value: "hello", ByteStart: 0, ByteEnd: 4}, + {Start: p(1500), Value: "world", ByteStart: 6, ByteEnd: 10}, + }, + }}} + + out := NormalizeLyrics(lyrics) + + Expect(out.Line[0].Start).To(Equal(p(1000))) + Expect(out.Line[0].End).To(BeNil()) + Expect(out.Line[0].Cue[0].End).To(BeNil()) + Expect(out.Line[0].Cue[1].End).To(BeNil()) + }) + + It("scans past untimed lines for a partial cue group's fallback", func() { + lyrics := Lyrics{Line: []Line{ + { + Start: p(1000), Value: "a b", + Cue: []Cue{ + {Start: p(1000), End: p(1200), Value: "a", ByteStart: 0, ByteEnd: 0}, + {Start: p(1500), Value: "b", ByteStart: 2, ByteEnd: 2}, + }, + }, + {Value: "untimed"}, + {Start: p(4000), Value: "timed"}, + }} + + out := NormalizeLyrics(lyrics) + + Expect(out.Line[0].Cue[1].End).To(Equal(p(4000))) + Expect(out.Line[0].End).To(BeNil()) + }) + + It("clears a partial group when it cannot complete consistently", func() { + lyrics := Lyrics{Line: []Line{{ + Value: "a b", + Cue: []Cue{ + {Start: p(1000), End: p(1200), Value: "a", ByteStart: 0, ByteEnd: 0}, + {Start: p(1500), Value: "b", ByteStart: 2, ByteEnd: 2}, + }, + }}} + + out := NormalizeLyrics(lyrics) + + Expect(out.Line[0].Cue[0].End).To(BeNil()) + Expect(out.Line[0].Cue[1].End).To(BeNil()) + }) + + It("clamps overlaps only inside the same agent", func() { + lyrics := Lyrics{ + Agents: []Agent{{ID: "lead"}, {ID: "backing"}}, + Line: []Line{{ + Value: "lead back next", + Cue: []Cue{ + {Start: p(1000), End: p(2500), Value: "lead", ByteStart: 0, ByteEnd: 3, AgentID: "lead"}, + {Start: p(1200), End: p(2200), Value: "back", ByteStart: 5, ByteEnd: 8, AgentID: "backing"}, + {Start: p(2000), End: p(3000), Value: "next", ByteStart: 10, ByteEnd: 13, AgentID: "lead"}, + }, + }}, + } + + out := NormalizeLyrics(lyrics) + + Expect(out.Line[0].Cue[0].End).To(Equal(p(2000))) + Expect(out.Line[0].Cue[1].End).To(Equal(p(2200)), "cross-agent overlap must remain") + Expect(out.Line[0].Cue[2].End).To(Equal(p(3000))) + }) + + It("allows different agents to reference the same byte range", func() { + lyrics := Lyrics{ + Agents: []Agent{{ID: "lead"}, {ID: "backing"}}, + Line: []Line{{ + Value: "hello", + Cue: []Cue{ + {Start: p(1000), End: p(2000), Value: "hello", ByteStart: 0, ByteEnd: 4, AgentID: "lead"}, + {Start: p(1200), End: p(2200), Value: "hello", ByteStart: 0, ByteEnd: 4, AgentID: "backing"}, + }, + }}, + } + + out := NormalizeLyrics(lyrics) + + Expect(out.Line[0].Cue).To(HaveLen(2)) + Expect(out.Line[0].Cue[0].AgentID).To(Equal("lead")) + Expect(out.Line[0].Cue[1].AgentID).To(Equal("backing")) + }) + + It("ignores missing cue starts while widening partially assembled lines", func() { + line := Line{ + Start: p(1000), + Value: "partial", + Cue: []Cue{{End: p(2000), Value: "partial", ByteStart: 0, ByteEnd: 6}}, + } + + var out Line + Expect(func() { out = normalizeLineTiming(line) }).ToNot(Panic()) + Expect(out.Start).To(Equal(p(1000))) + Expect(out.End).To(BeNil()) + }) + + It("preserves zero-duration markers and repairs reverse ends", func() { + lyrics := Lyrics{Line: []Line{ + {Start: p(1000), End: p(500), Value: "marker", Cue: []Cue{{Start: p(1000), End: p(1000), Value: "marker", ByteStart: 0, ByteEnd: 5}}}, + {Start: p(1500), End: p(1400), Value: "reverse"}, + }} + + out := NormalizeLyrics(lyrics) + + Expect(out.Line[0].Cue[0].End).To(Equal(p(1000))) + Expect(out.Line[0].End).To(Equal(p(1000))) + Expect(out.Line[1].End).To(BeNil()) + }) + + It("uses an explicit terminal cue end as an exact line end", func() { + lyrics := Lyrics{Line: []Line{{ + Start: p(2000), Value: "hello", + Cue: []Cue{{Start: p(1000), End: p(2500), Value: "hello", ByteStart: 0, ByteEnd: 4}}, + }}} + + out := NormalizeLyrics(lyrics) + + Expect(out.Line[0].Start).To(Equal(p(1000))) + Expect(out.Line[0].End).To(Equal(p(2500))) + }) + + DescribeTable("drops irreparable cue geometry but retains line data", + func(value string, cues []Cue, agents []Agent) { + lineStart, lineEnd := int64(1000), int64(2000) + out := NormalizeLyrics(Lyrics{ + Agents: agents, + Line: []Line{{Start: &lineStart, End: &lineEnd, Value: value, Cue: cues}}, + }) + + Expect(out.Line).To(HaveLen(1)) + Expect(out.Line[0].Value).To(Equal(value)) + Expect(out.Line[0].Start).To(Equal(&lineStart)) + Expect(out.Line[0].End).To(Equal(&lineEnd)) + Expect(out.Line[0].Cue).To(BeNil()) + Expect(out.Agents).To(BeNil()) + }, + Entry("invalid UTF-8", string([]byte{0xff}), []Cue{{Start: p(1000), Value: string([]byte{0xff}), ByteStart: 0, ByteEnd: 0}}, nil), + Entry("range outside the line", "hi", []Cue{{Start: p(1000), Value: "hi", ByteStart: 0, ByteEnd: 2}}, nil), + Entry("range splitting a CJK rune", "한", []Cue{{Start: p(1000), Value: string([]byte("한")[:2]), ByteStart: 0, ByteEnd: 1}}, nil), + Entry("cue value mismatch", "hello", []Cue{{Start: p(1000), Value: "world", ByteStart: 0, ByteEnd: 4}}, nil), + Entry("overlapping byte ranges", "hello", []Cue{ + {Start: p(1000), Value: "hel", ByteStart: 0, ByteEnd: 2}, + {Start: p(1100), Value: "llo", ByteStart: 2, ByteEnd: 4}, + }, nil), + Entry("out-of-order same-agent cues", "a b", []Cue{ + {Start: p(1500), Value: "a", ByteStart: 0, ByteEnd: 0}, + {Start: p(1000), Value: "b", ByteStart: 2, ByteEnd: 2}, + }, nil), + Entry("unknown agent attribution", "hi", []Cue{{Start: p(1000), Value: "hi", ByteStart: 0, ByteEnd: 1, AgentID: "missing"}}, []Agent{{ID: "known"}}), + ) + + It("validates CJK, emoji, and combining-mark byte boundaries", func() { + value := "한🙂é" + lyrics := Lyrics{Line: []Line{{ + Value: value, + Cue: []Cue{ + {Start: p(1000), Value: "한", ByteStart: 0, ByteEnd: 2}, + {Start: p(1100), Value: "🙂", ByteStart: 3, ByteEnd: 6}, + {Start: p(1200), Value: "é", ByteStart: 7, ByteEnd: 9}, + }, + }}} + + out := NormalizeLyrics(lyrics) + + Expect(out.Line[0].Cue).To(HaveLen(3)) + Expect([]string{out.Line[0].Cue[0].Value, out.Line[0].Cue[1].Value, out.Line[0].Cue[2].Value}).To(Equal([]string{"한", "🙂", "é"})) + }) + + It("prunes unused and duplicate agents", func() { + lyrics := Lyrics{ + Agents: []Agent{{ID: "lead"}, {ID: "unused"}, {ID: "lead"}}, + Line: []Line{{Value: "hi", Cue: []Cue{{Start: p(1000), Value: "hi", ByteStart: 0, ByteEnd: 1, AgentID: "lead"}}}}, + } + + out := NormalizeLyrics(lyrics) + + Expect(out.Agents).To(Equal([]Agent{{ID: "lead"}})) }) }) var _ = Describe("NormalizeCueEnds", func() { - // p returns a fresh pointer so cases don't share *int64 state. p := func(v int64) *int64 { return &v } - // endsOf extracts the resolved end times (nil-safe) for compact assertions. - endsOf := func(cues []Cue) []*int64 { - out := make([]*int64, len(cues)) - for i := range cues { - out[i] = cues[i].End - } - return out - } - - It("returns the input as-is when empty", func() { - Expect(NormalizeCueEnds(nil, p(1000))).To(BeNil()) - Expect(NormalizeCueEnds([]Cue{}, p(1000))).To(BeEmpty()) - }) - - It("fills a missing end from the next cue's start", func() { - cues := []Cue{ - {Start: p(1000)}, - {Start: p(1500)}, - } - + It("retains its response compatibility contract without mutating input", func() { + cues := []Cue{{Start: p(1000)}, {Start: p(1500)}} out := NormalizeCueEnds(cues, p(3000)) - Expect(endsOf(out)).To(Equal([]*int64{p(1500), p(3000)})) - }) - - It("fills the last cue's missing end from fallbackEnd", func() { - cues := []Cue{ - {Start: p(1000), End: p(1200)}, - {Start: p(1500)}, - } - - out := NormalizeCueEnds(cues, p(3000)) - - Expect(endsOf(out)).To(Equal([]*int64{p(1200), p(3000)})) - }) - - It("clamps an end that overruns the next cue's start", func() { - cues := []Cue{ - {Start: p(1000), End: p(9999)}, - {Start: p(1500), End: p(2000)}, - } - - out := NormalizeCueEnds(cues, p(3000)) - - Expect(endsOf(out)).To(Equal([]*int64{p(1500), p(2000)})) - }) - - It("clamps an end that precedes the cue's own start", func() { - cues := []Cue{ - {Start: p(1000), End: p(500)}, - } - - out := NormalizeCueEnds(cues, p(3000)) - - Expect(endsOf(out)).To(Equal([]*int64{p(1000)})) - }) - - It("clears all ends when any cue still lacks one (all-or-none)", func() { - // The last cue has no end and there is no fallback, so it stays nil and - // every end in the group is cleared. - cues := []Cue{ - {Start: p(1000), End: p(1200)}, - {Start: p(1500)}, - } - - out := NormalizeCueEnds(cues, nil) - - Expect(endsOf(out)).To(Equal([]*int64{nil, nil})) - }) - - It("does not mutate the input slice", func() { - cues := []Cue{ - {Start: p(1000)}, - {Start: p(1500)}, - } - - _ = NormalizeCueEnds(cues, p(3000)) - + Expect(out[0].End).To(Equal(p(1500))) + Expect(out[1].End).To(Equal(p(3000))) Expect(cues[0].End).To(BeNil()) Expect(cues[1].End).To(BeNil()) }) diff --git a/model/lyrics_parse.go b/model/lyrics_parse.go index 8aa095c50..04afd985b 100644 --- a/model/lyrics_parse.go +++ b/model/lyrics_parse.go @@ -3,32 +3,46 @@ package model import ( "bytes" "context" + "errors" "fmt" + "regexp" "slices" "strings" "github.com/navidrome/navidrome/log" ) -// lyricParser returns an empty list (not an error) when the input is not its -// format, so parsers can be tried in order. lang is the default for formats that -// do not carry their own. +// lyricParser parses content already claimed by its format. A nil list with no +// error means a recognized, valid document that contains no lyrics. type lyricParser func(lang string, contents []byte) (LyricList, error) +var errLyricsFormatMismatch = errors.New("lyrics format mismatch") + +type lyricFormat struct { + name string + suffixes []string + claims func([]byte) bool + parse lyricParser +} + // lyricFormats is the structured formats in content-sniff probe order; each // row's suffixes drive sidecar dispatch. LRC/plain is the unlisted fallback floor. -var lyricFormats = []struct { - suffixes []string - parse lyricParser -}{ - {[]string{".ttml"}, parseTTML}, - {[]string{".srt"}, parseSRT}, - {[]string{".yaml", ".yml"}, parseLyricsfile}, +var lyricFormats = []lyricFormat{ + {name: "TTML", suffixes: []string{".ttml"}, claims: claimsTTML, parse: parseTTML}, + {name: "SRT", suffixes: []string{".srt"}, claims: claimsSRT, parse: parseSRT}, + {name: "Lyricsfile", suffixes: []string{".yaml", ".yml"}, claims: claimsLyricsfile, parse: parseLyricsfile}, } +var ( + ttmlRootPrefixRegex = regexp.MustCompile(`(?is)^\s*(?:<\?xml\b[^>]*\?>\s*)?(?:\s*)*<(?:[[:alpha:]_][[:alnum:]_.-]*:)?tt(?:\s|/?>)`) + srtClaimRegex = regexp.MustCompile(`(?m)^\s*(?:\d+\s*\n\s*)?\d{1,2}:\d{2}:\d{2}[,.]\d{1,3}\s*-->\s*\d{1,2}:\d{2}:\d{2}[,.]\d{1,3}(?:\s|$)`) + lyricsfileRegex = regexp.MustCompile(`(?mi)^\s*["']?version["']?\s*:\s*["']?1\.0["']?\s*(?:#.*)?$`) +) + // ParseLyrics is the single entry point for parsing lyrics. A known suffix routes -// to that format's parser; an empty or "auto" suffix content-sniffs. Either way, -// a structured parser that does not match falls back to the LRC/plain-text floor. +// to that format's parser; an empty or "auto" suffix content-sniffs. Explicit +// structured suffixes are strict: malformed or mismatched structured content is +// returned as an error so a source resolver can continue to its next source. // // Parse failures are logged through ctx; callers that know the source should // attach it for attribution, e.g. log.NewContext(ctx, "file", path). @@ -37,37 +51,80 @@ func ParseLyrics(ctx context.Context, suffix, lang string, contents []byte) (Lyr suffix = strings.ToLower(suffix) sniff := suffix == "" || suffix == "auto" - // Sniffing tries every format in order; a known suffix selects just its own. - // Unmatched suffixes leave no candidates, so parseFirstMatch falls to plain. - candidates := make([]lyricParser, 0, len(lyricFormats)) + // Sniffing tries every structured format in order. A known structured suffix + // selects one strict parser; LRC/text and unknown textual suffixes retain the + // longstanding LRC/plain fallback. + candidates := make([]lyricFormat, 0, len(lyricFormats)) for _, f := range lyricFormats { if sniff || slices.Contains(f.suffixes, suffix) { - candidates = append(candidates, f.parse) + candidates = append(candidates, f) } } - return parseFirstMatch(ctx, sniff, lang, contents, candidates...) -} -func parseFirstMatch(ctx context.Context, sniff bool, lang string, contents []byte, candidates ...lyricParser) (LyricList, error) { - for _, parse := range candidates { - list, err := parse(lang, contents) - if err == nil && len(list) > 0 { - return list, nil + if !sniff && len(candidates) > 0 { + format := candidates[0] + list, err := parseClaimedLyrics(format, lang, contents) + if errors.Is(err, errLyricsFormatMismatch) { + err = fmt.Errorf("declared %s lyrics do not match the format: %w", format.name, err) } if err != nil { - // While sniffing, a probe rejecting content it does not own is expected - // control flow, so keep it at trace. A failure under an explicit suffix - // means the declared format is malformed and deserves a warning. - if sniff { - log.Trace(ctx, "Lyrics probe did not match, trying next format", err) - } else { - log.Warn(ctx, "Error parsing lyrics, falling back to plain text", err) - } + log.Warn(ctx, "Error parsing declared lyrics", "format", format.name, err) } + return list, err + } + + return parseFirstMatch(ctx, lang, contents, candidates...) +} + +func parseFirstMatch(ctx context.Context, lang string, contents []byte, candidates ...lyricFormat) (LyricList, error) { + for _, format := range candidates { + list, err := parseClaimedLyrics(format, lang, contents) + if errors.Is(err, errLyricsFormatMismatch) { + log.Trace(ctx, "Lyrics probe did not match, trying next format", "format", format.name) + continue + } + if err != nil { + return nil, fmt.Errorf("parsing claimed %s lyrics: %w", format.name, err) + } + // A claimed, valid-empty document owns the content and deliberately stops + // sniffing instead of being reinterpreted as another format or plain text. + return list, nil } return plainLRC(lang, contents) } +func parseClaimedLyrics(format lyricFormat, lang string, contents []byte) (LyricList, error) { + if !format.claims(contents) { + return nil, errLyricsFormatMismatch + } + list, err := format.parse(lang, contents) + if err != nil { + return nil, err + } + return list, nil +} + +func claimsTTML(contents []byte) bool { + trimmed := bytes.TrimSpace(contents) + if len(trimmed) == 0 || trimmed[0] != '<' { + return false + } + return isTTMLDocument(contents) || ttmlRootPrefixRegex.Match(contents) +} + +func claimsSRT(contents []byte) bool { + raw := bytes.ReplaceAll(contents, []byte("\r\n"), []byte("\n")) + raw = bytes.ReplaceAll(raw, []byte("\r"), []byte("\n")) + return srtClaimRegex.Match(raw) +} + +func claimsLyricsfile(contents []byte) bool { + if lyricsfileRegex.Match(contents) { + return true + } + return hasLyricsfileVersion(contents) +} + func plainLRC(lang string, contents []byte) (LyricList, error) { lyric, err := parseLRC(lang, string(contents)) if err != nil { diff --git a/model/lyrics_parse_test.go b/model/lyrics_parse_test.go index 0b47e55e9..dc2b1b439 100644 --- a/model/lyrics_parse_test.go +++ b/model/lyrics_parse_test.go @@ -1,8 +1,6 @@ package model import ( - "strings" - "github.com/navidrome/navidrome/log" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -34,6 +32,19 @@ var _ = Describe("ParseLyrics", func() { Expect(list[0].Line[0].Value).To(Equal("auto ttml")) }) + DescribeTable("accepts valid TTML XML prologs", + func(suffix, prolog string) { + contents := prolog + `

prolog ttml

` + list, err := ParseLyrics(GinkgoT().Context(), suffix, "eng", []byte(contents)) + + Expect(err).ToNot(HaveOccurred()) + Expect(list).To(HaveLen(1)) + Expect(list[0].Line[0].Value).To(Equal("prolog ttml")) + }, + Entry("explicit suffix with processing instruction", ".ttml", ``), + Entry("content sniffing with doctype", "", ``), + ) + It("empty suffix content-sniffs (YAML)", func() { yaml := "version: \"1.0\"\nmetadata:\n language: eng\nlines:\n - text: auto yaml\n start_ms: 1000\n" list, err := ParseLyrics(GinkgoT().Context(), "auto", "eng", []byte(yaml)) @@ -42,14 +53,62 @@ var _ = Describe("ParseLyrics", func() { Expect(list[0].Line[0].Value).To(Equal("auto yaml")) }) - It("falls back to plain text when a known suffix fails to parse structurally", func() { + DescribeTable("accepts quoted Lyricsfile version keys", + func(suffix string) { + contents := "\"version\": \"1.0\"\nmetadata:\n language: eng\nlines:\n - text: quoted version\n start_ms: 1000\n" + list, err := ParseLyrics(GinkgoT().Context(), suffix, "eng", []byte(contents)) + + Expect(err).ToNot(HaveOccurred()) + Expect(list).To(HaveLen(1)) + Expect(list[0].Line[0].Value).To(Equal("quoted version")) + }, + Entry("explicit YAML", ".yaml"), + Entry("content sniffing", "auto"), + ) + + It("returns an error when an explicit structured suffix does not match", func() { list, err := ParseLyrics(GinkgoT().Context(), ".srt", "eng", []byte("not actually an srt file")) + Expect(err).To(MatchError(ContainSubstring("declared SRT lyrics do not match"))) + Expect(list).To(BeNil()) + }) + + It("retains LRC/plain fallback for textual suffixes", func() { + list, err := ParseLyrics(GinkgoT().Context(), ".lrc", "eng", []byte("not actually timed")) Expect(err).ToNot(HaveOccurred()) Expect(list).To(HaveLen(1)) Expect(list[0].Synced).To(BeFalse()) - Expect(list[0].Line[0].Value).To(Equal("not actually an srt file")) + Expect(list[0].Line[0].Value).To(Equal("not actually timed")) }) + DescribeTable("recognized valid-empty structured documents stop sniffing", + func(contents string) { + list, err := ParseLyrics(GinkgoT().Context(), "", "eng", []byte(contents)) + Expect(err).ToNot(HaveOccurred()) + Expect(list).To(BeEmpty()) + }, + Entry("TTML", ``), + Entry("SRT", "1\n00:00:01,000 --> 00:00:02,000\n"), + Entry("Lyricsfile", "version: \"1.0\"\nmetadata:\n language: eng\n"), + ) + + It("does not reinterpret malformed claimed structured content as plain text", func() { + list, err := ParseLyrics(GinkgoT().Context(), "", "eng", []byte(`

Broken`)) + Expect(err).To(MatchError(ContainSubstring("parsing claimed TTML lyrics"))) + Expect(list).To(BeNil()) + }) + + DescribeTable("generic content remains eligible for plain fallback", + func(contents string) { + list, err := ParseLyrics(GinkgoT().Context(), "", "eng", []byte(contents)) + Expect(err).ToNot(HaveOccurred()) + Expect(list).To(HaveLen(1)) + Expect(list[0].Synced).To(BeFalse()) + }, + Entry("prose", "A plain lyric"), + Entry("generic XML", "A generic lyric"), + Entry("generic YAML", "title: A generic lyric"), + ) + Describe("logging on parser probe failures", func() { var hook *test.Hook @@ -92,10 +151,10 @@ var _ = Describe("ParseLyrics", func() { // failure is worth surfacing loudly — and it must name the file. It("warns and names the file when a requested suffix fails to parse", func() { ctx := log.NewContext(GinkgoT().Context(), "file", "/music/song.yaml") - list, err := ParseLyrics(ctx, ".yaml", "eng", []byte("not: [valid, yaml\n")) + list, err := ParseLyrics(ctx, ".yaml", "eng", []byte("version: \"1.0\"\nnot: [valid, yaml\n")) - Expect(err).ToNot(HaveOccurred()) - Expect(list).To(HaveLen(1)) // still falls back to plain text + Expect(err).To(HaveOccurred()) + Expect(list).To(BeNil()) entry := hook.LastEntry() Expect(entry).ToNot(BeNil()) Expect(entry.Level).To(Equal(logrus.WarnLevel)) @@ -234,25 +293,13 @@ Another subtitle line` Expect(list[0].Line[0].Cue).To(HaveLen(2)) }) - It("should fall back to plain lyrics when embedded TTML is invalid", func() { - content := ` - -

Broken

- -` + It("should reject malformed embedded TTML instead of returning raw markup", func() { + content := `

Broken` list, err := ParseLyrics(GinkgoT().Context(), "", "eng", []byte(content)) - Expect(err).ToNot(HaveOccurred()) - Expect(list).To(HaveLen(1)) - Expect(list[0].Lang).To(Equal("eng")) - Expect(list[0].Synced).To(BeFalse()) - Expect(list[0].Line).ToNot(BeEmpty()) - values := make([]string, 0, len(list[0].Line)) - for _, line := range list[0].Line { - values = append(values, line.Value) - } - Expect(strings.Join(values, "\n")).To(ContainSubstring("Broken")) + Expect(err).To(HaveOccurred()) + Expect(list).To(BeNil()) }) It("detects a Lyricsfile YAML payload via content-sniffing", func() { diff --git a/model/lyrics_srt.go b/model/lyrics_srt.go index 319a59961..ca25be3f0 100644 --- a/model/lyrics_srt.go +++ b/model/lyrics_srt.go @@ -1,6 +1,7 @@ package model import ( + "fmt" "regexp" "strconv" "strings" @@ -34,7 +35,7 @@ func parseSRT(language string, contents []byte) (LyricList, error) { return nil, nil } - lyrics := normalizeLyrics(Lyrics{ + lyrics := NormalizeLyrics(Lyrics{ Lang: normalizeLyricLang(language), Line: lines, Synced: true, @@ -91,6 +92,9 @@ func parseSRTBlock(block string) (Line, bool, error) { if err != nil { return Line{}, false, err } + if endMs < startMs { + return Line{}, false, fmt.Errorf("SRT cue end %d precedes start %d", endMs, startMs) + } textLines := make([]string, 0, len(lines)-startIdx-1) for _, line := range lines[startIdx+1:] { @@ -130,6 +134,9 @@ func parseSRTTime(value string) (int64, error) { if err != nil { return 0, err } + if minutes > 59 || seconds > 59 { + return 0, fmt.Errorf("SRT timestamp fields out of range: %q", value) + } millis, err := strconv.ParseInt(match[4], 10, 64) if err != nil { return 0, err diff --git a/model/lyrics_srt_test.go b/model/lyrics_srt_test.go index 2c0ab2242..691f88526 100644 --- a/model/lyrics_srt_test.go +++ b/model/lyrics_srt_test.go @@ -27,4 +27,27 @@ var _ = Describe("parseSRT", func() { Expect(err).ToNot(HaveOccurred()) Expect(list).To(BeNil()) }) + + It("preserves zero durations, overlaps, and document order", func() { + content := []byte("1\n00:00:05,000 --> 00:00:05,000\nMarker\n\n2\n00:00:03,000 --> 00:00:06,000\nEarlier overlapping line") + + list, err := parseSRT("eng", content) + + Expect(err).ToNot(HaveOccurred()) + Expect(list[0].Line).To(Equal([]Line{ + {Start: new(int64(5000)), End: new(int64(5000)), Value: "Marker"}, + {Start: new(int64(3000)), End: new(int64(6000)), Value: "Earlier overlapping line"}, + })) + }) + + DescribeTable("rejects invalid SRT timing", + func(timing string) { + list, err := parseSRT("eng", []byte("1\n"+timing+"\nInvalid")) + Expect(err).To(HaveOccurred()) + Expect(list).To(BeNil()) + }, + Entry("negative duration", "00:00:02,000 --> 00:00:01,999"), + Entry("minute out of range", "00:60:00,000 --> 01:00:01,000"), + Entry("second out of range", "00:00:60,000 --> 00:01:01,000"), + ) }) diff --git a/model/lyrics_ttml.go b/model/lyrics_ttml.go index 43d4699af..b1d790def 100644 --- a/model/lyrics_ttml.go +++ b/model/lyrics_ttml.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/xml" "errors" + "fmt" "io" "math" "regexp" @@ -77,9 +78,10 @@ type ttmlDefinedAgent struct { } type ttmlPiece struct { - raw string - cue *Cue - isBreak bool + raw string + cue *Cue + isBreak bool + invalidTiming bool } type ttmlParser struct { @@ -123,6 +125,9 @@ func parseTTML(defaultLang string, contents []byte) (LyricList, error) { // text — isTTMLDocument does a cheap decode that stops at the first element. // Checked after the encoding fixup so UTF-16-declared documents are recognized. if !isTTMLDocument(contents) { + if claimsTTML(contents) { + return nil, fmt.Errorf("malformed TTML document") + } return nil, nil } @@ -185,20 +190,20 @@ func (p *ttmlParser) parseElement(start xml.StartElement, parent ttmlTimingConte if err != nil { return err } - if ctx.invalid || lineText == "" { + if lineText == "" { return nil } parsedLine := Line{Value: lineText} - if ctx.hasBegin { + if !ctx.invalid && ctx.hasBegin { startMs := ctx.begin parsedLine.Start = &startMs } - if ctx.hasEnd { + if !ctx.invalid && ctx.hasEnd { endMs := ctx.end parsedLine.End = &endMs } - if len(tokens) > 0 { + if !ctx.invalid && len(tokens) > 0 { parsedLine.Cue = tokens } parsedLine = normalizeLineTiming(parsedLine) @@ -327,21 +332,17 @@ func (p *ttmlParser) parseMetadataText(start xml.StartElement, parent ttmlTiming } ctx := p.childContext(start.Attr, parent) - if ctx.invalid { - return ttmlMetadataEntry{}, false, nil - } - value, tokens := buildTTMLLineFromPieces(pieces) line := Line{Value: value} - if ctx.hasBegin { + if !ctx.invalid && ctx.hasBegin { startMs := ctx.begin line.Start = &startMs } - if ctx.hasEnd { + if !ctx.invalid && ctx.hasEnd { endMs := ctx.end line.End = &endMs } - if len(tokens) > 0 { + if !ctx.invalid && len(tokens) > 0 { line.Cue = tokens } line = normalizeLineTiming(line) @@ -412,7 +413,17 @@ func (p *ttmlParser) parseInlineElement(start xml.StartElement, parent ttmlTimin continue } - if local == "span" && hasOwnTiming && !ctx.invalid && !ttmlPiecesContainCue(pieces) { + if local == "span" && hasOwnTiming && ctx.invalid { + if len(pieces) == 0 { + pieces = append(pieces, ttmlPiece{}) + } + for i := range pieces { + pieces[i].invalidTiming = true + } + return pieces, nil + } + + if local == "span" && hasOwnTiming && !ttmlPiecesContainCue(pieces) { rawValue := concatTTMLPieceRaw(pieces) tokenText := sanitizeTTMLText(rawValue) if tokenText != "" { @@ -443,6 +454,10 @@ func (p *ttmlParser) parseInlineElement(start xml.StartElement, parent ttmlTimin } func buildTTMLLineFromPieces(pieces []ttmlPiece) (string, []Cue) { + invalidTiming := false + for _, piece := range pieces { + invalidTiming = invalidTiming || piece.invalidTiming + } finalized := finalizeTTMLLines(splitTTMLPiecesByBreak(pieces)) for len(finalized) > 0 && finalized[0].text == "" && len(finalized[0].cues) == 0 { finalized = finalized[1:] @@ -472,6 +487,9 @@ func buildTTMLLineFromPieces(pieces []ttmlPiece) (string, []Cue) { byteOffset += len(line.text) } + if invalidTiming { + cues = nil + } return value.String(), cues } @@ -509,8 +527,9 @@ func splitTTMLPiecesByBreak(pieces []ttmlPiece) [][]ttmlPiece { continue } lines[len(lines)-1] = append(lines[len(lines)-1], ttmlPiece{ - raw: raw, - cue: gg.Clone(piece.cue), + raw: raw, + cue: gg.Clone(piece.cue), + invalidTiming: piece.invalidTiming, }) prevEndedWithSpace = strings.HasSuffix(raw, " ") } @@ -697,7 +716,7 @@ func (p *ttmlParser) buildMetadataLyrics(kind string, langOrder []string, entrie func (p *ttmlParser) finalizeLyrics(lyrics Lyrics) Lyrics { lyrics.Line, lyrics.Agents = p.resolveAgents(lyrics.Line) - return normalizeLyrics(lyrics) + return NormalizeLyrics(lyrics) } func (p *ttmlParser) resolveAgents(lines []Line) ([]Line, []Agent) { @@ -975,6 +994,9 @@ func (p *ttmlParser) childContext(attrs []xml.Attr, parent ttmlTimingContext) tt ctx.end = calculatedEnd ctx.hasEnd = calculatedHasEnd + if ctx.hasBegin && ctx.hasEnd && ctx.end < ctx.begin { + ctx.invalid = true + } return ctx } diff --git a/model/lyrics_ttml_test.go b/model/lyrics_ttml_test.go index bbdf5c7c4..bac9f040b 100644 --- a/model/lyrics_ttml_test.go +++ b/model/lyrics_ttml_test.go @@ -43,7 +43,7 @@ var _ = Describe("parseTTML", func() { }) Describe("Unsupported cue handling", func() { - It("should skip wallclock cues and keep valid ones", func() { + It("should retain text with invalid wallclock timing and keep valid cues", func() { content := []byte(` @@ -57,9 +57,11 @@ var _ = Describe("parseTTML", func() { list, err := parseTTML("xxx", content) Expect(err).ToNot(HaveOccurred()) Expect(list).To(HaveLen(1)) - Expect(list[0].Line).To(HaveLen(1)) - Expect(list[0].Line[0].Start).To(Equal(new(int64(1000)))) - Expect(list[0].Line[0].Value).To(Equal("Keep me")) + Expect(list[0].Line).To(HaveLen(2)) + Expect(list[0].Line[0].Start).To(BeNil()) + Expect(list[0].Line[0].Value).To(Equal("Skip me")) + Expect(list[0].Line[1].Start).To(Equal(new(int64(1000)))) + Expect(list[0].Line[1].Value).To(Equal("Keep me")) }) }) @@ -548,4 +550,45 @@ var _ = Describe("parseTTML", func() { Expect(line.Cue[2]).To(Equal(Cue{Start: new(int64(3179)), End: new(int64(3582)), Value: "up", ByteStart: 7, ByteEnd: 8})) }) }) + + It("retains line text but drops enhanced cues for invalid paragraph timing", func() { + content := []byte(`

+

Still visible

+
`) + + list, err := parseTTML("eng", content) + + Expect(err).ToNot(HaveOccurred()) + Expect(list).To(HaveLen(1)) + Expect(list[0].Line).To(Equal([]Line{{Value: "Still visible"}})) + }) + + It("drops only the affected line's cues for invalid span timing", func() { + content := []byte(`
+

Good text

+

Timed

+
`) + + list, err := parseTTML("eng", content) + + Expect(err).ToNot(HaveOccurred()) + Expect(list).To(HaveLen(1)) + Expect(list[0].Line).To(HaveLen(2)) + Expect(list[0].Line[0].Value).To(Equal("Good text")) + Expect(list[0].Line[0].Cue).To(BeNil()) + Expect(list[0].Line[1].Value).To(Equal("Timed")) + Expect(list[0].Line[1].Cue).To(HaveLen(1)) + }) + + It("retains text when an end-only span has no usable cue start", func() { + content := []byte(`
+

Still visible

+
`) + + list, err := parseTTML("eng", content) + + Expect(err).ToNot(HaveOccurred()) + Expect(list).To(HaveLen(1)) + Expect(list[0].Line).To(Equal([]Line{{Value: "Still visible"}})) + }) }) diff --git a/plugins/lyrics_adapter_test.go b/plugins/lyrics_adapter_test.go index d110665f5..3269f58c9 100644 --- a/plugins/lyrics_adapter_test.go +++ b/plugins/lyrics_adapter_test.go @@ -150,6 +150,22 @@ var _ = Describe("LyricsPlugin", Ordered, func() { Entry("lrc", "lrc", true, "plugin lrc line"), Entry("plain", "plain", false, "plugin plain line"), ) + + It("keeps valid entries when another plugin lyric is malformed", func() { + manager, _ := createTestManagerWithPlugins(map[string]map[string]string{ + "test-lyrics": {"format": "mixed"}, + }, "test-lyrics"+PackageExtension) + + p, ok := manager.LoadLyricsProvider("test-lyrics") + Expect(ok).To(BeTrue()) + + result, err := p.GetLyrics(GinkgoT().Context(), &model.MediaFile{ID: "track-1"}) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].Line).To(Equal([]model.Line{ + {Start: new(int64(1000)), Value: "valid plugin line"}, + })) + }) }) Describe("PluginNames", func() { diff --git a/plugins/testdata/test-lyrics/main.go b/plugins/testdata/test-lyrics/main.go index 2ee2dabbf..062aab4bc 100644 --- a/plugins/testdata/test-lyrics/main.go +++ b/plugins/testdata/test-lyrics/main.go @@ -48,6 +48,13 @@ func (t *testLyrics) GetLyrics(input lyrics.GetLyricsRequest) (lyrics.GetLyricsR case "plain": lang = "eng" text = "plugin plain line" + case "mixed": + return lyrics.GetLyricsResponse{ + Lyrics: []lyrics.LyricsText{ + {Lang: "eng", Text: `

malformed`}, + {Lang: "eng", Text: "[00:01.00]valid plugin line"}, + }, + }, nil } if text != "" { return lyrics.GetLyricsResponse{