[pulseaudio-discuss] [PATCH] echo-cancel: Fix assert with webrtc's built-in drift compensation
arun at arunraghavan.net
Fri Mar 17 02:19:48 UTC 2017
On Mon, 13 Mar 2017, at 08:06 PM, Tanu Kaskinen wrote:
> On Mon, 2017-03-13 at 16:07 +0530, Arun Raghavan wrote:
> > The webrtc canceller requires that the set_stream_drift_samples() method
> > be called before every call of ProcessStream(). We do kind of leak this
> > into the generic bits of module-echo-cancel, but this should not be
> > harmful in the general case either.
> > Relatedly, we also require that this happen when there are no sink
> > samples being sent, so we just drop the special-case check for whether
> > the sink is running to do the drift compensation calculations.
> It's not clear to me from the commit message which assertion fails.
> Did the crash happen because set_drift() wasn't called before
> ProcessStream(), because we didn't call do_push_drift_comp() when the
> sink isn't running? If so, why did you move the set_drift() call? Was
> that a fix for some other bug?
It's actually a fix for two things:
1. We are calling do_push_drift_comp() when the sink isn't running,
which calls ProcessStream() without setting the drift
2. We are calling ProcessStream() in a loop, but not setting the drift
each time (the code seems to have changed to require a call each time).
I will update the message to reflect this.
> > @@ -947,7 +945,7 @@ static void source_output_push_cb(pa_source_output *o, const pa_memchunk *chunk)
> > }
> > /* process and push out samples, do drift compensation only if the sink is actually running */
> > - if (u->ec->params.drift_compensation && u->sink->thread_info.state == PA_SINK_RUNNING)
> > + if (u->ec->params.drift_compensation)
> > do_push_drift_comp(u);
> > else
> > do_push(u);
> It looks like the comment should be updated too.
Following up with an updated patch.
More information about the pulseaudio-discuss