fix segfault in cSkins::Message

  On Donnerstag 30 Juni 2005 10:31, Wolfgang Rohdewald wrote:
> this happens if the MENUTIMEOUT makes the OSD menu 
> disappear while a message is displayed.

more is needed - all delete osd should also nullify it. See
gdb output below.

also, I can only get rid of the segfault shown by valgrind below
if constructors with osd as private member nullify osd right
at the beginning, before calling NewOsd(). (I did not check which
constructor causes my segfault, just applied it to all of them).

I suppose other skins might have the same problem.

This looks to me as if access to osd might not be threadsafe,
but I am by no means a thread expert

Extended patch attached. While being at it, I replaced a few
more delete x by DELETENULL(x)

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 16384 (LWP 8769)]
0x080fec70 in cSkinClassicDisplayMessage::Flush (this=0x9f38ec0) at skinclassic.c:644
644       osd->Flush();
Current language:  auto; currently c++
(gdb) p osd
$1 = (class cOsd *) 0x10
(gdb) bt
#0  0x080fec70 in cSkinClassicDisplayMessage::Flush (this=0x9f38ec0) at skinclassic.c:644
#1  0x081015dc in cSkins::Flush (this=0x81ef5a4) at skins.c:210
#2  0x080b95ab in cInterface::GetKey (this=0x9e5b6b8, Wait=true) at interface.c:35
#3  0x081223c5 in main (argc=14, argv=0xbf85ffd4) at vdr.c:639

line 644 is:
  if (osd)
          osd->Flush();

==20735== Conditional jump or move depends on uninitialised value(s)
==20735==    at 0x80FED8D: cSkinClassicDisplayMessage::Flush() (skinclassic.c:644)
==20735==    by 0x81016FF: cSkins::Flush() (skins.c:210)
==20735==    by 0x80B9612: cInterface::GetKey(bool) (interface.c:35)
==20735==    by 0x81224E8: main (vdr.c:639)
==20735==
==20735== Use of uninitialised value of size 4
==20735==    at 0x80FED95: cSkinClassicDisplayMessage::Flush() (skinclassic.c:645)
==20735==    by 0x81016FF: cSkins::Flush() (skins.c:210)
==20735==    by 0x80B9612: cInterface::GetKey(bool) (interface.c:35)
==20735==    by 0x81224E8: main (vdr.c:639)
==20735==
==20735== Invalid read of size 4
==20735==    at 0x80FEDA3: cSkinClassicDisplayMessage::Flush() (skinclassic.c:645)
==20735==    by 0x81016FF: cSkins::Flush() (skins.c:210)
==20735==    by 0x80B9612: cInterface::GetKey(bool) (interface.c:35)
==20735==    by 0x81224E8: main (vdr.c:639)
==20735==  Address 0x6564698A is not stack'd, malloc'd or (recently) free'd
==20735==
==20735== Process terminating with default action of signal 11 (SIGSEGV)
==20735==  GPF (Pointer out of bounds?)
==20735==    at 0x80FEDA3: cSkinClassicDisplayMessage::Flush() (skinclassic.c:645)
==20735==    by 0x81016FF: cSkins::Flush() (skins.c:210)
==20735==    by 0x80B9612: cInterface::GetKey(bool) (interface.c:35)
==20735==    by 0x81224E8: main (vdr.c:639)
==20735==
  

Wolfgang Rohdewald wrote:
> On Donnerstag 30 Juni 2005 10:31, Wolfgang Rohdewald wrote:
> 
>>this happens if the MENUTIMEOUT makes the OSD menu 
>>disappear while a message is displayed.
> 
> 
> more is needed - all delete osd should also nullify it. See
> gdb output below.
> 
> also, I can only get rid of the segfault shown by valgrind below
> if constructors with osd as private member nullify osd right
> at the beginning, before calling NewOsd(). (I did not check which
> constructor causes my segfault, just applied it to all of them).
> 
> I suppose other skins might have the same problem.
> 
> This looks to me as if access to osd might not be threadsafe,
> but I am by no means a thread expert

