vdr shutdown handling / streamdev plugin

  Jörg Wendel wrote:
> a question about the 'new' vdr shutdown handling implemented a few versions ago.
> [..]
> But why vdr call this so often, isn't it only required if the vdr is going to shutdown?
> Even without the log message, at first view it looks like unnecessary load?

VDR calls this function together with several other activity checks in 
its main loop, and not just when no user activity was detected for the 
configured time.
This call is done on each cycle of the main loop (typically once a 
second, sometimes more often) unless there's an open OSD, a recording, 
or a cutting in progress.

If this call is used as a simple boolean check, the load is very small. 
Translating a string on each call is more serious. And dumping to syslog 
should definitely be avoided.

For plugin developers, I suggest to keep it simple in there. Its 
probably a good idea to tr() the string just once and cache it afterwards.

For VDR, the two if's in the inactivity shutdown should be swappable 
with no serious side effects, see attached diff. All the calls do noting 
important, except the cCutter::Active() call, and this one is called 
often enough in other situations. But even with this patch, an 
non-interactive idle VDR waiting for shutdown will call this very 
frequently.

Cheers,

Udo
  

Hi,

thanks, the loop looks much better for me, i haven't tested it yet, i will try 
it at weekend.

> For plugin developers, I suggest to keep it simple in there. Its
> probably a good idea to tr() the string just once and cache it afterwards.

this would be a nice change in the streamdev plugin.

Greets
Jörg

On Tuesday 14 November 2006 19:26, Udo Richter wrote:
> Jörg Wendel wrote:
> > a question about the 'new' vdr shutdown handling implemented a few
> > versions ago. [..]
> > But why vdr call this so often, isn't it only required if the vdr is
> > going to shutdown? Even without the log message, at first view it looks
> > like unnecessary load?
>
> VDR calls this function together with several other activity checks in
> its main loop, and not just when no user activity was detected for the
> configured time.
> This call is done on each cycle of the main loop (typically once a
> second, sometimes more often) unless there's an open OSD, a recording,
> or a cutting in progress.
>
> If this call is used as a simple boolean check, the load is very small.
> Translating a string on each call is more serious. And dumping to syslog
> should definitely be avoided.
>
> For plugin developers, I suggest to keep it simple in there. Its
> probably a good idea to tr() the string just once and cache it afterwards.
>
> For VDR, the two if's in the inactivity shutdown should be swappable
> with no serious side effects, see attached diff. All the calls do noting
> important, except the cCutter::Active() call, and this one is called
> often enough in other situations. But even with this patch, an
> non-interactive idle VDR waiting for shutdown will call this very
> frequently.
>
> Cheers,
>
> Udo
  

On Wed, 15 Nov 2006 07:35:47 +0100, Jörg Wendel wrote
> > For plugin developers, I suggest to keep it simple in there. Its
> > probably a good idea to tr() the string just once and cache it afterwards.
> 
> this would be a nice change in the streamdev plugin.

Filed it in the streamdev bugtracker.

Concerning your syslog problem: There was a time when streamdev-cvs was not
maintained at all. A few patches have been around to get it working with VDR
1.4 . Your streamdev used one of these patches (missing the german
translation). Consider updating to a recent CVS version. It contains the
translation and in the meantime some more bugfixes have been applied.

Cheers,
Frank
  

> Filed it in the streamdev bugtracker.

thanks, till then i update to the recent CVS version

> Concerning your syslog problem: There was a time when streamdev-cvs was not
> maintained at all. A few patches have been around to get it working with
> VDR 1.4 . Your streamdev used one of these patches (missing the german
> translation). Consider updating to a recent CVS version. It contains the
> translation and in the meantime some more bugfixes have been applied.

Jörg
  

