Merge lp:~ertain/elementaryos/pause-media-player-script into lp:~elementary-os/elementaryos/default-settings-trusty

[Jason Anderson]

This is a script which could pause all currently playing media players. It utilizes MPRIS and D-Bus to do this. While this script isn't finished, it's a start.

It is suppose to fix bug #769598. The filer of this bug wanted to a feature which stops all currently playing media players because they don't want their music to start playing when their laptop comes back from hibernation.

A way of testing this script is to put it into the /etc/pm/sleep.d/ directory, then start a music player (such as Banshee), put the computer into suspension, and finally bring it out of suspension. What should happen is that the currently playing media player is paused. If this doesn't occur, check the logs for why this didn't happen. In the system logs there should be markings for the script (for instance, look for "[13_pause_media_players] Pausing the media players...").

[lp:~ertain/elementaryos/pause-media-player-script]

127. By Jason Anderson on 2014-12-13

Updated cleaned up some code to 13_pause-media-players

128. By Jason Anderson on 2014-12-14

Cleaned up some of the code in 13_pause-media-players

129. By Jason Anderson on 2014-12-14

Refactored code in 13_pause-media-players

130. By Jason Anderson on 2014-12-15

13_pause-media-players Further tweaked the code. Still not working...

131. By Jason Anderson on 2014-12-18

Added a DISPLAY variable to the script because it cannot execute the command without X server running

[Sergey "Shnatsel" Davidoff (shnatsel)]

Cody asked me to review this, so I've commented on the diff.

[Jason Anderson (ertain)]

Thank you for reviewing over my script. All that stuff in there is just
to test to see what happens when the computer is suspending. I've gone
through a couple of iterations of the script, so I'll upload the latest
version I've been working on.

On 12/23/2014 01:50 PM, Sergey "Shnatsel" Davidoff wrote:
> Cody asked me to review this, so I've commented on the diff.
>
> Diff comments:
>
>> === added file '13_pause-media-players'
>> --- 13_pause-media-players 1970-01-01 00:00:00 +0000
>> +++ 13_pause-media-players 2014-12-18 20:36:38 +0000
>> @@ -0,0 +1,66 @@
>> +#!/bin/bash
>> +#
>> +# Copyright (c) Jason Anderson 2014 <sirius-C at iname dot com>
>> +# License: GPL-2
>> +# * December 1, 2014
>> +# - Version 0.1
>> +# - Initial version
>> +# * December 4, 2014
>> +# - Version 0.2
>> +# - Changed it for Totem video player
>> +# - This is compatible with Audacious, Clementine, and VLC
>> +# - Doesn't work for SMplayer
>> +# * December 13, 2014
>> +# - Cleaned up the code a little (added "suspend" to the case arguments).
>> +# - Found out Banshee works, too.
>> +# * December 14, 2014
>> +# - Version 0.3
>> +# - Refactored D-Bus code that sends the pause signal.
>> +# * Version 0.4
>> +# - December 15, 2014
>> +# - Really changed around the pause_music() function.
>> +# - Script still doesn't work.
>> +# * December 16, 2014
>> +# - Tried adding "DISPLAY" to the script as well as "dbus-launch" because
>> +# when the computer is suspending the script can't use "qdbus" nor
>> +# dbus-send.
>> +
>> +le_title="[13_pause-media-players]"
>
> why is this needed and not take from "$0"?
Just a placeholder. I'll add in "$0".

>
>> +# Just some random hacks
>> +DISPLAY=":0"
>
> Well that's an inspiring comment!
>
> Are you sure display ":0" will be always available, though? And which command requires it to be set, specifically?
>
>> +export $DISPLAY
>> +DBUS_SESSION_BUS_ADDRESS="unix:path=/run/dbus/system_bus_socket"
>> +export DBUS_SESSION_BUS_ADDRESS
>> +# Just some random hacks
>> +# TODO: use another D-Bus application that's installed by default (i.e. gdbus).
>> +output=$(qdbus org.mpris.MediaPlayer2.*)
>
> We could ship qdbus since it doesn't seem to have any Qt dependencies, despite its name.
> But if it's introducing the dependency on DISPLAY being set, then it's probably easiest to replace it with something that doesn't.
>
> I'm pretty sure that there is a way to send a message to all MPRIS players without querying them like this, because this way potentially introduces race conditions.
Yeah, I've been looking into gdbus, as well as dbus-monitor. If it's
not apparent, I check to see if the media player's dbus interface is
available. The thing is, qdbus can easily get the currently playing
media players. I've looked into using gdbus, but I can't figure out how
to get the exact interface, and use that in a call to dbus-send.

