Opened 7 years ago

Closed 7 years ago

Last modified 6 years ago

#12972 closed Patch - Bug Fix (Won't Fix)

Frontend idle timeout cannot be fully disabled

Reported by: amb@… Owned by: Peter Bennett
Priority: minor Milestone: 29.0
Component: MythTV - General Version: 0.28.0
Severity: low Keywords:
Cc: Ticket locked: no

Description

The idle timeout feature of mythfrontend should be disabled by setting FrontendIdleTimeout? to zero. There is one place where the idle screen can be displayed that does not check this value - in the automatic startup.

Attachments (1)

idle-timeout.diff (607 bytes) - added by amb@… 7 years ago.
Patch

Download all attachments as: .zip

Change History (22)

Changed 7 years ago by amb@…

Attachment: idle-timeout.diff added

Patch

comment:1 Changed 7 years ago by Stuart Auchterlonie

Milestone: unknown29.0

comment:2 Changed 7 years ago by Peter Bennett

I do not think this patch is valid.

Say a setup has a backend and a frontend on the same machine and the frontend automatically starts when the machine starts up. If there is no idle timeout set on the frontend, and the machine starts up to make a recording, the machine will never shut down again. There is code in the frontend to detect this situation and set the frontend into idle so that the machine can shut down after the recording is complete.

The patch will defeat this mechanism. The frontend logic to cater for this situation becomes irrelevant and will not achieve anything.

comment:3 Changed 7 years ago by amb@…

If I set the FrontendIdleTimeout? setting to zero I want it never to go to standby. If I want it to go to standby I set the FrontendIdleTimeout? setting to non-zero.

This is what the documentation string in the setup says:

"Number of minutes to wait when the frontend is idle before entering standby mode. Standby mode allows the backend to power down if configured to do so. Any remote or mouse input will cause the countdown to start again and/or exit idle mode. Video playback suspends the countdown. A value of zero prevents the frontend automatically entering standby."

Quite clearly it should never go to standby if the setting is zero - but it does.

For users of MythWelcome? the Mythfrontend standby screen is unwanted and setting FrontendIdleTimeout? to zero should fix it but it doesn't without the patch.

For your case where there is no MythWelcome? and Mythfrontend is started at boot then choose a non-zero value for FrontendIdleTimeout? and all will be well - even with the patch.

comment:4 Changed 7 years ago by Peter Bennett

This change could affect people who have systems with an automatic startup of the frontend, and don't want the system to stay running 24x7 after it starts up for a recording. As you say, they can all go and change their timeout interval, but before we force all those people to make a change, I would like to understand why this is important to you.

Please can you explain how this impacts you. I cannot understand when you will hit this issue. Are you using mythwelcome? Under what circumstances do you unexpectedly find you are in standby? I assume you do not run your frontend and backend 24x7.

comment:5 Changed 7 years ago by amb@…

  1. The code does not operate as the documentation string say so it is a bug (of one sort or another).
  1. The default value of this configuration setting is non-zero - it has to be explicitly set to zero by the user.
  1. There are three places in the code where FrontendIdleTimeout? is used
    1. One is in the function MythMainWindow::MythMainWindow?() where it starts a timer to go to standby after an idle time only if FrontendIdleTimeout?>0.
    2. One is in the function MythMainWindow::customEvent() where the idle timeout is restarted when the configuration item is changed but only if FrontendIdleTimeout?>0.
    3. The third one is the one that I patched where the system will go to standby on automatic startup whether the FrontendIdleTimeout? is zero or not.
  1. If a user has set the configuration value to zero then their system will only enter standby if mythfrontend is started just before a recording is scheduled to start. It will not enter standby if they leave it idle for hours because the timer that checks for idleness is never started.
  1. Yes, I am using Mythwelcome and my system doesn't run 24x7. The problem is triggered if I start the frontend from mythwelcome just before a recording is scheduled to start when it goes to standby and I have to press another key to exit standby.
  1. There is nobody who could be disadvantaged by this change.
    1. Without the change
      1. If they have set the configuration value to nono-zero then they will go to standby.
      2. If they have set the configuration value to zero then they will not go to standby except if they start mythfrontend just before a recording
    2. With the change
      1. If they have set the configuration value to non-zero then they will go to standby.
      2. If they have set the configuration value to zero then they will not go to standby ever.

comment:6 Changed 7 years ago by Peter Bennett

