[asterisk-dev] TLS architecture chan_sip

2014-04-10 Thread Olle E. Johansson
Hi! In the TLS architecture in chan_sip, are we assuming that Asterisk can operate without any server certs? In theory we should be able to operate as a TLS client, but I can not figure out if we are meant to do that or not. What is your opinion? /O --

Re: [asterisk-dev] [Code Review] 3428: Testsuite: ARI Playback Tones tests for channels and bridges

2014-04-10 Thread Matt Jordan
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3428/#review11540 ---

Re: [asterisk-dev] [Code Review] 3430: [channels/chan_unistim.c]: Improvements and bugfixes to chan_unistim.c

2014-04-10 Thread Matt Jordan
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3430/#review11541 --- The vast majority of these findings are simply Coding

Re: [asterisk-dev] [Code Review] 3349: Implement RFC-3966 TEL URI incoming INVITE

2014-04-10 Thread wdoekes
On March 22, 2014, 3:39 p.m., Olle E Johansson wrote: I don't see what happens with the phone-context argument. Shouldn't we pass that on as a channel variable? Geert Van Pamel wrote: We return this into the hostport. Geert Van Pamel wrote: According to RFC 3966

Re: [asterisk-dev] [Code Review] 3429: monitor: use app options parsing helper code

2014-04-10 Thread Joshua Colp
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3429/#review11543 --- Ship it! Ship It! - Joshua Colp On April 9, 2014, 12:46

Re: [asterisk-dev] [Code Review] 3427: ARI: Add tones playback resource

2014-04-10 Thread Joshua Colp
On April 9, 2014, 4:31 p.m., David Lee wrote: /trunk/CHANGES, line 125 https://reviewboard.asterisk.org/r/3427/diff/2/?file=57139#file57139line125 How hard would it be to add something like ?timeMillis= to the URI for tones? Or are tones usually something like busy, where you

Re: [asterisk-dev] [Code Review] 3427: ARI: Add tones playback resource

2014-04-10 Thread Joshua Colp
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3427/#review11545 --- Hrm - I'm not sure I like it being tones. When talking about

Re: [asterisk-dev] [Code Review] 3427: ARI: Add tones playback resource

2014-04-10 Thread Matt Jordan
On April 9, 2014, 11:31 a.m., David Lee wrote: /trunk/CHANGES, line 125 https://reviewboard.asterisk.org/r/3427/diff/2/?file=57139#file57139line125 How hard would it be to add something like ?timeMillis= to the URI for tones? Or are tones usually something like busy, where you

Re: [asterisk-dev] [Code Review] 3427: ARI: Add tones playback resource

2014-04-10 Thread Matt Jordan
On April 10, 2014, 9:19 a.m., Joshua Colp wrote: Hrm - I'm not sure I like it being tones. When talking about these you don't refer to them as plural. Busy tone for an example. Thoughts? tone works for me. - Matt --- This is an

Re: [asterisk-dev] [Code Review] 3357: testsuite: Add off-nominal subscription tests for PJSIP.

2014-04-10 Thread opticron
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3357/#review11548 --- Ship it! Ship It! - opticron On March 27, 2014, 3:43 p.m.,

Re: [asterisk-dev] [Code Review] 3419: Test for PJSIP fast picture update

2014-04-10 Thread opticron
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3419/#review11549 ---

Re: [asterisk-dev] [Code Review] 3427: ARI: Add tones playback resource

2014-04-10 Thread Jonathan Rose
On April 9, 2014, 11:31 a.m., David Lee wrote: /trunk/CHANGES, line 125 https://reviewboard.asterisk.org/r/3427/diff/2/?file=57139#file57139line125 How hard would it be to add something like ?timeMillis= to the URI for tones? Or are tones usually something like busy, where you

Re: [asterisk-dev] [Code Review] 3427: ARI: Add tones playback resource

2014-04-10 Thread Jonathan Rose
On April 10, 2014, 9:19 a.m., Joshua Colp wrote: Hrm - I'm not sure I like it being tones. When talking about these you don't refer to them as plural. Busy tone for an example. Thoughts? Matt Jordan wrote: tone works for me. tone will be fine. I just went by the description on the

Re: [asterisk-dev] [Code Review] 3427: ARI: Add tones playback resource

2014-04-10 Thread Jonathan Rose
On April 9, 2014, 4:20 p.m., Matt Jordan wrote: /trunk/main/app.c, lines 1056-1060 https://reviewboard.asterisk.org/r/3427/diff/2/?file=57141#file57141line1056 This would be an appropriate place for a ternary operator: ast_playtones_start(chan, 0, ts ? ts-data : tone,

Re: [asterisk-dev] [Code Review] 3419: Test for PJSIP fast picture update

