Re: [asterisk-dev] [Code Review] 4177: app_confbridge: Play 'leader has left' sound file even when musiconhold is enabled

2014-11-14 Thread Joshua Colp
On Nov. 13, 2014, 7:30 p.m., Mark Michelson wrote: Looks good to me. The only suggestion I have is to modify the tests/apps/confbridge/confbridge_marked_unmarked test in the testsuite to expect the conf-hasleft sound to be played back in scenarios 2 and 3 to the Normal-user channel.

Re: [asterisk-dev] [Code Review] 4175: Fix race condition where identical SIP requests are processed by multiple threads (Asterisk 13)

2014-11-14 Thread Mark Michelson
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4175/ --- (Updated Nov. 14, 2014, 8:22 a.m.) Status -- This change has been

Re: [asterisk-dev] [Code Review] 4093: Codec Format Is Not Included in the SDP Media Attributes When SLIN48 Codec Is Used

2014-11-14 Thread Matt Jordan
On Nov. 5, 2014, 6:49 a.m., Joshua Colp wrote: /tags/12.4.0/main/rtp_engine.c, lines 2012-2018 https://reviewboard.asterisk.org/r/4093/diff/1/?file=68394#file68394line2012 This is not compliant to the way L16 is supposed to be declared within SDP. The payload name is supposed to

Re: [asterisk-dev] [Code Review] 4174: Fix race condition where identical SIP requests are processed by multiple threads.

2014-11-14 Thread Mark Michelson
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4174/ --- (Updated Nov. 14, 2014, 8:24 a.m.) Status -- This change has been

Re: [asterisk-dev] [Code Review] 4177: app_confbridge: Play 'leader has left' sound file even when musiconhold is enabled

2014-11-14 Thread Joshua Colp
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4177/ --- (Updated Nov. 14, 2014, 8:54 a.m.) Status -- This change has been

Re: [asterisk-dev] [Code Review] 4135: Resolve race condition that can result in ARI apps not being notified of transfers

2014-11-14 Thread Mark Michelson
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4135/ --- (Updated Nov. 14, 2014, 9 a.m.) Status -- This change has been

Re: [asterisk-dev] [Code Review] 4167: Allow mis-dialed DTMF-initiated transfers to be re-attempted.

2014-11-14 Thread Matt Jordan
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4167/#review13764 --- Make sure you update CHANGES with the new feature as well.

Re: [asterisk-dev] [Code Review] 4182: core: avoid rasterisk crash due to long identifier

2014-11-14 Thread Scott Griepentrog
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4182/ --- (Updated Nov. 14, 2014, 9:12 a.m.) Review request for Asterisk

Re: [asterisk-dev] [Code Review] 4183: res_pjsip_phoneprov_provider: bug: call ast_sorcery_apply_config so config can come from realtime.

2014-11-14 Thread Joshua Colp
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4183/#review13766 --- branches/12/res/res_pjsip_phoneprov_provider.c

Re: [asterisk-dev] [Code Review] 4167: Allow mis-dialed DTMF-initiated transfers to be re-attempted.

2014-11-14 Thread Mark Michelson
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4167/#review13767 --- This review has been shipped. I wrote the feature against

Re: [asterisk-dev] [Code Review] 4168: Testsuite: transferdialattempts test

2014-11-14 Thread Mark Michelson
On Nov. 13, 2014, 11:07 p.m., Kevin Harwell wrote: /asterisk/trunk/tests/bridge/atxfer_retries/configs/ast1/extensions.conf, lines 18-40 https://reviewboard.asterisk.org/r/4168/diff/1/?file=68858#file68858line18 Are these extensions required by the test? This is a template, so

Re: [asterisk-dev] [Code Review] 4168: Testsuite: transferdialattempts test

2014-11-14 Thread Mark Michelson
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4168/ --- (Updated Nov. 14, 2014, 3:35 p.m.) Review request for Asterisk

Re: [asterisk-dev] [Code Review] 4168: Testsuite: transferdialattempts test

2014-11-14 Thread Kevin Harwell
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4168/#review13768 --- The new diff is missing the test files. - Kevin Harwell On

Re: [asterisk-dev] [Code Review] 4139: stun: fix size of attribute string to match rfc

2014-11-14 Thread Scott Griepentrog
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4139/ --- (Updated Nov. 14, 2014, 9:48 a.m.) Status -- This change has been

Re: [asterisk-dev] [Code Review] 4183: res_pjsip_phoneprov_provider: bug: call ast_sorcery_apply_config so config can come from realtime.

