Re: [asterisk-dev] [Code Review] 3997: bridge_native_rtp: Fix odd audio issues when transitioning from native remote RTP bridge to softmix

2014-10-13 Thread rmudgett
/bridge_native_rtp.c https://reviewboard.asterisk.org/r/3997/#comment24023 Similar here with glue1 check. - rmudgett On Oct. 13, 2014, 12:32 p.m., Matt Jordan wrote: --- This is an automatically generated e-mail. To reply, visit: https

[asterisk-dev] [Code Review] 4074: AMI: Missing SetVar events when a channel inherits variables.

2014-10-13 Thread rmudgett
are now generated for all channel variables initially passed from Local;1 to Local;2. Before this patch the AMI SetVar events didn't happen on channel variable inheritance and the initial Local;2 channel variables obtained from Local;1. Thanks, rmudgett

Re: [asterisk-dev] [Code Review] 4074: AMI: Add missing VarSet events when a channel inherits variables.

2014-10-13 Thread rmudgett
happen on channel variable inheritance and the initial Local;2 channel variables obtained from Local;1. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list

Re: [asterisk-dev] [Code Review] 4075: parking/tests: Running res_parking unit tests would cause assertions and possibly a crash due to attempting to play MOH on a channel with no formats

2014-10-13 Thread rmudgett
://reviewboard.asterisk.org/r/4075/#comment24027 Why is chan in parentheses? - rmudgett On Oct. 13, 2014, 3:59 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4075

Re: [asterisk-dev] [Code Review] 4075: parking/tests: Running res_parking unit tests would cause assertions and possibly a crash due to attempting to play MOH on a channel with no formats

2014-10-13 Thread rmudgett
://reviewboard.asterisk.org/r/4075/#comment24029 You don't need to use safe_channel_release() here. You can just use ast_channel_release() since you know that alice is not NULL. - rmudgett On Oct. 13, 2014, 5:18 p.m., Jonathan Rose wrote

[asterisk-dev] [Code Review] 4066: stasis_channels.c: Resolve unfinished Dials when doing masquerades (Part 2)

2014-10-09 Thread rmudgett
the Bridge application on the caller channel. 4) A callee channel masquerades out of dial. The case happens when using the Bridge application on a peer channel. The four cases are now handled as expected from the dial events perspective by this patch. Thanks, rmudgett

Re: [asterisk-dev] [Code Review] 4057: bridge: Every time a bridge lies during the smart bridge operation I cry

2014-10-09 Thread rmudgett
Similar here /branches/12/main/bridge.c https://reviewboard.asterisk.org/r/4057/#comment24002 This is now the comment for the loop: Add any new channels or re-add existing channels to the bridge. - rmudgett On Oct. 8, 2014, 11:41 a.m., Joshua Colp wrote

Re: [asterisk-dev] [Code Review] 4057: bridge: Every time a bridge lies during the smart bridge operation I cry

2014-10-09 Thread rmudgett
the preincrement/predecrement form in stand alone statements instead of post-increment/post-decrement... :) /branches/12/main/bridge.c https://reviewboard.asterisk.org/r/4057/#comment24003 ...re-add existing channels... - rmudgett On Oct. 9, 2014, 1:16 p.m., Joshua Colp wrote

Re: [asterisk-dev] [Code Review] 4067: CallerID: Fix parsing regression

2014-10-09 Thread rmudgett
removing the quotes. Suggest manually doing ast_strip() before calling ast_strip_quoted() to detect that just quotes were stripped. - rmudgett On Oct. 9, 2014, 1:59 p.m., opticron wrote: --- This is an automatically generated e

Re: [asterisk-dev] [Code Review] 4067: CallerID: Fix parsing regression

2014-10-09 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4067/#review13485 --- Ship it! Ship It! - rmudgett On Oct. 9, 2014, 2:37 p.m

Re: [asterisk-dev] [Code Review] 4041: features.c: Fix lingering channel ref while Bridge() application is active.

2014-10-06 Thread rmudgett
bridge. 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] 4047: astobj2: ao2_callback with OBJ_MULTIPLE causes REF_DEBUG to report false leaks

2014-10-06 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4047/#review13456 --- Ship it! Ship It! - rmudgett On Oct. 5, 2014, 7:44 a.m

