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

Multiple player dev #21

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

koolskateguy89
Copy link
Contributor

@koolskateguy89 koolskateguy89 commented Feb 19, 2021

Start of attempt to solve #16 (request to support other players)

  • Added checking which player is open (linear time ;) )
  • 'Verified' that VLC & Elisa work - if the media file has defined Title & Artist tags

Also extension.js:59 there should be let/var before children as it's previously undefined and was giving JS Warnings in Gnome-SHELL logs.

I still need to check the other players and check which other players support MPRIS, including Clementine & RhythmBox that the issue OP wanted (I mean add it to the players object and check it works).

@koolskateguy89
Copy link
Contributor Author

I have no idea how to request review....

@koolskateguy89 koolskateguy89 marked this pull request as draft February 19, 2021 01:05
@koolskateguy89 koolskateguy89 marked this pull request as ready for review February 19, 2021 01:07
@koolskateguy89
Copy link
Contributor Author

So apparently the PR author can't request review so I'm just have to tag you @mheine :)

@koolskateguy89
Copy link
Contributor Author

Note: I just got a JS ERROR: get_meta_window(...).get_wm_class(...) is null - no idea why.
I'll try and reproduce it tomorrow despite having no idea what caused it 😬

@mheine mheine self-requested a review February 19, 2021 08:05
@mheine
Copy link
Owner

mheine commented Feb 19, 2021

Assigned myself as a reviewer, hoping to add some thoughts and input when I find the time 🧑‍💻

@koolskateguy89
Copy link
Contributor Author

Thread about checking which player is open

https://unix.stackexchange.com/a/480820

@mheine mheine added the in progress Used for a PR currently being under development label Feb 21, 2021
@koolskateguy89
Copy link
Contributor Author

  • Gwenview is an image viewer (seems to also be a video viewer but I couldn't get that to work and gave up after like 5 seconds) so I removed it

  • Okular is a PDF viewer so I removed it

  • Amarok seems to no longer be maintained and I can't find a way to install it without compiling the source code so I can't confirm it works

  • I couldn't install Vvave when following the steps in their README (doing make gave error message No targets specified and no makefile found. Stop.) so I can't confirm it works

  • Clementine works fine - again as long as the media has defined Artist & Title tags

  • Plasma media center requires KDE Plasma to be installed

  • So does Plasma browser integration

(I'm on Ubuntu with Gnome Shell and not willing to install KDE Plasma tbh)

Every other player works for me and issue #16 (specifically Rhythmbox & Clementine) is solved :)

@mheine

@koolskateguy89
Copy link
Contributor Author

koolskateguy89 commented Feb 21, 2021

However checking the open player is difficult, the thread I posted earlier uses qdbus which when I run, gives an error could not find a Qt installation of '' so ¯\_(ツ)_/¯

@koolskateguy89
Copy link
Contributor Author

School starts again for me tomorrow so unfortunately I'll be quite inactive o7

Protects against future behaviour change
Copy link

@mavit mavit left a comment

Choose a reason for hiding this comment

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

A couple of general suggestions:

Hope this helps!

if (players[windowName])
return players[windowName];
}
}
Copy link

Choose a reason for hiding this comment

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

What about players that don't have windows? E.g., https://github.com/mavit/slimpris2/

It make more sense to ask DBus for a list of available players. E.g.,

dbus-send --print-reply --dest=org.freedesktop.DBus /org/freedesktop/DBus org.freedesktop.DBus.ListNames | grep -E '^ +string "org\.mpris\.MediaPlayer2\.'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in progress Used for a PR currently being under development
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants