[asterisk-dev] [RFC] Modularising echo cancellers

[Nic Bellamy]

Tzafrir Cohen wrote:

>On Wed, Jun 28, 2006 at 03:40:02PM +1200, Nic Bellamy wrote:
>  
>
>>Hi all,
>>   I've developed a patch to Zaptel that modularises the software echo 
>>cancelers, allowing multiple ones to be loaded simultaneously.
>>    
>>
[snip]

>>The userspace-kernelspace interface required changes to allow sending a 
>>canceler name, rather than just on/off requests, so I added a new IOCTL 
>>interface:
>>
>>#define ZT_ECHO_CAN_NAMELEN     (16)
>>typedef struct zt_echocancelv2
>>    
>>
>
>Hmmm... I don't like the name. "v2" for version that will actually be
>used, and the shorter name for the version not to use.
>
>And then again, the echo canceller is an implementation detail. Not a
>matter of interface. Ideally I would like to be able to switch with ECs
>through /sys/modules/zaptel/parameters or whatever. Please use sysfs or
>procfs for such implementation details.
>
>Thus you could set them at run-time by a script.
>
>The fact that you need to switch an echo canceller indicates that the
>current echo canceller is simply not good enough. But there should be no
>real need for Asterisk to set the implementation . For the sysadmin:
>probably yes.
>  
>
The main point about setting them from Asterisk was the ability to set 
them per-channel for experimentation purposes. I guess it could be done 
via procfs/sysfs, but that's going to add quite a bit more complexity 
compared to the ioctl - the ioctl was already there, all I've done is 
expand its capabilities (and yeah, I know many people hate ioctls...).

The main reason I named it ZT_ECHOCANCEL_V2 is so I didn't break the 
existing interface; this allows your Zaptel and Asterisk to be out of 
sync, but still work as expected.

If I'm aiming to get this into 1.6, then breaking the existing interface 
is probably going to be acceptable - I don't think people are going to 
expect to be able to mix and match 1.2/1.4/1.6 Zaptel and Asterisk.

>>{
>>       char name[ZT_ECHO_CAN_NAMELEN];
>>       int taps;
>>} ZT_ECHOCANCELV2_DATA;
>>
>>/* Enable or disable echo cancellation on a channel (new version)
>>* The number is zero to disable echo cancellation and non-zero
>>* to enable echo cancellation.  If the number is between 32
>>* and 256, it will also set the number of taps in the echo canceler
>>*/
>>#define ZT_ECHOCANCELV2         _IOW (ZT_CODE, 93, struct zt_echocancelv2)
>>    
>>
>
>Why limit yourself to 256 buckets? IIRC in HEAD the limit is 1024. It
>may grow.
>  
>
Err, code fine, comment broken - the code itself doesn't limit the 
length (but each echo canceller module can choose to implement arbitary 
limits).

>>Your thoughts?
>>    
>>
>
>bugs.digium.com ?
>
>If only they were tiny bit tolerant to anything that doesn't concern
>current HEAD.
>
Russell Bryant has expressed interest, so I'll sort out a disclaimer and 
get the code up there.

Regards,
    Nic.

-- 
Nic Bellamy - <nic.bellamy at vadacom.co.nz>
Head Of Engineering, Vadacom Ltd - http://www.vadacom.co.nz/
Ph. +64-9-974-2790 - DDI +64-9-974-2639 - Mob. +64-21-251-8954