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

[Tako Schotanus]

Kenneth Aafløy wrote:

>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?
>  
>
Sorry, have no idea about the rationale for making it a compile time option.

>  
>
>>>>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.
>
Seems like a sound idea.

> 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.
>  
>
Is it that much data?

>  
>
>>>>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.
>  
>
Ok, so does this mean that it does this as well when the OSD (with 
postition) is on-screen?

>  
>
>>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.
>
>  
>
It was more a rethorical question ;-)

A lot of LCD displays don't even support per-pixel progress bars so they 
would see even less, but imagine a 500 pixel wide display (which is very 
large) and a 15 minute program (900 seconds): 900 / 500 = 1.8. So a 
display that wide with a very short program would only need to be 
updated every 2 seconds to have a very precise progress bar.  A one hour 
long program with a 200 pixel display needs an update only every 18 
seconds. Of course you could do this calculation for each program but 
that's overkill, let's just say that updating once a second is more than 
enough for the lcd IMO.

>>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
>
>  
>
-Tako