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

Integrate mpris #93

Closed
martpie opened this issue Jun 30, 2016 · 33 comments
Closed

Integrate mpris #93

martpie opened this issue Jun 30, 2016 · 33 comments

Comments

@martpie
Copy link
Owner

martpie commented Jun 30, 2016

Basically:

ubuntu


Edit: 11/09/18

Things to think about:

@YurySolovyov
Copy link
Collaborator

Found mpris-service, but it seems like it uses native deps.
There is also node-dbus in pure JS. Might worth doing a fork or something

@vprigent
Copy link
Contributor

Could better system integration also include notifications and computer specific shortcuts for play/pause ?

Notifications could be a whole new issue though ...

@YurySolovyov
Copy link
Collaborator

@vprigent what kind of info do you expect to see in notifications?

@vprigent
Copy link
Contributor

Which music in currently playing

Depending on the notification system, you can probably include prev/next buttons
I don't see that as a top priority. As a user, i would be more interested in interacting with the software without the need to switch to the program.

@martpie
Copy link
Owner Author

martpie commented Sep 12, 2016

@vprigent so when a track changes, add a native notification ? Seems fair to me.

About prev/next buttons, they are present on Windows, the Ubuntu integration is in progress but it's kind of complicated, and the native key for next/prev/play on Mac should work fine in the next release.

@vprigent
Copy link
Contributor

@KeitIG Yup ! I've seen that kind in Rhythmbox ( Linux app )
I need to test the app on Windows ! Currently linux fan ..

@martpie martpie mentioned this issue Sep 21, 2016
18 tasks
@mhanoglu
Copy link

mhanoglu commented Apr 7, 2017

And sometimes Fn+F11 / Fn+F12 key combinations for prev & next song not working..
Thanks for program.

OS: ElementaryOS 0.4 ( Ubuntu 16.04 )

@martpie
Copy link
Owner Author

martpie commented Apr 7, 2017

@marzochi Playback keyboard shortcuts "should" be supported, even if I could not test it personally (all laptops have different shortcuts for these). Can you send us the model of your laptop?

@mhanoglu
Copy link

mhanoglu commented Apr 8, 2017

Thanks for reply.

description: Laptop
product: IdeaPad Z580 (System SKUNumber)
vendor: LENOVO
version: Lenovo Z580
elementary OS 0.4 Loki (64-bit)
Built on "Ubuntu 16.04.2 LTS"

@martpie
Copy link
Owner Author

martpie commented Feb 6, 2018

So I war re-reading http://www.omgubuntu.co.uk/2017/09/will-you-miss-gnome-legacy-tray again to understand why Gnome wanted Trays to disappear.

The explanation is basically: don't use Trays when you can use MPRIS.

Reminder:

@martpie
Copy link
Owner Author

martpie commented Feb 6, 2018

Apparently https://getharmony.xyz managed to do it

@martpie martpie changed the title Better system integration on Ubuntu/Gnome Integrate mpris Apr 4, 2018
@martpie
Copy link
Owner Author

martpie commented Apr 4, 2018

@YurySolovyov
Copy link
Collaborator

node-dbus-next (pure JS) has MPRIS media player example that might be of use.

@igorer88
Copy link
Contributor

igorer88 commented Mar 1, 2019

Working on it ;)

@martpie martpie added this to the 0.11 milestone Mar 2, 2019
@martpie
Copy link
Owner Author

martpie commented Mar 13, 2019

with mpris-service@2.0.0 elementary os (Ubuntu 18.04) stack trace


> museeks@0.10.1 museeks /home/pierre/dev/museeks
> electron .

