On Wed, 2016-02-24 at 18:32 +0200, Tanu Kaskinen wrote: > On Wed, 2016-02-17 at 19:47 +0530, arun at accosted.net wrote: > > @@ -411,12 +414,12 @@ void pa_webrtc_ec_record(pa_echo_canceller *ec, const uint8_t *rec, uint8_t *out > > Â Â Â Â Â Â Â Â Â } > > Â > > Â Â Â Â Â Â Â Â Â if (old_volume != new_volume) { > > -Â Â Â Â Â Â Â Â Â Â Â Â pa_cvolume_set(&v, rec_ss->channels, webrtc_volume_to_pa(new_volume)); > > +Â Â Â Â Â Â Â Â Â Â Â Â pa_cvolume_set(&v, out_ss->channels, webrtc_volume_to_pa(new_volume)); > > Surely this belongs to the previous patch? Yep, dropped from this one. > Anyway, this change isn't sufficient. module-echo-cancel.c has this > code: > > Â Â Â Â case ECHO_CANCELLER_MESSAGE_SET_VOLUME: { > Â Â Â Â Â Â Â Â Â Â Â Â pa_cvolume *v = (pa_cvolume *) userdata; > > Â Â Â Â Â Â Â Â Â Â Â Â if (u->use_volume_sharing) > Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â pa_source_set_volume(u->source, v, true, false); > Â Â Â Â Â Â Â Â Â Â Â Â else > Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â pa_source_output_set_volume(u->source_output, v, false, true); > > Â Â Â Â Â Â Â Â Â Â Â Â break; > > So depending on u->use_volume_sharing, sometimes it's correct to use > rec_ss and sometimes out_ss. I think we should keep this as rec_ss -- it feels more correct to expect the canceller to ask for the gain to be applied on the source data than on its output. module-echo-cancel would then remap the volume if needed in the !use_volume_sharing case. Sounds okay? -- Arun