vdr slowness when browsing channels

Message ID 42F5C202.50602@gmx.de
State New
Headers

Commit Message

Reinhard Nissl Aug. 7, 2005, 8:10 a.m. UTC
  Hi,

Guy Roussin wrote:

> I notice that vdr is slow when i try to quickly browse 
> channels. it eats around 18-20% of the cpu (atlon1200) when
> i hold left or right key when browsing channels (vdr 1.3.27 
> vanilla - classic skin) .
> With text2skin (1.0) and Enigma, it eats around 40-50% of
> the cpu and browsing channels is very very slow ...
> Note: cat channels.conf | wc -l  --> 2600 lines

Just give the attached patch a try and report the effect.

The problem seems to be, that for scrolling one line, all 2600 channels 
are sent to plugins. The patch changes this behaviour to just send those 
channels to plugins that will be shown on screen by VDR.

Other people say that this change is not good as some plugins are able 
to display more lines than VDR does and therefore they nolonger receive 
the necessary data.

I'd vote for a change in API where VDR just announces the lines it will 
display on screen and any plugin that needs more lines than that should 
be able to pull this data on its own.

Bye.
  

Comments

Klaus Schmidinger Aug. 7, 2005, 9:38 a.m. UTC | #1
Reinhard Nissl wrote:
> Hi,
> 
> Guy Roussin wrote:
> 
>> I notice that vdr is slow when i try to quickly browse channels. it 
>> eats around 18-20% of the cpu (atlon1200) when
>> i hold left or right key when browsing channels (vdr 1.3.27 vanilla - 
>> classic skin) .
>> With text2skin (1.0) and Enigma, it eats around 40-50% of
>> the cpu and browsing channels is very very slow ...
>> Note: cat channels.conf | wc -l  --> 2600 lines
> 
> 
> Just give the attached patch a try and report the effect.
> 
> The problem seems to be, that for scrolling one line, all 2600 channels 
> are sent to plugins. The patch changes this behaviour to just send those 
> channels to plugins that will be shown on screen by VDR.

These are only sent to plugins that actually implement a cStatus object.
Why would text2skin need a cStatus?

Klaus
  
Reinhard Nissl Aug. 7, 2005, 11:14 a.m. UTC | #2
Hi,

Klaus Schmidinger wrote:

>>> I notice that vdr is slow when i try to quickly browse channels. it 
>>> eats around 18-20% of the cpu (atlon1200) when
>>> i hold left or right key when browsing channels (vdr 1.3.27 vanilla - 
>>> classic skin) .
>>> With text2skin (1.0) and Enigma, it eats around 40-50% of
>>> the cpu and browsing channels is very very slow ...
>>> Note: cat channels.conf | wc -l  --> 2600 lines
>>
>> Just give the attached patch a try and report the effect.
>>
>> The problem seems to be, that for scrolling one line, all 2600 
>> channels are sent to plugins. The patch changes this behaviour to just 
>> send those channels to plugins that will be shown on screen by VDR.
> 
> These are only sent to plugins that actually implement a cStatus object.
> Why would text2skin need a cStatus?

 From the sources:
