Fedora users, please test mci patch about time-out/hanging machine

Hi,

Testhq.org results from 17th of November about winmm:mci were striking:
All machines producing a timeout had "f11" in their name.
(Another day, some f11 machines passed the tests).
http://test.winehq.org/data/4990ca0ea342bd115a9a46047b3b2de599cb916e/index_W...

Please test whether the following patch correlates with hanging machines.
Not checking waveoutwrite's return code for sure has the potential to hang the
play loop, waiting for a buffer to come back that was never sent out in case of error.

But then, why did waveOutWrite fail? That's what I'd like you to investigate. Why Fedora 11?

Below patch only adds error reporting to the mciplay loop. Feel free to add the same to the mcirecord loop.

diff --git a/dlls/mciwave/mciwave.c b/dlls/mciwave/mciwave.c
index cdf9146..fdb6b18 100644
--- a/dlls/mciwave/mciwave.c
+++ b/dlls/mciwave/mciwave.c
@@ -882,6 +882,7 @@ static DWORD WAVE_mciPlay(MCIDEVICEID wDevID, DWORD_PTR dwFlags, DWORD_PTR pmt,
 	      &waveHdr[whidx], waveHdr[whidx].dwBufferLength,
 	      waveHdr[whidx].dwBytesRecorded);
 	dwRet = waveOutWrite(wmw->hWave, &waveHdr[whidx], sizeof(WAVEHDR));
+	if (dwRet) ERR("Write %d\n", dwRet);
 	left -= count;
 	wmw->dwPosition += count;
 	TRACE("after WODM_WRITE dwPosition=%u\n", wmw->dwPosition);
