-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
MPV MPRIS plugin fix #24285
MPV MPRIS plugin fix #24285
Conversation
srcpkgs/mpv-mpris/template
Outdated
vmkdir usr/lib/${pkgname} | ||
vinstall mpris.so 0644 usr/lib/${pkgname} | ||
vmkdir etc/mpv/scripts | ||
vinstall mpris.so 0644 etc/mpv/scripts/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this enable it by default? In my setup I have ~/.config/mpv/scripts/mpris.so -> /usr/lib/mpv-mpris/mpris.so
, and it just works. Having it enabled for everyone wouldn't be ideal for a multiuser setup or similar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, would this result in errors for people who aren't using a D-Bus bus?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In either case @ericonr makes a good point. However, if we revert this change, I think it would be helpful to include a post-install message letting users know to ln -s /usr/lib/mpv-mpris/mpris.so ~/.config/mpv/scripts/mpris.so
. At least for slightly noobish users like me ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about vdoc
ing the README? https://github.com/hoyon/mpv-mpris
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does that do exactly (I added vdoc anyways)? I was thinking something more along the lines of using INSTALL.msg
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It adds the README to the package, so people can hunt for how to enable it in /usr/share/doc/mpv-mpris
.
This change is part of an effort to fix integration between MPV and its MPRIS plugin. mpv: Remove trailing whitespace
Closed in c26a756 and its parent. |
mpv and the mpv-mpris plugin weren't working together out-of-the-box for me, so these edits fix that.
I've confirmed on my end that these changes work :)
The changes made are based on the documentation for mpv-mpris here.