[pulseaudio-discuss] [PATCH v4 20/23] echo-cancel: Use webrtc's deinterleaved API

[Tanu Kaskinen]

On Thu, 2016-02-25 at 09:20 +0530, Arun Raghavan wrote:
> 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?

Yes. I wonder, though, if it would be even better to change
pa_echo_canceller_set_capture_volume() to take a pa_volume_t rather
than pa_cvolume. There doesn't seem to be need for supporting different
volume for different channels.

-- 
Tanu