@@ -893,7 +894,8 @@ static DWORD WAVE_mciPlay(MCIDEVICEID wDevID, DWORD_PTR dwFlags, DWORD_PTR pmt,
     WAVE_mciPlayWaitDone(wmw); /* to balance first buffer */
 
     /* just to get rid of some race conditions between play, stop and pause */
-    waveOutReset(wmw->hWave);
+    dwRet = waveOutReset(wmw->hWave);
+    if (dwRet) ERR("Reset %d\n", dwRet);
 
     waveOutUnprepareHeader(wmw->hWave, &waveHdr[0], sizeof(WAVEHDR));
     waveOutUnprepareHeader(wmw->hWave, &waveHdr[1], sizeof(WAVEHDR));

A future patch will abort the loop in case of error.  However so far, when creating
situations that hung on my (non-Fedora) machine (cf. bug #20664), the above ERR message
was never triggered.
So the cause of random mci time-out on test.winehq may as well lay elsewhere.

Thanks for your help,
	Jörg Höhle

[Michael Stefaniuc]

Hello!

Joerg-Cyril.Hoehle(a)t-systems.com wrote:
> Testhq.org results from 17th of November about winmm:mci were striking:
> All machines producing a timeout had "f11" in their name.
> (Another day, some f11 machines passed the tests).
> http://test.winehq.org/data/4990ca0ea342bd115a9a46047b3b2de599cb916e/index_W...
>
> Please test whether the following patch correlates with hanging machines.
> Not checking waveoutwrite's return code for sure has the potential to hang the
> play loop, waiting for a buffer to come back that was never sent out in case of error.
>
> But then, why did waveOutWrite fail? That's what I'd like you to investigate. Why Fedora 11?
>
> Below patch only adds error reporting to the mciplay loop. Feel free to add the same to the mcirecord loop.
>
> diff --git a/dlls/mciwave/mciwave.c b/dlls/mciwave/mciwave.c
> index cdf9146..fdb6b18 100644
> --- a/dlls/mciwave/mciwave.c
> +++ b/dlls/mciwave/mciwave.c
> @@ -882,6 +882,7 @@ static DWORD WAVE_mciPlay(MCIDEVICEID wDevID, DWORD_PTR dwFlags, DWORD_PTR pmt,
>         &waveHdr[whidx], waveHdr[whidx].dwBufferLength,
>         waveHdr[whidx].dwBytesRecorded);
>   dwRet = waveOutWrite(wmw->hWave, &waveHdr[whidx], sizeof(WAVEHDR));
> +	if (dwRet) ERR("Write %d\n", dwRet);
>   left -= count;
>   wmw->dwPosition += count;
>   TRACE("after WODM_WRITE dwPosition=%u\n", wmw->dwPosition);
> @@ -893,7 +894,8 @@ static DWORD WAVE_mciPlay(MCIDEVICEID wDevID, DWORD_PTR dwFlags, DWORD_PTR pmt,
>      WAVE_mciPlayWaitDone(wmw); /* to balance first buffer */
>
> /* just to get rid of some race conditions between play, stop and pause */
> -    waveOutReset(wmw->hWave);
> +    dwRet = waveOutReset(wmw->hWave);
> +    if (dwRet) ERR("Reset %d\n", dwRet);
>
> waveOutUnprepareHeader(wmw->hWave, &waveHdr[0], sizeof(WAVEHDR));
>      waveOutUnprepareHeader(wmw->hWave, &waveHdr[1], sizeof(WAVEHDR));
>
> A future patch will abort the loop in case of error.  However so far, when creating
> situations that hung on my (non-Fedora) machine (cf. bug #20664), the above ERR message
> was never triggered.
> So the cause of random mci time-out on test.winehq may as well lay elsewhere.
> Like in PulseAudio? That may well be. PA in F11 made sound be more
> broken in Wine.

bye
	michael

Michael,
> > So the cause of random mci time-out on test.winehq may as 
> > well lay elsewhere.
> > Like in PulseAudio? That may well be. PA in F11 made sound be more
> > broken in Wine.
> > Is that your own experience or other people's reports?

My experience with PA in Ubuntu Intrepid -- it's not the newest Ubuntu
at all -- is that it has been stable for several month now, to a point
where I regularly forget to run "pactl exit" to stop it every time I
log into Gnome as I did half a year ago.

Lastly, Wine has performed equally well in a PA, ALSA or OSS setting AFAICT.
It's long ago that PA caused known bugs such as stuttering (in Hardy and
Intrepid); they must have been fixed by a combination of changes in
Wine and perhaps Ubuntu updates(?).
Of course, YMMV.

One issue remains in Intrepid: Because Timidity was not modified for
PulseAudio[*], it grabs the ALSA device when it starts, causing all
subsequent PA clients to loose access to sound.  So it is still advisable
to issue "pactl exit" to let Wine (and Timidity) share ALSA well.

Regards,
	Jörg Höhle
[*] or because PA was badly integrated?  Why should every sound using
package be modified when something like PA jumps into a distribution?!?

[Michael Stefaniuc]

Joerg-Cyril.Hoehle(a)t-systems.com wrote:
> Michael,
> > > So the cause of random mci time-out on test.winehq may as 
> > > well lay elsewhere.
> > > Like in PulseAudio? That may well be. PA in F11 made sound be more
> > > broken in Wine.
> > > Is that your own experience or other people's reports?
> > > My own from running winetest.exe; some of the dsound tests are failing
> > > now which they didn't with F10. But according to Maarten the sound in
> > > Wine is to be blamed for that.
> My experience with PA in Ubuntu Intrepid -- it's not the newest Ubuntu
> at all -- is that it has been stable for several month now, to a point
> where I regularly forget to run "pactl exit" to stop it every time I
> log into Gnome as I did half a year ago.
> I don't have high requirements on sound, only youtube and xine/mplayer
> and those just work. That's what squelched my desire to remove PA.
> Lastly, Wine has performed equally well in a PA, ALSA or OSS setting AFAICT.
> It's long ago that PA caused known bugs such as stuttering (in Hardy and
> Intrepid); they must have been fixed by a combination of changes in
> Wine and perhaps Ubuntu updates(?).
> Of course, YMMV.
> Well, I don't use Wine ;)

bye
	michael

[Hin-Tak Leung]

pulseaudio occasionally gets into a confusion by itself. I found I need to do 

pulseaudio --cleanup-shm 
from time to time.

--- On Thu, 19/11/09, Joerg-Cyril.Hoehle(a)t-systems.com <Joerg-Cyril.Hoehle(a)t-systems.com> wrote:
> Hi,
>
> Testhq.org results from 17th of November about winmm:mci
> were striking:
> All machines producing a timeout had "f11" in their name.
> (Another day, some f11 machines passed the tests).
> http://test.winehq.org/data/4990ca0ea342bd115a9a46047b3b2de599cb916e/index_W...
>
> Please test whether the following patch correlates with
> hanging machines.
> Not checking waveoutwrite's return code for sure has the
> potential to hang the
> play loop, waiting for a buffer to come back that was never
> sent out in case of error.
>
> But then, why did waveOutWrite fail? That's what I'd like
> you to investigate. Why Fedora 11?
>
> Below patch only adds error reporting to the mciplay loop.
> Feel free to add the same to the mcirecord loop.
>
> diff --git a/dlls/mciwave/mciwave.c
> b/dlls/mciwave/mciwave.c
> index cdf9146..fdb6b18 100644
> --- a/dlls/mciwave/mciwave.c
> +++ b/dlls/mciwave/mciwave.c
> @@ -882,6 +882,7 @@ static DWORD WAVE_mciPlay(MCIDEVICEID
> wDevID, DWORD_PTR dwFlags, DWORD_PTR pmt,
>
> &waveHdr[whidx], waveHdr[whidx].dwBufferLength,
>
> waveHdr[whidx].dwBytesRecorded);
>      dwRet = waveOutWrite(wmw->hWave,
> &waveHdr[whidx], sizeof(WAVEHDR));
> +    if (dwRet) ERR("Write %d\n", dwRet);
>      left -= count;
>      wmw->dwPosition += count;
>      TRACE("after WODM_WRITE
> dwPosition=%u\n", wmw->dwPosition);
> @@ -893,7 +894,8 @@ static DWORD WAVE_mciPlay(MCIDEVICEID
> wDevID, DWORD_PTR dwFlags, DWORD_PTR pmt,
>      WAVE_mciPlayWaitDone(wmw); /* to
> balance first buffer */
>
> /* just to get rid of some race
> conditions between play, stop and pause */
> -    waveOutReset(wmw->hWave);
> +    dwRet = waveOutReset(wmw->hWave);
> +    if (dwRet) ERR("Reset %d\n", dwRet);
>
> waveOutUnprepareHeader(wmw->hWave,
> &waveHdr[0], sizeof(WAVEHDR));
>
> waveOutUnprepareHeader(wmw->hWave,
> &waveHdr[1], sizeof(WAVEHDR));
>
> A future patch will abort the loop in case of error. 
> However so far, when creating
> situations that hung on my (non-Fedora) machine (cf. bug
> #20664), the above ERR message
> was never triggered.
> So the cause of random mci time-out on test.winehq may as
> well lay elsewhere.
>
> Thanks for your help,
>     Jörg Höhle