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

[Arun Raghavan]

On Thu, 2016-02-11 at 12:17 +0200, Tanu Kaskinen wrote:
> 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.

The only failure case that I see currently is where we send a different
block size fro mwhat we promised.

> 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.

We shouldn't spam the log, since ratelimit will stop that from
happening. Does that not suffice? If not, I'll drop it to debug level.

-- Arun