(node:4575) UnhandledPromiseRejectionWarning: Error: unknown bus address
    at createStream (/home/pierre/dev/museeks/dist/main/webpack:/node_modules/dbus-next/index.js:22:1)
    at createConnection (/home/pierre/dev/museeks/dist/main/webpack:/node_modules/dbus-next/index.js:76:1)
    at Object../node_modules/dbus-next/index.js.module.exports.createClient (/home/pierre/dev/museeks/dist/main/webpack:/node_modules/dbus-next/index.js:136:1)
    at Object../node_modules/dbus-next/index.js.module.exports.sessionBus (/home/pierre/dev/museeks/dist/main/webpack:/node_modules/dbus-next/index.js:149:1)
    at Player../node_modules/mpris-service/dist/index.js.Player.init (/home/pierre/dev/museeks/dist/main/webpack:/node_modules/mpris-service/dist/index.js:105:1)
    at new Player (/home/pierre/dev/museeks/dist/main/webpack:/node_modules/mpris-service/dist/index.js:97:1)
    at Player (/home/pierre/dev/museeks/dist/main/webpack:/node_modules/mpris-service/dist/index.js:90:1)
    at new MprisModule (/home/pierre/dev/museeks/dist/main/webpack:/src/main/modules/mpris.ts:14:12)
    at /home/pierre/dev/museeks/dist/main/webpack:/src/main/main.ts:124:5
    at step (/home/pierre/dev/museeks/dist/main/bundle.js:62402:23)
(node:4575) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:4575) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Ubuntu 18.10:

➜  museeks git:(mpris) npm run museeks

> museeks@0.10.1 museeks /home/pierre/Documents/museeks
> electron .

(node:3444) UnhandledPromiseRejectionWarning: Error: unknown bus address
    at /home/pierre/Documents/museeks/dist/main/bundle.js:13:4223
    at createConnection (/home/pierre/Documents/museeks/dist/main/bundle.js:13:5336)
    at Object.module.exports.createClient (/home/pierre/Documents/museeks/dist/main/bundle.js:13:6211)
    at Object.module.exports.sessionBus (/home/pierre/Documents/museeks/dist/main/bundle.js:13:6536)
    at Player.init (/home/pierre/Documents/museeks/dist/main/bundle.js:130:29716)
    at new Player (/home/pierre/Documents/museeks/dist/main/bundle.js:130:29497)
    at Player (/home/pierre/Documents/museeks/dist/main/bundle.js:130:29346)
    at new MprisModule (/home/pierre/Documents/museeks/dist/main/bundle.js:417:129483)
    at /home/pierre/Documents/museeks/dist/main/bundle.js:417:136413
    at /home/pierre/Documents/museeks/dist/main/bundle.js:417:134144
(node:3444) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:3444) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

@martpie
Copy link
Owner Author

martpie commented Mar 13, 2019

@acrisci Any idea what could go wrong here? When editing the module directly and forcing it to be 'unix:path=/var/run/dbus/system_bus_socket', other errors are thrown.

@acrisci
Copy link

acrisci commented Mar 13, 2019

You should be running this on the session bus which for me is at /run/user/$UID/bus.

The dbus library should be reading the DBUS_SESSION_BUS_ADDRESS environment variable for the path of the session bus.

$ echo $DBUS_SESSION_BUS_ADDRESS 
unix:path=/run/user/1000/bus

I'm looking at the code for dbus-next and it might not actually be doing that. It might just be trying to read it from the x11 property, but I need to look into that.

If you're running this on Wayland, that's not tested and may introduce some bugs that need to be fixed.

Looking into all of this now.

@acrisci
Copy link

acrisci commented Mar 13, 2019

I looked into dbus-next and confirmed it is reading DBUS_SESSION_BUS_ADDRESS for the session bus address. You will get this error only when that environment variable is not set.

None of the X11 code is working, so that's pretty much the only way to specify the bus address right now.

The session manager should be starting dbus (like with dbus-launch) to start the bus and setting those variables but yours might not be doing that. My stock 18.04 does set those variables, but I didn't test with elementary. This might be their bug, but getting the dbus-next X11 code working might fix this (unless they use Wayland and don't have X11 support compiled into their dbus libraries, which is a possibility).

@martpie
Copy link
Owner Author

martpie commented Mar 13, 2019

thanks; I'll investigate on my side why they are not set!

@igorer88
Copy link
Contributor

@acrisci Mint 19.1 is based on Ubuntu 18.04. So if it works there, it should work in here too. I agree with you on fixing dbus-next X11 would solve the problem.

@martpie
Copy link
Owner Author

martpie commented Mar 13, 2019

