Skip to content

Adapt LrcLyricParser to new LrcParser version #14263

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Jun 24, 2025
Merged

Adapt LrcLyricParser to new LrcParser version #14263

merged 4 commits into from
Jun 24, 2025

Conversation

Maxr1998
Copy link
Member

@Maxr1998 Maxr1998 commented Jun 9, 2025 •
edited
Loading

As described in #14254, the positions returned with the ELRC cues are currently incorrect due to whitespace removal. After fixing some parsing issues in the upstream parser library, this PR updates the LrcLyricParser to work with the new library version and removes the white space workaround. As a result, the ELRC cues are finally correct.

Changes
This also fixes positions getting incorrectly parsed in the ELRC cues, and removes the broken white space workaround.

Issues
Fixes #14254.

@Maxr1998
Copy link
Member Author

Maxr1998 commented Jun 9, 2025

As an aside: while working with the new API on a client, I honestly missed an end position/width from the cues. This was originally part of the PR adding ELRC support, but removed due to a potential misunderstanding. Without this field, clients always have to query both the current and the next cue to extract the current slice, which creates unnecessary boilerplate. Thus, I suggest adding back a width or endPosition field to the API. Preferably, this will still happen for 10.11 since the current usability is severely impacted and adding fields shouldn't cause huge issues with the API — the original motivation presented in the ELRC PR still applies.

If desirable, I can handle this change as part of the current PR.

@Maxr1998

This comment was marked as outdated.

Sign in to view
@Maxr1998

This comment was marked as outdated.

Sign in to view
@Maxr1998 Maxr1998 marked this pull request as draft June 9, 2025 23:33
@nielsvanvelzen nielsvanvelzen mentioned this pull request Jun 18, 2025
Closed
1 task
Maxr1998 added 2 commits June 23, 2025 21:52
@Maxr1998
Update dependency LrcParser to 2025.623.0
a9f6d71
@Maxr1998
Adapt LrcLyricParser to new LrcParser version
da188f5 
This also fixes positions getting incorrectly parsed in the ELRC cues,
and removes the broken white space workaround.

Fixes jellyfin#14254
@Maxr1998 Maxr1998 changed the title Fix incorrectly parsed positions in ELRC cues Adapt LrcLyricParser to new LrcParser version Jun 23, 2025
@Maxr1998 Maxr1998 marked this pull request as ready for review June 23, 2025 20:44
@Maxr1998
Copy link
Member Author

PR has been updated to use the new library and adapt the current code to work with it. Might need a bit more testing, but is ready for an initial review.

@Shadowghost Shadowghost requested a review from a team June 23, 2025 21:43
JPVenson
JPVenson requested changes Jun 23, 2025
View reviewed changes
nielsvanvelzen
nielsvanvelzen approved these changes Jun 24, 2025
View reviewed changes
Copy link
Member

@nielsvanvelzen nielsvanvelzen left a comment

Choose a reason for hiding this comment

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

✅ API changes (discussed in chat to include them in 10.11)

@github-actions GitHub Actions
Copy link

Changes in OpenAPI specification found. Expand to see details.

What's Changed

GET /Audio/{itemId}/Lyrics
Return Type:

Changed response : 200 OK

Lyrics returned.

  • Changed content type : application/json

    • Changed property Lyrics (array)

      Gets or sets a collection of individual lyric lines.

      Changed items (object):
      > Lyric model.

      • Changed property Cues (array)

        Gets the time-aligned cues for the song's lyrics.

        Changed items (object):
        > LyricLineCue model, holds information about the timing of words within a LyricLine.

        • Added property EndPosition (integer)

          Gets the end character index of the cue.

        • Changed property Position (integer)

          Gets the start character index of the cue.

  • Changed content type : application/json; profile="CamelCase"

    • Changed property Lyrics (array)

      Gets or sets a collection of individual lyric lines.

      Changed items (object):
      > Lyric model.

      • Changed property Cues (array)

        Gets the time-aligned cues for the song's lyrics.

        Changed items (object):
        > LyricLineCue model, holds information about the timing of words within a LyricLine.

        • Added property EndPosition (integer)

          Gets the end character index of the cue.

        • Changed property Position (integer)

          Gets the start character index of the cue.

  • Changed content type : application/json; profile="PascalCase"

    • Changed property Lyrics (array)

      Gets or sets a collection of individual lyric lines.

      Changed items (object):
      > Lyric model.

      • Changed property Cues (array)

        Gets the time-aligned cues for the song's lyrics.

        Changed items (object):
        > LyricLineCue model, holds information about the timing of words within a LyricLine.

        • Added property EndPosition (integer)

          Gets the end character index of the cue.

        • Changed property Position (integer)

          Gets the start character index of the cue.

