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. Indeed. Following up with an updated patch. -- Arun