2014-04-10 Thread Benjamin Keith Ford
On April 10, 2014, 2:56 p.m., opticron wrote: ./asterisk/trunk/tests/channels/pjsip/video_calls/fast_picture_update/sipp/start-call.xml, line 58 https://reviewboard.asterisk.org/r/3419/diff/1/?file=57067#file57067line58 This needs to send a response 200 OK back and should also

Re: [asterisk-dev] [Code Review] 3379: ARI: Make bridges/{bridgeId}/play queue sound files if sounds are already playing on the bridge instead of playing them simultaneously as they are called

2014-04-10 Thread Mark Michelson
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3379/#review11552 --- /branches/12/res/ari/resource_bridges.h

Re: [asterisk-dev] [Code Review] 3433: bridge_unreal: An alternative implementation for optimizing Unreal/Local channels.

2014-04-10 Thread Mark Michelson
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3433/#review11554 --- Well, this is really cool. I love reviews that mostly remove

Re: [asterisk-dev] [Code Review] 3357: testsuite: Add off-nominal subscription tests for PJSIP.

2014-04-10 Thread Mark Michelson
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3357/#review11555 --- Ship it! Ship It! - Mark Michelson On March 27, 2014, 8:43

Re: [asterisk-dev] [Code Review] 3349: Implement RFC-3966 TEL URI incoming INVITE

2014-04-10 Thread Matt Jordan
On March 22, 2014, 10:39 a.m., Olle E Johansson wrote: I don't see what happens with the phone-context argument. Shouldn't we pass that on as a channel variable? Geert Van Pamel wrote: We return this into the hostport. Geert Van Pamel wrote: According to RFC 3966

Re: [asterisk-dev] [Code Review] 3377: ref count logs: Redo structure of log file, provide a python debugging tool

2014-04-10 Thread Mark Michelson
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3377/#review11558 --- Ship it! /branches/1.8/include/asterisk/astobj2.h

Re: [asterisk-dev] [Code Review] 3420: Testsuite: Call Files' Max Retries

2014-04-10 Thread opticron
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3420/#review11556 ---

Re: [asterisk-dev] [Code Review] 3349: Implement RFC-3966 TEL URI incoming INVITE

2014-04-10 Thread wdoekes
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3349/#review11559 --- Ship it! Either way, the variable will go in with this

Re: [asterisk-dev] [Code Review] 3422: testsuite: Fix Asterisk shutdown timeout in chan_sip session_timer tests by hanging up channels

2014-04-10 Thread opticron
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3422/#review11560 --- Ship it! Ship It! - opticron On April 5, 2014, 7:38 p.m.,

Re: [asterisk-dev] [Code Review] 3424: mixmonitor: Add option to enable a periodic beep

2014-04-10 Thread Mark Michelson
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3424/#review11562 --- /trunk/apps/app_mixmonitor.c

Re: [asterisk-dev] [Code Review] 3419: Test for PJSIP fast picture update

2014-04-10 Thread opticron
On April 10, 2014, 9:56 a.m., opticron wrote: ./asterisk/trunk/tests/channels/pjsip/video_calls/fast_picture_update/sipp/start-call.xml, line 58 https://reviewboard.asterisk.org/r/3419/diff/1/?file=57067#file57067line58 This needs to send a response 200 OK back and should also

Re: [asterisk-dev] [Code Review] 3403: Test for channel Pickup

2014-04-10 Thread Mark Michelson
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3403/#review11563 --- Ship it! Ship It! - Mark Michelson On April 9, 2014, 2:55

Re: [asterisk-dev] [Code Review] 3417: Add AMI events for all device state and presence state changes

2014-04-10 Thread Mark Michelson
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3417/ --- (Updated April 10, 2014, 8:10 p.m.) Status -- This change has been

Re: [asterisk-dev] [Code Review] 3407: Test Suite: Nominal caller initiated blind transfer tests using PJSIP

2014-04-10 Thread Mark Michelson
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3407/#review11565 --- Ship it! Ship It! - Mark Michelson On April 4, 2014, 10:44

Re: [asterisk-dev] [Code Review] 3339: Testsuite: ARI test for playback of audio to a channel in a bridge.

2014-04-10 Thread Mark Michelson
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3339/ --- (Updated April 10, 2014, 8:13 p.m.) Status -- This change has been

Re: [asterisk-dev] [Code Review] 3424: mixmonitor: Add option to enable a periodic beep

2014-04-10 Thread Corey Farrell
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3424/#review11564 --- /trunk/apps/app_mixmonitor.c

Re: [asterisk-dev] [Code Review] 3417: Add AMI events for all device state and presence state changes

2014-04-10 Thread Mark Michelson
On April 5, 2014, 7:02 a.m., Olle E Johansson wrote: I would like to see a configuration option for this, as it will generate a massive amount of events in busy servers. Mark Michelson wrote: That's fair. I can think of two ways to do this: 1) The DeviceStateChange and

