Re: Added PulseAudio sound driver

[Michael Stefaniuc]

NACK.

[e-mail] wrote:
> I'm submitting the PulseAudio patch from Arthur Talyor.

Guys, stop doing that as it doesn't increases the chance of the patch 
being accepted; quite the contrary.
> As most distros have PulseAudio by default Wine should provide upstream 
> support.

No, not really. winealsa.drv works just fine with PulseAudio too.
> Both Fedora and Mandriva have included this work and it is reported to 
> work very well.

As well as the ALSA driver.
> I used latest patch from http://art.ified.ca/?page_id=40
> I just did a autoconf and autoheader.
>
> This feature has been confirmed by popular vote at 
> http://bugs.winehq.org/show_bug.cgi?id=10495

They should stop voting for a redundant feature. If they want to have it 
they should help make the patch *PERFECT*. That's about the only chance 
the patch has to get in.

Just taking and old version of the winealsa.drv and replacing "ALSA" 
with "PULSE" doesn't cut it. And the patch already bitrotted.

bye
    michael

Added PulseAudio sound driver

[Rémi Assailly]

[resent]
Let's see.

If distros include this driver, they >> This feature has been  
confirmed by popular vote at
> > http://bugs.winehq.org/show_bug.cgi?id=10495
>
> They should stop voting for a redundant feature.

Nobody submitted to wine-patches before, I prefer let Alexandre chose  
what he wants to do with it.> If they want to have it they should help  
make the patch *PERFECT*. That's about the only chance
> the patch has to get in.

So, while your work is not perfect but working you never try to submit  
it. Direct X support is not *PERFECT*, wine is not *PERFECT*, but you  
can use it and improve it.

Added PulseAudio sound driver

[Rémi Assailly]

[resent]
Let's see.
> > This feature has been confirmed by popular vote at
> > http://bugs.winehq.org/show_bug.cgi?id=10495
>
> They should stop voting for a redundant feature.

Nobody submitted to wine-patches before, I prefer let Alexandre choose
what he wants to do with it.
> If they want to have it they should help make the patch *PERFECT*.  
> That's about the only chance the patch has to get in.

So, while your work is not perfect but working you never try to submit
it. Direct X support is not *PERFECT*, wine is not *PERFECT*, but you
can use it and improve it.

Added PulseAudio sound driver

[Michael Stefaniuc]

Rémi Assailly wrote:
> [resent]
> Let's see.
> > > This feature has been confirmed by popular vote at
> > > http://bugs.winehq.org/show_bug.cgi?id=10495
> >
> > They should stop voting for a redundant feature.
>
> Nobody submitted to wine-patches before, I prefer let Alexandre choose
> what he wants to do with it.

*Sigh*; sometimes I really really hate it that Alexandre is so polite. 
If he wouldn't his answer would be something along the lines of "No 
fucking way"... Just read the previous threads on wine-devel regarding a 
Wine PulseAudio driver; Alexandre stated his position.
> > If they want to have it they should help make the patch *PERFECT*. 
> > That's about the only chance the patch has to get in.
>
> So, while your work is not perfect but working you never try to submit
> it. Direct X support is not *PERFECT*, wine is not *PERFECT*, but you
> can use it and improve it.

I didn't say perfect working code but a perfect patch; those are two 
different things. I tried to make you hope that their might be a tiny 
tiny chance to override Alexandre's made up decision to not include a 
PulseAudio driver with great code.

Now a code review of the submitted patch:
- Copy of an old version of winealsa.drv hacked up for PulseAudio.
- Inconsistent indentations all over the place.
- Tabs have nothing to loose in a file with a "4 space" indentation level.
- Inconsistent naming of identifiers (functions, variables, etc.)
- Bad names used for parameters and variables.
- Commented out code.

The above is not acceptable for new code and alone a reason for 
rejecting the patch. Thanks, but no thanks, we already have basically 
seven audio drivers that look like that in Wine; we don't need the 
eighth one to bitrot.

And there are technical problems with the patch too, two I have seen 
only on a quick glance:
- The code is not Win64 ready; most other Wine audio drivers were 
already fixed.
- The driver provides the old DSound interface from DirectX 5. The 
header include aka dsdriver.h doesn't exist on a modern Windows version.

bye
    michael

Added PulseAudio sound driver

[Ben Klein]

2009/4/9 Michael Stefaniuc :
> NACK.
>
> [e-mail] wrote:
> > I'm submitting the PulseAudio patch from Arthur Talyor.
>
> Guys, stop doing that as it doesn't increases the chance of the patch being
> accepted; quite the contrary.
> > As most distros have PulseAudio by default Wine should provide upstream
> > support.
>
> No, not really. winealsa.drv works just fine with PulseAudio too.

Sorry, but Ubuntu and Fedora (and derivatives) do not constitute "most distros".

Also, winealsa.drv doesn't do very well with PulseAudio on most
systems. Some people report better success with wineoss + padsp, but
it's still not a real solution. Real solution is to either have a
libpulse driver with 0 additional latency issues (as opposed to an
ALSA driver for cases when ALSA uses the pulse plugin), or to kill
PulseAudio and just use ALSA directly.
> > Both Fedora and Mandriva have included this work and it is reported to
> > work very well.

Sorry, but Ubuntu and Fedora and Mandriva (and derivatives) still do
not constitute "most distros". I didn't realise they also shipped it
by default.