[RFC] Shutdown rewrite for 1.5.x

Message ID 20070113202823.GB3730@localhost.localdomain
State New
Headers

Commit Message

Marko Mäkelä Jan. 13, 2007, 8:28 p.m. UTC
  On Wed, Jan 10, 2007 at 02:28:24PM +0100, Udo Richter wrote:
> Hi list,
> 
> I've finished a second version of the shutdown rewrite. This time, there 
> are two patches available, one for VDR 1.5.0, and one with slight 
> changes for 1.4.x.
> 
> http://www.udo-richter.de/vdr/patches.html#shutdown
> http://www.udo-richter.de/vdr/patches.en.html#shutdown

I had a short look at this patch:
http://www.udo-richter.de/vdr/files/vdr-1.4.5-shutdown-rewrite-0.2.diff

A minor cosmetic thing: the '#' of pre-processor directives should always
be located at the first column of the line.  You can add spaces after
the '#' if you want to indent the directives.  This is probably not a
problem, if VDR is only compiled with GCC.  At least the compiler on
SGI IRIX (r.i.p.) would choke on such code.

A more important suggestion: Could you please add a notification method
to status.h for notifying plugins whether VDR is currently in
interactive mode?  This would be useful for at least these three
plugins: softdevice, subtitles, and relay (my own plugin for controlling
a solid-state relay that powers the display on or off).  Here are the
relevant chunks from my suspend patch
<http://www.iki.fi/~msmakela/software/vdr/#suspend>, which is to my
knowledge included in some VDR distributions:

   virtual void OsdTitle(const char *Title) {}
@@ -77,6 +80,7 @@ public:
   static void MsgSetVolume(int Volume, bool Absolute);
   static void MsgSetAudioTrack(int Index, const char * const *Tracks);
   static void MsgSetAudioChannel(int AudioChannel);
+  static void MsgSetSuspend(bool Suspend);
   static void MsgOsdClear(void);
   static void MsgOsdTitle(const char *Title);
   static void MsgOsdStatusMessage(const char *Message);

I'd suggest a method SetInteractive(bool Interactive).  When VDR is
in non-interactive mode, the display would be powered off and the
software decoding of video would be suspended.  For full-featured
cards, the suspendoutput plugin could perhaps be adapted:
http://users.tkk.fi/~phintuka/vdr/vdr-suspendoutput/

If you implemented the method, it would make my vdr-suspend patch
unnecessary.  I could then try your patch and adapt the three
plugins I use (relay, softdevice, and subtitles, in that order).

	Marko
  

Comments

Udo Richter Jan. 13, 2007, 8:48 p.m. UTC | #1
Marko Mäkelä wrote:
> A minor cosmetic thing: the '#' of pre-processor directives should always
> be located at the first column of the line.  

The only indented directives are for the debug output I think, and this 
will probably be removed before the final integration anyway.

> A more important suggestion: Could you please add a notification method
> to status.h for notifying plugins whether VDR is currently in
> interactive mode?  

You can poll this state on your own by calling 
Shutdown.IsUserInactive(). There is currently no internal notification 
anyway, VDR itself does poll it and would need to poll it to call 
MsgSetSuspend. The function itself is a light-weighted inline function, 
so its no problem to poll it once a second from cPlugin::MainThreadHook 
or similar.

Cheers,

Udo
  
Marko Mäkelä Jan. 15, 2007, 8:10 p.m. UTC | #2
On Sat, Jan 13, 2007 at 09:48:47PM +0100, Udo Richter wrote:
> >A more important suggestion: Could you please add a notification method
> >to status.h for notifying plugins whether VDR is currently in
> >interactive mode?  
> 
> You can poll this state on your own by calling 
> Shutdown.IsUserInactive(). There is currently no internal notification 
> anyway, VDR itself does poll it and would need to poll it to call 
> MsgSetSuspend.

Can you please explain to me why this could not be based on events?

While VDR is running, I would guess that the only way it can become
interactive is by receiving button events from the remote control unit.
You could have the RCU dispatcher thread (or any other place that
changes the flag) call MsgSetInteractive().  If this is not possible,
then you could declare a condition variable.  The main loop of the
main thread would check it with pthread_cond_timedwait() and call
MsgSetInteractive() when the status changes.

> The function itself is a light-weighted inline function, 
> so its no problem to poll it once a second from cPlugin::MainThreadHook 
> or similar.

Users would certainly prefer not to wait for a random time (0..1 seconds)
for VDR to become active.

In softdevice, this delay could be bypassed when softdevice is in charge
of the remote control unit.  But softdevice (or DirectFB) doesn't support
all types of remote control units.

	Marko
  
Udo Richter Jan. 15, 2007, 11:15 p.m. UTC | #3
Marko Mäkelä wrote:
> While VDR is running, I would guess that the only way it can become
> interactive is by receiving button events from the remote control unit.

Hmm, ok, I was mainly thinking of becoming inactive - since this is 
after a certain time has passed, it can only be polled. For lots of 
cases the current patch wont poll this state at all for a long time.

But becoming active again is a different thing of course.

> Users would certainly prefer not to wait for a random time (0..1 seconds)
> for VDR to become active.

Actually, the key press lets the VDR main loop spin, and directly after 
the key press was handled, there's a call to MainThreadHook, so its not 
that long.

Anyway, I'll think about that.

Cheers,

Udo
  