Re: [asterisk-dev] [Code Review] 3997: bridge_native_rtp: Fix odd audio issues when transitioning from native remote RTP bridge to softmix

2014-10-06 Thread rmudgett
. - rmudgett On Oct. 5, 2014, 6:08 p.m., Matt Jordan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3997/ --- (Updated Oct

Re: [asterisk-dev] [Code Review] 4035: Dialplan function to get first/head caller channel on queue

2014-10-03 Thread rmudgett
permissions to do so. - rmudgett On Oct. 3, 2014, 1:14 a.m., Kristian Høgh wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4035

Re: [asterisk-dev] [Code Review] 4034: chan_pjsip: Fix deadlock when masquerading PJSIP channels.

2014-10-03 Thread rmudgett
usually deadlock. * Performed a SIP attended transfer to an appliction. The transfer still works. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list

Re: [asterisk-dev] [Code Review] 4046: audiohooks: Reevaluate the bridge technology when an audiohook is added or removed.

2014-10-03 Thread rmudgett
changed to simple_bridge 4 Stopped the MixMonitor recorder using AMI StopMixMonitor action. 5 The bridge technology now changes to native with the patch. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api

Re: [asterisk-dev] [Code Review] 4043: chan_motif: Release format capabilities on module load error

2014-10-03 Thread rmudgett
://reviewboard.asterisk.org/r/4043/#comment23979 You should add: ao2_global_obj_release(globals); after aco_info_destroy() here in case one of the last three goto end's were taken. - rmudgett On Oct. 1, 2014, 5:28 p.m., Corey Farrell wrote

Re: [asterisk-dev] [Code Review] 4037: Release AMI connections on shutdown

2014-10-03 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4037/#review13452 --- Ship it! Ship It! - rmudgett On Oct. 3, 2014, 3:39 p.m

Re: [asterisk-dev] [Code Review] 4043: chan_motif: Release format capabilities on module load error

2014-10-03 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4043/#review13453 --- Ship it! Ship It! - rmudgett On Oct. 3, 2014, 3:28 p.m

Re: [asterisk-dev] [Code Review] 4035: Dialplan function to get first/head caller channel on queue

2014-10-02 Thread rmudgett
On Oct. 1, 2014, 10:47 a.m., rmudgett wrote: /trunk/apps/app_queue.c, lines 8364-8366 https://reviewboard.asterisk.org/r/4035/diff/2-4/?file=67798#file67798line8364 guidelines: variable declarations go at the beginning of a block. struct call_queue tmpq

Re: [asterisk-dev] [Code Review] 4040: Manager: Add missing fields and documentation for CoreShowChannels

2014-10-02 Thread rmudgett
a; int b; ... - rmudgett On Oct. 1, 2014, 2:54 p.m., opticron wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4040

[asterisk-dev] [Code Review] 4046: audiohooks: Reevaluate the bridge technology when an audiohook is added or removed.

2014-10-02 Thread rmudgett
with the patch. 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] 4018: res_pjsip: Make transport cipher option accept a comma separated list of cipher names.

2014-10-02 Thread rmudgett
be used with the cipher option. 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

Re: [asterisk-dev] [Code Review] 4035: Dialplan function to get first/head caller channel on queue

2014-10-01 Thread rmudgett
) != 1) { ast_log(...); return -1; } /trunk/apps/app_queue.c https://reviewboard.asterisk.org/r/4035/#comment23953 use ast_copy_string() instead - rmudgett On Oct. 1, 2014, 7:32 a.m., Kristian Høgh wrote

Re: [asterisk-dev] [Code Review] 4018: res_pjsip: Make transport cipher option accept a comma separated list of cipher names.

2014-10-01 Thread rmudgett
, 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] 4018: res_pjsip: Make transport cipher option accept a comma separated list of cipher names.

2014-10-01 Thread rmudgett
-SHA removed. cipher= Blank cipher does not cause a problem. cipher=bad-name Invalid cipher name is rejected and the transport is not created as expected. The new 'pjsip list ciphers' CLI command outputs the available cipher names that can be used with the cipher option. Thanks, rmudgett

[asterisk-dev] [Code Review] 4041: features.c: Fix lingering channel ref while Bridge() application is active.

2014-10-01 Thread rmudgett
, the Surrogate channel no longer shows up when using Bridge on a channel in an application like Wait. It still works if the channel were in a BridgeWait bridge. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided

Re: [asterisk-dev] [Code Review] 4042: repotools: Give up with an error if configure or Makefile.in is newer than Makefile

2014-10-01 Thread rmudgett
/#comment23963 It shouldn't be necessary to run configure to uninstall. - rmudgett On Oct. 1, 2014, 3:59 p.m., Corey Farrell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4042

Re: [asterisk-dev] [Code Review] 4042: repotools: Give up with an error if configure or Makefile.in is newer than Makefile

2014-10-01 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4042/#review13434 --- Ship it! Ship It! - rmudgett On Oct. 1, 2014, 4:14 p.m

Re: [asterisk-dev] [Code Review] 3976: New module: res_pjsip_phoneprov_provider provides the integration between res_pjsip and res_phoneprov.

2014-09-30 Thread rmudgett
/res_pjsip_phoneprov_provider.c https://reviewboard.asterisk.org/r/3976/#comment23903 pp is ref leaked i iterator is not destroyed branches/12/res/res_pjsip_phoneprov_provider.c https://reviewboard.asterisk.org/r/3976/#comment23904 stray blank line - rmudgett On Sept. 18, 2014, 7:55

Re: [asterisk-dev] [Code Review] 3970: res_phoneprov: Refactor phoneprov to allow pluggable config providers.

2014-09-30 Thread rmudgett
/#comment23927 idem branches/12/res/res_phoneprov.c https://reviewboard.asterisk.org/r/3970/#comment23929 idem branches/12/res/res_phoneprov.c https://reviewboard.asterisk.org/r/3970/#comment23930 Check for failure - rmudgett On Sept. 30, 2014, 11:02 a.m., George Joseph wrote

Re: [asterisk-dev] [Code Review] 4034: chan_pjsip: Fix deadlock when masquerading PJSIP channels.

2014-09-30 Thread rmudgett
attended transfer to an appliction. The transfer still works. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http

Re: [asterisk-dev] [Code Review] 3970: res_phoneprov: Refactor phoneprov to allow pluggable config providers.

2014-09-30 Thread rmudgett
and v13 at this late a date especially since v13 is a LTS and currently feature frozen. branches/12/res/res_phoneprov.c https://reviewboard.asterisk.org/r/3970/#comment23939 ; ?? - rmudgett On Sept. 30, 2014, 2:41 p.m., George Joseph wrote

Re: [asterisk-dev] [Code Review] 3976: New module: res_pjsip_phoneprov_provider provides the integration between res_pjsip and res_phoneprov.

2014-09-30 Thread rmudgett
/12/res/res_pjsip_phoneprov_provider.c https://reviewboard.asterisk.org/r/3976/#comment23941 idem - rmudgett On Sept. 30, 2014, 1:03 p.m., George Joseph wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: [asterisk-dev] [Code Review] 4017: chan_pjsip: Don't attempt to apply formats if there aren't any capabilities defined when creating a new channel

2014-09-30 Thread rmudgett
://reviewboard.asterisk.org/r/4017/#comment23943 Why is goto being used here? This should be simply: if not caps empty { set formats } - rmudgett On Sept. 23, 2014, 11:32 a.m., Jonathan Rose wrote: --- This is an automatically generated

Re: [asterisk-dev] [Code Review] 4013: Alembic: Add 'outgoing' enum value to sippeers directmedia enumerator

2014-09-30 Thread rmudgett
add the standard license header to the file. /branches/12/contrib/ast-db-manage/config/versions/10aedae86a32_add_outgoing_enum_va.py https://reviewboard.asterisk.org/r/4013/#comment23944 extraneous blank line - rmudgett On Sept. 23, 2014, 10:41 a.m., Jonathan Rose wrote

Re: [asterisk-dev] [Code Review] 3976: New module: res_pjsip_phoneprov_provider provides the integration between res_pjsip and res_phoneprov.

2014-09-30 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3976/#review13423 --- Ship it! Ship It! - rmudgett On Sept. 30, 2014, 3:43 p.m

Re: [asterisk-dev] [Code Review] 4035: Dialplan function to get first/head caller channel on queue

