Opened 14 years ago

Closed 14 years ago

Last modified 14 years ago

#2462 closed defect (fixed)

Eject media button fails if tray is empt

Reported by: list-mythtv@… Owned by: Nigel
Priority: minor Milestone: unknown
Component: mythtv Version: 0.20
Severity: medium Keywords:
Cc: Ticket locked: no

Description

If the cdrom tray is empty the eject button does nothing. Looking at the code for mythmediamonitor.ccp it seems like this is working as designed. It would be nice if eject button would open the tray regardless of if media is present or not.

Change History (21)

comment:1 Changed 14 years ago by list-mythtv@…

Type: enhancementdefect

I'm changing this to a bug after doing more investigating. The following situation seems to happen which should not:

1) Start mythfrontend while a DVD is in the drive. the FE will notice it and mount it issuing the correct events. 2) Select the Eject media button - The FE will NOT eject the media, it will only unmount it 3 Select the Eject media button a second time - The FE will eject the media.

I expect that when the Eject media button is used that the tray is opened regardless of if the media is mounted or not. I also expect that the tray is opened if no media is present. :)

I'll take a look at the code and try to come up with a patch.

comment:2 Changed 14 years ago by stuartm

Owner: changed from Isaac Richards to stuartm

comment:3 Changed 14 years ago by list-mythtv@…

FWIW, I did look at the code and it's obvious that it's not working. I didn't bother to rewrite this as it looked like the current code also supported hotplug media (USB drives for example) and I didn't have anything to test with. Not sure if anyone actually cares about hotplug media mounting for optical disc playback but that's what the code looks like :)

This might be easier to simplify things and just make it work with ejectable media.

comment:4 Changed 14 years ago by Nigel

1) Hotplug media is mainly so users can easily get photos/songs/video-clips off USB flash drives, [i]et ci. The DVD ripper doesn't use it, but other parts of MythTV could. And users need some way of unmounting those drives - via the eject action.
2) Around line 102 of mythcdrom.cpp, note the comment: HACK make it possible to eject a DVD by unmounting it
I think ChooseAndEjectMedia?() needs to check if it is mounted (and if so unmount it), and then check if it is closed (and if so, unlock and open it)?

comment:5 Changed 14 years ago by Nigel

The double-eject requirement seems to be when the drive is mounted. Here is a patch (against 0-20-fixes) that seems to work for me (which also makes sure, if the drive is mounted, to unlock it before ejecting):

% svn diff mythmediamonitor.cpp
Index: mythmediamonitor.cpp
===================================================================
--- mythmediamonitor.cpp        (revision 12786)
+++ mythmediamonitor.cpp        (working copy)
@@ -157,6 +157,7 @@
     if (!selected)
         return;
 
+    bool doEject = false;
     int status = selected->getStatus();
     QString dev = selected->getVolumeID();
 
@@ -177,8 +178,13 @@
                                       "eject unmount fail",
                                       tr("Failed to unmount %1").arg(dev));
         }
+        else
+            doEject = true;
     }
     else
+        doEject = true;
+
+    if (doEject)
     {
         selected->unlock();
 

The original problem (open the empty tray) will have to wait for some new code that calls GetCDROMBlockDevices() to work out what to open.

comment:6 Changed 14 years ago by Nigel

(In [12789]) Remove the need to eject twice (one unmounts, one unlocks and ejects). See #2462

comment:7 Changed 14 years ago by Nigel

(In [12790]) Minor CD volume detection and double-eject fixes. See #2462. Closes #3073.

comment:8 Changed 14 years ago by anonymous

My "workaround" solution to ejecting was to configure lirc to run: eject -T /dev/cdrom || eject /dev/cdrom when I press the eject button on my remote.

The -T flag is supposed to make eject toggle opening and closing the cdrom drive, but it appears not to be able to open it reliably, while eject with no flags can. Thus the
, to try the no flags if the -T fails.

comment:9 Changed 14 years ago by Nigel

Owner: changed from stuartm to Nigel
Status: newassigned

Slightly tested patch to allow opening empty CD/DVD drive's tray:

% svn diff libs/libmyth
Index: libs/libmyth/mythcdrom-linux.cpp
===================================================================
--- libs/libmyth/mythcdrom-linux.cpp    (revision 13047)
+++ libs/libmyth/mythcdrom-linux.cpp    (working copy)
@@ -10,6 +10,9 @@
 
 MediaError MythCDROMLinux::eject(bool open_close)
 {
+    if (!isDeviceOpen())
+        openDevice()
+
     if (open_close)
         return (ioctl(m_DeviceHandle, CDROMEJECT) == 0) ? MEDIAERR_OK
                                                         : MEDIAERR_FAILED;
@@ -96,11 +99,15 @@
                 // If the disk is ok but not yet mounted we'll test it further down after this switch exits.
                 break;
             case CDS_TRAY_OPEN:
-            case CDS_NO_DISC:
-                //cout << "Tray open or no disc" << endl;
+                //cout << "Tray open" << endl;
                 m_MediaType = MEDIATYPE_UNKNOWN;
                 return setStatus(MEDIASTAT_OPEN, OpenedHere);
                 break;
+            case CDS_NO_DISC:
+                //cout << "No disc" << endl;
+                m_MediaType = MEDIATYPE_UNKNOWN;
+                return setStatus(MEDIASTAT_NODISK, OpenedHere);
+                break;
             case CDS_NO_INFO:
             case CDS_DRIVE_NOT_READY:
                 //cout << "No info or drive not ready" << endl;
Index: libs/libmyth/mythmedia.h
===================================================================
--- libs/libmyth/mythmedia.h    (revision 13047)
+++ libs/libmyth/mythmedia.h    (working copy)
@@ -9,7 +9,8 @@
     MEDIASTAT_ERROR,
     MEDIASTAT_UNKNOWN,
     MEDIASTAT_UNPLUGGED,
-    MEDIASTAT_OPEN,
+    MEDIASTAT_OPEN,       /**< CD/DVD tray open. Meaningless for other types */
+    MEDIASTAT_NODISK,     /**< CD/DVD tray closed, SCSI drive unformatted? */
     MEDIASTAT_USEABLE,    
     MEDIASTAT_NOTMOUNTED,
     MEDIASTAT_MOUNTED

comment:10 Changed 14 years ago by Nigel

(In [13091]) Fixes for opening CDROM tray when there is no disk in it. See #2462

comment:11 Changed 14 years ago by Nigel

(In [13092]) Slip of the finger in vi. See #2462

comment:12 Changed 14 years ago by Nigel

(In [13093]) Fixes for opening CDROM tray when there is no disk in it. See #2462

comment:13 Changed 14 years ago by list-mythtv@…

Running mythtv-0.20_p13110 on gentoo and still not seeing this working. Recreated as:

1) Start be/fe with dvd in the drive 2) use mythtv to eject the disc then close the tray 3) attempt to open the tray with mythtv

