Skip to content

[OS]: Lyrics extension #2656

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 34 commits into from
Dec 28, 2023
Merged

[OS]: Lyrics extension #2656

merged 34 commits into from
Dec 28, 2023

Conversation

kgarner7
Copy link
Contributor

Draft of opensubsonic/open-subsonic-api#62

Lyrics are currently stored in the media file itself (with zero-width space between encodings/lyrics). I'd be open to also using a table instead.

Works best with Taglib (for multi-valued). FFmpeg works well when parsing ID3V2.4 tags, but not other multivalued fields. In some cases, you will have multiple lyrics separated by ;, which is hard to identify, and in other cases only one lyric will be shown (e.g. WMA).

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 26, 2023 •
edited
Loading

Download the artifacts for this pull request:

@kgarner7
Copy link
Contributor Author

kgarner7 commented Dec 3, 2023

.-. Taglib is doing something silly. Probably a version issue? 1.13.1 behaves properly...

@kgarner7 kgarner7 marked this pull request as ready for review December 5, 2023 05:26
@deluan deluan mentioned this pull request Dec 9, 2023
Open
47 tasks
@metalheim metalheim mentioned this pull request Dec 14, 2023
Closed
@deluan deluan mentioned this pull request Dec 14, 2023
Closed
@dweymouth
Copy link
Contributor

@kgarner7 Would it be feasible to implement #1421 as well, in this PR or as a quick follow-up?

@deluan
Copy link
Member

deluan commented Dec 16, 2023

@kgarner7 Would it be feasible to implement #1421 as well, in this PR or as a quick follow-up?

@dweymouth, we discussed this and we will implement it in a following PR, as this one is already big enough.

@dweymouth
Copy link
Contributor

Sounds good, I'm hoping they can land together in a release, since I do have .lrc files in my library but AFAIK no embedded tags. I'll be adding synced lyrics support to Supersonic, but maybe waiting until external .lrc files are supported (especially if no other servers have added OpenSubsonic lyrics support yet). Of course, no rush!

deluan
deluan requested changes Dec 18, 2023
View reviewed changes
Copy link
Member

@deluan deluan left a comment

Choose a reason for hiding this comment

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

Almost there! Please check my comments below. Ping me if you have any questions.

@apkatsikas apkatsikas mentioned this pull request Dec 26, 2023
Closed
3 tasks
@deluan deluan linked an issue Dec 26, 2023 that may be closed by this pull request
[Bug]: If the time of the lyrics is: [xx:xx.xxx], "No lyrics" is displayed. #2619
Closed
3 tasks
@deluan deluan mentioned this pull request Dec 27, 2023
Closed
@deluan deluan merged commit 814161d into navidrome:master Dec 28, 2023
@deluan deluan mentioned this pull request Dec 28, 2023
Closed
@github-actions GitHub Actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@deluan deluan deluan approved these changes

Assignees
No one assigned
Labels
frozen-due-to-age
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]: If the time of the lyrics is: [xx:xx.xxx], "No lyrics" is displayed.
3 participants
@kgarner7 @dweymouth @deluan