http://bugs.winehq.org/show_bug.cgi?id=13204
Summary: winealsa.drv makes incorrect assumptions about hardware Product: Wine Version: unspecified Platform: PC OS/Version: Linux Status: UNCONFIRMED Severity: enhancement Priority: P2 Component: -unknown AssignedTo: wine-bugs@winehq.org ReportedBy: galtgendo@o2.pl
The sound drivers should be among components to select. Now the bug: this is the conclusion of my own research and a bit of googling. I decided to open a new bug for it to make it more outstanding. The material for this bug: my comments in http://bugs.winehq.org/show_bug.cgi?id=13090 a comment in alsa bug tracking - https://bugtrack.alsa-project.org/alsa-bug/view.php?id=3943 and finally a post on pulseaudio-discuss from pulseaudio author https://tango.0pointer.de/pipermail/pulseaudio-discuss/2008-May/001785.html
The short of it is: using snd_pcm_delay for checking, if all sound has been played is plain wrong, and breaks not only pulseaudio, but is probably responsible for http://bugs.winehq.org/show_bug.cgi?id=12228 (and even if it's not it's still broken)
http://bugs.winehq.org/show_bug.cgi?id=13204
--- Comment #1 from Rafał Mużyło galtgendo@o2.pl 2008-05-15 07:20:47 --- Created an attachment (id=13076) --> (http://bugs.winehq.org/attachment.cgi?id=13076) Patch that works for me
This patch works for me, it's applied to 1.0_rc1 (so still with the incorrectly reverted no-deprecated-alsa patch). One part of it is what seemed to be missing in the depr. patch. The rest is based on some guess work. I've never really worked with threads, so I don't actually understand them. However, if the InterlockedExchange is really needed, that seemed like a good place for it. As you may see, wodUpdatePlayedTotal is now a noop.
http://bugs.winehq.org/show_bug.cgi?id=13204
--- Comment #2 from Vitaliy Margolen vitaliy@kievinfo.com 2008-05-17 01:22:17 --- That's the way DirectSound works - it's broken by design. But that is what lots of applications depend on. The bug you point to is broken for a different reason. And what you proposing doesn't work for Wine. Looks like pulseaudio folks have to really fix their stuff.
This bug is a won't fix - this is intended functionality that lots of dsound apps depend on. Unless you have a better way to calculate current playing position?
http://bugs.winehq.org/show_bug.cgi?id=13204
--- Comment #3 from Rafał Mużyło galtgendo@o2.pl 2008-05-17 05:13:35 --- (In reply to comment #2)
That's the way DirectSound works - it's broken by design. But that is what lots of applications depend on. The bug you point to is broken for a different reason. And what you proposing doesn't work for Wine. Looks like pulseaudio folks have to really fix their stuff.
Could you please elaborate on "what you proposing doesn't work for Wine" ? It does seem to work for more, no obvious crackling and the sound is there, as opposed to complete silence and Wine stuck in 'waiting to finish playing'. Did you actually test my patch or simply looked at it and deemed it wrong ? Also both pulseaudio AND alsa developers claim that using snd_pcm_delay, the way you use it, is definitely incorrect. And at least alsa team definitely knows this stuff.
http://bugs.winehq.org/show_bug.cgi?id=13204
Tomas Carnecky tom@dbservice.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |tom@dbservice.com
http://bugs.winehq.org/show_bug.cgi?id=13204
--- Comment #4 from Tomas Carnecky tom@dbservice.com 2008-05-18 04:22:18 --- I have to agree with Vitaly, this patch is definitely not correct. Did you run the winmm wave test to see if it still passes? I bet it does not, and the patch thus won't be accepted. Instead you should ask the alsa developers what better way there is to find out if a sample has been already played and use that. If there isn't, then I'm afraid this bug will have to be closed. And then you will have to hope for a native pulse driver.
http://bugs.winehq.org/show_bug.cgi?id=13204
--- Comment #5 from Rafał Mużyło galtgendo@o2.pl 2008-05-18 06:31:27 --- I'm happy to finally get some feedback. My answer is that you're both right and wrong. While after my patch, `wine winmm_test wave` output only:
fixme:mixer:ALSA_MixerInit No master control found on MPU-401 UART, disabling mixer fixme:mixer:ALSA_MixerInit No master control found on VirMIDI, disabling mixer wave.c:1375:found 1 WaveOut devices wave.c:898: 0: "default" (winealsa: default) 1.0 (2:104) wave.c:901: channels=32 formats=0ffff support=002c wave.c:903: WAVECAPS_VOLUME WAVECAPS_LRVOLUME WAVECAPS_SAMPLEACCURATE wave.c:694:Playing 1 second silence at 22050x 8x1 1 header 0 loops 22050 bytes WAVE_FORMAT_PCM CALLBACK_EVENT
then stops indefinitely, before that patch (in 1.0_rc1), the output was:
fixme:mixer:ALSA_MixerInit No master control found on MPU-401 UART, disabling mixer fixme:mixer:ALSA_MixerInit No master control found on VirMIDI, disabling mixer wave.c:1375:found 1 WaveOut devices wave.c:898: 0: "default" (winealsa: default) 1.0 (2:104) wave.c:901: channels=32 formats=0ffff support=002c wave.c:903: WAVECAPS_VOLUME WAVECAPS_LRVOLUME WAVECAPS_SAMPLEACCURATE wave.c:694:Playing 1 second silence at 22050x 8x1 1 header 0 loops 22050 bytes WAVE_FORMAT_PCM CALLBACK_EVENT wave.c:765: Test failed: The sound played for 1139 ms instead of 1000 ms wave.c:694:Playing 1 second silence at 22050x 8x1 1 header 0 loops 22050 bytes WAVE_FORMAT_PCM CALLBACK_EVENT wave.c:739:pausing for 0.5 seconds wave.c:694:Playing 1 second silence at 22050x 8x1 1 header 0 loops 22050 bytes WAVE_FORMAT_PCM CALLBACK_FUNCTION wave.c:694:Playing 1 second silence at 22050x 8x1 1 header 0 loops 22050 bytes WAVE_FORMAT_PCM CALLBACK_FUNCTION wave.c:739:pausing for 0.5 seconds wave.c:694:Playing 1 second silence at 22050x 8x1 1 header 0 loops 22050 bytes WAVE_FORMAT_PCM CALLBACK_THREAD wave.c:694:Playing 1 second silence at 22050x 8x1 1 header 0 loops 22050 bytes WAVE_FORMAT_PCM CALLBACK_THREAD wave.c:739:pausing for 0.5 seconds wave.c:694:Playing 1 second silence at 22050x 8x1 10 headers 0 loops 22050 bytes WAVE_FORMAT_PCM CALLBACK_EVENT and then it stopped indefinitely, too. So it was broken even before my patch.
http://bugs.winehq.org/show_bug.cgi?id=13204
--- Comment #6 from Tomas Carnecky tom@dbservice.com 2008-05-18 06:49:40 --- It was broken before, that's correct, but your patch didn't fix it. So your patch is wrong. Only if the patch doesn't cause any regressions _and_ fixes the alsa driver when using pulseaudio it will be accepted.
http://bugs.winehq.org/show_bug.cgi?id=13204
--- Comment #7 from Rafał Mużyło galtgendo@o2.pl 2008-05-18 06:50:04 --- And, while I can't really check that (unless I would fetch git of alsa-plugins, I'll think about it), failure with my patch may be simply related to the fact that current alsa-pulse plugin doesn't respect start_threshold (https://bugtrack.alsa-project.org/alsa-bug/view.php?id=3944), while the one without my patch is definitely related to your incorrect use of snd_pcm_delay.
http://bugs.winehq.org/show_bug.cgi?id=13204
Rafał Mużyło galtgendo@o2.pl changed:
What |Removed |Added ---------------------------------------------------------------------------- Attachment #13076|0 |1 is obsolete| |
--- Comment #8 from Rafał Mużyło galtgendo@o2.pl 2008-05-18 09:37:47 --- Created an attachment (id=13154) --> (http://bugs.winehq.org/attachment.cgi?id=13154) updated pactch
I got bit futher with this one. While a few tests fail, I don't think those are critical failures. It still hangs, but quite a bit futher. wave.c:739:pausing for 0.5 seconds wave.c:694:Playing 1 second silence at 22050x 8x1 1 header 0 loops 22050 bytes WAVE_FORMAT_PCM CALLBACK_FUNCTION wave.c:767: Test failed: The sound played for 479 ms instead of 1000 ms wave.c:694:Playing 1 second silence at 22050x 8x1 1 header 0 loops 22050 bytes WAVE_FORMAT_PCM CALLBACK_FUNCTION wave.c:739:pausing for 0.5 seconds fixme:mixer:ALSA_MixerInit No master control found on VirMIDI, disabling mixer wave.c:1377:found 1 WaveOut devices wave.c:900: 0: "default" (winealsa: default) 1.0 (2:104) wave.c:903: channels=32 formats=0ffff support=002c wave.c:905: WAVECAPS_VOLUME WAVECAPS_LRVOLUME WAVECAPS_SAMPLEACCURATE wave.c:694:Playing 1 second silence at 22050x 8x1 1 header 0 loops 22050 bytes WAVE_FORMAT_PCM CALLBACK_EVENT wave.c:767: Test failed: The sound played for 479 ms instead of 1000 ms wave.c:694:Playing 1 second silence at 22050x 8x1 1 header 0 loops 22050 bytes WAVE_FORMAT_PCM CALLBACK_EVENT wave.c:767: Test failed: The sound played for 1920 ms instead of 1500 ms wave.c:694:Playing 1 second silence at 22050x 8x1 1 header 0 loops 22050 bytes WAVE_FORMAT_PCM CALLBACK_THREAD wave.c:767: Test failed: The sound played for 514 ms instead of 1000 ms wave.c:694:Playing 1 second silence at 22050x 8x1 1 header 0 loops 22050 bytes WAVE_FORMAT_PCM CALLBACK_THREAD wave.c:739:pausing for 0.5 seconds wave.c:767: Test failed: The sound played for 1940 ms instead of 1500 ms wave.c:694:Playing 1 second silence at 22050x 8x1 10 headers 0 loops 22050 bytes WAVE_FORMAT_PCM CALLBACK_EVENT wave.c:720: Test failed: WHDR_INQUEUE WHDR_PREPARED expected, got= WHDR_DONE WHDR_PREPARED wave.c:725: Test failed: waveOutWrite(0): WAVE_STILLPLAYING expected, got MMSYSERR_NOERROR(The specified command was carried out.) wave.c:729: Test failed: WHDR_INQUEUE WHDR_PREPARED expected, got WHDR_DONE WHDR_PREPARED
So now it hangs when there's more than one headers and despite those errors sound doesn't seem cutoff.
http://bugs.winehq.org/show_bug.cgi?id=13204
--- Comment #9 from Rafał Mużyło galtgendo@o2.pl 2008-05-18 09:40:00 --- And, as you may noticed (I failed to at first), this gets me to the point, where wine 1.0_rc1 was failing.
http://bugs.winehq.org/show_bug.cgi?id=13204
--- Comment #10 from Tomas Carnecky tom@dbservice.com 2008-05-18 10:01:01 --- You don't seem to know much about alsa. So please go ask the alsa developers which functions to use instead of trying to experiment until something works. This issue needs a proper patch, not some hacks.
This change for example: InterlockedExchange((LONG*)&wwo->dwPlayedTotal, wwo->dwWrittenTotal+snd_pcm_frames_to_bytes(wwo->pcm, written));
is totally wrong. This basically means that as soon as alsa has written the data to the soundcard, the frames are considered played. This will introduce all kinds of timing issues in applications. You have to find out a proper way to find out which frames have been played. If there is no way, then there's no point in trying to come up with patches.
http://bugs.winehq.org/show_bug.cgi?id=13204
--- Comment #11 from Rafał Mużyło galtgendo@o2.pl 2008-05-18 10:51:53 --- This patch being incorrect, doesn't make winealsa.drv less broken than it is now, and at least there's sound instead of silence. If you don't like it, at least add functionality to winecfg that allows to override 'default' name for alsa device with another string, cause as I said before, `with hw:0 it fails, with plughw:0,0 it works`.
http://bugs.winehq.org/show_bug.cgi?id=13204
--- Comment #12 from Tomas Carnecky tom@dbservice.com 2008-05-18 11:29:36 --- (In reply to comment #11)
This patch being incorrect, doesn't make winealsa.drv less broken than it is now,
I completely agree with you that the driver is buggy. However, incorrect patches don't improve anything. I'm not saying that you should stop doing whatever you do, but don't just test random changes to the code. Instead, look for a proper solution (if there is one): read the alsa API and ask the ALSA developers. They know their API best and will be able to tell you whether it's possible to do what the winealsa driver needs to do.
and at least there's sound instead of silence. If you don't like it, at least add functionality to winecfg that allows to override 'default' name for alsa device with another string, cause as I said before, `with hw:0 it fails, with plughw:0,0 it works`.
That is an entirely different issue. Feel free to open a new bug if there isn't already.
http://bugs.winehq.org/show_bug.cgi?id=13204
--- Comment #13 from Rafał Mużyło galtgendo@o2.pl 2008-05-18 11:53:10 --- In that case, a minor request, could you see if this patch doesn't WORK correctly, as opposed to not LOOKING correctly. And one more thing, with this patch, when I add WINEDEBUG for winmm and wave, I can see that the test hangs in trace:wave:wodPlayer_NotifyCompletions Empty queue trace:wave:wodPlayer waiting 11ms (11,4294967295) loop.
An on a related note: I think that the author of the driver should ask those questions to alsa developers, not me.
http://bugs.winehq.org/show_bug.cgi?id=13204
--- Comment #14 from Tomas Carnecky tom@dbservice.com 2008-05-18 13:33:43 --- (In reply to comment #13)
In that case, a minor request, could you see if this patch doesn't WORK correctly, as opposed to not LOOKING correctly.
Besides that your patch CAN'T WORK (if you analyze what it does and compare that with what it should do) ...
And one more thing, with this patch, when I add WINEDEBUG for winmm and wave, I can see that the test hangs in trace:wave:wodPlayer_NotifyCompletions Empty queue trace:wave:wodPlayer waiting 11ms (11,4294967295) loop.
... I don't even have to test it myself. If the winmm test fails then the patch is wrong (unless the tests are wrong which I doubt here). And btw, I tested it earlier today and the test hang for me, too.
An on a related note: I think that the author of the driver should ask those questions to alsa developers, not me.
This particular issue is currently low priority. There are valid workarounds: disable pulseaudio, pasuspend, paoss etc. If you want to have it fixed ASAP, you'll have to dig into it yourself.
http://bugs.winehq.org/show_bug.cgi?id=13204
--- Comment #15 from Rafał Mużyło galtgendo@o2.pl 2008-05-18 14:01:06 --- While this patch breaks current driver's logic, it doesn't break the sound, cause despite the tests I can hear the sound and it SEEMS fine. Those tests may fail simply because they rely on the old logic and, as I already mention (quite a few times), as of now it does NOT work and with this patch test stops in the same place were it does without it.
http://bugs.winehq.org/show_bug.cgi?id=13204
--- Comment #16 from Tomas Carnecky tom@dbservice.com 2008-05-18 14:10:19 --- The tests rely on Windows' logic. Wine has to mimic the Win32 API as accurately as possible. If the wine API behaves differently than the Win32 API, then that is a bug in wine. And the tests ensure exactly that. Developers use the tests to see how Windows behaves so they can see if wine behaves the same. Just because something seems to be correct, doesn't mean it is.
Did you test your patch with pulseaudio disabled? So that wine uses the native alsa driver and not the pulse plugin? As far as I know, the winmm wave test passes in that case. So you need to make sure that your patch passes in that situation as well.
http://bugs.winehq.org/show_bug.cgi?id=13204
--- Comment #17 from Rafał Mużyło galtgendo@o2.pl 2008-05-18 16:10:06 --- Well, for pure alsa (default = dmix:0), it hangs in the same place: in winmm/tests/wave.c: it goes into for (j = 0; j <= loops; j++) loop but does not leave it. It hangs on writing header 9 of 10. I can't quite figure out, what should be changed for this to work.
But still, issue of being able to override 'default' with custom string should be addressed.
http://bugs.winehq.org/show_bug.cgi?id=13204
--- Comment #18 from Rafał Mużyło galtgendo@o2.pl 2008-05-18 16:34:15 --- No, correction, it's hang is really WaitForSingleObject(hevent,INFINITE) executed on the last header, though I can't see why this fails only in test of 10 headers and not before.
http://bugs.winehq.org/show_bug.cgi?id=13204
--- Comment #19 from Rafał Mużyło galtgendo@o2.pl 2008-05-18 16:48:26 --- What's more, if I switch around test for 5 and 10 headers, test completes with 17 failures. Some of them are due incorrect time, so I'd ignore them for now, as I know I broke time counting, but the ones right after 5 headers give me: wave.c:791: Test failed: waveOutClose(0): rc=WAVERR_STILLPLAYING(Cannot perform this operation while media data is still playing. Reset the device, or wait until the data is finished playing.) This could give me a hint, what should I fix in my patch first, if I knew more about wine. Anybody else wants to give me that hint ?
http://bugs.winehq.org/show_bug.cgi?id=13204
Rafał Mużyło galtgendo@o2.pl changed:
What |Removed |Added ---------------------------------------------------------------------------- Attachment #13154|0 |1 is obsolete| |
--- Comment #20 from Rafał Mużyło galtgendo@o2.pl 2008-05-19 11:10:04 --- Created an attachment (id=13182) --> (http://bugs.winehq.org/attachment.cgi?id=13182) a bit more correct patch
This one updated dwPlayedTotal in a more controlled way. Still breaks time logic, but is closer to original. Tests get a bit further before they freeze.
http://bugs.winehq.org/show_bug.cgi?id=13204
--- Comment #21 from Vitaliy Margolen vitaliy@kievinfo.com 2008-05-23 14:05:41 --- (In reply to comment #20)
Created an attachment (id=13182)
--> (http://bugs.winehq.org/attachment.cgi?id=13182) [details]
a bit more correct patch
It is still wrong. Read MSDN to understand what played position means.
All games depend on that to trigger some game events. What's worse - it's used by Wine itself to trigger those position events in dsound. If you break it - you will get MUCH shortened audio effects in most games, too fast running scripts, not synchronized video, cut scenes, etc.
This is the way DSound API is made and this is how it's used by 99% of programs.
http://bugs.winehq.org/show_bug.cgi?id=13204
--- Comment #22 from Rafał Mużyło galtgendo@o2.pl 2008-05-23 17:52:09 --- Well, it looks like there's a real problem now. With 1.0_rc2 (without my patch), winmm_test wave sometimes completes (with 0-2 failures), but sometimes it hangs. What's worse, it does that in random places. Looks like a race condition.
However, sound seems to work. But there's another thing. Quoting from alsa.c: ** FIXME: It is probably short sighted to hard code and fixate on PCM Playback Volume Yeah, it's short sighted. While for pcm it seems to select hw:0 for default, for volume control it selects a different device, one that has only Master Playback Volume. Could, perhaps , some kind of fallback be implemented ?
http://bugs.winehq.org/show_bug.cgi?id=13204
--- Comment #23 from Rafał Mużyło galtgendo@o2.pl 2008-05-23 19:00:22 --- Well, no. It looks like my problem with volume lies elsewhere. I'll have to do more checking.
That problem is that for some reason when default == hw:0 main volume control doesn't work (it does for plughw:0,0)
http://bugs.winehq.org/show_bug.cgi?id=13204
--- Comment #24 from Rafał Mużyło galtgendo@o2.pl 2008-05-30 17:05:20 --- In case you want to know, in rc3 completing the test is still random.
http://bugs.winehq.org/show_bug.cgi?id=13204
--- Comment #25 from Rafał Mużyło galtgendo@o2.pl 2008-06-27 15:44:10 --- Created an attachment (id=14401) --> (http://bugs.winehq.org/attachment.cgi?id=14401) deprecated alsa functions removal
I think I'll drop the issue. Certain posts on both pulseaudio and alsa made me think that the problem is too complicated for me, as both wine and alsa are a problem. Wine works on incorrect assumptions and alsa doesn't seem to have a way to get the needed values in a sane way (at least not yet, this may eventually change).
Before I quit, I'll attach a patch, that is my guess of how that removal of deprecated alsa functions should have looked like. It's almost the same as the reverted git patch, except for the part, that I think was missing from the original patch.
http://bugs.winehq.org/show_bug.cgi?id=13204
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |patch
--- Comment #26 from Austin English austinenglish@gmail.com 2008-12-29 10:55:49 --- Is this still an issue in current (1.1.11 or newer) wine?
http://bugs.winehq.org/show_bug.cgi?id=13204
--- Comment #27 from Rafał Mużyło galtgendo@o2.pl 2008-12-29 14:52:53 --- Not really, unless you'll count in the leftovers - those little bits in bug 16416 comment 6 and bug 16607 comment 7.
http://bugs.winehq.org/show_bug.cgi?id=13204
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |RESOLVED Resolution| |FIXED
--- Comment #28 from Austin English austinenglish@gmail.com 2009-06-30 11:02:14 --- Reported fixed a long time ago...
http://bugs.winehq.org/show_bug.cgi?id=13204
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #29 from Alexandre Julliard julliard@winehq.org 2009-07-03 12:14:01 --- Closing bugs fixed in 1.1.25.