2014-09-30 Thread rmudgett
) { ... return 0; } var = ast_load_realtime(); if (var) { ... return 0; } ast_log(LOG_WARNING, ...); return 0; - rmudgett On Sept. 30, 2014, 3:04 a.m., Kristian Høgh wrote

Re: [asterisk-dev] [Code Review] 4017: chan_pjsip: Don't attempt to apply formats if there aren't any capabilities defined when creating a new channel

2014-09-30 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4017/#review13426 --- Ship it! Ship It! - rmudgett On Sept. 30, 2014, 4:25 p.m

Re: [asterisk-dev] [Code Review] 4025: pjsip_cli: Suppress header print on error or no objects

2014-09-29 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4025/#review13397 --- Ship it! Ship It! - rmudgett On Sept. 26, 2014, 11:26 a.m

[asterisk-dev] [Code Review] 4034: chan_pjsip: Fix deadlock when masquerading PJSIP channels.

2014-09-29 Thread rmudgett
. 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] 4015: Get rid of most old libc free/malloc/realloc and replace with ast_free and friends.

2014-09-26 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4015/#review13387 --- Ship it! Ship It! - rmudgett On Sept. 26, 2014, 3:18 a.m

Re: [asterisk-dev] [Code Review] 4015: Get rid of most old libc free/malloc/realloc and replace with ast_free and friends.

2014-09-25 Thread rmudgett
with this patch: main/tdd.c The part of this patch that prevents compiling is a breaking change and affects third party modules compiling. Not good. What remains is essentially just a cosmetic change and due to its size should just be done on trunk. - rmudgett On Sept. 22, 2014, 1:55 p.m

Re: [asterisk-dev] [Code Review] 4015: Get rid of most old libc free/malloc/realloc and replace with ast_free and friends.

2014-09-25 Thread rmudgett
for the mp3 code needing the script rerun) /trunk/main/file.c https://reviewboard.asterisk.org/r/4015/#comment23881 Since you removed the redundant if to one ast_free here you might as well do it to the other three. - rmudgett On Sept. 25, 2014, 5 p.m., wdoekes wrote

Re: [asterisk-dev] [Code Review] 3954: pjsip_options.c: Fix race condition stopping periodic out of dialog OPTIONS request.

2014-09-24 Thread rmudgett
would crash. With the new patch, it keeps on going. * Set the qualify_frequency option to different values and reloaded res_pjsip each time. The OPTIONS poll frequency changed, started, and stopped according to the new qualify_frequency value. Thanks, rmudgett

[asterisk-dev] [Code Review] 4018: res_pjsip: Make transport cipher option accept a comma separated list of cipher names.

2014-09-23 Thread rmudgett
cipher name is rejected and the transport is not created as expected. The new 'pjsip list ciphers' CLI command outputs the available cipher names that can be used with the cipher option. Thanks, rmudgett -- _ -- Bandwidth

Re: [asterisk-dev] [Code Review] 4000: res_pjsip_sdp_rtp.c: Fix native formats containing formats that were not negotiated.

2014-09-19 Thread rmudgett
to a Polycom that negotiates ulaw. Before the patch, Asterisk would send g722 frames to the Polycom. The resulting call had one way audio because the Polycom does not understand g722. After the patch, Asterisk sends ulaw frames to the Polycom. Thanks, rmudgett

Re: [asterisk-dev] [Code Review] 3954: pjsip_options.c: Fix race condition stopping periodic out of dialog OPTIONS request.

2014-09-19 Thread rmudgett
qualify_frequency value. 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

Re: [asterisk-dev] [Code Review] 3954: pjsip_options.c: Fix race condition stopping periodic out of dialog OPTIONS request.

2014-09-19 Thread rmudgett
value. 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] 3954: pjsip_options.c: Fix race condition stopping periodic out of dialog OPTIONS request.

2014-09-19 Thread rmudgett
the qualify_frequency option to different values and reloaded res_pjsip each time. The OPTIONS poll frequency changed, started, and stopped according to the new qualify_frequency value. Thanks, rmudgett -- _ -- Bandwidth and Colocation

Re: [asterisk-dev] [Code Review] 3954: pjsip_options.c: Fix race condition stopping periodic out of dialog OPTIONS request.

2014-09-19 Thread rmudgett
to different values and reloaded res_pjsip each time. The OPTIONS poll frequency changed, started, and stopped according to the new qualify_frequency value. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http

