"If someone wants to write a pulseaudio driver, he should feel free to do so, but he should be aware that just writing a simple 20 line linear PCM out driver is not sufficient." http://bugs.winehq.org/show_bug.cgi?id=10495 I have written a waveout/in driver for wine to use pulseaudio which is in bugzilla. I would appreciate feedback on it, as I have received little thus far. I would appreciate it also if people took care to ensure that their instance of pulseaudio is properly setup before trying. Current problems include properly recovering from disconnects of both audio streams and server connections. WaveIn is currently buggy and usually has a larger latency. Multiple instances of one sink/source is also currently a problem. Currently WaveIn / WaveOut wodMessage / widMessage functions use "device IDs" which are basically array indexes of an array of available "devices." Without instance information it is hard to allow an application to open the same device more than once. Thanks - Art
Hi Arthur,
I have written a waveout/in driver for wine to use pulseaudio which is in bugzilla. I would appreciate feedback on it, as I have received little thus far.
Great. In general, emailing the patch here will get more feedback than posting it to bugzilla.
I would appreciate it also if people took care to ensure that their instance of pulseaudio is properly setup before trying.
How do we do that, if we're so inclined? Thanks, --Juan
On Sat, 2008-09-27 at 11:37 -0700, Juan Lang wrote:
Hi Arthur,
I have written a waveout/in driver for wine to use pulseaudio which is in bugzilla. I would appreciate feedback on it, as I have received little thus far.
Great. In general, emailing the patch here will get more feedback than posting it to bugzilla.
Woots!
I would appreciate it also if people took care to ensure that their instance of pulseaudio is properly setup before trying.
How do we do that, if we're so inclined?
Firstly, ensure pulseaudio is running, and works (paplay <sound file>.) Secondly, ensure the pulseaudio daemon has realtime privileges as it stops playback stutter. Thirdly check to see that your default sample spec and the sample spec of your sinks are sane. Either `paman` or using `pacmd`'s CLI will tell you. Fourthly, pulseaudio prefers dirrect hw access. You can run PA through dmix, but it gets silly. The glitch-free stuff in 0.9.11 for ALSA is pretty sweet. Dynamic hw buffers... Also, in the patch, configure checks for pulseaudio >= 0.9.7. I have since noticed that it uses parts of the api from 0.9.11 and compilation will fail against previous versions. Thanks -- Arthur Taylor <theycallhimart(a)gmail.com>
Great. In general, emailing the patch here will get more feedback than posting it to bugzilla.
Woots!
Um.. I still don't see the patch? Sorry, but I'm too lazy to get it myself. Could you send it here? Also, just to warn you: some people disagree that a PulseAudio driver is a good idea. Even if you write a good one, it might have to live out of tree. The usual arguments are: 1. Drivers can bitrot (and have in the past), and having 6 poorly implemented audio drivers is clearly worse than having one good one, even if that one is using a deprecated API and doesn't work nicely with other apps. 2. We ought to be able to use PulseAudio's ALSA emulation. If that doesn't work, then either our ALSA driver should be fixed, or PulseAudio's ALSA interface should be fixed. I have no particular opinion and whether this gets accepted isn't up to me. I'm just reiterating what's been said about audio drivers in the past. So, please don't attempt to persuade me, as it won't make any difference ;-) --Juan
Um.. I still don't see the patch? Sorry, but I'm too lazy to get it myself. Could you send it here?
Attached is a newer copy from the one currently in bugzilla. I'm still not sure of the proper procedure for doing things as this is probably my largest contribution yet. (Set the driver using the registry, winecfg patch is separate.)
Also, just to warn you: some people disagree that a PulseAudio driver is a good idea. Even if you write a good one, it might have to live out of tree. The usual arguments are: 1. Drivers can bitrot (and have in the past), and having 6 poorly implemented audio drivers is clearly worse than having one good one, even if that one is using a deprecated API and doesn't work nicely with other apps.
I've noticed a lot of hostility toward pulseaudio in general ;-) My thought is that if x11 was thrust on people today everyone would hate it too, but that isn't a good comparison as x11 is ancient by comparison and de-facto.
2. We ought to be able to use PulseAudio's ALSA emulation. If that doesn't work, then either our ALSA driver should be fixed, or PulseAudio's ALSA interface should be fixed.
IMHO this is a situation were it makes more sense to use pulse's native API over going through ALSA's, as we aren't just trying to join ALSA-pulse, we are trying to join WaveOut-ALSA-pulse (or even dsound on top of waveout.) Currently I can match the alsa driver's perceptive quality (buzz word) on my system using this code, dsound games and all. Mind you, my sound hardware is nothing to write home about. By far beats the current ALSA-pulse or ESD solutions.
I have no particular opinion and whether this gets accepted isn't up to me. I'm just reiterating what's been said about audio drivers in the past. So, please don't attempt to persuade me, as it won't make any difference ;-) --Juan
To wine developers: would it be useful to review dlls/winmm? Currently all mm-drivers (aux, mid, mod, wid, wod) functions are called using MDRV_Message calls. The arguments of a MDRV_Message are the device id (an array index), the message identifier, dwUser as the driver instance, and two parameters that depend upon the message. The driver instance DWORD seems useless to me, and it's value, while consistent, looks uninitialized (on my system is either 57 if I enable tracing, or random crazy large for a pointer if not.) My problem is that WaveIn / WaveOut devices are supposed to be able to support multiple streams if the hardware allows it, however it is impossible to support this currently as all MDRV_Message gives us is the device id the message is for (the only arguments to a WODM_PAUSE is the device id for instance.) This makes multiple streams impossible and therefor waveout drivers only support one stream per device. wineesd.drv gets around this by staticly making 10 waveout and 10 wavein devices, all effectively the same. If MDRV_Message were to pass in dwUser the device opening instance handle which winmm creates and apps use for the wave{out,in}* calls it would be possible to support multiple streams per device, and devices could represent hardware 1:1 like they are supposed to. So my questions are this: would changing the value of dwUser to the device instance handle be a good idea, and do any current mm-drivers depend upon dwUser being the seemingly uninitialized driver instance value? Thanks -- Arthur Taylor <theycallhimart(a)gmail.com>
Umm, disregard all of this... sorry for spam
To wine developers: would it be useful to review dlls/winmm? Currently all mm-drivers (aux, mid, mod, wid, wod) functions are called using MDRV_Message calls. The arguments of a MDRV_Message are the device id (an array index), the message identifier, dwUser as the driver instance, and two parameters that depend upon the message. The driver instance DWORD seems useless to me, and it's value, while consistent, looks uninitialized (on my system is either 57 if I enable tracing, or random crazy large for a pointer if not.) My problem is that WaveIn / WaveOut devices are supposed to be able to support multiple streams if the hardware allows it, however it is impossible to support this currently as all MDRV_Message gives us is the device id the message is for (the only arguments to a WODM_PAUSE is the device id for instance.) This makes multiple streams impossible and therefor waveout drivers only support one stream per device. wineesd.drv gets around this by staticly making 10 waveout and 10 wavein devices, all effectively the same. If MDRV_Message were to pass in dwUser the device opening instance handle which winmm creates and apps use for the wave{out,in}* calls it would be possible to support multiple streams per device, and devices could represent hardware 1:1 like they are supposed to. So my questions are this: would changing the value of dwUser to the device instance handle be a good idea, and do any current mm-drivers depend upon dwUser being the seemingly uninitialized driver instance value?
I have put a newer patch in bugzilla: http://bugs.winehq.org/attachment.cgi?id=16412&action=edit Currently I am looking for any feedback at all. Should I send a message to wine-patches as well?
2. We ought to be able to use PulseAudio's ALSA emulation. If that doesn't work, then either our ALSA driver should be fixed, or PulseAudio's ALSA interface should be fixed.
IMHO this is a situation were it makes more sense to use pulse's native API over going through ALSA's, as we aren't just trying to join ALSA-pulse, we are trying to join WaveOut-ALSA-pulse (or even dsound on top of waveout.) Currently I can match the alsa driver's perceptive quality (buzz word) on my system using this code, dsound and all. By far beats the current ALSA-pulse or ESD solutions. Thanks - Art
Hi Art,
Currently I am looking for any feedback at all.
I believe you've already gotten some: split up your patches so that we can read them a little more easily. If you send a large patch as a new contributor, it's very unlikely to get committed. Also, this function: +/* +const char * PULSE_getMessage(UINT msg) { + static char unknown[32]; is commented out. Please leave stuff like that out of your patches, it's a little distracting. That was the only instance of that I saw in the patch, so your patch looks of generally good quality. Still, try to make your patches as clean as possible.
Should I send a message to wine-patches as well? Only if it's ready to be committed. Since you're asking for feedback, I assume that's not the case ;-) Since you're asking for feedback: what, in particular, do you want feedback on? Are you soliciting "does this work for you" feedback? Is there some aspect of the design you're not sure about?
Also, I'm happy to try it, but the bug is just asking for a feature, not describing something that doesn't work without this. Is there some other criterion I can use to evaluate it? Doesn't steal audio, doesn't stutter as much, ??? --Juan
Hi Art,
Currently I am looking for any feedback at all.
I believe you've already gotten some: split up your patches so that we can read them a little more easily. If you send a large patch as a new contributor, it's very unlikely to get committed.
Humm, I can see splitting the changes to configure.ac and Makefile.in, but as for splitting the rest of it, should I split it per-file?
Is there some other criterion I can use to evaluate it? Doesn't steal audio, doesn't stutter as much, ???
I'm looking for instances where the patch falls on its face so I can fix them (where falling on it's face is defined as) - Crashing / stalling an app - Crashing the PA server - Continual stuttering or no audio. I'm also looking for comments on the code. Is it sane, are the variable names okay? Are there functions I should divide / join? Thanks - Art
Humm, I can see splitting the changes to configure.ac and Makefile.in, but as for splitting the rest of it, should I split it per-file?
No. Patches should be split along functional lines. E.g., patch 1: add stub driver patch 2: add small feature x to driver patch 3: add small feature y to driver patch 4: add pulseaudio option to winecfg
I'm looking for instances where the patch falls on its face so I can fix them (where falling on it's face is defined as) - Crashing / stalling an app - Crashing the PA server - Continual stuttering or no audio.
Ah. For that I expect you'll need to solicit a wider audience. I don't actually use Wine for all that much, hacking on it is my hobby. You might build it and ask for users at wine-users to give feedback, just as a thought.
I'm also looking for comments on the code. Is it sane, are the variable names okay? Are there functions I should divide / join?
That's much harder to do until you've split up your driver a lot more. --Juan
Hello. I have updated the testing pulseaudio waveout patch for http://bugs.winehq.org/show_bug.cgi?id=10495 The patch has been split in two. The first adds a stub driver that just creates waveout devices based upon pulseaudio sinks and adds checks for pulseaudio to configure.ac. The second patch adds the actual waveout functionality. The patch now detects pulseaudio version prior to 0.9.11 (aka most ubuntu versions) and can build against them. I hope to receive your feedback on this. Thanks - Art
Arthur Taylor wrote:
Also, in the patch, configure checks for pulseaudio >= 0.9.7. I have since noticed that it uses parts of the api from 0.9.11 and compilation will fail against previous versions.
Thanks
As an aside, 0.9.11 is newer than most users have (Ubuntu 8.04 has 0.9.10, for instance). This isn't a bad thing at the moment, just something to be aware of. Thanks, Scott Ritchie
2008/9/29 Scott Ritchie <scott(a)open-vote.org>:
Arthur Taylor wrote:
Also, in the patch, configure checks for pulseaudio >= 0.9.7. I have since noticed that it uses parts of the api from 0.9.11 and compilation will fail against previous versions.
Thanks
As an aside, 0.9.11 is newer than most users have (Ubuntu 8.04 has 0.9.10, for instance). This isn't a bad thing at the moment, just something to be aware of.
Intrepid (Ubuntu 8.10) is using the same version - 0.9.10. $ paplay --version paplay 0.9.10 Compiled with libpulse 0.9.10 Linked with libpulse 0.9.10 - Reece
Reece Dunn wrote:
2008/9/29 Scott Ritchie <scott(a)open-vote.org>:
Arthur Taylor wrote:
Also, in the patch, configure checks for pulseaudio >= 0.9.7. I have since noticed that it uses parts of the api from 0.9.11 and compilation will fail against previous versions.
Thanks As an aside, 0.9.11 is newer than most users have (Ubuntu 8.04 has 0.9.10, for instance). This isn't a bad thing at the moment, just something to be aware of.
Intrepid (Ubuntu 8.10) is using the same version - 0.9.10.
$ paplay --version paplay 0.9.10 Compiled with libpulse 0.9.10 Linked with libpulse 0.9.10
- Reece
Yeah. For a while I thought Intrepid might get 0.9.11, but then 0.9.12 came out and it had some regressions so they'll probably stick with 0.9.11. Thanks, Scott Ritchie
Hi Arthur, 2008/9/27, Arthur Taylor <theycallhimart(a)gmail.com>:
"If someone wants to write a pulseaudio driver, he should feel free to do so, but he should be aware that just writing a simple 20 line linear PCM out driver is not sufficient." http://bugs.winehq.org/show_bug.cgi?id=10495
I have written a waveout/in driver for wine to use pulseaudio which is in bugzilla. I would appreciate feedback on it, as I have received little thus far. I would appreciate it also if people took care to ensure that their instance of pulseaudio is properly setup before trying.
Current problems include properly recovering from disconnects of both audio streams and server connections. WaveIn is currently buggy and usually has a larger latency. Multiple instances of one sink/source is also currently a problem. Currently WaveIn / WaveOut wodMessage / widMessage functions use "device IDs" which are basically array indexes of an array of available "devices." Without instance information it is hard to allow an application to open the same device more than once. As the maintainer of the winealsa driver I don't feel a good written pulseaudio driver is bad per se. But if you want a complete review, you will need to split up the patches in easily reviewable chunks, and then send the patch series to wine-patches. This allows others to review it as well.
First in the patch series that will create a simple waveout driver in chunks. Then capture, then accelerated waveout, then accelerated capture. Cheers, Maarten.
participants (6)
-
Arthur Taylor -
Arthur Taylor -
Juan Lang -
Maarten Lankhorst -
Reece Dunn -
Scott Ritchie