plugin: Synced Lyrics #2207

Non0reo · 2024-07-08T14:33:54Z

Hi 👋

I made this plugin to make lyrics synchronous with the music.
I used LRCLIB to get lyrics as it's also another open-source project with an easy-to-use API.

I took code from the lyrics-genius to have a base for my plugin. I modified a part of it so it is compatible with new version of Youtube Music (while adding functionalities do fetch, change, and display lyrics)

ArjixWasTaken · 2024-07-11T20:42:38Z

Thanks for this amazing plugin! ❤️

Just curious, would it be fine if I merged the ideology of my draft plugin refined-lyrics (that has nothing implemented yet 😭) with yours?

By that I mean, if I extended your plugin with the features I planned to have in mine.

multiple lyric sources
chinese to pinyin
japanese to romaji
etc

It turns out that I have been way too busy and I've been procrastinating a lot to the point that I can't focus on that plugin.

Su-Yong

I recommend the plugin works with restartNeeded: false. but it's okay, I'm not forcing that. anyway, It seems to be great.

src/plugins/synced-lyrics/index.ts

Non0reo · 2024-07-12T10:15:01Z

Hi, thanks for your feedbacks

Yeah I have no problems merging your work here 👍
I also planed to add a feature to get plain lyrics from genius when the song does not have any lyrics (and making essentially what lyrics-genius did). Is it pertinent to implement it ? And have you worked on something similar, where in this case I should not bother to implement that ?

JellyBrick · 2024-07-14T10:54:11Z

You can get lyrics from YT using youtubei.js

src/plugins/synced-lyrics/index.ts

src/plugins/synced-lyrics/menu.ts

Non0reo · 2024-07-14T12:30:00Z

You can get lyrics from YT using youtubei.js

My plan was actually to have an additional source of lyrics and not only lyrics from YouTube as well as giving lyrics of a song even if it's a video. But it seems to be pretty annoying to scrape genius.com, so I don't think that I will try to implement this feature (unless I find a better way to scrape the html from the site)

Non0reo · 2024-07-14T13:11:03Z

src/plugins/synced-lyrics/renderer/lyrics/progress.ts

+      // MAGIC: Since the transition takes 300ms, we need to add a delay of 300ms to the current time
+      currentTime = secToMilisec(t) + 300;


Also, since this change, the lyrics seems to be displayed too early now. But if the way it worked before would have been a source of error we can keep it like that

chiqors · 2024-07-17T06:02:33Z

is this PR ready to test?

Non0reo · 2024-07-17T22:17:12Z

@chiqors I theory this plugin is functional

ArjixWasTaken · 2024-07-17T23:45:46Z

@Non0reo

~~I am doing some small refactoring, is it ok if I push my changes once done?~~
~~Or should I make a PR to your fork?~~

--

~~I'll go ahead and make it a PR to your fork.~~
Done, the PR is here!

ArjixWasTaken · 2024-07-18T01:03:10Z

In case you did not already do this, I'd suggest you use eslint and prettier, that way the code not only follows our formatting standards, it is also easier to review in pull requests.

PS: I was the one to mark the two comments about the text alignment as off-topic, please do not take offense.

ArjixWasTaken · 2024-07-18T11:23:27Z

LGTM

Additions like more lyric providers can be done in a separate PR.

So I think we should iron out:

translatable strings
maybe make more things configurable?
when in combination with album theme plugin, the lyrics are really hard to read (when an ad is playing for example)

Are there more stuff left to iron out?

Non0reo · 2024-07-18T12:02:38Z

Great for your contribution (you made me realized that my code was not that well written 😅)
For me the plugin seems great as it is, and in addition in what you already said I think we should investigate the followings:

Add other lyrics provider (and maybe add plain lyrics provider as I mentioned here)
When clicking on the Discover tab and then clicking on the Lyrics tab, the mutation isn't detected and the lyrics don't update correctly
Clicking on shuffle playlist button in an album does not trigger the ytmd:update-song-info and thus does not fetch the lyrics. I will create an issue for this one

