From 13e96a0e815546d1de986a95783325e0cac3678c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Sat, 27 Jun 2026 10:37:25 -0400 Subject: [PATCH] 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
. 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,
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). --- model/lyrics_normalize.go | 39 +++++++++++- model/lyrics_ttml.go | 80 +++++++++++++++--------- model/lyrics_ttml_test.go | 126 +++++++++++++++++++++++++++++++++++++- 3 files changed, 214 insertions(+), 31 deletions(-) diff --git a/model/lyrics_normalize.go b/model/lyrics_normalize.go index 276aa4da3..a7d4b4e2a 100644 --- a/model/lyrics_normalize.go +++ b/model/lyrics_normalize.go @@ -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 diff --git a/model/lyrics_ttml.go b/model/lyrics_ttml.go index 95aab3485..43d4699af 100644 --- a/model/lyrics_ttml.go +++ b/model/lyrics_ttml.go @@ -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
+// (§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 { diff --git a/model/lyrics_ttml_test.go b/model/lyrics_ttml_test.go index a175eef0e..bbdf5c7c4 100644 --- a/model/lyrics_ttml_test.go +++ b/model/lyrics_ttml_test.go @@ -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(` + + +
+

+ It + in, + + (When you + slide) + +

+
+ +`) + + 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
as a line break", func() { + content := []byte(` + + +
+

+ first +
+ second +

+
+ +`) + + 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("\n" + + "\n" + + " \n" + + "
\n" + + "

\n" + + " あ　い\n" + + " う\n" + + "

\n" + + "
\n" + + " \n" + + "") + + 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(` + + +
+

real slow (When you slide)

+
+ +`) + + 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(` @@ -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}))