[asterisk-dev] [RFC] Modularising echo cancellers

[Tzafrir Cohen]

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.
> 
> In conjunction with this, I've developed a patch to Asterisk's chan_zap 
> that allows setting which echo canceler to use on a per-channel basis 
> (an "echocanceltype" parameter).
> 
> We have found this invaluable for evaluating different software 
> cancelers in varying situations, and we're using in production at a 
> number of sites on both E1 PRI and TDM400 analogue lines. It has allowed 
> me to do fun things like modularising the MG2 changes from trunk, 
> loading the module on a production system, and starting the use the new 
> canceler after just issuing a "reload".
> 
> The patch is currently for Zaptel 1.2.x, and I'm looking for comments as 
> to whether there's sufficient interest in it for me to forward-port it 
> to trunk, and submit it for inclusion.
> 
> The basic API looks like this:
> 
> /* Register an echo canceler with Zaptel */
> int zt_register_echocan(struct zt_echo_can *ec);
> /* Unregister an echo canceler */
> int zt_unregister_echocan(struct zt_echo_can *ec);
> /* Find the canceler structure called "name". Handles locking and module 
> use counts. */
> struct zt_echo_can *zt_echo_can_get(char *name);
> /* Return an instance of an echo canceler back to Zaptel, handling 
> locking and module use counts */
> void zt_echo_can_put(struct zt_echo_can *ec);
> 
> struct zt_echo_can {
>        /* Module-visible bits */
>        char name[ZT_ECHO_CAN_NAMELEN];
>        struct module *owner;
>        zt_echo_can_state_t *(*create)(int taps);
>        void (*free)(zt_echo_can_state_t *ec);
>        short (*update)(zt_echo_can_state_t *ec, short tx, short rx);
>        /* update_block: run the echo canceler over a block of data.
>         *
>         * It must deal with the possibility of rx == out.
>         */
>        void (*update_block)(zt_echo_can_state_t *ec,
>                        short *tx, short *rx, short *out, size_t len);
>        int (*traintap)(zt_echo_can_state_t *ec, int pos, short val);
> 
>        /* Bits for Zaptel only */
>        struct list_head list;
> };
> 
> 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.

> {
>        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.

> 
> The patch to chan_zap tries the V2 interface, then falls back to the 
> original if it doesn't work - on the assumption that in general, the 
> wrong canceler is better than no canceler.
> 
> One of the things I'd like to do going forward is to add a way of 
> sending options from userspace up to the canceler to add runtime control 
> of things like nonlinear processing, suppression, comfort noise 
> generation, convergence speed, etc. (admittedly, most cancelers don't 
> even have these knobs - yet). At this point it's only possible to 
> specify the tail length.
> 
> Your thoughts?

bugs.digium.com ?

If only they were tiny bit tolerant to anything that doesn't concern
current HEAD.

> 
> If anyone wants to try out the full patch, send me an email offlist - 
> I'll have to separate it out from my other fiddling, which I haven't 
> done as yet. Oh, and it's only tested on Linux 2.6.x; I doubt it'll even 
> compile on 2.4 without changes.
> 
> 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
> 
> _______________________________________________
> --Bandwidth and Colocation provided by Easynews.com --
> 
> asterisk-dev mailing list
> To UNSUBSCRIBE or update options visit:
>   http://lists.digium.com/mailman/listinfo/asterisk-dev
> 

-- 
Tzafrir Cohen      sip:tzafrir at local.xorcom.com
icq#16849755       iax:tzafrir at local.xorcom.com
+972-50-7952406           
tzafrir.cohen at xorcom.com  http://www.xorcom.com