My point is -

  • If users have left idle timeout as zero, which is the default setting on an install.
  • AND If the users have auto login and auto startup of the frontend when the machine boots, which I believe is an option in the setup of MythBuntu.

Then when the machine starts up for a recording -

  • Without the change, the frontend goes into idle immediately, and the machine can shut down after the recording is complete.
  • With the change, the frontend will never go into idle, and the machine will not shut down after the recording, so it will stay running until somebody takes manual action.

So I suspect that if we make the suggested change there may be users who start to complain that their machine does not shut down after making a recording.

comment:7 Changed 7 years ago by Roger Siddons

P Bennett is correct. This is the wrong fix.

6.b.i If they have set the configuration value to non-zero then they will go to standby.

Settings aren't created in the database until they're modified, so a default installation would never go to standby. This could be rectified by:

GetNumSetting("FrontendIdleTimeout", 999) > 0

However that isn't appropriate either.

Standby mode is used for 2 separate requirements;

  1. A frontend that has started (ie. not in standby mode) will go into standby when left idle. This is what the setting/documentation is referring to.
  2. A BE/FE that starts automatically (without MythWelcome) must go into standby so that it will shutdown. The idle timeout isn't relevant here. The doc refers to the setting/idle function, not Standby mode.

A user may set idle timeout to 0 to prevent Myth ever shutting down once they have started it. But they still need it to shutdown after automatic starts though.

The problem is triggered if I start the frontend from mythwelcome just before a recording is scheduled to start when it goes to standby and I have to press another key to exit standby.

This is the real issue. In fact it's worse than that because WasAutomaticStart uses *within* rather than *before*: restarting the FE up to 15 mins after a recording has started will also put it into standby IIRC.

Ideally WasAutomaticStart could be improved for everyone. But for this specific case a better fix may be for it to return false if MythWelcome is running (as in BackendIsRunning)

Out of curiosity, why do you still use MythWelcome ? Standby is its preferred replacement, although there are some features that haven't been ported (yet). Are you using those ?

comment:8 Changed 7 years ago by amb@…

The point rsiddons makes about the database entry not existing until created is a good point and one that I will admit having forgotten. However where it is used in MythMainWindow? the code does assume a non-zero default (90 minutes in this case) so not being set is equivalent to being set non-zero:

    d->idleTime = gCoreContext->GetNumSetting("FrontendIdleTimeout",
                                              STANDBY_TIMEOUT);

Using the same default in my patch would make it equivalent to the other uses of this configuration setting which makes it consistent with its documentation string.

The problem with the patch, which hasn't been fully explained above, is that the backend also has an automatic startup test. If the backend was started automatically and the frontend hasn't connected then it will be allowed to go to sleep after the recording even if the idle timeout is zero. The patch will not make the frontend go to standby so it will connect to the backend and cancel the idle timeout for the special case.

This problem only exists if the backend's idle timeout is set to zero. If this is not the case then when the machine is started so that a recording can be performed the frontend will go to standby after 90 minutes (or the value of FrontendIdleTimeout? if set) anyway and the backend will allow shutdown after its own timeout.

We could fix the patch for this special case by checking for the backend's idle timeout as well, like this:

FrontendIdleTimeout? idleTimeoutSecs mythfrontend action explanation
set to zero not set or set to zero go to standby Current behaviour
set to zero set to non-zero normal start up User's explicit choice to set FrontendIdleTimeout? to zero, requires patch
not set or set to non-zero not set or set to zero go to standby Current behaviour
not set or set to non-zero set to non-zero go to standby Current behaviour

I think that this works for everybody.

comment:9 in reply to:  8 ; Changed 7 years ago by Peter Bennett

Replying to amb@…:

The point rsiddons makes about the database entry not existing until created is a good point and one that I will admit having forgotten. However where it is used in MythMainWindow? the code does assume a non-zero default (90 minutes in this case) so not being set is equivalent to being set non-zero:

    d->idleTime = gCoreContext->GetNumSetting("FrontendIdleTimeout",
                                              STANDBY_TIMEOUT);

Using the same default in my patch would make it equivalent to the other uses of this configuration setting which makes it consistent with its documentation string.

I think this is a reasonable solution.

The problem with the patch, which hasn't been fully explained above, is that the backend also has an automatic startup test. If the backend was started automatically and the frontend hasn't connected then it will be allowed to go to sleep after the recording even if the idle timeout is zero. The patch will not make the frontend go to standby so it will connect to the backend and cancel the idle timeout for the special case.