Appart from that, everything is okay

ArjixWasTaken · 2024-07-18T12:10:01Z

When clicking on the Discover tab and then clicking on the Lyrics tab, the mutation isn't detected and the lyrics don't update correctly

In my now closed PR for refined-lyrics I made it always enable the lyrics tab, and it always called my code to replace the html within.

https://github.com/ArjixWasTaken/youtube-music/blob/refined-lyrics/src/plugins/refined-lyrics/contexts/renderer.ts

Maybe you can ~~steal~~ use that?
Although the way you and I implemented these, is way too different ngl.

For one, I was using SolidJS instead of generating html.

I can make another PR to migrate this to use SolidJS instead!

Non0reo · 2024-07-18T12:18:40Z

Oh cleaver ! I'm not really familiar with SolidJS, and also because you already fixed this issue I think that your PR would be better than my code

ArjixWasTaken · 2024-07-19T16:53:35Z

~~looks like I pushed to the wrong origin...~~
~~that was part 1, it temporarily broke the plugin as the SolidJS part is not implemented~~

Update: Working on it right now

ArjixWasTaken · 2024-07-21T01:54:28Z

@Non0reo I finally migrated the UI code to use SolidJS, there are some kinks I need to iron out though, like the text size is too small, notify the user that no lyrics were found, etc.

ArjixWasTaken · 2024-07-28T02:09:29Z

Other than fixing the styling, I don't think there is much to change for an initial version.

ArjixWasTaken · 2024-07-28T02:10:31Z

I'll re-review this tomorrow and try to get it merged.

- Change type assertion code - Replace span to `yt-formatted-string` - Add refetch button

JellyBrick · 2024-07-28T10:28:07Z

need i18n

ArjixWasTaken · 2024-07-28T21:51:39Z

Added i18n, well, for english at least.

src/plugins/synced-lyrics/renderer/components/LyricsContainer.tsx

Non0reo · 2024-07-30T17:12:51Z

I don't know if you're working on it but the text size seems too small. Maybe I did something wrong with my version or something like that though

Also, when the app start for the first time the lyrics arent fetched properly

ArjixWasTaken · 2024-07-30T17:15:01Z

I don't know if you're working on it but the text size seems too small

~~The css may need to be updated~~

Edit: Nope, looks fine for me

Also, when the app start for the first time the lyrics arent fetched properly

~~That is a known bug that I am trying to fix, although your screenshot is different from what I was experiencing~~
Edit: It is actually a general ytm bug unrelated to this PR.

ArjixWasTaken · 2024-07-30T18:11:46Z

@JellyBrick any more thoughts?

JellyBrick · 2024-07-31T10:58:34Z

I have a question. What happens if I run lyric-genius and synced lyrics at the same time?

ArjixWasTaken · 2024-07-31T12:04:25Z

I have a question. What happens if I run lyric-genius and synced lyrics at the same time?

is lyrics-genius even working?
either way, I think it would be better if we removed lyrics genius once synced-lyrics has multiple lyric providers

I'll handle that on my upcoming PR

* Added Plugin File * Added Logic * Known issue * Finished Backend part * Before cleanup * Added Style Removed log * Fixed time and visibility issues * Changed lyrics style * Changed way lyrics are selected * Fix * Added style lyrics options * Cleanup * Fix lyrics styling Changed how lyrics status are changed * Moved code to make file more readable * Change Tab Size * Fixed issue with overlapping lyrics * Removed debug console.log * Added style adaptation for music videos * Changed file indent * Revered back to original pnpm file * Removed unnecessary option * Fix lyrics status bug Removed leftover logs * Started to implement fetching for genius lyrics * feat(synced-lyrics): add `addedVersion` field * Made changes according to feedbacks * fix: add a delay of 300ms to the current time - Since the transition takes 300ms, we need to add a delay of 300ms to the current time * Removed test about genius.com scraping * Removed 300ms delay * chore: cleaned up the code * Specified path and variable * chore: always enable lyrics tab * chore: use SolidJS to render the lyrics * chore: remove useless signal * chore: feature-parity with original PR (+some nice stuff) * recreate lock file * show json decode error * feat(synced-lyrics): improve ui - Change type assertion code - Replace span to `yt-formatted-string` - Add refetch button * chore: make the lyric styling a solidjs effect * feat: i18n * chore: apply suggestion --------- Co-authored-by: Su-Yong <[email protected]> Co-authored-by: JellyBrick <[email protected]> Co-authored-by: Angelos Bouklis <[email protected]>

