[vdr] Re: Fixed tuner lock detection

[Klaus Schmidinger <Klaus.Schmidinger@cadsoft.de>]

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_ ;-)
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.

Klaus