Udo Richter wrote:
> Jörg Wendel wrote:
>> a question about the 'new' vdr shutdown handling implemented a few 
>> versions ago.
>> [..]
>> But why vdr call this so often, isn't it only required if the vdr is 
>> going to shutdown?
>> Even without the log message, at first view it looks like unnecessary 
>> load?
> 
> VDR calls this function together with several other activity checks in 
> its main loop, and not just when no user activity was detected for the 
> configured time.
> This call is done on each cycle of the main loop (typically once a 
> second, sometimes more often) unless there's an open OSD, a recording, 
> or a cutting in progress.
> 
> If this call is used as a simple boolean check, the load is very small. 
> Translating a string on each call is more serious. And dumping to syslog 
> should definitely be avoided.
> 
> For plugin developers, I suggest to keep it simple in there. Its 
> probably a good idea to tr() the string just once and cache it afterwards.
> 
> For VDR, the two if's in the inactivity shutdown should be swappable 
> with no serious side effects, see attached diff. All the calls do noting 
> important, except the cCutter::Active() call, and this one is called 
> often enough in other situations. But even with this patch, an 
> non-interactive idle VDR waiting for shutdown will call this very 
> frequently.
> 
> Cheers,
> 
> Udo
> 
> 
> ------------------------------------------------------------------------
> 
> --- vdr.c.bak	2006-11-14 19:17:37.342544928 +0100
> +++ vdr.c	2006-11-14 19:16:50.045735136 +0100
> @@ -1149,9 +1149,9 @@
>                   Skins.Message(mtInfo, tr("Editing process finished"));
>                }
>             }
> -        if (!Interact && ((!cRecordControls::Active() && !cCutter::Active() && !cPluginManager::Active() && (!Interface->HasSVDRPConnection() || UserShutdown)) || ForceShutdown)) {
> -           time_t Now = time(NULL);
> -           if (Now - LastActivity > ACTIVITYTIMEOUT) {
> +        time_t Now = time(NULL);
> +        if (Now - LastActivity > ACTIVITYTIMEOUT) {
> +            if (!Interact && ((!cRecordControls::Active() && !cCutter::Active() && !cPluginManager::Active() && (!Interface->HasSVDRPConnection() || UserShutdown)) || ForceShutdown)) {
>                // Shutdown:
>                if (Shutdown && (Setup.MinUserInactivity || LastActivity == 1) && Now - LastActivity > Setup.MinUserInactivity * 60) {
>                   cTimer *timer = Timers.GetNextActiveTimer();
> 
> ------------------------------------------------------------------------

I don't really think this will improve things very much.

What about making cPluginManager::Active() remember the last state of
plugin activity, and, if Prompt is NULL, only call the individual plugins'
Active() functions if, say, one minute has passed since the last check?
If the timeout has not passed yet, and the remembered state is "true",
it can return that state right away.

This would reduce any possible overhead.

Klaus
  

Klaus Schmidinger wrote:
>> For VDR, the two if's in the inactivity shutdown should be swappable 
>> with no serious side effects, see attached diff. All the calls do 
>> noting important, except the cCutter::Active() call, and this one is 
>> called often enough in other situations. But even with this patch, an 
>> non-interactive idle VDR waiting for shutdown will call this very 
>> frequently.
>
> I don't really think this will improve things very much.

It will at least stop calling Active() when VDR is running normally, and 
is far away from automatic shutdown.

> What about making cPluginManager::Active() remember the last state of
> plugin activity, and, if Prompt is NULL, only call the individual plugins'
> Active() functions if, say, one minute has passed since the last check?

Sounds ok to me. One minute is close enough to avoid an idle machine 
running on, and not too often so a plugin can do more complex checks, 
for example call an external script to search for activity.

Cheers,

Udo
  

Udo Richter wrote:
> Klaus Schmidinger wrote:
>>> For VDR, the two if's in the inactivity shutdown should be swappable 
>>> with no serious side effects, see attached diff. All the calls do 
>>> noting important, except the cCutter::Active() call, and this one is 
>>> called often enough in other situations. But even with this patch, an 
>>> non-interactive idle VDR waiting for shutdown will call this very 
>>> frequently.
>>
>> I don't really think this will improve things very much.
> 
> It will at least stop calling Active() when VDR is running normally, and 
> is far away from automatic shutdown.

Are you sure?
ACTIVITYTIMEOUT is 60 seconds, so wouldn't your change just
avoid the calls until 60 seconds after the last user activity?

>> What about making cPluginManager::Active() remember the last state of
>> plugin activity, and, if Prompt is NULL, only call the individual 
>> plugins'
>> Active() functions if, say, one minute has passed since the last check?
> 
> Sounds ok to me. One minute is close enough to avoid an idle machine 
> running on, and not too often so a plugin can do more complex checks, 
> for example call an external script to search for activity.

Ok, then I'll make it so.

Klaus
  

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?

Cheers,

Udo