[vlc-devel] [PATCH 0/8] asynchronous aout stream

[Thomas Guillem]

On Sun, Mar 10, 2019, at 17:08, Remi Denis-Courmont wrote:
> Le 2019-03-10 10:10, Thomas Guillem a écrit :
> > On Sat, Mar 9, 2019, at 19:09, Rémi Denis-Courmont wrote:
> >> Le perjantaina 8. maaliskuuta 2019, 17.24.38 EET Thomas Guillem a 
> >> écrit :
> >> > Hello,
> >> >
> >> > This patch set should be applied on top of my clock branch:
> >> > https://code.videolan.org/jbk/vlc/commits/clock-core/32
> >> >
> >> > All aout stream commands are now asynchronous.
> >> >
> >> > From an aout module point view, this mean that play/drain/flush/start/stop
> >> > are always executed from the same thread.
> >> 
> >> This is not a promise that should be made. If/when audio decoder or 
> >> filter get
> >> threaded, it will not be reasonable to provide such guarantee any 
> >> longer. It
> >> wouldn't be the first or even the second time that we shoot ourselves 
> >> in the
> >> feet with that silly API guarantee.
> > 
> > OK but these calls need to be protected with a mutex. It is possible
> > now to call flush and play from 2 threads without a protection.
> 
> It is the caller's responsibility that this does not happen. Adding a 
> mutex will not change that: flusing and playing at the same time makes 
> no sense.
> 
> I fail to see a problem here.
> 
> >> If you don't want Play to block, you need to implement it that way in 
> >> the
> >> individual plugins. You cannot cheat your way out of that with core 
> >> changes.
> >> However, again, this will break flow control, so it does not seem like 
> >> a good
> >> idea anyhow.
> > 
> > It's quite easy to make the play not blocking. Modules should be able
> > to return the same block with an offsert change, corresponding to data
> > to written.
> 
> Well that's one way. Another way is to queue inside the aout plugin, 
> which does not involve changing the Play() calling convention.

This will introduce a new delay and it just move the problem since we can't queue indefinitely. If the aout plugin is configured for a given buffer size, we should not increase it by having an external queue. I really prefer to wait from the core using a cond/mutex (so that we can still control the aout).  That way, we introduce a natural pacing for the audio decoder. I think it's a good first step for 5.0 buffering rewrite. 

> 
> But anyway, the points are:
> 1) It cannot be done (entirely) in the core without changing plugins.
> 2) It cannot be done unless/until another mechanism handles flow 
> control.
> 3) Adding yet another thread in the pipeline does not help with this.
> 
> >> And if you don't want Flush to block, then you actually need to change 
> >> the
> >> plugin API so that we know when the buffers are drained/flushed. You 
> >> cannot
> >> cheat your way out if it either
> > 
> > I'm not sure that all audio API provide a way to have a
> > async/nonblocking drain. I will check tomorrow.
> 
> If an aout plugin does not support asynchronous flush/drain, nor can be 
> made reentrant, then it's unfixable in the sense that draining or 
> flushing will involve a latency. This patch series will not fix what's 
> unfixable.
> 
> -- 
> Rémi Denis-Courmont
>