.
.
class cText2SkinStatus: public cStatus {
.
.

But it doesn't implement OsdItem(), so maybe my assumption was wrong.

Maybe it would make sense to have a second list of status monitors which 
contains all those cStatus objects which implement OsdItem() and to skip 
the loop completely if noone is interested this information.

What do you think about that, Klaus?
Shall I create a patch for this?

Bye.
  
Luca Olivetti Aug. 7, 2005, 11:15 a.m. UTC | #3
Klaus Schmidinger wrote:

> 
> These are only sent to plugins that actually implement a cStatus object.
> Why would text2skin need a cStatus?

And why would a cStatus object need to be resent all objects on the 
display even if none has changed?
Besides, just calling cStatus::MsgOsdItem should have some overhead.

Bye
  
Klaus Schmidinger Aug. 7, 2005, 1:08 p.m. UTC | #4
Reinhard Nissl wrote:
> Hi,
> 
> Klaus Schmidinger wrote:
> 
>>>> I notice that vdr is slow when i try to quickly browse channels. it 
>>>> eats around 18-20% of the cpu (atlon1200) when
>>>> i hold left or right key when browsing channels (vdr 1.3.27 vanilla 
>>>> - classic skin) .
>>>> With text2skin (1.0) and Enigma, it eats around 40-50% of
>>>> the cpu and browsing channels is very very slow ...
>>>> Note: cat channels.conf | wc -l  --> 2600 lines
>>>
>>>
>>> Just give the attached patch a try and report the effect.
>>>
>>> The problem seems to be, that for scrolling one line, all 2600 
>>> channels are sent to plugins. The patch changes this behaviour to 
>>> just send those channels to plugins that will be shown on screen by VDR.
>>
>>
>> These are only sent to plugins that actually implement a cStatus object.
>> Why would text2skin need a cStatus?
> 
> 
>  From the sources:
> .
> .
> class cText2SkinStatus: public cStatus {
> .
> .
> 
> But it doesn't implement OsdItem(), so maybe my assumption was wrong.
> 
> Maybe it would make sense to have a second list of status monitors which 
> contains all those cStatus objects which implement OsdItem() and to skip 
> the loop completely if noone is interested this information.
> 
> What do you think about that, Klaus?
> Shall I create a patch for this?

I don't think so.
Besides, OsdItem() and OsdCurrentItem() don't really play that well
together, anyway. As it is now, a cStatus object gets the full list
of all items when cOsdMenu::Display() is called, but the current
item is only referred to by its text, no index is given.
So I think this needs some more thought.

But let's first find out whether this is really the problem here.
On my system I also have a cStatus object in the RCU remote control,
and it also doesn't implement OsdItem(). Paging through the channel
list is not slow at all.

@Guy: please comment out the lines

        for (cOsdItem *item = First(); item; item = Next(item))
            cStatus::MsgOsdItem(item->Text(), ni++);

in cOsdMenu::Display() (osdbase.c) and let us know whether this
makes any change.

Klaus
  
Oliver Endriss Aug. 7, 2005, 1:26 p.m. UTC | #5
Klaus Schmidinger wrote:
> Reinhard Nissl wrote:
> > Hi,
> > 
> > Guy Roussin wrote:
> > 
> >> I notice that vdr is slow when i try to quickly browse channels. it 
> >> eats around 18-20% of the cpu (atlon1200) when
> >> i hold left or right key when browsing channels (vdr 1.3.27 vanilla - 
> >> classic skin) .
> >> With text2skin (1.0) and Enigma, it eats around 40-50% of
> >> the cpu and browsing channels is very very slow ...
> >> Note: cat channels.conf | wc -l  --> 2600 lines
> ...
> > The problem seems to be, that for scrolling one line, all 2600 channels 
> > are sent to plugins. The patch changes this behaviour to just send those 
> > channels to plugins that will be shown on screen by VDR.
> 
> These are only sent to plugins that actually implement a cStatus object.

Iirc that was a real performance issue on my old 200 MHz machine. :-(

Two possible solutions:
- The list of items should only be sent when the menu opens, not for
  each cursor movement.
- Only items at cursor_pos-N ... cursor_pos+N should be sent through
  cStatus. N = 50 or 100 should be sufficient.

Oliver
  
Oliver Endriss Aug. 7, 2005, 1:45 p.m. UTC | #6
Klaus Schmidinger wrote:
> Besides, OsdItem() and OsdCurrentItem() don't really play that well
> together, anyway. As it is now, a cStatus object gets the full list
> of all items when cOsdMenu::Display() is called, but the current
> item is only referred to by its text, no index is given.
> So I think this needs some more thought.

Yes, that's not optimal. At the moment a plugin has to do a string
search in the OsdItem list to find the index of OsdCurrentItem.
Works most of the time, but fails if there are two similar OsdItems,
for example in some setup menus.

It would be great, if the current index could be queried somehow or
would be passed as a parameter through OsdCurrentItem().

Oliver
  
Luca Olivetti Aug. 7, 2005, 2:01 p.m. UTC | #7
Klaus Schmidinger wrote:

> @Guy: please comment out the lines
> 
>        for (cOsdItem *item = First(); item; item = Next(item))
>            cStatus::MsgOsdItem(item->Text(), ni++);

didn't make a big difference, exactly nothing with the dxr3 plugin (cpu 
use around 100%, probably most of the cpu is used by the dxr3 osd 
routines) and around 5-7% with the xine plugin (cpu use something less 
that 60% without the cStatus::MsgOsdItem, around 65% with).
I'm using a duron at 1200Mhz, probably the difference would be bigger 
with a less powerful cpu.

Bye
  
Klaus Schmidinger Aug. 7, 2005, 2:05 p.m. UTC | #8
Luca Olivetti wrote:
> Klaus Schmidinger wrote:
> 
>> @Guy: please comment out the lines
>>
>>        for (cOsdItem *item = First(); item; item = Next(item))
>>            cStatus::MsgOsdItem(item->Text(), ni++);
> 
> 
> didn't make a big difference, exactly nothing with the dxr3 plugin (cpu 
> use around 100%, probably most of the cpu is used by the dxr3 osd 
> routines) and around 5-7% with the xine plugin (cpu use something less 
> that 60% without the cStatus::MsgOsdItem, around 65% with).
> I'm using a duron at 1200Mhz, probably the difference would be bigger 
> with a less powerful cpu.

So the cStatus thing isn't what's causing this.

Well, must be something else in text2skin then.
After all, this is a rather complex plugin that does a lot
of "eye candy" stuff - apparently beauty comes at a price... ;-)

