DirectSound problems - dlls/dsound/mixer.c

[Davin McCall]

Hi,

I'm trying to sort out sound problems with StarCraft and have made some progress by hacking around with the "dlls/dsound/mixer.c" file. I'm new to Wine hacking however, and there's some things in the code which I don't understand (and others which seem blatantly wrong).

Hopefully someone can help me with these questions:

1) In DSOUND_PerformMix(), two values inq and maxq are calculated which seem to represent the amount of already-mixed (prebuffered) data in the primary buffer and the maximum amount that should be prebuffered. See lines 1001 - 1012.

Then the comparison "inq > maxq" is made to detect whether all buffered data has been played, and if so the sound is stopped by calling DSOUND_PrimaryStop. However, inq is based on the *writepos* value which is the play position plus a safety margin (latency) - see line 972.

So, sound is stopped before the play position actually reaches the end of the buffered audio. This seems wrong. Can anyone tell me if this is correct (and if so why) or if it really is wrong.

2) (same function) on line 1048, after using DSOUND_PrimaryStop(), the playpos is retrieved by making a driver call. The comment says that DSOUND_PrimaryStop() is supposed to reset playpos to 0, but OSS cannot do this and that is why IDsDriverBuffer_GetPosition() is called. However, with ALSA, the driver returns the last played position, even though when the sound is restarted (ie. when DSOUND_PrimaryPlay is called) the position does go back to 0. The result is that playpos is wrong in the meantime. If a sound buffer is added to the queue it gets mixed at the wrong position.

So, should the ALSA driver return 0 for IDsDriverBuffer_GetPosition() or is it simply wrong to use that function to retrieve play position after stopping the device? (and if so, is there a better solution)?

3) Was whoever wrote this code allergic to comments? :-)

I'd appreciate any help I can get.

Davin

[Molle Bestefich]

Can't offer any help, except to say that I've noticed too that the
mixer is obviously broken:
http://article.gmane.org/gmane.comp.emulators.wine.devel/34994/

Thanks for working on this :-)

[Davin McCall]

Yes, if sounds are being clipped at the end it does seem like the answer to my first question is "yes this is wrong". Thanks. I'll keep looking at it.

Davin

On Fri, 28 Oct 2005 16:34:05 +0200
Molle Bestefich  wrote:
> Can't offer any help, except to say that I've noticed too that the
> mixer is obviously broken:
> http://article.gmane.org/gmane.comp.emulators.wine.devel/34994/
>
> Thanks for working on this :-)

[Oliver Stieber]

--- Davin McCall  wrote:
> Hi,
>
> I'm trying to sort out sound problems with StarCraft and have made some progress by hacking
> around with the "dlls/dsound/mixer.c" file. I'm new to Wine hacking however, and there's some
> things in the code which I don't understand (and others which seem blatantly wrong).

There are more problems with Mixer.

I've been getting some deadlocks in the mixer code with games like
http://www.download.com/The-Incredibles-demo/3000-7505_4-10334423.html?tag=l....

Blink video uses two threads to smoothly switch between videos, sometimes I'll get a dead lock
where the mixer threads are waiting for another thread that doesn't seem to exist any more. The
problem sometimes shows up in  winmm/time.c TIME_MMSysTimeCallback as a invalid TIME_TimersList,
it seems like something's not being cleaned up properly.

I've got some more notes on the problem kicking around somewhere.

Oliver

___________________________________________________________ 
Yahoo! Messenger - NEW crystal clear PC to PC calling worldwide with voicemail http://uk.messenger.yahoo.com

[Davin McCall]

Good news - I have got it working. It required work in both the mixer and the Wine ALSA sound driver. It works on my system if the ALSA driver is used and hw emulation is NOT used (in winecfg).

If anyone would like to test patches, give me a hoy. In particular I'd like to make sure it doesn't break anything else - StarCraft is one of the few Windows apps I actually have :-)

Davin