I'll try to capture some logs later tonight.

comment:14 Changed 14 years ago by Nigel

The logs may not narrow it down much (unless there is an obvious error). Adding some extra debug, something like this, may help:

% svn diff mythcdrom-linux.cpp mythmediamonitor.cpp 
Index: mythcdrom-linux.cpp
===================================================================
--- mythcdrom-linux.cpp (revision 13125)
+++ mythcdrom-linux.cpp (working copy)
@@ -11,11 +11,23 @@
 MediaError MythCDROMLinux::eject(bool open_close)
 {
     if (!isDeviceOpen())
+    {
+        cout << "MythCDROMLinux::eject(" << open_close
+             << ") Opening device for eject/closetray ioctl()";
         openDevice();
+    }
 
     if (open_close)
-        return (ioctl(m_DeviceHandle, CDROMEJECT) == 0) ? MEDIAERR_OK
-                                                        : MEDIAERR_FAILED;
+    {
+        int stat = ioctl(m_DeviceHandle, CDROMEJECT) == 0;
+
+        if (stat)
+            return MEDIAERR_OK;
+
+        cout << "MythCDROMLinux::eject(" << open_close
+             << ") ioctl(CDROMEJECT) - error " << strerror(errno);
+        return MEDIAERR_FAILED;
+    }
     else
     {
         // If the tray is empty, this will fail (Input/Output error)
Index: mythmediamonitor.cpp
===================================================================
--- mythmediamonitor.cpp        (revision 13125)
+++ mythmediamonitor.cpp        (working copy)
@@ -166,6 +166,7 @@
 
     if (MEDIASTAT_OPEN == status)
     {
+        puts("Tray is open. Trying to close it.");
         selected->eject(false);
     }
     else if (MEDIASTAT_MOUNTED == status)
@@ -186,6 +187,7 @@
 
     if (doEject)
     {
+        puts("Unlocking device before attempting eject()");
         selected->unlock();
 
         if (selected->eject() == MEDIAERR_UNSUPPORTED)

(Note that is just typed in - untested)

comment:15 Changed 14 years ago by Nigel

I have added some more debug and managed to reproduce a similar problem on a machine with two CDROMs. It seems that the status ioctl on the drive always returns CDS_TRAY_OPEN until the tray is closed with a disk in it. When there is one CDROM or one DVD drive, no problem. [BR] I will try to look through the kernel source, but I don't fancy my chances of working out this in any hurry. May have to commit something ugly.

comment:16 Changed 14 years ago by Nigel

(In [13173]) Try to workaround Linux CD drives that don't know the difference between OPEN and NODISK (See #2462). Changed priority of several VERBOSE messages. Eliminated a few warnings about '/sys/block/hdc/holders' and friends.

comment:17 Changed 14 years ago by Nigel

OK. The 0-20-fixes branch now has more debug logging (-v media), and a fix for recalcitrant drives. Not really a fix, but at least it should tell the user what's going on.
Hopefully that will help Scott work out the problem and provide logs.

comment:18 Changed 14 years ago by Nigel

(In [13175]) Try to workaround Linux CD drives that don't know the difference between OPEN and NODISK (See #2462). Changed priority of several VERBOSE messages. Eliminated a few warnings about '/sys/block/hdc/holders' and friends.

comment:19 Changed 14 years ago by list-mythtv@…

Thanks, I'll pull this in to my ebuild and try this weekend. Note that my drive does work as expected with eject -T from the command line.

comment:20 Changed 14 years ago by Nigel

Resolution: fixed
Status: assignedclosed

(In [13207]) Also allow ejecting if "Monitor CD/DVD" is disabled. Closes #2462.

comment:21 Changed 14 years ago by Nigel

(In [13211]) Also allow ejecting if "Monitor CD/DVD" is disabled. Closes #2462.

Note: See TracTickets for help on using tickets.