[vlc-devel] [PACKAGERS] [RFCv2] PulseAudio removal

[Rémi Denis-Courmont]

Le mardi 5 avril 2011 21:59:43 Colin Guthrie, vous avez écrit :
> > On a related note, most functions fail to document their return values.
> > Also some functions and structure parameters lack details in what they
> > really do. It might be obvious for PulseAudio developers. But I dare say
> > that they should not be the target *audience*. In particular, what
> > exactly is the stream "latency"? There are many ways to define latency
> > in the context of a PA (playback) stream.
> 
> Can you give me some examples here? (not necessarily all of them, but
> perhaps just a few and some sort of indication if other functions in a
> similar area of the API are also affected). That way I can make a point
> of going through them and adding appropriate documentation (or perhaps
> getting others to do so - there are several people on the PA mailing
> list who submit such documentation updates so in an ideal world I'd ask
> them!)

I believe you can find which functions don't declare their return without me.

My main problem has been with pa_stream_get_latency(). I don't know if it 
returns the latency for the read offset, the write offset, or yet something 
else. But as I still have not solved the sync problem, I don't really know 
what I need to use...

While I survived, it was also unclear to me:
* how pa_thread_mainloop_(start|stop) interacted, if at all with 
pa_threaded_mainloop_(lock|unlock),
* whether pa_..._unref() functions would implicitly call pa_..._disconnect() 
if needed
* whether the same applies for pa_threaded_mainloop_free() and ..._stop(),
* whether an operation is completed (except for errors) if it is unreferenced, 
* that the stream_moved callback is _not_ called at startup.

I have not been through volume control, recording and device enumeration as 
this is not implemented in VLC.

> I did happen to notice pa_stream_write() doesn't document it's return
> which is pretty bad :s (albeit not that important in terms of the the
> likelihood of it causing problems)

Yes...

(...)
> > There are different ways to construe backward compatibility. Evidently,
> > you do need to keep the intricate stuff that you have once exposed. But
> > you can still simplify things through wrappers around the complicated
> > stuff. If you mean to remove complexity completely, then indeed you have
> > to break backward compatibility.
> > 
> > VLC depends on libpulse >= 0.9.22, so we would have been totally fine
> > depending further "new" libpulse functions.
> 
> Can you give me a couple examples here?

One obvious thing would be synchronous function to wait for context and stream 
to become ready or fail when using the threaded mainloop. It seems all users 
have more or less the same callbacks boilerplate there... except the old VLC 
that lacked a loop around condition variable wait.

I was also annoyed that pa_stream_write() does not accept an opaque pointer 
for the free callback. VLC has an ugly kludge around this.

And then I'm still stuck with latency and synchronization.

> We're adding a couple new functions in the 1.0 API to deal with
> passthrough streams and I would be happy enough to do as you suggest
> with other "legacy" approaches if it's at all possible.

Unfortunately, it's a bit late, as a lot of water will go below the bridges 
before VLC can depend on 1.0. We probably won't have a convenient "excuse" 
like we had to depend on 0.9.22 ;-)

(...)
> I have to admit to only using VLC relatively infrequently and when I do,
> the PA output has never been so terrible it caused me any significant
> issues (the buffer metrics are not ideally chosen for network latencies
> but that's not a primary focus). While I know it was sub-optimal, I
> wasn't aware it was an issue that was so problematic. I apologise for
> not getting a fuller picture of things from you in the past. If you
> could provide a presee of the problems you encounter that would be great.

There are two known issues with the current (1.1) code:

1) Several users reported that "sometimes" there is an audible glitch, then 
sounds stops and VLC eats all of the memory that the system will give.
I _guess_ the old output assumed that PulseAudio would invoke the stream write 
callback at regular interval, which it for some reasons, did not do. Thus VLC 
would lamely queue all audio buffers to anonymous memory forever.

But I don't know why PulseAudio would stop calling the stream write callback. 
I assume the memory leak is fixed with the new approach without callback. But 
I also assume there is an underlying problem that is not fixed.

2) Stuttering. I hope this is fixed now with the huge buffers which is really 
the maximum that VLC can accomodate without redesign of the audio subsystem 
(1,5s target, 0,5s prebuf). This will probably ruin low-latency use cases. And 
I very much fear for the "stability" of the lip synchronization over long 
playback times, as there is no latency feedback at all anymore.

> > (2) seems quite impossible as PulseAudio is restarted even on SIGKILL.
> 
> This is, of course, by design. It's autospawning which is enabled by
> default. This allows console applications to work happily when PA is not
> explicitly started by some kind of init systems (e.g. session launch).
> Sensible distros will ensure that they turn autospawn off if the user
> disabled PA (certainly I do in Mandriva/Mageia and have advised other
> distros in the past to do so also).
> 
> There are two very specific ways to interact with PA on a "nice" level.
> (1) Via the suspend system. You can specifically request that a given
> sink/source is suspended, via the pa_context_suspend_* APIs. This
> ensures that PA closes the device and allows another application to use
> it. This is what the pasuspender command line tool does.
> (2) There is also a DBUS API that allows control of the audio device to
> be handed over to another application. This is used e.g. to hand control
> over to JACK. This allows JACK to take exclusive control and then for
> use to load modules which allow us to output sound via JACK, so the user
> should, in theory be (mostly) unaffected.
> 
> Using this DBUS protocol is fine, but consideration should also be given
> to what state it leaves PA in, e.g. all other applications can no longer
> output sound until a suitable sink is loaded into PA. Using the protocol
> without considering these issues is obviously not "playing nice" with
> the users system.

VLC can and will crash, usually when it's most inconvenient.

If PulseAudio is not keeping track of contexts or D-Bus name owners 
respectively, this is going to be bad. 'pulseaudio -k' + autospawn seems more 
viable otherwise.

I'd rather have PulseAudio output work correctly though, *and* explicitly 
"break" ALSA when the selected device is the PulseAudio asound plugin, or is 
used by PulseAudio.

> > (3) is just a waiver of warranty of sorts, and cannot cripple the user's
> > system any further than it already would be.
> 
> I'm really not sure what you mean here. If you took this action on my
> system, it very much would "cripple it more than it already would be".
> In what way was my *system* broken before? Some examples would be nice.

It's just about warning the user that we don't support his/her configuration 
and that he should turn PA off to continue, or whatever else in other 
comparable circumstances.

It's not optimal. But users screwing up their VLC configuration looking for an 
impossible solution or installing mpl^W^W^Wis worse.

-- 
Rémi Denis-Courmont
http://www.remlab.info/
http://fi.linkedin.com/in/remidenis