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

X11 regression: mpv no longer prevents screen lock #4752

Closed
catharanthus opened this issue Aug 11, 2017 · 27 comments
Closed

X11 regression: mpv no longer prevents screen lock #4752

catharanthus opened this issue Aug 11, 2017 · 27 comments

Comments

@catharanthus
Copy link

mpv version and platform

mpv version: 0.26.0-140-g1a1f6e8581

Kubuntu 17.04 with KDE Plasma 5.9.4

Reproduction steps

  1. In system settings, set the screen lock delay to some small value (like 1 min)

  2. Start playing some video with mpv

  3. Wait for a specified time without keyboard/mouse events

Expected behavior

mpv should prevent the system from locking the screen when video is being played.

Actual behavior

The system automatically locks the screen.

The regression manifests itself since 3f75b3c.

Log file

https://pastebin.com/4QRpbmEZ

@CounterPillow
Copy link
Contributor

The problem is a bug in xdg-screensaver making it not work on KDE's powerdevil

see https://bugs.freedesktop.org/show_bug.cgi?id=102124

good luck ever getting this fixed since Freedesktop thinks their shit being completely broken is just "minor"

@mc4man
Copy link

mc4man commented Aug 12, 2017

If xscrnsaver helps you just add back, easy revert to maintain, at least for a while
https://gist.github.com/mc4man/d4f183fd916515e6038654877070d3a1

@catharanthus
Copy link
Author

@mc4man

If xscrnsaver helps you just add back, easy revert to maintain, at least for a while
https://gist.github.com/mc4man/d4f183fd916515e6038654877070d3a1

Thanks. For the time being, I already reverted to the last release (0.26.0) which also works fine. I just hope that the problem will be eventually dealt with somehow.

@kkkrackpot
Copy link
Contributor

Similar problem here.
mpv 0.26.0-151-gc6628a5fb-dirty with no DE.
It's funny, but now my screen goes black after 10 minutes or so. I can bring the picture back only by pressing a key. Watching long movies is now kinda annoying...
mc4man's patch fixes the issue.
Seems like this scrnsaver part is especially helpful with no screensaver installed.

@CounterPillow
Copy link
Contributor

@fhlfibh try installing xdg-screensaver.

@kkkrackpot
Copy link
Contributor

kkkrackpot commented Aug 13, 2017

@CounterPillow
Without the patch?
You mean xscreensaver or xdg-utils?

UPD.

These are the packages that would be merged, in order:

Calculating dependencies... done!
[ebuild  N     ] dev-libs/libatomic_ops-7.4.2::gentoo  113 KiB
[ebuild  N     ] x11-apps/xprop-1.2.2::gentoo  143 KiB
[ebuild  N     ] x11-apps/xset-1.2.3::gentoo  139 KiB
[ebuild  N     ] dev-libs/boehm-gc-7.4.2::gentoo  USE="cxx -static-libs -threads" 1058 KiB
[ebuild  N     ] www-client/w3m-0.5.3-r9::gentoo  USE="X ssl unicode -fbcon -gpm -gtk -imlib (-libressl) -lynxkeymap -nls -nntp -xface" L10N="-ja" 2127 KiB
[ebuild  N     ] virtual/w3m-0::gentoo  0 KiB
[ebuild   R    ] app-text/xmlto-0.0.26-r1::gentoo  USE="text* -latex" 0 KiB
[ebuild  N     ] x11-misc/xdg-utils-1.1.1-r1::gentoo  USE="-doc -perl" 289 KiB

Total: 8 packages (7 new, 1 reinstall), Size of downloads: 3866 KiB

7 packages only to let the monitor not to go black...
Really, I prefer mpv's old code and mc4man's patch much, much better.

@CounterPillow
Copy link
Contributor

@fhlfibh no, I mean xdg-utils

@CounterPillow
Copy link
Contributor

@fhlfibh well you go tell that to the chucklefucks that complained about the xscreensaver dependency then

@ghost
Copy link

ghost commented Aug 13, 2017

I have xdg-screensaver, but i was experiencing this problem until i did apply the mc4man's patch.

@CounterPillow
Copy link
Contributor

If you have xdg-screensaver and it doesn't work, then it's most likely the above-linked bug in xdg-screensaver.

If you do not have xdg-screensaver, then it's because mpv now solely relies on xdg-screensaver for this functionality, because of people like @teod0r in #4706

@kkkrackpot
Copy link
Contributor

kkkrackpot commented Aug 13, 2017

