Skip to content

fix(lyrics): preserve Telugu shaping in karaoke - #1174

Open
Alvaro-Manzo wants to merge 2 commits into
rukamori:mainfrom
Alvaro-Manzo:fix/enhanced-lyrics-telugu-shaping
Open

fix(lyrics): preserve Telugu shaping in karaoke#1174
Alvaro-Manzo wants to merge 2 commits into
rukamori:mainfrom
Alvaro-Manzo:fix/enhanced-lyrics-telugu-shaping

Conversation

@Alvaro-Manzo

Copy link
Copy Markdown

Fixes #1084.

Timed TTML can split Telugu words into multiple karaoke fragments. Rendering those fragments separately breaks Telugu text shaping and can cause overlapping glyphs.

This change groups fragments belonging to the same Telugu word before creating karaoke syllables, while preserving word-level timing and phonetics.

@Alvaro-Manzo
Alvaro-Manzo marked this pull request as ready for review August 16, 2026 00:04
Copilot AI lite review requested due to automatic review settings August 16, 2026 00:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves Enhanced Lyrics TTML karaoke rendering for Telugu by grouping TTML fragments into larger layout nodes so Telugu grapheme shaping is preserved and glyph overlap is avoided.

Changes:

  • Group WordTimestamp fragments for Telugu into “syllable groups” before building KaraokeSyllables.
  • Merge grouped fragment text into a single KaraokeSyllable.content while keeping timing derived from the group boundaries.
  • Aggregate per-fragment romanization (“phonetics”) into a single phonetic string per grouped syllable.

1. PR Summary

The grouping approach is directionally correct for fixing Telugu shaping issues caused by TTML fragmenting. However, the current phonetic aggregation inserts spaces between fragments, which can break word-level romanization, and the phonetic index computation is O(n²) in the common (non-Telugu) case.

2. Blocking Issues (Changes Requested)

  • Grouped fragment phonetics are joined with spaces, which can introduce incorrect word breaks in romanization and conflicts with the PR’s intent to preserve word-level phonetics.

3. Non-Blocking Feedback (Nitpicks & Polish)

  • phoneticStartIndex is recomputed via prefix summing inside mapIndexed, making toKaraokeSyllables() O(n²) when no grouping occurs (typical for non-Telugu). This is avoidable with a small refactor.

4. Suggested Code Fixes

app/src/main/kotlin/moe/rukamori/archivetune/ui/component/LyricsEnhanced.kt

  • Concatenate grouped fragment phonetics without separators:
.filterNotNull()
.joinToString(separator = "")
.takeIf(String::isNotEmpty)
  • Precompute phonetic offsets once instead of summing inside mapIndexed:
val phoneticOffsets =
    IntArray(syllableGroups.size).also { offsets ->
        var offset = 0
        syllableGroups.forEachIndexed { i, group ->
            offsets[i] = offset
            offset += group.size
        }
    }

// ...
val phoneticStartIndex = phoneticOffsets[index]

5. Final Verdict

REQUEST CHANGES


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +1230 to +1234
.drop(phoneticStartIndex)
.take(group.size)
.filterNotNull()
.joinToString(" ")
.takeIf(String::isNotEmpty),
Comment on lines +1212 to +1218
val syllableGroups = groupTeluguSyllables()

return syllableGroups.mapIndexed { index, group ->
val start = group.first().startTime.toMilliseconds()
val nextStart = syllableGroups.getOrNull(index + 1)?.first()?.startTime?.toMilliseconds()
val rawEnd = group.last().endTime.toMilliseconds()
val phoneticStartIndex = syllableGroups.take(index).sumOf { it.size }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enhanced Lyrics mode Telugu font issue

2 participants