Unreliable screen saver disactivation

[nicolas.george@…]

The current X11 screen saver handling is unreliable, due to its save/restore scheme:

• If mplayer crashes unexpectedly (such things happens, especially with non-free

codecs :( ), the screen saver settings are not restored.

• If one leaves a paused video somewhere on a hidden virtual desktop, the screen

saver stays disactivated.

• If a video ends while none touched the keyboard for some time (which is quite

normal, while watching a video), the screen saver is restored and the monitor
may go directly to suspend mode.

I think that a better way to prevent the screen saver from interrupting a video
is to notify the X11 server, at each new frame, that the user is probably
looking at its screen. X11 does not have an extension to do that, as far as I
know, but it is possible to use XTest to do that by sending a fake key press
event of some harmless key (shift, for example).

I have written a patch for that. I put it on <URL:
​http://www.eleves.ens.fr/home/george/info/prg/mplayer-20050304-noidle.diff >
(does not Bugzilla allow to upload patches?). It adds the -xtest-noidle
<keycode> option to enable this scheme. It only covers x11, xv and gl driver,
but extending the relevant drivers should be straightforward. I do not
understand the autoconf/configure tools, so I left out that part: the HAVE_XTEST
must be added to config.h, and -lXtst to the linked libraries.

[nicolas.george@…]

-xtest-noidle option

[nicolas.george@…]

I knew Bugzilla allowed to upload patches.

[reimar]

This has been discussed afaik, and the result was that there is no "harmless"
keypress. E.g. shift is often used for accessibility features, control is
sometimes used for something like this too sometimes etc.
To tell the truth, the only useful way at the moment seem to be: just don't
install xscreensaver :-(
Concerning your patch: Adding a function call to all vos that use x11 seems
unacceptable to me, especially in the time-critical flip_page function. Also you
missed at least the gl2, xmga and xvidix vos.

[nicolas.george@…]

(In reply to comment #2)

This has been discussed afaik, and the result was that there is no "harmless"
keypress. E.g. shift is often used for accessibility features, control is
sometimes used for something like this too sometimes etc.

You are right, there is no universally harmless keypress. But for any given
configuration, there is probably at least one. The patch I propose requires that
the user choose a keycode for the event, and if he does not, the old
disable/enable scheme is used.

For example, I know that shift and control are quite harmless on my config (they
may cause me to type the wrong key, but I rarely type on my keyboard while
watching a video). I could also use a keycode that does not correspond to any
key and have no attached keysym.

Concerning your patch: Adding a function call to all vos that use x11 seems
unacceptable to me, especially in the time-critical flip_page function.

My reasonning was: flip_page calls very expensive functions (a whole frame copy
to the server), one more function call, which most of the time only decrements a
counter, would be negligible.

But I had another idea, which I think is cleaner: let us have a global "void
(*noidle_function)(void) = NULL;", and in the main loop, in mplayer.c, near
print_status at line 2575, "if(noidle_function != NULL) noidle_function();" (I
have checked, testing for NULL is much faster than calling a dummy function).

At this point, there is enough time for output to the terminal, there should be
enough time for this function. Furthermore, the counter to send the fake
keypress only once every n frames (30 in my proposition) could be inlined there,
causing even less overhead.

Also you

missed at least the gl2, xmga and xvidix vos.

I know, I grepped on #include "x11_common.h". But I do not have these drivers
(or maybe I could have them, but I do not want to try, the support for my video
card is not really stable), so I could not test.

It may be noted that my new proposal: a noidle function pointer in the main loop
near print_status, would cover all available drivers automatically, and could
even be extended to non-X11 drivers if necessary.

[reimar]

(In reply to comment #3)

You are right, there is no universally harmless keypress. But for any given
configuration, there is probably at least one. The patch I propose requires that
the user choose a keycode for the event, and if he does not, the old
disable/enable scheme is used.

I would prefer if the screensaver developers would come up with something useful
(if they haven't already and I just don't know it).

Concerning your patch: Adding a function call to all vos that use x11 seems
unacceptable to me, especially in the time-critical flip_page function.

My reasonning was: flip_page calls very expensive functions (a whole frame copy
to the server), one more function call, which most of the time only decrements a
counter, would be negligible.

At least for vo_gl the upload is done in draw_frame, not in flip_page. And any
delay in flip_page probably affect A-V sync.

But I had another idea, which I think is cleaner: let us have a global "void
(*noidle_function)(void) = NULL;", and in the main loop, in mplayer.c, near
print_status at line 2575, "if(noidle_function != NULL) noidle_function();" (I
have checked, testing for NULL is much faster than calling a dummy function).

Actually I think there is already a function like this, xscreensaver_heartbeat,
and it actually should do what you want to achieve by sending X events to the
xscreensaver - if it doesn't work right it's maybe even a bug in xscreensaver?

[yuri@…]

On my FreeBSD 7.0 screensaver kicks in when movie is playing in mplayr.
Does this also fall under this PR or I better file a new one?

[reimar]

The screensaver stuff was completely rewritten. If your screensaver does not support the official X APIs (XScreenSaverReset and XScreenSaverSuspend) - which you recognize by the fact that MPlayer does not deactivate it out-of-the-box - you have to use the -heartbeat-cmd option.

[reimar]

Reopen if the new code is still has problems (excluding the fact that using -heartbeat-cmd may be inconvenient - complain to the screensaver authors for not providing a common, clean, lightweight API to disable them).