POST /Audio/{itemId}/Lyrics
Return Type:

Changed response : 200 OK

Lyrics uploaded.

  • Changed content type : application/json

    • Changed property Lyrics (array)

      Gets or sets a collection of individual lyric lines.

      Changed items (object):
      > Lyric model.

      • Changed property Cues (array)

        Gets the time-aligned cues for the song's lyrics.

        Changed items (object):
        > LyricLineCue model, holds information about the timing of words within a LyricLine.

        • Added property EndPosition (integer)

          Gets the end character index of the cue.

        • Changed property Position (integer)

          Gets the start character index of the cue.

  • Changed content type : application/json; profile="CamelCase"

    • Changed property Lyrics (array)

      Gets or sets a collection of individual lyric lines.

      Changed items (object):
      > Lyric model.

      • Changed property Cues (array)

        Gets the time-aligned cues for the song's lyrics.

        Changed items (object):
        > LyricLineCue model, holds information about the timing of words within a LyricLine.

        • Added property EndPosition (integer)

          Gets the end character index of the cue.

        • Changed property Position (integer)

          Gets the start character index of the cue.

  • Changed content type : application/json; profile="PascalCase"

    • Changed property Lyrics (array)

      Gets or sets a collection of individual lyric lines.

      Changed items (object):
      > Lyric model.

      • Changed property Cues (array)

        Gets the time-aligned cues for the song's lyrics.

        Changed items (object):
        > LyricLineCue model, holds information about the timing of words within a LyricLine.

        • Added property EndPosition (integer)

          Gets the end character index of the cue.

        • Changed property Position (integer)

          Gets the start character index of the cue.

POST /Audio/{itemId}/RemoteSearch/Lyrics/{lyricId}
Return Type:

Changed response : 200 OK

Lyric downloaded.

  • Changed content type : application/json

    • Changed property Lyrics (array)

      Gets or sets a collection of individual lyric lines.

      Changed items (object):
      > Lyric model.

      • Changed property Cues (array)

        Gets the time-aligned cues for the song's lyrics.

        Changed items (object):
        > LyricLineCue model, holds information about the timing of words within a LyricLine.

        • Added property EndPosition (integer)

          Gets the end character index of the cue.

        • Changed property Position (integer)

          Gets the start character index of the cue.

  • Changed content type : application/json; profile="CamelCase"

    • Changed property Lyrics (array)

      Gets or sets a collection of individual lyric lines.

      Changed items (object):
      > Lyric model.

      • Changed property Cues (array)

        Gets the time-aligned cues for the song's lyrics.

        Changed items (object):
        > LyricLineCue model, holds information about the timing of words within a LyricLine.

        • Added property EndPosition (integer)

          Gets the end character index of the cue.

        • Changed property Position (integer)

          Gets the start character index of the cue.

  • Changed content type : application/json; profile="PascalCase"

    • Changed property Lyrics (array)

      Gets or sets a collection of individual lyric lines.

      Changed items (object):
      > Lyric model.

      • Changed property Cues (array)

        Gets the time-aligned cues for the song's lyrics.

        Changed items (object):
        > LyricLineCue model, holds information about the timing of words within a LyricLine.

        • Added property EndPosition (integer)

          Gets the end character index of the cue.

        • Changed property Position (integer)

          Gets the start character index of the cue.

GET /Providers/Lyrics/{lyricId}
Return Type:

Changed response : 200 OK

