ERROR: video data stream broken on second dvb card , but szap works (new findings!)

Message ID 45D83DB8.9030800@gmx.de
State New
Headers

Commit Message

Reinhard Nissl Feb. 18, 2007, 11:51 a.m. UTC
  Hi,

Dieter Bloms wrote:

> my primary dvb card works fine on both of my Twin-LNB connectors.
> I can switch the second card via szap and get a video stream via
> "cat /dev/dvb/adapter1/dvr0 > /tmp/bla" on H and V channels.
> VDR doesn't get data any data from H channels, but gets data from V
> channels.
> I will try to strace vdr and szap to get any difference, maybe they do
> it in a different way.

I've had a further look into szap's source how it detects the status
FE_LOCKED. Attached is an updated tuner patch which now also reports
details for FE_READ_STATUS.

One difference between VDR and szap regarding FE_READ_STATUS is, that
VDR only honors the read status when ioctl() returns 0 while szap prints
just an error when ioctl() returns -1.

Furthermore, VDR's handling of errno == EINTR seems to be wrong due to
the do {} while (0); loop.

BTW: I still assume, that your logfile reports a tuning timeout. If this
is no longer the case, then you may want to experiment with
WAIT_FOR_TUNER_LOCK in device.c.

Bye.
  

Comments

Ilariu Raducan Feb. 18, 2007, 3:20 p.m. UTC | #1
Hi,

I will ike accentuate the do {}while(0) behavior.
I've seen this in few projects including VDR and at least one plugin
(xineliboutput)
In a code like:
do {

    if (condition1)
        continue;
} while(condition2);
continue does not translate to a jump to "do", but to a jump to "while"
where the condition2 is evaluated.

do {
   if (condition1)
       continue;
} while(0);

if equivalent with:
do {
   if (condition1)
       break;
} while (0);

Regards,
Ilariu
On 2/18/07, Reinhard Nissl <rnissl@gmx.de> wrote:
>
> Hi,
>
> Dieter Bloms wrote:
>
> > my primary dvb card works fine on both of my Twin-LNB connectors.
> > I can switch the second card via szap and get a video stream via
> > "cat /dev/dvb/adapter1/dvr0 > /tmp/bla" on H and V channels.
> > VDR doesn't get data any data from H channels, but gets data from V
> > channels.
> > I will try to strace vdr and szap to get any difference, maybe they do
> > it in a different way.
>
> I've had a further look into szap's source how it detects the status
> FE_LOCKED. Attached is an updated tuner patch which now also reports
> details for FE_READ_STATUS.
>
> One difference between VDR and szap regarding FE_READ_STATUS is, that
> VDR only honors the read status when ioctl() returns 0 while szap prints
> just an error when ioctl() returns -1.
>
> Furthermore, VDR's handling of errno == EINTR seems to be wrong due to
> the do {} while (0); loop.
>
> BTW: I still assume, that your logfile reports a tuning timeout. If this
> is no longer the case, then you may want to experiment with
> WAIT_FOR_TUNER_LOCK in device.c.
>
> Bye.
> --
> Dipl.-Inform. (FH) Reinhard Nissl
> mailto:rnissl@gmx.de
>
> _______________________________________________
> vdr mailing list
> vdr@linuxtv.org
> http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr
>
>
>
  
Dieter Bloms Feb. 27, 2007, 4:29 p.m. UTC | #2
Hello Reinhard,

On Sun, Feb 18, Reinhard Nissl wrote:

> I've had a further look into szap's source how it detects the status
> FE_LOCKED. Attached is an updated tuner patch which now also reports
> details for FE_READ_STATUS.
> 
> One difference between VDR and szap regarding FE_READ_STATUS is, that
> VDR only honors the read status when ioctl() returns 0 while szap prints
> just an error when ioctl() returns -1.
> 
> Furthermore, VDR's handling of errno == EINTR seems to be wrong due to
> the do {} while (0); loop.
> 
> BTW: I still assume, that your logfile reports a tuning timeout. If this
> is no longer the case, then you may want to experiment with
> WAIT_FOR_TUNER_LOCK in device.c.