sH1222J · 2024-08-04T09:32:58Z

As a user who enjoys listening to K-pop and J-pop, there are almost no applied lyrics.

JellyBrick · 2024-08-04T10:36:37Z

@sH1222J https://lrclib.net/

heyluft · 2024-08-24T11:02:41Z

The lyrics for the song are on lrclib, but it doesnt work with the app, it just says 'No lyrics found for this song' 'Refetch lyrics'.

ArjixWasTaken · 2024-08-24T12:58:42Z

@heyluft this is not the place to hold such discussion, please either open an issue, or comment on an existing issue.

For example there is an existing issue similar to this, that I have made a PR to address it.

That issue and PR are specifically about tracks featuring multiple artists.
If your issue persists on tracks that do not feature multiple artists, then please make a new issue.

And always make sure to give example songs that have lyrics on lrclib but do not display in the app.

Rairof · 2024-08-24T21:08:30Z

I have a question. What happens if I run lyric-genius and synced lyrics at the same time?

is lyrics-genius even working? either way, I think it would be better if we removed lyrics genius once synced-lyrics has multiple lyric providers

I'll handle that on my upcoming PR

Would be great if users could just select the lyric provider manually as well through in plugin settings but by default it should probably be auto selected to whatever source is best whenever multiple lyrics source is implemented <3

ArjixWasTaken · 2024-08-24T21:42:42Z

Would be great if users could just select the lyric provider manually as well through in plugin settings but by default it should probably be auto selected to whatever source is best whenever multiple lyrics source is implemented <3

I'd imagine a priority order would be best.

Rairof · 2024-08-24T21:55:51Z

I'd imagine a priority order would be best.

Affirmative sir.

JellyBrick requested review from JellyBrick, Su-Yong and ArjixWasTaken July 10, 2024 09:22

JellyBrick added the enhancement New feature or request label Jul 10, 2024

Su-Yong approved these changes Jul 12, 2024

View reviewed changes

src/plugins/synced-lyrics/index.ts Show resolved Hide resolved

JellyBrick requested changes Jul 14, 2024

View reviewed changes

src/plugins/synced-lyrics/index.ts Outdated Show resolved Hide resolved

src/plugins/synced-lyrics/menu.ts Outdated Show resolved Hide resolved

Non0reo commented Jul 14, 2024

View reviewed changes

JellyBrick linked an issue Jul 14, 2024 that may be closed by this pull request

[Feature Request]: Lyrical Translation #2195

Open

2 tasks

JellyBrick removed a link to an issue Jul 14, 2024

[Feature Request]: Lyrical Translation #2195

Open

2 tasks

MulverineX mentioned this pull request Jul 15, 2024

plugin: Refined Lyrics #1768

Closed

4 tasks

Non0reo requested a review from JellyBrick July 17, 2024 20:09

This comment was marked as off-topic.

Sign in to view

ArjixWasTaken mentioned this pull request Jul 18, 2024

[Feature Request] Musixmatch synced lyrics integration #650

Open

ArjixWasTaken mentioned this pull request Jul 18, 2024

[Feature Request]: "Guess" Genius lyrics from song & artist name #2256

Closed

2 tasks

ArjixWasTaken self-assigned this Jul 21, 2024

ArjixWasTaken added 2 commits July 28, 2024 04:38

recreate lock file

df90e80

show json decode error

