[vlc-devel] [PATCH] pulse: avoid overflows

Rémi Denis-Courmont remi at remlab.net
Mon Feb 11 17:31:10 CET 2019


Le maanantaina 11. helmikuuta 2019, 18.26.25 EET Thomas Guillem a écrit :
> On Mon, Feb 11, 2019, at 17:18, Rémi Denis-Courmont wrote:
> > Le maanantaina 11. helmikuuta 2019, 18.11.04 EET Thomas Guillem a écrit :
> > > Compare the maxlength and the delay in order to detect if we should pace
> > > from the Play() function. Overflows never happened with the default VLC
> > > configuration, but it can be triggered easily when the input buffering
> > > is
> > > tweaked. Furthermore, this will allow interactive playback with a low
> > > maxlength value.
> > > 
> > > Most common audio outputs modules (on Windows, Android, macOS) are
> > > already
> > > pacing from the play callback.
> > > 
> > > This is also a small step for the input clock/buffering rework since we
> > > may
> > > want to pace from the output in the future.
> > > ---
> > > 
> > >  modules/audio_output/pulse.c | 19 ++++++++++++++++++-
> > >  1 file changed, 18 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/modules/audio_output/pulse.c b/modules/audio_output/pulse.c
> > > index 8c024006af..d20553c92c 100644
> > > --- a/modules/audio_output/pulse.c
> > > +++ b/modules/audio_output/pulse.c
> > > @@ -69,6 +69,7 @@ typedef struct
> > > 
> > >      pa_time_event *trigger; /**< Deferred stream trigger */
> > >      pa_cvolume cvolume; /**< actual sink input volume */
> > >      vlc_tick_t last_date; /**< Play system timestamp of last buffer */
> > > 
> > > +    vlc_tick_t maxlength; /**< Maximum audio buffer length */
> > > 
> > >      pa_volume_t volume_force; /**< Forced volume (stream must be NULL)
> > >      */
> > >      pa_stream_flags_t flags_force; /**< Forced flags (stream must be
> > >      NULL)
> > > 
> > > */ @@ -282,8 +283,10 @@ static void stream_state_cb(pa_stream *s, void
> > > *userdata) static void stream_buffer_attr_cb(pa_stream *s, void
> > > *userdata)
> > > 
> > >  {
> > >  
> > >      audio_output_t *aout = userdata;
> > > 
> > > +    aout_sys_t *sys = aout->sys;
> > > 
> > >      const pa_buffer_attr *pba = pa_stream_get_buffer_attr(s);
> > > 
> > > +    sys->maxlength = pba->maxlength;
> > > 
> > >      msg_Dbg(aout, "changed buffer metrics: maxlength=%u, tlength=%u, "
> > >      
> > >              "prebuf=%u, minreq=%u",
> > >              pba->maxlength, pba->tlength, pba->prebuf, pba->minreq);
> > > 
> > > @@ -491,6 +494,20 @@ static void Play(audio_output_t *aout, block_t
> > > *block,
> > > vlc_tick_t date) void *ptr;
> > > 
> > >          size_t len = block->i_buffer;
> > > 
> > > +        /* Check if we should pace to prevent an overflow */
> > > +        if (pa_stream_is_corked(s) <= 0)
> > > +        {
> > > +            assert(sys->maxlength != -1);
> > > +            const pa_sample_spec *ss = pa_stream_get_sample_spec(s);
> > > +            vlc_tick_t len_us = pa_usec_to_bytes(len, ss);
> > > +            while (len_us + vlc_pa_get_latency(aout, sys->context, s)
> > > +                   > sys->maxlength)
> > > +            {
> > > +                vlc_tick_t sleep_us = __MAX(len_us, sys->maxlength /
> > > 100);
> > > +                vlc_tick_sleep(sleep_us);
> > > +            }
> > > +        }
> > 
> > This looks like it will underflow the buffer if the block is larger than
> > the current buffering. There should be an underflow callback to handle
> > overflow.
> haha, or maybe add a way to play a small part of the buffer.
> 
> > But this will hold the audio controls during pacing. The audio output
> > threading first needs to be adjusted so that controls remain available
> > while play() is blocking.
> 
> Like having play() returning EGAIN (after modifying p_block offset) ?

That is half one potential solution. It would also requires a callback to tell 
the core when to retry.

An alternative is to change the (re)entrancy rules for audio outputs so that 
user control callbacks can be called at the same time (from another thread) as 
playback callbacks. But it might make audio output plugin implementation more 
confusing.

> I don't know if this change is for 4.0 or 5.0. If it's for 5.0, we should
> merge this commit anyway, in order to have the same behavior for all (or
> most commons) audio outputs (that is: waiting from play()).

Currently the PulseAudio output is configured with buffers larger than the 
maximum advance guaranteed by VLC core.

So this change should be useless until input pace control is removed.

-- 
Реми Дёни-Курмон
http://www.remlab.net/





More information about the vlc-devel mailing list