Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#5893 closed defect (fixed)

keystroke handling for Lirc too expensive

Reported by: km@… Owned by: stuartm
Priority: minor Milestone: 0.22
Component: libmythui Version: head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

From mythmainwindow.cpp handling a Lirc keycode --- if (keycode)

{

GetMythUI()->ResetScreensaver?(); if (GetMythUI()->GetScreenIsAsleep?())

return;

--- ResetScreensaver? (in screensaver-x11.cpp) invokes

myth_system("gnome-screensaver-command --poke

So there is a fork/exec of gnome-screensaver-command on every Lirc key press. On a system with little free memory this can several seconds per keystroke.

The strategy should be changed. At very least there should be an option to disable screensaver checking.

Attachments (3)

mythtv-5893-throttle_screensaver_deactivate.patch (1.2 KB) - added by sphery <mtdean@…> 11 years ago.
fixes missing check for active GNOME screensaver and throttles screensaver deactivation
mythtv-5893-throttle_screensaver_deactivate-2.patch (1.3 KB) - added by sphery <mtdean@…> 11 years ago.
Updated patch--only does date math if a screensaver is actually running
mythtv-fixes-5893--throttle_screensaver_deactivate.patch (1.4 KB) - added by sphery <mtdean@…> 11 years ago.
Patch for -fixes

Download all attachments as: .zip

Change History (7)

Changed 11 years ago by sphery <mtdean@…>

fixes missing check for active GNOME screensaver and throttles screensaver deactivation

Changed 11 years ago by sphery <mtdean@…>

Updated patch--only does date math if a screensaver is actually running

comment:1 Changed 11 years ago by sphery <mtdean@…>

The attached patch, mythtv-5893-throttle_screensaver_deactivate-2.patch , fixes the missing check for whether GNOME screensaver is active and also throttles screensaver deactivation by only running the command at most once every 31 seconds (i.e. if it's been more than 30 seconds since the last time we ran the command to deactivate the screensaver).

I chose 30 seconds thinking it was a reasonable value as people are unlikely to ever set their screensaver to activate due to 30 seconds or less of idle time. In truth, I can't imagine anyone using an idle timeout of less than 1 minute, so increasing the value to 50 (or even 58?) seconds may make sense.

This throttling does mean that not every button press will reset the clock for the screensaver timeout, so from that perspective, using a value of 28 may make sense for an expected minimum screensaver timeout of 1 minute. In practical terms, throttling deactivation with a value of 30 for the time comparison simply means that the user may experience a screensaver timeout after a LIRC button press/joystick event that's up to 31 seconds shorter than expected--which seems reasonable.

As far as the "option to disable screensaver checking" goes, the patch allows this by only running the deactivate command if either xscreensaver or gnome-screensaver is actually running. Therefore, after the patch is applied, when not running xscreensaver and gnome-screensaver, Myth will not attempt to deactivate the screensaver.

The -2 version of the patch is the same as the first one I uploaded, but also adds a check for IsScreenSaverRunning?() before doing the date math so it's only performed if a screensaver is running.

comment:2 Changed 11 years ago by stuartm

Resolution: fixed
Status: newclosed

(In [19069]) Only deactivate the screen saver if it is running and rate limit it so that we don't execute the system commands on every single button press. Patch by Sphery. Closes #5893

comment:3 Changed 11 years ago by stuartm

Milestone: unknown0.22
Version: unknownhead

Changed 11 years ago by sphery <mtdean@…>

Patch for -fixes

comment:4 Changed 11 years ago by Anduin Withers

(In [19222]) Closes #5893

Merge [19069] from trunk.

Original commit message:

Only deactivate the screen saver if it is running and rate limit it so that we don't execute the system commands on every single button press. Patch by Sphery.

Thanks to Michael T. Dean for the -fixes patch.

Note: See TracTickets for help on using tickets.