Skip to content
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.

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

[Shortcuts] MPRIS fixes, Repeat Language bug fix #1005

Merged
merged 6 commits into from
Feb 9, 2023
Merged

[Shortcuts] MPRIS fixes, Repeat Language bug fix #1005

merged 6 commits into from
Feb 9, 2023

Conversation

Skydeke
Copy link
Contributor

@Skydeke Skydeke commented Jan 28, 2023

The MPRIS-Interface (Linux) has the range 0.0 - 1.0 for volume. It is also better to use the constants provided by the library. I changed the setupRepeatChangeListener() function because it returned the mode of the repeat of the music in the language that Youtube-Music has active, but because those strings are hard coded in mpris.js, it would break MPRIS if YT-music was set to another language. Thus I used the cookie set by YT-music to determine the repeat mode and mapped that to the english valuse since those were used in code.

plugins/shortcuts/mpris.js Outdated Show resolved Hide resolved
providers/song-info-front.js Outdated Show resolved Hide resolved
@Araxeus
Copy link
Collaborator

Araxeus commented Jan 28, 2023

LGTM (except the missing padding spaces inside curly braces, but that's just a style diff 😋)

@Skydeke
Copy link
Contributor Author

Skydeke commented Feb 3, 2023

After some more testing I did find a bug: Sometimes when the volume is changed cpu usage will spike to 100% for a few seconds. Don't merge until I have fixed that.

@Skydeke
Copy link
Contributor Author

Skydeke commented Feb 3, 2023

The reason the cpu sometimes spiked was that sometimes the mpris volume slider and the internal one got desynced and then recursively called the update function of the other. Its fixed now.

@th-ch th-ch merged commit 7bdbab5 into th-ch:master Feb 9, 2023
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.

3 participants