mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-31 07:30:32 +00:00
fix(lyrics): correct TTML background-vocal cue timing and whitespace (#5672)
* fix(lyrics): correct TTML background-vocal cue timing and whitespace Two parsing defects surfaced by Apple Music TTML files that mix a main vocal with an x-bg (background) span group within the same line: - Cue end-time normalization ran over the whole line's cue list in document order. Background cues are stored after the main cues but interleave earlier on the timeline, so the next-cue clamp collapsed the last main cue's end down to its own start (start == end). End times are now normalized per agent group, matching how the Subsonic serializer already groups cues, so parallel layers no longer corrupt each other. - Whitespace between elements was treated as significant: pretty-printed (indented) TTML injected spurious newlines into the line text, turning one line into many. Per TTML2 default xml:space handling (linefeeds treat-as-space, whitespace-collapse), formatting whitespace now collapses to a single space and hard line breaks come only from <br/>. The line-level value and per-agent cueLine.value remain the full line text, as required by the OpenSubsonic songLyrics v2 contract; the per-agent text is carried in each cueLine's cue[] array. Two existing tests that encoded the buggy newline-as-break behavior are corrected; new tests cover whitespace collapse, <br/> preservation, and interleaved background cue timing. * fix(lyrics): only collapse XML whitespace, preserve other Unicode spaces Whitespace collapsing used unicode.IsSpace, which matches more than the XML S production (space, tab, CR, LF): it also folds characters like NBSP and U+3000 into a regular space, silently altering content. Restrict collapsing to the four XML whitespace characters so other Unicode spaces pass through unchanged, and add a regression test. Also clarify the doc comment that collapsing is applied unconditionally (xml:space="preserve" is not supported).
This commit is contained in:
Deluan Quintão
GitHub
parent
bd9fa1c602
commit
13e96a0e81
@ -85,10 +85,47 @@ func normalizeCueLine(line Line, fallbackEnd *int64) Line {
|
||||
if len(line.Cue) == 0 {
|
||||
return line
|
||||
}
|
||||
line.Cue = NormalizeCueEnds(line.Cue, fallbackEnd)
|
||||
line.Cue = normalizeCueEndsByAgent(line.Cue, fallbackEnd)
|
||||
return normalizeLineTiming(line)
|
||||
}
|
||||
|
||||
// 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 {
|
||||
groups := make(map[string][]int)
|
||||
order := make([]string, 0, 2)
|
||||
for i := range cues {
|
||||
id := cues[i].AgentID
|
||||
if _, ok := groups[id]; !ok {
|
||||
order = append(order, id)
|
||||
}
|
||||
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]
|
||||
}
|
||||
group = NormalizeCueEnds(group, fallbackEnd)
|
||||
for gi, pos := range idxs {
|
||||
out[pos] = group[gi]
|
||||
}
|
||||
}
|
||||
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
|
||||
|
||||
@ -77,8 +77,9 @@ type ttmlDefinedAgent struct {
|
||||
}
|
||||
|
||||
type ttmlPiece struct {
|
||||
raw string
|
||||
cue *Cue
|
||||
raw string
|
||||
cue *Cue
|
||||
isBreak bool
|
||||
}
|
||||
|
||||
type ttmlParser struct {
|
||||
@ -382,7 +383,7 @@ func (p *ttmlParser) parseParagraph(parent ttmlTimingContext) (string, []Cue, er
|
||||
func (p *ttmlParser) parseInlineElement(start xml.StartElement, parent ttmlTimingContext) ([]ttmlPiece, error) {
|
||||
local := strings.ToLower(start.Name.Local)
|
||||
if local == "br" {
|
||||
return []ttmlPiece{{raw: "\n"}}, nil
|
||||
return []ttmlPiece{{isBreak: true}}, nil
|
||||
}
|
||||
|
||||
ctx := p.childContext(start.Attr, parent)
|
||||
@ -442,7 +443,7 @@ func (p *ttmlParser) parseInlineElement(start xml.StartElement, parent ttmlTimin
|
||||
}
|
||||
|
||||
func buildTTMLLineFromPieces(pieces []ttmlPiece) (string, []Cue) {
|
||||
finalized := finalizeTTMLLines(splitTTMLPiecesByNewline(pieces))
|
||||
finalized := finalizeTTMLLines(splitTTMLPiecesByBreak(pieces))
|
||||
for len(finalized) > 0 && finalized[0].text == "" && len(finalized[0].cues) == 0 {
|
||||
finalized = finalized[1:]
|
||||
}
|
||||
@ -488,34 +489,30 @@ func finalizeTTMLLines(lines [][]ttmlPiece) []ttmlFinalLine {
|
||||
return finalized
|
||||
}
|
||||
|
||||
func splitTTMLPiecesByNewline(pieces []ttmlPiece) [][]ttmlPiece {
|
||||
func splitTTMLPiecesByBreak(pieces []ttmlPiece) [][]ttmlPiece {
|
||||
lines := [][]ttmlPiece{{}}
|
||||
prevEndedWithSpace := true // leading whitespace on a fresh line is dropped
|
||||
for _, piece := range pieces {
|
||||
raw := normalizeTTMLPieceRaw(piece.raw)
|
||||
if raw == "" {
|
||||
if piece.isBreak {
|
||||
lines = append(lines, []ttmlPiece{})
|
||||
prevEndedWithSpace = true
|
||||
continue
|
||||
}
|
||||
|
||||
start := 0
|
||||
for i := 0; i < len(raw); i++ {
|
||||
if raw[i] != '\n' {
|
||||
continue
|
||||
}
|
||||
if start < i {
|
||||
lines[len(lines)-1] = append(lines[len(lines)-1], ttmlPiece{
|
||||
raw: raw[start:i],
|
||||
cue: gg.Clone(piece.cue),
|
||||
})
|
||||
}
|
||||
lines = append(lines, []ttmlPiece{})
|
||||
start = i + 1
|
||||
raw := normalizeTTMLPieceRaw(piece.raw)
|
||||
// Collapse whitespace across piece boundaries: a piece's leading space is
|
||||
// redundant when the text emitted so far already ends with one.
|
||||
if prevEndedWithSpace {
|
||||
raw = strings.TrimPrefix(raw, " ")
|
||||
}
|
||||
if start < len(raw) {
|
||||
lines[len(lines)-1] = append(lines[len(lines)-1], ttmlPiece{
|
||||
raw: raw[start:],
|
||||
cue: gg.Clone(piece.cue),
|
||||
})
|
||||
if raw == "" {
|
||||
continue
|
||||
}
|
||||
lines[len(lines)-1] = append(lines[len(lines)-1], ttmlPiece{
|
||||
raw: raw,
|
||||
cue: gg.Clone(piece.cue),
|
||||
})
|
||||
prevEndedWithSpace = strings.HasSuffix(raw, " ")
|
||||
}
|
||||
return lines
|
||||
}
|
||||
@ -555,11 +552,38 @@ func finalizeTTMLLogicalLine(line []ttmlPiece) (string, []Cue) {
|
||||
return trimmed, cues
|
||||
}
|
||||
|
||||
// normalizeTTMLPieceRaw collapses whitespace following TTML's default mode
|
||||
// (xml:space="default", the root default per TTML2 §8.1.1): per §8.2.10 that
|
||||
// means linefeed-treatment="treat-as-space" and white-space-collapse="true", so
|
||||
// linefeeds and other whitespace runs collapse to a single space. Collapsing is
|
||||
// applied unconditionally; xml:space="preserve" is not supported (no lyric
|
||||
// source in practice relies on it). Hard line breaks come only from <br/>
|
||||
// (§8.1.7), tracked separately via ttmlPiece.isBreak, so pretty-printed
|
||||
// indentation between elements does not inject spurious newlines.
|
||||
func normalizeTTMLPieceRaw(raw string) string {
|
||||
raw = str.SanitizeText(raw)
|
||||
raw = strings.ReplaceAll(raw, "\r\n", "\n")
|
||||
raw = strings.ReplaceAll(raw, "\r", "\n")
|
||||
return raw
|
||||
return collapseTTMLWhitespace(raw)
|
||||
}
|
||||
|
||||
func collapseTTMLWhitespace(raw string) string {
|
||||
var b strings.Builder
|
||||
b.Grow(len(raw))
|
||||
prevSpace := false
|
||||
for _, r := range raw {
|
||||
// Only the XML S production (space, tab, CR, LF) is collapsible whitespace.
|
||||
// Other Unicode spaces (e.g. NBSP, U+3000) are content characters, not
|
||||
// whitespace, so they pass through unchanged.
|
||||
if r == ' ' || r == '\t' || r == '\n' || r == '\r' {
|
||||
if !prevSpace {
|
||||
b.WriteByte(' ')
|
||||
prevSpace = true
|
||||
}
|
||||
continue
|
||||
}
|
||||
b.WriteRune(r)
|
||||
prevSpace = false
|
||||
}
|
||||
return b.String()
|
||||
}
|
||||
|
||||
func concatTTMLPieceRaw(pieces []ttmlPiece) string {
|
||||
|
||||
@ -135,7 +135,7 @@ var _ = Describe("parseTTML", func() {
|
||||
|
||||
line := list[0].Line[0]
|
||||
Expect(line.Start).To(Equal(new(int64(1000))))
|
||||
Expect(line.Value).To(Equal("Hello\necho"))
|
||||
Expect(line.Value).To(Equal("Hello echo"))
|
||||
Expect(line.End).To(Equal(new(int64(3000))))
|
||||
Expect(line.Cue).To(HaveLen(3))
|
||||
|
||||
@ -269,6 +269,128 @@ var _ = Describe("parseTTML", func() {
|
||||
})
|
||||
})
|
||||
|
||||
Describe("Whitespace handling", func() {
|
||||
It("should collapse pretty-print indentation between spans into single spaces, not line breaks", 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="1:22.889" end="1:26.859" ttm:agent="v2">
|
||||
<span begin="1:22.889" end="1:23.127">It</span>
|
||||
<span begin="1:23.374" end="1:23.938">in,</span>
|
||||
<span ttm:role="x-bg">
|
||||
<span begin="1:23.881" end="1:24.243">(When you</span>
|
||||
<span begin="1:26.232" end="1:26.859">slide)</span>
|
||||
</span>
|
||||
</p>
|
||||
</div>
|
||||
</body>
|
||||
</tt>`)
|
||||
|
||||
list, err := parseTTML("xxx", content)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(list).To(HaveLen(1))
|
||||
Expect(list[0].Line).To(HaveLen(1))
|
||||
|
||||
line := list[0].Line[0]
|
||||
Expect(line.Value).To(Equal("It in, (When you slide)"))
|
||||
Expect(line.Value).ToNot(ContainSubstring("\n"))
|
||||
Expect(line.Cue).To(HaveLen(4))
|
||||
Expect(line.Cue[0]).To(Equal(Cue{Start: new(int64(82889)), End: new(int64(83127)), Value: "It", ByteStart: 0, ByteEnd: 1, AgentID: "v2"}))
|
||||
Expect(line.Cue[1]).To(Equal(Cue{Start: new(int64(83374)), End: new(int64(83938)), Value: "in,", ByteStart: 3, ByteEnd: 5, AgentID: "v2"}))
|
||||
Expect(line.Cue[2]).To(Equal(Cue{Start: new(int64(83881)), End: new(int64(84243)), Value: "(When you", ByteStart: 7, ByteEnd: 15, AgentID: "__nd_bg__|v2"}))
|
||||
Expect(line.Cue[3]).To(Equal(Cue{Start: new(int64(86232)), End: new(int64(86859)), Value: "slide)", ByteStart: 17, ByteEnd: 22, AgentID: "__nd_bg__|v2"}))
|
||||
})
|
||||
|
||||
It("should preserve explicit <br/> as a line break", 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">
|
||||
<span begin="00:01.000" end="00:01.400">first</span>
|
||||
<br/>
|
||||
<span begin="00:02.000" end="00:02.500">second</span>
|
||||
</p>
|
||||
</div>
|
||||
</body>
|
||||
</tt>`)
|
||||
|
||||
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].Value).To(Equal("first\nsecond"))
|
||||
})
|
||||
|
||||
It("should only collapse XML whitespace, leaving other Unicode spaces intact", func() {
|
||||
// Whitespace collapsing only touches the XML S characters
|
||||
// (space/tab/CR/LF). Other Unicode spaces like U+3000 are left as-is:
|
||||
// the U+3000 inside a span survives, while the pretty-print newline
|
||||
// between spans still collapses to a single space.
|
||||
content := []byte("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" +
|
||||
"<tt xmlns=\"http://www.w3.org/ns/ttml\">\n" +
|
||||
" <body xml:lang=\"jpn\">\n" +
|
||||
" <div>\n" +
|
||||
" <p begin=\"00:01.000\" end=\"00:03.000\">\n" +
|
||||
" <span begin=\"00:01.000\" end=\"00:01.400\">あ い</span>\n" +
|
||||
" <span begin=\"00:02.000\" end=\"00:02.500\">う</span>\n" +
|
||||
" </p>\n" +
|
||||
" </div>\n" +
|
||||
" </body>\n" +
|
||||
"</tt>")
|
||||
|
||||
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].Value).To(Equal("あ い う"))
|
||||
Expect(list[0].Line[0].Cue[0].Value).To(Equal("あ い"))
|
||||
})
|
||||
})
|
||||
|
||||
Describe("Interleaved background cue timing", func() {
|
||||
It("should not corrupt a main cue's end time when a background cue is earlier in time", func() {
|
||||
// Background spans (x-bg) appear after the main spans in document order
|
||||
// but their timings interleave with the main timeline. End-time
|
||||
// normalization must be per agent so the last main cue keeps its real
|
||||
// end instead of collapsing to its own start.
|
||||
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="1:22.889" end="1:26.859" ttm:agent="v2"><span begin="1:25.593" end="1:25.934">real</span> <span begin="1:25.934" end="1:26.751">slow</span> <span ttm:role="x-bg"><span begin="1:23.881" end="1:24.243">(When you</span> <span begin="1:26.232" end="1:26.859">slide)</span></span></p>
|
||||
</div>
|
||||
</body>
|
||||
</tt>`)
|
||||
|
||||
list, err := parseTTML("xxx", content)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(list).To(HaveLen(1))
|
||||
Expect(list[0].Line).To(HaveLen(1))
|
||||
|
||||
line := list[0].Line[0]
|
||||
Expect(line.Cue).To(HaveLen(4))
|
||||
|
||||
cuesByAgent := map[string][]Cue{}
|
||||
for _, c := range line.Cue {
|
||||
cuesByAgent[c.AgentID] = append(cuesByAgent[c.AgentID], c)
|
||||
}
|
||||
|
||||
mainCues := cuesByAgent["v2"]
|
||||
Expect(mainCues).To(HaveLen(2))
|
||||
Expect(*mainCues[0].End).To(Equal(int64(85934))) // "real"
|
||||
// "slow" must keep its real end (86751), not collapse to its start.
|
||||
Expect(*mainCues[1].Start).To(Equal(int64(85934)))
|
||||
Expect(*mainCues[1].End).To(Equal(int64(86751)))
|
||||
|
||||
bgCues := cuesByAgent["__nd_bg__|v2"]
|
||||
Expect(bgCues).To(HaveLen(2))
|
||||
Expect(*bgCues[0].End).To(Equal(int64(84243))) // "(When you"
|
||||
Expect(*bgCues[1].End).To(Equal(int64(86859))) // "slide)"
|
||||
})
|
||||
})
|
||||
|
||||
Describe("Ambiguous decimal timing", func() {
|
||||
It("should prefer absolute timing when values fall inside parent window", func() {
|
||||
content := []byte(`<?xml version="1.0" encoding="UTF-8"?>
|
||||
@ -290,7 +412,7 @@ var _ = Describe("parseTTML", func() {
|
||||
|
||||
line := list[0].Line[0]
|
||||
Expect(line.Start).To(Equal(new(int64(43444))))
|
||||
Expect(line.Value).To(Equal("go\ngo"))
|
||||
Expect(line.Value).To(Equal("go go"))
|
||||
Expect(line.End).To(Equal(new(int64(45570))))
|
||||
Expect(line.Cue).To(HaveLen(2))
|
||||
Expect(line.Cue[0]).To(Equal(Cue{Start: new(int64(43444)), End: new(int64(43716)), Value: "go", ByteStart: 0, ByteEnd: 1}))
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user