File returned.

  • Changed content type : application/json

    • Changed property Lyrics (array)

      Gets or sets a collection of individual lyric lines.

      Changed items (object):
      > Lyric model.

      • Changed property Cues (array)

        Gets the time-aligned cues for the song's lyrics.

        Changed items (object):
        > LyricLineCue model, holds information about the timing of words within a LyricLine.

        • Added property EndPosition (integer)

          Gets the end character index of the cue.

        • Changed property Position (integer)

          Gets the start character index of the cue.

  • Changed content type : application/json; profile="CamelCase"

    • Changed property Lyrics (array)

      Gets or sets a collection of individual lyric lines.

      Changed items (object):
      > Lyric model.

      • Changed property Cues (array)

        Gets the time-aligned cues for the song's lyrics.

        Changed items (object):
        > LyricLineCue model, holds information about the timing of words within a LyricLine.

        • Added property EndPosition (integer)

          Gets the end character index of the cue.

        • Changed property Position (integer)

          Gets the start character index of the cue.

  • Changed content type : application/json; profile="PascalCase"

    • Changed property Lyrics (array)

      Gets or sets a collection of individual lyric lines.

      Changed items (object):
      > Lyric model.

      • Changed property Cues (array)

        Gets the time-aligned cues for the song's lyrics.

        Changed items (object):
        > LyricLineCue model, holds information about the timing of words within a LyricLine.

        • Added property EndPosition (integer)

          Gets the end character index of the cue.

        • Changed property Position (integer)

          Gets the start character index of the cue.

GET /Audio/{itemId}/RemoteSearch/Lyrics
Return Type:

Changed response : 200 OK

Lyrics retrieved.

  • Changed content type : application/json

    Changed items (object):
    > The remote lyric info dto.

    • Changed property Lyrics (object)

      Gets the lyrics.

      Updated LyricDto :

      • Changed property Lyrics (array)

        Gets or sets a collection of individual lyric lines.

        Changed items (object):
        > Lyric model.

        • Changed property Cues (array)

          Gets the time-aligned cues for the song's lyrics.

          Changed items (object):
          > LyricLineCue model, holds information about the timing of words within a LyricLine.

          • Added property EndPosition (integer)

            Gets the end character index of the cue.

          • Changed property Position (integer)

            Gets the start character index of the cue.

  • Changed content type : application/json; profile="CamelCase"

    Changed items (object):
    > The remote lyric info dto.

    • Changed property Lyrics (object)

      Gets the lyrics.

      Updated LyricDto :

      • Changed property Lyrics (array)

        Gets or sets a collection of individual lyric lines.

        Changed items (object):
        > Lyric model.

        • Changed property Cues (array)

          Gets the time-aligned cues for the song's lyrics.

          Changed items (object):
          > LyricLineCue model, holds information about the timing of words within a LyricLine.

          • Added property EndPosition (integer)

            Gets the end character index of the cue.

          • Changed property Position (integer)

            Gets the start character index of the cue.

  • Changed content type : application/json; profile="PascalCase"

    Changed items (object):
    > The remote lyric info dto.

    • Changed property Lyrics (object)

      Gets the lyrics.

      Updated LyricDto :

      • Changed property Lyrics (array)

        Gets or sets a collection of individual lyric lines.

        Changed items (object):
        > Lyric model.

        • Changed property Cues (array)

          Gets the time-aligned cues for the song's lyrics.

          Changed items (object):
          > LyricLineCue model, holds information about the timing of words within a LyricLine.

          • Added property EndPosition (integer)

            Gets the end character index of the cue.

          • Changed property Position (integer)

            Gets the start character index of the cue.

@jellyfin-bot
Copy link
Contributor

ABI Difference 

MediaBrowser.Model.dll
API compatibility errors between './abi-base/MediaBrowser.Model.dll' (left) and './abi-head/MediaBrowser.Model.dll' (right):
CP0002: Member 'MediaBrowser.Model.Lyrics.LyricLineCue.LyricLineCue(int, long, System.Nullable<long>)' exists on ./abi-base/MediaBrowser.Model.dll but not on ./abi-head/MediaBrowser.Model.dll
API breaking changes found. If those are intentional, the APICompat suppression file can be updated by specifying the '--generate-suppression-file' parameter.

@crobibero crobibero merged commit 9b8c12d into jellyfin:master Jun 24, 2025
18 checks passed
@Maxr1998 Maxr1998 deleted the fix-elrc-cues branch June 24, 2025 23:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nielsvanvelzen nielsvanvelzen nielsvanvelzen approved these changes

@Shadowghost Shadowghost Shadowghost approved these changes

@JPVenson JPVenson JPVenson approved these changes

@crobibero crobibero crobibero approved these changes

@coderabbitai coderabbitai[bot] coderabbitai[bot] approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

LrcLyricParser returns mismatched cues / whitespace removal breaks cue positions
6 participants
@Maxr1998 @jellyfin-bot @nielsvanvelzen @Shadowghost @JPVenson @crobibero