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

Player hooks with data arguments #190

Closed
apprehensions opened this issue May 5, 2023 · 26 comments · Fixed by #244
Closed

Player hooks with data arguments #190

apprehensions opened this issue May 5, 2023 · 26 comments · Fixed by #244
Labels
enhancement New feature or request

Comments

@apprehensions
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Recently, I have stopped using spotify-player simply because libreapot does not has support for external connections such as last.fm, and can not do notifications without Dbus.

Describe the solution you'd like
A hook system that launches a program with arguments set in the configuration for events such as pre, post, during (second interval) for sub-events for playing, pausing, skipping, etc..

I can use these to pass to notification or lastfm scrobller clients.

Describe alternatives you've considered
Using librespot or spotifyd directly, I believe librespot has direct support for this feature, so I don't think it would be trivial to implement it here.

@apprehensions apprehensions added the enhancement New feature or request label May 5, 2023
@apprehensions
Copy link
Contributor Author

@aome510 i would like to hear from this.

i am tired of the propietary spotify client.

@aome510
Copy link
Owner

aome510 commented Jul 21, 2023

I've been quite busy lately, so I won't be able to implement this soon. However, will put this up in my TODO list and see look into ways to integrate the last.fm APIs, if any.

@apprehensions
Copy link
Contributor Author

well, this feature request is for hooks, which is alot simpler than implementing the last.fm API, since that can be handled by another program making the stack simpler.

@aome510
Copy link
Owner

aome510 commented Aug 26, 2023

Plan to implement this next week or so. Does writing player events in a cache file work? You can write a script to watch for changes in that file every second or so, and port that to last.fm client. Lmk if I need to include any fields other than the event data (I plan to include the timestamp).

@apprehensions
Copy link
Contributor Author

honestly been using the propietary spotify client for a long while now, i'd much rather have librespot itself actually supporting this shit rather than me having to send last.fm api requests constantly

@aome510
Copy link
Owner

aome510 commented Aug 26, 2023

Just saw this issue: jpochyla/psst#424. Look like lastfm scrobbling is supported via Spotify connect, so it should work by default with spotify_player. I just double check on last.fm website and it shows all the scrobbles when I use spotify_player. Can you confirm? If that's the case, I'll close this issue.

Screenshot 2023-08-26 at 4 32 40 PM

@apprehensions
Copy link
Contributor Author

Wow, okay!!!! I'll check tommorow!!

I will be very happy if this is true.

@apprehensions
Copy link
Contributor Author

image
SEEMS TO WORK!!
although, spotify cant quite see what i'm playing, so that spotify yearly thing wont quite work

@apprehensions
Copy link
Contributor Author

thank u

@aome510
Copy link
Owner

aome510 commented Aug 27, 2023

although, spotify cant quite see what i'm playing, so that spotify yearly thing wont quite work

Related librespot-org/librespot#913. That's one thing I miss from the official app and wish that librespot figure out ways to reverse-engineer the feature.

@apprehensions
Copy link
Contributor Author

It is a bit infuriating to be honest, considering its been a problem for 4 years and no one had stepped up and reversed engineered it :(

@apprehensions
Copy link
Contributor Author

Wait, this issue still remains.

@apprehensions apprehensions reopened this Aug 31, 2023
@apprehensions
Copy link
Contributor Author

I do want to have some form of player hooks to be honest; my use case being to avoid constant D-Bus requests.

@aome510
Copy link
Owner

aome510 commented Sep 2, 2023

I do want to have some form of player hooks to be honest; my use case being to avoid constant D-Bus requests.

Does writing player events into a file work? I'm not too familiar with the player hook concept and how to implement one.

@apprehensions
Copy link
Contributor Author

Player hooks is to launch a program with certain metadata (such as song, album, title) as arguments, on certain events (such as play, new song, pause, etc)

@aome510
Copy link
Owner

aome510 commented Sep 3, 2023

@apprehensions
Copy link
Contributor Author

why json? :(

aome510 added a commit that referenced this issue Sep 4, 2023
Resolves #190. 
Resolves #103.

Added `player_event_hook_command` as a config option. Each time the application receives a player event, `player_event_hook_command ` is executed with the event JSON string as the standard input.
@aome510
Copy link
Owner

aome510 commented Sep 4, 2023

why json? :(

for consistency with CLI commands' output and because the app already supports exporting Spotify data in JSON format.

@apprehensions
Copy link
Contributor Author

Why not ususe JSON for big Spotify data? For example for simple data it should simply be it in plain text, and the arguments to be passed as the arguments themselves:

script "Playing" "Artist" "Album" "Song", this ensures that scripting is very easy. I'd appreciate it if you did this as this is what I had expected..

@apprehensions
Copy link
Contributor Author

I can't use this feature; setting player_event_hook_command to a valid binary path gives: player_event_hook_command: None.

player_event_hook_command = "spotify_player-hook"
2023-09-07T16:28:20.475855Z  INFO spotify_player: General configurations: AppConfig { theme: "sewn", client_id: "XXXXXXXXXXXXXXXXXXXXX", client_port: 8080, copy_command: Command { command: "xclip", args: ["-sel", "c"] }, player_event_hook_command: None, playback_format: "{track}\n{artists} - {album}\n{metadata}"...

@aome510
Copy link
Owner

aome510 commented Sep 7, 2023

Please refer to the linked document to use a correct format. It should be player_event_hook_command = { command = "spotify_player-hook", args = [] }

Regarding why JSON but not plain text with arguments,

  1. different events have different fields, passing the fields as command's arguments simply does not scale.
  2. script can be arbitrarily complexed as user wants, the passing data to the script logic from spotify_player side is not. As a result, the passing data logic should be simple (e.g passing the event as a single input).

@apprehensions
Copy link
Contributor Author

apprehensions commented Sep 7, 2023

different events have different fields, passing the fields as command's arguments simply does not scale.

#!/bin/sh

echo "command: $1"

case "$1" in
        Changed) echo "old_track_id: $3, new_track_id: $3" ;;
        Playing) echo "track_id: $2, position_ms: $3, duration_ms: $4" ;;
        Paused) echo "track_id: $2, position_ms: $3, duration_ms: $4" ;;
        EndOfTrack) echo "track_id: $2" ;;
esac

I hightly suggest to you to please use a arguement system, as it makes my life and others much easier when scripting for spotify-player.

@aome510
Copy link
Owner

aome510 commented Sep 7, 2023

I hightly suggest to you to please use a arguement system, as it makes my life and others much easier when scripting for spotify-player.

Ok, I'll think about it.

@aome510
Copy link
Owner

aome510 commented Sep 7, 2023

Just a note that with the arguments approach, new user will be confused between the arguments, e.g is it Changed old_track_id new_track_id or Changed new_track_id old_track_id? Then they are required to actually read the document, which many don't usually.

@apprehensions
Copy link
Contributor Author

apprehensions commented Sep 7, 2023

Well? They need to read the docs to understand what the hooks do, their arguments and commands to begin with.

@aome510
Copy link
Owner

aome510 commented Sep 10, 2023

Should be updated with #251.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants