On Mon, 2016-02-01 at 13:18 +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>
> >
> > This is required to make sure the capture output has sufficient
> > energy
> > for the AGC to do its job.
>
> I think the word "value" in the first line of the commit message
> should
> be replaced with "volume". After reading just the commit message, I
> didn't know what the patch was about.
>
> > ---
> > Â src/modules/echo-cancel/echo-cancel.h |Â Â 1 +
> >  src/modules/echo-cancel/webrtc.cc     | 14 +++++++++++++-
> > Â 2 files changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/modules/echo-cancel/echo-cancel.h
> > b/src/modules/echo-cancel/echo-cancel.h
> > index 142f8ac..b570095 100644
> > --- a/src/modules/echo-cancel/echo-cancel.h
> > +++ b/src/modules/echo-cancel/echo-cancel.h
> > @@ -68,6 +68,7 @@ struct pa_echo_canceller_params {
> > Â Â Â Â Â Â Â Â Â Â Â Â Â pa_sample_spec sample_spec;
> > Â Â Â Â Â Â Â Â Â Â Â Â Â bool agc;
> > Â Â Â Â Â Â Â Â Â Â Â Â Â bool trace;
> > +Â Â Â Â Â Â Â Â Â Â Â Â bool first;
> > Â Â Â Â Â Â Â Â Â } 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 c2585a8..6431651 100644
> > --- a/src/modules/echo-cancel/webrtc.cc
> > +++ b/src/modules/echo-cancel/webrtc.cc
> > @@ -52,6 +52,7 @@ PA_C_DECL_END
> > Â #define DEFAULT_TRACE false
> > Â
> > Â #define WEBRTC_AGC_MAX_VOLUME 255
> > +#define WEBRTC_AGC_START_VOLUME 85
> > Â
> > Â static const char* const valid_modargs[] = {
> > Â Â Â Â Â "high_pass_filter",
> > @@ -297,6 +298,7 @@ bool pa_webrtc_ec_init(pa_core *c,
> > pa_echo_canceller *ec,
> > Â Â Â Â Â ec->params.priv.webrtc.sample_spec = *out_ss;
> > Â Â Â Â Â ec->params.priv.webrtc.blocksize =
> > (uint64_t)pa_bytes_per_second(out_ss) * BLOCK_SIZE_US /
> > PA_USEC_PER_SEC;
> > Â Â Â Â Â *nframes = ec->params.priv.webrtc.blocksize /
> > pa_frame_size(out_ss);
> > +Â Â Â Â ec->params.priv.webrtc.first = true;
> > Â
> > Â Â Â Â Â pa_modargs_free(ma);
> > Â Â Â Â Â return true;
> > @@ -354,7 +356,17 @@ void pa_webrtc_ec_record(pa_echo_canceller
> > *ec, const uint8_t *rec, uint8_t *out
> > Â Â Â Â Â apm->ProcessStream(&out_frame);
> > Â
> > Â Â Â Â Â if (ec->params.priv.webrtc.agc) {
> > -Â Â Â Â Â Â Â Â new_volume = apm->gain_control()->stream_analog_level();
> > +Â Â Â Â Â Â Â Â if (PA_UNLIKELY(ec->params.priv.webrtc.first)) {
> > +Â Â Â Â Â Â Â Â Â Â Â Â /* We start at a sane default volume (taken from the
> > Chromium
> > +Â Â Â Â Â Â Â Â Â Â Â Â Â * condition on the experimental AGC in
> > audio_processing.h). This is
> > +Â Â Â Â Â Â Â Â Â Â Â Â Â * needed to make sure that there's enough energy in
> > the capture
> > +Â Â Â Â Â Â Â Â Â Â Â Â Â * signal for the AGC to work */
> > +Â Â Â Â Â Â Â Â Â Â Â Â ec->params.priv.webrtc.first = false;
> > +Â Â Â Â Â Â Â Â Â Â Â Â new_volume = WEBRTC_AGC_START_VOLUME;
>
> This is done only on initial loading of the module. Does AGC work ok
> after resuming a suspended source?
After suspend/resume is not a problem since the source volume won't get
reset from whatever it was set to before. This is mostly to make sure
that when the AGC first starts, it is able to actually get going.
-- Arun
