From patchwork Mon Apr 17 09:37:37 2006 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Klaus Schmidinger X-Patchwork-Id: 12259 Received: from raven.cadsoft.de ([217.7.101.211]) by www.linuxtv.org with esmtp (Exim 4.50) id 1FVQAv-0006wc-8V for vdr@linuxtv.org; Mon, 17 Apr 2006 11:37:41 +0200 Received: from [192.168.100.10] (hawk.cadsoft.de [192.168.100.10]) by raven.cadsoft.de (8.13.3/8.13.3) with ESMTP id k3H9bdbF016549 for ; Mon, 17 Apr 2006 11:37:39 +0200 Message-ID: <444361E1.3060302@cadsoft.de> Date: Mon, 17 Apr 2006 11:37:37 +0200 From: Klaus Schmidinger Organization: CadSoft Computer GmbH User-Agent: Mozilla Thunderbird 1.0.6 (X11/20050716) X-Accept-Language: en MIME-Version: 1.0 To: vdr@linuxtv.org Subject: Re: [vdr] calling MainMenuAction() from an other plugin References: <4428FAD4.8090001@gmx.de> <200603281220.31543.uhanke@gmx.de> <200603281529.30272.wolfgang@rohdewald.de> <44294895.5070403@gmx.de> <4429A484.7020901@gmx.de> <4429A8E2.8000808@cadsoft.de> <4429AEF6.6070104@gmx.de> In-Reply-To: <4429AEF6.6070104@gmx.de> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-2.0 (raven.cadsoft.de [192.168.1.1]); Mon, 17 Apr 2006 11:37:40 +0200 (CEST) X-BeenThere: vdr@linuxtv.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: VDR Mailing List List-Id: VDR Mailing List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 17 Apr 2006 09:37:41 -0000 Status: O X-Status: X-Keywords: X-UID: 8870 Reinhard Nissl wrote: > Hi, > > Klaus Schmidinger wrote: > >>>>>> if (ShowMessage && !Skins.IsOpen() && !cOsd::IsOpen()) { >>>>>> ShowMessage = false; >>>>>> cRemote::CallPlugin("myShowMessage"); >>>>>> } >>>>> >>>>> >>>>> >>>>> this looks racy to me. What if two different threads do this? >>>>> The OSD could open between the if () and ::CallPlugin >>>> >>>> >>>> >>>> CallPlugin is always racy, as only one plugin call will be remembered. >>>> >>>> The only safe way is to set up a signal and wait for MainMenuAction >>>> being called. If it is not called within timeout, try again. >>>> >>>> The IsOpen() calls should ideally be made from foreground thread, >>>> but usually there's no way to get there. Testing from within >>>> MainMenuAction is pointless, as OSD and menu get closed right before >>>> MainMenuAction. >>> >>> >>> >>> I did something similar in my vdr-xine plugin. Things could be >>> improved using the attached patch. >>> >>> It allows to remember up to 16 calls to different plugins and the >>> caller can detect, whether putting the call in the remote key fifo >>> was successful. >> >> >> Just so I get this right: with your patch it will be possible >> to put several plugin calls into the key queue. Let's assume that >> there are 3 plugin calls currently in the queue. When VDR goes through >> the next main loop, it will encounter a k_Plugin in the queue and call >> the first plugin's MainMenuAction(). Since there is more input in the >> queue, that plugin's OSD won't be displayed, yet (that's only >> done in cInterface::GetKey() in case the queue is empty). So it takes >> the next k_Plugin out of the queue, deletes the menu it just created >> and call the second plugin's MainMenuAction(). Before displaying that one >> it will get the third k_Plugin out of the queue, call that plugin's >> MainMenuAction(), and that's what will be finally displayed. >> >> Unless I'm missing something here, queueing (up to 16) k_Plugins >> doesn't have any different result (from the user's point of view) >> than just overwriting any previously stored plugin call. The only thing >> that appears to be important is the (currently missing) mutex lock >> in cRemote::CallPlugin(). >> >> Am I missing something here? > > > In my case, I don't show an OSD. I just use this functionality as a > trampoline to have the VDR main thread execute my code for switching the > primary device, as it doesn't work reliably when it is done in any other > thread. > > So you are right, when a plugin opens an OSD (which is the typical > case), one will only see the OSD of the last plugin. On the other hand, > it would be useful to know for the caller, that the MenuMenuAction of > the specified plugin will be called when CallPlugin() returned true. > Otherwise it would need more "intelligent" code at the caller to achieve > the call under race conditions. > > In the case where the above is of no interest, there is no need to have > an additional mutex lock in CallPlugin(), as Put() has one in remote.c:79. I'm not particularly fond of that FIFO of yours. However, I do realize that it is useful to tell the caller of cRemote::CallPlugin() whether the call was successful. The attached patch vdr-1.3.46-callplugin.diff makes cRemote::CallPlugin() return false if there is currently a plugin call pending. The ability to "catch the main thread" will be implemented by the second attached patch (vdr-1.3.46-mainthreadhook.diff), so that no "dirty tricks" should be necessary. This patch may have its line numbers a little off, because I have already made other changes to these files, but I wanted to give you a chance to look at this and maybe comment on it before I release version 1.3.47 later today. Klaus --- plugin.h 2005/08/27 16:13:17 1.10 +++ plugin.h 2006/04/17 09:18:16 @@ -39,6 +39,7 @@ virtual bool Start(void); virtual void Stop(void); virtual void Housekeeping(void); + virtual void MainThreadHook(void); virtual const char *MainMenuEntry(void); virtual cOsdObject *MainMenuAction(void); @@ -89,6 +91,7 @@ bool InitializePlugins(void); bool StartPlugins(void); void Housekeeping(void); + void MainThreadHook(void); static bool HasPlugins(void); static cPlugin *GetPlugin(int Index); static cPlugin *GetPlugin(const char *Name); --- plugin.c 2006/04/09 14:16:17 1.18 +++ plugin.c 2006/04/17 09:20:05 @@ -69,6 +70,10 @@ { } +void cPlugin::MainThreadHook(void) +{ +} + const char *cPlugin::MainMenuEntry(void) { return NULL; @@ -364,6 +374,15 @@ } } +void cPluginManager::MainThreadHook(void) +{ + for (cDll *dll = pluginManager->dlls.First(); dll; dll = pluginManager->dlls.Next(dll)) { + cPlugin *p = dll->Plugin(); + if (p) + p->MainThreadHook(); + } +} + bool cPluginManager::HasPlugins(void) { return pluginManager && pluginManager->dlls.Count(); --- vdr.c 2006/04/09 12:22:46 1.254 +++ vdr.c 2006/04/17 09:23:23 @@ -830,6 +830,8 @@ // Queued messages: if (!Skins.IsOpen()) Skins.ProcessQueuedMessages(); + // Main thread hooks of plugins: + PluginManager.MainThreadHook(); // User Input: cOsdObject *Interact = Menu ? Menu : cControl::Control(); eKeys key = Interface->GetKey((!Interact || !Interact->NeedsFastResponse()) && time(NULL) - LastCamMenu > LASTCAMMENUTIMEOUT);