On Fri, 28 Oct 2005 18:24:36 +1000
Davin McCall  wrote:
> Hi,
>
> I'm trying to sort out sound problems with StarCraft and have made some progress by hacking around with the "dlls/dsound/mixer.c" file. I'm new to Wine hacking however, and there's some things in the code which I don't understand (and others which seem blatantly wrong).
>
> Hopefully someone can help me with these questions:
> - In DSOUND_PerformMix(), two values inq and maxq are calculated which seem to represent the amount of already-mixed (prebuffered) data in the primary buffer and the maximum amount that should be prebuffered. See lines 1001 - 1012.
>
> Then the comparison "inq > maxq" is made to detect whether all buffered data has been played, and if so the sound is stopped by calling DSOUND_PrimaryStop. However, inq is based on the *writepos* value which is the play position plus a safety margin (latency) - see line 972.
>
> So, sound is stopped before the play position actually reaches the end of the buffered audio. This seems wrong. Can anyone tell me if this is correct (and if so why) or if it really is wrong.
> - (same function) on line 1048, after using DSOUND_PrimaryStop(), the playpos is retrieved by making a driver call. The comment says that DSOUND_PrimaryStop() is supposed to reset playpos to 0, but OSS cannot do this and that is why IDsDriverBuffer_GetPosition() is called. However, with ALSA, the driver returns the last played position, even though when the sound is restarted (ie. when DSOUND_PrimaryPlay is called) the position does go back to 0. The result is that playpos is wrong in the meantime. If a sound buffer is added to the queue it gets mixed at the wrong position.
>
> So, should the ALSA driver return 0 for IDsDriverBuffer_GetPosition() or is it simply wrong to use that function to retrieve play position after stopping the device? (and if so, is there a better solution)?
> - Was whoever wrote this code allergic to comments? :-)
>
> I'd appreciate any help I can get.
>
> Davin

[Stefan Dösinger]

Am Sonntag, 30. Oktober 2005 14:23 schrieb Davin McCall:
> Good news - I have got it working. It required work in both the mixer and
> the Wine ALSA sound driver. It works on my system if the ALSA driver is
> used and hw emulation is NOT used (in winecfg).
>
> If anyone would like to test patches, give me a hoy. In particular I'd like
> to make sure it doesn't break anything else - StarCraft is one of the few
> Windows apps I actually have :-)

Can you send me the patch / upload it somewhere? I can test it with a few 
games and tell you if everything works as it should.

stefan

[Davin McCall]

Here is the patch.

Davin

On Sun, 30 Oct 2005 15:31:15 +0100
Stefan Dösinger  wrote:
> Am Sonntag, 30. Oktober 2005 14:23 schrieb Davin McCall:
> > Good news - I have got it working. It required work in both the mixer and
> > the Wine ALSA sound driver. It works on my system if the ALSA driver is
> > used and hw emulation is NOT used (in winecfg).
> >
> > If anyone would like to test patches, give me a hoy. In particular I'd like
> > to make sure it doesn't break anything else - StarCraft is one of the few
> > Windows apps I actually have :-)
>
> Can you send me the patch / upload it somewhere? I can test it with a few 
> games and tell you if everything works as it should.
>
> stefan

[Stefan Dösinger]

Hello,
> Here is the patch.

And here are my test results:

* Half-Life: It works with alsa+standard hardware acceleration(yeah!). 
Navigating the menus is much faster(great!), it's the same speed as on 
windows. I didn't expect the sound output as the reason for the slowlyness.

* Counterstrike: Alsa still doesn't work(as before), I have to fall back to 
OSS(which doesn't have acceleration because it's only emulated by alsa). No 
change here. This is strange, because CS is only a HL mod.

* Jedi Knight Jedi Academy: Works nicely with alsa(don't know if it did 
before), still has a 0.5 secound sound lag with acceleration(as it did 
before)

