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

2014-11-13 Thread Joshua Colp
On Nov. 5, 2014, 12:49 p.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] 4175: Fix race condition where identical SIP requests are processed by multiple threads (Asterisk 13)

2014-11-13 Thread Joshua Colp
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4175/#review13740 --- Ship it! /branches/13/main/features.c

Re: [asterisk-dev] [Code Review] 4163: Stasis: Fix StasisEnd message ordering

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

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

2014-11-13 Thread Joshua Colp
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4174/#review13741 --- Ship it! Ship It! /branches/12/res/res_pjsip_pubsub.c

Re: [asterisk-dev] [Code Review] 4170: testsuite: Delete bridges on completion for a bunch of rest_api tests

2014-11-13 Thread Joshua Colp
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4170/#review13743 --- Ship it! Ship It! - Joshua Colp On Nov. 11, 2014, 10:41

Re: [asterisk-dev] [Code Review] 4160: chan_sip: Fix theoretical leak of p-refer

2014-11-13 Thread Mark Michelson
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4160/#review13744 --- Ship it! Ship It! - Mark Michelson On Nov. 12, 2014, 8:28

Re: [asterisk-dev] [Code Review] 4166: testsuite: tests/bridge/bridge_action leaves a channel open

2014-11-13 Thread Mark Michelson
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4166/#review13745 --- With the fix being made to the leaked bridge in Asterisk, is

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

2014-11-13 Thread Joshua Colp
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4167/#review13746 --- Ship it! Ship It! - Joshua Colp On Nov. 12, 2014, 6:32

Re: [asterisk-dev] [Code Review] 4155: PJSIP: Allow contact rewriting to fall back for reliable transports

2014-11-13 Thread opticron
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4155/ --- (Updated Nov. 13, 2014, 10:53 a.m.) Review request for Asterisk

[asterisk-dev] session-refresher ignored if UAC supports but does not request timers.

2014-11-13 Thread Dave WOOLLEY
The same rule in the RFC covers the case where Session-Expires is sent, but no refresher is sent as covers the case where no Session-Expires is sent at all, as long as session timers are supported: UAC supports? refresher parameter refresher parameter in

Re: [asterisk-dev] [Code Review] 4155: PJSIP: Allow contact rewriting to fall back for reliable transports

2014-11-13 Thread Joshua Colp
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4155/#review13747 --- branches/12/res/res_pjsip_nat.c

[asterisk-dev] Adding Required header to OK response will fail.

2014-11-13 Thread Dave WOOLLEY
I don't believe that the code that adds Required: timers to a 200 OK response will work, even in Asterisk 13, current branch version. In my back port, it produces an error saying headers cannot be added after lines have been added. The same conditions for this seem to apply in version 13: In

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

2014-11-13 Thread Joshua Colp
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4177/ --- Review request for Asterisk Developers. Repository: Asterisk

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

2014-11-13 Thread Mark Michelson
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4177/#review13749 --- Ship it! Looks good to me. The only suggestion I have is to

Re: [asterisk-dev] Adding Required header to OK response will fail.

2014-11-13 Thread Mark Michelson
On 11/13/2014 12:34 PM, Dave WOOLLEY wrote: I don’t believe that the code that adds Required: timers to a 200 OK response will work, even in Asterisk 13, current branch version. In my back port, it produces an error saying headers cannot be added after lines have been added. The same

[asterisk-dev] [Code Review] 4178: res_pjsip_outbound_publish: stack overflow when using non-default sorcery wizard

2014-11-13 Thread Kevin Harwell
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4178/ --- Review request for Asterisk Developers. Bugs: ASTERISK-24514

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

2014-11-13 Thread Scott Griepentrog
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4182/ --- Review request for Asterisk Developers. Repository: Asterisk

Re: [asterisk-dev] [Code Review] 4170: testsuite: Delete bridges on completion for a bunch of rest_api tests

2014-11-13 Thread Corey Farrell
On Nov. 13, 2014, 11:06 a.m., Joshua Colp wrote: Ship It! I have to look into this again now that kmoore has committed testsuite revision 5921, many of these changes use the StasisEnd event to do cleanup. - Corey --- This is an

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

2014-11-13 Thread Corey Farrell
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4182/#review13751 --- Could this be solved by: read(ast_consock, buf, sizeof(buf) -

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

2014-11-13 Thread Mark Michelson
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4182/#review13752 --- I typically take issue with patches like this because it's not

Re: [asterisk-dev] [Code Review] 4164: res_pjsip_outbound_registration: stack overflow when using non-default sorcery wizard

2014-11-13 Thread Kevin Harwell
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4164/ --- (Updated Nov. 13, 2014, 3:56 p.m.) Status -- This change has been

Re: [asterisk-dev] [Code Review] 4166: testsuite: tests/bridge/bridge_action leaves a channel open

2014-11-13 Thread Corey Farrell
On Nov. 13, 2014, 11:30 a.m., Mark Michelson wrote: With the fix being made to the leaked bridge in Asterisk, is this change still required? Does hanging up self.channels[1] not result in self.channels[3] and the bridge being destroyed as expected? Still required, I'm guessing that

Re: [asterisk-dev] [Code Review] 4166: testsuite: tests/bridge/bridge_action leaves a channel open

2014-11-13 Thread Corey Farrell
On Nov. 13, 2014, 11:30 a.m., Mark Michelson wrote: With the fix being made to the leaked bridge in Asterisk, is this change still required? Does hanging up self.channels[1] not result in self.channels[3] and the bridge being destroyed as expected? Corey Farrell wrote: Still

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

2014-11-13 Thread Matt Jordan
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4109/#review13756 --- Ship it! Ship It! - Matt Jordan On Oct. 22, 2014, 5:44

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

2014-11-13 Thread Kevin Harwell
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4168/#review13755 ---

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

2014-11-13 Thread Matt Jordan
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4135/#review13759 --- Ship it! Ship It! - Matt Jordan On Nov. 7, 2014, 12:31

Re: [asterisk-dev] [Code Review] 4166: testsuite: tests/bridge/bridge_action leaves a channel open

2014-11-13 Thread Mark Michelson
On Nov. 13, 2014, 4:30 p.m., Mark Michelson wrote: With the fix being made to the leaked bridge in Asterisk, is this change still required? Does hanging up self.channels[1] not result in self.channels[3] and the bridge being destroyed as expected? Corey Farrell wrote: Still

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

2014-11-13 Thread Kevin Harwell
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4156/#review13757 --- /asterisk/trunk/lib/python/asterisk/pcap.py

Re: [asterisk-dev] [Code Review] 4101: Channel Originate/Continue via ARI support for labels in dialplan is incomplete

2014-11-13 Thread Matt Jordan
On Nov. 5, 2014, 11:43 a.m., Joshua Colp wrote: Matt brought it up that this is actually a backwards incompatible change - specifically changing priority into a string from an integer. What about having label as a separate argument that is optional? If present it's treated as a label

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

2014-11-13 Thread George Joseph
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4183/ --- Review request for Asterisk Developers. Bugs: ASTERISK-24520

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

2014-11-13 Thread George Joseph
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4183/ --- (Updated Nov. 13, 2014, 5:34 p.m.) Review request for Asterisk

Re: [asterisk-dev] [Code Review] 4165: res_pjsip_config_wizard: Allow streamlined configuration of common pjsip scenarios.

2014-11-13 Thread George Joseph
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4165/ --- (Updated Nov. 13, 2014, 5:35 p.m.) Status -- This change has been