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

Better automatic player selection #160

Closed
wants to merge 1 commit into from

Conversation

panzi
Copy link

@panzi panzi commented Jan 11, 2020

New selection algorithm tries to find the first player that has the status
playing. If that fails it takes the first player with the status paused.
And if that also fails it takes the first remaining player (if any).

New selection algorithm tries to find the first player that has the status
playing. If that fails it takes the first player with the status paused.
And if that also fails it takes the first remaining player (if any).
@acrisci
Copy link
Member

acrisci commented Jan 11, 2020

I use Audacious and Amarok. I had music playing in Audacious and triggered playerctl play-pause (and another time playerctl next) and instead of issuing that command to Audacious to pause it, it made Amarok continue playing and thus two things played at once. And for the next case it made Amarok go to the next track and play it instead of doing that for the currently playing Audacious.

Thanks for your contribution. It's a novel idea and I'm not totally against it. Player selection definitely could use some more thought. But I can think of some weird cases and problems off the top of my head.

This is a bit more complicated which makes it hard to communicate what's happening. If I have two players and use play-pause and then play-pause again, it may act on two different players. That would be true in your case. The first play-pause would be for Audacious (correct) and the second would be for Amarok (incorrect) since prioritizing a playing player would no longer apply. So we've sacrificed some consistency and simplicity by adding more state into the equation without gaining total correctness. We can debate whether this trade-off is worth it. I've come to the conclusion that we cannot gain total correctness without mind reading hardware.

The second is that player priority via selection is a feature that people are using and this breaks that. So for instance, playerctl --player vlc,mpv will select vlc first if it's available and then mpv next. With #143, we can do an even better job with playerctl --player ANY,mpv to select a particular player last always or playerctl --player mpv,ANY to select a particular player first always. This feature would not be consistent with the new rule. Having two different rules when the --player flag is passed or not adds even more complexity.

So #143 satisfy what you want? It sounds like your intention is you would like to prioritize one player over another and this gives you that.

@panzi
Copy link
Author

panzi commented Jan 11, 2020

True. But while not perfect this patch is good enough for me. I'm going to use it until Audacious works again with the KDE player control widget, which tracks what the last active player was.

Another way to get it to work properly would be: run playerctl in some sort of daemon mode and expose itself a dbus interface. Then playerctl should first select the active player with some sort of heuristic, and then it will track which player comes online/goes offline and which was the last one in the "Playing" state. It will then issue commands to that player. The user can switch the active player by manually clicking play in a player.

Then I can bind the dbus interface of playerctl to global hotkeys in KDE settings.

@acrisci
Copy link
Member

acrisci commented Jan 11, 2020

The daemon mode is definitely something I'm interesting in implementing. I'm thinking about how that should work now. I don't think there is a proper issue for it, but it's been discussed before. So open a new issue for that.

Glad this works for you.

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.

None yet

2 participants