[mythtv] [mythtv-commits] Ticket #1502: DVB API requires usleep after certain calls to get reliable tuning on certain drivers (esp Nova-T).

[Daniel Kristjansson]

On Wed, 2006-03-15 at 22:05 +1100, Doug Scoular wrote:
>     In your code you are only doing a usleep after the FE_SET_FRONTEND
>     ioctl but my code does this after all FE_SET_FRONTEND and
>     FE_READ_STATUS ioctls. Apologies if I am misreading your
>     code. IMHO I think my implementation is a closer mimickery
>     of the scan.c code.
I believe the sleep after FE_READ_STATUS in scan is just to give the card 
some breathing room. We do the same thing in SignalMonitor::MonitorLoop()
   usleep(update_rate * 1000);

>     Your code fails to tune on my Nova-T for certain channels and
>     reports random noise for the BER and UNC on both my Nova-T
>     and AverTV A800 tuners. I also get a blank dialog window when
>     it fails to get lock after some other signal monitor timeout.
The BER and UNC are mostly meaningless the way we read them.
These are cumulative values, so we should throw out the first
read and we should also sum these over a time period and report
errors seen in that period, but we do neither. There is a comment
in DVBSignalMonitor::UpdateValues() about this...

We trust the FE_LOCK from the DVB drivers for tuning and these
values aren't anywhere near as useful as signal and s/n for
antenna adjustment so implementing the normalizing code is
pretty low priority for me.

>     I spent a while last night talking to the linuxtv guys and they are
>     adamant (remember Ant music ;) the example reference for the API
>     is linuxtv-apps and they still insist the usleep(200000) is a safety
>     margin which they have factored into the API design to allow the
>     driver writers a margin of error. The driver code already has
>     usleeps throughout it but this extra usleep allows them to tweak
>     the safety factor across all drivers and leave the kernel code
>     untouched. If you are doing FE_READ_STATUS without
>     waiting for their current safety margin of 200,000usecs then
>     you are likely to get random noise returned as values.
I think you may have misinterpreted what they said. A delay is needed
for the cumulative values to look less random, but you can and should
sum these values over a larger period than the between read delay.

>     Would you like me to produce a new prototype patch based on your
>     latest code ?
There is a constant in tv_rec.cpp:
/// How many milliseconds the signal monitor should wait between checks
const uint TVRec::kSignalMonitoringRate = 50; /* msec */
Set this to 250 milliseconds to simulate the code you had in
the original patch.

But you are talking about a huge cumulative delay here, I'm afraid you
might just be getting more successful tuning because you are sleeping
for hundreds of milliseconds. Perhaps this driver never reports anything
but "successfully tuned", so the delay is just waiting until this is
true in the general case, so even if you add all these delays it will
never tell you there is anything wrong with the signal which makes the
signal monitoring pretty useless on the whole...

>     Oh, and I had also removed the select(2) call in wait_for_backend
>     in dvbchannel.cpp (I think the usleep makes it unnecessary but I'm
>     not 100% sure).
It doesn't make it unnecessary for those of us not using the hack :]

>     P.S. I really like the way you did the mythtv-setup allowing
>     all card owners the ability to configure the usleep... nice.
I think it needs to be that way, a lot of MythTV users are
afraid to change a constant if it requires recompiling :)

>     Ooh... and the new fading transitions between mythtv
>     screens is just so nice!!! Ooh... and... ooh... ;^)
That's all Isaac :)

-- Daniel