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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vdr-1.3.46-callplugin.diff
Type: text/x-patch
Size: 2444 bytes
Desc: not available
Url : http://www.linuxtv.org/pipermail/vdr/attachments/20060417/f1e728d9/vdr-1.3.46-callplugin-0001.bin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vdr-1.3.46-mainthreadhook.diff
Type: text/x-patch
Size: 1636 bytes
Desc: not available
Url : http://www.linuxtv.org/pipermail/vdr/attachments/20060417/f1e728d9/vdr-1.3.46-mainthreadhook-0001.bin
