[pulseaudio-discuss] [PATCH] echo-cancel: Convert AGC API to deal with pa_volume_t

[Arun Raghavan]

On 25 February 2016 at 18:54, Tanu Kaskinen <tanuk at iki.fi> wrote:

> On Thu, 2016-02-25 at 18:01 +0530, arun at accosted.net wrote:
> > From: Arun Raghavan <git at arunraghavan.net>
> >
> > It is expected that the underlying AGC mechanism will likely provide a
> > single volume for the source rather than a per-channel volume. Dealing
> > with per-channel volumes just adds complexity with regards to the
> > actual volume setting (depending on whether volume sharing is enabled or
> > not, we would set the volume on the source output of the virtual source,
> > and their sample specs may be different).
> >
> > Using a single volume maps allows us to sidestep this problem entirely.
>
> "maps" in the last line looks like it shouldn't be there.
>
>

Yup, fixed.


> >  /* Called by the canceller, so source I/O thread context. */
> > -void pa_echo_canceller_set_capture_volume(pa_echo_canceller *ec,
> pa_cvolume *v) {
> > +void pa_echo_canceller_set_capture_volume(pa_echo_canceller *ec,
> pa_volume_t v) {
> >  #ifndef ECHO_CANCEL_TEST
> > -    if
> (!pa_cvolume_equal(&ec->msg->userdata->thread_info.current_volume, v)) {
> > -        pa_cvolume *vol = pa_xnewdup(pa_cvolume, v, 1);
> > -
> > -        pa_asyncmsgq_post(pa_thread_mq_get()->outq,
> PA_MSGOBJECT(ec->msg), ECHO_CANCELLER_MESSAGE_SET_VOLUME, vol, 0, NULL,
> > -                pa_xfree);
> > +    if (pa_cvolume_avg(&ec->msg->userdata->thread_info.current_volume)
> != v) {
> > +        pa_asyncmsgq_post(pa_thread_mq_get()->outq,
> PA_MSGOBJECT(ec->msg), ECHO_CANCELLER_MESSAGE_SET_VOLUME, PA_UINT_TO_PTR(v),
> > +                0, NULL, NULL);
>
> Since pa_volume_t is defined to be 32 bits, maybe it would be better to
> use PA_UINT32_TO_PTR here (and PA_PTR_TO_UINT32 when converting back).
> I guess in practice this doesn't matter, though.
>

I thought I'd do it this way for the extremely unlikely case that we change
the type of pa_volume_t in the future. As you say, it doesn't really
matter, though, so leaving it as is.


> >      }
> >  #endif
> >  }
> > diff --git a/src/modules/echo-cancel/webrtc.cc
> b/src/modules/echo-cancel/webrtc.cc
> > index 12bb556..f8aee33 100644
> > --- a/src/modules/echo-cancel/webrtc.cc
> > +++ b/src/modules/echo-cancel/webrtc.cc
> > @@ -537,9 +537,8 @@ void pa_webrtc_ec_record(pa_echo_canceller *ec,
> const uint8_t *rec, uint8_t *out
> >      pa_deinterleave(rec, (void **) buf, rec_ss->channels,
> pa_sample_size(rec_ss), n);
> >
> >      if (ec->params.webrtc.agc) {
> > -        pa_cvolume_init(&v);
> > -        pa_echo_canceller_get_capture_volume(ec, &v);
> > -        old_volume = webrtc_volume_from_pa(pa_cvolume_avg(&v));
> > +        pa_volume_t v = pa_echo_canceller_get_capture_volume(ec);
>
> v is declared already earlier (the earlier declaration should be
> removed).
>
>
Odd that the compiler didn't warn about this. Fixing and pushing.

Thanks!
Arun
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/pulseaudio-discuss/attachments/20160225/29998c8a/attachment.html>