I am not with you here - if the backend idle timeout is zero then it never goes to sleep, it is on 24x7.

This problem only exists if the backend's idle timeout is set to zero. If this is not the case then when the machine is started so that a recording can be performed the frontend will go to standby after 90 minutes (or the value of FrontendIdleTimeout? if set) anyway and the backend will allow shutdown after its own timeout.

As I said above, if the backend idle timeout is zero it never goes to sleep.

We could fix the patch for this special case by checking for the backend's idle timeout as well, like this:

FrontendIdleTimeout? idleTimeoutSecs mythfrontend action explanation
set to zero not set or set to zero go to standby Current behaviour
set to zero set to non-zero normal start up User's explicit choice to set FrontendIdleTimeout? to zero, requires patch
not set or set to non-zero not set or set to zero go to standby Current behaviour
not set or set to non-zero set to non-zero go to standby Current behaviour

I think that this works for everybody.

I don't think that this is needed, I think all that is needed it to change the default in your original patch so that it uses STANDBY_TIMEOUT as the default if frontend idle timeout does not exist.

comment:10 Changed 7 years ago by Peter Bennett

Owner: set to Peter Bennett
Status: newassigned

comment:11 in reply to:  9 ; Changed 7 years ago by Roger Siddons

Replying to pbennett:

I don't think that this is needed, I think all that is needed it to change the default in your original patch so that it uses STANDBY_TIMEOUT as the default if frontend idle timeout does not exist.

STANDBY_TIMEOUT is a macro defined in a different unit so it would have to be duplicated. Actually the default value is irrelevant - the setting is only being used as a "Allow Standby on startup" flag so why not use the traditional 1 ?

Users that currently set it to 0 because they never want the FE to time-out whilst in use would be forced to start using the max timeout to retain the ability of shutdown after automatic recordings. Or go back to using MythWelcome. So the setting documentation https://www.mythtv.org/wiki/Configuring_Frontend#Shutdown.2FReboot_Settings requires changing as well...

I agree there's a (minor) issue of sometimes incorrectly starting up in Standby when invoked by MythWelcome. Its purpose is to prevent the FE starting up during automatic recordings. So a FE starting under MythWelcome is always a manual start and WasAutomaticStart is redundant in that case. IMO an explicit check is simple, clear and can easily be removed if/when MythWelcome is retired.

I'm also confused by the other comments. The OP needs to clarify:

  1. what other issues they are experiencing
  2. what they are trying to achieve
  3. why they are trying to use (abuse?) the "FrontendIdleTimeout" setting to control startup behaviour.

comment:12 in reply to:  11 ; Changed 7 years ago by Peter Bennett

Replying to rsiddons:

STANDBY_TIMEOUT is a macro defined in a different unit so it would have to be duplicated. Actually the default value is irrelevant - the setting is only being used as a "Allow Standby on startup" flag so why not use the traditional 1 ?

I agree.

Users that currently set it to 0 because they never want the FE to time-out whilst in use would be forced to start using the max timeout to retain the ability of shutdown after automatic recordings. Or go back to using MythWelcome. So the setting documentation https://www.mythtv.org/wiki/Configuring_Frontend#Shutdown.2FReboot_Settings requires changing as well...

This is a valid point and is the point I was trying to make from the beginning. After I discovered that the default is actually 90, I thought that most users may have just left it as 90 and so would not be affected. That may not be true.

comment:13 in reply to:  11 ; Changed 7 years ago by amb@…

Replying to rsiddons:

Replying to pbennett:

I agree there's a (minor) issue of sometimes incorrectly starting up in Standby when invoked by MythWelcome. Its purpose is to prevent the FE starting up during automatic recordings. So a FE starting under MythWelcome is always a manual start and WasAutomaticStart is redundant in that case. IMO an explicit check is simple, clear and can easily be removed if/when MythWelcome is retired.

Provided that starting MythFrontEnd by any method at the "wrong time" to trigger the automatic startup test doesn't automatically go to standby I don't care how it is done. Checking whether it was started from MythWelcome is not sufficient because if mythbackend is left always running and mythfrontend is started manually at the "wrong time" it will go to standby.

I'm also confused by the other comments. The OP needs to clarify:

  1. what other issues they are experiencing

The only issue I have is the frontend going to standby when started even though the FrontendIdleTimeout has been deliberately set to zero (which the documentation string tells me prevents it ever going to standby).

  1. what they are trying to achieve