[asterisk-dev] [Code Review] 4000: res_pjsip_sdp_rtp.c: Fix native formats containing formats that were not negotiated.

2014-09-18 Thread rmudgett
because the Polycom does not understand g722. After the patch, Asterisk sends ulaw frames to the Polycom. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list

Re: [asterisk-dev] [Code Review] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial

2014-09-18 Thread rmudgett
/r/3990/#comment23822 Allocation failure is not checked here. Though it seems like it might not cause a problem anyway. - rmudgett On Sept. 18, 2014, 1:25 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail

Re: [asterisk-dev] [Code Review] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial

2014-09-18 Thread rmudgett
On Sept. 18, 2014, 3:10 p.m., rmudgett wrote: /branches/12/main/stasis_channels.c, lines 381-394 https://reviewboard.asterisk.org/r/3990/diff/5/?file=67398#file67398line381 Why is the caller channel lock held for the ast_channel_publish_dial_forward() call? A dead

Re: [asterisk-dev] [Code Review] 3970: res_phoneprov: Refactor phoneprov to allow pluggable config providers.

2014-09-18 Thread rmudgett
/3970/#comment23840 Is the cast necessary? branches/12/res/res_phoneprov.c https://reviewboard.asterisk.org/r/3970/#comment23841 privider is ref leaked Use: for (; (provider = ao2_it_next()); ao2_ref(provider, -1)) - rmudgett On Sept. 18, 2014, 3:42 p.m., George Joseph

Re: [asterisk-dev] [Code Review] 4000: res_pjsip_sdp_rtp.c: Fix native formats containing formats that were not negotiated.

2014-09-18 Thread rmudgett
if it is the specified type. I'll leave it as is unless there is a stronger argument for changing it. - rmudgett --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4000/#review13351

Re: [asterisk-dev] [Code Review] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial

2014-09-18 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3990/#review13358 --- Ship it! Ship It! - rmudgett On Sept. 18, 2014, 5:50 p.m

Re: [asterisk-dev] [Code Review] 3976: New module: res_pjsip_phoneprov_provider provides the integration between res_pjsip and res_phoneprov.

2014-09-18 Thread rmudgett
you could use continue instead of goto cleanup. Probably should break up the while loop body into functions. It is rather large. branches/12/res/res_pjsip_phoneprov_provider.c https://reviewboard.asterisk.org/r/3976/#comment23846 Check for failure. - rmudgett On Sept. 18

Re: [asterisk-dev] [Code Review] 3996: Fix zero duration vm description when using MixMonitor m option

2014-09-16 Thread rmudgett
the new ast_ratestream() could be called instead of having it inlined. - rmudgett On Sept. 16, 2014, 9:42 a.m., Scott Griepentrog wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3996

Re: [asterisk-dev] [Code Review] 3971: astobj2.c/refcounter.py: Fix to deal with invalid object refs.

2014-09-16 Thread rmudgett
with an additional log line indicating that the object was destroyed abnormally. The next unref is reported as an unref of an invalid object. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com

Re: [asterisk-dev] [Code Review] 3996: Fix zero duration vm description when using MixMonitor m option

2014-09-15 Thread rmudgett
v13. /branches/13/apps/app_voicemail.c https://reviewboard.asterisk.org/r/3996/#comment23784 guidelines: curly /branches/13/apps/app_voicemail.c https://reviewboard.asterisk.org/r/3996/#comment23785 Capitalize: Unable... - rmudgett On Sept. 15, 2014, 4:37 p.m., Scott

Re: [asterisk-dev] [Code Review] 3971: astobj2.c/refcounter.py: Fix to deal with invalid object refs.

2014-09-15 Thread rmudgett
to do substring comparison again. I'm not seeing the optimization. Are you referring to avoiding the 'invalid' not in parsed_line['state'] comparison? That only happens when the object is destroyed or is invalid. - rmudgett

Re: [asterisk-dev] [Code Review] 3986: config: bug: fix truncation of included config files on permissions error

2014-09-10 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3986/#review13275 --- Ship it! Ship It! - rmudgett On Sept. 9, 2014, 8:34 p.m

Re: [asterisk-dev] [Code Review] 3954: pjsip_options.c: Fix race condition stopping periodic out of dialog OPTIONS request.

2014-09-10 Thread rmudgett
or the other we are going to leak something or the returned error codes need to be mapped to which ones indicate that the callback will happen. - rmudgett --- This is an automatically generated e-mail. To reply, visit: https

Re: [asterisk-dev] [Code Review] 3954: pjsip_options.c: Fix race condition stopping periodic out of dialog OPTIONS request.

2014-09-10 Thread rmudgett
(was this tested with TCP for example?) that the sending of the request may not block and return an error, but it would inevitably end up with a transport error callback. An example: A target using TCP transport that has no existing established connection. rmudgett wrote: I

Re: [asterisk-dev] [Code Review] 3954: pjsip_options.c: Fix race condition stopping periodic out of dialog OPTIONS request.

2014-09-10 Thread rmudgett
. 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] 3983: func_channel: Add CHANNEL(onhold) item to get the current hold status of the channel.

2014-09-09 Thread rmudgett
--- A calls B Issued AMI getvar action on B for CHANNEL(onhold) and got 0. A puts B on hold Issued AMI getvar action on B for CHANNEL(onhold) and got 1. Works as intended. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided

Re: [asterisk-dev] [Code Review] 3986: config: bug: fix truncation of included config files on permissions error

2014-09-09 Thread rmudgett
is necessary. - rmudgett On Sept. 9, 2014, 1:57 p.m., George Joseph wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3986

[asterisk-dev] [Code Review] 3983: func_channel: Add CHANNEL(onhold) item to get the current hold status of the channel.

2014-09-08 Thread rmudgett
. Works as intended. 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

Re: [asterisk-dev] [Code Review] 3982: res_rtp_asterisk: Fix a slew of TURN issues.

2014-09-08 Thread rmudgett
would help when using gdb since it can convert the enum values to their symbolic names. - rmudgett --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3982/#review13261

Re: [asterisk-dev] [Code Review] 3982: res_rtp_asterisk: Fix a slew of TURN issues.

2014-09-08 Thread rmudgett
On Sept. 8, 2014, 1:35 p.m., Matt Jordan wrote: /branches/13/res/res_rtp_asterisk.c, line 850 https://reviewboard.asterisk.org/r/3982/diff/1/?file=67291#file67291line850 Nitpick: no space between (int) and status. rmudgett wrote: I prefer (int) status, over

Re: [asterisk-dev] [Code Review] 3954: pjsip_options.c: Fix race condition stopping periodic out of dialog OPTIONS request.

2014-09-04 Thread rmudgett
for pjsip_endpt_send_request() can safely unref req_data and return an error. Callers of send_out_of_dialog_request() can safely unref/free their data on an error return as well. I'll have to investigate some more. - rmudgett

Re: [asterisk-dev] [Code Review] 3962: CDRs: preserve context/extension when executing a Macro or GoSub

2014-09-04 Thread rmudgett
://reviewboard.asterisk.org/r/3962/#comment23725 unsigned int for single bit bitfields - rmudgett On Sept. 4, 2014, 11:25 a.m., Matt Jordan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3962

Re: [asterisk-dev] [Code Review] 3979: Call IDs: channel Call ID appears as gibberish when shown via CLI command core show channel for a channel that doesn't have call ID set

2014-09-04 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3979/#review13246 --- Ship it! Ship It! - rmudgett On Sept. 4, 2014, 6:04 p.m

Re: [asterisk-dev] [Code Review] 3954: pjsip_options.c: Fix race condition stopping periodic out of dialog OPTIONS request.

2014-09-04 Thread rmudgett
qualify_frequency value. 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] 3954: pjsip_options.c: Fix race condition stopping periodic out of dialog OPTIONS request.

2014-09-03 Thread rmudgett
qualify_frequency value. 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

Re: [asterisk-dev] [Code Review] 3954: pjsip_options.c: Fix race condition stopping periodic out of dialog OPTIONS request.

2014-09-03 Thread rmudgett
and reloaded res_pjsip each time. The OPTIONS poll frequency changed, started, and stopped according to the new qualify_frequency value. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com

Re: [asterisk-dev] [Code Review] 3962: CDRs: preserve context/extension when executing a Macro or GoSub

2014-09-03 Thread rmudgett
The comment seems wrong. - rmudgett On Aug. 29, 2014, 9:33 p.m., Matt Jordan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3962