* Battlefield 1942: Difficult to explain: The old situation was that the sound 
started playing when the game starts, but if short sounds(shooting, clicking 
buttons) are played, fragments of the sounds remain in the sound queue and 
form a louder and louder background noise. This makes the sound unuseable. 
Additionally, activating the sound output in winecfg has a very negative 
impact on the speed.
New: No sound without acceleration, but also no speed impact(so an 
improvement). The old sound situation with standard acceleration, speed is 
still good.

I haven't found any regression, as long as the "no sound without acceleration" 
in BF1942 isn't considered an regression.

Continue the good work ;-)

Stefan

PS: I haven't done any sound specific tests lately, so some changes I report 
might be caused by other patches(If there were any)

Stefan

[Davin McCall]

Trying again as an inline attachment.

Davin

On Sun, 30 Oct 2005 15:31:15 +0100
Stefan Dösinger  wrote:
> Am Sonntag, 30. Oktober 2005 14:23 schrieb Davin McCall:
> > Good news - I have got it working. It required work in both the mixer and
> > the Wine ALSA sound driver. It works on my system if the ALSA driver is
> > used and hw emulation is NOT used (in winecfg).
> >
> > If anyone would like to test patches, give me a hoy. In particular I'd like
> > to make sure it doesn't break anything else - StarCraft is one of the few
> > Windows apps I actually have :-)
>
> Can you send me the patch / upload it somewhere? I can test it with a few 
> games and tell you if everything works as it should.
>
> stefan

[Randall Walls]

COMMENTS!!!!!!

Wonderful, beautiful, glorious, HELPFUL comments. Good show, mate.

I'll test it on a few games later today and report back. I think this 
may also fix the 'length is not a factor of blocksize' error that I was 
trying to tackle a while back.

Randall Walls

Davin McCall wrote:
> Trying again as an inline attachment.
>
> Davin
>
> On Sun, 30 Oct 2005 15:31:15 +0100
> Stefan Dösinger  wrote:
> > Am Sonntag, 30. Oktober 2005 14:23 schrieb Davin McCall:
> > > Good news - I have got it working. It required work in both the mixer and
> > > the Wine ALSA sound driver. It works on my system if the ALSA driver is
> > > used and hw emulation is NOT used (in winecfg).
> > >
> > > If anyone would like to test patches, give me a hoy. In particular I'd like
> > > to make sure it doesn't break anything else - StarCraft is one of the few
> > > Windows apps I actually have :-)
> >
> > Can you send me the patch / upload it somewhere? I can test it with a few 
> > games and tell you if everything works as it should.
> >
> > stefan

[Randall Walls]

With patch applied I can't get mixer.c to compile. I'm getting an error 
that 'still_behind' is undeclared, but apparently DSOUND_MixOne is 
attempting to use it.

I get no errors when I patch mixer.c and audio.c. I'm patching against 
the 0.9 release, not against cvs, but I don't think that would matter, I 
don't think that the content of the files has changed since the release.

I'll run through the patch and see if I can spot anything odd.

Randall

Davin McCall wrote:
> Trying again as an inline attachment.
>
> Davin
>
> On Sun, 30 Oct 2005 15:31:15 +0100
> Stefan Dösinger  wrote:
> > Am Sonntag, 30. Oktober 2005 14:23 schrieb Davin McCall:
> > > Good news - I have got it working. It required work in both the mixer and
> > > the Wine ALSA sound driver. It works on my system if the ALSA driver is
> > > used and hw emulation is NOT used (in winecfg).
> > >
> > > If anyone would like to test patches, give me a hoy. In particular I'd like
> > > to make sure it doesn't break anything else - StarCraft is one of the few
> > > Windows apps I actually have :-)
> >
> > Can you send me the patch / upload it somewhere? I can test it with a few 
> > games and tell you if everything works as it should.
> >
> > stefan

[James Hawkins]

