On Sat, 2016-01-30 at 13:51 +0200, Tanu Kaskinen wrote:
> On Mon, 2016-01-18 at 13:06 +0530, arun at accosted.net wrote:
> > From: Arun Raghavan <git at arunraghavan.net>
> >
> > ---
> > Â src/modules/echo-cancel/echo-cancel.h |Â Â 1 +
> >  src/modules/echo-cancel/webrtc.cc     | 34
> > ++++++++++++++++++++++++++++++++++
> > Â 2 files changed, 35 insertions(+)
> >
> > diff --git a/src/modules/echo-cancel/echo-cancel.h
> > b/src/modules/echo-cancel/echo-cancel.h
> > index 29d1574..142f8ac 100644
> > --- a/src/modules/echo-cancel/echo-cancel.h
> > +++ b/src/modules/echo-cancel/echo-cancel.h
> > @@ -67,6 +67,7 @@ struct pa_echo_canceller_params {
> > Â Â Â Â Â Â Â Â Â Â Â Â Â uint32_t blocksize;
> > Â Â Â Â Â Â Â Â Â Â Â Â Â pa_sample_spec sample_spec;
> > Â Â Â Â Â Â Â Â Â Â Â Â Â bool agc;
> > +Â Â Â Â Â Â Â Â Â Â Â Â bool trace;
> > Â Â Â Â Â Â Â Â Â } webrtc;
> > Â #endif
> > Â Â Â Â Â Â Â Â Â /* each canceller-specific structure goes here */
> > diff --git a/src/modules/echo-cancel/webrtc.cc b/src/modules/echo-
> > cancel/webrtc.cc
> > index 4dbd2fc..53ab7e1 100644
> > --- a/src/modules/echo-cancel/webrtc.cc
> > +++ b/src/modules/echo-cancel/webrtc.cc
> > @@ -35,6 +35,7 @@ PA_C_DECL_END
> > Â
> > Â #includeÂ
> > Â #includeÂ
> > +#includeÂ
> > Â
> > Â #define BLOCK_SIZE_US 10000
> > Â
> > @@ -48,6 +49,7 @@ PA_C_DECL_END
> > Â #define DEFAULT_DRIFT_COMPENSATION false
> > Â #define DEFAULT_EXTENDED_FILTER false
> > Â #define DEFAULT_INTELLIGIBILITY_ENHANCER false
> > +#define DEFAULT_TRACE false
> > Â
> > Â static const char* const valid_modargs[] = {
> > Â Â Â Â Â "high_pass_filter",
> > @@ -60,6 +62,7 @@ static const char* const valid_modargs[] = {
> > Â Â Â Â Â "drift_compensation",
> > Â Â Â Â Â "extended_filter",
> > Â Â Â Â Â "intelligibility_enhancer",
> > +Â Â Â Â "trace",
> > Â Â Â Â Â NULL
> > Â };
> > Â
> > @@ -78,6 +81,20 @@ static int routing_mode_from_string(const char
> > *rmode) {
> > Â Â Â Â Â Â Â Â Â return -1;
> > Â }
> > Â
> > +class PaWebrtcTraceCallback : public webrtc::TraceCallback {
> > +Â Â Â Â void Print(webrtc::TraceLevel level, const char *message, int
> > length)
>
> I hope the presence of a length parameter doesn't mean that message
> isn't null-terminated.
I don't know why the length is there, but it is null-terminated.
> > +Â Â Â Â {
> > +Â Â Â Â Â Â Â Â if (level & webrtc::kTraceError || level &
> > webrtc::kTraceCritical)
> > +Â Â Â Â Â Â Â Â Â Â Â Â pa_log(message);
> > +Â Â Â Â Â Â Â Â else if (level & webrtc::kTraceWarning)
> > +Â Â Â Â Â Â Â Â Â Â Â Â pa_log_warn(message);
> > +Â Â Â Â Â Â Â Â else if (level & webrtc::kTraceInfo)
> > +Â Â Â Â Â Â Â Â Â Â Â Â pa_log_info(message);
> > +Â Â Â Â Â Â Â Â else
> > +Â Â Â Â Â Â Â Â Â Â Â Â pa_log_debug(message);
> > +Â Â Â Â }
> > +};
> > +
> > Â static void pa_webrtc_ec_fixate_spec(pa_sample_spec *rec_ss,
> > pa_channel_map *rec_map,
> > Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â pa_sample_spec *play_ss,
> > pa_channel_map *play_map,
> > Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â pa_sample_spec *out_ss,
> > pa_channel_map *out_map)
> > @@ -201,6 +218,18 @@ bool pa_webrtc_ec_init(pa_core *c,
> > pa_echo_canceller *ec,
> > Â Â Â Â Â if (intelligibility)
> > Â Â Â Â Â Â Â Â Â config.Set(new webrtc::Intelligibility(true));
> > Â
> > +Â Â Â Â ec->params.priv.webrtc.trace = DEFAULT_TRACE;
> > +Â Â Â Â if (pa_modargs_get_value_boolean(ma, "trace", &ec-
> > >params.priv.webrtc.trace) < 0) {
> > +Â Â Â Â Â Â Â Â pa_log("Failed to parse trace value");
> > +Â Â Â Â Â Â Â Â goto fail;
> > +Â Â Â Â }
> > +
> > +Â Â Â Â if (ec->params.priv.webrtc.trace) {
> > +Â Â Â Â Â Â Â Â webrtc::Trace::CreateTrace();
> > +Â Â Â Â Â Â Â Â webrtc::Trace::set_level_filter(webrtc::kTraceAll);
> > +Â Â Â Â Â Â Â Â webrtc::Trace::SetTraceCallback(new
> > PaWebrtcTraceCallback());
>
> Does webrtc::Trace delete the object that you allocate here?
It does not. I've now redone this and will send out the update.
> > +Â Â Â Â }
> > +
> > Â Â Â Â Â pa_webrtc_ec_fixate_spec(rec_ss, rec_map, play_ss, play_map,
> > out_ss, out_map);
> > Â
> > Â Â Â Â Â apm = webrtc::AudioProcessing::Create(config);
> > @@ -263,6 +292,8 @@ bool pa_webrtc_ec_init(pa_core *c,
> > pa_echo_canceller *ec,
> > Â fail:
> > Â Â Â Â Â if (ma)
> > Â Â Â Â Â Â Â Â Â pa_modargs_free(ma);
> > +Â Â Â Â if (ec->params.priv.webrtc.trace)
> > +Â Â Â Â Â Â Â Â webrtc::Trace::ReturnTrace();
>
> What does this do? Is it safe to call this if you haven't yet set up
> the tracing?
Drops the reference on the Trace objected created by CreateTrace().
Also fixed this up to be safe against initialisation.
-- Arun
