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

Add Linux implementation #10

Merged
merged 25 commits into from
Jan 9, 2024
Merged

Add Linux implementation #10

merged 25 commits into from
Jan 9, 2024

Conversation

holybaechu
Copy link
Contributor

@holybaechu holybaechu commented Jan 7, 2024

Adds Linux implementation using MPRIS and DBus

Will work on almost all distros that uses systemd (Almost all distros use systemd)

@holybaechu
Copy link
Contributor Author

holybaechu commented Jan 7, 2024

I would appreciate it if LabyStudio update his Spotify addon on FlintMC along with this PR if it gets merged

@holybaechu
Copy link
Contributor Author

holybaechu commented Jan 7, 2024

I’m currently writing DBus parser to use this without playerctl installed.
Might take long time

@holybaechu
Copy link
Contributor Author

I’m JavaScript developer so I may not use Java naming conventions.
I will try to refactor the variables and classes

@LabyStudio
Copy link
Owner

Hey, nice work!
Could you wrap the dbus communication in an extra class? Like the AppleScript implementation for macOS.

@holybaechu
Copy link
Contributor Author

holybaechu commented Jan 8, 2024

Hey, nice work!
Could you wrap the dbus communication in an extra class? Like the AppleScript implementation for macOS.

I warpped D-Bus communication to MPRISCommunicator class now.

Also LabyStudio, Please note that this only works with Linux distros that uses systemd.
Here is a list of Linux distros that doesn't use systemd. (Ex: Android)

@LabyStudio
Copy link
Owner

I've changed the structure of the dbus & mpris api, but it may not work anymore because I'm on Windows and can't test it properly. I guess you will have to debug it really quick and adjust the parsing to fix the issues

@holybaechu
Copy link
Contributor Author

holybaechu commented Jan 8, 2024

I don't know exactly why but runtime.exec didn't work well with dbus-send in past and now.
I'll edit the code to use ProcessBuilder probably tomorrow.

@LabyStudio
Copy link
Owner

I managed to connect to the dbus of my WSL and test the Linux implementation under Windows.
It works without any problems! It would be good to know if it works on your system too!

@holybaechu
Copy link
Contributor Author

holybaechu commented Jan 9, 2024

Note that position unit is in seconds, not milliseconds.
I edited getTrackLength method and the if condition to make it work.
Everything else is working fine.

@holybaechu
Copy link
Contributor Author

Nevermind, It was in milliseconds.
But I don't know why the position is not updating frequently.
I'll look into this issue right now.

@holybaechu
Copy link
Contributor Author

2024-01-09.12-55-46.mp4

I ported this PR to the Spotify addon and this is happening.
Whenever i do any action, the timestamp goes away for like 1 seconds and appears again and when i pause and play the song, It flickers.

Any solutions to this?

@holybaechu
Copy link
Contributor Author

holybaechu commented Jan 9, 2024

I tested this on my actual LabyMod config that i use when playing, And whenever i pause and play, The widget disappers.

@holybaechu
Copy link
Contributor Author

I tested it and this is the output when I play and pause:

Connected to Spotify!
Track changed: [7a5Z1tB8JiO5zSDIdVuuf3] 제발 - Kim Bum Soo (4m 41s)
Loaded track cover: 640x640
Position changed: 1m 18s of 4m 41s (27%)
Triggered Play/Pause Media key
Disconnected: Invalid variant: 
Connected to Spotify!
Song started playing
Position changed: 1m 20s of 4m 41s (28%)
Triggered Play/Pause Media key
Disconnected: Invalid variant: 
Connected to Spotify!
Song stopped playing

@holybaechu
Copy link
Contributor Author

It was because when sending dbus-send to call function, It does not print anything except for method ~ so the result of builder.toString() in method DBusSend.send() would be empty. So i added if condition to check if the builder.toString()'s result is empty or not.

Bae Joon Hoo and others added 3 commits January 9, 2024 13:27
…nt if the media player interrupts or changes its current "direction".

added a getPosition output line at onSync in SpotifyListenerTest to debug its current calculated position
@LabyStudio
Copy link
Owner

The onPositionChanged in the listener you are using should not be used as an indicator of its actual current position. It is actually an indicator of a change in the current "flow" or "direction".

The api.getPosition() function automatically calculates Spotify's current position by adding the elapsed time to the last known position.
It is implemented in such a way that it provides a consistent estimate of the current position without flooding the application with requests.

If for some reason your position still jumps, that must be a different problem. I just tested it again and it works fine for me.
Try removing the debug message in onSync that I just added. That's the actual position you want to keep track of.

@holybaechu
Copy link
Contributor Author

Ooh so the last known position works like offset for getting current position.
LGTM!

@LabyStudio
Copy link
Owner

Nice! Ready for merge?

@holybaechu
Copy link
Contributor Author

Yup

@LabyStudio LabyStudio changed the base branch from master to update January 9, 2024 11:06
@LabyStudio
Copy link
Owner

Thank you for your awesome contribution!

@LabyStudio LabyStudio merged commit 941cfa2 into LabyStudio:update Jan 9, 2024
1 check passed
@holybaechu
Copy link
Contributor Author

holybaechu commented Jan 9, 2024

Thank you for your awesome contribution!

No problem!
Also, Please update the Spotify addon as soon as possible, Thanks!

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.

2 participants