[asterisk-dev] [Code Review] 3434: libpri: Make TE-PTP mode respond to MDL TEI check requests.

2014-04-10 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3434/ --- Review request for Asterisk Developers. Bugs: PRI-165

Re: [asterisk-dev] [Code Review] 3427: ARI: Add tones playback resource

2014-04-10 Thread Jonathan Rose
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3427/ --- (Updated April 10, 2014, 3:55 p.m.) Review request for Asterisk

Re: [asterisk-dev] [Code Review] 3419: Test for PJSIP fast picture update

2014-04-10 Thread Benjamin Keith Ford
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3419/ --- (Updated April 10, 2014, 9:21 p.m.) Review request for Asterisk

Re: [asterisk-dev] [Code Review] 3379: ARI: Make bridges/{bridgeId}/play queue sound files if sounds are already playing on the bridge instead of playing them simultaneously as they are called

2014-04-10 Thread Jonathan Rose
On April 10, 2014, 1:44 p.m., Mark Michelson wrote: /branches/12/res/ari/resource_bridges.h, lines 282-292 https://reviewboard.asterisk.org/r/3379/diff/5/?file=56855#file56855line282 There's no need to put \brief for one-line doxygen comments. Doxygen automatically treats these

Re: [asterisk-dev] [Code Review] 3379: ARI: Make bridges/{bridgeId}/play queue sound files if sounds are already playing on the bridge instead of playing them simultaneously as they are called

2014-04-10 Thread Jonathan Rose
On April 10, 2014, 1:44 p.m., Mark Michelson wrote: /branches/12/res/ari/resource_bridges.c, lines 378-384 https://reviewboard.asterisk.org/r/3379/diff/5/?file=56856#file56856line378 The pointer check for playback_url here is incorrect. According to the man page for asprintf:

Re: [asterisk-dev] [Code Review] 3417: Add AMI events for all device state and presence state changes

2014-04-10 Thread Corey Farrell
On April 5, 2014, 3:02 a.m., Olle E Johansson wrote: I would like to see a configuration option for this, as it will generate a massive amount of events in busy servers. Mark Michelson wrote: That's fair. I can think of two ways to do this: 1) The DeviceStateChange and

Re: [asterisk-dev] [Code Review] 3379: ARI: Make bridges/{bridgeId}/play queue sound files if sounds are already playing on the bridge instead of playing them simultaneously as they are called

2014-04-10 Thread Jonathan Rose
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3379/ --- (Updated April 10, 2014, 4:59 p.m.) Review request for Asterisk

Re: [asterisk-dev] [Code Review] 3415: bridging: Ensure proper locking during snapshot creation

2014-04-10 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3415/#review11570 --- Ship it! Only a minor technicality nit.

Re: [asterisk-dev] [Code Review] 3433: bridge_unreal: An alternative implementation for optimizing Unreal/Local channels.

2014-04-10 Thread rmudgett
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3433/#review11572 --- I'm not seeing any protection from loss of frames when the

Re: [asterisk-dev] [Code Review] 3433: bridge_unreal: An alternative implementation for optimizing Unreal/Local channels.

2014-04-10 Thread Joshua Colp
On April 10, 2014, 6:52 p.m., Mark Michelson wrote: Well, this is really cool. I love reviews that mostly remove code in order to simplify something. Some concerns: 1) Documentation wise, there is a mixture of nomenclature used for the subchannels of a local channel. You have

Re: [asterisk-dev] [Code Review] 3433: bridge_unreal: An alternative implementation for optimizing Unreal/Local channels.

2014-04-10 Thread Joshua Colp
On April 10, 2014, 11:20 p.m., rmudgett wrote: I'm not seeing any protection from loss of frames when the channels optimize out. Losing media frames isn't nice but is tollerable. Losing control frames is unacceptable. 1. Can you explain the scenario and how I would lose frames. 2.

Re: [asterisk-dev] [Code Review] 3429: monitor: use app options parsing helper code

2014-04-10 Thread Russell Bryant
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3429/ --- (Updated April 10, 2014, 8:13 p.m.) Status -- This change has been

Re: [asterisk-dev] [Code Review] 3377: ref count logs: Redo structure of log file, provide a python debugging tool

2014-04-10 Thread Matt Jordan
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3377/ --- (Updated April 10, 2014, 8:33 p.m.) Status -- This change has been

Re: [asterisk-dev] [Code Review] 3424: mixmonitor: Add option to enable a periodic beep

2014-04-10 Thread Russell Bryant
On April 10, 2014, 4:21 p.m., Corey Farrell wrote: /trunk/apps/app_mixmonitor.c, line 1070 https://reviewboard.asterisk.org/r/3424/diff/2/?file=57155#file57155line1070 Mark's suggestion to use ast_custom_function_find made me think about module references. We don't keep a