IIRC we already had this discussion some time ago.
The point is that you're not supposed to call any of the skin
functions from a thread. These functions are only supposed to be called
by VDR itself.

Klaus
  

On 30 Jun 2005 Klaus Schmidinger <Klaus.Schmidinger@cadsoft.de> wrote:

> The point is that you're not supposed to call any of the skin
> functions from a thread. These functions are only supposed to be called
> by VDR itself.

I assume that this means "to be called from the foreground thread
only". No matter if it's VDR itself or a plugin.

Or am I wrong?

Regards.
  

Stefan Huelswitt wrote:
> On 30 Jun 2005 Klaus Schmidinger <Klaus.Schmidinger@cadsoft.de> wrote:
> 
> 
>>The point is that you're not supposed to call any of the skin
>>functions from a thread. These functions are only supposed to be called
>>by VDR itself.
> 
> 
> I assume that this means "to be called from the foreground thread
> only". No matter if it's VDR itself or a plugin.

Yes, that´s what I meant.

Klaus
  

On Donnerstag 30 Juni 2005 16:53, Klaus Schmidinger wrote:
> IIRC we already had this discussion some time ago.
> The point is that you're not supposed to call any of the skin
> functions from a thread. These functions are only supposed to be called
> by VDR itself.

how else can a plugin display a message or ask a question? since 
cInterface::Confirm() also calls Skins.Message(), I suppose Confirm()
is also illegal for plugins?

this is how muggle has always been doing this:

#if VDRVERSNUM >= 10307
        Skins.Message (mtInfo, buffer,duration);
        Skins.Flush ();
#else
        Interface->Status (buffer);
        Interface->Flush ();
#endif

and

    if (!Interface->Confirm(tr("Import items?")))
            return false;
  

Wolfgang Rohdewald wrote:
> On Donnerstag 30 Juni 2005 16:53, Klaus Schmidinger wrote:
> 
>>IIRC we already had this discussion some time ago.
>>The point is that you're not supposed to call any of the skin
>>functions from a thread. These functions are only supposed to be called
>>by VDR itself.
> 
> 
> how else can a plugin display a message or ask a question? since 
> cInterface::Confirm() also calls Skins.Message(), I suppose Confirm()
> is also illegal for plugins?
> 
> this is how muggle has always been doing this:
> 
> #if VDRVERSNUM >= 10307
>         Skins.Message (mtInfo, buffer,duration);
>         Skins.Flush ();
> #else
>         Interface->Status (buffer);
>         Interface->Flush ();
> #endif
> 
> and
> 
>     if (!Interface->Confirm(tr("Import items?")))
>             return false;

