From d7cc4e639c9ac15e8cdc11bf3b9ac6995906b912 Mon Sep 17 00:00:00 2001 From: Joren Date: Sat, 3 Oct 2026 22:05:20 +0200 Subject: [PATCH] fix: keep verse breaks in qobuz plain lyrics, handle timestamped blanks The no-space form after the timestamp is correct and unchanged: LRC is [mm:ss.xx]text, and the Deezer output already matches. Two real issues behind that question, though. Qobuz separates verses with an entry whose line is empty and which has no start or end at all. Those were dropped entirely, so the plain lyrics tag came out as one undifferentiated block. They are now kept as single blank lines, collapsed so a run of separators cannot produce several blanks, and trimmed at both ends so a tag never begins or ends with one. Checked against the live track: 41 plain lines with 8 stanza breaks, where before it was 33 flat lines. They are still excluded from the LRC, which is the correct behaviour rather than an omission: a synced entry needs a timestamp, and these have none, so emitting one would park a blank lyric at the wrong moment. Separately, an empty line that *does* carry a timestamp is a different thing: a display-clear marker. Qobuz does not appear to emit those, but other lyrics sources do, and the parser now handles them properly by writing a bare [mm:ss.xx] entry into the LRC and keeping it out of the plain text. The LRC itself is unchanged for real data: 33 entries, every one timestamped. Co-Authored-By: Claude Opus 5 --- internal/provider/qobuz/client.go | 48 +++++++++++++++------- internal/provider/qobuz/client_test.go | 55 +++++++++++++++++++++++++- 2 files changed, 87 insertions(+), 16 deletions(-) diff --git a/internal/provider/qobuz/client.go b/internal/provider/qobuz/client.go index 5e656b8..8407016 100644 --- a/internal/provider/qobuz/client.go +++ b/internal/provider/qobuz/client.go @@ -1627,44 +1627,62 @@ func (c *Client) fetchLyricsDocument(ctx context.Context, trackID, docURL string // parseQobuzLyrics turns a lyrics object into plain text and, when the lines // carry timings, an LRC transcript. +// +// Qobuz separates verses with entries that have an empty line and no start or +// end at all. Those are kept as blank lines in the plain text, so stanzas stay +// readable, but omitted from the LRC: a synced entry needs a timestamp, and +// inventing one would park a blank line at the wrong moment. func parseQobuzLyrics(obj map[string]any) (string, string) { lines, ok := obj["lines"].([]any) if !ok || len(lines) == 0 { return "", "" } // wsync documents are line objects that additionally carry per-word - // timings; the line text and start are in the same place, so both synced + // timings; the line text and start sit in the same place, so both synced // variants are handled identically. - synced := strings.EqualFold(jsonutil.StringFromAny(obj["type"]), "lsync") || - strings.EqualFold(jsonutil.StringFromAny(obj["type"]), "wsync") + kind := strings.ToLower(strings.TrimSpace(jsonutil.StringFromAny(obj["type"]))) + synced := kind == "lsync" || kind == "wsync" plainParts := make([]string, 0, len(lines)) lrcParts := make([]string, 0, len(lines)) - timed := false for _, raw := range lines { m, ok := raw.(map[string]any) if !ok { continue } text := strings.TrimSpace(jsonutil.StringFromAny(m["line"])) + start, hasStart := m["start"] + timed := synced && hasStart && start != nil + if text == "" { + if timed { + // A blank line that does carry a timestamp is a display-clear + // marker: it belongs in the LRC, where it blanks the lyric at + // that moment, but not in the plain text. + ms := jsonutil.IntFromAny(start) + lrcParts = append(lrcParts, fmt.Sprintf("[%02d:%05.2f]", ms/60000, float64(ms%60000)/1000.0)) + continue + } + // Untimed blank: a verse separator. Keep a single blank line, and + // never a leading one. + if len(plainParts) > 0 && plainParts[len(plainParts)-1] != "" { + plainParts = append(plainParts, "") + } continue } plainParts = append(plainParts, text) - if synced { - if start, has := m["start"]; has { - ms := jsonutil.IntFromAny(start) - lrcParts = append(lrcParts, fmt.Sprintf("[%02d:%05.2f]%s", ms/60000, float64(ms%60000)/1000.0, text)) - timed = true - } + + if timed { + ms := jsonutil.IntFromAny(start) + lrcParts = append(lrcParts, fmt.Sprintf("[%02d:%05.2f]%s", ms/60000, float64(ms%60000)/1000.0, text)) } } + // Drop a trailing separator. + for len(plainParts) > 0 && plainParts[len(plainParts)-1] == "" { + plainParts = plainParts[:len(plainParts)-1] + } if len(plainParts) == 0 { return "", "" } - lrc := "" - if timed { - lrc = strings.Join(lrcParts, "\n") - } - return strings.Join(plainParts, "\n"), lrc + return strings.Join(plainParts, "\n"), strings.Join(lrcParts, "\n") } diff --git a/internal/provider/qobuz/client_test.go b/internal/provider/qobuz/client_test.go index 52037ad..7bc42be 100644 --- a/internal/provider/qobuz/client_test.go +++ b/internal/provider/qobuz/client_test.go @@ -481,10 +481,12 @@ func TestTrackLyricsLineSynced(t *testing.T) { if err != nil { t.Fatalf("TrackLyrics() error = %v", err) } + // The timestamped blank is a display-clear marker: it belongs in the LRC + // but not in the readable plain text. if plain != "First line\nSecond line" { t.Errorf("plain = %q", plain) } - want := "[00:05.78]First line\n[01:05.25]Second line" + want := "[00:05.78]First line\n[00:09.00]\n[01:05.25]Second line" if lrc != want { t.Errorf("lrc = %q, want %q", lrc, want) } @@ -578,3 +580,54 @@ func TestTrackLyricsEmptyIDIsNoOp(t *testing.T) { t.Fatalf("got plain=%q lrc=%q err=%v", plain, lrc, err) } } + +// TestTrackLyricsKeepsVerseBreaks covers Qobuz's real separator shape: an +// empty line with no start or end. Those keep the plain text in stanzas but +// cannot become LRC entries, since there is no timestamp to use. +func TestTrackLyricsKeepsVerseBreaks(t *testing.T) { + srv := newLyricsServer(t, map[string]any{ + "original": map[string]any{ + "type": "lsync", + "lines": []any{ + map[string]any{"line": "verse one line one", "start": 1000}, + map[string]any{"line": "verse one line two", "start": 2000}, + map[string]any{"line": ""}, + map[string]any{"line": ""}, + map[string]any{"line": "verse two line one", "start": 3000}, + map[string]any{"line": ""}, + }, + }, + }, http.StatusOK) + + plain, lrc, err := lyricsClient(t, srv).TrackLyrics(context.Background(), "42") + if err != nil { + t.Fatal(err) + } + wantPlain := "verse one line one\nverse one line two\n\nverse two line one" + if plain != wantPlain { + t.Errorf("plain = %q, want %q", plain, wantPlain) + } + wantLRC := "[00:01.00]verse one line one\n[00:02.00]verse one line two\n[00:03.00]verse two line one" + if lrc != wantLRC { + t.Errorf("lrc = %q, want %q", lrc, wantLRC) + } +} + +// TestTrackLyricsLeadingSeparatorDropped guards against a lyric tag that +// starts with a blank line. +func TestTrackLyricsLeadingSeparatorDropped(t *testing.T) { + srv := newLyricsServer(t, map[string]any{ + "original": map[string]any{ + "type": "plain", + "lines": []any{map[string]any{"line": ""}, map[string]any{"line": "first real line"}}, + }, + }, http.StatusOK) + + plain, _, err := lyricsClient(t, srv).TrackLyrics(context.Background(), "42") + if err != nil { + t.Fatal(err) + } + if plain != "first real line" { + t.Errorf("plain = %q, want no leading blank", plain) + } +}