What's also strange is that, when I test this script (e.g. closing my
laptop lid to send the computer into suspension), it doesn't pick up the
currently playing media player. So I'll have Audacious or Clementine
playing, then suspend the laptop, and finally bring it out of
suspension. My hunch is that the scri...

[lp:~ertain/elementaryos/pause-media-player-script]

132. By Jason Anderson on 2014-12-24

More editing of the 13_pause-media-players script

133. By Jason Anderson on 2014-12-24

Took out the variable from 13_pause-media-players

[Sergey "Shnatsel" Davidoff (shnatsel)]

> What's also strange is that, when I test this script (e.g. closing my laptop lid to send the omputer into suspension), it doesn't pick up the currently playing media player.

That's because your script runs as root, not as the "current user" of which there may be several - multiseat is still a thing, not to mention several users being logged in at the same time.

So you'd have to iterate over all the users currently logged in, drop privileges to each user and then execute your MPRIS poking magic.

review: Needs Fixing

[lp:~ertain/elementaryos/pause-media-player-script]

134. By Jason Anderson on 2014-12-27

13_pause-media-players - Added code to change to user and get interface to media player

135. By Jason Anderson on 2015-01-01

Put in setuid code and took out environmental variable code.

[Jason Anderson (ertain)]

Okay, I have managed to get it to work.

What I have done is used setuid and checked whether the user is running
the media player. I had to use qdbus exclusively just to get the media
player and to tell it to pause.

I'll update my branch soon with the latest script.

On 12/26/2014 07:51 PM, Sergey "Shnatsel" Davidoff wrote:
> Review: Needs Fixing
>
>> What's also strange is that, when I test this script (e.g. closing my laptop lid to send the omputer into suspension), it doesn't pick up the currently playing media player.
>
> That's because your script runs as root, not as the "current user" of which there may be several - multiseat is still a thing, not to mention several users being logged in at the same time.
>
> So you'd have to iterate over all the users currently logged in, drop privileges to each user and then execute your MPRIS poking magic.
>

[Corentin Noël (tintou)]

Can't you use gdbus and not qdbus? Because elementary is a GLib environment…

[Jason Anderson (ertain)]

I've been trying to use the gdbus command. I might be able to use it
to send out the command, but I still have to figure out which media
players are running at the time of the suspension. And currently qdbus
is the only thing which can detect which media player is running.

On Sat, Jan 3, 2015 at 8:44 PM, Corentin Noël
<email address hidden> wrote:
> Can't you use gdbus and not qdbus? Because elementary is a GLib
> environment…
> --
> https://code.launchpad.net/~ertain/elementaryos/pause-media-player-script/+merge/244673
> You are the owner of
> lp:~ertain/elementaryos/pause-media-player-script.

[lp:~ertain/elementaryos/pause-media-player-script]

136. By Jason Anderson on 2015-01-06

Swapped out qdbus code for mdbus2 and gdbus code.'

[Jason Anderson (ertain)]

Okay, I think I've got it. I have swapped out the qdbus code for
mdbus2 and gdbus code. I think this will run on eOS just fine.

On Sat, Jan 3, 2015 at 8:44 PM, Corentin Noël
<email address hidden> wrote:
> Can't you use gdbus and not qdbus? Because elementary is a GLib
> environment…
> --
> https://code.launchpad.net/~ertain/elementaryos/pause-media-player-script/+merge/244673
> You are the owner of
> lp:~ertain/elementaryos/pause-media-player-script.

[lp:~ertain/elementaryos/pause-media-player-script]

137. By Jason Anderson on 2015-01-11

Code clean-up

138. By Jason Anderson on 2015-01-29

Changed some gdbus parameters and code for detecting DISPLAY variable

139. By Jason Anderson on 2015-01-31

