[Bug 26160] New: Broken control path in mcicda

[wine-bugs at winehq.org]

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 at winehq.org
        ReportedBy: hoehle at 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.

-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.