Hello,
a question about the 'new' vdr shutdown handling implemented a few versions ago.
Every time i connect a client to the streamdev plugin the log is full of these message:
Nov 14 14:26:42 linvdr vdr: [8480] streamdev-server: no translation found for 'Streaming busy, can't shutdown' in language 1 (Deutsch) Nov 14 14:27:13 linvdr last message repeated 31 times Nov 14 14:27:19 linvdr last message repeated 6 times
This is the code i found in the streamdev plugin:
cString cPluginStreamdevServer::Active(void) { if (cStreamdevServer::Active()) return tr("Streaming busy, can't shutdown"); return 0; }
I know, it's bescause of the missing translation ;) 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?
Regards Jörg
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();
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
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.
The attached patch does it. This patch has one side effect: The message wont be re-translated if the selected VDR OSD language is changed, you have to restart to see language changes.
Cheers,
Udo
--- streamdev-server.c.bak 2006-11-15 14:29:20.000000000 +0100 +++ streamdev-server.c 2006-11-15 14:30:51.000000000 +0100 @@ -59,7 +59,9 @@ { if (cStreamdevServer::Active()) { - return tr("Streaming active"); + static const char *Message = NULL; + if (!Message) Message = tr("Streaming active"); + return Message; } return NULL; }
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
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
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
Am Samstag, 2. Dezember 2006 16:24 schrieb Matthias Schwarzott:
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
The attached patch does what you suggest, but I don't see any correlation to the shutdown behaviour of plugins...
Tim
On Fri, Dec 08, 2006 at 08:21:07PM +0100, Thiemo Gehrke wrote:
Am Samstag, 2. Dezember 2006 16:24 schrieb Matthias Schwarzott:
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
The attached patch does what you suggest, but I don't see any correlation to the shutdown behaviour of plugins...
What about a third possibility: the computer is started by Wake-on-LAN or Wake-on-Ring? It is kind of manual startup, but there should not be any output in that case.
I have written a patch for making the Power key suspend the output. Some softdevice users have found it useful, but I've been told that the patch does not stop playback on full-featured cards. You can access the patch here: http://www.iki.fi/~msmakela/software/vdr/#suspend
Quoting from there:
"The patch introduces three 'Setup/Miscellanous' menu items. The Booleans 'Power button suspends playback' and 'Playback suspended' should be self-explanatory. The integer, 'Suspend to shutdown timeout (s)', is 0 by default, meaning that no shutdown will be initiated when playback is suspended by pressing the Power button. If you would like to use the Power button as a smart power-off button that works in any context, set this timeout to a suitable value, e.g., five seconds. Should you only want to suspend playback and not power the system off, you can press Power followed by OK. The latter button-press will confirm the 'Press any key to cancel shutdown prompt' if one is presented. (If any timed recordings are in progress, the prompt will be presented after they have finished, unless the shutdown was cancelled by pressing any button.)"
I originally made the patch for vdr 1.3.32, and back then it was too late to change the shutdown behaviour of 1.4. I hope that the feature will make it to 1.5 in some form.
Marko
Marko Mäkelä wrote:
I have written a patch for making the Power key suspend the output. Some softdevice users have found it useful, but I've been told that the patch does not stop playback on full-featured cards. You can access the patch here: http://www.iki.fi/~msmakela/software/vdr/#suspend
I originally made the patch for vdr 1.3.32, and back then it was too late to change the shutdown behaviour of 1.4. I hope that the feature will make it to 1.5 in some form.
Since I've taken the job to re-design the shutdown stuff for 1.5.x, I probably have to take a look into this too.
I'm not sure whether this will be a core feature of VDR, but maybe we can add the needed interfaces so this can be done with a plugin.
I'm not yet sure how exactly the new shutdown code will look like, but I'm quite sure there will be a way to detect whether VDR is currently interactive (some user is watching) or not (unattended start, timer recording, no activity for x hours, power button pressed but not confirmed). Based on that, a plugin could probably take over the output device and suspend it or show a still. Or softdevice itself could react on this.
Cheers,
Udo
On Sat, Dec 09, 2006 at 05:16:22PM +0100, Udo Richter wrote:
Marko Mäkelä wrote:
I have written a patch for making the Power key suspend the output. Some softdevice users have found it useful, but I've been told that the patch does not stop playback on full-featured cards. You can access the patch here: http://www.iki.fi/~msmakela/software/vdr/#suspend
I originally made the patch for vdr 1.3.32, and back then it was too late to change the shutdown behaviour of 1.4. I hope that the feature will make it to 1.5 in some form.
Since I've taken the job to re-design the shutdown stuff for 1.5.x, I probably have to take a look into this too.
Thank you, I really appreciate that!
I'm not sure whether this will be a core feature of VDR, but maybe we can add the needed interfaces so this can be done with a plugin.
At the very least, the plugins should be able to receive suspend and resume events. I think it would be useful to expose the interface for generating suspend and resume events as well.
I'm not yet sure how exactly the new shutdown code will look like, but I'm quite sure there will be a way to detect whether VDR is currently interactive (some user is watching) or not (unattended start, timer recording, no activity for x hours, power button pressed but not confirmed). Based on that, a plugin could probably take over the output device and suspend it or show a still. Or softdevice itself could react on this.
If the suspend and resume events were to be processed in softdevice, then also the subtitles plugin would have to process the events. Otherwise, the subtitles would keep running on the OSD even though video and audio are suspended.
Regards,
Marko
Marko Mäkelä wrote:
At the very least, the plugins should be able to receive suspend and resume events. I think it would be useful to expose the interface for generating suspend and resume events as well.
Adding support for a suspended device state is not really part of the shutdown, so you'll have to convince Klaus on that. Plus, if you just want to disable the video decoder, you can feed it with a dummy signal and don't need a special suspended state. It works for streamdev at least.
confirmed). Based on that, a plugin could probably take over the output device and suspend it or show a still. Or softdevice itself could react on this.
If the suspend and resume events were to be processed in softdevice, then also the subtitles plugin would have to process the events. Otherwise, the subtitles would keep running on the OSD even though video and audio are suspended.
If Softdevice knows that there is no current user, then it is free to also ignore subtitles together with the video signal. I don't see a problem here.
Cheers,
Udo
I demand that Udo Richter may or may not have written...
[snip]
I'm not yet sure how exactly the new shutdown code will look like, but I'm quite sure there will be a way to detect whether VDR is currently interactive (some user is watching) or not (unattended start, timer recording, no activity for x hours, power button pressed but not confirmed). Based on that, a plugin could probably take over the output device and suspend it or show a still. Or softdevice itself could react on this.
It also needs to cope with being shut down by a signal without any possibility of user intervention. Two levels should be available: stop now, and stop when inactive.
This helps init scripts when upgrading a packaged-for-$DISTRIBUTION version of vdr since there's no guarantee that the upgrade won't be scheduled for when nothing is being recorded.
Darren Salt wrote:
It also needs to cope with being shut down by a signal without any possibility of user intervention. Two levels should be available: stop now, and stop when inactive.
This helps init scripts when upgrading a packaged-for-$DISTRIBUTION version of vdr since there's no guarantee that the upgrade won't be scheduled for when nothing is being recorded.
you mean, a way to kill the VDR process only when there's no background activity? Why not just send a power key via SVDRP?
For the purpose of installing updates, I wouldn't like it if the update happens when I'm watching live TV either, so we're at the same point again: Shut down when no user is active and no background activity is going on. Could be done by abusing the existing shutdown script.
Another possibility would be to query the active/inactive state by SVDRP commands, either by VDR itself, or by a plugin.
Cheers,
Udo
I demand that Udo Richter may or may not have written...
Darren Salt wrote:
It also needs to cope with being shut down by a signal without any possibility of user intervention. Two levels should be available: stop now, and stop when inactive.
This helps init scripts when upgrading a packaged-for-$DISTRIBUTION version of vdr since there's no guarantee that the upgrade won't be scheduled for when nothing is being recorded.
you mean, a way to kill the VDR process only when there's no background activity? Why not just send a power key via SVDRP?
That doesn't guarantee that vdr will exit promptly or as soon as it becomes inactive. A different port may have been chosen, SVDRP may have been disabled, or the suspend patch may be in use (and my vdr package has this patch).
There's "restart on becoming inactive" too, but I think that the restart part of that is best implemented in runvdr since it too may need to be restarted.
For the purpose of installing updates, I wouldn't like it if the update happens when I'm watching live TV either, so we're at the same point again: Shut down when no user is active and no background activity is going on. Could be done by abusing the existing shutdown script.
Perhaps. You could have a look at my packaging:
http://zap.tartarus.org/~ds/debian/dists/unstable/main/source/vdr_1.4.4-1.ds...
Another possibility would be to query the active/inactive state by SVDRP commands, either by VDR itself, or by a plugin.
I still prefer kill(1) and kill(2) - no faffing around with sockets :-)
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
-
Darren Salt -
Frank Schmirler -
Jörg Wendel -
Klaus Schmidinger -
Marko Mäkelä -
Matthias Schwarzott -
Thiemo Gehrke -
Udo Richter