[vdr] Re: Fixed tuner lock detection

[Wolfgang Fritz <wolfgang.fritz@gmx.net>]

Klaus Schmidinger wrote:
> Wolfgang Fritz wrote:
> 
>>Klaus Schmidinger wrote:
>>
>>>Wolfgang Fritz wrote:
>>>
>>>
>>>>...
>>>>
>>>>
>>>>>        if (tunerStatus != tsIdle) {
>>>>>           dvb_frontend_event event;
>>>>>-           if (ioctl(fd_frontend, FE_GET_EVENT, &event) == 0) {
>>>>>-              if (event.status & FE_REINIT) {
>>>>>-                 tunerStatus = tsSet;
>>>>>-                 esyslog("ERROR: frontend %d was reinitialized - re-tuning", cardIndex);
>>>>>+           while (ioctl(fd_frontend, FE_GET_EVENT, &event) == 0) {
>>>>>+                 if (event.status & FE_REINIT) {
>>>>>+                    tunerStatus = tsSet;
>>>>>+                    esyslog("ERROR: frontend %d was reinitialized - re-tuning", cardIndex);
>>>>>+                    }
>>>>>+                 if (event.status & FE_HAS_LOCK) {
>>>>
>>>>Question 1: Are you sure the drivers always signal a valid lock on the
>>>>first "rising edge" of FE_HAS_LOCK? They didn't in the past (the "spike
>>>>problem").
>>>
>>>
>>>Well, if the driver sends an event that indicates FE_HAS_LOCK I'd say
>>>it's ok for the application to assume that there actually _is_ a lock.
>>>Anyting else is a driver bug and should be fixed there. IIRC there was
>>>a fix in the driver for this recently.
>>>
>>
>>OK, maybe I'm too paranoid here. But looking the increasing number of
>>supported frontends, the different "flavours" of the DVB drivers (2.4 /
>>2.6 kernel, CVS...) I find it quite hard to believe that all of them are
>>bug free.
> 
> 
> Well, then they should be fixed - it can't be a serious way to have _all_
> applications work around driver bugs.
> 
> 
>>>The problem in the old code in VDR was that it has fetched the current
>>>status with ioctl(fd_frontend, FE_READ_STATUS, &status) instead of reacting
>>>on FE events. My tests showed that after tuning to a different transponder,
>>>the FE_READ_STATUS immediately returned FE_HAS_LOCK, while the actual
>>>event arrived somewhat later. I guess the FE_READ_STATUS call (sometimes)
>>>delivered the status of the _previous_ lock.
>>>
>>
>>I observed that too, but in my opinion the FE_GET_EVENT was also wrong.
>>This was the same bug which showed the FE_HAS_LOCK spikes (abt 100 ms)
>>when switching from a locked channel to channel that could not lock
>>(e.g. the "channel 0"). When switching from a locked channel to a valid
>>channel, this wrong FE_HAS_LOCK was combined with the valid FE_HAS_LOCK.
> 
> 
> As far as I have observed, FE_READ_STATUS has always immediately returned
> FE_HAS_LOCK, even if FE_GET_EVENT took several loops to do so. So my
> interpretaton of this is that FE_GET_EVENT is the right way to go. But
> once FE_GET_EVENT returns FE_HAS_LOCK, it must be ok for the application
> to set filters etc. After all it says _HAS_ lock, and not
> I_THINK_I_MIGHT_HAVE_A_LOCK_BUT_I_M_NOT_SURE_YET_ ;-)

FE_HAS_LOCK _may_ mean I_HAVE_LOCK_NOW_BUT_WHO_KNOWS_WHAT_IS_200MS_LATER
when the signal conditions are marginal. With the current
implementation, one "rising edge" of FE_HAS_LOCK signals a good locking
condition until you retune to another channel. I never trust edges on
level based signals. A little debouncing never hurts. OK, you may argue
that it has to be done in the drivers.

> If the "spikes" you have observed appear, this is definitely a driver
> bug and must be fixed there.
> 

> 
>>>>Question 2: How is the case of a loss of lock handled (May occur e.g. in
>>>>bad weather conditions?). This case is not handled in
>>>>DvbTuner::Action(). Do we get a VDSB error and an emergency exit?
>>>
>>>
>>>There hasn't been any special code regarding loss of lock so far, and this
>>>modification doesn't introduce one, either. I did try to do FE_READ_STATUS
>>>in every loop and wanted to retune if the lock was lost, but apparently
>>>the FE_HAS_LOCK sometimes gets 0 even though there is a perfect lock.
>>>Maybe this is caused by the problem with the ves1x93, which sometimes
>>>returns bogus status values.
>>>
>>
>>Mmh, but this is a driver bug and should be fixed in the driver,
>>shouldn't it ;-)
> 
> 
> Yes, and it already _is_ fixed there ;-) But maybe only for driver
> internal re-tuning. Maybe the value used for generating events still
> is the one having bogus bit settings. You might want to look into the
> ves1x93.c code to check if the workaround I have built in there is also
> necessary some place else.
> 
> 
>>Anyway, FYI I attach the version of dvbdevice.c I'm currently running.
>>It has a a lock/loss of lock detection which suppresses short glitches.
>>The diff is against 1.3.12.
> 
> 
> If this is the only difference to what I have posted, this should be
> fixed in the driver.
> 
> 
>>>Anyway, if there is no more data while a recording is going on, there
>>>will still be an emergency exit. This fix here is only supposed to prevent
>>>the DVB card from getting into an unstable condition by starting filters
>>>before there is an actual lock.
>>>
>>
>>Would it be possible to introduce a "mild form" of emergency exit, say,
>>if a recording gets no more data, it declares an emergency, but instead
>>of triggering an immediate restart, check if there are other recordings
>>running without problems (on other cards maybe) and delay the restart
>>until these recordings have finished? Unfortunately I'm not familiar
>>enough with the VDR code and internals yet to test this.
> 
> 
> I believe this makes things only unnecessarily complex.
> What we really should focus on is to avoid this situation in the
> first place. The modification to VDR's tuning code is certainly
> a step in the riht direction. Now if the driver delivers "spikes"
> of FE_HAS_LOCK, these must be fixed in the driver.
> 

I dont' focus on driver bugs here but valid reasons for a loss of lock
(marginal receiving conditions, faulty cable etc.)

Two real scenarios I've experienced myself:

1. One recording running on card 1, another one on card 2. Cable to card
2 interrupted accidently by user (me).
Result: emergency exit, recording on working card 1 corrupted unnecessarily.

2. I took my VDR to a friend for a demonstration who has only on cable
available. During my demonstration, a timer started a recording on the
card without cable.
Result: Repeating emergency exits, VDR unusable until I managed to kill
the offending timer via the RC after several tries (a 20 second timeout
is sometimes too short). My friend was not impressed.

But at least the change in cDvbDevice::SetChannelDevice() should prevent
the repeating restarts and emergency exits in this case as the recording
on the faulty channel wouldn't be restarted again. This is definitely an
improvement.

Wolfgang

> Klaus
> 
>