On 10/30/05, Randall Walls  wrote:
> With patch applied I can't get mixer.c to compile. I'm getting an error
> that 'still_behind' is undeclared, but apparently DSOUND_MixOne is
> attempting to use it.
>
> I get no errors when I patch mixer.c and audio.c. I'm patching against
> the 0.9 release, not against cvs, but I don't think that would matter, I
> don't think that the content of the files has changed since the release.
>
> I'll run through the patch and see if I can spot anything odd.

Just comment out the TRACE that contains still_behind.  gcc reports
the file and line number of the error.

--
James Hawkins

[Randall Walls]

OK,

Tested with Battlezone 2. Both patched and unpatched the in-game (non 
movie) sound exhibits a popping sound (minor but mildly annoying) with 
lines and lines of

fixme:wave:DSD_CreateSecondaryBuffer 
(0x454d16b8,0x4686cad8,ca,0,0x47a002ec,0x4041b054,0x47a002c8): stub

and

err:dsound:DSOUND_MixInBuffer length not a multiple of block size, len = 
281, block size = 4

The first error I expected to see again, and I believe that this is the 
one responsible for the popping noises. The second I thought could be 
rectified by your patch, which seems to have tried to pad the length to 
match a block size multiple, but it appears that either I interpreted 
your comments wrong or the patch did not succeed.

Also tested DeusEx - sound exhibited no errors. In the past the game had 
always sounded just a bit fuzzy, although I have not played it in so 
long that it might not be your patch that fixed this issue.

I've got a couple other apps that I can test. Will report back.

Randall

James Hawkins wrote:
> On 10/30/05, Randall Walls  wrote:
> > With patch applied I can't get mixer.c to compile. I'm getting an error
> > that 'still_behind' is undeclared, but apparently DSOUND_MixOne is
> > attempting to use it.
> >
> > I get no errors when I patch mixer.c and audio.c. I'm patching against
> > the 0.9 release, not against cvs, but I don't think that would matter, I
> > don't think that the content of the files has changed since the release.
> >
> > I'll run through the patch and see if I can spot anything odd.
>
> Just comment out the TRACE that contains still_behind.  gcc reports
> the file and line number of the error.
>
> --
> James Hawkins

[Randall Walls]

Scratch that.... after spending about an hour playing Battlezone 2 
tonight (with patch applied) I noticed that certain sounds (mainly 
sounds longer than a few seconds) would exhibit a strange problem with 
the last .5 seconds of the clip repeating for a random number of times. 
For example:

In the game the units 'talk' to you. One might say "Got it covered.", 
but with the patch applied it comes out:

"Got it covered"
"ered"
"ered"
"ered"
"ered"
"ered"

It might repeat once, it might repeat 6 times, or any number in between. 
The only console messages are the same as I noted before. I can run a 
trace if it would be beneficial for you.

Randall

Randall Walls wrote:
> OK,
>
> Tested with Battlezone 2. Both patched and unpatched the in-game (non 
> movie) sound exhibits a popping sound (minor but mildly annoying) with 
> lines and lines of
>
> fixme:wave:DSD_CreateSecondaryBuffer 
> (0x454d16b8,0x4686cad8,ca,0,0x47a002ec,0x4041b054,0x47a002c8): stub
>
> and
>
> err:dsound:DSOUND_MixInBuffer length not a multiple of block size, len 
> = 281, block size = 4
>
> The first error I expected to see again, and I believe that this is 
> the one responsible for the popping noises. The second I thought could 
> be rectified by your patch, which seems to have tried to pad the 
> length to match a block size multiple, but it appears that either I 
> interpreted your comments wrong or the patch did not succeed.
>
> Also tested DeusEx - sound exhibited no errors. In the past the game 
> had always sounded just a bit fuzzy, although I have not played it in 
> so long that it might not be your patch that fixed this issue.
>
> I've got a couple other apps that I can test. Will report back.
>
> Randall
>
> James Hawkins wrote:
> > On 10/30/05, Randall Walls  wrote:
> > > With patch applied I can't get mixer.c to compile. I'm getting an error
> > > that 'still_behind' is undeclared, but apparently DSOUND_MixOne is
> > > attempting to use it.
> > >
> > > I get no errors when I patch mixer.c and audio.c. I'm patching against
> > > the 0.9 release, not against cvs, but I don't think that would 
> > > matter, I
> > > don't think that the content of the files has changed since the 
> > > release.
> > >
> > > I'll run through the patch and see if I can spot anything odd.
> >
> > Just comment out the TRACE that contains still_behind.  gcc reports
> > the file and line number of the error.
> >
> > -- 
> > James Hawkins

