Wine PulseAudio Driver

[Arthur Taylor]

"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

[Juan Lang]

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

[Arthur Taylor]

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

[Juan Lang]

> > 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

[Arthur Taylor]

> 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:
> - 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.
> - 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]

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?

[Arthur Taylor]

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?
> - 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

[Juan Lang]

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

[Arthur Taylor]

> 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

[Juan Lang]

> 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

[Arthur Taylor]

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

[Scott Ritchie]

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

[Reece Dunn]

2008/9/29 Scott Ritchie :
> 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

[Scott Ritchie]

Reece Dunn wrote:
> 2008/9/29 Scott Ritchie :
> > 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

[Maarten Lankhorst]

Hi Arthur,

2008/9/27, Arthur Taylor :
> "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.