[pulseaudio-discuss] [PATCH v3 18/24] echo-cancel: Improve webrtc canceller error handling a bit

[Tanu Kaskinen]

On Wed, 2016-02-10 at 09:56 +0530, Arun Raghavan wrote:
> On 9 February 2016 at 23:41, Tanu Kaskinen <tanuk at iki.fi> wrote:
> > On Mon, 2016-01-18 at 13:06 +0530, arun at accosted.net wrote:
> > > @@ -387,7 +392,8 @@ void pa_webrtc_ec_record(pa_echo_canceller *ec, const uint8_t *rec, uint8_t *out
> > >      }
> > > 
> > >      apm->set_stream_delay_ms(0);
> > > -    apm->ProcessStream(&out_frame);
> > > +    if (apm->ProcessStream(&out_frame) != webrtc::AudioProcessing::kNoError)
> > > +        pa_log("Failed to process capture stream");
> > 
> > I don't like error messages that can spam the syslog. Would a hard
> > failure be acceptable here (that is, unload the module, or change to a
> > failure state where the module stays around but does no processing)?
> 
> This particular case is extremely low probability, so I'd like to not
> change it if possible. I don't think the extra code to add a noop mode
> is justified since we're very unlikely to hit this.
> 
> Maybe we could/should change the run/play/record vmethods to return an
> error code, but I'd prefer to do that separately if it's done.

So what are the extremely rare conditions where this error is
triggered? I'll assume that this error doesn't indicate a bug in our
code or in the webrtc library, and can really happen, so an assertion
is not appropriate here.

Adding a noop mode was just one idea, and I understand if that feels
overkill. But what about unloading the module? Other ideas: lower the
message level to info or debug, or prevent the error message being
printed more than once.

Flooding syslog is never a good error handling strategy. This looks
like if there is one error, there will probably be thousand.

-- 
Tanu