Sorry, I was a little too vague here.
What I meant to say was that these functions shall only be called
from the _foreground_ thread (the one that VDR's main loop runs in).

Klaus
  

On Donnerstag 30 Juni 2005 17:37, Klaus Schmidinger wrote:
> Stefan Huelswitt wrote:
> > On 30 Jun 2005 Klaus Schmidinger <Klaus.Schmidinger@cadsoft.de> wrote:
> > 
> > 
> >>The point is that you're not supposed to call any of the skin
> >>functions from a thread. These functions are only supposed to be called
> >>by VDR itself.
> > 
> > 
> > I assume that this means "to be called from the foreground thread
> > only". No matter if it's VDR itself or a plugin.
> 
> Yes, that´s what I meant.

in the case of muggle: There is a menu entry in the muggle setup which
starts a thread importing tracks. This thread shows status messages
like "Imported 1000 item". When the OSD times out while such a status
message is displayed, segfault.

So will that importing thread have to pass the message to vdr through
port 2001 or is there an easier way?

from cInterface::GetKey():
     if (!Skins.IsOpen()) {
        char *message = SVDRP->GetMessage();
        if (message) {
           Skins.Message(mtInfo, message);
           free(message);
        }
  

On Donnerstag, 30. Juni 2005 17:43, Klaus Schmidinger wrote:
> Wolfgang Rohdewald wrote:
> > On Donnerstag 30 Juni 2005 16:53, Klaus Schmidinger wrote:
> > 
> >>IIRC we already had this discussion some time ago.
> >>The point is that you're not supposed to call any of the skin
> >>functions from a thread. These functions are only supposed to be called
> >>by VDR itself.
> > 
> > 
> > how else can a plugin display a message or ask a question? since 
> > cInterface::Confirm() also calls Skins.Message(), I suppose Confirm()
> > is also illegal for plugins?
> > 
> > this is how muggle has always been doing this:
> > 
> > #if VDRVERSNUM >= 10307
> >         Skins.Message (mtInfo, buffer,duration);
> >         Skins.Flush ();
> > #else
> >         Interface->Status (buffer);
> >         Interface->Flush ();
> > #endif
> > 
> > and
> > 
> >     if (!Interface->Confirm(tr("Import items?")))
> >             return false;
> 
> Sorry, I was a little too vague here.
> What I meant to say was that these functions shall only be called
> from the _foreground_ thread (the one that VDR's main loop runs in).

Is there another way for a plugin to display a message during
background processing ? 
Preferably a _non_ blocking way,as I noticed that Skins.Message()
does it's job blocking.
  

Stefan Lucke wrote:
> On Donnerstag, 30. Juni 2005 17:43, Klaus Schmidinger wrote:
> 
>>Wolfgang Rohdewald wrote:
>>
>>>On Donnerstag 30 Juni 2005 16:53, Klaus Schmidinger wrote:
>>>
>>>
>>>>IIRC we already had this discussion some time ago.
>>>>The point is that you're not supposed to call any of the skin
>>>>functions from a thread. These functions are only supposed to be called
>>>>by VDR itself.
>>>
>>>
>>>how else can a plugin display a message or ask a question? since 
>>>cInterface::Confirm() also calls Skins.Message(), I suppose Confirm()
>>>is also illegal for plugins?
>>>
>>>this is how muggle has always been doing this:
>>>
>>>#if VDRVERSNUM >= 10307
>>>        Skins.Message (mtInfo, buffer,duration);
>>>        Skins.Flush ();
>>>#else
>>>        Interface->Status (buffer);
>>>        Interface->Flush ();
>>>#endif
>>>
>>>and
>>>
>>>    if (!Interface->Confirm(tr("Import items?")))
>>>            return false;
>>
>>Sorry, I was a little too vague here.
>>What I meant to say was that these functions shall only be called
>>from the _foreground_ thread (the one that VDR's main loop runs in).
> 
> 
> Is there another way for a plugin to display a message during
> background processing ? 
> Preferably a _non_ blocking way,as I noticed that Skins.Message()
> does it's job blocking.

Is there a solution for this problem?
I now also need to notify the user of a background event, and the 
Housekeeping method (a possible candidate since it's called from the 
foreground thread) isn't suitable, since it's called only once every 60 
seconds.

Bye
  

On Dienstag 26 Juli 2005 14:29, Luca Olivetti wrote:
> Stefan Lucke wrote:
> > Is there another way for a plugin to display a message during
> > background processing ? 
> > Preferably a _non_ blocking way,as I noticed that Skins.Message()
> > does it's job blocking.
> 
> Is there a solution for this problem?
> I now also need to notify the user of a background event, and the 
> Housekeeping method (a possible candidate since it's called from the 
> foreground thread) isn't suitable, since it's called only once every 60 
> seconds.

as far as I can see the only other method is to connect to port 2001.

For now we disabled those messages in muggle.
  

On Dienstag, 26. Juli 2005 18:52, Wolfgang Rohdewald wrote:
> On Dienstag 26 Juli 2005 14:29, Luca Olivetti wrote:
> > Stefan Lucke wrote:
> > > Is there another way for a plugin to display a message during
> > > background processing ? 
> > > Preferably a _non_ blocking way,as I noticed that Skins.Message()
> > > does it's job blocking.
> > 
> > Is there a solution for this problem?
> > I now also need to notify the user of a background event, and the 
> > Housekeeping method (a possible candidate since it's called from the 
> > foreground thread) isn't suitable, since it's called only once every 60 
> > seconds.
> 
> as far as I can see the only other method is to connect to port 2001.

The SVDRP port is not hardcoded, it can be specified via "-p number".
So the next question would be, how can a plugin read the port
our main program is listening on ? So plugins need the -p option too.

Isn't it more suitable to make Skins.Message() usable for plugins ?
Klaus ?

> 
> For now we disabled those messages in muggle.
>
  

Stefan Lucke wrote:
> On Dienstag, 26. Juli 2005 18:52, Wolfgang Rohdewald wrote:
> 
>>On Dienstag 26 Juli 2005 14:29, Luca Olivetti wrote:
>>
>>>Stefan Lucke wrote:
>>>
>>>>Is there another way for a plugin to display a message during
>>>>background processing ? 
>>>>Preferably a _non_ blocking way,as I noticed that Skins.Message()
>>>>does it's job blocking.
>>>
>>>Is there a solution for this problem?
>>>I now also need to notify the user of a background event, and the 
>>>Housekeeping method (a possible candidate since it's called from the 
>>>foreground thread) isn't suitable, since it's called only once every 60 
>>>seconds.
>>
>>as far as I can see the only other method is to connect to port 2001.
> 
> 
> The SVDRP port is not hardcoded, it can be specified via "-p number".
> So the next question would be, how can a plugin read the port
> our main program is listening on ? So plugins need the -p option too.
> 
> Isn't it more suitable to make Skins.Message() usable for plugins ?
> Klaus ?

A plugin may well call Skins.Message(), but only from the _main_
thread.

I'm thinking about a function like

   cOsdObject *cPlugin::Popup(void);

which would be called in every turn of the main loop (i.e. from the
foreground thread) whenever there is no OSD display open. A plugin
could either return a cOsdObject that will be presented to the user,
or could directly display a message (even with user interaction, if
necessary) and retrun NULL. Of course, if a plugin's thread wants to
issue a message, it will have to store the message inside the plugin
and wait until asked through a call to its Popup() function.

Would that be a useful solution?

Klaus
  

On Sonntag, 31. Juli 2005 16:56, Klaus Schmidinger wrote:
> Stefan Lucke wrote:
> > On Dienstag, 26. Juli 2005 18:52, Wolfgang Rohdewald wrote:
> > 
> >>On Dienstag 26 Juli 2005 14:29, Luca Olivetti wrote:
> >>
> >>>Stefan Lucke wrote:
> >>>
> >>>>Is there another way for a plugin to display a message during
> >>>>background processing ? 
> >>>>Preferably a _non_ blocking way,as I noticed that Skins.Message()
> >>>>does it's job blocking.
> >>>
> >>>Is there a solution for this problem?
> >>>I now also need to notify the user of a background event, and the 
> >>>Housekeeping method (a possible candidate since it's called from the 
> >>>foreground thread) isn't suitable, since it's called only once every 60 
> >>>seconds.
> >>
> >>as far as I can see the only other method is to connect to port 2001.
> > 
> > 
> > The SVDRP port is not hardcoded, it can be specified via "-p number".
> > So the next question would be, how can a plugin read the port
> > our main program is listening on ? So plugins need the -p option too.
> > 
> > Isn't it more suitable to make Skins.Message() usable for plugins ?
> > Klaus ?
> 
> A plugin may well call Skins.Message(), but only from the _main_
> thread.
> 
> I'm thinking about a function like
> 
>    cOsdObject *cPlugin::Popup(void);
> 
> which would be called in every turn of the main loop (i.e. from the
> foreground thread) whenever there is no OSD display open. A plugin
> could either return a cOsdObject that will be presented to the user,
> or could directly display a message (even with user interaction, if
> necessary) and retrun NULL. Of course, if a plugin's thread wants to
> issue a message, it will have to store the message inside the plugin
> and wait until asked through a call to its Popup() function.
> 
> Would that be a useful solution?
> 

In some way yes, as it is something like InteractiveHousekeeping().
But that would introduce message queueing code into each plugin, which
wants to display a message. The condition "whenever there is no OSD
display open" will cause an undetermined delay. "normal" OSDs have
an area where such messages could be displayed.
I would prefer something like Skins.QueueMessage() so that there is
only one peace of code and each plugin can use it.
For more complex OSDs (interactive ones) Popup() is more suitable:
"You've new mail: read now (y (green) / n (red)) ?"
  

Klaus Schmidinger wrote:

> A plugin may well call Skins.Message(), but only from the _main_
> thread.
> 
> I'm thinking about a function like
> 
>   cOsdObject *cPlugin::Popup(void);
> 
> which would be called in every turn of the main loop (i.e. from the
> foreground thread) whenever there is no OSD display open. A plugin
> could either return a cOsdObject that will be presented to the user,

In this case the function wouldn't be called again, would it?
Isn't it possible to use Skins.Message even with an osd already open?
Maybe

cOsdObject *cPlugin::Popup(bool canpopup)

called regardless of an osd open (and canpopup set to false if the 
plugin cannot create a cOsdObject)?

> or could directly display a message (even with user interaction, if
> necessary) and retrun NULL. Of course, if a plugin's thread wants to
> issue a message, it will have to store the message inside the plugin
> and wait until asked through a call to its Popup() function.
> 
> Would that be a useful solution?

I think it would be useful. For example, I could use it in my actuator 
plugin to show the dish position instead of patching vdr as I do now, 
and as soon as the user presses a button I could close my cOsdObject and 
reinject the key to vdr.
But it's not a complete solution, what if more than one plugin needs to 
show something on screen? I reckon there's no simple solution to that, 
but maybe the solution would be implementing something like the ttxsubs 
or subtitles patches do (something like more than one osd opened at 
once, with only the highest priority one actually shown), or some kind 
of windowing system...I really don't know.

Bye
  

On Sonntag 31 Juli 2005 16:56, Klaus Schmidinger wrote:
> Stefan Lucke wrote:
> > Isn't it more suitable to make Skins.Message() usable for plugins ?
> > Klaus ?
> 
> A plugin may well call Skins.Message(), but only from the _main_
> thread.
> 
> I'm thinking about a function like
> 
>    cOsdObject *cPlugin::Popup(void);
> 
> which would be called in every turn of the main loop (i.e. from the
> foreground thread) whenever there is no OSD display open. A plugin
> could either return a cOsdObject that will be presented to the user,
> or could directly display a message (even with user interaction, if
> necessary) and retrun NULL. Of course, if a plugin's thread wants to
> issue a message, it will have to store the message inside the plugin
> and wait until asked through a call to its Popup() function.
> 
> Would that be a useful solution?

Sounds good to me.

How long would the cOsdObject be displayed? Could this be definable
by a parameter like in cSkins::Message()?

Does vdr have a way to notice if a background thread accesses the
OSD? If so, and if this happens thru cSkins::Message() or cInterface::Confirm(),
could vdr automatically handle this? Or just issue a warning
like "You should use ::Popup()"
  

Wolfgang Rohdewald wrote:
> On Sonntag 31 Juli 2005 16:56, Klaus Schmidinger wrote:
> 
>>Stefan Lucke wrote:
>>
>>>Isn't it more suitable to make Skins.Message() usable for plugins ?
>>>Klaus ?
>>
>>A plugin may well call Skins.Message(), but only from the _main_
>>thread.
>>
>>I'm thinking about a function like
>>
>>   cOsdObject *cPlugin::Popup(void);
>>
>>which would be called in every turn of the main loop (i.e. from the
>>foreground thread) whenever there is no OSD display open. A plugin
>>could either return a cOsdObject that will be presented to the user,
>>or could directly display a message (even with user interaction, if
>>necessary) and retrun NULL. Of course, if a plugin's thread wants to
>>issue a message, it will have to store the message inside the plugin
>>and wait until asked through a call to its Popup() function.
>>
>>Would that be a useful solution?
> 
> 
> Sounds good to me.
> 
> How long would the cOsdObject be displayed? Could this be definable
> by a parameter like in cSkins::Message()?

The cOsdObject would be displayed until its ProcessKey() function
returns osEnd. Note that this would be like any other menu, and it would
run in the _foreground_ thread.

> Does vdr have a way to notice if a background thread accesses the
> OSD?

A background thread should never access the OSD directly.

> If so, and if this happens thru cSkins::Message() or cInterface::Confirm(),
> could vdr automatically handle this? Or just issue a warning
> like "You should use ::Popup()"

Again: if a plugin wants to interact with the user "out of the blue"
(meaning: not through the plugins main menu entry), cPlugin::Popup()
can either return a cOsdObject (which can be anything) that will be
further handled by VDR's main loop and will end when its ProcessKey()
function tells VDR to do so (or when the user hits kMenu - maybe there are even
other reasons), or it can do something like directly call Skins::Message()
and display a message (and return NULL).

But all this will be done from the _foreground_ thread, so any information
a plugin's _background_ thread wants to display needs to be stored in the
plugin somehow, until the plugin is asked for it by a call to its
Popup() function. I don't think that this will be much of a problem, because
typically such messages would be progress messages, like "processed x out of
y items", so the background thread could simply increment some counter and
the foreground thread could display the message whenever the difference
between the previous message and the current counter exceeds some limit.
OMHO there is most certainly no need for a "message queue" for background thread
messages.

Klaus
  

Stefan Lucke wrote:
> On Sonntag, 31. Juli 2005 16:56, Klaus Schmidinger wrote:
> ...
>>I'm thinking about a function like
>>
>>   cOsdObject *cPlugin::Popup(void);
>>
>>which would be called in every turn of the main loop (i.e. from the
>>foreground thread) whenever there is no OSD display open. A plugin
>>could either return a cOsdObject that will be presented to the user,
>>or could directly display a message (even with user interaction, if
>>necessary) and retrun NULL. Of course, if a plugin's thread wants to
>>issue a message, it will have to store the message inside the plugin
>>and wait until asked through a call to its Popup() function.
>>
>>Would that be a useful solution?
>>
> 
> 
> In some way yes, as it is something like InteractiveHousekeeping().
> But that would introduce message queueing code into each plugin, which
> wants to display a message. The condition "whenever there is no OSD
> display open" will cause an undetermined delay. "normal" OSDs have
> an area where such messages could be displayed.
> I would prefer something like Skins.QueueMessage() so that there is
> only one peace of code and each plugin can use it.
> For more complex OSDs (interactive ones) Popup() is more suitable:
> "You've new mail: read now (y (green) / n (red)) ?"

Why would you want to queue such messages?

If I understand this correctly, the original purpose of this was to
display some sort of progress indication. So, if a plugins's background
thread has something to say (like "processed x out of y items"), it can
do so at any time. There's no need to queue all messages, like

   processed 10 out of 1000 items
   processed 20 out of 1000 items
   processed 30 out of 1000 items
   processed 40 out of 1000 items
   processed 50 out of 1000 items
   processed 60 out of 1000 items
   processed 70 out of 1000 items
   processed 80 out of 1000 items

If VDR's main loop decides to ask the plugin whether it has something to say,
it doesn't really matter whether this is

   processed 20 out of 1000 items

or
   processed 70 out of 1000 items

The key point here is that the background thread does _not_ direcly display
a message. It rather keeps some indication of its current state, and whenever
suitable, the foreground thread will be asked to display it.

Klaus
  

Luca Olivetti wrote:
> Klaus Schmidinger wrote:
> 
>> A plugin may well call Skins.Message(), but only from the _main_
>> thread.
>>
>> I'm thinking about a function like
>>
>>   cOsdObject *cPlugin::Popup(void);
>>
>> which would be called in every turn of the main loop (i.e. from the
>> foreground thread) whenever there is no OSD display open. A plugin
>> could either return a cOsdObject that will be presented to the user,
> 
> 
> In this case the function wouldn't be called again, would it?

The Popup() function would be called whenever there is nothing else on
teh OSD. If it returns a cOsdObject, then that object's ProcessKey()
function will be called to actually process the display, and while doing
so the Plugin() function would, of course, not be called, since there
is already an object on the OSD.

> Isn't it possible to use Skins.Message even with an osd already open?
> Maybe
> 
> cOsdObject *cPlugin::Popup(bool canpopup)
> 
> called regardless of an osd open (and canpopup set to false if the 
> plugin cannot create a cOsdObject)?

No. Such messages from plugins come "out of the blue", so if there
is currently a menu open, the user shouldn't be irritated by messages
that don't have anything to do with this menu.

>> or could directly display a message (even with user interaction, if
>> necessary) and retrun NULL. Of course, if a plugin's thread wants to
>> issue a message, it will have to store the message inside the plugin
>> and wait until asked through a call to its Popup() function.
>>
>> Would that be a useful solution?
> 
> 
> I think it would be useful. For example, I could use it in my actuator 
> plugin to show the dish position instead of patching vdr as I do now, 
> and as soon as the user presses a button I could close my cOsdObject and 
> reinject the key to vdr.
> But it's not a complete solution, what if more than one plugin needs to 
> show something on screen? I reckon there's no simple solution to that, 
> but maybe the solution would be implementing something like the ttxsubs 
> or subtitles patches do (something like more than one osd opened at 
> once, with only the highest priority one actually shown), or some kind 
> of windowing system...I really don't know.

Only one plugin's Popup() function will be called at a time, and if that
plugin decides to display something, it will have the OSD for itself
until it gives it up (or VDR closes it for other reasons).  The subtitle
plugins should be able to use this mechanism just as well.

Klaus
  

On Sonntag 31 Juli 2005 18:29, Klaus Schmidinger wrote:
> > Does vdr have a way to notice if a background thread accesses the
> > OSD?
> 
> A background thread should never access the OSD directly.

I do understand that. I just think it might be helpful to not
simply say it is forbidden but to enforce it instead of
segfaulting.

Considering the enormous amount of plugins there will always be
some plugin developers trying to do forbidden things and if a
user tells them "sometimes vdr restarts but I have no idea why"
it is almost impossible to debug that.

If you take my original two patches you could do

 void cSkinClassicDisplayMessage::Flush(void)
 {
  if (osd)
         osd->Flush();
  else
	     warn: background thread is not allowed to access osd
 }



Otherwise I believe I understand what you propose and it seems
to be a good solution to me.
  

Wolfgang Rohdewald wrote:
> On Sonntag 31 Juli 2005 18:29, Klaus Schmidinger wrote:
> 
>>>Does vdr have a way to notice if a background thread accesses the
>>>OSD?
>>
>>A background thread should never access the OSD directly.
> 
> 
> I do understand that. I just think it might be helpful to not
> simply say it is forbidden but to enforce it instead of
> segfaulting.
> 
> Considering the enormous amount of plugins there will always be
> some plugin developers trying to do forbidden things and if a
> user tells them "sometimes vdr restarts but I have no idea why"
> it is almost impossible to debug that.
> 
> If you take my original two patches you could do
> 
>  void cSkinClassicDisplayMessage::Flush(void)
>  {
>   if (osd)
>          osd->Flush();
>   else
> 	     warn: background thread is not allowed to access osd
>  }

You don't _know_ that this is because of a background thread.
Besides, I'd hate to have to check 'osd' all over the place
(and every plugin that implements a skin would have to do the same).

I guess I'll simply put a test into cSkinDisplay::cSkinDisplay()
that aborts in case it gets called from a thread other than the
foreground thread.

> Otherwise I believe I understand what you propose and it seems
> to be a good solution to me.

Ok, so I'll implement it that way.

Klaus
  

Klaus Schmidinger wrote:
> Wolfgang Rohdewald wrote:
> 
>> On Sonntag 31 Juli 2005 18:29, Klaus Schmidinger wrote:
>>
>>>> Does vdr have a way to notice if a background thread accesses the
>>>> OSD?
>>>
>>>
>>> A background thread should never access the OSD directly.
>>
>>
>>
>> I do understand that. I just think it might be helpful to not
>> simply say it is forbidden but to enforce it instead of
>> segfaulting.
>>
>> Considering the enormous amount of plugins there will always be
>> some plugin developers trying to do forbidden things and if a
>> user tells them "sometimes vdr restarts but I have no idea why"
>> it is almost impossible to debug that.
>>
>> If you take my original two patches you could do
>>
>>  void cSkinClassicDisplayMessage::Flush(void)
>>  {
>>   if (osd)
>>          osd->Flush();
>>   else
>>          warn: background thread is not allowed to access osd
>>  }
> 
> 
> You don't _know_ that this is because of a background thread.
> Besides, I'd hate to have to check 'osd' all over the place
> (and every plugin that implements a skin would have to do the same).
> 
> I guess I'll simply put a test into cSkinDisplay::cSkinDisplay()
> that aborts in case it gets called from a thread other than the
> foreground thread.
> 
>> Otherwise I believe I understand what you propose and it seems
>> to be a good solution to me.
> 
> 
> Ok, so I'll implement it that way.
> 
> Klaus

Well, after a weekend of consideration I decided to implement
a function that can be used by background threads to queue a
message for display (the thread can even wait for user input)
in VDR 1.3.37. Since a plugin's MainMenuFunction can
now (since VDR 1.3.32) be launched programmatically, a plugin that
wants to do more than just display a message can always launch
itself and interact with the user in the foreground.

I've abandoned the idea of implementing something like cPlugin::Popup()
because it would only make things very complicated.

Since handling subtitles shall be made an integral part of the
core VDR code in version 1.5.x I'll postpone all further OSD
handling considerations until then.

Klaus
  

En/na Klaus Schmidinger ha escrit:
> 
> Well, after a weekend of consideration I decided to implement
> a function that can be used by background threads to queue a
> message for display (the thread can even wait for user input)
> in VDR 1.3.37. Since a plugin's MainMenuFunction can
> now (since VDR 1.3.32) be launched programmatically, a plugin that
> wants to do more than just display a message can always launch
> itself and interact with the user in the foreground.

It isn't working really well for my use case (dish position): while the
dish is moving I'm trying to call QueueMessage either with Seconds=0 and
Seconds=-1. Timeout is -1 (so that vdr should display the message no
matter what). Instead of a steady message with the position updating in
real time I just see a flash with the (same) position for a couple of
messages, then the message disappears to show a new one with a new position.

> 
> I've abandoned the idea of implementing something like cPlugin::Popup()
> because it would only make things very complicated.

this one would have worked :-(

Bye
  

En/na Luca Olivetti ha escrit:
> It isn't working really well for my use case (dish position): while the
> dish is moving I'm trying to call QueueMessage either with Seconds=0 and
> Seconds=-1. Timeout is -1 (so that vdr should display the message no
> matter what). Instead of a steady message with the position updating in
> real time I just see a flash with the (same) position for a couple of
> messages, then the message disappears to show a new one with a new position.

that should read "for a couple of seconds".

Bye