2014-11-14 Thread George Joseph
On Nov. 14, 2014, 8:12 a.m., Joshua Colp wrote: branches/12/res/res_pjsip_phoneprov_provider.c, line 380 https://reviewboard.asterisk.org/r/4183/diff/1/?file=68985#file68985line380 Is this really needed? ast_sorcery_open calls __ast_sorcery_open with the module name. This then

Re: [asterisk-dev] [Code Review] 4183: res_pjsip_phoneprov_provider: bug: call ast_sorcery_apply_config so config can come from realtime.

2014-11-14 Thread George Joseph
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4183/ --- (Updated Nov. 14, 2014, 8:56 a.m.) Status -- This change has been

Re: [asterisk-dev] [Code Review] 4156: Test Suite: Allow setting snaplen buffer_size for packet capturing

2014-11-14 Thread jbigelow
On Nov. 13, 2014, 5:17 p.m., Kevin Harwell wrote: /asterisk/trunk/lib/python/pcap_listener.py, lines 61-64 https://reviewboard.asterisk.org/r/4156/diff/1/?file=68770#file68770line61 Instead of checking for None here and then setting the value just set these as the defaults on the

Re: [asterisk-dev] [Code Review] 4109: Documentation: CDR unanswered behavior

2014-11-14 Thread Jonathan Rose
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4109/ --- (Updated Nov. 14, 2014, 11:42 a.m.) Status -- This change has been

Re: [asterisk-dev] [Code Review] 4156: Test Suite: Allow setting snaplen buffer_size for packet capturing

2014-11-14 Thread Kevin Harwell
On Nov. 13, 2014, 5:17 p.m., Kevin Harwell wrote: /asterisk/trunk/lib/python/pcap_listener.py, lines 61-64 https://reviewboard.asterisk.org/r/4156/diff/1/?file=68770#file68770line61 Instead of checking for None here and then setting the value just set these as the defaults on the

Re: [asterisk-dev] [Code Review] 4156: Test Suite: Allow setting snaplen buffer_size for packet capturing

2014-11-14 Thread Kevin Harwell
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4156/#review13773 --- Ship it! Ship It! - Kevin Harwell On Nov. 6, 2014, 5:05

[asterisk-dev] [Code Review] 4184: app_confbridge: Don't delay playing 'you have been kicked' prompts to end_marked users when there are only end_marked users in the conference

2014-11-14 Thread Matt Jordan
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4184/ --- Review request for Asterisk Developers. Bugs: ASTERISK-24522

Re: [asterisk-dev] [Code Review] 4167: Allow mis-dialed DTMF-initiated transfers to be re-attempted.

2014-11-14 Thread Matt Jordan
On Nov. 14, 2014, 9:30 a.m., Mark Michelson wrote: This review has been shipped. I wrote the feature against trunk, but I'm curious if 13's proposed policy would allow for such a change to be added to it as well. Thoughts? It would, and I think this should be fine for 13. It's a

Re: [asterisk-dev] [Code Review] 4184: app_confbridge: Don't delay playing 'you have been kicked' prompts to end_marked users when there are only end_marked users in the conference

2014-11-14 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4184/#review13779 --- Ship it! This simple patch works fine for the one case since

Re: [asterisk-dev] [Code Review] 4182: core: avoid rasterisk crash due to long identifier

2014-11-14 Thread Corey Farrell
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4182/#review13777 --- /branches/13/main/asterisk.c

Re: [asterisk-dev] [Code Review] 4182: core: avoid rasterisk crash due to long identifier

2014-11-14 Thread Scott Griepentrog
On Nov. 14, 2014, 4:08 p.m., Corey Farrell wrote: /branches/13/main/asterisk.c, line 3203 https://reviewboard.asterisk.org/r/4182/diff/2/?file=68987#file68987line3203 Does this actually initialize 256 bytes of '\0', or just initialize the first byte? Initializing a char array

Re: [asterisk-dev] [Code Review] 4182: core: avoid rasterisk crash due to long identifier

2014-11-14 Thread Scott Griepentrog
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4182/ --- (Updated Nov. 14, 2014, 5:03 p.m.) Review request for Asterisk

[asterisk-dev] [Code Review] 4185: sorcery: Make ast_sorcery_is_object_field_registered handle field names that are regexes.

2014-11-14 Thread George Joseph
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4185/ --- Review request for Asterisk Developers, Joshua Colp and Mark Michelson.