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

playerctl module: hide non-can-play players and use chrome and chromium as default value. #2256

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions py3status/modules/playerctl.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
*(default '[\?color=status [\?if=status=Playing > ][\?if=status=Paused \|\| ]'
'[\?if=status=Stopped .. ][[{artist}][\?soft - ][{title}|{player}]]]')*
format_player_separator: show separator if more than one player (default ' ')
player_hide_non_canplay: For hiding players which doesn't close mpris interface after media closing
(default ['chromium', 'chrome'])
players: list of players to track. An empty list tracks all players (default [])
seek_delta: time (in seconds) to change the playback's position by (default 5)
thresholds: specify color thresholds to use for different placeholders
Expand Down Expand Up @@ -98,6 +100,7 @@ class Py3status:
r"[\?if=status=Stopped .. ][[{artist}][\?soft - ][{title}|{player}]]]"
)
format_player_separator = " "
player_hide_non_canplay = ["chromium", "chrome"]
players = []
seek_delta = 5
thresholds = {"status": [("Playing", "good"), ("Paused", "degraded"), ("Stopped", "bad")]}
Expand Down Expand Up @@ -273,6 +276,12 @@ def playerctl(self):
players = []
cached_until = self.py3.CACHE_FOREVER
for player in tracked_players:
if (
not player.props.can_play
Copy link
Contributor

@lasers lasers Aug 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without digging in, Is not player.props.can_play alone enough without player_hide_non_canplay config.... This seems like an mpris issue on chrome/chromium side.

Do chrome/chromium change this flag when it's playing/not playing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually there are other chromium based browsers like brave which has same bug. Chromium issue tracker has already bug in their system: https://issues.chromium.org/issues/40703847
Even the issue is from older raport states same issue: JasonLG1979/gnome-shell-extension-mpris-indicator-button#10

Added some pictures about different "can" properties:

State when chromium is playing media
image

State when chromium media is paused:
image

State when chromium mpris is idle state:
image

VLC is opened but no media opened (just opened vlc, no media selected):
image

VLC is opened, media loaded and stopped playing media.
image

I don't wan't to blindly hide all players with can_play == false state because I prefer to see VLC media player on statusbar just after I opened it as empty player.

It littlebit seems like hide_non_can_play is too technical so even we can simplify it to level: hide media players stopped state and user provides list on media players which he want to hide.

I searched some chromium source code and found out where this all "can" properties false comes. I suspect that line is this: https://source.chromium.org/chromium/chromium/src/+/main:components/system_media_controls/linux/system_media_controls_linux.cc;l=288

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have been out of scene with py3status. Just sharing thoughts.

I think it doesn't benefit users to display empty players. If anything, this is maybe something that should be added in first place to ensure better quality control/behavior.

I do think hide_non_can_play config is too specific that partially addresses chromium-based players and not that general... I would like an approach that that users can allow/deny (all) can_xxx_xxx configs (either by a dict config or new placeholders). New placeholders that exposes boolean like this... is maybe hard for users to come up with a new format_player that excludes those players... Dict may be be easier.

I prefer to see VLC media player on statusbar just after I opened it as empty player.

It should be easier to just ignore players that doesn't have can_play flag true because of things like... You saying you don't like this empty-player thing on chrome/chromium #2255 but are okay with this on vlc... I think it would make more sense to avoid all empty-players in first place... to keep it simple.

This approach also may omit the need for adding |{player} at end of format_player config too.

Try it for a while. See if this small but simple tweak works quite well.

and player.props.player_name in self.player_hide_non_canplay
):
continue

player_data = self._get_player_data(player)

# Check if the player should cause the module to continuously update
Expand Down
Loading