http://bugs.winehq.org/show_bug.cgi?id=26160
Summary: Broken control path in mcicda Product: Wine Version: 1.3.13 Platform: x86 OS/Version: Mac OS X 10.5 Status: UNCONFIRMED Severity: normal Priority: P2 Component: winmm&mci AssignedTo: wine-bugs@winehq.org ReportedBy: hoehle@users.sourceforge.net
I'm thankful to C. Robinson for making mcicda work on my Linux machines since wine-1.1.5. Since then the code supports 2 modes of operation, either via - IOCTL_CDROM_*_AUDIO (PLAY/STOP/PAUSE/RESUME/SEEK) - or via a separate thread using DirectSound with IOCTL_CDROM_RAW_READ. READ_TOC is needed in both modes.
However, commit 8afa626faa3c5aa2d32d17dca77edaf9efb3a5da uses as selector if (wmcda->hThread != 0) { which causes inconsistent results as the thread may be long dead. I believe that is not the correct way to select whether to invoke either thread/DS+RAW_READ commands or IOCTL_CDROM_*_AUDIO ones.
Consider this sequence of MCI commands: cmd hThread comment play -- start hThread stop # !0 => SetEvent stopEvent, hThread becomes 0 stop # =0 => DeviceIoControl resume # =0 => DeviceioControl play resume # !0 => DSB+Play, unlike previously status mode !0 GetStatus yields PLAY, later STOP it will never detect that a disk was long ejected! stop status mode =0 GetStatus performs IoControl instead and hence reports current and correct state.
It's fine that the code supports 2 modes of operation, the bug is that the current code confuses itself as to which branch should be taken and produces results depending on the history of commands rather than the current state (and HW capabilities), wrongly mixing both code paths, leading to incorrect results.
IMHO a binary decision via hThread is wrong, there are more states to consider: - DSB + RAW_READ is useable / being used; - IOCTL_CDROM_PLAY_AUDIO is useable / being used - don't know yet.
This issue becomes more important now as MacOS support is getting close (see bug #20323). This issue becomes more important now as MacOS support is getting close (see bug #20323). It does not implement the IOCTL_CDROM_*_AUDIO and spits out unneeded err: and fixme: to the console. I mentioned that I'd about this issue in bug #20323 comment #4 last year.
http://bugs.winehq.org/show_bug.cgi?id=26160
Jörg Höhle hoehle@users.sourceforge.net changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |chris.kcat@gmail.com
http://bugs.winehq.org/show_bug.cgi?id=26160
--- Comment #1 from Jörg Höhle hoehle@users.sourceforge.net 2011-02-18 06:43:03 CST --- Created an attachment (id=33330) --> (http://bugs.winehq.org/attachment.cgi?id=33330) mcicda+cdrom trace
The attached trace excerpt shows a mixture of IOCTL_CDROM calls, mixing unsuccessful *_AUDIO ones with others. E.g. when DirectSound + RAW_READ is used, MCI_STOP should probably not issue IOCTL_CDROM_AUDIO_STOP (and sometimes it doesn't, depending on the command history).
The excerpt also shows the confusion that follows IOCTL_STORAGE_EJECT_MEDIA but that's another issue.
http://bugs.winehq.org/show_bug.cgi?id=26160
--- Comment #2 from Jörg Höhle hoehle@users.sourceforge.net 2011-02-18 06:59:04 CST --- As a consequence of this bug, MCI_PLAY fails on MacOS because READ_Q_CHANNEL is used to get the current position and it's not implemented.
I argue that this failure is wrong because the code path that depends on Direct Sound + RAW_READ to play must not use IOCTL_CDROM to get the current position, because (on Linux at least), this position is independent from RAW_READ.
In effect, on Linux (although this bug is about Mac OS), MCI_STATUS_POSITION reports bogus positions, as an effect of this bug.
Instead the code should use the wmcda->start position maintained by MCICDA_playLoop. (BTW, native appears to behave similarly w.r.t. concurrent seeking by other apps).
Likewise, MCI_SEEK should merely set an internal state variable when DS + RAW_READ is used (this is also bug #26160). IOCTL_CDROM_SEEK_AUDIO_MSF should only be used when IOCTL_*_AUDIO is used to play music, not RAW_READ. (I was considering this scenario when I wrote "don't know yet [which mode to use]" earlier.)
http://bugs.winehq.org/show_bug.cgi?id=26160
Jörg Höhle hoehle@users.sourceforge.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |20323
http://bugs.winehq.org/show_bug.cgi?id=26160
--- Comment #3 from Chris chris.kcat@gmail.com 2011-02-19 00:32:15 CST --- Perhaps if there's a way to detect when a device is opened if it can use digital extraction or not, and keeping a persistent flag with the struct. That way, any given opened device will only use one path or the other, avoiding the IOCTL_CDROM_*_AUDIO calls if digital extraction works.
The non-AUDIO path will have to have more code made for it, though.
http://bugs.winehq.org/show_bug.cgi?id=26160
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- OS/Version|Mac OS X 10.5 |Mac OS X
http://bugs.winehq.org/show_bug.cgi?id=26160
--- Comment #4 from Austin English austinenglish@gmail.com 2013-11-13 16:52:09 CST --- This is your friendly reminder that there has been no bug activity for 2 years. Is this still an issue in current (1.7.6 or newer) wine? If so, please attach the terminal output in 1.7.6 (see http://wiki.winehq.org/FAQ#get_log).
https://bugs.winehq.org/show_bug.cgi?id=26160
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |RESOLVED Resolution|--- |ABANDONED
--- Comment #5 from Austin English austinenglish@gmail.com --- Abandoned.
https://bugs.winehq.org/show_bug.cgi?id=26160
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #6 from Austin English austinenglish@gmail.com --- Closing.
https://bugs.winehq.org/show_bug.cgi?id=26160
WineBuG winebugs140@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |winebugs140@gmail.com
--- Comment #7 from WineBuG winebugs140@gmail.com --- Since Bug 20323 (which depends on this exact bug) seems to be unfixed in Wine 1.7.53, I suggest reopening this issue.
https://bugs.winehq.org/show_bug.cgi?id=26160
WineBuG winebugs140@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |austinenglish@gmail.com
https://bugs.winehq.org/show_bug.cgi?id=26160
--- Comment #8 from WineBuG winebugs140@gmail.com --- (In reply to WineBuG from comment #7)
Since Bug 20323 (which depends on this exact bug) seems to be unfixed in Wine 1.7.53, I suggest reopening this issue.
I meant 1.7.51. :P
https://bugs.winehq.org/show_bug.cgi?id=26160
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|CLOSED |REOPENED Resolution|ABANDONED |--- Ever confirmed|0 |1
--- Comment #9 from Austin English austinenglish@gmail.com --- (In reply to WineBuG from comment #8)
(In reply to WineBuG from comment #7)
Since Bug 20323 (which depends on this exact bug) seems to be unfixed in Wine 1.7.53, I suggest reopening this issue.
I meant 1.7.51. :P
Reopening.
https://bugs.winehq.org/show_bug.cgi?id=26160
Zevans zack23evans@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |zack23evans@gmail.com