Minor code and comment clean-up

[Sergey "Shnatsel" Davidoff (shnatsel)]

"setuid" command is not shipped by default. Please use "su" instead.

And please run it through http://www.shellcheck.net/ and fix it up - that thing has some very valid suggestions. Most notably "if [ $(pgrep totem) ]; then" should be "if pgrep totem > /dev/null; then"

And are you sure you still need to hardcode DISPLAY=":0" in there? Keep in mind this specific display number may not always be valid, and there may be several active displays.
If it's still needed, you might need to iterate over the existing displays for commands that depend on it, e.g. http://askubuntu.com/questions/230835/list-existing-x-display-names provides a method.
Off the top of my head, Totem invocation in here might still require it, but Totem has a D-bus interface. You'd have to check if bug #1182301 still applies, though.

You might want to run the output of "users" through "sort -u", because a run of "users" prints my username 3 times in a row. Or not - it shouldn't really matter.

Comments still mention qdbus despite it not being used in the script.

Sorry for the late review and thanks for getting this thing to work!

review: Needs Fixing

[lp:~ertain/elementaryos/pause-media-player-script]

140. By Jason Anderson on 2015-02-10

Made script more compatible with eOS standards.

141. By Jason Anderson on 2015-02-15

Changed sorting code for user logins

[Jason Anderson (ertain)]

Okay, I've got some good news and some bad news.

The good news is that I have taken out the calls to "setuid" and
substituted "su". That part seems to work. Also I have solved the
duplicate user problem as well as that Totem problem. Now for the bad news.

When I try out the script (start the music, place it in /etc/pm/sleep.d,
put the computer to sleep [hibernation or suspend], and then bring the
system back up), the system waits a full second before
suspending/hibernating. I have no clue why this happens. I looked for
a bottleneck, but I can't find the source. It could have something to
do with the command "users | tr ' ' '\n'" that I put in to weed out the
duplicate users. It could have something to do with ConsoleKit and
DBus. I don't really know. And worse, it can shut off WiFi (what I
think happens is that, when the computer comes back from sleep it
doesn't tell the wireless to wake up).

Anyway, I'll work on this and keep you posted.

On 02/09/2015 02:06 PM, Sergey "Shnatsel" Davidoff wrote:
> Review: Needs Fixing
>
> "setuid" command is not shipped by default. Please use "su" instead.
>
> And please run it through http://www.shellcheck.net/ and fix it up - that thing has some very valid suggestions. Most notably "if [ $(pgrep totem) ]; then" should be "if pgrep totem > /dev/null; then"
>
> And are you sure you still need to hardcode DISPLAY=":0" in there? Keep in mind this specific display number may not always be valid, and there may be several active displays.
> If it's still needed, you might need to iterate over the existing displays for commands that depend on it, e.g. http://askubuntu.com/questions/230835/list-existing-x-display-names provides a method.
> Off the top of my head, Totem invocation in here might still require it, but Totem has a D-bus interface. You'd have to check if bug #1182301 still applies, though.
>
> You might want to run the output of "users" through "sort -u", because a run of "users" prints my username 3 times in a row. Or not - it shouldn't really matter.
>
> Comments still mention qdbus despite it not being used in the script.
>
> Sorry for the late review and thanks for getting this thing to work!
>

[Jason Anderson (ertain)]

Well, guys, looks like I've hit a brick wall.

The script I was writing uses "su" so as to get on the user's DBus
session and figure out which media player they're using. I can't do
that any more. Since systemd is used primarily in Freya Beta 2, I
can't get on the user's DBus session via "su". In fact, I can't use
"setuid" either.

Unless there's a way we can get the user's DBus session through
systemd, it makes me think that there isn't a solution to this problem.

Even if I am mistaken about systemd, I can't use "su" anymore.

[lp:~ertain/elementaryos/pause-media-player-script]

142. By Jason Anderson on 2015-03-03

Switched to using sudo instead of su

143. By Jason Anderson on 2015-03-06

Added code to unset DISPLAY variable after testing user for media player

[Cody Garver (codygarver)]

Remove the changelog in the script

[Jason Anderson (ertain)]

Can do.

On 03/05/2015 10:03 PM, Cody Garver wrote:
> Remove the changelog in the script
>

