[Bug 20232] New: mciwave breaks on MSDN example

http://bugs.winehq.org/show_bug.cgi?id=20232

Summary: mciwave breaks on MSDN example
           Product: Wine
           Version: 1.1.30
          Platform: All
        OS/Version: All
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: winmm&mci
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]

I'm reviewing mciwave and mciseq and found numerous issues.  Concurrently, I'm
augmenting the MCI testsuite.  I've attached my current mciwave
work-in-progress tests to give you an early insight into my work.

The tests so far confirm my model of MCI: I view it like an old-fashioned tape
deck with buttons, which you can press in any order.  Still there's error
checking, e.g. you can't resume when stopped.  Like a tape deck, you can switch
from playing to recording at any time.
Oddly, you can combine fast forward and play in one command, but not rewind
with play.

The attached tests work fine on (at least one machine with) MS-Windows, but
cause Wine to fail and hang in multiple ways. You've been warned!

I'd be pleased if people could run the tests on more instances of MS-Windows.

I plan to strip down this test file until it at least does not hang anymore in
Wine.  Then only can it be submitted.  Concurrently, I can submit patches to
the mci* codebase.

Current issues with the mci code in Wine are:
 - not distinguishing between STOPPING and STOPPED, PLAYING and GOING TO PLAY,
etc.;
 - seriously broken asynchronous execution;
 - non-error-proof use of InterlockDecrement. It must only be called
   when waveInAddBuffer and waveOutWrite succeed, as those cause
   callbacks to happen;
 - RIFF .wav file not always correctly written;
 - the mmio is as much as resource as the wave device and must be
   properly released (mmioAscend, not only when saving);
 - many items here and there:
  + bogus 44000Hz frequency;
  + premature return (in mcicda);
  + conversion between #bytes and samples;
  + switching fInput while playing;
  + copy&paste errors (InterlockedDecrement need be initialised
    differently when recording and playing)

The major issue is concurrency. There I'm not sure how to proceed and rewrite
mciwave (and mciseq and mcicda).

o One model, Erlang-like, which the OSS driver also implements, is one thread
per play and exclusively using message passing to receive commands.  This would
simplify NOTIFY everywhere.  It clearly offers the advantage of being closest
to the sequential execution model that is easy to reason about.  IMHO, it does
not play nicely with the pause command (state machine).

o Continue as currently written, but then think twice and even 3-7 times about
how to perform locking among the concurrent threads:
 - Put InterlockIncrement() to more uses?
 - Put Events to more uses?
 - Unlike mciavi, there's no CriticalSection in mciwave and mcicda, yet
   I'm unconvinced that the current synchronisation via volatile dwStatus
   can suffice.
 - But then, identifying the critical sections will be tedious and error prone.

o Combinations of both, e.g. one thread per play or record (not unlike the
present code), and dealing (correctly) with PAUSE, STOP and RESUME inside it
(via e.g. events to restart a paused thread, or indirectly, by relying on the
callbacks+events sent when invoking waveOutRestart and waveInStart -- is it
legal at all to call those from a different thread)?

o Is there any other concurrency model suitable in Wine?

Regards,
 Jörg Höhle

[Bug 20232] mciwave breaks on MSDN example

http://bugs.winehq.org/show_bug.cgi?id=20232

