[pulseaudio-discuss] [PATCH] echo-cancel: Limit the maximum sink/source latency

[Arun Raghavan]

On Thu, 9 Mar 2017, at 04:01 PM, Georg Chini wrote:
> On 09.03.2017 06:53, Arun Raghavan wrote:
> > On systems with constrained CPUs, we might run into a situation where
> > the master source/sink is configured to have too high a latency.
> >
> > On the source side, this would cause us to wake up with a large chunk of
> > data to process, which might cause us to exhust our RT limit and thus be
> > killed.
> >
> > So it makes sense to limit the overall latency that we request from the
> > source (and correspondingly, the sink, so we don't starve for playback
> > data on the source side).
> >
> > The 10 blocks maximum is somewhat arbitrary (I'm assuming the system has
> > enough headroom to process 10 chunks through the canceller without
> > getting close to the RT limit). This might make sense to make tunable in
> > the future.
> > ---
> >   src/modules/echo-cancel/module-echo-cancel.c | 32 +++++++++++++++++++++-------
> >   1 file changed, 24 insertions(+), 8 deletions(-)
> >
> > diff --git a/src/modules/echo-cancel/module-echo-cancel.c b/src/modules/echo-cancel/module-echo-cancel.c
> > index ed75e0c..0a9f290 100644
> > --- a/src/modules/echo-cancel/module-echo-cancel.c
> > +++ b/src/modules/echo-cancel/module-echo-cancel.c
> > @@ -145,6 +145,8 @@ static const pa_echo_canceller ec_table[] = {
> >   
> >   #define MEMBLOCKQ_MAXLENGTH (16*1024*1024)
> >   
> > +#define MAX_LATENCY_BLOCKS 10
> > +
> >   /* Can only be used in main context */
> >   #define IS_ACTIVE(u) ((pa_source_get_state((u)->source) == PA_SOURCE_RUNNING) && \
> >                         (pa_sink_get_state((u)->sink) == PA_SINK_RUNNING))
> > @@ -515,6 +517,7 @@ static int sink_set_state_cb(pa_sink *s, pa_sink_state_t state) {
> >   /* Called from source I/O thread context */
> >   static void source_update_requested_latency_cb(pa_source *s) {
> >       struct userdata *u;
> > +    pa_usec_t latency;
> >   
> >       pa_source_assert_ref(s);
> >       pa_assert_se(u = s->userdata);
> > @@ -525,15 +528,17 @@ static void source_update_requested_latency_cb(pa_source *s) {
> >   
> >       pa_log_debug("Source update requested latency");
> >   
> > -    /* Just hand this one over to the master source */
> > -    pa_source_output_set_requested_latency_within_thread(
> > -            u->source_output,
> > -            pa_source_get_requested_latency_within_thread(s));
> > +    /* Cap the maximum latency so we don't have to process too large chunks */
> > +    latency = PA_MIN(pa_source_get_requested_latency_within_thread(s),
> > +                     pa_bytes_to_usec(u->source_blocksize, &s->sample_spec) * MAX_LATENCY_BLOCKS);
> > +
> > +    pa_source_output_set_requested_latency_within_thread(u->source_output, latency);
> >   }
> >   
> >   /* Called from sink I/O thread context */
> >   static void sink_update_requested_latency_cb(pa_sink *s) {
> >       struct userdata *u;
> > +    pa_usec_t latency;
> >   
> >       pa_sink_assert_ref(s);
> >       pa_assert_se(u = s->userdata);
> > @@ -544,10 +549,11 @@ static void sink_update_requested_latency_cb(pa_sink *s) {
> >   
> >       pa_log_debug("Sink update requested latency");
> >   
> > -    /* Just hand this one over to the master sink */
> > -    pa_sink_input_set_requested_latency_within_thread(
> > -            u->sink_input,
> > -            pa_sink_get_requested_latency_within_thread(s));
> > +    /* Cap the maximum latency so we don't have to process too large chunks */
> > +    latency = PA_MIN(pa_sink_get_requested_latency_within_thread(s),
> > +                     pa_bytes_to_usec(u->sink_blocksize, &s->sample_spec) * MAX_LATENCY_BLOCKS);
> > +
> > +    pa_sink_input_set_requested_latency_within_thread(u->sink_input, latency);
> >   }
> >   
> >   /* Called from sink I/O thread context */
> > @@ -1658,6 +1664,7 @@ int pa__init(pa_module*m) {
> >       uint32_t temp;
> >       uint32_t nframes = 0;
> >       bool use_master_format;
> > +    pa_usec_t blocksize_usec;
> >   
> >       pa_assert(m);
> >   
> > @@ -2020,6 +2027,15 @@ int pa__init(pa_module*m) {
> >   
> >       u->thread_info.current_volume = u->source->reference_volume;
> >   
> > +    /* We don't want to deal with too many chunks at a time */
> > +    blocksize_usec = pa_bytes_to_usec(u->source_blocksize, &u->source->sample_spec);
> > +    pa_source_set_latency_range(u->source, blocksize_usec, blocksize_usec * MAX_LATENCY_BLOCKS);
> > +    pa_source_output_set_requested_latency(u->source_output, blocksize_usec * MAX_LATENCY_BLOCKS);
> > +
> > +    blocksize_usec = pa_bytes_to_usec(u->sink_blocksize, &u->sink->sample_spec);
> > +    pa_sink_set_latency_range(u->sink, blocksize_usec, blocksize_usec * MAX_LATENCY_BLOCKS);
> > +    pa_sink_input_set_requested_latency(u->sink_input, blocksize_usec * MAX_LATENCY_BLOCKS);
> > +
> >       pa_sink_put(u->sink);
> >       pa_source_put(u->source);
> >   
> 
> You should save the initial latency ranges and restore them when the 
> module is unloaded.

The latency range is set on the echo-cancel sink and source (not the
master), so when the module is unloaded, that sink and source go away as
well.

-- Arun