Re: [asterisk-dev] [Code Review] 4512: dns: Add res_resolver_unbound module with unit tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4512/#review14748 --- Ship it! This gets a ship it! from me based on the resolver implementation. As the writer of the unit tests, though, I feel like getting approval from someone else would be a good plan. - Mark Michelson On March 19, 2015, 6:36 p.m., Joshua Colp wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4512/ --- (Updated March 19, 2015, 6:36 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24836 https://issues.asterisk.org/jira/browse/ASTERISK-24836 Repository: Asterisk Description --- This change adds a res_resolver_unbound module which uses libunbound and implements the core DNS resolver API. Queries can be started and cancelled as expected. Configuration also exists to change the behavior of the resolver some. Unit tests are present which use the libunbound zone and data API to add local records and then confirm they can be queried and are as expected. Diffs - /trunk/res/res_resolver_unbound.c PRE-CREATION /trunk/makeopts.in 433107 /trunk/configure.ac 433107 /trunk/configs/samples/resolver_unbound.conf.sample PRE-CREATION /trunk/build_tools/menuselect-deps.in 433107 Diff: https://reviewboard.asterisk.org/r/4512/diff/ Testing --- Hacked in a query to my own domain and confirmed it worked. Also ran the unit tests and confirmed they pass. Thanks, Joshua Colp -- _ -- 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
Re: [asterisk-dev] [Code Review] 4511: Audit ast_pjsip_rdata_get_endpoint() usage for ref leaks.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4511/#review14749 --- Ship it! Ship It! - Jonathan Rose On March 17, 2015, 10:04 p.m., rmudgett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4511/ --- (Updated March 17, 2015, 10:04 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- Valgrind found some memory leaks associated with ast_pjsip_rdata_get_endpoint(). The leaks would manifest when sending responses to OPTIONS requests, processing MESSAGE requests, and res_pjsip supplements implementing the incoming_request callback. * Fix ast_pjsip_rdata_get_endpoint() endpoint ref leaks in res/res_pjsip.c:supplement_on_rx_request(), res/res_pjsip/pjsip_options.c:send_options_response(), res/res_pjsip_messaging.c:rx_data_to_ast_msg(), and res/res_pjsip_messaging.c:send_response(). * Eliminated RAII_VAR() use with ast_pjsip_rdata_get_endpoint() in res/res_pjsip_nat.c:nat_on_rx_message(). * Fixed inconsistent but benign return value in res/res_pjsip/pjsip_options.c:options_on_rx_request(). Diffs - /branches/13/res/res_pjsip_nat.c 433089 /branches/13/res/res_pjsip_messaging.c 433089 /branches/13/res/res_pjsip/pjsip_options.c 433089 /branches/13/res/res_pjsip.c 433089 Diff: https://reviewboard.asterisk.org/r/4511/diff/ Testing --- Added temporary logging messages to show that incoming OPTIONS and MESSAGE requests hit the code that leaked the endpoint object refs. Thanks, rmudgett -- _ -- 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
Re: [asterisk-dev] [Code Review] 4511: Audit ast_pjsip_rdata_get_endpoint() usage for ref leaks.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4511/#review14751 --- Ship it! /branches/13/res/res_pjsip_messaging.c https://reviewboard.asterisk.org/r/4511/#comment25336 Incoming SIP requests have to match an endpoint, so I'd either switch this if statement to be an ast_assert() or add an ast_assert() in addition to the if check. - Mark Michelson On March 18, 2015, 3:04 a.m., rmudgett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4511/ --- (Updated March 18, 2015, 3:04 a.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- Valgrind found some memory leaks associated with ast_pjsip_rdata_get_endpoint(). The leaks would manifest when sending responses to OPTIONS requests, processing MESSAGE requests, and res_pjsip supplements implementing the incoming_request callback. * Fix ast_pjsip_rdata_get_endpoint() endpoint ref leaks in res/res_pjsip.c:supplement_on_rx_request(), res/res_pjsip/pjsip_options.c:send_options_response(), res/res_pjsip_messaging.c:rx_data_to_ast_msg(), and res/res_pjsip_messaging.c:send_response(). * Eliminated RAII_VAR() use with ast_pjsip_rdata_get_endpoint() in res/res_pjsip_nat.c:nat_on_rx_message(). * Fixed inconsistent but benign return value in res/res_pjsip/pjsip_options.c:options_on_rx_request(). Diffs - /branches/13/res/res_pjsip_nat.c 433089 /branches/13/res/res_pjsip_messaging.c 433089 /branches/13/res/res_pjsip/pjsip_options.c 433089 /branches/13/res/res_pjsip.c 433089 Diff: https://reviewboard.asterisk.org/r/4511/diff/ Testing --- Added temporary logging messages to show that incoming OPTIONS and MESSAGE requests hit the code that leaked the endpoint object refs. Thanks, rmudgett -- _ -- 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
Re: [asterisk-dev] [Code Review] 4500: ast_register_atexit should only be used when absolutely needed (11 version)
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4500/#review14750 --- Ship it! I'm in the same boat as Matt on this one, but I'll go ahead and tick the Ship it! box. - Mark Michelson On March 15, 2015, 10:33 a.m., Corey Farrell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4500/ --- (Updated March 15, 2015, 10:33 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24142, ASTERISK-24683, ASTERISK-24805, and ASTERISK-24881 https://issues.asterisk.org/jira/browse/ASTERISK-24142 https://issues.asterisk.org/jira/browse/ASTERISK-24683 https://issues.asterisk.org/jira/browse/ASTERISK-24805 https://issues.asterisk.org/jira/browse/ASTERISK-24881 Repository: Asterisk Description --- We've had many issues related to core stop now or core restart now causing segmentation faults. The solution to this is to change almost everything to use ast_register_cleanup. Exceptions: CDR: Flush records. res_musiconhold: Kill external applications. AstDB: Close the DB. canary_exit: Kill canary process. Although some changes from ast_register_atexit to ast_register_cleanup are not strictly necessary, the point is for nothing to use ast_register_atexit except where required. For this reason the change is across the board. Diffs - /branches/11/main/xmldoc.c 432991 /branches/11/main/utils.c 432991 /branches/11/main/udptl.c 432991 /branches/11/main/translate.c 432991 /branches/11/main/timing.c 432991 /branches/11/main/threadstorage.c 432991 /branches/11/main/test.c 432991 /branches/11/main/taskprocessor.c 432991 /branches/11/main/stun.c 432991 /branches/11/main/pbx.c 432991 /branches/11/main/named_acl.c 432991 /branches/11/main/message.c 432991 /branches/11/main/manager.c 432991 /branches/11/main/indications.c 432991 /branches/11/main/image.c 432991 /branches/11/main/http.c 432991 /branches/11/main/format.c 432991 /branches/11/main/file.c 432991 /branches/11/main/features.c 432991 /branches/11/main/event.c 432991 /branches/11/main/dnsmgr.c 432991 /branches/11/main/data.c 432991 /branches/11/main/config.c 432991 /branches/11/main/cli.c 432991 /branches/11/main/channel.c 432991 /branches/11/main/cel.c 432991 /branches/11/main/ccss.c 432991 /branches/11/main/astobj2.c 432991 /branches/11/main/astmm.c 432991 /branches/11/main/astfd.c 432991 /branches/11/main/asterisk.c 432991 /branches/11/main/aoc.c 432991 /branches/11/include/asterisk.h 432991 Diff: https://reviewboard.asterisk.org/r/4500/diff/ Testing --- Compiled, started and ran 'core stop now'. Thanks, Corey Farrell -- _ -- 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
Re: [asterisk-dev] [Code Review] 4189: chan_sip: Simplify dialog/peer references, add REF_DEBUG passthrough of callers to sip_alloc and find_call.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4189/ --- (Updated March 19, 2015, 4:53 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 433115 Bugs: ASTERISK-24882 https://issues.asterisk.org/jira/browse/ASTERISK-24882 Repository: Asterisk Description --- This does have a minor change to sip_ref_peer and dialog_ref - the error messages about trying to reference a NULL is removed. This message provided nothing useful. The changes to sip_alloc / find_call make it easier to trace REF_DEBUG logs for leaked dialogs. Note: I've posted the version of this patch for 13. In trunk the 'struct ast_callid *' type has been replaced with a typedef 'ast_callid', effecting the parameter logger_callid of sip_alloc. Diffs - /branches/13/channels/sip/include/sip.h 433002 /branches/13/channels/sip/include/dialog.h 433002 /branches/13/channels/chan_sip.c 433002 Diff: https://reviewboard.asterisk.org/r/4189/diff/ Testing --- Ran a few testsuite chan_sip tests. Verified that REF_DEBUG log shows caller of sip_alloc. Thanks, Corey Farrell -- _ -- 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
Re: [asterisk-dev] [Code Review] 4497: logger: init_logger_chain has unreachable code
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4497/ --- (Updated March 19, 2015, 5:19 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 433122 Bugs: ASTERISK-24817 https://issues.asterisk.org/jira/browse/ASTERISK-24817 Repository: Asterisk Description --- init_logger_chain has a block of code to set a default console logger if logger.conf was invalid or not found. Since the code is unreachable, no logger channels to be created. This changes it so the default console logger is applied any time logger.conf load fails. Diffs - /branches/11/main/logger.c 432991 Diff: https://reviewboard.asterisk.org/r/4497/diff/ Testing --- Verified that nothing was logged to console with missing logger.conf, and that this patch caused console logging to be enabled in that case. Also verified that valid logger.conf was not effected. Thanks, Corey Farrell -- _ -- 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
[asterisk-dev] [BOUNTY] FATAL: unhandled exception PJLIB/No memory!
Hi, We are getting the following error with asterisk 12 and 13: [Mar 19 10:14:08] ERROR[40185]: pjsip:0 ?: except.c .!!!FATAL: unhandled exception PJLIB/No memory! Can you please advise if I can either increase the memory for PJLIB or the best way to identify what is causing the issue? I am willing to offer a bounty to identify and resolve the issue. Kind regards, Ross -- _ -- 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
Re: [asterisk-dev] [Code Review] 4491: chan_sip: Fix dialog reference leaked to scheduler for reinvite_timeout.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4491/ --- (Updated March 19, 2015, 4:40 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 433112 Bugs: ASTERISK-24876 https://issues.asterisk.org/jira/browse/ASTERISK-24876 Repository: Asterisk Description --- This leak was found by tests/channels/local/local_optimize_away (13+). Scheduler reference to dialog for reinvite timeout is not released during dialog_unlink_all. This issue likely only applies when the system is shutdown before the reinvite timeout expires. Diffs - /branches/11/channels/chan_sip.c 432991 Diff: https://reviewboard.asterisk.org/r/4491/diff/ Testing --- No more leaks for tests/channels/local/local_optimize_away on 13. Thanks, Corey Farrell -- _ -- 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
[asterisk-dev] Re: [BOUNTY] FATAL: unhandled exception PJLIB/No memory!
On 3/19/2015 6:38 AM, Ross Beer wrote: We are getting the following error with asterisk 12 and 13: [Mar 19 10:14:08] ERROR[40185]: pjsip:0 ?: except.c .!!!FATAL: unhandled exception PJLIB/No memory! Ross, Is there an associated issue in Jira? https://issues.asterisk.org/ Kind Regards, Sean -- _ -- 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
Re: [asterisk-dev] [Code Review] 4391: Add blank line between headers and output for Command action response
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4391/ --- (Updated March 20, 2015, 2:34 a.m.) Review request for Asterisk Developers. Changes --- New Patch Behaviour: Output from the CLI command is now sent to the client as a series of Output: headers. malloc, lseek and read errors will now cause an error response to be sent to the client rather than sending no output. Bugs: ASTERISK-24730 https://issues.asterisk.org/jira/browse/ASTERISK-24730 Repository: Asterisk Description --- This patch adds a blank line between the headers and the output in the Command action response. The reason for this change is to make it easier to determine where the headers end and the output from the command starts. Currently, to parse a response to a Command action, one has to: 1. Read one line, if line is Response: Error, parse the remaining as a standard AMI response and stop. 2. Read one more line - or two if you used an ActionID - those lines are the headers. 3. Then read everything up to --END COMMAND--\r\n\r\n. That could be changed to: 1. Read standard AMI response. 2. If Response: Follows, then read everything up to --END COMMAND--\r\n\r\n. The AMI version has been increased to 2.8.0 as this is a protocol change and so that clients detect the new behavior. Adding a blank line should not cause older parsers to fail as they have to read everything up to --END COMMAND--\r\n\r\n anyway. Adding a blank line will also not cause the AMI to HTML/XML encoder to fail to separate the headers from the output as it checks for the presence of a : character, which a blank line does not contain. Diffs (updated) - /trunk/main/manager.c 433198 /trunk/include/asterisk/manager.h 433198 /trunk/UPGRADE.txt 433198 Diff: https://reviewboard.asterisk.org/r/4391/diff/ Testing --- Connected to manager, issued 'core show uptime' command and verified that there was a blank line between the headers and output. Thanks, gareth -- _ -- 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
Re: [asterisk-dev] [Code Review] 4391: Add blank line between headers and output for Command action response
On Jan. 30, 2015, 1:22 a.m., George Joseph wrote: If you run the Testsuite, you'll get a good indication of whether this is truly backwards compatible. gareth wrote: I ran the test apps/queue/set_penalty which makes use of ami.command and it failed. However this appears to be a bug with starpy, it is treading the \r\n as the end of message. Changing it to output just \n results in a successful test. Breaking an existing client library isn't ideal, but the correct delimiter for command output is --END COMMAND--\r\n\r\n, not \r\n\r\n. Corey Farrell wrote: If we do consider modifying the AMI protocol to support text blobs like this, I'd prefer we think about how that should be implemented generically, rather than looking at this one action. So maybe adding 'Text-Terminator: --END COMMAND--' or 'Content-Length: XX' to the headers would allow the next packet to be read as a text block. This way the AMI client libraries could have generic support for this type of response. On the other hand AMI is not HTTP. This is a perfect example of something ARI is better suited for. For that reason I'm not actually in favor of having AMI support text blobs. I'd rather see the command output converted to headers: Response: Success ActionID: actionid Output: line 1 of cli output Output: more Output headers per line of cli output This would of course break existing clients, but would get rid of the exception to the AMI protocol. gareth wrote: Switching to a header-based output would be much better, the current output format seems to lend itself to being mis-parsed. Adding a header to the request would allow users to choose the new format, eg: Action: Command Command: ... OutputHeader: true Corey Farrell wrote: I'm not sure about giving an option. I think if we're bumping the AMI version because of an incompatible change, we should just get rid of what was broken. That's just my opinion though, it's worth hearing opinions of others. Matt Jordan wrote: So, I agree with Corey. I think if we're going to fix this and bump the version number, then let's just bite the bullet and do it. My suggestion is to do the following: 1) Make sure we are happy with this patch and that it is ready to go. 2) When that occurs, we should make a note on the wiki page here: https://wiki.asterisk.org/wiki/display/AST/Asterisk+API+Improvements 3) Sometime prior to cutting a new major branch, we should get together on the -dev list and discuss whether or not it is worth bumping the major version. Given the other things on the list, I think the answer will be yes - and if you think it is worth having the discussion now, then we certainly can start it. Okay, the old output format will be removed. While there, I might as well fix another problem with action_command where it does not send back an error response if malloc, lseek or read fail. - gareth --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4391/#review14384 --- On Jan. 30, 2015, 12:25 a.m., gareth wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4391/ --- (Updated Jan. 30, 2015, 12:25 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24730 https://issues.asterisk.org/jira/browse/ASTERISK-24730 Repository: Asterisk Description --- This patch adds a blank line between the headers and the output in the Command action response. The reason for this change is to make it easier to determine where the headers end and the output from the command starts. Currently, to parse a response to a Command action, one has to: 1. Read one line, if line is Response: Error, parse the remaining as a standard AMI response and stop. 2. Read one more line - or two if you used an ActionID - those lines are the headers. 3. Then read everything up to --END COMMAND--\r\n\r\n. That could be changed to: 1. Read standard AMI response. 2. If Response: Follows, then read everything up to --END COMMAND--\r\n\r\n. The AMI version has been increased to 2.8.0 as this is a protocol change and so that clients detect the new behavior. Adding a blank line should not cause older parsers to fail as they have to read everything up to --END COMMAND--\r\n\r\n anyway. Adding a blank line will also not cause the AMI to HTML/XML encoder to fail to separate the headers from the output as it checks for the presence of a : character, which a blank line does not contain. Diffs -
Re: [asterisk-dev] [Code Review] 4500: ast_register_atexit should only be used when absolutely needed (11 version)
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4500/#review14754 --- Thanks for taking a look. I plan to give this review a couple days after 2 ship it's, it will not be rushed. Also this review won't be committed until r4501 (the version 13 patch) has approval. - Corey Farrell On March 15, 2015, 6:33 a.m., Corey Farrell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4500/ --- (Updated March 15, 2015, 6:33 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24142, ASTERISK-24683, ASTERISK-24805, and ASTERISK-24881 https://issues.asterisk.org/jira/browse/ASTERISK-24142 https://issues.asterisk.org/jira/browse/ASTERISK-24683 https://issues.asterisk.org/jira/browse/ASTERISK-24805 https://issues.asterisk.org/jira/browse/ASTERISK-24881 Repository: Asterisk Description --- We've had many issues related to core stop now or core restart now causing segmentation faults. The solution to this is to change almost everything to use ast_register_cleanup. Exceptions: CDR: Flush records. res_musiconhold: Kill external applications. AstDB: Close the DB. canary_exit: Kill canary process. Although some changes from ast_register_atexit to ast_register_cleanup are not strictly necessary, the point is for nothing to use ast_register_atexit except where required. For this reason the change is across the board. Diffs - /branches/11/main/xmldoc.c 432991 /branches/11/main/utils.c 432991 /branches/11/main/udptl.c 432991 /branches/11/main/translate.c 432991 /branches/11/main/timing.c 432991 /branches/11/main/threadstorage.c 432991 /branches/11/main/test.c 432991 /branches/11/main/taskprocessor.c 432991 /branches/11/main/stun.c 432991 /branches/11/main/pbx.c 432991 /branches/11/main/named_acl.c 432991 /branches/11/main/message.c 432991 /branches/11/main/manager.c 432991 /branches/11/main/indications.c 432991 /branches/11/main/image.c 432991 /branches/11/main/http.c 432991 /branches/11/main/format.c 432991 /branches/11/main/file.c 432991 /branches/11/main/features.c 432991 /branches/11/main/event.c 432991 /branches/11/main/dnsmgr.c 432991 /branches/11/main/data.c 432991 /branches/11/main/config.c 432991 /branches/11/main/cli.c 432991 /branches/11/main/channel.c 432991 /branches/11/main/cel.c 432991 /branches/11/main/ccss.c 432991 /branches/11/main/astobj2.c 432991 /branches/11/main/astmm.c 432991 /branches/11/main/astfd.c 432991 /branches/11/main/asterisk.c 432991 /branches/11/main/aoc.c 432991 /branches/11/include/asterisk.h 432991 Diff: https://reviewboard.asterisk.org/r/4500/diff/ Testing --- Compiled, started and ran 'core stop now'. Thanks, Corey Farrell -- _ -- 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
Re: [asterisk-dev] [Code Review] 4503: SAC: Configure customer advocate/sales queues
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4503/#review14752 --- As an update - when testing with my external connectivity patch I've run into some one-way audio and issues where in certain calling scenarios MOH does not play. I'm investigating that further tomorrow. - rnewton On March 16, 2015, 3:37 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4503/ --- (Updated March 16, 2015, 3:37 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- As described on the wiki at: The Sales Queue may be reached externally by dialing 256-555-1200, or internally by dialing 1200. The Customer Experience Queue may be reached externally by dialing 256-555-1250, or internally by dialing 1250. Sales Queue Calls to the Sales Queue should ring Terry, Garnet and Franny in ring-all fashion If no one answers a call to the Sales Queue after 5 minutes, the caller should be directed to the Operator so that the Operator can take a message and have a Sales person contact the caller at a later time. Customer Advocate Queue Calls to the Customer Advocate Queue should ring Maria, Dusty and Tommie in ring-all fashion. If no one answers a call to the Customer Advocate queue after 20 minutes, the caller should be directed to the Operator so that the Operator can take a message and have a Customer Advocate contact the caller at a later time. NOTE: This review is accidentally against trunk, but the patch is perfectly portable, so that shouldn't be a problem. NOTE 2: External extensions for the queues still need to be added since the outside connectivity patch hasn't been merged yet https://reviewboard.asterisk.org/r/4488/diff/#index_header Diffs - /trunk/configs/basic-pbx/queues.conf PRE-CREATION /trunk/configs/basic-pbx/modules.conf 432443 /trunk/configs/basic-pbx/extensions.conf 432443 Diff: https://reviewboard.asterisk.org/r/4503/diff/ Testing --- Had some phones register as agents in the queues as well as the operator, made sure the queue members were dialed in ring-all fashion and that the timeouts occurred and the operator was dialed as expected. Thanks, Jonathan Rose -- _ -- 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
Re: [asterisk-dev] [Code Review] 4515: build_peer peer mailbox management bug
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4515/#review14755 --- /trunk/channels/chan_sip.c https://reviewboard.asterisk.org/r/4515/#comment25337 This could result in a change of behaviour. In build_peer 'first_pass' will no longer decide if the existing mailboxes get cleared, it would only be done when '!dev_state'. I feel that a safer change would be to just remove the delme variable, let the current logic live on. I could be convinced otherwise if you can show that the current logic produces the wrong results, but even then we'd have to very careful. - Corey Farrell On March 19, 2015, 11:02 p.m., gareth wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4515/ --- (Updated March 19, 2015, 11:02 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24871 https://issues.asterisk.org/jira/browse/ASTERISK-24871 Repository: Asterisk Description --- During a reload, build_peer iterates over the peer's mailboxes and tags them for removal via the delme variable. It adds any new, unique mailboxes to the peer via add_peer_mailboxes and then removes any mailboxes with delme still set. However, there isn't any code to unset delme, so this would remove any previously configured mailboxes. That is not what happens though because build_peer also calls set_peer_defaults which clears out all of the configured mailboxes using clear_peer_mailboxes making the setting of delme redundant. So in the end there is no impact to the user because all the configured mailboxes get added regardless. Patch unsets delme for existing, still-configured mailboxes in add_peer_mailboxes and removes call to clear_peer_mailboxes. Diffs - /trunk/channels/chan_sip.c 433198 Diff: https://reviewboard.asterisk.org/r/4515/diff/ Testing --- Added new mailboxes to peer, reloaded chan_sip and verified that existing mailboxes were still there and new mailboxes had been added. Removed mailboxes from peer, reloaded chan_sip and verified that those mailboxes were no longer assigned to peer. Thanks, gareth -- _ -- 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
[asterisk-dev] [Code Review] 4515: build_peer peer mailbox management bug
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4515/ --- Review request for Asterisk Developers. Bugs: ASTERISK-24871 https://issues.asterisk.org/jira/browse/ASTERISK-24871 Repository: Asterisk Description --- During a reload, build_peer iterates over the peer's mailboxes and tags them for removal via the delme variable. It adds any new, unique mailboxes to the peer via add_peer_mailboxes and then removes any mailboxes with delme still set. However, there isn't any code to unset delme, so this would remove any previously configured mailboxes. That is not what happens though because build_peer also calls set_peer_defaults which clears out all of the configured mailboxes using clear_peer_mailboxes making the setting of delme redundant. So in the end there is no impact to the user because all the configured mailboxes get added regardless. Patch unsets delme for existing, still-configured mailboxes in add_peer_mailboxes and removes call to clear_peer_mailboxes. Diffs - /trunk/channels/chan_sip.c 433198 Diff: https://reviewboard.asterisk.org/r/4515/diff/ Testing --- Added new mailboxes to peer, reloaded chan_sip and verified that existing mailboxes were still there and new mailboxes had been added. Removed mailboxes from peer, reloaded chan_sip and verified that those mailboxes were no longer assigned to peer. Thanks, gareth -- _ -- 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
Re: [asterisk-dev] [Code Review] 4477: app_confbridge (11): file playback blocks dtmf
On March 18, 2015, 12:39 p.m., rmudgett wrote: branches/11/apps/app_confbridge.c, lines 1847-1853 https://reviewboard.asterisk.org/r/4477/diff/2/?file=72633#file72633line1847 Swaping which plays first will allow the channel's prompt to be interruptable. Although doing this will make it kind of awkward that there will be a delay in the user hearing the prompt. Good suggestion, but I think swapping it and having a delayed playback is more ackward than not being allowed to interrupt the playback. - Kevin --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4477/#review14733 --- On March 17, 2015, 1 p.m., Kevin Harwell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4477/ --- (Updated March 17, 2015, 1 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 432991 branches/11/apps/confbridge/conf_state_multi_marked.c 432991 branches/11/apps/app_confbridge.c 432991 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
Re: [asterisk-dev] [BOUNTY] FATAL: unhandled exception PJLIB/No memory!
Hi Sean, I was just about to pose the reference: https://issues.asterisk.org/jira/browse/ASTERISK-24893 I have just uploaded a backtrace for the latest crash. I would be extremely grateful if you could take a look. Kind regards, Ross To: asterisk-dev@lists.digium.com From: sean.bri...@gmail.com Date: Thu, 19 Mar 2015 08:43:01 -0400 Subject: [asterisk-dev] Re: [BOUNTY] FATAL: unhandled exception PJLIB/No memory! On 3/19/2015 6:38 AM, Ross Beer wrote: We are getting the following error with asterisk 12 and 13: [Mar 19 10:14:08] ERROR[40185]: pjsip:0 ?: except.c .!!!FATAL: unhandled exception PJLIB/No memory! Ross, Is there an associated issue in Jira? https://issues.asterisk.org/ Kind Regards, Sean -- _ -- 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 -- _ -- 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
Re: [asterisk-dev] [Code Review] 4503: SAC: Configure customer advocate/sales queues
On March 17, 2015, 9:44 p.m., Mark Michelson wrote: Everything looks good here. Should this review stay open for the external extensions to be added, or will that be a separate review? Jonathan Rose wrote: I'll leave both reviews open for now. I'm finally testing with these with outside connectivity this morning, so we'll see if anything needs to change. - rnewton --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4503/#review14730 --- On March 16, 2015, 3:37 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4503/ --- (Updated March 16, 2015, 3:37 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- As described on the wiki at: The Sales Queue may be reached externally by dialing 256-555-1200, or internally by dialing 1200. The Customer Experience Queue may be reached externally by dialing 256-555-1250, or internally by dialing 1250. Sales Queue Calls to the Sales Queue should ring Terry, Garnet and Franny in ring-all fashion If no one answers a call to the Sales Queue after 5 minutes, the caller should be directed to the Operator so that the Operator can take a message and have a Sales person contact the caller at a later time. Customer Advocate Queue Calls to the Customer Advocate Queue should ring Maria, Dusty and Tommie in ring-all fashion. If no one answers a call to the Customer Advocate queue after 20 minutes, the caller should be directed to the Operator so that the Operator can take a message and have a Customer Advocate contact the caller at a later time. NOTE: This review is accidentally against trunk, but the patch is perfectly portable, so that shouldn't be a problem. NOTE 2: External extensions for the queues still need to be added since the outside connectivity patch hasn't been merged yet https://reviewboard.asterisk.org/r/4488/diff/#index_header Diffs - /trunk/configs/basic-pbx/queues.conf PRE-CREATION /trunk/configs/basic-pbx/modules.conf 432443 /trunk/configs/basic-pbx/extensions.conf 432443 Diff: https://reviewboard.asterisk.org/r/4503/diff/ Testing --- Had some phones register as agents in the queues as well as the operator, made sure the queue members were dialed in ring-all fashion and that the timeouts occurred and the operator was dialed as expected. Thanks, Jonathan Rose -- _ -- 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
Re: [asterisk-dev] [Code Review] 4504: SAC: Add conferences for employees / employees+customers
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4504/#review14741 --- Looks good. I'm finally testing with these with outside connectivity this morning, so we'll see if anything needs to change. - rnewton On March 16, 2015, 5:48 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4504/ --- (Updated March 16, 2015, 5:48 p.m.) Review request for Asterisk Developers and rnewton. Repository: Asterisk Description --- From: https://wiki.asterisk.org/wiki/display/AST/Super+Awesome+Company SAC requires two conference rooms, one for use by employees only and one for use by employees and customers (outside connectivity still needs to be established so that 555-6500 can be added and customers can actually dial into said conference) Diffs - /branches/13/configs/basic-pbx/modules.conf 432996 /branches/13/configs/basic-pbx/extensions.conf 432996 /branches/13/configs/basic-pbx/confbridge.conf PRE-CREATION Diff: https://reviewboard.asterisk.org/r/4504/diff/ Testing --- Made sure app_confbridge loaded and internal users were able to dial into the conferences. Thanks, Jonathan Rose -- _ -- 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
Re: [asterisk-dev] [Code Review] 4512: dns: Add res_resolver_unbound module with unit tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4512/ --- (Updated March 19, 2015, 6:36 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24836 https://issues.asterisk.org/jira/browse/ASTERISK-24836 Repository: Asterisk Description --- This change adds a res_resolver_unbound module which uses libunbound and implements the core DNS resolver API. Queries can be started and cancelled as expected. Configuration also exists to change the behavior of the resolver some. Unit tests are present which use the libunbound zone and data API to add local records and then confirm they can be queried and are as expected. Diffs (updated) - /trunk/res/res_resolver_unbound.c PRE-CREATION /trunk/makeopts.in 433107 /trunk/configure.ac 433107 /trunk/configs/samples/resolver_unbound.conf.sample PRE-CREATION /trunk/build_tools/menuselect-deps.in 433107 Diff: https://reviewboard.asterisk.org/r/4512/diff/ Testing --- Hacked in a query to my own domain and confirmed it worked. Also ran the unit tests and confirmed they pass. Thanks, Joshua Colp -- _ -- 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
Re: [asterisk-dev] [Code Review] 4488: Super Awesome Company: Phase 1 - Patch 2 - Outside Connectivity!
On March 16, 2015, 1:57 p.m., Matt Jordan wrote: /branches/13/configs/basic-pbx/extensions.conf, lines 36-42 https://reviewboard.asterisk.org/r/4488/diff/1/?file=72117#file72117line36 Does this really need to be a separate context? I'm all for having contexts break up logical groupings of subroutines, but the dialplan here feels like it is getting a bit out of control. Subroutines can already be named via an actual extension name - when you have 'catch alls' in the various contexts, that feels like a sign that things aren't being set up at the right granularity. For example, this could just be in your [Internal] context: [Internal] exten = internal_setup,1,NoOp() same = n,Set(CDR_PROP(disable)=1) same = n,Return() Instead of invoking this with an explicit Goto, use GoSub. That way it can be called from anywhere, and doesn't require a lot of Gotos to jump around. Generally, you should always prefer Goto over GoSub, unless you *really* want them to leave the context they are in. Jonathan Rose wrote: Hmmm, the reason he did this was because he didn't want to add setup code to every extension in the [Internal] context. Using a gosub works here, but it will still require invoking the gosub on every extension and what he wanted to do was just automatically call this stuff on everything that goes into the Internal context. Plus this way the CDR disabling stuff gets ran when an internal context calls into an extension that is just included by Internal. Matt Jordan wrote: Hm. That's a fair point. I'd rename this slightly then: have the Pre-Internal be Internal - since it should be applied to all internal extensions - and place the actual internal extensions into some other context. rnewton wrote: I'm changing Pre-Internal to Internal and Internal to General as the contexts included there are not really specific to internal use. Hah I forgot about the default general context. That caused me some confusion. - rnewton --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4488/#review14698 --- On March 13, 2015, 2:32 p.m., rnewton wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4488/ --- (Updated March 13, 2015, 2:32 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- Howdy, here is another patch for the Super Awesome Company configuration. We are still in phase 1. The general requirements are posted on the wiki: https://wiki.asterisk.org/wiki/display/AST/Super+Awesome+Company The specific requirements this patch meets are below: pjsip.conf * SIP ITSP configuration example and have place holders for the required authentication bits. ** Assume that Asterisk does not have a public IP address, and sits behind a NAT with its desk phones. * Have outbound registration to the SIP trunk, and an endpoint that represents the SIP trunk. * Inbound calls received from the SIP trunk should go into their own context. extensions.conf * Match the outbound dial request so that it can only dial US area codes. ** Don't let people dial 900 numbers, international numbers, or any other numbers that could result in a charge * Inbound calls from the SIP trunk should hit a basic Auto Attendant that prompts them for the extension to dial, after greeting them to SAC. * If an inbound call matches a DID that maps to a specific extension/device, dial that extension/device directly. Billing * Make sure CDRs output all calls that are from/to the SIP trunk. These should be logged to a CSV. * For intra-office calls, kill the CDRs. Additional Requirements Noted: * For outbound calls, each SAC employee’s 10-digit DID number is provided as their Caller ID. * Voicemail may be accessed remotely by employees who dial 256-555-1234. When employees dial voicemail remotely, they must input both their mailbox number and their pin code. * 7, 10 and 10+1 digit dialing for local and long distance calls. * Internal dialing of otherwise inbound features, ** 1100 to reach the main IVR. * The IVR options possible without getting into Phase 2. Diffs - /branches/13/configs/basic-pbx/pjsip.conf 432866 /branches/13/configs/basic-pbx/modules.conf 432866 /branches/13/configs/basic-pbx/logger.conf 432866 /branches/13/configs/basic-pbx/extensions.conf 432866 Diff: https://reviewboard.asterisk.org/r/4488/diff/ Testing --- Setup with a Digium Cloud Services trunk and a few internal
Re: [asterisk-dev] [Code Review] 4391: Add blank line between headers and output for Command action response
On Jan. 29, 2015, 7:22 p.m., George Joseph wrote: If you run the Testsuite, you'll get a good indication of whether this is truly backwards compatible. gareth wrote: I ran the test apps/queue/set_penalty which makes use of ami.command and it failed. However this appears to be a bug with starpy, it is treading the \r\n as the end of message. Changing it to output just \n results in a successful test. Breaking an existing client library isn't ideal, but the correct delimiter for command output is --END COMMAND--\r\n\r\n, not \r\n\r\n. Corey Farrell wrote: If we do consider modifying the AMI protocol to support text blobs like this, I'd prefer we think about how that should be implemented generically, rather than looking at this one action. So maybe adding 'Text-Terminator: --END COMMAND--' or 'Content-Length: XX' to the headers would allow the next packet to be read as a text block. This way the AMI client libraries could have generic support for this type of response. On the other hand AMI is not HTTP. This is a perfect example of something ARI is better suited for. For that reason I'm not actually in favor of having AMI support text blobs. I'd rather see the command output converted to headers: Response: Success ActionID: actionid Output: line 1 of cli output Output: more Output headers per line of cli output This would of course break existing clients, but would get rid of the exception to the AMI protocol. gareth wrote: Switching to a header-based output would be much better, the current output format seems to lend itself to being mis-parsed. Adding a header to the request would allow users to choose the new format, eg: Action: Command Command: ... OutputHeader: true Corey Farrell wrote: I'm not sure about giving an option. I think if we're bumping the AMI version because of an incompatible change, we should just get rid of what was broken. That's just my opinion though, it's worth hearing opinions of others. So, I agree with Corey. I think if we're going to fix this and bump the version number, then let's just bite the bullet and do it. My suggestion is to do the following: 1) Make sure we are happy with this patch and that it is ready to go. 2) When that occurs, we should make a note on the wiki page here: https://wiki.asterisk.org/wiki/display/AST/Asterisk+API+Improvements 3) Sometime prior to cutting a new major branch, we should get together on the -dev list and discuss whether or not it is worth bumping the major version. Given the other things on the list, I think the answer will be yes - and if you think it is worth having the discussion now, then we certainly can start it. - Matt --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4391/#review14384 --- On Jan. 29, 2015, 6:25 p.m., gareth wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4391/ --- (Updated Jan. 29, 2015, 6:25 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24730 https://issues.asterisk.org/jira/browse/ASTERISK-24730 Repository: Asterisk Description --- This patch adds a blank line between the headers and the output in the Command action response. The reason for this change is to make it easier to determine where the headers end and the output from the command starts. Currently, to parse a response to a Command action, one has to: 1. Read one line, if line is Response: Error, parse the remaining as a standard AMI response and stop. 2. Read one more line - or two if you used an ActionID - those lines are the headers. 3. Then read everything up to --END COMMAND--\r\n\r\n. That could be changed to: 1. Read standard AMI response. 2. If Response: Follows, then read everything up to --END COMMAND--\r\n\r\n. The AMI version has been increased to 2.8.0 as this is a protocol change and so that clients detect the new behavior. Adding a blank line should not cause older parsers to fail as they have to read everything up to --END COMMAND--\r\n\r\n anyway. Adding a blank line will also not cause the AMI to HTML/XML encoder to fail to separate the headers from the output as it checks for the presence of a : character, which a blank line does not contain. Diffs - /trunk/main/manager.c 431113 /trunk/include/asterisk/manager.h 431113 /trunk/CHANGES 431113 Diff: https://reviewboard.asterisk.org/r/4391/diff/ Testing --- Connected to manager, issued 'core show uptime' command and verified that there was a
Re: [asterisk-dev] [Code Review] 4513: res_pjsip_sdp_rtp, sorcery: Fix invalid access and memory leak respectively.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4513/#review14745 --- Ship it! Ship It! - Matt Jordan On March 18, 2015, 4:51 p.m., rmudgett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4513/ --- (Updated March 18, 2015, 4:51 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- Valgrind found a memory leak and invalid access. * Fix invalid access by sscanf() being fed a non-nul terminated string of digits in res/res_pjsip_sdp_rtp.c:get_codecs(). * Fix memory leak in main/sorcery.c:sorcery_object_field_destructor(). * Fix potential NULL pointer dereference in main/xmldoc.c:xmldoc_get_syntax_config_option(). Diffs - /branches/13/res/res_pjsip_sdp_rtp.c 433107 /branches/13/main/xmldoc.c 433107 /branches/13/main/sorcery.c 433107 Diff: https://reviewboard.asterisk.org/r/4513/diff/ Testing --- * Placed a PJSIP call and observed that valgrind no longer complains of sscanf() performing an invalid read in get_codecs(). * Valgrind no longer complains of definitely leaked memory resulting from the sorcery_object_field_destructor(). Thanks, rmudgett -- _ -- 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
Re: [asterisk-dev] [Code Review] 4505: Asterisk 13 PJSIP sends RTP packets in wrong byte order on Intel platform when using slin codec
On March 17, 2015, 11:57 a.m., Matt Jordan wrote: /tags/13.2.0/include/asterisk/codec.h, lines 77-80 https://reviewboard.asterisk.org/r/4505/diff/1/?file=72588#file72588line77 I don't think you can trust that the codec will know its endianness. Looking at the resample code, I don't _think_ it actually determines the endianness of its encoding/decoding, and instead relies on the underlying machine to make that determination. As such, I don't think this should be a property on the codec structure. Frankie Chin wrote: Matt, I actually followed the implementation in Asterisk 12.8.1 where the AST_SMOOTHER_FLAG_BE was defined for all the SLIN codecs in main/format.c under the format_list_init() method. Do you mean this implementation back in 12.8.1 was inappropriate? FYI, slin codec used to work fine in Asterisk 12.8.1 for our application. Apologies; you are absolutely correct about that. I'll need a day or two to think about whether or not this is the right way to handle passing smoother flags in. A part of me dislikes having an explicit BE byte order integer, but I'm also not a giant fan of storing a bunch of smoother properties directly on the codec. There may just not be a better place for it. - Matt --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4505/#review14719 --- On March 16, 2015, 10:36 p.m., Frankie Chin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4505/ --- (Updated March 16, 2015, 10:36 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24858 https://issues.asterisk.org/jira/browse/ASTERISK-24858 Repository: Asterisk Description --- In Asterisk 13.2.0 when SLIN codec is used in two Asterisk servers registered to one another via PJSIP, the RTP payload is sent in the wrong byte order. The patch addresses the following based on the correct behavior in Asterisk 12.8.1: 1) Save ptime = 20 as the framing in the ast_rtp_codecs structure when creating outgoing SDP packet (res_pjsip_sdp_rtp.c) 2) Do not copy the framing when copying the payload (rtp_engine.c) 3) Introduce the new smoother_be flagin the ast_codec structure. Set this flag = 1 for all the SLIN codecs (codec_builtin.c). 4) Check for this smoother_be flag before using the smoother on the data (res_rtp_asterisk.c) Diffs - /tags/13.2.0/res/res_rtp_asterisk.c 433002 /tags/13.2.0/res/res_pjsip_sdp_rtp.c 433002 /tags/13.2.0/main/rtp_engine.c 433002 /tags/13.2.0/main/format.c 433002 /tags/13.2.0/main/codec_builtin.c 433002 /tags/13.2.0/include/asterisk/format.h 433002 /tags/13.2.0/include/asterisk/codec.h 433002 Diff: https://reviewboard.asterisk.org/r/4505/diff/ Testing --- The patch was tested using the scenario described in ASTERISK-24858 Thanks, Frankie Chin -- _ -- 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
Re: [asterisk-dev] [Code Review] 4500: ast_register_atexit should only be used when absolutely needed (11 version)
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4500/#review14747 --- I looked through everything, and it all made sense to me - but I think it might be good to have another pair of eyes on this as well. I'll ping rmudgett and see if he wouldn't mind taking a look at it. - Matt Jordan On March 15, 2015, 5:33 a.m., Corey Farrell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4500/ --- (Updated March 15, 2015, 5:33 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24142, ASTERISK-24683, ASTERISK-24805, and ASTERISK-24881 https://issues.asterisk.org/jira/browse/ASTERISK-24142 https://issues.asterisk.org/jira/browse/ASTERISK-24683 https://issues.asterisk.org/jira/browse/ASTERISK-24805 https://issues.asterisk.org/jira/browse/ASTERISK-24881 Repository: Asterisk Description --- We've had many issues related to core stop now or core restart now causing segmentation faults. The solution to this is to change almost everything to use ast_register_cleanup. Exceptions: CDR: Flush records. res_musiconhold: Kill external applications. AstDB: Close the DB. canary_exit: Kill canary process. Although some changes from ast_register_atexit to ast_register_cleanup are not strictly necessary, the point is for nothing to use ast_register_atexit except where required. For this reason the change is across the board. Diffs - /branches/11/main/xmldoc.c 432991 /branches/11/main/utils.c 432991 /branches/11/main/udptl.c 432991 /branches/11/main/translate.c 432991 /branches/11/main/timing.c 432991 /branches/11/main/threadstorage.c 432991 /branches/11/main/test.c 432991 /branches/11/main/taskprocessor.c 432991 /branches/11/main/stun.c 432991 /branches/11/main/pbx.c 432991 /branches/11/main/named_acl.c 432991 /branches/11/main/message.c 432991 /branches/11/main/manager.c 432991 /branches/11/main/indications.c 432991 /branches/11/main/image.c 432991 /branches/11/main/http.c 432991 /branches/11/main/format.c 432991 /branches/11/main/file.c 432991 /branches/11/main/features.c 432991 /branches/11/main/event.c 432991 /branches/11/main/dnsmgr.c 432991 /branches/11/main/data.c 432991 /branches/11/main/config.c 432991 /branches/11/main/cli.c 432991 /branches/11/main/channel.c 432991 /branches/11/main/cel.c 432991 /branches/11/main/ccss.c 432991 /branches/11/main/astobj2.c 432991 /branches/11/main/astmm.c 432991 /branches/11/main/astfd.c 432991 /branches/11/main/asterisk.c 432991 /branches/11/main/aoc.c 432991 /branches/11/include/asterisk.h 432991 Diff: https://reviewboard.asterisk.org/r/4500/diff/ Testing --- Compiled, started and ran 'core stop now'. Thanks, Corey Farrell -- _ -- 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
Re: [asterisk-dev] [Code Review] 4510: app_confbridge (13): file playback blocks dtmf
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4510/ --- (Updated March 19, 2015, 4:59 p.m.) Review request for Asterisk Developers. Changes --- Addressed review findings. Also decided to leave the playback_and_continue code alone for now as it currently handles what it needs to do on its own. Bugs: ASTERISK-24864 https://issues.asterisk.org/jira/browse/ASTERISK-24864 Repository: Asterisk Description --- This is the Asterisk 13 version of the following: https://reviewboard.asterisk.org/r/4477/ 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. Unlike the Asterisk 11 patch this version does not re-queue the dtmf frame, but instead uses an already available function that monitors for dtmf presses during playback. Diffs (updated) - branches/13/main/bridge_channel.c 433195 branches/13/include/asterisk/bridge_channel.h 433195 branches/13/apps/app_confbridge.c 433195 Diff: https://reviewboard.asterisk.org/r/4510/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
Re: [asterisk-dev] [Code Review] 4477: app_confbridge (11): file playback blocks dtmf
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4477/ --- (Updated March 19, 2015, 4:59 p.m.) Review request for Asterisk Developers. Changes --- Addressed review findings. 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 (updated) - branches/11/apps/app_confbridge.c 433195 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
Re: [asterisk-dev] [Code Review] 4473: chan_pjsip: Add rpid_immediate option to prevent unnecessary 180 Ringing messages.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4473/ --- (Updated March 19, 2015, 10:59 a.m.) Review request for Asterisk Developers. Changes --- Addressed feedback: Added note in CHANGES and added alembic script. Still need to make tests for the testsuite to cover the new rpid_immediate option. Bugs: ASTERISK-24781 https://issues.asterisk.org/jira/browse/ASTERISK-24781 Repository: Asterisk Description --- Incoming PJSIP call legs that have not been answered yet send unnecessary 180 Ringing or 183 Progress messages every time a connected line update happens. If the outgoing channel is also PJSIP then the incoming channel will always send a 180 Ringing or 183 Progress message when the outgoing channel sends the INVITE. Consequences of these unnecessary messages: * The caller can start hearing ringback before the far end even gets the call. * Many phones tend to grab the first connected line information and refuse to update the display if it changes. The first information is not likely to be correct if the call goes to an endpoint not under the control of the first Asterisk box. When connected line first went into Asterisk in v1.8, chan_sip received an undocumented option rpid_immediate that defaults to disabled. When enabled, the option immediately passes connected line update information to the caller in 180 Ringing or 183 Progress messages as described above. * Added rpid_immediate option to prevent unnecessary 180 Ringing or 183 Progress messages. The default is no to disable sending the unnecessary messages. Diffs (updated) - /branches/13/res/res_pjsip/pjsip_configuration.c 433152 /branches/13/res/res_pjsip.c 433152 /branches/13/include/asterisk/res_pjsip.h 433152 /branches/13/contrib/ast-db-manage/config/versions/23530d604b96_add_rpid_immediate.py PRE-CREATION /branches/13/configs/samples/pjsip.conf.sample 433152 /branches/13/channels/chan_pjsip.c 433152 /branches/13/CHANGES 433152 Diff: https://reviewboard.asterisk.org/r/4473/diff/ Testing --- * Ran the tests/channels/pjsip testsuite tests. They still pass. * Made a call chain as follows: 100 - * - * - * - 200. With the patch there are no unnecessary messages. Without the patch there were several 180 Ringing messages sent back to the caller. Thanks, rmudgett -- _ -- 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
Re: [asterisk-dev] [Code Review] 4514: Add raw DNS answer to DNS results
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4514/ --- (Updated March 19, 2015, 10:16 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers and Joshua Colp. Changes --- Committed in revision 433150 Repository: Asterisk Description --- Adding NAPTR and SRV support involves the need to decode domain-names in records into strings. The way this is typically done is through the dn_expand() function. The problem is that the dn_expand() function requires the entire DNS answer in order to decode the domain. The current DNS API does not grant access to the raw DNS answer, meaning that trying to parse NAPTR or SRV records is not possible. This patch adds the DNS answer to the ast_dns_result structure, as well as a function to retrieve the DNS answer from the structure. The unit tests have been updated to add a phony DNS answer where necessary. The nominal DNS result test checks for the DNS answer to be what is expected. The off-nominal test for setting DNS results also ensures that a NULL answer or a 0 answer length will be rejected by the DNS core. Diffs - /team/group/dns/tests/test_dns_recurring.c 433109 /team/group/dns/tests/test_dns.c 433109 /team/group/dns/res/res_resolver_unbound.c 433109 /team/group/dns/main/dns_core.c 433109 /team/group/dns/include/asterisk/dns_resolver.h 433109 /team/group/dns/include/asterisk/dns_internal.h 433109 /team/group/dns/include/asterisk/dns_core.h 433109 Diff: https://reviewboard.asterisk.org/r/4514/diff/ Testing --- All unit tests continue to pass. Thanks, Mark Michelson -- _ -- 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
