<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On 25 February 2016 at 18:54, Tanu Kaskinen <span dir="ltr"><<a href="mailto:tanuk@iki.fi" target="_blank">tanuk@iki.fi</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Thu, 2016-02-25 at 18:01 +0530, <a href="mailto:arun@accosted.net">arun@accosted.net</a> wrote:<br>
> From: Arun Raghavan <<a href="mailto:git@arunraghavan.net">git@arunraghavan.net</a>><br>
><br>
> It is expected that the underlying AGC mechanism will likely provide a<br>
> single volume for the source rather than a per-channel volume. Dealing<br>
> with per-channel volumes just adds complexity with regards to the<br>
> actual volume setting (depending on whether volume sharing is enabled or<br>
> not, we would set the volume on the source output of the virtual source,<br>
> and their sample specs may be different).<br>
><br>
> Using a single volume maps allows us to sidestep this problem entirely.<br>
<br>
</span>"maps" in the last line looks like it shouldn't be there.<br>
<span class=""></span> <br></blockquote><div><br></div><div>Yup, fixed.<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
>  /* Called by the canceller, so source I/O thread context. */<br>
> -void pa_echo_canceller_set_capture_volume(pa_echo_canceller *ec, pa_cvolume *v) {<br>
> +void pa_echo_canceller_set_capture_volume(pa_echo_canceller *ec, pa_volume_t v) {<br>
>  #ifndef ECHO_CANCEL_TEST<br>
> -    if (!pa_cvolume_equal(&ec->msg->userdata->thread_info.current_volume, v)) {<br>
> -        pa_cvolume *vol = pa_xnewdup(pa_cvolume, v, 1);<br>
> -<br>
> -        pa_asyncmsgq_post(pa_thread_mq_get()->outq, PA_MSGOBJECT(ec->msg), ECHO_CANCELLER_MESSAGE_SET_VOLUME, vol, 0, NULL,<br>
> -                pa_xfree);<br>
> +    if (pa_cvolume_avg(&ec->msg->userdata->thread_info.current_volume) != v) {<br>
> +        pa_asyncmsgq_post(pa_thread_mq_get()->outq, PA_MSGOBJECT(ec->msg), ECHO_CANCELLER_MESSAGE_SET_VOLUME, PA_UINT_TO_PTR(v),<br>
> +                0, NULL, NULL);<br>
<br>
</span>Since pa_volume_t is defined to be 32 bits, maybe it would be better to<br>
use PA_UINT32_TO_PTR here (and PA_PTR_TO_UINT32 when converting back).<br>
I guess in practice this doesn't matter, though.<span class=""><br></span></blockquote><div><br></div><div>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.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
>      }<br>
>  #endif<br>
>  }<br>
> diff --git a/src/modules/echo-cancel/webrtc.cc b/src/modules/echo-cancel/webrtc.cc<br>
> index 12bb556..f8aee33 100644<br>
> --- a/src/modules/echo-cancel/webrtc.cc<br>
> +++ b/src/modules/echo-cancel/webrtc.cc<br>
> @@ -537,9 +537,8 @@ void pa_webrtc_ec_record(pa_echo_canceller *ec, const uint8_t *rec, uint8_t *out<br>
>      pa_deinterleave(rec, (void **) buf, rec_ss->channels, pa_sample_size(rec_ss), n);<br>
>  <br>
>      if (ec->params.webrtc.agc) {<br>
> -        pa_cvolume_init(&v);<br>
> -        pa_echo_canceller_get_capture_volume(ec, &v);<br>
> -        old_volume = webrtc_volume_from_pa(pa_cvolume_avg(&v));<br>
> +        pa_volume_t v = pa_echo_canceller_get_capture_volume(ec);<br>
<br>
</span>v is declared already earlier (the earlier declaration should be<br>
removed).<br>
<span class="HOEnZb"><font color="#888888"><br></font></span></blockquote><div><br></div><div>Odd that the compiler didn't warn about this. Fixing and pushing.<br><br></div><div>Thanks!<br></div><div>Arun <br></div></div><br></div></div>