@CounterPillow actually, it was me #4404

But as far as I remember, first a) mpv had xscreensaver (and xinerama) configure options, but I never used them, and everything worked happily, nevertheless (https://bugs.gentoo.org/show_bug.cgi?id=617516).
Then b) these became hardcoded and configure options were gone. I pulled depended libs in, and it all worked well again.
Now it looks like I'm back in the position "a)" again, but this time it doesn't work for some reason...

Anyway, personally I have only a little idea about how all these screensavers work, I only noted an issue and that the patch fixes it.

@CounterPillow
Copy link
Contributor

@fhlfibh so you complained about the dependencies being there and wanted to disable them, and now you complain about them being gone?

Fuck this I'm out. I clearly cannot comprehend the enlightened intellect it takes to be a gentoo user.

@kkkrackpot
Copy link
Contributor

kkkrackpot commented Aug 13, 2017

@CounterPillow Being a gentoo user actually requires no intellect, only an ass to sit on. And I'm sitting on mine long enough.
Anyway, that's how it goes in my case, shortly:
First) xscreensaver option disabled, no deps -- it works
Then) xscreensaver hardcoded, deps pulled in -- it works
Now) xscreensaver stuff and its deps dropped -- it doesn't work.
In other words, I'm complaining not about deps being gone, but about that now it doesn't work without them as it once used to...
However, I don't believe that much can now be done about it except using the patch or pulling it's code back into mpv or using xgd-screensaver.

@mc4man
Copy link

mc4man commented Aug 13, 2017

All that 'patch' does is revert the commit behind this, i.e. the way it was.
What should happen is it should be reverted in master & any nonsense issues about the libxss dep (xscrnsaver.h) should be ignored or closed.
xscrnsaver.h is not xscreensaver, just a little X11 protocol, period.

@ghost
Copy link

ghost commented Aug 14, 2017

Well, after replacing the xdg-screensaver script with newer version, it works fine.

@kkkrackpot
Copy link
Contributor

@cranes-bill What exactly "newer version" do you mean?

Not sure if I should open a new issue or keep going here... Anyway:
After this commit 6694048 I again started having black screen after 10-15 minutes.

xdg-utils-1.1.2 doesn't help:

# cat mpv.log | grep -i screen
[  46.573][v][vo/opengl/x11] Disabling screensaver.
[  46.588][v][vo/opengl/x11] Updating screensaver failed (4). Make sure the xdg-screensaver script is installed.
[ 830.766][v][vo/opengl/x11] Enabling screensaver.

And the patch from above doesn't work anymore...

@mc4man
Copy link

mc4man commented Aug 15, 2017

Yeah, they took what worked fine for 99% of users & now screwed it up but good. If 6694048 means one has to find some new xdg script then it's even stupider than the 1st minor screwup.
Should of left well enough alone..

@kkkrackpot
Copy link
Contributor

kkkrackpot commented Aug 15, 2017

@mc4man Is is possible to make a new patch that replaces the recent commit with the old code?

Maybe the new xscrnsaver thing now works out Ok for someones, but for me personally all this screensaver story goes only from bad to worse, from its very beginning...

@catharanthus
Copy link
Author

I can confirm that 6694048 doesn't fix the issue for me.

@Hello71
Copy link
Contributor

Hello71 commented Aug 16, 2017

anyone affected by this, you must:

  1. install xdg-utils (obviously)
  2. if you are using KDE, apply this patch to /usr/bin/xdg-screensaver to workaround https://bugs.kde.org/show_bug.cgi?id=383575:
--- a/xdg-screensaver
+++ b/xdg-screensaver
@@ -681,7 +681,7 @@
   cleanup_suspend
 }
 
-XPROP=`which xprop 2> /dev/null`
+XPROP=
 
 check_window_id()
 {
@@ -752,12 +752,12 @@
     case "$1" in
         suspend)
         dbus-send --session \
-                  --dest=org.freedesktop.ScreenSaver \
+                  --dest=org.freedesktop.PowerManagement.Inhibit \
                   --type=method_call \
                   --print-reply \
                   --reply-timeout=2000 \
-                  /ScreenSaver \
-                  org.freedesktop.ScreenSaver.Inhibit \
+                  /org/freedesktop/PowerManagement/Inhibit \
+                  org.freedesktop.PowerManagement.Inhibit.Inhibit \
                   string:$window_id \
                   string:xdg-screensaver \
                   | grep uint32 | cut -d ' ' -f 5 >| "$screensaver_file.cookie" \
