----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4477/#review14643 -----------------------------------------------------------
branches/11/apps/app_confbridge.c <https://reviewboard.asterisk.org/r/4477/#comment25208> It seems like this function could be written in terms of somthing like action_playback_and_continue() where the cur_dtmf passed in is "". branches/11/apps/app_confbridge.c <https://reviewboard.asterisk.org/r/4477/#comment25199> Space after commas. branches/11/apps/app_confbridge.c <https://reviewboard.asterisk.org/r/4477/#comment25209> This function has the potential to allow a user to blow the stack because of the recursion. Say *2 toggles locking the conference. If you repeatedly lock/unlock the conference and not allow the prompt playback to complete you recurse another level. Eventually you can blow the stack. branches/11/apps/app_confbridge.c <https://reviewboard.asterisk.org/r/4477/#comment25205> When this function is called, the user may or may not be in the softmix_bridge. I've looked at most menu actions and this seems to be OK to do. branches/11/apps/app_confbridge.c <https://reviewboard.asterisk.org/r/4477/#comment25204> Interrupting this sequence with DTMF is going to be hilarious. "There are" 45 "others in the conference" Interrupt "There are" with a DTMF menu request and the first part is interrupted and the remaining in the series will resume playing after the menu action. branches/11/apps/app_confbridge.c <https://reviewboard.asterisk.org/r/4477/#comment25203> This function appears to be called by post join actions and join_conference_bridge(). These calls happen before the softmix_bridge is actually joined. Allowing execution of menu options here is likely to cause problems. I've looked at most menu actions and this seems to be OK to do. branches/11/apps/app_confbridge.c <https://reviewboard.asterisk.org/r/4477/#comment25200> Long line wrap. Really should continue lines indented one tab stop. Horizontal space is precious while vertical space is cheap. branches/11/apps/app_confbridge.c <https://reviewboard.asterisk.org/r/4477/#comment25202> We haven't even joined the softmix_bridge yet. Allowing execution of menu options here is likely to cause problems. I've looked at most menu actions and this seems to be OK to do. branches/11/apps/app_confbridge.c <https://reviewboard.asterisk.org/r/4477/#comment25207> Why isn't this using conf_playback()? branches/11/apps/app_confbridge.c <https://reviewboard.asterisk.org/r/4477/#comment25206> This changes the playback_and_continue menu action behaviour. Before * - starts playback and continue to allow DTMF collection The user can then press 1 to get *1 action to trigger. *1 - submenu action After * - starts playback and continue The user now must restart DTMF entry to match *1. - rmudgett On March 11, 2015, 12:05 p.m., Kevin Harwell wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/4477/ > ----------------------------------------------------------- > > (Updated March 11, 2015, 12:05 p.m.) > > > Review request for Asterisk Developers. > > > Bugs: ASTERISK-24864 > https://issues.asterisk.org/jira/browse/ASTERISK-24864 > > > Repository: Asterisk > > > Description > ------- > > Attempting to execute DTMF in a confbridge while file playback (prompt, > announcement, etc) is occurring is not allowed. You have to wait until the > sound file has completed before entering DTMF. This patch fixes it so that > app_confbridge now monitors for dtmf key presses during file playback. If a > key is pressed playback stops and it executes the matched menu option. > > > Diffs > ----- > > branches/11/apps/confbridge/include/confbridge.h 432763 > branches/11/apps/confbridge/conf_config_parser.c 432763 > branches/11/apps/app_confbridge.c 432763 > > Diff: https://reviewboard.asterisk.org/r/4477/diff/ > > > Testing > ------- > > Manual testing done. Setup a basic conference bridge that allowed both > regular and admin users to enter. Ran through various menu options to make > sure the sound file playback would stop (no longer have to wait) and a new > option was executed when appropriate. Also ran the app_confbridge testsuite > tests to make sure they still passed. > > > Thanks, > > Kevin Harwell > >
-- _____________________________________________________________________ -- 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
