[asterisk-dev] [Code Review] Modify bridging to properly evaluate DTMF after first warning is played

[Kaloyan Kovachev]

On Wed, 18 Feb 2009 10:32:01 -0600 (CST), Joshua Colp wrote
> ----- "Kaloyan Kovachev" <kkovachev at varna.net> wrote:
> 
> > by changing the config structure (in channel.h) it already makes too
> > much
> > changes, so maybe dedicated variables (and the other changes below)
> > will not
> > add much to that.
> 
> It made the minimal changes needed to accomplish fixing the issue.
> 
> > other changes:
> >  removing of the last variable of ast_generic_bridge - will not harm
> > anything
> > and is used only in channel.c
> >  adding feature_start and feature_first_pass variables in config too -
> > will
> > only change channel.c and features.c, but will also make the entire
> > backup
> > config in features.c unnecessary ... removing it will need more
> > changes though:
> > 	config->play_warning = 0;
> > 	ast_clear_flag(&(config->features_caller),
> > AST_FEATURE_PLAY_WARNING);
> > 	ast_clear_flag(&(config->features_callee),
> > AST_FEATURE_PLAY_WARNING);
> > 	config->warning_freq = 0;
> > 	config->warning_sound = NULL;
> > 	config->end_sound = NULL;
> > 	config->start_sound = NULL;
> > 	config->firstpass = 0;
> >    can be safely removed if in channel.c caller_warning and
> > callee_warning
> > (lines 4081 & 4082) are moved inside the "} else if
> > (config->timelimit) {"
> > block, because in case of feature the "if (config->feature_timer) {"
> > block
> > will match and no warnings will be played.
> > 
> >    more problematic is the feature_timer itself as there is some
> > simultaneous
> > use of it from the backup config, so maybe another config variable
> > old_feature_timer will be needed (not sure what exactly happens here
> > and is it
> > possible to use a local variable)
> >
> 
> We can certainly clean the code up in trunk, just not in any release
branches. If you post a patch on Mantis I'm sure Jeff and others
> will give it a look.
> 
OK, i will. Just wasn't sure for which branch ... trunk then :)

> > > I also thought about playing the warnings not in the bridge. Changes
> > such as
> > these are more appropriate for trunk though.
> > > 
> > > I assume you've been keeping up with the new review here:
> > > http://reviewboard.digium.com/r/93/
> > > 
> > 
> > yes, waiting for it to be committed, but it still lacks the interval
> > hooks and
> 
> These will actually go in for phase 2 of bridging which will provide the
remaining options used by two channel bridges primarily.
> 
> > the most demanding part for the 'out of bridge warnings' is the
> > 'wishper file'
> > or similar from the audiohooks
> >
> 
> I think given the APIs we currently have it is possible to accomplish this.
You just have to build something using them. There are two
> ways that come to mind to use audiohooks to accomplish it.
> 
> 1. Open the file you want to stream, attach a manipulate audiohook, for each
callback you get simply replace the frame with a new frame that has the
> number of samples required from the open stream.

I was thinking about the same, but there is no hook's private data in the
structure ... i guess i will need to learn how to use datastores and the
volume audiohook seems a good example ... will go this way. Thank you for the
pointers.

> 
> 2. Attach a whisper audiohook and have a separate thread that is timed to
read audio from the file and write it into the audiohook.

This will need a separate thread for each call.

> 
> -- 
> Joshua Colp
> Digium, Inc. | Software Developer
> 445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
> Check us out at:  www.digium.com  & www.asterisk.org
> 
> _______________________________________________
> --Bandwidth and Colocation Provided by http://www.api-digital.com--
> 
> asterisk-dev mailing list
> To UNSUBSCRIBE or update options visit:
>    http://lists.digium.com/mailman/listinfo/asterisk-dev