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

[Joshua Colp]

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

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.

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