Klaus
  
Luca Olivetti Aug. 7, 2005, 2:14 p.m. UTC | #9
Klaus Schmidinger wrote:

>> didn't make a big difference, exactly nothing with the dxr3 plugin 
>> (cpu use around 100%, probably most of the cpu is used by the dxr3 osd 
>> routines) and around 5-7% with the xine plugin (cpu use something less 
>> that 60% without the cStatus::MsgOsdItem, around 65% with).
>> I'm using a duron at 1200Mhz, probably the difference would be bigger 
>> with a less powerful cpu.
> 
> 
> So the cStatus thing isn't what's causing this.
> 
> Well, must be something else in text2skin then.
> After all, this is a rather complex plugin that does a lot
> of "eye candy" stuff - apparently beauty comes at a price... ;-)

Note that I did my tests with ST:TNG just to keep text2skin out of the 
picture ;-) but of course you're right: text2skin takes a noticeable hit 
on the cpu.
Anyway, even with enigma channel scrolling isn't slow with the xine 
plugin[*] but it is with the dxr3 (due to 100% cpu usage), so definitely 
cStatus isn't the cause (and I have ~4500 lines in channel.conf).
Since Guy's processor is comparable to mine (heck, his is an athlon, 
mine is a duron), there should be something else.
[*] with xine running on a different machine, probably running xine and 
vdr on the same machine would use too much cpu too.

Bye
  
Guy Roussin Aug. 9, 2005, 7:19 a.m. UTC | #10
Hi,

> @Guy: please comment out the lines
> 
>        for (cOsdItem *item = First(); item; item = Next(item))
>            cStatus::MsgOsdItem(item->Text(), ni++);
> 
> in cOsdMenu::Display() (osdbase.c) and let us know whether this
> makes any change.
No, that does not change anything. Around 20% of the cpu for vdr process
when browsing channels with left or right keys of my remote control.

I test this patch with vdr 1.3.27 and 1.3.28, same results.
I test with left and right keys (keyboard) and now the scroll is even
faster but it take around 35-40% of the cpu (with or without the patch).

With text2skin and Enigma again, i notice no changes with this patch :
40-50% of the cpu with remote and even 65-70% with the keyboard.

I note that browsing with the keyboard is very fast and no jerks as
with the remote control. It seems that some press on the remote control
are lost by vdr.


Thanks,

Guy
  

Patch

--- ../vdr-1.3.27-orig/osdbase.c	2005-06-18 12:30:51.000000000 +0200
+++ osdbase.c	2005-07-16 17:49:47.000000000 +0200
@@ -196,8 +196,11 @@  void cOsdMenu::Display(void)
   int count = Count();
   if (count > 0) {
      int ni = 0;
-     for (cOsdItem *item = First(); item; item = Next(item))
+     for (cOsdItem *item = Get(first); item; item = Next(item)) {
          cStatus::MsgOsdItem(item->Text(), ni++);
+         if (ni == displayMenuItems)
+            break;
+         }
      if (current < 0)
         current = 0; // just for safety - there HAS to be a current item!
      if (current - first >= displayMenuItems || current < first) {