[lp:~ertain/elementaryos/pause-media-player-script]

144. By Jason Anderson on 2015-03-08

Moved the changelog to its own file

[Cameron Norman (cameronnemo)]

The approach here is a deadend, as mentioned[1] in the previous comment. You need to listen for the PrepareForSleep() signal [2] in Noise and pause the music when you get that signal.

[1] https://code.launchpad.net/~ertain/elementaryos/pause-media-player-script/+merge/244673/comments/620756

[2] www.freedesktop.org/wiki/Software/systemd/inhibit/

[Cameron Norman (cameronnemo)]

review: Disapprove

[Jason Anderson (ertain)]

So I can use gdbus to listen for PrepareForSleep()? When that does come
up, how can I exactly pause the music? I mean, don't I need to be on
the user's session bus in order to pause it?

On 06/08/2015 01:48 PM, Cameron Norman wrote:
> The approach here is a deadend, as mentioned[1] in the previous comment. You need to listen for the PrepareForSleep() signal [2] in Noise and pause the music when you get that signal.
>
>
> [1] https://code.launchpad.net/~ertain/elementaryos/pause-media-player-script/+merge/244673/comments/620756
>
> [2] www.freedesktop.org/wiki/Software/systemd/inhibit/
>

[Cameron Norman (cameronnemo)]

Yeah you can just add the code into Noise itself and use the libgdbus
vala bindings

http://valadoc.org/#!wiki=dbus-glib-1/index

On Mon, Jun 8, 2015 at 3:33 PM, Jason Anderson <email address hidden> wrote:
> So I can use gdbus to listen for PrepareForSleep()? When that does come
> up, how can I exactly pause the music? I mean, don't I need to be on
> the user's session bus in order to pause it?
>
> On 06/08/2015 01:48 PM, Cameron Norman wrote:
>> The approach here is a deadend, as mentioned[1] in the previous comment. You need to listen for the PrepareForSleep() signal [2] in Noise and pause the music when you get that signal.
>>
>>
>> [1] https://code.launchpad.net/~ertain/elementaryos/pause-media-player-script/+merge/244673/comments/620756
>>
>> [2] www.freedesktop.org/wiki/Software/systemd/inhibit/
>>
>
>
> --
> https://code.launchpad.net/~ertain/elementaryos/pause-media-player-script/+merge/244673
> You are reviewing the proposed merge of lp:~ertain/elementaryos/pause-media-player-script into lp:~elementary-os/elementaryos/default-settings-trusty.

[Jason Anderson (ertain)]

So Noise listens for when the system goes to sleep (via libgdbus) and
pauses its music, or the music of other players?

On 06/08/2015 06:45 PM, Cameron Norman wrote:
> Yeah you can just add the code into Noise itself and use the libgdbus
> vala bindings
>
> http://valadoc.org/#!wiki=dbus-glib-1/index
>
> On Mon, Jun 8, 2015 at 3:33 PM, Jason Anderson <email address hidden> wrote:
>> So I can use gdbus to listen for PrepareForSleep()? When that does come
>> up, how can I exactly pause the music? I mean, don't I need to be on
>> the user's session bus in order to pause it?
>>
>> On 06/08/2015 01:48 PM, Cameron Norman wrote:
>>> The approach here is a deadend, as mentioned[1] in the previous comment. You need to listen for the PrepareForSleep() signal [2] in Noise and pause the music when you get that signal.
>>>
>>>
>>> [1] https://code.launchpad.net/~ertain/elementaryos/pause-media-player-script/+merge/244673/comments/620756
>>>
>>> [2] www.freedesktop.org/wiki/Software/systemd/inhibit/
>>>
>>
>>
>> --
>> https://code.launchpad.net/~ertain/elementaryos/pause-media-player-script/+merge/244673
>> You are reviewing the proposed merge of lp:~ertain/elementaryos/pause-media-player-script into lp:~elementary-os/elementaryos/default-settings-trusty.
>

[Cameron Norman (cameronnemo)]

Just its music.

[Jason Anderson (ertain)]

So how can I use libgdbus (and possibly Noise) to pause the music of
other music players?

On 06/08/2015 07:30 PM, Cameron Norman wrote:
> Just its music.
>

