[mythtv] [PATCH] Limit the updates for nonexisting LCD

[Kenneth Aafløy]

On Thursday 07 October 2004 16:59, Tako Schotanus wrote:
> Kenneth Aafløy wrote:
> > On Thursday 07 October 2004 14:15, Tako Schotanus wrote:
> > > Dan Morphis wrote:
> > > > If the LCD doesn't exist, then why not update your settings.pro
> > > > file and comment out the LCD stuff and recompile?
> >
> > That approach would not work, because then it still would be broken for
> > those that really use the LCD..also the code in question is not covered
> > by the LCD compile time configurable, did you read the patch and
> > sorrounding code?
>
> You're now responding to two people at the same time :-)
> I agree with you that this is not enough.

Also, after I had a look at this compile time option I must say I do not 
understand why this was even made a compile time option. I would say that 
what qualifies as a compile time option is a large part of code that is 
either very unstable and/or contains external dependecies that not every
user would want or need to have. LcdDevice is neither, right?

> > > He could do that but that doesn't change the fact that it should work,
> > > maybe not with a hasLCD() function but using the frontend settings where
> > > you can decide which info to show on the display. You should at least be
> > > able to disable the LCD by unchecking all those items.
> >
> >It still won't disable the code in question.
>
> True, what I meant with "should" is, that it "should have been
> implemented because the options to enable/disable it exist".

I would like to propose removing the LCD_DEVICE compile time option and also 
adding a master enable/disable switch for the lcddevice on the globalsettings 
screen, just in case a user have another daemon running on the port the 
lcdproc server is running on. I also would like to move most of the lcddevice 
data into a private data object that is created after the lcddevice has 
connected to the lcdproc server, to minimize the memory footprint this class 
represents if no lcdproc server is running.

> > > Strange thing is that I wrote most of that code and I thought that when
> > > Isaac checked it in he had changed it to make it update a lot less
> > > often, my original code would update it every second. Obviously I was
> > > wrong.
> >
> >The original loop goes something like this:
<snip>
> >So essentially this code is not only flodding the backend, but it's
> >essentially also blocking user interaction.
>
> The "pull lcd info from backend" part I don't understand, why do you
> think the backend is involved here?
>
> It shouldn't be if I understand and remember correctly. It's just a pointer
> to the local lcd device on the frontend as far as I know. And the info about
> the currently playing program was already available so calculating the
> position should not take any (extra) time.

If you follow the nvp->calcSliderPos() call you would discover that in two out 
of three cases this call requires a call to the backend. The only case this 
backend query would not happen is when you have access to the backend 
recording directory on your frontend, so that the ringbuffer can access a 
file directly. I do not have things setup that way here, so for me using 
MythTV will always go through the backend.

> That doesn't mean you are right in limiting the number of updates. I
> even think you haven't limited it enough because how much can the
> progress bar change in 1/4 of a second anyway?

I have no idea since I don't have access to a lcd and have not tried the 
curses based version of this since I don't plan on getting one.

> So as far as I'm concerned you can lower the frequency to once a second or
> even less. 

Excelent, could just merge both the checks into a single check here then.

Kenneth