[Davin McCall]

On Tue, 01 Nov 2005 02:29:25 +0000
Randall Walls  wrote:
> Scratch that.... after spending about an hour playing Battlezone 2 
> tonight (with patch applied) I noticed that certain sounds (mainly 
> sounds longer than a few seconds) would exhibit a strange problem with 
> the last .5 seconds of the clip repeating for a random number of times.

Sounds like a buffer underrun. No pun intended.

ALSA is a strange beast; I'm still trying to figure out if my patch increases the likelihood of underruns. It depends on the semantics of some ALSA functions which aren't terribly well documented. I'm joining the ALSA developers mailing list to try and get some answers.

(You are using the ALSA driver, by the way? With hardware "acceleration" turned on?)
> The only console messages are the same as I noted before. I can run a 
> trace if it would be beneficial for you.

Don't worry about it for now. There may well be a second version of the patch forthcoming fairly soon.

Davin

[Dan Kegel]

I'm seeing a lot of stuttering in the sound for Kid Pix 4; see
http://bugs.winehq.org/show_bug.cgi?id=3553
for a log with last night's cvs.
Should I try your patch?

[Davin McCall]

By all means.
Davin

On Mon, 31 Oct 2005 22:06:20 -0800
Dan Kegel  wrote:
> I'm seeing a lot of stuttering in the sound for Kid Pix 4; see
> http://bugs.winehq.org/show_bug.cgi?id=3553
> for a log with last night's cvs.
> Should I try your patch?

[Robert Reif]

Randall Walls wrote:
> OK,
>
> Tested with Battlezone 2. Both patched and unpatched the in-game (non 
> movie) sound exhibits a popping sound (minor but mildly annoying) with 
> lines and lines of
>
> fixme:wave:DSD_CreateSecondaryBuffer 
> (0x454d16b8,0x4686cad8,ca,0,0x47a002ec,0x4041b054,0x47a002c8): stub

This fixme is harmless.  It just means your hardware supports hardware 
mixing but the wine OSS driver doesn't and falls back to software mixing.
> Also tested DeusEx - sound exhibited no errors. In the past the game 
> had always sounded just a bit fuzzy, although I have not played it in 
> so long that it might not be your patch that fixed this issue.

This may be related to this problem:

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

I have a patch which fixes some of the buffer underruns but all of them:

http://www.winehq.org/pipermail/wine-devel/2005-September/040049.html

There is also a bug in the wine ALSA driver when hardware
acceleration is set to Full and you have wine set to use the device hw:?
Direct sound expects the buffer size to remain the same when changing
primary buffer formats but the wine ALSA driver doesn't do that.  This
causes wine to use a buffer different from what ALSA uses.

Try to use the wine regression tests in wine/dlls/winmm/tests and
wine/dlls/dsound/tests in the interactive mode to find problems.

Also, if you find and fix a bug, create a test for it and add it to the
existing tests so the same bug can be found and fixed in the other
drivers or so the problem doesn't return later (regression).

[Davin McCall]

As James Hawkins pointed out, one of the TRACEs still references still_behind, a variable which I removed. I'll remove the trace as well. Thanks for bringing this up (I don't enable traces myself - call me archaic but I still prefer inserting/removing printf statements).

Davin