Re: [asterisk-dev] [Code Review] 3963: testsuite: Add a test that verifies CDRs with a Dial embedded in a subroutine/macro

2014-09-03 Thread rmudgett
-config.yaml https://reviewboard.asterisk.org/r/3963/#comment23704 Need to add dependencies for: app_dial app_echo app_macro app_playback app_stack - rmudgett On Aug. 29, 2014, 9:38 p.m., Matt Jordan wrote

Re: [asterisk-dev] [Code Review] 3964: CDRs: Fix crash caused by infinite generation of new CDR records when a channel enters, leaves, and enters a multi-party bridge

2014-09-03 Thread rmudgett
by pulling the above for loop into its own routine. Then the cdr lock also won't be recursively obtained. - rmudgett On Sept. 1, 2014, 11:51 a.m., Matt Jordan wrote: --- This is an automatically generated e-mail. To reply, visit

Re: [asterisk-dev] [Code Review] 3968: Dial API: Add an option to indicate that a dial is being used to replace the dialing channel from a bridge

2014-09-03 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3968/#review13221 --- Ship it! Ship It! - rmudgett On Sept. 2, 2014, 5:29 p.m

Re: [asterisk-dev] [Code Review] 3964: CDRs: Fix crash caused by infinite generation of new CDR records when a channel enters, leaves, and enters a multi-party bridge

2014-09-03 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3964/#review13222 --- Ship it! Ship It! - rmudgett On Sept. 3, 2014, 3:22 p.m

Re: [asterisk-dev] [Code Review] 3964: CDRs: Fix crash caused by infinite generation of new CDR records when a channel enters, leaves, and enters a multi-party bridge

2014-09-03 Thread rmudgett
On Sept. 3, 2014, 2:08 p.m., rmudgett wrote: /branches/12/main/cdr.c, lines 2466-2469 https://reviewboard.asterisk.org/r/3964/diff/1/?file=67001#file67001line2466 This is a tail recursion that could be eliminated by pulling the above for loop into its own routine. Then the cdr

Re: [asterisk-dev] [Code Review] 3969: Manager: FullyBooted events are sent to AMI users that log in even if they don't have system level read permission.

2014-09-03 Thread rmudgett
/r/3969/#comment23712 You need to have the isolation parens for the bitwise operators. (send_events x) (readperm x) - rmudgett On Sept. 2, 2014, 6:24 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail

Re: [asterisk-dev] [Code Review] 3969: Manager: FullyBooted events are sent to AMI users that log in even if they don't have system level read permission.

2014-09-03 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3969/#review13227 --- Ship it! Ship It! - rmudgett On Sept. 3, 2014, 5:15 p.m

[asterisk-dev] [Code Review] 3971: astobj2.c/refcounter.py: Fix to deal with invalid object refs.

2014-09-02 Thread rmudgett
log that had code unreffing a destroyed object. The patched script now highlights the invalid objects in the log. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev

Re: [asterisk-dev] [Code Review] 3929: ARI: Holding bridge issues with bridge Music on Hold, playback operations, and default channel roles

