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

[Thomas Guillem]

On Mon, Feb 11, 2019, at 17:31, Rémi Denis-Courmont wrote:
> 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.

Do you know where it is done ? I'm currently trying to fix pulse underflow with ogg files using the new output clock. I tweaked the input buffering and ended up having overflow...

> 
> So this change should be useless until input pace control is removed.
> 
> -- 
> Реми Дёни-Курмон
> http://www.remlab.net/
> 
> 
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel