Opened 12 years ago

Closed 10 years ago

#3666 closed defect (fixed)

DVD speed setting only works when using media monitoring

Reported by: anonymous Owned by: Isaac Richards
Priority: minor Milestone: unknown
Component: mythtv Version: head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

The changes in #3379 appear to have broken DVD speed setting when "Monitor CD / DVD" is not checked. I assume this is because MediaMonitor::SetCDSpeed uses MediaMonitor to set the speed.

Change History (9)

comment:1 Changed 12 years ago by Nigel

Untested quick patch for this, and #3662:

% svn diff
Index: mythmediamonitor.h
===================================================================
--- mythmediamonitor.h  (revision 13796)
+++ mythmediamonitor.h  (working copy)
@@ -97,7 +97,12 @@
     QValueList<MythMediaDevice*> m_RemovedDevices;
     QMap<MythMediaDevice*, int>  m_UseCount;
 
-    bool                         m_Active;
+    /// List of devices/mountpoints that the user doesn't want to monitor:
+    QStringList                  m_IgnoreList;
+
+    bool                         m_Active;    ///> After StartMonitoring()
+    bool                         m_SendEvent; ///> Should we postEvent()?
+
     MonitorThread                *m_Thread;
     unsigned long                m_MonitorPollingInterval;
     bool                         m_AllowEject;
Index: mediamonitor-darwin.cpp
===================================================================
--- mediamonitor-darwin.cpp     (revision 13796)
+++ mediamonitor-darwin.cpp     (working copy)
@@ -489,9 +489,16 @@
     /// Devices on Mac OS X don't change status the way Linux ones do,
     /// so we need a different way to inform plugins that a new drive
     /// is available. I think this leaks a little memory?
-    QApplication::postEvent((QObject*)gContext->GetMainWindow(),
-                            new MediaEvent(MEDIASTAT_USEABLE, pDevice));
+    if (m_SendEvent)
+        QApplication::postEvent((QObject*)gContext->GetMainWindow(),
+                                new MediaEvent(MEDIASTAT_USEABLE, pDevice));
 
+
+    // If the user doesn't want this device to be monitored, stop now:
+    if (m_IgnoreList.contains(pDevice->getMountPath()) ||
+        m_IgnoreList.contains(pDevice->getDevicePath()) )
+        return false;
+
     m_Devices.push_back( pDevice );
     m_UseCount[pDevice] = 0;
 
Index: mythmediamonitor.cpp
===================================================================
--- mythmediamonitor.cpp        (revision 13796)
+++ mythmediamonitor.cpp        (working copy)
@@ -251,11 +251,29 @@
     }
 }
 
+/**
+ * \brief    Lookup some settings, and do OS-specific stuff in sub-classes
+ *
+ * \warning  If the user changes the MonitorDrives or IgnoreDevices settings,
+ *           it will have no effect until the frontend is restarted.
+ */
 MediaMonitor::MediaMonitor(QObject* par, unsigned long interval, 
                            bool allowEject) 
     : QObject(par), m_Active(false), m_Thread(NULL),
       m_MonitorPollingInterval(interval), m_AllowEject(allowEject)
 {
+    // Devices are now always monitored,
+    // but by default we don't send status changed events:
+    m_SendEvent = gContext->GetNumSetting("MonitorDrives");
+
+
+    // User can specify that some devices are not monitored
+    QString ignore = gContext->GetSetting("IgnoreDevices", "");
+
+    if (ignore.length())
+        m_IgnoreList = QStringList::split(',', ignore);
+    else
+        m_IgnoreList = QStringList::QStringList();  // Force empty list
 }
 
 MediaMonitor::~MediaMonitor()