I've tried your patch and the new vdr-1.4.5-2 for some days now and
it works even when I disable diseqc !
So I have to say: greate work Reinhard and many many thanks to you !!!
  
Reinhard Nissl March 2, 2007, 8:11 p.m. UTC | #3
Hi,

Dieter Bloms wrote:

>> I've had a further look into szap's source how it detects the status
>> FE_LOCKED. Attached is an updated tuner patch which now also reports
>> details for FE_READ_STATUS.
> 
> I've tried your patch and the new vdr-1.4.5-2 for some days now and
> it works even when I disable diseqc !
> So I have to say: greate work Reinhard and many many thanks to you !!!

Would you mind posting some of the new log lines which the last patch
added? It would be good to prove my assumptions ;-)

BTW: vdr-1.5.1 uses a further "developed" loop (in regard the the last
patch) which is much closer to what szap does. The relevant part of
dvbdevice.c looks like that:

  while (1) {
        if (ioctl(fd_frontend, FE_READ_STATUS, &Status) != -1)
           return true;
        if (errno != EINTR)
           break;
        }
  return false;

Bye.
  

Patch

--- ../vdr-1.4.5-orig/dvbdevice.c	2006-08-14 11:38:32.000000000 +0200
+++ dvbdevice.c	2007-02-18 12:23:47.000000000 +0100
@@ -147,6 +147,15 @@  bool cDvbTuner::Locked(int TimeoutMs)
   return tunerStatus >= tsLocked;
 }
 
+#include <sys/time.h>
+static double now() { int e = errno; timeval t; gettimeofday(&t, 0); errno = e; return t.tv_sec + t.tv_usec / 1e6; }
+static int check(int c, int r, char *a, double t) { int e = errno; fprintf(stderr, "t: %.3lf, c: %d, r: %d, a: %s\n", t, c, r, a); errno = e; return r; } 
+static int check5(int status, int c, int r, char *a, double t) { int e = errno; fprintf(stderr, "t: %.3lf, c: %d, r: %d, a: %s => Status: %02x\n", t, c, r, a, status); errno = e; return r; } 
+#define CHECK5(s) ::check5(Status, cardIndex, s, #s, ::now())
+#define CHECK4(s) ::check(cardIndex, 0, s, ::now())
+#define CHECK3(s) ::check(cardIndex, s, #s, ::now())
+#define CHECK2(s) CHECK(::check(cardIndex, s, #s, ::now()))
+
 bool cDvbTuner::GetFrontendStatus(fe_status_t &Status, int TimeoutMs)
 {
   if (TimeoutMs) {
@@ -157,15 +166,16 @@  bool cDvbTuner::GetFrontendStatus(fe_sta
               ; // just to clear the event queue - we'll read the actual status below
         }
      }
-  do {
-     int stat = ioctl(fd_frontend, FE_READ_STATUS, &Status);
-     if (stat == 0)
-        return true;
-     if (stat < 0) {
-        if (errno == EINTR)
-           continue;
+  while (1) {
+        int stat = CHECK5(ioctl(fd_frontend, FE_READ_STATUS, &Status));
+        if (stat == 0)
+           return true;
+        if (stat < 0) {
+           if (errno == EINTR)
+              continue;
+           }
+        break;
         }
-     } while (0);
   return false;
 }
 
@@ -195,12 +205,12 @@  bool cDvbTuner::SetFrontend(void)
                   for (char *CurrentAction = NULL; (da = diseqc->Execute(&CurrentAction)) != cDiseqc::daNone; ) {
                       switch (da) {
                         case cDiseqc::daNone:      break;
-                        case cDiseqc::daToneOff:   CHECK(ioctl(fd_frontend, FE_SET_TONE, SEC_TONE_OFF)); break;
-                        case cDiseqc::daToneOn:    CHECK(ioctl(fd_frontend, FE_SET_TONE, SEC_TONE_ON)); break;
-                        case cDiseqc::daVoltage13: CHECK(ioctl(fd_frontend, FE_SET_VOLTAGE, SEC_VOLTAGE_13)); break;
-                        case cDiseqc::daVoltage18: CHECK(ioctl(fd_frontend, FE_SET_VOLTAGE, SEC_VOLTAGE_18)); break;
-                        case cDiseqc::daMiniA:     CHECK(ioctl(fd_frontend, FE_DISEQC_SEND_BURST, SEC_MINI_A)); break;
-                        case cDiseqc::daMiniB:     CHECK(ioctl(fd_frontend, FE_DISEQC_SEND_BURST, SEC_MINI_B)); break;
+                        case cDiseqc::daToneOff:   CHECK2(ioctl(fd_frontend, FE_SET_TONE, SEC_TONE_OFF)); break;
+                        case cDiseqc::daToneOn:    CHECK2(ioctl(fd_frontend, FE_SET_TONE, SEC_TONE_ON)); break;
+                        case cDiseqc::daVoltage13: CHECK2(ioctl(fd_frontend, FE_SET_VOLTAGE, SEC_VOLTAGE_13)); break;
+                        case cDiseqc::daVoltage18: CHECK2(ioctl(fd_frontend, FE_SET_VOLTAGE, SEC_VOLTAGE_18)); break;
+                        case cDiseqc::daMiniA:     CHECK2(ioctl(fd_frontend, FE_DISEQC_SEND_BURST, SEC_MINI_A)); break;
+                        case cDiseqc::daMiniB:     CHECK2(ioctl(fd_frontend, FE_DISEQC_SEND_BURST, SEC_MINI_B)); break;
                         case cDiseqc::daCodes: {
                              int n = 0;
                              uchar *codes = diseqc->Codes(n);
@@ -208,7 +218,7 @@  bool cDvbTuner::SetFrontend(void)
                                 struct dvb_diseqc_master_cmd cmd;
                                 memcpy(cmd.msg, codes, min(n, int(sizeof(cmd.msg))));
                                 cmd.msg_len = n;
-                                CHECK(ioctl(fd_frontend, FE_DISEQC_SEND_MASTER_CMD, &cmd));
+                                CHECK2(ioctl(fd_frontend, FE_DISEQC_SEND_MASTER_CMD, &cmd));
                                 }
                              }
                              break;
@@ -285,7 +295,7 @@  bool cDvbTuner::SetFrontend(void)
          esyslog("ERROR: attempt to set channel with unknown DVB frontend type");
          return false;
     }
-  if (ioctl(fd_frontend, FE_SET_FRONTEND, &Frontend) < 0) {
+  if (CHECK3(ioctl(fd_frontend, FE_SET_FRONTEND, &Frontend)) < 0) {
      esyslog("ERROR: frontend %d: %m", cardIndex);
      return false;
      }
@@ -306,11 +316,13 @@  void cDvbTuner::Action(void)
           case tsIdle:
                break;
           case tsSet:
+CHECK4("----------------------------------------------");
                tunerStatus = SetFrontend() ? tsTuned : tsIdle;
                Timer.Set(tuneTimeout);
                continue;
           case tsTuned:
                if (Timer.TimedOut()) {
+CHECK4("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~");
                   tunerStatus = tsSet;
                   diseqcCommands = NULL;
                   if (time(NULL) - lastTimeoutReport > 60) { // let's not get too many of these
@@ -328,6 +340,7 @@  void cDvbTuner::Action(void)
                   continue;
                   }
                else if (Status & FE_HAS_LOCK) {
+if (tunerStatus != tsLocked) CHECK4("==============================================");
                   if (LostLock) {
                      isyslog("frontend %d regained lock on channel %d, tp %d", cardIndex, channel.Number(), channel.Transponder());
                      LostLock = false;