043862a

feat(synced-lyrics): improve ui

3e6fc5a

- Change type assertion code - Replace span to `yt-formatted-string` - Add refetch button

JellyBrick approved these changes Jul 28, 2024

View reviewed changes

JellyBrick and others added 3 commits July 28, 2024 19:28

Merge branch 'master' into th-ch-master

fb79977

chore: make the lyric styling a solidjs effect

bb3a4d6

feat: i18n

80380df

JellyBrick requested changes Jul 29, 2024

View reviewed changes

src/plugins/synced-lyrics/renderer/components/LyricsContainer.tsx Outdated Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

ArjixWasTaken force-pushed the th-ch-master branch from f50e2b9 to 80380df Compare July 30, 2024 17:08

chore: apply suggestion

12d9647

JellyBrick approved these changes Jul 31, 2024

View reviewed changes

JellyBrick merged commit 8ce91b1 into th-ch:master Jul 31, 2024
6 checks passed

		// MAGIC: Since the transition takes 300ms, we need to add a delay of 300ms to the current time
		currentTime = secToMilisec(t) + 300;

plugin: Synced Lyrics #2207

plugin: Synced Lyrics #2207

Conversation

Non0reo commented Jul 8, 2024

ArjixWasTaken commented Jul 11, 2024 • edited Loading

Su-Yong left a comment • edited Loading

Choose a reason for hiding this comment

Non0reo commented Jul 12, 2024

JellyBrick commented Jul 14, 2024

Non0reo commented Jul 14, 2024 • edited Loading

Non0reo Jul 14, 2024

Choose a reason for hiding this comment

chiqors commented Jul 17, 2024

This comment was marked as off-topic.

This comment was marked as off-topic.

Non0reo commented Jul 17, 2024

ArjixWasTaken commented Jul 17, 2024 • edited Loading

ArjixWasTaken commented Jul 18, 2024 • edited Loading

ArjixWasTaken commented Jul 18, 2024

Non0reo commented Jul 18, 2024 • edited Loading

ArjixWasTaken commented Jul 18, 2024 • edited Loading

Non0reo commented Jul 18, 2024

ArjixWasTaken commented Jul 19, 2024 • edited Loading

ArjixWasTaken commented Jul 21, 2024 • edited Loading

ArjixWasTaken commented Jul 28, 2024

ArjixWasTaken commented Jul 28, 2024

JellyBrick commented Jul 28, 2024

ArjixWasTaken commented Jul 28, 2024

This comment was marked as resolved.

Non0reo commented Jul 30, 2024

ArjixWasTaken commented Jul 30, 2024 • edited Loading

ArjixWasTaken commented Jul 30, 2024

JellyBrick commented Jul 31, 2024

ArjixWasTaken commented Jul 31, 2024

sH1222J commented Aug 4, 2024

JellyBrick commented Aug 4, 2024

heyluft commented Aug 24, 2024

ArjixWasTaken commented Aug 24, 2024 • edited Loading

Rairof commented Aug 24, 2024 • edited Loading

ArjixWasTaken commented Aug 24, 2024

Rairof commented Aug 24, 2024

ArjixWasTaken commented Jul 11, 2024 •

edited

Loading

Su-Yong left a comment •

edited

Loading

Non0reo commented Jul 14, 2024 •

edited

Loading

ArjixWasTaken commented Jul 17, 2024 •

edited

Loading

ArjixWasTaken commented Jul 18, 2024 •

edited

Loading

Non0reo commented Jul 18, 2024 •

edited

Loading

ArjixWasTaken commented Jul 18, 2024 •

edited

Loading

ArjixWasTaken commented Jul 19, 2024 •

edited

Loading

ArjixWasTaken commented Jul 21, 2024 •

edited

Loading

ArjixWasTaken commented Jul 30, 2024 •

edited

Loading

ArjixWasTaken commented Aug 24, 2024 •

edited

Loading

Rairof commented Aug 24, 2024 •

edited

Loading