@@ -479,7 +497,7 @@
                                       MythMediaDevice* pMedia)
 {
     // If we're not active then ignore signal.
-    if (!m_Active)
+    if (!m_Active || !m_SendEvent)
         return;
     
     VERBOSE(VB_IMPORTANT, QString("Media status changed...  New status is: %1 "
Index: mediamonitor-unix.cpp
===================================================================
--- mediamonitor-unix.cpp       (revision 13796)
+++ mediamonitor-unix.cpp       (working copy)
@@ -290,7 +290,9 @@
     device->setDeviceModel(desc);
 }
 
-// Given a media deivce add it to our collection.
+/**
+ * Given a media device, add it to our collection
+ */
 bool MediaMonitorUnix::AddDevice(MythMediaDevice* pDevice)
 {
     if ( ! pDevice )
@@ -299,6 +301,11 @@
         return false;
     }
 
+    // If the user doesn't want this device to be monitored, stop now:
+    if (m_IgnoreList.contains(pDevice->getMountPath()) ||
+        m_IgnoreList.contains(pDevice->getDevicePath()) )
+        return false;
+
     dev_t new_rdev;
     struct stat sb;
 
Index: mediamonitor-windows.cpp
===================================================================
--- mediamonitor-windows.cpp    (revision 13796)
+++ mediamonitor-windows.cpp    (working copy)
@@ -48,6 +48,24 @@
 }
 
 
+bool MediaMonitorWindows::AddDevice(MythMediaDevice* pDevice)
+{
+    if (! pDevice)
+    {
+        VERBOSE(VB_IMPORTANT, "Error - MediaMonitor::AddDevice(null)");
+        return false;
+    }
+
+    // If the user doesn't want this device to be monitored, stop now:
+    if (m_IgnoreList.contains(pDevice->getDevicePath()))
+        return false;
+
+    m_Devices.push_back(pDevice);
+    m_UseCount[pDevice] = 0;
+
+    return true;
+}
+
 QStringList MediaMonitorWindows::GetCDROMBlockDevices()
 {
     QStringList  list;

comment:2 Changed 12 years ago by Nigel

Resolution: fixed
Status: newclosed

(In [13882]) Always create MediaMonitor?, but only send events if needed. Closes #3666

comment:3 Changed 10 years ago by Nigel

Resolution: fixed
Status: closednew

It seems that the problem still exists. My changes in [13882] allowed the media monitor to be on, but inactive (i.e. sending no events). Initial examination of MediaMonitor::SetCDSpeed() reveals that setSpeed() is only called if the MediaMonitor? knows about the device, which will not be the case if "Monitor CD/DVD" is off.
SetCDSpeed() could have a fallthru for both ifs, and call a static MythMediaDevice::setSpeed(const char *device);

comment:4 Changed 10 years ago by Nigel

(In [20223]) Put up a -v media message when setSpeed is unimplemented. Refs #3666. (Note that this should only be called for CDs,

and currently only Linux implements this call)

Let isSameDevice(), and thus GetMedia?(), work on Mac OS X from mythvideo.

comment:5 Changed 10 years ago by Nigel

(In [20224]) A more direct API for setting the CDROM speed. See #3666. I tried to make a static method, but couldn't work out how to have virtual subclasses of it.

comment:6 Changed 10 years ago by Nigel

(In [20225]) One subclass implementation of setSpeed() API change [20224]. See #3666. It may be possible to use DKIOCCDSETSPEED on Darwin, but I haven't tried. I have no idea for Windows.

comment:7 Changed 10 years ago by Nigel

(In [20226]) Allow CD speed setting when Media Monitor is inactive. See #3666. Creating a MythCDROM just for this ugly, but the platform-specific selection has to happen somehow, and I couldn't find a tidier way.

comment:8 Changed 10 years ago by Nigel

(In [20298]) Allow eject when MediaMonitor? disabled (20221-20227 from trunk). See #3666

comment:9 Changed 10 years ago by Nigel

Resolution: fixed
Status: newclosed

(In [20573]) Add CD/DVD speed setting for Mac OS X. Closes #3666. (Note that the FreeBSD code doesn't have setSpeed() implemented yet)

Note: See TracTickets for help on using tickets.