[David Hewitt (davidmhewitt)]

As mentioned above, this is the wrong approach for pausing media players on sleep. If this is still a requirement, Noise itself should listen for the sleep events on D-BUS and act on them then.

I'll leave this open for a couple more days for information, but will close afterwards to clean up the review list.

review: Disapprove

Unmerged revisions

144. By Jason Anderson on 2015-03-08

Moved the changelog to its own file

143. By Jason Anderson on 2015-03-06

Added code to unset DISPLAY variable after testing user for media player

142. By Jason Anderson on 2015-03-03

Switched to using sudo instead of su

141. By Jason Anderson on 2015-02-15

Changed sorting code for user logins

140. By Jason Anderson on 2015-02-10

Made script more compatible with eOS standards.

139. By Jason Anderson on 2015-01-31

Minor code and comment clean-up

138. By Jason Anderson on 2015-01-29

Changed some gdbus parameters and code for detecting DISPLAY variable

137. By Jason Anderson on 2015-01-11

Code clean-up

136. By Jason Anderson on 2015-01-06

Swapped out qdbus code for mdbus2 and gdbus code.'

135. By Jason Anderson on 2015-01-01

Put in setuid code and took out environmental variable code.

=== added file '13_pause-media-players'
--- 13_pause-media-players	1970-01-01 00:00:00 +0000
+++ 13_pause-media-players	2015-03-08 20:15:07 +0000
@@ -0,0 +1,57 @@
+#!/bin/bash
+# 
+# Copyright (c) Jason Anderson 2014 <JasonA.594 at gmail dot com>
+# License: GPL-2
+
+# Function for pausing the audio of each MPRIS-enabled media player.
+pause_music() {
+	# Since this script uses mdbus2, and it's not installed by default on
+	# eOS, check to see if it's installed.
+	if not hash mdbus2 2>/dev/null; then
+	  logger "[$0] Install mdbus2 and try script again."
+	  exit 0
+	fi
+    # Use the users command and go through each user that's logged in.
+    # This sorting code was recommend from the user go|dfish in the #Bash
+    # IRC channel.
+    for user in $(users | tr ' ' '\n' | sort -u); do      
+      # Check to see if the $DISPLAY variable is defined.  If not, then use some Bash kung-fu to
+      # get the proper DISPLAY.
+      if [ -z "$DISPLAY" ]; then
+        DISPLAY=$(w -hs "$user" | awk '{print $3}')
+        export DISPLAY
+      fi
+      # Here's the hard part: making the system think that the originating user issued the command.
+      output=$(sudo -u "$user" mdbus2 | grep MediaPlayer2)
+      # Now let's check to see if we really did have some output from that mdbus2 query.
+	  if [ -z "$output" ]; then
+	    logger "[$0] No media player detected."
+	    # Just so that the DISPLAY variable isn't reused for the next user,
+	    # we'll unset it.
+	    unset DISPLAY
+	    # Since we didn't find any media players running for this user, let's
+	    # go on to the next logged-in user.
+	    continue
+	  else
+	    logger "[$0] Now attempting to pause the media player..."
+	    sudo -u "$user" gdbus call --session --dest "$output" --object-path /org/mpris/MediaPlayer2 --method org.mpris.MediaPlayer2.Player.PlayPause
+	    logger "[$0] ...Okay, it's been paused."
+      fi
+    done
+}
+
+case "$1" in
+     hibernate|suspend)
+       logger "[$0] We're hibernating.  Going down!"
+       pause_music
+            ;;
+     resume|thaw)
+       logger "[$0] We're unthawing.  Rise and shine!"
+            ;;
+# If we have anything else than a hibernate or a thaw, try to exit gracefully.
+     *) 
+       exit 0
+            ;;
+esac
+
+exit 0

=== modified file 'debian/install'
--- debian/install	2014-12-27 05:28:46 +0000
+++ debian/install	2015-03-08 20:15:07 +0000
@@ -2,3 +2,4 @@
 policykit.desktop /usr/share/applications/
 plank/dock1 /etc/skel/.config/plank/
 settings.ini /etc/gtk-3.0/
+13_pause-media-players /etc/pm/sleep.d/