On Sun, 30 Oct 2005 19:14:52 +0000
Randall Walls  wrote:
> With patch applied I can't get mixer.c to compile. I'm getting an error 
> that 'still_behind' is undeclared, but apparently DSOUND_MixOne is 
> attempting to use it.
>
> I get no errors when I patch mixer.c and audio.c. I'm patching against 
> the 0.9 release, not against cvs, but I don't think that would matter, I 
> don't think that the content of the files has changed since the release.
>
> I'll run through the patch and see if I can spot anything odd.
>
> Randall
>
> Davin McCall wrote:
> > Trying again as an inline attachment.
> >
> > Davin
> >
> > On Sun, 30 Oct 2005 15:31:15 +0100
> > Stefan Dösinger  wrote:
> > > Am Sonntag, 30. Oktober 2005 14:23 schrieb Davin McCall:
> > > > Good news - I have got it working. It required work in both the mixer and
> > > > the Wine ALSA sound driver. It works on my system if the ALSA driver is
> > > > used and hw emulation is NOT used (in winecfg).
> > > >
> > > > If anyone would like to test patches, give me a hoy. In particular I'd like
> > > > to make sure it doesn't break anything else - StarCraft is one of the few
> > > > Windows apps I actually have :-)
> > >
> > > Can you send me the patch / upload it somewhere? I can test it with a few 
> > > games and tell you if everything works as it should.
> > >
> > > stefan

[Vitaliy Margolen]

Sunday, October 30, 2005, 6:23:17 AM, Davin McCall wrote:
> Good news - I have got it working. It required work in both the mixer and
> the Wine ALSA sound driver. It works on my system if the ALSA driver is used
> and hw emulation is NOT used (in winecfg).
> If anyone would like to test patches, give me a hoy. In particular I'd like
> to make sure it doesn't break anything else - StarCraft is one of the few
> Windows apps I actually have :-)

The really good and simple test is winapm. I'm using an older version so there
is less problems with other unrelated stuff.

What I've found is that when you pause the sound wait a bit then resume, it's
like it never stopped playing (skips ahead by the length of the time it was
paused). It doesn't happen all the time though.

Second problem - some times after pause I'm getting white loud noise for a long
period of time.

Third problem - is when you try forward/rewind song number of times eventually
you get some really strange results with it..

With your patch only the second problem fixed. And the 3rd is match better (I've
managed to confuse it few times, but it's like it took some time before play
position got updated).

Also I tested you patch with several games: Heroes of Might & Magic III - butch
better! I can set HW Accel to Full and no sound glitches! Sweet!

HL1 & HL2 - Doesn't start at all in Full. HL1 crashes id different places on
exit in dsound with each setting, except Emulation. Sounds is match better and I
don't have any delays at all! Nice!

Thank you for attending to this abandoned baby. Your work is really appreciated!

Vitaliy

[Davin McCall]

Thanks to everyone who responded. It sounds like the patch might not have solved all the problems, but it's generally a good improvement.

I will do some more work yet before submitting anything to wine-patches (well, I've submitted one trivial patch already - it hasn't yet been accepted). ALSA with emulation and OSS both (still) do not work on my own system (OSS with emulation, acceleration not supported due to ALSAs crummy emulation). I want to at least sus out why - there may be a trivial fix lurking in there somewhere.

Also I want to look at the 'length is not a factor of blocksize' problem that Randall Walls mentioned (I see the same message sometimes when I exit StarCraft).

Will post another test patch here when I've made better progress.

Davin

On Mon, 31 Oct 2005 00:23:17 +1100
Davin McCall  wrote:
> Good news - I have got it working. It required work in both the mixer and the Wine ALSA sound driver. It works on my system if the ALSA driver is used and hw emulation is NOT used (in winecfg).
>
> If anyone would like to test patches, give me a hoy. In particular I'd like to make sure it doesn't break anything else - StarCraft is one of the few Windows apps I actually have :-)
>
> Davin