--- Comment #1 from Jörg Höhle   2009-10-01 11:24:43 ---
Created an attachment (id=23868)
 --> (http://bugs.winehq.org/attachment.cgi?id=23868)
winmm/tests/mci.c testcase work-in-progress

The MSDN example that breaks Wine is at
http://msdn.microsoft.com/en-us/library/dd798631(VS.85).aspx
Recording with a Waveform-Audio Device

The test should create a tempfile.wav of 2 seconds exactly. If you have no
microphone (I haven't), it'll record 2 seconds of (near) silence.

00000000: 5249 4646 4656 0000 5741 5645 666d 7420  RIFFFV..WAVEfmt 
00000010: 1000 0000 0100 0100 112b 0000 112b 0000  .........+...+..
00000020: 0100 0800 6461 7461 2256 0000 8080 8080  ....data"V......
                              ^^^^^^^^^
One bug in Wine is that the length of the data chunk is left as 0 because of
the switch from recording to playing.

[Bug 20232] mciwave breaks on MSDN example

http://bugs.winehq.org/show_bug.cgi?id=20232

Vitaliy Margolen  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
           Platform|All                         |Other
         Resolution|                            |INVALID
         OS/Version|All                         |other

--- Comment #2 from Vitaliy Margolen   2009-10-01 20:25:41 ---
One problem per bug.
DO NOT attach source code you have to rights to distribute. You better read the
license first.

Invalid.

[Bug 20232] mciwave breaks on MSDN example

http://bugs.winehq.org/show_bug.cgi?id=20232

Vitaliy Margolen  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #3 from Vitaliy Margolen   2009-10-01 20:25:49 ---
Closing.

[Bug 20232] mciwave breaks on MSDN example

http://bugs.winehq.org/show_bug.cgi?id=20232

Nikolay Sivov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|CLOSED                      |UNCONFIRMED
         Resolution|INVALID                     |

--- Comment #4 from Nikolay Sivov   2009-10-02 04:37:25 ---
(In reply to comment #2)
> One problem per bug.

Read comment 1 for bug info, or a summary at least.
> DO NOT attach source code you have to rights to distribute. You better read the
> license first.

What are talking about? This patch wasn't committed, it's a draft.
> Invalid.

Let people help with audio subsystem, don't be so formal with that please.

2Jorg:

Please remove copyrighted copy/paste parts from your test, just rework them to
do the same.

[Bug 20232] mciwave breaks when mixing record & play

http://bugs.winehq.org/show_bug.cgi?id=20232

Jörg Höhle  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|mciwave breaks on MSDN      |mciwave breaks when mixing
                   |example                     |record & play

--- Comment #5 from Jörg Höhle   2009-10-02 06:40:40 ---
I will rewrite the copy&paste parts.
The subject states the one bug that shall close this issue once fixed.  The
rest is comments from my analysis, and I'll turn to wine-devel to gather
information about CriticalSections, Evens etc.

[Bug 20232] mciwave breaks when mixing record & play

http://bugs.winehq.org/show_bug.cgi?id=20232

Jörg Höhle  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #23868|0                           |1
        is obsolete|                            |

--- Comment #6 from Jörg Höhle   2009-10-16 05:02:15 ---
Created an attachment (id=24161)
 --> (http://bugs.winehq.org/attachment.cgi?id=24161)
Many MCI waveaudio tests

The attached file replaces winmm/tests/mci.c as of 1.1.31.  Several sections
have been protected by #if FULL_TEST because they cause Wine to hang or crash,
so they are disabled by default.  It does not perform the original record+play
hangs test, which will be added later.

The full tests pass on MS-w2k and XP (non virtual).
The non-full parts, loaded with todo_wine, pass on Linux without hanging using
ALSA, OSS, both HW and emulation.

I was about to submit them when I noted that the (non-full) tests cause Wine to
hang in MacOS.  Thus I want to move even more tests into the FULL_TEST section
until no hang occurs on MacOS.  That'll be better for test.winehq.org.
What do you think?

My file provide a correct testcase for bug #15060 (RECORD ignores SET wave
format).
I believe it demonstrates bug #15423 (hang after second PlaySound).
If we had had these tests earlier, we'd better known broken areas in Wine (and
not written some code at all, e.g. the MCI sequencer does not support RESUME
(MIDI tests not included here)).  Interesting insight into test driven
development (TDD).

Of course, the goal is to improve Wine so that FULL_TEST can be merged into the
normal tests.  I already have a few minor patches that fix some of the
todo_wine.  But not yet the major hangs or random crashes at program exit.

I also plan to add mcimidi and cdaudio tests.  Is it ok to add a sample .mid to
the Wine tree? (and where to find a free one?)

[Bug 20232] mciwave breaks when mixing record & play

http://bugs.winehq.org/show_bug.cgi?id=20232

--- Comment #7 from Eric Pouech   2009-10-17 02:38:17 ---
Created an attachment (id=24179)
 --> (http://bugs.winehq.org/attachment.cgi?id=24179)
patch for mciwave.c (better sync of async commands)

actually, some issues brought to light by the new test file are because our
async command handling is not correctly synchronized (sic)
so basically, the attached patch modifies the way async command management
should be done:
- it splits command management into two blocks
    1/ the first block, which is not done asynchronously (ie that we'll be
performed before the MCI command returns)
    2/ the second block, which will be run asynchronously
- so for example, in a play command, all the init, wave out stream opening,
from/to position computations... is done in first block ; the actual playback
(aka feeding the wave headers) is done in second block
- the attached patch is a quick hack for testing purposes (on my machine it
works correctly, ie the hang is removed, and the test crash in NULL pointer
deref in MCI_SYSINFO, that should be easy to fix)

[Bug 20232] mciwave breaks when mixing record & play

http://bugs.winehq.org/show_bug.cgi?id=20232

--- Comment #8 from Jörg Höhle   2009-10-18 12:51:17 ---
Initially, a sequence as simple as
open tempfile.wav alias x
play x
pause x # immediately afterwards
stop x
would hang Wine on MacOS. A dozen patch files to mciwave later, none of them
specific to the Mac, I reached the opposite situation. Linux hangs during the
full tests, MacOS passes (with errors).
My dozen patches will appear tomorrow.

Uh oh, now I see that Eric worked on the same files. That'll certainly give
merge conflicts.

Eric, it's good that you tackled the synchronization issue, as I fixed many
things but did not yet attack that one. So my dozen patches should be
independent on yours.

[Bug 20232] mciwave breaks when mixing record & play

http://bugs.winehq.org/show_bug.cgi?id=20232

--- Comment #9 from Jörg Höhle   2009-11-03 03:22:00 ---
Created an attachment (id=24528)
 --> (http://bugs.winehq.org/attachment.cgi?id=24528)
patch atop MCI test file to implement interactive MCI shell

This code implements the interactive MCI shell that I've been using to submit
MCI string commands and perform exploratory testing on w95, w2k and xp systems.
It's a patch, but so simple that you could apply it by hand anywhere.

[Bug 20232] mciwave breaks when mixing record & play

http://bugs.winehq.org/show_bug.cgi?id=20232

Jörg Höhle  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #24528|application/octet-stream    |text/x-diff
          mime type|                            |

[Bug 20232] mciwave breaks when mixing record & play

http://bugs.winehq.org/show_bug.cgi?id=20232

--- Comment #10 from Jörg Höhle   2009-11-03 03:30:38 ---
Created an attachment (id=24529)
 --> (http://bugs.winehq.org/attachment.cgi?id=24529)
binary that contains interactive MCI shell and MCI tests

This was compiled using MSVC and Wine's testing framework from my code posted
above.
wintest.exe mci performs the tests from comment #7.
wintest.exe mcishell gives you the interactive shell from comment #9.

With a good MCI reference such as http://www.saettler.com/RIFFMCI/riffmci.html
or MSDN you now control the media player (CD+MIDI audio, .wav and .avi files)!

[Bug 20232] mciwave breaks when mixing record & play

http://bugs.winehq.org/show_bug.cgi?id=20232

--- Comment #11 from Jörg Höhle   2009-12-18 10:48:48 ---
FWIW, the current state of this bug in wine-1.1.34 is that record + plays works
if and only if the file is saved after recording, so that the length
information (ckWaveData.cksize) is updated by mmioAscend.  Native does not need
this intermediate step.
Many other issues have been fixed since comment #0.

[Bug 20232] mciwave breaks when mixing record & play

http://bugs.winehq.org/show_bug.cgi?id=20232

--- Comment #12 from Jörg Höhle   2010-05-19 23:46:18 ---
The call of WAVE_mciOpenFile within WAVE_mciSave causes several memory leaks in
winmm/mci.c and mciwave.c that Valgrind detects in winmm/tests/mci.c.

Yet I'm not much inclined to patch WAVE_mciReadFmt to fix the one leak in
mciwave (wmw->lpWaveFormat) because I feel that calling OpenFile within
MCI_SAVE is somehow bogus and should be changed anyway.
 a) It causes state to be lost which the native MCI does not loose, e.g. status
position (verified by tests I once performed).
 b) Using mmioClose in Save leaves the driver in an inconsistent state should
anything fail afterwards (in particular mciOpenFile). Or if save itself fails,
the device is gone (save "X:\nosuchdevice"; play => MCIERR_FILE_NOT_FOUND --
imagine your text editor loosing your text when trying to save).

I'm not convinced that the current use of MMIO within mciwave is "the right
thing". It is not easy to implement MCI_RECORD_INSERT (which is the default in
native, not MCI_RECORD_OVERWRITE) and MCI_DELETE (section within file, see
tests/mci.c) with it, but I'm not familiar with MMIO.

Native mciwave's Record command behaves like a text editor: you can insert and
delete things at arbitrary positions.

[Bug 20232] mciwave breaks when mixing record & play

http://bugs.winehq.org/show_bug.cgi?id=20232

Raymond  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #13 from Raymond  2011-03-22 22:37:50 CDT ---
are you testing with winealsa.drv ?

why ALSA_AddCaptureDevice() assign

wwi.incaps.wPid = MM_CREATIVE_SBP16_WAVEOUT;

in winealsa.drv/waveinit.c

[Bug 20232] mciwave breaks when mixing record & play

http://bugs.winehq.org/show_bug.cgi?id=20232

Julian Rüger  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

[Bug 20232] mciwave breaks when mixing record & play

https://bugs.winehq.org/show_bug.cgi?id=20232

--- Comment #14 from Austin English  ---
This is your friendly reminder that there has been no bug activity for 2 years.
Is this still an issue in current (1.7.16 or newer) wine?

[Bug 20232] mciwave breaks when mixing record & play

https://bugs.winehq.org/show_bug.cgi?id=20232

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |ABANDONED

--- Comment #15 from Austin English  ---
(In reply to Austin English from comment #14)
> This is your friendly reminder that there has been no bug activity for 2
> years. Is this still an issue in current (1.7.16 or newer) wine?

Abandoned.

[Bug 20232] mciwave breaks when mixing record & play

https://bugs.winehq.org/show_bug.cgi?id=20232

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #16 from Austin English  ---
Closing.