@@ -769,10 +769,10 @@
         if [ -f "$screensaver_file.cookie" ] ; then
             value=`cat "$screensaver_file.cookie"`
             dbus-send --session \
-                      --dest=org.freedesktop.ScreenSaver \
+                      --dest=org.freedesktop.PowerManagement.Inhibit \
                       --type=method_call \
-                      /ScreenSaver \
-                      org.freedesktop.ScreenSaver.UnInhibit \
+                      /org/freedesktop/PowerManagement/Inhibit \
+                      org.freedesktop.PowerManagement.Inhibit.UnInhibit \
                       uint32:$value \
                       2> /dev/null
             rm -f "$screensaver_file.cookie"
@@ -801,17 +801,12 @@
         ;;
 
         reset)
-        if [ -f "$screensaver_file.cookie" ] ; then
-            value=`cat "$screensaver_file.cookie"`
-            dbus-send --session \
-                      --dest=org.freedesktop.ScreenSaver \
-                      --type=method_call \
-                      /ScreenSaver \
-                      org.freedesktop.ScreenSaver.UnInhibit \
-                      uint32:$value \
-                      2> /dev/null
-            rm -f "$screensaver_file.cookie"
-        fi
+        dbus-send --session \
+                  --dest=org.freedesktop.ScreenSaver \
+                  --type=method_call \
+                  /ScreenSaver \
+                  org.freedesktop.ScreenSaver.SimulateUserActivity \
+                 2> /dev/null
         result=$?
         ;;

@Hello71
Copy link
Contributor

Hello71 commented Aug 16, 2017

fwiw, for comparison, since mpv devs apparently like looking to vlc for guidance, the situation there is that after much bikeshedding, dbus was finally implemented as the least worst solution: https://trac.videolan.org/vlc/ticket/4739, https://trac.videolan.org/vlc/ticket/7824, https://github.com/videolan/vlc/blob/master/modules/misc/inhibit/dbus.c

@avih
Copy link
Member

avih commented Aug 16, 2017

since mpv devs apparently like ...

There's no need for sarcasm, and it won't help your cause. You do realize the changes are not made to annoy users, right? and they can be improved or reverted over time, based on feedback.

The information you provide is good enough.

@mc4man
Copy link

mc4man commented Aug 16, 2017 via email

@ghost
Copy link

ghost commented Aug 16, 2017

@fhlfibh commented 10 hours ago:

After this commit 6694048 I again started having black screen after 10-15 minutes.

@catharanthus commented 10 hours ago:

I can confirm that 6694048 doesn't fix the issue for me.

The same to me!

CounterPillow referenced this issue Aug 16, 2017
If it doesn't work this time, I'll remove all X11 screensaver code.

Fixes #4763.
@ekce
Copy link

ekce commented Aug 19, 2017

This is now broken in XFCE as well, I double checked that I have xdg-utils installed.

The logs with the latest mpv (git) show:
[ 0.206][v][vo/opengl/x11] Updating screensaver failed (4). Make sure the xdg-screensaver script is installed.

However, opening mpv and then separately calling xdg-screensaver suspend manually with the Window ID seems to work. In other words, first start playing the file in mpv, then in another terminal run something like the following (replacing movie.mkv with the actual file name).

$ XDG_UTILS_DEBUG_LEVEL=2 xdg-screensaver suspend $(xwininfo -name "movie.mkv - mpv" | grep xwininfo | awk '{print $4}')

In my case, in XFCE, I get output resembling:

mv -T available
Running /usr/bin/xprop -id 0x4200002

Note: While the error still appears in the logs, it does stop the display from blanking out while the video is playing. I don't know why the current mpv implementation doesn't work for XFCE but hopefully this is helpful towards finding a solution.

@voltmtr
Copy link

voltmtr commented Aug 19, 2017

Broken in LXDE too, xdg-tools is installed. Can you just revert these screensaver related commits to how it was? It is useless trying to fix this if Gnome and KDE guys can't agree upon a simple protocol, at least legacy protocol worked

@PranavBhattarai
Copy link

Sadly I am having the same behavior "screen lock" behavior with MPV 0.32.0 in Ubuntu 20.04.1 with GNOME 3.36.3 as reported here:
https://bugs.launchpad.net/ubuntu/+source/mpv/+bug/1895941

Should I create a new issue!? @LaserEyess

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

No branches or pull requests

9 participants