As for 1 above.

  1. why they are trying to use (abuse?) the "FrontendIdleTimeout" setting to control startup behaviour.

The FrontendIdleTimeout controls when the frontend goes to standby, this is it's only purpose. Therefore every possible place in the software that could go to standby must check this setting otherwise the frontend could go to standby when forbidden by the setting.

comment:14 in reply to:  9 ; Changed 7 years ago by amb@…

Replying to pbennett:

Replying to amb@…:

The problem with the patch, which hasn't been fully explained above, is that the backend also has an automatic startup test. If the backend was started automatically and the frontend hasn't connected then it will be allowed to go to sleep after the recording even if the idle timeout is zero. The patch will not make the frontend go to standby so it will connect to the backend and cancel the idle timeout for the special case.

I am not with you here - if the backend idle timeout is zero then it never goes to sleep, it is on 24x7.

This problem only exists if the backend's idle timeout is set to zero. If this is not the case then when the machine is started so that a recording can be performed the frontend will go to standby after 90 minutes (or the value of FrontendIdleTimeout if set) anyway and the backend will allow shutdown after its own timeout.

As I said above, if the backend idle timeout is zero it never goes to sleep.

I thought that if the backend idle timeout is zero and it is woken by a recording starting then it will record and then sleep again. Everything I wrote about the backend idle timeout was based on this premise.

If I'm wrong then I apologise for the confusion that my comments on this setting created.

comment:15 in reply to:  14 Changed 7 years ago by Peter Bennett

Replying to amb@…:

I thought that if the backend idle timeout is zero and it is woken by a recording starting then it will record and then sleep again. Everything I wrote about the backend idle timeout was based on this premise.

No - if the backend idle timeout is zero the backend will never shut down, and consequently the concept of it being woken for a recording does not apply. Some people leave their backend running 24x7.

If I'm wrong then I apologise for the confusion that my comments on this setting created.

comment:16 in reply to:  13 ; Changed 7 years ago by Roger Siddons

Replying to amb@…:

The FrontendIdleTimeout controls when the frontend goes to standby, this is it's only purpose. Therefore every possible place in the software that could go to standby must check this setting otherwise the frontend could go to standby when forbidden by the setting.

The only issue I have is the frontend going to standby when started even though the FrontendIdleTimeout has been deliberately set to zero (which the documentation string tells me prevents it ever going to standby).

I disagree. Its only role is to time out the FE & shut down the system when its been left idle after use. The setup 'documentation' is brief because it has limited space. The wiki documentation (comment:12) "There is one special case where the frontend will enter standy if this is zero..." is wrong (probably changed as part of this discussion). The code has never used the idle timeout when determining its start-up state. When automatically started it goes into standby; otherwise it does full startup. So the issue lies with WasAutomaticStart. It can be improved, but ultimately it's hard for an application to know why it's been started without external (OS-specific) hints.

Maybe we should change the title/description to "FE sometimes goes into Standby when manually started".

comment:17 in reply to:  12 Changed 7 years ago by Roger Siddons

Replying to pbennett:

After I discovered that the default is actually 90, I thought that most users may have just left it as 90 and so would not be affected. That may not be true.

It's probably true in 29. I suspect the settings behaviour has changed. See #13046 (comment:32)

comment:18 in reply to:  16 Changed 7 years ago by Peter Bennett

Replying to rsiddons:

The wiki documentation (comment:12) "There is one special case where the frontend will enter standy if this is zero..." is wrong (probably changed as part of this discussion).

I have removed the wrong comment

comment:19 in reply to:  16 Changed 7 years ago by amb@…

Replying to rsiddons:

Replying to amb@…:

So the issue lies with WasAutomaticStart. It can be improved, but ultimately it's hard for an application to know why it's been started without external (OS-specific) hints.

There is one entity that knows whether it was started for a recording - the backend (it already has code to decide this). The frontend could ask the backend if it has just automatically started and this would be correct more often but perhaps not always.

Maybe we should change the title/description to "FE sometimes goes into Standby when manually started".

I would be happy with that, it is a better description.

comment:20 Changed 7 years ago by Peter Bennett

Resolution: Won't Fix
Status: assignedclosed

comment:21 Changed 6 years ago by Peter Bennett

Owner: changed from Peter Bennett to Peter Bennett
Note: See TracTickets for help on using tickets.