Marko Mäkelä Jan. 16, 2007, 9:31 a.m. UTC | #4
On Tue, Jan 16, 2007 at 12:15:32AM +0100, Udo Richter wrote:
> Marko Mäkelä wrote:
> >While VDR is running, I would guess that the only way it can become
> >interactive is by receiving button events from the remote control unit.
> 
> Hmm, ok, I was mainly thinking of becoming inactive - since this is 
> after a certain time has passed, it can only be polled. For lots of 
> cases the current patch wont poll this state at all for a long time.

Right, becoming inactive is not that time-critical.  I will give your
suggestion a try: in the MainThreadHook of each plugin, poll
Shutdown.IsUserInactive().

> Actually, the key press lets the VDR main loop spin, and directly after 
> the key press was handled, there's a call to MainThreadHook, so its not 
> that long.

Okay, this sounds reasonable.  It is also good that every plugin will
notice the change of Shutdown.IsUserInactive() virtually at the same
time.

One more thing: are the timeouts for interactive->inactive->shutdown
configureable?  Is there a way to force VDR to enter inactive mode
without immediate shutdown?

	Marko
  
Udo Richter Jan. 16, 2007, 11:25 p.m. UTC | #5
Marko Mäkelä wrote:
> Right, becoming inactive is not that time-critical.  I will give your
> suggestion a try: in the MainThreadHook of each plugin, poll
> Shutdown.IsUserInactive().

It should work, though the first key press may trigger some longer work, 
and the MainThreadHook will be called at the end.
For softdevice and skins it may be interesting to watch for skin / osd 
activity while inactive.

> One more thing: are the timeouts for interactive->inactive->shutdown
> configureable?  Is there a way to force VDR to enter inactive mode
> without immediate shutdown?

Inactive mode is triggered by the Min User Inactivity timeout config 
option, by default 3 hours. 3 hours after the last key press VDR changes 
to inactive.

At most 5 minutes before this timeout, but only if there's no background 
activity, the countdown timer starts. If the countdown runs to 0 and 
there is no further activity, VDR will initiate the shutdown. After that 
the 6 minutes retry counter starts, to retry the shutdown. (there's no 
way how the shutdown script can report failure.) These timeouts are 
hard-coded as constants in vdr.c.

To force inactive mode, one can call Shutdown.SetUserInactive(); - the 
power key does it for example. This will lead to a shutdown as soon as 
there's no more activity - if you want to be inactive, but avoid 
shutdown, you can announce your activity by cPlugin::Active().

Cheers,

Udo
  
Peter Dittmann Feb. 5, 2007, 12:29 p.m. UTC | #6
Udo Richter wrote:
> Marko Mäkelä wrote:
>> One more thing: are the timeouts for interactive->inactive->shutdown
>> configureable?  Is there a way to force VDR to enter inactive mode
>> without immediate shutdown?
> 
> 
> Inactive mode is triggered by the Min User Inactivity timeout config 
> option, by default 3 hours. 3 hours after the last key press VDR changes 
> to inactive.

How about a new two level inactivity timeout ?
After VDR starts the inactivity timeout uses a first short timeout (e.g. 
5min) assuming that VDR has automaticly being started.
Only after seeing any real user activity the normal Inactivity timeout 
(e.g. 2..3h) is used.
Of course this can be combined with override functions on VDR startup 
(e.g. force plugin or timer start) as already suggested.

Last but not least I would personally wish for an new 
anti-timeout-plugin that would block VDR from shutting down on request.
I sometimes watching TV in the workshop via RF modulated TV from my 
master VDR.
Sometimes you miss the shutdown countdown and are surprised that the TV 
turns to noise only, sometimes it's just anoying to go up to switch the 
countdouwn off ;-))

However the last one now look like it's just a very simple plugin.

kind regards Peter
  
Udo Richter Feb. 5, 2007, 7:37 p.m. UTC | #7
Peter Dittmann wrote:
> How about a new two level inactivity timeout ?
> After VDR starts the inactivity timeout uses a first short timeout (e.g. 
> 5min) assuming that VDR has automaticly being started.

Actually, even in worst case you'll see the 5-minute-countdown, so you 
already have enough time to become interactive.

> Last but not least I would personally wish for an new 
> anti-timeout-plugin that would block VDR from shutting down on request.

You could just set the Min User Inactivity to 0, and VDR will not shut 
down automatically. (unless you hit the power key - this will even work 
delayed, eg. after a recording ends.)

> However the last one now look like it's just a very simple plugin.

It is. Just type ./newplugin Noshutdown, then edit Noshutdown.c so that 
cPluginNoshutdown::Active() always returns a string. Done.


Cheers,

Udo
  

Patch

diff -pu vdr-1.4.0/status.h vdr-1.4.0-suspend/status.h
--- vdr-1.4.0/status.h	2005-12-31 17:15:25.000000000 +0200
+++ vdr-1.4.0-suspend/status.h	2006-05-01 21:42:34.000000000 +0300
@@ -44,6 +44,9 @@  protected:
   virtual void SetAudioChannel(int AudioChannel) {}
                // The audio channel has been set to the given value.
                // 0=stereo, 1=left, 2=right, -1=no information
                // available.
+  virtual void SetSuspend(bool Suspend) {}
+               // Suspend or resume audio/video playback.
+               // true=suspend, false=resume.
   virtual void OsdClear(void) {}
                // The OSD has been cleared.