[MPlayer-dev-eng] [PATCH] PulseAudio audio output

[Lennart Poettering]

On Mon, 17.07.06 14:35, Reimar Doeffinger (Reimar.Doeffinger at stud.uni-karlsruhe.de) wrote:

> > Neither do I like "senseless" renames. But this one isn't senseless. 
> > See this blog story for an explanation:
> > 
> > http://0pointer.de/blog/projects/pulse.html
> 
> If that's the same as I read it quite fits my definition of senseless.
> But that's actually mostly irrelevant.

As I said: a less controversial name was something like a requirement to
get PA into the big distros and accepted as future sound server for
the desktop. And you don't think that this is worth the effort?

> > Polypaudio never got widespread acceptance, thus we decided to do a
> > clean cut and not support the older versions any more.
> 
> I don't think it's acceptable to rip out functionality where the
> replacement is really young and untested.

> If there's no way to merge the two so older versions are supported as
> well IMHO ao_polyp must stay.

Take it from the main developer of Polypaudio/PulseAudio: Polypaudio
never gained much acceptance. The number of users was absolutely
negectible and all of them have already switched to PulseAudio. How do
I now that? The old ao_polyp hasn't been working for quite a while and
nobody noticed.

Keeping the old driver in the sources is a call for bitrotting. In
fact the bitrotting is already there. 

Sometimes old, unsupported code should just die, especially when
nobody is actually using it.

> > The API changes are usually cleanups. But this shouldn't matter to
> > MPlayer. 
> 
> If it means the same code (with maybe a few #ifdefs) can't be used for
> both API it matters a lot.

The PulseAudio/Polypaudio API was explicitly declared unstable in
versions < 0.9. Only since 0.9 we declared API stability. I see not
much point in supporting old APIs that were actually more "developer
previews" than proper, well defined APIs.

> > The problem with "svn mv" in this case is that the patch generated
> > with "make diff" is no longer directly applicable to the source tree
> > with "patch" because it doesn't contain the information that
> > ao_pulse.c is actually based on ao_polyp.c.
> 
> "make diff"? I think svn diff contains that information, but only in
> a human-readable form, not something that patch can use. Also svn
> copy/move should be used when applying, so this patch will avoid
> making a mistake here.

I am sorry, i meant "svn diff" instead of "make diff". And it doesn't
contain that info, neither usable for "patch", nor human
readable. Just check the patch:

> > http://0pointer.de/public/mplayer-pulse-with-mv.patch
> 
> > Ahem. What's the point of that? The intermediate version would not
> > compile and would be strangely named because the file would be called
> > "ao_polyp.c" and the internals are actually called "pulse". It makes
> > much more sense to do that change atomically. 
> 
> Well, you were talking about a rename, I didn't realize that you
> actually (as far as the API is concerned) you meant a complete
> rewrite.

It's not a complete rewrite. But yes, I touched every single function
ina ao_polyp.c

> > > Judging by the number of pa_threaded_mainloop* stuff this was not
> > > exactly introduced in a transparent way. Isn't there an API that gets
> > > away with a bit less locking and provides a synchronous API?
> > 
> > The answer is no. There must be some communication between the
> > playback thread and the main thread. In modern kernels (which futexes
> > and stuff) locking doesn't hurt much and this stuff is not really time
> > critical anyway. 
> 
> I don't care about speed in this case, and if it requires a 10GHz CPU to
> play a MP3. The point I was trying to make is that it is a lot of code
> just to emulate a synchronous API with a lot of ugly whiles.

Yes, I must agree. Our asynchronous API is not fun to play with. I
guess this is a problem with asynchronous APIs in general. 

> > PulseAudio offers a simplified synchronous API. But this API is
> > mostly just a wrapper around the async API whith exactly the same
> > pa_threaded_mainloop* stuff used internally. 
> 
> Well, if it saves 1/5 of the code, I'd say that's a very good reason to
> use it. I didn't read the code well, but my impression was that the
> asynchronous, locking based API has no advantage due to the way it is
> used.

I am both the author of ao_polyp/ao_pulse and of our simple API. If
the simple API would have been sufficient for usage in Mplayer i would
have used it. But unfortunately it isn't. Why? Because it only offers
a subset of the features the async API offers. In addition some of the
operations in ao_pulse.c are executed asynchronously without waiting
for the response. This is impossible with the "simple" API.

Yes, I could have chosen to beef up the "simple" API instead. But - if
I did that it wouldn't be that simple anymore. 

> > > > * Proper surround sound support
> > 
> > > Should be a separate patch as well (unless it is directly connected to
> > > the new API).
> > 
> > We get that actually nearly for free with the new API. Not activating
> > this is a matter of a single function argument. I don't see a point in
> > removing that argument for a seperate patch and than adding it again. 
> 
> So this means surround support is not possible or at least not possible
> in the same way in the old API? That's why I said "unless it is directly
> connected to the new API".

The API used in ao_polyp.c was the one of Polypaudio 0.7. However,
only Polypaudio 0.8 added proper support for surround sound.

> I just find it unjustified to call this a "rename" when it actually
> seems to break any program using the old API. Then it's actually a new
> thing and should be treated as such. And the news entry on the homepage
> isn't clear about how massive the changes seem to be.

This is the announcement of Polypaudio 0.8:

http://0pointer.de/blog/projects/polypaudio-0.8.html

It contains a list of the changes since Polypaudio 0.7 (which as I
said is the version ao_polyp.c was developed for.)

Lennart

-- 
Lennart Poettering; lennart [at] poettering [dot] net
ICQ# 11060553; GPG 0x1A015CC4; http://0pointer.net/lennart/