2014-08-29 Thread rmudgett
. - rmudgett On Aug. 29, 2014, 3:15 p.m., Matt Jordan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3929/ --- (Updated

Re: [asterisk-dev] [Code Review] 3955: confbridge: Add Duration to ConfbridgeList event

2014-08-29 Thread rmudgett
://reviewboard.asterisk.org/r/3955/#comment23694 This header name is misleading. How about AnswerTime instead. - rmudgett On Aug. 28, 2014, 1:06 p.m., George Joseph wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: [asterisk-dev] [Code Review] 3958: manager: Make WaitEvent action respect eventfilters

2014-08-29 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3958/#review13205 --- Ship it! This applies to v1.8+ - rmudgett On Aug. 28, 2014

Re: [asterisk-dev] [Code Review] 3955: confbridge: Add Duration to ConfbridgeList event

2014-08-29 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3955/#review13206 --- Ship it! Ship It! - rmudgett On Aug. 29, 2014, 5:27 p.m

Re: [asterisk-dev] [Code Review] 3929: ARI: Holding bridge issues with bridge Music on Hold, playback operations, and default channel roles

2014-08-29 Thread rmudgett
On Aug. 29, 2014, 3:52 p.m., rmudgett wrote: /branches/12/res/stasis/stasis_bridge.c, line 140 https://reviewboard.asterisk.org/r/3929/diff/2/?file=66975#file66975line140 Optional nit: Extra blank line :) Matt Jordan wrote: Well, that extra blank line was there to prevent

[asterisk-dev] [Code Review] 3954: pjsip_options.c: Fix race condition stopping periodic out of dialog OPTIONS request.

2014-08-28 Thread rmudgett
or was the reason for the reported crash. Set the qualify_frequency option to different values and reloaded res_pjsip each time. The OPTIONS poll frequency changed, started, and stopped according to the new qualify_frequency value. Thanks, rmudgett

Re: [asterisk-dev] [Code Review] 3950: confbridge: Add 'Admin' parameter to join, leave, mute, unmute and talking events

2014-08-27 Thread rmudgett
://reviewboard.asterisk.org/r/3950/#comment23673 silly use of RAII_VAR Just free before returning branches/12/apps/confbridge/confbridge_manager.c https://reviewboard.asterisk.org/r/3950/#comment23674 silly use of RAII_VAR Just free before returning - rmudgett On Aug. 26, 2014, 8:37

Re: [asterisk-dev] [Code Review] 3950: confbridge: Add 'Admin' parameter to join, leave, mute, unmute and talking events

2014-08-27 Thread rmudgett
/app_confbridge.c https://reviewboard.asterisk.org/r/3950/#comment23680 This probably fits on one line now. - rmudgett On Aug. 27, 2014, 11:56 a.m., George Joseph wrote: --- This is an automatically generated e-mail. To reply, visit

Re: [asterisk-dev] [Code Review] 3944: app_confbridge: Add 'all' to channel target for mute and unmute.

2014-08-26 Thread rmudgett
://reviewboard.asterisk.org/r/3944/#comment23645 strncmp/strncasecmp seems just wrong here. Should just be strcasecmp. branches/12/apps/app_confbridge.c https://reviewboard.asterisk.org/r/3944/#comment23646 and here - rmudgett On Aug. 25, 2014, 6:53 p.m., George Joseph wrote

Re: [asterisk-dev] [Code Review] 3923: CallerID: Fix parsing of malformed callerid strings

2014-08-26 Thread rmudgett
://reviewboard.asterisk.org/r/3923/#comment23649 Probably should have a comment stating what case this quote stripping is being done to handle. - rmudgett On Aug. 25, 2014, 8:08 p.m., opticron wrote: --- This is an automatically generated e-mail

Re: [asterisk-dev] [Code Review] 3944: app_confbridge: Add 'all' to channel target for mute and unmute.

2014-08-26 Thread rmudgett
/12/apps/app_confbridge.c https://reviewboard.asterisk.org/r/3944/#comment23657 Line getting long branches/12/apps/app_confbridge.c https://reviewboard.asterisk.org/r/3944/#comment23658 Line getting long - rmudgett On Aug. 26, 2014, 12:35 p.m., George Joseph wrote

Re: [asterisk-dev] [Code Review] 3944: app_confbridge: Add 'all' to channel target for mute and unmute.

2014-08-26 Thread rmudgett
On Aug. 26, 2014, 3:01 p.m., opticron wrote: branches/12/apps/app_confbridge.c, line 230 https://reviewboard.asterisk.org/r/3944/diff/3/?file=66808#file66808line230 Why is this being removed? Because it doesn't do it. - rmudgett

Re: [asterisk-dev] [Code Review] 3933: CallerID: Fix parsing of malformed callerid strings - Asterisk 12+

2014-08-26 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3933/#review13184 --- Ship it! Ship It! - rmudgett On Aug. 25, 2014, 8:59 p.m

Re: [asterisk-dev] [Code Review] 3944: app_confbridge: Add 'all' to channel target for mute and unmute.

2014-08-26 Thread rmudgett
/#comment23663 same here branches/12/apps/app_confbridge.c https://reviewboard.asterisk.org/r/3944/#comment23660 curly branches/12/apps/app_confbridge.c https://reviewboard.asterisk.org/r/3944/#comment23661 curly - rmudgett On Aug. 26, 2014, 4:26 p.m., George Joseph wrote

<    1   2   3   4   5   6   7   8   9   >