I don't think it is a X11 problem, because mpris-service@1.4 worked fine. This problem is only with mpris-service@2.0.0.

Maybe a missing DE dependency...

@acrisci
Copy link

acrisci commented Mar 14, 2019

I did a detailed writeup on the issue on dbus-next here.

@martpie
Copy link
Owner Author

martpie commented Mar 26, 2019

With dbus-next@0.4.1, the error is a bit more precise:

(node:5641) UnhandledPromiseRejectionWarning: Error: could not get DISPLAY environment variable to get dbus address
    at getDbusAddressFromFs (/home/pierre/Documents/museeks/dist/main/bundle.js:417:25058)
    at /home/pierre/Documents/museeks/dist/main/bundle.js:13:5792
    at createConnection (/home/pierre/Documents/museeks/dist/main/bundle.js:13:6896)
    at Object.module.exports.createClient (/home/pierre/Documents/museeks/dist/main/bundle.js:13:7771)
    at Object.module.exports.sessionBus (/home/pierre/Documents/museeks/dist/main/bundle.js:13:8096)
    at Player.init (/home/pierre/Documents/museeks/dist/main/bundle.js:130:43113)
    at new Player (/home/pierre/Documents/museeks/dist/main/bundle.js:130:42894)
    at Player (/home/pierre/Documents/museeks/dist/main/bundle.js:130:42743)
    at new MprisModule (/home/pierre/Documents/museeks/dist/main/bundle.js:417:132442)
    at /home/pierre/Documents/museeks/dist/main/bundle.js:417:139372
(node:5641) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:5641) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

(Ubuntu 18.04.2)

@acrisci
Copy link

acrisci commented Mar 26, 2019

I've run the mpris-service integration test suite on stock Ubuntu 18.04 and it works fine. Both the DISPLAY environment variable is set and the session manager sets DBUS_SESSION_BUS_ADDRESS.

Can you give me some more info on your setup so I can try to reproduce the issue?

@martpie
Copy link
Owner Author

martpie commented Mar 26, 2019

Ah, interesting thing, I am running the Communitheme snap session. Let me try with the default one.

@martpie
Copy link
Owner Author

martpie commented Mar 26, 2019

Mmh getting the same with the classic session, let me try with a fresh install.

@martpie
Copy link
Owner Author

martpie commented Mar 26, 2019

Getting the same issue on a fresh install. I guess there is a problem with the environment variables then. Maybe Wepback is doing something nasty.

I am now trying to not bundle node-dbus-next so it is called from node_modules.

@martpie
Copy link
Owner Author

martpie commented Mar 26, 2019

➜  museeks git:(mpris) ✗ echo $DISPLAY
:0

Does it look good?

@acrisci How do you usually get the DISPLAY environment variable with Node.js?

@martpie
Copy link
Owner Author

martpie commented Mar 26, 2019

ok, process.env is returning all the good results when asking the node prompt, but is only returning { NODE_ENV: 'production' } when running the app after bundling it.

I think Wepback or Electron is somewhat sanitizing these variables at build/runtime.

I need to find a way to let these variables be accessible at runtime

@martpie
Copy link
Owner Author

martpie commented Mar 27, 2019

BAM! Now mpris-service needs an update because of 0.4's breaking changes, but I guess we're almost good to go.

Screen Shot 2019-03-27 at 15 56 30

The problem was indeed Webpack, and more specifically the DefinePlugin:

new webpack.DefinePlugin({
  'process.env': {
    NODE_ENV: '"development"',
  },
}),

Would replace all process.env by the provided object in the bundle.

@martpie
Copy link
Owner Author

martpie commented Mar 27, 2019

Et voilà. A couple of bugfixes to do and I guess we're good.

Screen Shot 2019-03-27 at 16 39 33

@martpie
Copy link
Owner Author

martpie commented Mar 27, 2019

Closed by #477 🎉

Base64 covers are not supported, but I guess it's fine for now.

@martpie martpie closed this as completed Mar 27, 2019
@igorer88
Copy link
Contributor

igorer88 commented Mar 27, 2019

Wow!! Excellent!! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants