Opened 18 years ago

Closed 18 years ago

Last modified 18 years ago

#1502 closed patch (invalid)

DVB API requires usleep after certain calls to get reliable tuning on certain drivers (esp Nova-T).

Reported by: dscoular@… Owned by: danielk
Priority: minor Milestone: unknown
Component: dvb Version:
Severity: medium Keywords: DVB Nova-T
Cc: Ticket locked: no

Description

Hi Danielk et al,

The dvb api v3 authors contend that certain api calls require a usleep to wait for the low-level driver to complete the operation. They also hint that the dvb api event mechanism is flawed and shouldn't really be relied upon.

This is, indeed, a sorry state of affairs for would-be dvb api users.

Manu Abraham, of linuxtv fame, suggests that user-space applications mimic the techniques used by the linuxtv-apps util/scan/scan.c which works across the broadest range of supported cards.

1470 if (ioctl(frontend_fd, FE_SET_FRONTEND, &p) == -1) {
1471   errorn("Setting frontend parameters failed");
1472   return -1;
1473 }
1474
1475 for (i = 0; i < 10; i++) {
1476   usleep (200000);
1477
1478   if (ioctl(frontend_fd, FE_READ_STATUS, &s) == -1) {
1479     errorn("FE_READ_STATUS failed");
1480     return -1;
1481   }
1482
1483   verbose(">>> tuning status == 0x%02x\n", s);
1484
1485   if (s & FE_HAS_LOCK) {
1486     t->last_tuning_failed = 0;
1487     return 0;
1488   }
1489 }
1490
1491 warning(">>> tuning failed!!!\n");
1492

My interpretation of the above is that we should implement a usleep(200000) after a FE_SET_FRONTEND and FE_READ_STATUS to mimic the core scan.c behaviour.

Daniel suggested that this should be optional and the usleep configurable. Perhaps, combined with the detection of the card type. However, I would suggest that since more than just the Nova-T may be affected here, coupled with the fact that the frontend "name" field varies between revisions of the actual Nova-T hardware - that making this specific to the "DiBcom 3000P/M-C DVB-T" might be a bad idea.

Especially, since Manu's point is that scan.c has to work across the broadest range of cards. All DVB driver authors will be primarily testing their implementations using the scan.c code.

So if mythtv uses this dumb usleep method it has the best chance of working against the widest range of dvb drivers.

Contrary to the above, my first attempt at a patch uses a hardwired usleep of 200000 microseconds and only operates when the dbi api card name is "DiBcom 3000P/M-C DVB-T". Instead I think it could probably use the "channel_timeout" from mythtv-setup/videosource.cpp and work across all cards IMHO.

Anyway, here's the initial, possibly, dumb patch...

Index: libs/libmythtv/dvbsignalmonitor.h
===================================================================
--- libs/libmythtv/dvbsignalmonitor.h   (revision 9336)
+++ libs/libmythtv/dvbsignalmonitor.h   (working copy)
@@ -48,6 +48,10 @@
     int GetDVBCardNum(void) const;

     bool SupportsTSMonitoring(void);
+
+    DVBChannel *GetDVBChannel(void) const { return(channel); };
+    void SetDVBChannel(DVBChannel *_channel) { channel = _channel; };
+
   protected:
     SignalMonitorValue signalToNoise;
     SignalMonitorValue bitErrorRate;
@@ -58,6 +62,7 @@
     pthread_t          table_monitor_thread;

     FilterMap          filters; ///< PID filters for table monitoring
+    DVBChannel         *channel;
 };

 #endif // DVBSIGNALMONITOR_H
Index: libs/libmythtv/dvbchannel.cpp
===================================================================
--- libs/libmythtv/dvbchannel.cpp       (revision 9336)
+++ libs/libmythtv/dvbchannel.cpp       (working copy)
@@ -633,6 +633,11 @@
                   "Setting Frontend tuning parameters failed.");
             return false;
         }
+
+       // Special case for Nova-T. Should use configurable timeout.
+       if (GetFrontendName() == "DiBcom 3000P/M-C DVB-T") {
+         usleep(2000000);
+       }
         wait_for_backend(fd_frontend, 5 /* msec */);

         prev_tuning.params = params;
Index: libs/libmythtv/dvbsignalmonitor.cpp
===================================================================
--- libs/libmythtv/dvbsignalmonitor.cpp (revision 9336)
+++ libs/libmythtv/dvbsignalmonitor.cpp (working copy)
@@ -83,6 +83,8 @@
     QString msg = QString("DVBSignalMonitor(%1)::constructor(%2,%3): %4")
         .arg(channel->GetDevice()).arg(capturecardnum);

+    SetDVBChannel(_channel);
+
 #define DVB_IO(WHAT,WHERE,ERRMSG,FLAG) \
     if (ioctl(_channel->GetFd(), WHAT, WHERE)) \
         VERBOSE(VB_IMPORTANT, msg.arg(ERRMSG).arg(strerror(errno))); \
@@ -542,10 +544,16 @@
     uint32_t ber = 0, ublocks = 0;
     fe_status_t  status;
     bzero(&status, sizeof(status));
+    DVBChannel *channel = GetDVBChannel();

     // Get info from card
     int fd_frontend = channel->GetFd();
     ioctl(fd_frontend, FE_READ_STATUS, &status);
+    // Special case for Nova-T. Should use configurable timeout.
+    if (channel->GetFrontendName() == "DiBcom 3000P/M-C DVB-T") {
+      usleep(2000000);
+    }
+
     if (HasFlags(kDTVSigMon_WaitForSig))
         ioctl(fd_frontend, FE_READ_SIGNAL_STRENGTH, &sig);
     if (HasFlags(kDVBSigMon_WaitForSNR))

Also, note that since my C++ is incredibly lame and rusty that I put the setDVBChannel(_channel) into the DVBSignalMonitor constructor... this may not be an appropriate place for it... especially if the constructor is not called on each card change initiated via the 'Y' key.

For even more background on this ticket please refer to the rather lengthy discussion thread on mythtv-users:

http://www.gossamer-threads.com/lists/mythtv/users/187725#1state87725

AtDhVaAnNkCsE

Doug


"The big print giveth and the small print taketh away"

Attachments (3)

myth.diff (2.4 KB) - added by dscoular@… 18 years ago.
Patch to add usleep delay for reliable tuning on Nova-T DVB cards.
myth-diff-against-9403 (2.4 KB) - added by dscoular@… 18 years ago.
This updates dvbchannel.cpp to do an initial usleep after FE_READ_STATUS.
myth-diff-against-9403.2 (2.4 KB) - added by dscoular@… 18 years ago.
This updates dvbchannel.cpp to do an initial usleep after FE_READ_STATUS.

Download all attachments as: .zip

Change History (10)

Changed 18 years ago by dscoular@…

Attachment: myth.diff added

Patch to add usleep delay for reliable tuning on Nova-T DVB cards.

comment:1 Changed 18 years ago by anonymous

Component: mythtvdvb
Owner: changed from Isaac Richards to danielk

comment:2 Changed 18 years ago by danielk

Resolution: fixed
Status: newclosed

(In [9349]) Closes #1502. Adds a workaround for broken DVB drivers recommended by DVB guy Manu Abraham.

Basically, this adds a user specified hard delay after telling the driver what transport to tune to, but before we ask if it has finished tuning yet. This defaults to off, and is configurable in mythtv-setup's "Recording Options" pane for a DVB card.

If we detect a frontend name of "DiBcom? 3000P/M-C DVB-T" when initially setting up a DVB card, we auto-magically set this delay to two seconds.

"DiBcom? 3000P/M-C DVB-T" corresponds to a revision of the Hauppauge Nova-T with currently broken Linux DVB drivers. Kenneth Aafloy previously reported that this would be fixed in the 2.6.12 release of Linux, but since it hasn't been fixed yet and we are at 2.6.15.6 now I presuming that fixing the tuning bug for this card has a very low priority for the DVB driver folks.

comment:3 Changed 18 years ago by dscoular@…

Hi Daniel, Wow! that was speedy work... however I think there are a few problems with this patch.

1) the delay used by scan.c is 200000 microseconds which is equal to 0.20 seconds, not the

2 second delay you have set as the default.

2) You have made the usleep only apply if we detect a frontend name of "DiBcom 3000P/M-C DVB-T"

which ignores the fact that this usleep is a catch all for ALL cards. That being said, if you insist that it is conditional... can the DVBTuningDelay spinbox also be conditional (just curious really) ?

And finally just to really drive you nuts...

Having got reliable tuning on my Nova-T I decided to turn my attention to my AverMedia !AverTV DVB-T USB 2.0 card... and was pleased to find that after a history of never tuning on either 0.18.1 or 0.19 (but again perfect on tzap) two of my six channels were, indeed, tuning... so perhaps the timeout was working some magic there too.

However, the really interesting thing I found after spending 3 hours trying to get the other 4 channels working was that the values in dtv_multiplex were really screwed up when compared with any of my channels.conf scan.c based scans. When I manually fixed the dtv_multiplex to more accurately reflect the channels.conf output... tuning started working perfectly on all channels on both cards. It seems the Nova-T can handle more auto or incorrect settings than the !AverTV card... however, both did improve with the timeout code.

What I now want to do tonight (Sydney time) is rip out my usleep code and see how tuning fairs with more accurate dtv_multiplex data. The data had been a result of a fresh full scan ignoring timeouts. I'd also tried loading the channel.conf but that seemed to produce some bad data too. Tuning with known good data will let me really determine the value of the usleep code. I'm sorry I didn't realise that the dtv_multiplex data wasn't accurate as this sets everything in a new light.

I still think that Manu's point about mimicking scan.c standing the best chance of reliable

tuning across the broadest collection of cards, in spite of the dumbness of an API that allows this, is a fair and reasonable point. I'll be able to confirm or refute this tonight.

Anyway, thanks again for an amazing responsiveness in what must be a very frustrating component area (DVB). I'm sorry if I'm adding to those frustrations.

Cheers,

Doug

-- "The big print giveth and the small print taketh away"

comment:4 Changed 18 years ago by dscoular@…

Hi Daniel, I've found doing the usleep after FE_READ_STATUS improves my nova-t and avermedia a800 tuning reliability.

I've created a patch against revision 9403 and attached it here. As always, please check over my C++ as I'm not terribly confident. wait_for_backend is static so I had to modify it so I could pass in the tuning_delay. I'm not too clear on the pros and cons of static class function versus instance member method with respect to wait_for_backend... please QA.

Cheers,

Doug

Changed 18 years ago by dscoular@…

Attachment: myth-diff-against-9403 added

This updates dvbchannel.cpp to do an initial usleep after FE_READ_STATUS.

Changed 18 years ago by dscoular@…

Attachment: myth-diff-against-9403.2 added

This updates dvbchannel.cpp to do an initial usleep after FE_READ_STATUS.

comment:5 Changed 18 years ago by dscoular@…

Resolution: fixed
Status: closedreopened

Hi Daniel,

Oops accidently uploaded the same patch twice as myth-diff-against-9403 and myth-diff-against-9403.2. They are identical... apologies.

Doug -- "the big print giveth and the small print taketh away"

comment:6 Changed 18 years ago by danielk

Resolution: invalid
Status: reopenedclosed

I will address remainder of this problem in #1552.

comment:7 Changed 18 years ago by danielk

(In [10127]) Refs #1502. Refs #1552. Backport DVB tuning delay for broken DVB drivers.

This slows down tuning when you use the Nova-T or TerraTech? USB 2.0 DVB-T devices.

A more flexible version of this is in SVN head, but it requires a DB schema change. This instead hardcodes the two devices with known broken drivers.

The DVB folks do not intend to fix these bugs anytime soon, so we have to work around the bugs.

Note: See TracTickets for help on using tickets.