vdr shutdown handling / streamdev plugin
Commit Message
Udo Richter wrote:
> Klaus Schmidinger wrote:
>> Are you sure?
>> ACTIVITYTIMEOUT is 60 seconds, so wouldn't your change just
>> avoid the calls until 60 seconds after the last user activity?
>
> Hmm, you're right. Its not a test on Setup.MinUserInactivity * 60. My
> patch would just avoid these calls the usual 60 seconds after a key press.
>
> However, I still think that Active() should be called only if VDR is
> really willing to shut down, and not while running normally. Or is
> Active() supposed to also delay any housekeeping tasks?
Since cPlugin::Active() is only intended to be a means of preventing
VDR from shutting down, I'd say you're right - it should only be called
when VDR is actually trying to shut down.
The attached patch makes it call cPluginManager::Active() only
if it really wants to shut down, and if there is actually an option
for plugins to delay the shutdown (i.e. this is not a forced shutdown).
If cPluginManager::Active() returns 'true' once, it waits for SHUTDOWNRETRY
minutes before trying again.
Please give this a try and let me know whether it works.
Klaus
Comments
Klaus Schmidinger wrote:
> ...
> --- vdr.c 2006/10/14 10:01:32 1.280
> +++ vdr.c 2006/12/02 10:20:30
> @@ -1154,11 +1154,15 @@
> ...
> + LastActivity = time(NULL) - Setup.MinUserInactivity * 60 + SHUTDOWNRETRY; // try again later
Just in case somebody noticed: I just realized that 'Now' can be used
instead of 'time(NULL)' here.
Klaus
Klaus Schmidinger wrote:
> The attached patch makes it call cPluginManager::Active() only
> if it really wants to shut down, and if there is actually an option
> for plugins to delay the shutdown (i.e. this is not a forced shutdown).
>
> If cPluginManager::Active() returns 'true' once, it waits for SHUTDOWNRETRY
> minutes before trying again.
I'd suggest moving the if block a bit down in the code, at least behind
the manual start check, so plugins cannot interfere with this. I think
it should be ok to move it down right before the "ForceShutdown =
false;" line - thats the point where the "Press any key to cancel
shutdown" will be asked for sure.
SHUTDOWNRETRY defaults to 5 minutes, thats ok, though I would go for
something around 1 minute.
Cheers,
Udo
On Saturday 02 December 2006 16:09, Udo Richter wrote:
> Klaus Schmidinger wrote:
> > The attached patch makes it call cPluginManager::Active() only
> > if it really wants to shut down, and if there is actually an option
> > for plugins to delay the shutdown (i.e. this is not a forced shutdown).
> >
> > If cPluginManager::Active() returns 'true' once, it waits for
> > SHUTDOWNRETRY minutes before trying again.
>
> I'd suggest moving the if block a bit down in the code, at least behind
> the manual start check, so plugins cannot interfere with this. I think
To manual start check I have another idea. Perhaps vdr can get a parameter for
setting explicitly manual-start or start based on timer. Then the startskript
could check the time that was written into nvram/acpi/wherever and tell vdr
if start was manual or not.
Matthias
Udo Richter wrote:
> Klaus Schmidinger wrote:
>> The attached patch makes it call cPluginManager::Active() only
>> if it really wants to shut down, and if there is actually an option
>> for plugins to delay the shutdown (i.e. this is not a forced shutdown).
>>
>> If cPluginManager::Active() returns 'true' once, it waits for
>> SHUTDOWNRETRY
>> minutes before trying again.
>
> I'd suggest moving the if block a bit down in the code, at least behind
> the manual start check, so plugins cannot interfere with this. I think
> it should be ok to move it down right before the "ForceShutdown =
> false;" line - thats the point where the "Press any key to cancel
> shutdown" will be asked for sure.
I agree to move it down until before the line
if (timer && Delta < Setup.MinEventTimeout * 60 && ForceShutdown) {
in order to not interfere with the "assuming manual start of VDR" stuff.
But if we move it further down, the dsyslog("reboot at %s", *TimeToString(Next))
will be executed, even if the plugin's activity prevents that from
happening.
> SHUTDOWNRETRY defaults to 5 minutes, thats ok, though I would go for
> something around 1 minute.
Well, this is only important if VDR shuts down by itself, unattended.
So I guess it dosn't really matter if it stays up a little longer ;-)
Klaus
Klaus Schmidinger wrote:
> I agree to move it down until before the line
>
> if (timer && Delta < Setup.MinEventTimeout * 60 && ForceShutdown) {
>
> in order to not interfere with the "assuming manual start of VDR" stuff.
> But if we move it further down, the dsyslog("reboot at %s",
> *TimeToString(Next))
> will be executed, even if the plugin's activity prevents that from
> happening.
No, I thought of that too. This code part explicitly requests "&&
ForceShutdown", while the new code requires "!ForceShutdown", so they
exclude each other.
Moving it down to before "ForceShutdown = false;" will also prevent
Active() from being called within the MinEventTimeout gap.
The whole shutdown code is an ugly mess, and could use a clean rewrite
in the 1.5.x cycle. I'll volunteer if that helps...
Cheers,
Udo
Udo Richter wrote:
> Klaus Schmidinger wrote:
>> I agree to move it down until before the line
>>
>> if (timer && Delta < Setup.MinEventTimeout * 60 && ForceShutdown) {
>>
>> in order to not interfere with the "assuming manual start of VDR" stuff.
>> But if we move it further down, the dsyslog("reboot at %s",
>> *TimeToString(Next))
>> will be executed, even if the plugin's activity prevents that from
>> happening.
>
> No, I thought of that too. This code part explicitly requests "&&
> ForceShutdown", while the new code requires "!ForceShutdown", so they
> exclude each other.
Oh, I must have missed that.
> Moving it down to before "ForceShutdown = false;" will also prevent
> Active() from being called within the MinEventTimeout gap.
Ok, I'll move it before the line
if (!Next || Delta > Setup.MinEventTimeout * 60 || ForceShutdown) {
then.
> The whole shutdown code is an ugly mess, and could use a clean rewrite
> in the 1.5.x cycle. I'll volunteer if that helps...
Sure, once the 1.5 cycle has been opened, I'll be glad to see
how you can do this cleaner ;-)
Klaus
@@ -1154,11 +1154,15 @@
Skins.Message(mtInfo, tr("Editing process finished"));
}
}
- if (!Interact && ((!cRecordControls::Active() && !cCutter::Active() && !cPluginManager::Active() && (!Interface->HasSVDRPConnection() || UserShutdown)) || ForceShutdown)) {
+ if (!Interact && ((!cRecordControls::Active() && !cCutter::Active() && (!Interface->HasSVDRPConnection() || UserShutdown)) || ForceShutdown)) {
time_t Now = time(NULL);
if (Now - LastActivity > ACTIVITYTIMEOUT) {
// Shutdown:
if (Shutdown && (Setup.MinUserInactivity || LastActivity == 1) && Now - LastActivity > Setup.MinUserInactivity * 60) {
+ if (!ForceShutdown && cPluginManager::Active()) {
+ LastActivity = time(NULL) - Setup.MinUserInactivity * 60 + SHUTDOWNRETRY; // try again later
+ continue;
+ }
cTimer *timer = Timers.GetNextActiveTimer();
time_t Next = timer ? timer->StartTime() : 0;
time_t Delta = timer ? Next - Now : 0;