Re: [asterisk-dev] [Code Review] 3817: res_pjsip_notify: Add the ability for PJSIPNotify AMI command and pjsip send notify CLI command to send to a URI instead of an endpoint
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3817/#review12901 --- Ship it! Ship It! - Joshua Colp On July 24, 2014, 3:58 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3817/ --- (Updated July 24, 2014, 3:58 p.m.) Review request for Asterisk Developers, Matt Jordan and Mark Michelson. Repository: Asterisk Description --- Example: Action: PJSIPNotify URI: sip:10.10.10.10 pjsip send notify uri type uri uri [uri...] Adds the ability to use URIs with the PJSIP notify commands instead of endpoints. These mostly work the same as the endpoint notifications. Diffs - /trunk/res/res_pjsip_notify.c 419109 Diff: https://reviewboard.asterisk.org/r/3817/diff/ Testing --- Used existing endpoint notification and compared it to similar notifies done against URIs. The outgoing messages looked the same as far as the details I was concerned with went. 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] 3854: manager.c - Improve documentation for manager command Getvar, Setvar
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3854/#review12902 --- /branches/1.8/main/manager.c https://reviewboard.asterisk.org/r/3854/#comment23275 This confuses me, even after reading a few times. Specifically what you've added: directly or via function. Dialplan functions usually don't set variables. Set a channel variable or execute a dialplan function. is clearer Same applies for the other modifications. Thoughts? - Joshua Colp On July 25, 2014, 2:20 p.m., rnewton wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3854/ --- (Updated July 25, 2014, 2:20 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-21178 https://issues.asterisk.org/jira/browse/ASTERISK-21178 Repository: Asterisk Description --- The documentation wasn't clear that AMI Getvar and Setvar could accept function calls. This is a slight modification to improve clarity. Diffs - /branches/1.8/main/manager.c 419562 Diff: https://reviewboard.asterisk.org/r/3854/diff/ Testing --- Once finalized I'll build in dev-mode with it to make sure I didn't screw up any tags. Thanks, rnewton -- _ -- 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] 3780: res_pjsip_outbound_publish / res_pjsip_publish_asterisk: Add outbound PUBLISH support with 'asterisk' event type.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3780/ --- (Updated July 29, 2014, 2:40 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- This adds two PJSIP modules which add outbound PUBLISH support and an 'asterisk' event type. The res_pjsip_outbound_publish module is a common module which provides basic logic for setting up outbound PUBLISH clients, handling authentication requests, handling configuration, and lifetime. Extra modules implement specific event types which are registered with res_pjsip_outbound_publish. Since it takes care of configuration when an outbound PUBLISH is configured extra configuration can be passed to the event type implementation to further configure itself. The res_pjsip_publish_asterisk module implements inbound and outbound support for an 'asterisk' event type. This event type conveys device and mailbox state between Asterisk instances using a JSON content body. As internal device or mailbox state changes the module sends a PUBLISH message to other configured instances. When a PUBLISH is received the contents are examined and a device or mailbox state change queued up within Asterisk. To restrict what is sent and received filtering is available using regular expressions which can reduce SIP traffic. A wiki page is available at https://wiki.asterisk.org/wiki/display/~jcolp/Exchanging+Device+and+Mailbox+State+Using+PJSIP which has some configuration details with some examples. This should also be reviewed. Diffs (updated) - /trunk/res/res_pjsip_pubsub.exports.in 419753 /trunk/res/res_pjsip_pubsub.c 419753 /trunk/res/res_pjsip_publish_asterisk.c PRE-CREATION /trunk/res/res_pjsip_outbound_publish.exports.in PRE-CREATION /trunk/res/res_pjsip_outbound_publish.c PRE-CREATION /trunk/include/asterisk/res_pjsip_pubsub.h 419753 /trunk/include/asterisk/res_pjsip_outbound_publish.h PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3780/diff/ Testing --- Set up two Asterisk instances, configured both sides to publish to eachother, made calls and manipulated voicemail. Watched PUBLISH messages go between them and state change. 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] 3856: Fix Dynamic IAX2 Registrations From Failing Or Not Retrying After Temporary DNS Failure
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3856/#review12904 --- /branches/11/channels/chan_iax2.c https://reviewboard.asterisk.org/r/3856/#comment23277 Fixed sizes always bite us. I'd opt for making this zero sized and allocate the needed room to store the hostname with the reg structure itself. /branches/11/channels/chan_iax2.c https://reviewboard.asterisk.org/r/3856/#comment23278 While here we may as well sscanf this. - Joshua Colp On July 25, 2014, 2:47 p.m., Michael Young wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3856/ --- (Updated July 25, 2014, 2:47 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-23767 https://issues.asterisk.org/jira/browse/ASTERISK-23767 Repository: Asterisk Description --- The reporter on the issue found some issues when upgrading from version 10 to 11 on 55 hosts. Two situations that can occur with dynamic registrations. 1. With dnsmgr disabled, if the host is not resolvable we are not trying to resolve the host again when it is time to attempt to register again. 2. With dnsmgr enabled, when the host is temporarily not resolvable the address is set to 0.0.0.0:0 and then when the host is resolvable the port is not being restored and stays set to 0. This patch resolves these two issues. Diffs - /branches/11/channels/chan_iax2.c 419562 Diff: https://reviewboard.asterisk.org/r/3856/diff/ Testing --- This was tested on a test machine. The reporter of the issue also tested this and has confirmed that it solves both issues. Thanks, Michael Young -- _ -- 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] 3833: Allow sending voicemail to multiple email addresses
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3833/#review12906 --- This change needs documentation since as it is unless you see this review or the commit you won't know it's possible. - Joshua Colp On July 21, 2014, 9:54 p.m., Jason Parker wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3833/ --- (Updated July 21, 2014, 9:54 p.m.) Review request for Asterisk Developers and Jacob Barber. Bugs: ASTERISK-24045 https://issues.asterisk.org/jira/browse/ASTERISK-24045 Repository: Asterisk Description --- Allow sending voicemail to multiple email addresses. Should this supersede https://reviewboard.asterisk.org/r/3829/ ? I would think so. Diffs - trunk/apps/app_voicemail.c 404951 Diff: https://reviewboard.asterisk.org/r/3833/diff/ Testing --- Patch in FreePBX distro for ~6 months. Thanks, Jason Parker -- _ -- 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] 3810: res_hep_rtcp: Add module that sends RTCP information to a Homer Server
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3810/#review12908 --- Ship it! Ship It! - Joshua Colp On July 27, 2014, 3:39 p.m., Matt Jordan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3810/ --- (Updated July 27, 2014, 3:39 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- This patch adds a new module to Asterisk, res_hep_rtcp. The module subscribes to Stasis and receives RTCP information back from the message bus, which it encodes into HEPv3 packets and sends to the res_hep module for transmission. Using this, someone with a Homer server can get live call quality monitoring for all channels in their Asterisk 12+ systems. NOTE: There were a few bugs uncovered by the tests written for the Asterisk Test Suite. As it turned out, these bugs were actually all in modules other than res_hep_rtcp, but I've included them with this diff as they are relatively small. 1) chan_pjsip failed to set its channel unique ids on its RTP instance on outbound calls. It now does this in the appropriate location, in the serialized call callback. 2) The rtp_engine was overflowing some values when packed into JSON. Specifically, some longs and unsigned ints can't be be packed into integer values, for obvious reasons. Since libjansson only supports integers, floats, strings, booleans, and objects, we print these values into strings. 3) res_rtp_asterisk had a few problems: (a) it would emit a source IP address of 0.0.0.0 if bound to that IP address. We now use ast_find_ourip to get a better IP address, and properly marshal the result into an ast_strdupa'd string. (b) Reports can be generated with no report bodies. In particular, this occurs when a sender is transmitting information to a receiver (who will send no RTP back to the sender). As such, the sender has no report body for what it received. We now properly handle this case, and the sender will emit SR reports with no body. Likewise, if we receive an RTCP packet with no report body, we will still generate the appropriate events. Diffs - /branches/12/res/res_rtp_asterisk.c 419680 /branches/12/res/res_hep_rtcp.c PRE-CREATION /branches/12/main/rtp_engine.c 419680 /branches/12/channels/chan_pjsip.c 419680 /branches/12/CHANGES 419680 Diff: https://reviewboard.asterisk.org/r/3810/diff/ Testing --- Some manual testing has be done, and automated tests have been written that exercise two scenarios: * One where both sides transmit RTP information to each other (rtcp-sender) * One where one side transmits RTP information, and the other is a passive receiver (rtcp-receiver) See https://reviewboard.asterisk.org/r/3863 As a side note, Alexander actually demo'd this at Kamailio World - you can see it on the 'dangerous demos' here - http://www.youtube.com/watch?v=ykBdOTCCSHs Thanks, Matt Jordan -- _ -- 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] 3869: res_pjsip_publish_asterisk: Inbound and outbound device state test
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3869/ --- Review request for Asterisk Developers. Bugs: ASTERISK-24115 https://issues.asterisk.org/jira/browse/ASTERISK-24115 Repository: testsuite Description --- This change adds two things: 1. The ability for an action to be configured for an AMI event handler. Right now two actions are supported: none and stop. If the stop action is configured the test will be terminated when the minimum number of events is received. 2. A test which establishes a relationship between two running Asterisk instances and publishes device state from one to the other. The test confirms that the second Asterisk receives all device state changes in the expected order. Diffs - /asterisk/trunk/tests/channels/pjsip/tests.yaml 5316 /asterisk/trunk/tests/channels/pjsip/publish/tests.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/configs/ast2/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/configs/ast2/extensions.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/lib/python/asterisk/ami.py 5316 Diff: https://reviewboard.asterisk.org/r/3869/diff/ Testing --- Executed test, it passes. Sabotaged test, it fails. 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] 3780: res_pjsip_outbound_publish / res_pjsip_publish_asterisk: Add outbound PUBLISH support with 'asterisk' event type.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3780/ --- (Updated July 31, 2014, 3:28 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- This adds two PJSIP modules which add outbound PUBLISH support and an 'asterisk' event type. The res_pjsip_outbound_publish module is a common module which provides basic logic for setting up outbound PUBLISH clients, handling authentication requests, handling configuration, and lifetime. Extra modules implement specific event types which are registered with res_pjsip_outbound_publish. Since it takes care of configuration when an outbound PUBLISH is configured extra configuration can be passed to the event type implementation to further configure itself. The res_pjsip_publish_asterisk module implements inbound and outbound support for an 'asterisk' event type. This event type conveys device and mailbox state between Asterisk instances using a JSON content body. As internal device or mailbox state changes the module sends a PUBLISH message to other configured instances. When a PUBLISH is received the contents are examined and a device or mailbox state change queued up within Asterisk. To restrict what is sent and received filtering is available using regular expressions which can reduce SIP traffic. A wiki page is available at https://wiki.asterisk.org/wiki/display/~jcolp/Exchanging+Device+and+Mailbox+State+Using+PJSIP which has some configuration details with some examples. This should also be reviewed. Diffs (updated) - /trunk/res/res_pjsip_pubsub.exports.in 419850 /trunk/res/res_pjsip_pubsub.c 419850 /trunk/res/res_pjsip_publish_asterisk.c PRE-CREATION /trunk/res/res_pjsip_outbound_publish.exports.in PRE-CREATION /trunk/res/res_pjsip_outbound_publish.c PRE-CREATION /trunk/include/asterisk/res_pjsip_pubsub.h 419850 /trunk/include/asterisk/res_pjsip_outbound_publish.h PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3780/diff/ Testing --- Set up two Asterisk instances, configured both sides to publish to eachother, made calls and manipulated voicemail. Watched PUBLISH messages go between them and state change. 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] 3869: res_pjsip_publish_asterisk: Inbound and outbound device state test
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3869/ --- (Updated Aug. 3, 2014, 10:58 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24115 https://issues.asterisk.org/jira/browse/ASTERISK-24115 Repository: testsuite Description --- This change adds two things: 1. The ability for an action to be configured for an AMI event handler. Right now two actions are supported: none and stop. If the stop action is configured the test will be terminated when the minimum number of events is received. 2. A test which establishes a relationship between two running Asterisk instances and publishes device state from one to the other. The test confirms that the second Asterisk receives all device state changes in the expected order. Diffs (updated) - /asterisk/trunk/tests/channels/pjsip/tests.yaml 5316 /asterisk/trunk/tests/channels/pjsip/publish/tests.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/configs/ast2/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/configs/ast2/extensions.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/lib/python/asterisk/ami.py 5316 Diff: https://reviewboard.asterisk.org/r/3869/diff/ Testing --- Executed test, it passes. Sabotaged test, it fails. 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] 3780: res_pjsip_outbound_publish / res_pjsip_publish_asterisk: Add outbound PUBLISH support with 'asterisk' event type.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3780/ --- (Updated Aug. 3, 2014, 10:58 p.m.) Review request for Asterisk Developers. Changes --- Separated device and mailbox into separate events. Repository: Asterisk Description --- This adds two PJSIP modules which add outbound PUBLISH support and an 'asterisk' event type. The res_pjsip_outbound_publish module is a common module which provides basic logic for setting up outbound PUBLISH clients, handling authentication requests, handling configuration, and lifetime. Extra modules implement specific event types which are registered with res_pjsip_outbound_publish. Since it takes care of configuration when an outbound PUBLISH is configured extra configuration can be passed to the event type implementation to further configure itself. The res_pjsip_publish_asterisk module implements inbound and outbound support for an 'asterisk' event type. This event type conveys device and mailbox state between Asterisk instances using a JSON content body. As internal device or mailbox state changes the module sends a PUBLISH message to other configured instances. When a PUBLISH is received the contents are examined and a device or mailbox state change queued up within Asterisk. To restrict what is sent and received filtering is available using regular expressions which can reduce SIP traffic. A wiki page is available at https://wiki.asterisk.org/wiki/display/~jcolp/Exchanging+Device+and+Mailbox+State+Using+PJSIP which has some configuration details with some examples. This should also be reviewed. Diffs (updated) - /trunk/res/res_pjsip_pubsub.exports.in 419936 /trunk/res/res_pjsip_pubsub.c 419936 /trunk/res/res_pjsip_publish_asterisk.c PRE-CREATION /trunk/res/res_pjsip_outbound_publish.exports.in PRE-CREATION /trunk/res/res_pjsip_outbound_publish.c PRE-CREATION /trunk/include/asterisk/res_pjsip_pubsub.h 419936 /trunk/include/asterisk/res_pjsip_outbound_publish.h PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3780/diff/ Testing --- Set up two Asterisk instances, configured both sides to publish to eachother, made calls and manipulated voicemail. Watched PUBLISH messages go between them and state change. 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] 3780: res_pjsip_outbound_publish / res_pjsip_publish_asterisk: Add outbound PUBLISH support with 'asterisk' event type.
Olle E. Johansson wrote: On 31 Jul 2014, at 17:28, Joshua Colp reviewbo...@asterisk.org mailto:reviewbo...@asterisk.org wrote: This adds two PJSIP modules which add outbound PUBLISH support and an 'asterisk' event type. I don't think it's a good idea to mix different events in one event tag. Will make it hard to handle in proxys and stuff. We will have to parse the json to find out what it is and what to do with it. It's not a good solution. Use asterisk-mwi and asterisk-devstate instead. I have incorporated this feedback and posted a new review for both the implementation and the test for device state. Cheers, -- Joshua Colp Digium, Inc. | Senior Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - US Check us out at: www.digium.com www.asterisk.org -- _ -- 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] Read dtmf after ast_bridge_call
vassilux . wrote: Hi, Kia ora, I try to read DTMF digits from a channel after calling ast_bridge_call. There are a lot of errors about : called with no recorded file descriptor from static struct ast_frame *__ast_read(struct ast_channel *chan, int dropaudio) function. I can read DTMF digits before execute ast_bridge_call. I made something wrong but I don't know :-) thank to help me Have you waited for frames on the channel using something like ast_waitfor? If you try to read when nothing is available on the channel you will see the message above. Cheers, -- Joshua Colp Digium, Inc. | Senior Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - US Check us out at: www.digium.com www.asterisk.org -- _ -- 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] 3603: func_jitterbuffer: fix errors and leaks caused by certain masquerade's
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3603/#review12967 --- I think this should be taken to the asterisk-dev mailing list so we can get a consensus on the exact behavior that should occur here. I realize that this solves the problem for you but I want to make sure everyone is on the same page and in agreement for the resulting behavior. - Joshua Colp On July 30, 2014, 11:19 p.m., Corey Farrell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3603/ --- (Updated July 30, 2014, 11:19 p.m.) Review request for Asterisk Developers, Joshua Colp and Matt Jordan. Bugs: ASTERISK-22409 https://issues.asterisk.org/jira/browse/ASTERISK-22409 Repository: Asterisk Description --- During masquerade it is possible for the AST_JITTERBUFFER_FD to be cleared (set to -1). This change adds a check when copying channel fd's to prevent clearing an FD with -1. This seems to resolve the bad audio quality experienced after the masquerade. When AST_JITTERBUFFER_FD was set to -1, this prevented the channel from polling that timer. This caused RTP packets to be received late, and discarded. The changes to func_jitterbuffer.c were created first. ast_free(jbframe); is needed to prevent jbframe from leaking if it is rejected by jb_impl. This ensures we don't start leaking packets if they are received too late or rejected by jb_impl for any other reason. The second change to func_jitterbuffer prevents a leak of ast_null_frame's that were duplicated (ie with ast_frdup or ast_frisolate). I believe this leak might actually be unrelated to the masquerade issue, and likely occurs for every single ast_null_frame. Diffs - /branches/11/main/channel.c 419821 /branches/11/funcs/func_jitterbuffer.c 419821 Diff: https://reviewboard.asterisk.org/r/3603/diff/ Testing --- Verified the scenario outlined in ASTERISK-22409 no longer experiences audio quality loss, and no longer causes leaks (tested under valgrind). I patched asterisk to ensure that ast_frfree performed an immediate free to ensure valgrind would report any attempted use after free. In early testing, I used debug messages instead of the added ast_frfree's - I verified the leaked frames reported by valgrind matched exactly to the number of debug messages. For the masquerade fix I tested with some debug code that showed the old and new FD, this is how I found the valid FD being replaced by -1. See JIRA ticket for example output. I have not tested this issue or fix against 12+, but the relevant code is the same as 11 - func_jitterbuffer code was moved to core but still the same code. 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] Read dtmf after ast_bridge_call
vassilux . wrote: Yes I use ast_waitfor with 2nd parameter -1 into a detached background thread. The function ast_waitfor used into my application function called from the dialplan and also into the dtmf read thread. This behavior is observed after bridging the incoming channel(the channel where I try to read dtmf) to an other channel created with dial API. I modified channel.c (line 3924) to disable the logging this error (just for an experience ). I can get the DTMF from the incoming. Er - are the two threads both calling ast_waitfor/ast_read at the same time? If so this is not supported. You effectively have two threads thinking they both need to service the channel at the same time. Only one thread can. You can either use a frame hook so you receive a callback on each frame where you can examine it and if DTMF do something OR use ast_channel_bridge and configure it so it will unbridge/return when it receives DTMF on the respective channel you want. -- Joshua Colp Digium, Inc. | Senior Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - US Check us out at: www.digium.com www.asterisk.org -- _ -- 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] Read dtmf after ast_bridge_call
vassilux . wrote: Thank for the information about the frame hook. I understand that both functions can be called by different threads because lock/unlock used internally. If you are referring to ast_waitfor and ast_read those functions are NOT safe to be called form separate threads simultaneously, even with locking internally. It won't work as you expect/want as only a single thread can service a channel. You would essentially end up with two threads fighting over the frames, with some going to one thread and some going to the other. -- Joshua Colp Digium, Inc. | Senior Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - US Check us out at: www.digium.com www.asterisk.org -- _ -- 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] 3870: alembic: Adjust sippeers, queue_members, and voicemail_messages tables.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3870/#review12980 --- Ship it! Ship It! - Joshua Colp On Aug. 4, 2014, 9:39 p.m., rmudgett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3870/ --- (Updated Aug. 4, 2014, 9:39 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-23825, ASTERISK-23847 and ASTERISK-23909 https://issues.asterisk.org/jira/browse/ASTERISK-23825 https://issues.asterisk.org/jira/browse/ASTERISK-23847 https://issues.asterisk.org/jira/browse/ASTERISK-23909 Repository: Asterisk Description --- * Increased the sippeers useragent max string size to 255. * Changed the queue_members uniqueid to an auto incremented integer instead of a string. * Increased the voicemail_messages BLOB size to LONGBLOB on mysql. * Fixed the add_tables_for_pjsip config change version downgrade actions to drop a table it created. * Adjusted the sample alembic.ini files cdr.ini.sample, config.ini.sample, and voicemail.ini.sample to give a mysql and postgres sqlalchemy.url lines. Diffs - /branches/12/contrib/ast-db-manage/voicemail/versions/39428242f7f5_increase_recording_column_size.py PRE-CREATION /branches/12/contrib/ast-db-manage/voicemail.ini.sample 420024 /branches/12/contrib/ast-db-manage/config/versions/5139253c0423_make_q_member_uniqueid_autoinc.py PRE-CREATION /branches/12/contrib/ast-db-manage/config/versions/43956d550a44_add_tables_for_pjsip.py 420024 /branches/12/contrib/ast-db-manage/config/versions/1758e8bbf6b_increase_useragent_column_size.py PRE-CREATION /branches/12/contrib/ast-db-manage/config.ini.sample 420024 /branches/12/contrib/ast-db-manage/cdr.ini.sample 420024 Diff: https://reviewboard.asterisk.org/r/3870/diff/ Testing --- The alembic table adjustments work in the upgrade and downgrade modes for mysql and postgres. The queue_members change does give a postgres warning message from the postgres alembic backend about a mysql only kind of change. The warning is benign and can be ignored as it is an alembic only warning. 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] 3889: format: Remove incorrectly assigned format compatibility bits for Opus and VP8.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3889/#review12982 --- Can you elaborate on what actually happens here between 11-12 and 12-12 that makes it so we need to remove the newly added ones? - Joshua Colp On Aug. 4, 2014, 10:47 p.m., rmudgett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3889/ --- (Updated Aug. 4, 2014, 10:47 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- The format compatibility bits need to remain frozen to new codecs. Otherwise, older channel drivers like chan_iax2 could attempt to negotiate them when they have no support for negotiating any of the format's other potential parameters. In addition, the chan_iax2 format preference order values sent over the wire have no defined fixed value to represent Opus and VP8. Diffs - /branches/12/main/format.c 420025 Diff: https://reviewboard.asterisk.org/r/3889/diff/ Testing --- Compiles and code inspeciton. See format.c:format_list_init() and format_pref.c:ast_codec_pref_convert(). The format_list container is used to generate the format_list_array which is then is used by chan_iax2 to determine the format index sent over the wire in IAX_IE_CODEC_PREFS. 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] 3889: format: Remove incorrectly assigned format compatibility bits for Opus and VP8.
On Aug. 5, 2014, 1:03 p.m., Joshua Colp wrote: Can you elaborate on what actually happens here between 11-12 and 12-12 that makes it so we need to remove the newly added ones? rmudgett wrote: The format compatibility bits need to remain frozen to new codecs because chan_iax2 leaks internal implementation values and doesn't know how to negotiate any codec options. Opus has options can be negotiated. I don't know about VP8 but as its a newer codec it likely has negotiable options. The IAX_IE_CODEC_PREFS body is a sequence of 8 bit values. In v1.8 and earlier the preference values are the index+1 into the frame.c:AST_FORMAT_LIST[]. In v10-v11 the preference values are the order that formats are put into the format_list ao2 list conatiner in format.c:format_list_init(). The format_list is then converted to the format_list_array object and indexed like AST_FORMAT_LIST[]. For backwards compatibility, the first 28 match the order in v1.8's AST_FORMAT_LIST[] and have format compatibility bits assigned. There is a comment saying the order of the first 28 must not change. The comment does not state why. However, that order cannot change because those formats are in the historical AST_FORMAT_LIST[], those formats have compatibility bits, and the index values are sent over the wire by IAX2. After the fixed formats there is a comment saying the order may change. The comment does not state why. However, the order may change because none of the following fo rmats have a format combatibility bit defined. In v12 the Opus and VP8 formats were added to the format_list eight formats after the end of the fixed formats and also assigned format compatibility bits. Interoperation between v12 and earlier Asterisk version should not be a problem because earlier versions doesn't know about Opus and VP8 and thus won't pick those codecs. Between v12 instances, Opus and VP8 could be picked. However, because of their order in the format_list there will be a gap of eight between G719 and Opus that will be difficult to maintain without dire comments that won't be followed. Also as stated, IAX2 doesn't know how to negotiate any codec options. It's fine to not negotiate attributes, it just yields a subpar experience in many cases. By removing these from 12 we're essentially removing something that people could have used. I'm not okay with that. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3889/#review12982 --- On Aug. 4, 2014, 10:47 p.m., rmudgett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3889/ --- (Updated Aug. 4, 2014, 10:47 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- The format compatibility bits need to remain frozen to new codecs. Otherwise, older channel drivers like chan_iax2 could attempt to negotiate them when they have no support for negotiating any of the format's other potential parameters. In addition, the chan_iax2 format preference order values sent over the wire have no defined fixed value to represent Opus and VP8. Diffs - /branches/12/main/format.c 420025 Diff: https://reviewboard.asterisk.org/r/3889/diff/ Testing --- Compiles and code inspeciton. See format.c:format_list_init() and format_pref.c:ast_codec_pref_convert(). The format_list container is used to generate the format_list_array which is then is used by chan_iax2 to determine the format index sent over the wire in IAX_IE_CODEC_PREFS. 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] 3869: res_pjsip_publish_asterisk: Inbound and outbound device state test
On Aug. 5, 2014, 5:17 p.m., Matt Jordan wrote: /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/configs/ast1/pjsip.conf, line 9 https://reviewboard.asterisk.org/r/3869/diff/2/?file=66140#file66140line9 This looks a little odd. Just to double check... should the '@' sign be there? Yes, it's on purpose. The device_state_filter is an extended configuration option on the object. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3869/#review12996 --- On Aug. 3, 2014, 10:58 p.m., Joshua Colp wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3869/ --- (Updated Aug. 3, 2014, 10:58 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24115 https://issues.asterisk.org/jira/browse/ASTERISK-24115 Repository: testsuite Description --- This change adds two things: 1. The ability for an action to be configured for an AMI event handler. Right now two actions are supported: none and stop. If the stop action is configured the test will be terminated when the minimum number of events is received. 2. A test which establishes a relationship between two running Asterisk instances and publishes device state from one to the other. The test confirms that the second Asterisk receives all device state changes in the expected order. Diffs - /asterisk/trunk/tests/channels/pjsip/tests.yaml 5316 /asterisk/trunk/tests/channels/pjsip/publish/tests.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/configs/ast2/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/configs/ast2/extensions.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/lib/python/asterisk/ami.py 5316 Diff: https://reviewboard.asterisk.org/r/3869/diff/ Testing --- Executed test, it passes. Sabotaged test, it fails. 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] 3869: res_pjsip_publish_asterisk: Inbound and outbound device state test
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3869/ --- (Updated Aug. 5, 2014, 5:37 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24115 https://issues.asterisk.org/jira/browse/ASTERISK-24115 Repository: testsuite Description --- This change adds two things: 1. The ability for an action to be configured for an AMI event handler. Right now two actions are supported: none and stop. If the stop action is configured the test will be terminated when the minimum number of events is received. 2. A test which establishes a relationship between two running Asterisk instances and publishes device state from one to the other. The test confirms that the second Asterisk receives all device state changes in the expected order. Diffs (updated) - /asterisk/trunk/tests/channels/pjsip/tests.yaml 5316 /asterisk/trunk/tests/channels/pjsip/publish/tests.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/configs/ast2/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/configs/ast2/extensions.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/lib/python/asterisk/ami.py 5316 Diff: https://reviewboard.asterisk.org/r/3869/diff/ Testing --- Executed test, it passes. Sabotaged test, it fails. 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] 3780: res_pjsip_outbound_publish / res_pjsip_publish_asterisk: Add outbound PUBLISH support with 'asterisk' event type.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3780/ --- (Updated Aug. 5, 2014, 5:37 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- This adds two PJSIP modules which add outbound PUBLISH support and an 'asterisk' event type. The res_pjsip_outbound_publish module is a common module which provides basic logic for setting up outbound PUBLISH clients, handling authentication requests, handling configuration, and lifetime. Extra modules implement specific event types which are registered with res_pjsip_outbound_publish. Since it takes care of configuration when an outbound PUBLISH is configured extra configuration can be passed to the event type implementation to further configure itself. The res_pjsip_publish_asterisk module implements inbound and outbound support for an 'asterisk' event type. This event type conveys device and mailbox state between Asterisk instances using a JSON content body. As internal device or mailbox state changes the module sends a PUBLISH message to other configured instances. When a PUBLISH is received the contents are examined and a device or mailbox state change queued up within Asterisk. To restrict what is sent and received filtering is available using regular expressions which can reduce SIP traffic. A wiki page is available at https://wiki.asterisk.org/wiki/display/~jcolp/Exchanging+Device+and+Mailbox+State+Using+PJSIP which has some configuration details with some examples. This should also be reviewed. Diffs (updated) - /trunk/res/res_pjsip_pubsub.exports.in 420047 /trunk/res/res_pjsip_pubsub.c 420047 /trunk/res/res_pjsip_publish_asterisk.c PRE-CREATION /trunk/res/res_pjsip_outbound_publish.exports.in PRE-CREATION /trunk/res/res_pjsip_outbound_publish.c PRE-CREATION /trunk/include/asterisk/res_pjsip_pubsub.h 420047 /trunk/include/asterisk/res_pjsip_outbound_publish.h PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3780/diff/ Testing --- Set up two Asterisk instances, configured both sides to publish to eachother, made calls and manipulated voicemail. Watched PUBLISH messages go between them and state change. 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] 3780: res_pjsip_outbound_publish / res_pjsip_publish_asterisk: Add outbound PUBLISH support with 'asterisk' event type.
On Aug. 6, 2014, 5:39 p.m., opticron wrote: /trunk/res/res_pjsip_outbound_publish.c, lines 524-528 https://reviewboard.asterisk.org/r/3780/diff/7/?file=66217#file66217line524 This feels like another implementation of string fields where the allocation is embedded in the struct. Is there a reason these values need to be a part of the same allocation? String fields aren't exactly the same. String fields can have some extra space, unless I calculate and determine the exact amount of space ahead of time I suppose. This pattern is used elsewhere, though. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3780/#review13027 --- On Aug. 5, 2014, 5:37 p.m., Joshua Colp wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3780/ --- (Updated Aug. 5, 2014, 5:37 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- This adds two PJSIP modules which add outbound PUBLISH support and an 'asterisk' event type. The res_pjsip_outbound_publish module is a common module which provides basic logic for setting up outbound PUBLISH clients, handling authentication requests, handling configuration, and lifetime. Extra modules implement specific event types which are registered with res_pjsip_outbound_publish. Since it takes care of configuration when an outbound PUBLISH is configured extra configuration can be passed to the event type implementation to further configure itself. The res_pjsip_publish_asterisk module implements inbound and outbound support for an 'asterisk' event type. This event type conveys device and mailbox state between Asterisk instances using a JSON content body. As internal device or mailbox state changes the module sends a PUBLISH message to other configured instances. When a PUBLISH is received the contents are examined and a device or mailbox state change queued up within Asterisk. To restrict what is sent and received filtering is available using regular expressions which can reduce SIP traffic. A wiki page is available at https://wiki.asterisk.org/wiki/display/~jcolp/Exchanging+Device+and+Mailbox+State+Using+PJSIP which has some configuration details with some examples. This should also be reviewed. Diffs - /trunk/res/res_pjsip_pubsub.exports.in 420047 /trunk/res/res_pjsip_pubsub.c 420047 /trunk/res/res_pjsip_publish_asterisk.c PRE-CREATION /trunk/res/res_pjsip_outbound_publish.exports.in PRE-CREATION /trunk/res/res_pjsip_outbound_publish.c PRE-CREATION /trunk/include/asterisk/res_pjsip_pubsub.h 420047 /trunk/include/asterisk/res_pjsip_outbound_publish.h PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3780/diff/ Testing --- Set up two Asterisk instances, configured both sides to publish to eachother, made calls and manipulated voicemail. Watched PUBLISH messages go between them and state change. 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] 3780: res_pjsip_outbound_publish / res_pjsip_publish_asterisk: Add outbound PUBLISH support with 'asterisk' event type.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3780/ --- (Updated Aug. 6, 2014, 6:23 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- This adds two PJSIP modules which add outbound PUBLISH support and an 'asterisk' event type. The res_pjsip_outbound_publish module is a common module which provides basic logic for setting up outbound PUBLISH clients, handling authentication requests, handling configuration, and lifetime. Extra modules implement specific event types which are registered with res_pjsip_outbound_publish. Since it takes care of configuration when an outbound PUBLISH is configured extra configuration can be passed to the event type implementation to further configure itself. The res_pjsip_publish_asterisk module implements inbound and outbound support for an 'asterisk' event type. This event type conveys device and mailbox state between Asterisk instances using a JSON content body. As internal device or mailbox state changes the module sends a PUBLISH message to other configured instances. When a PUBLISH is received the contents are examined and a device or mailbox state change queued up within Asterisk. To restrict what is sent and received filtering is available using regular expressions which can reduce SIP traffic. A wiki page is available at https://wiki.asterisk.org/wiki/display/~jcolp/Exchanging+Device+and+Mailbox+State+Using+PJSIP which has some configuration details with some examples. This should also be reviewed. Diffs (updated) - /trunk/res/res_pjsip_pubsub.exports.in 420253 /trunk/res/res_pjsip_pubsub.c 420253 /trunk/res/res_pjsip_publish_asterisk.c PRE-CREATION /trunk/res/res_pjsip_outbound_publish.exports.in PRE-CREATION /trunk/res/res_pjsip_outbound_publish.c PRE-CREATION /trunk/include/asterisk/res_pjsip_pubsub.h 420253 /trunk/include/asterisk/res_pjsip_outbound_publish.h PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3780/diff/ Testing --- Set up two Asterisk instances, configured both sides to publish to eachother, made calls and manipulated voicemail. Watched PUBLISH messages go between them and state change. 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] 3869: res_pjsip_publish_asterisk: Inbound and outbound device state test
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3869/ --- (Updated Aug. 7, 2014, 9:19 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 5371 Bugs: ASTERISK-24115 https://issues.asterisk.org/jira/browse/ASTERISK-24115 Repository: testsuite Description --- This change adds two things: 1. The ability for an action to be configured for an AMI event handler. Right now two actions are supported: none and stop. If the stop action is configured the test will be terminated when the minimum number of events is received. 2. A test which establishes a relationship between two running Asterisk instances and publishes device state from one to the other. The test confirms that the second Asterisk receives all device state changes in the expected order. Diffs - /asterisk/trunk/tests/channels/pjsip/tests.yaml 5316 /asterisk/trunk/tests/channels/pjsip/publish/tests.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/configs/ast2/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/configs/ast2/extensions.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/publish/asterisk_event_devicestate/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/lib/python/asterisk/ami.py 5316 Diff: https://reviewboard.asterisk.org/r/3869/diff/ Testing --- Executed test, it passes. Sabotaged test, it fails. 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] 3780: res_pjsip_outbound_publish / res_pjsip_publish_asterisk: Add outbound PUBLISH support with 'asterisk' event type.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3780/ --- (Updated Aug. 7, 2014, 9:35 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 420314 Repository: Asterisk Description --- This adds two PJSIP modules which add outbound PUBLISH support and an 'asterisk' event type. The res_pjsip_outbound_publish module is a common module which provides basic logic for setting up outbound PUBLISH clients, handling authentication requests, handling configuration, and lifetime. Extra modules implement specific event types which are registered with res_pjsip_outbound_publish. Since it takes care of configuration when an outbound PUBLISH is configured extra configuration can be passed to the event type implementation to further configure itself. The res_pjsip_publish_asterisk module implements inbound and outbound support for an 'asterisk' event type. This event type conveys device and mailbox state between Asterisk instances using a JSON content body. As internal device or mailbox state changes the module sends a PUBLISH message to other configured instances. When a PUBLISH is received the contents are examined and a device or mailbox state change queued up within Asterisk. To restrict what is sent and received filtering is available using regular expressions which can reduce SIP traffic. A wiki page is available at https://wiki.asterisk.org/wiki/display/~jcolp/Exchanging+Device+and+Mailbox+State+Using+PJSIP which has some configuration details with some examples. This should also be reviewed. Diffs - /trunk/res/res_pjsip_pubsub.exports.in 420253 /trunk/res/res_pjsip_pubsub.c 420253 /trunk/res/res_pjsip_publish_asterisk.c PRE-CREATION /trunk/res/res_pjsip_outbound_publish.exports.in PRE-CREATION /trunk/res/res_pjsip_outbound_publish.c PRE-CREATION /trunk/include/asterisk/res_pjsip_pubsub.h 420253 /trunk/include/asterisk/res_pjsip_outbound_publish.h PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3780/diff/ Testing --- Set up two Asterisk instances, configured both sides to publish to eachother, made calls and manipulated voicemail. Watched PUBLISH messages go between them and state change. 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
[asterisk-dev] [Code Review] 3914: res_http_websocket: Fix bug where query parameters are not present in client request
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3914/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- When acting as a client the res_http_websocket client drops any query parameters given to it in the URI when creating the client connection. This change ensures they are present in the resulting HTTP GET request. Diffs - /branches/13/res/res_http_websocket.c 421123 Diff: https://reviewboard.asterisk.org/r/3914/diff/ Testing --- 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] 3914: res_http_websocket: Fix bug where query parameters are not present in client request
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3914/ --- (Updated Aug. 17, 2014, 11:10 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 421210 Repository: Asterisk Description --- When acting as a client the res_http_websocket client drops any query parameters given to it in the URI when creating the client connection. This change ensures they are present in the resulting HTTP GET request. Diffs - /branches/13/res/res_http_websocket.c 421123 Diff: https://reviewboard.asterisk.org/r/3914/diff/ Testing --- 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] 3917: ARI: /channels/continue doesn't work on a channel originated to a Stasis application with no PBX
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3917/#review13095 --- /branches/12/res/res_stasis.c https://reviewboard.asterisk.org/r/3917/#comment23547 Add a comment explaining why this is being done. - Joshua Colp On Aug. 15, 2014, 3:53 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3917/ --- (Updated Aug. 15, 2014, 3:53 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24043 https://issues.asterisk.org/jira/browse/ASTERISK-24043 Repository: Asterisk Description --- Steps for reproduction: * one extension in dial plan [default] exten = 1000,1,NoOp(Show me something running in verbosity 3 please) * Start a websocket listening for a stasis application I use the websocket echo test to listen for application 'hello' * Use the command POST /channels to call an endpoint and put it into stasis I used the args endpoint='SIP/1601', app='hello', channelID='test' * Use the POST /channels/{channelID}/continue command to make the channel continue into the PBX I used the args channelID='test', context='default', extension='1000', priority=1 Expectations: * The channel should leave stasis and start a new life in the PBX. Or to simplify, I should see a StasisEnd event on the websocket and I should see extension 1000 executing on the SIP/1601- channel since I run at verbosity 3. Actual results prior to the patch: * StasisEnd is received, but the channel never enters the PBX and I don't see my NoOp application being ran. The patch solves this problem by calling ast_pbx_run_args on the channel before it would leave the stasis_app_exec function when there isn't a PBX running on the channel and when the channel isn't hung up. Patch: stasis-continue.diff submitted by Krandon Bruze Diffs - /branches/12/res/res_stasis.c 421079 Diff: https://reviewboard.asterisk.org/r/3917/diff/ Testing --- Ran through reproduction with and without patch to confirm that the issue existed and that the patch fixes the issue. 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] 3856: Fix Dynamic IAX2 Registrations From Failing Or Not Retrying After Temporary DNS Failure
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3856/#review13096 --- Ship it! Ship It! /branches/11/channels/chan_iax2.c https://reviewboard.asterisk.org/r/3856/#comment23548 strcpy with a comment that it's safe is fine here - Joshua Colp On Aug. 6, 2014, 6:38 p.m., Michael Young wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3856/ --- (Updated Aug. 6, 2014, 6:38 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-23767 https://issues.asterisk.org/jira/browse/ASTERISK-23767 Repository: Asterisk Description --- The reporter on the issue found some issues when upgrading from version 10 to 11 on 55 hosts. Two situations that can occur with dynamic registrations. 1. With dnsmgr disabled, if the host is not resolvable we are not trying to resolve the host again when it is time to attempt to register again. 2. With dnsmgr enabled, when the host is temporarily not resolvable the address is set to 0.0.0.0:0 and then when the host is resolvable the port is not being restored and stays set to 0. This patch resolves these two issues. Diffs - /branches/11/channels/chan_iax2.c 420257 Diff: https://reviewboard.asterisk.org/r/3856/diff/ Testing --- This was tested on a test machine. The reporter of the issue also tested this and has confirmed that it solves both issues. Thanks, Michael Young -- _ -- 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] 3893: fromdomainport used even if not set
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3893/#review13097 --- Ship it! This applies to 1.8+ - Joshua Colp On Aug. 7, 2014, 9:29 p.m., ebroad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3893/ --- (Updated Aug. 7, 2014, 9:29 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24178 https://issues.asterisk.org/jira/browse/ASTERISK-24178 Repository: Asterisk Description --- From the issue: Long story short, we set fromdomainport to STANDARD_SIP_PORT if it is not set by the user in fromdomain(see http://doxygen.asterisk.org/trunk/df/df0/chan__sip_8c-source.html#l30599). The issue is, in two places, we do the following: int ourport = (p-fromdomainport) ? p-fromdomainport : ast_sockaddr_port(p-ourip); Which means we will always use fromdomainport even if it wasn't actually set by the user. A side effect of this is when using SIP over TLS, an MWI response will always include port 5060 in the Message-Account field even though TLS uses 5061 (see http://doxygen.asterisk.org/trunk/df/df0/chan__sip_8c-source.html#l15086). The attached patch resolves this issue. Diffs - trunk/channels/chan_sip.c 420308 Diff: https://reviewboard.asterisk.org/r/3893/diff/ Testing --- Made and received calls without issue. Tested MWI notifications as well. Thanks, ebroad -- _ -- 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] 3907: ARI: Fix implicit answer of channel when playback is initiated on unanswered channels
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3907/#review13098 --- Ship it! Ship It! - Joshua Colp On Aug. 13, 2014, 10:04 p.m., Matt Jordan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3907/ --- (Updated Aug. 13, 2014, 10:04 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24229 https://issues.asterisk.org/jira/browse/ASTERISK-24229 Repository: Asterisk Description --- When issuing a POST /channels/{channel_id}/play on a channel that is not yet answered, ARI is supposed to: * Queue up an AST_CONTROL_PROGRESS on the channel * Start up the playback of the media Instead, we sneak an answer on the channel right before starting playing media. This is due to ARI's usage of control_streamfile. This function implicitly answers the channel (and doesn't give ARI the option to stop it). The answering of the channel here is probably unnecessary: * app_voicemail, by far the biggest consumer of this function, always answers the channels anyway * control stream file (in res_agi) and ControlPlayback probably shouldn't be implicitly answering the channel. As it turns out, the answering of the channel here is pretty old: 356042twilson if (ast_channel_state(chan) != AST_STATE_UP) { 3087 anthm res = ast_answer(chan); 180259 tilghman } (As in, ancient?) This patch goes with simply removing the ast_answer. If we're okay with this, I'll note in the UPGRADE.txt notes that ControlPlayback and control stream file no longer implicitly answer the channel. Diffs - /branches/12/main/app.c 420978 Diff: https://reviewboard.asterisk.org/r/3907/diff/ Testing --- With this patch, ARI can now playback media to a channel without having it be answered. My early howler monkeys were pleased. Thanks, Matt Jordan -- _ -- 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] 3917: ARI: /channels/continue doesn't work on a channel originated to a Stasis application with no PBX
On Aug. 18, 2014, 2:24 p.m., Joshua Colp wrote: /branches/12/res/res_stasis.c, lines 1330-1333 https://reviewboard.asterisk.org/r/3917/diff/1/?file=66549#file66549line1330 Add a comment explaining why this is being done. Jonathan Rose wrote: Honestly, I don't believe this is necessary and as far as I can tell the only real purpose of doing this is to free up these resources prior to doing an operation that will lock the channel. I disagree, what happens if the PBX then invokes Stasis again? The control would still exist for the channel. What would the behavior be? - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3917/#review13095 --- On Aug. 15, 2014, 3:53 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3917/ --- (Updated Aug. 15, 2014, 3:53 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24043 https://issues.asterisk.org/jira/browse/ASTERISK-24043 Repository: Asterisk Description --- Steps for reproduction: * one extension in dial plan [default] exten = 1000,1,NoOp(Show me something running in verbosity 3 please) * Start a websocket listening for a stasis application I use the websocket echo test to listen for application 'hello' * Use the command POST /channels to call an endpoint and put it into stasis I used the args endpoint='SIP/1601', app='hello', channelID='test' * Use the POST /channels/{channelID}/continue command to make the channel continue into the PBX I used the args channelID='test', context='default', extension='1000', priority=1 Expectations: * The channel should leave stasis and start a new life in the PBX. Or to simplify, I should see a StasisEnd event on the websocket and I should see extension 1000 executing on the SIP/1601- channel since I run at verbosity 3. Actual results prior to the patch: * StasisEnd is received, but the channel never enters the PBX and I don't see my NoOp application being ran. The patch solves this problem by calling ast_pbx_run_args on the channel before it would leave the stasis_app_exec function when there isn't a PBX running on the channel and when the channel isn't hung up. Patch: stasis-continue.diff submitted by Krandon Bruze Diffs - /branches/12/res/res_stasis.c 421079 Diff: https://reviewboard.asterisk.org/r/3917/diff/ Testing --- Ran through reproduction with and without patch to confirm that the issue existed and that the patch fixes the issue. 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] 3917: ARI: /channels/continue doesn't work on a channel originated to a Stasis application with no PBX
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3917/#review13102 --- Ship it! Ship It! /branches/12/res/res_stasis.c https://reviewboard.asterisk.org/r/3917/#comment23552 a new PBX - Joshua Colp On Aug. 18, 2014, 3:27 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3917/ --- (Updated Aug. 18, 2014, 3:27 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24043 https://issues.asterisk.org/jira/browse/ASTERISK-24043 Repository: Asterisk Description --- Steps for reproduction: * one extension in dial plan [default] exten = 1000,1,NoOp(Show me something running in verbosity 3 please) * Start a websocket listening for a stasis application I use the websocket echo test to listen for application 'hello' * Use the command POST /channels to call an endpoint and put it into stasis I used the args endpoint='SIP/1601', app='hello', channelID='test' * Use the POST /channels/{channelID}/continue command to make the channel continue into the PBX I used the args channelID='test', context='default', extension='1000', priority=1 Expectations: * The channel should leave stasis and start a new life in the PBX. Or to simplify, I should see a StasisEnd event on the websocket and I should see extension 1000 executing on the SIP/1601- channel since I run at verbosity 3. Actual results prior to the patch: * StasisEnd is received, but the channel never enters the PBX and I don't see my NoOp application being ran. The patch solves this problem by calling ast_pbx_run_args on the channel before it would leave the stasis_app_exec function when there isn't a PBX running on the channel and when the channel isn't hung up. Patch: stasis-continue.diff submitted by Krandon Bruze Diffs - /branches/12/res/res_stasis.c 421079 Diff: https://reviewboard.asterisk.org/r/3917/diff/ Testing --- Ran through reproduction with and without patch to confirm that the issue existed and that the patch fixes the issue. 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] 3913: chan_pjsip: Attended transfer does not update connected line name.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3913/#review13104 --- Ship it! Ship It! - Joshua Colp On Aug. 15, 2014, 1:55 a.m., rmudgett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3913/ --- (Updated Aug. 15, 2014, 1:55 a.m.) Review request for Asterisk Developers. Bugs: AFS-98 https://issues.asterisk.org/jira/browse/AFS-98 Repository: Asterisk Description --- A calls B B SIP attended transfers to C C answers, B and C can see each other's connected line information B completes the transfer A has number but no name connected line information about C while C has the full information about A I examined the incoming and outgoing party id information handling of chan_pjsip and found several issues: * Fixed ast_sip_session_create_outgoing() not setting up the configured endpoint id as the new channel's caller id. This is why party A got default connected line information. * Made update_initial_connected_line() use the channel's CALLERID(id) information. The core, app_dial, or predial routine may have filled in or changed the endpoint caller id information. * Fixed chan_pjsip_new() not setting the full party id information available on the caller id and ANI party id. This includes the configured callerid_tag string and other party id fields. * Fixed accessing channel party id information without the channel lock held. * Fixed using the effective connected line id without doing a deep copy outside of holding the channel lock. Shallow copy string pointers can become stale if the channel lock is not held. * Made queue_connected_line_update() also update the channel's CALLERID(id) information. Moving the channel to another bridge would need the information there for the new bridge peer. * Fixed off nominal memory leak in update_incoming_connected_line(). * Added callerid_tag string to party id information from enabled trust_inbound endpoint in caller_id_incoming_request(). Diffs - /branches/13/res/res_pjsip_session.c 421122 /branches/13/res/res_pjsip_caller_id.c 421122 /branches/13/channels/chan_pjsip.c 421122 Diff: https://reviewboard.asterisk.org/r/3913/diff/ Testing --- Attended transfer gives correct party id information to all parties involved. Blind transfer gives correct party id information to all parties involved. 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] 3930: PJSIP: Resolve race condition regarding media handling when receiving 200 OK
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3930/#review13157 --- My only concern with this is that we somehow have code which expected to be invoked at #3 - do you think this is possible at all? - Joshua Colp On Aug. 24, 2014, 3:52 p.m., Mark Michelson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3930/ --- (Updated Aug. 24, 2014, 3:52 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24212 https://issues.asterisk.org/jira/browse/ASTERISK-24212 Repository: Asterisk Description --- In /r/3927, I introduced a changeset that fixes a scheduler race condition. I noted on that issue that even though I was no longer seeing the same crashes when running tests, I was still seeing occasional test failures due to some media-related race condition. I have determined the cause of this race condition. PJSIP's inv_session API provides two callbacks, on_state_changed() and on_tsx_state_changed(). Both of these are called into when the state of a transaction changes, and the documentation is not particularly clear about what the difference is between these. Through some investigation, I've found that what happens is: 1) Transaction layer detects a state change (such as a request/response being sent/received, or a timeout) 2) Transaction layer informs the inv_session layer. 3) inv_session layer sets the new state and calls the on_state_changed() callback. 4) inv_session layer performs its processing of the transaction state change (including potential calls to the on_media_update() callback). 5) inv_session layer calls the on_tsx_state_changed() callback. res_pjsip_session is hooked into step (3) such that when a new SIP request or response is sent or received, we call into session supplements to let them react to the new message. One of these session supplements is in chan_pjsip and queues an AST_CONTROL_ANSWER frame when a 200 OK response is received. In the test where I am seeing the race condition occur, a call is originated to a PJSIP channel (Alice). Once Alice answers, her channel is placed into the dialplan to execute the TalkDetect() application. The problem here is that at step (3), the AST_CONTROL_ANSWER frame is queued onto Alice's channel, and the PBX thread moves the channel into the TalkDetect() application before step (4) is executed. Step (4) is important for setting formats on Alice's channel and setting appropriate translation paths as well. Since step (4) has not been completed yet, the attempt to play a file as part of TalkDetect() fails, and therefore the test fails. In most runs, however, step (4) does get completed before the PBX thread moves Alice's channel into TalkDetect(). The fix presented here is pretty simple. For transaction state changes, we are now hooked into step (5) instead of step (3) to call session supplements. This means that we do not call into session supplements until media has been negotiated on the channel. This eliminates the race condition entirely. Diffs - /branches/12/res/res_pjsip_session.c 421883 Diff: https://reviewboard.asterisk.org/r/3930/diff/ Testing --- I ran the tests/channels/pjsip/basic_calls/two_parties/nominal/alice_initiated/alice_hangs_up test in a loop on my console both before and after applying this patch. Before applying the patch, I would usually encounter a test failure within an hour or two. After applying this patch, I left the test running in a loop for over 24 hours and never had a test failure. 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
Re: [asterisk-dev] [Code Review] 3930: PJSIP: Resolve race condition regarding media handling when receiving 200 OK
On Aug. 24, 2014, 3:56 p.m., Joshua Colp wrote: My only concern with this is that we somehow have code which expected to be invoked at #3 - do you think this is possible at all? It may also be useful to document your flow discovery above within the code somewhere for future reference. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3930/#review13157 --- On Aug. 24, 2014, 3:52 p.m., Mark Michelson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3930/ --- (Updated Aug. 24, 2014, 3:52 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24212 https://issues.asterisk.org/jira/browse/ASTERISK-24212 Repository: Asterisk Description --- In /r/3927, I introduced a changeset that fixes a scheduler race condition. I noted on that issue that even though I was no longer seeing the same crashes when running tests, I was still seeing occasional test failures due to some media-related race condition. I have determined the cause of this race condition. PJSIP's inv_session API provides two callbacks, on_state_changed() and on_tsx_state_changed(). Both of these are called into when the state of a transaction changes, and the documentation is not particularly clear about what the difference is between these. Through some investigation, I've found that what happens is: 1) Transaction layer detects a state change (such as a request/response being sent/received, or a timeout) 2) Transaction layer informs the inv_session layer. 3) inv_session layer sets the new state and calls the on_state_changed() callback. 4) inv_session layer performs its processing of the transaction state change (including potential calls to the on_media_update() callback). 5) inv_session layer calls the on_tsx_state_changed() callback. res_pjsip_session is hooked into step (3) such that when a new SIP request or response is sent or received, we call into session supplements to let them react to the new message. One of these session supplements is in chan_pjsip and queues an AST_CONTROL_ANSWER frame when a 200 OK response is received. In the test where I am seeing the race condition occur, a call is originated to a PJSIP channel (Alice). Once Alice answers, her channel is placed into the dialplan to execute the TalkDetect() application. The problem here is that at step (3), the AST_CONTROL_ANSWER frame is queued onto Alice's channel, and the PBX thread moves the channel into the TalkDetect() application before step (4) is executed. Step (4) is important for setting formats on Alice's channel and setting appropriate translation paths as well. Since step (4) has not been completed yet, the attempt to play a file as part of TalkDetect() fails, and therefore the test fails. In most runs, however, step (4) does get completed before the PBX thread moves Alice's channel into TalkDetect(). The fix presented here is pretty simple. For transaction state changes, we are now hooked into step (5) instead of step (3) to call session supplements. This means that we do not call into session supplements until media has been negotiated on the channel. This eliminates the race condition entirely. Diffs - /branches/12/res/res_pjsip_session.c 421883 Diff: https://reviewboard.asterisk.org/r/3930/diff/ Testing --- I ran the tests/channels/pjsip/basic_calls/two_parties/nominal/alice_initiated/alice_hangs_up test in a loop on my console both before and after applying this patch. Before applying the patch, I would usually encounter a test failure within an hour or two. After applying this patch, I left the test running in a loop for over 24 hours and never had a test failure. 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
Re: [asterisk-dev] [Code Review] 3930: PJSIP: Resolve race condition regarding media handling when receiving 200 OK
On Aug. 24, 2014, 3:56 p.m., Joshua Colp wrote: My only concern with this is that we somehow have code which expected to be invoked at #3 - do you think this is possible at all? Joshua Colp wrote: It may also be useful to document your flow discovery above within the code somewhere for future reference. Mark Michelson wrote: The only thing I could think of is if a session supplement specifically wants to be called before media negotiation so that it may play a part in how media ends up getting negotiated. Does anything do that? I don't believe so, nope. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3930/#review13157 --- On Aug. 24, 2014, 3:52 p.m., Mark Michelson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3930/ --- (Updated Aug. 24, 2014, 3:52 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24212 https://issues.asterisk.org/jira/browse/ASTERISK-24212 Repository: Asterisk Description --- In /r/3927, I introduced a changeset that fixes a scheduler race condition. I noted on that issue that even though I was no longer seeing the same crashes when running tests, I was still seeing occasional test failures due to some media-related race condition. I have determined the cause of this race condition. PJSIP's inv_session API provides two callbacks, on_state_changed() and on_tsx_state_changed(). Both of these are called into when the state of a transaction changes, and the documentation is not particularly clear about what the difference is between these. Through some investigation, I've found that what happens is: 1) Transaction layer detects a state change (such as a request/response being sent/received, or a timeout) 2) Transaction layer informs the inv_session layer. 3) inv_session layer sets the new state and calls the on_state_changed() callback. 4) inv_session layer performs its processing of the transaction state change (including potential calls to the on_media_update() callback). 5) inv_session layer calls the on_tsx_state_changed() callback. res_pjsip_session is hooked into step (3) such that when a new SIP request or response is sent or received, we call into session supplements to let them react to the new message. One of these session supplements is in chan_pjsip and queues an AST_CONTROL_ANSWER frame when a 200 OK response is received. In the test where I am seeing the race condition occur, a call is originated to a PJSIP channel (Alice). Once Alice answers, her channel is placed into the dialplan to execute the TalkDetect() application. The problem here is that at step (3), the AST_CONTROL_ANSWER frame is queued onto Alice's channel, and the PBX thread moves the channel into the TalkDetect() application before step (4) is executed. Step (4) is important for setting formats on Alice's channel and setting appropriate translation paths as well. Since step (4) has not been completed yet, the attempt to play a file as part of TalkDetect() fails, and therefore the test fails. In most runs, however, step (4) does get completed before the PBX thread moves Alice's channel into TalkDetect(). The fix presented here is pretty simple. For transaction state changes, we are now hooked into step (5) instead of step (3) to call session supplements. This means that we do not call into session supplements until media has been negotiated on the channel. This eliminates the race condition entirely. Diffs - /branches/12/res/res_pjsip_session.c 421883 Diff: https://reviewboard.asterisk.org/r/3930/diff/ Testing --- I ran the tests/channels/pjsip/basic_calls/two_parties/nominal/alice_initiated/alice_hangs_up test in a loop on my console both before and after applying this patch. Before applying the patch, I would usually encounter a test failure within an hour or two. After applying this patch, I left the test running in a loop for over 24 hours and never had a test failure. 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
Re: [asterisk-dev] [Code Review] 3930: PJSIP: Resolve race condition regarding media handling when receiving 200 OK
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3930/#review13164 --- Review your doxygen because I think you are repeating yourself, and also run through all the PJSIP tests. Do that and this is good to go. - Joshua Colp On Aug. 24, 2014, 11:37 p.m., Mark Michelson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3930/ --- (Updated Aug. 24, 2014, 11:37 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24212 https://issues.asterisk.org/jira/browse/ASTERISK-24212 Repository: Asterisk Description --- In /r/3927, I introduced a changeset that fixes a scheduler race condition. I noted on that issue that even though I was no longer seeing the same crashes when running tests, I was still seeing occasional test failures due to some media-related race condition. I have determined the cause of this race condition. PJSIP's inv_session API provides two callbacks, on_state_changed() and on_tsx_state_changed(). Both of these are called into when the state of a transaction changes, and the documentation is not particularly clear about what the difference is between these. Through some investigation, I've found that what happens is: 1) Transaction layer detects a state change (such as a request/response being sent/received, or a timeout) 2) Transaction layer informs the inv_session layer. 3) inv_session layer sets the new state and calls the on_state_changed() callback. 4) inv_session layer performs its processing of the transaction state change (including potential calls to the on_media_update() callback). 5) inv_session layer calls the on_tsx_state_changed() callback. res_pjsip_session is hooked into step (3) such that when a new SIP request or response is sent or received, we call into session supplements to let them react to the new message. One of these session supplements is in chan_pjsip and queues an AST_CONTROL_ANSWER frame when a 200 OK response is received. In the test where I am seeing the race condition occur, a call is originated to a PJSIP channel (Alice). Once Alice answers, her channel is placed into the dialplan to execute the TalkDetect() application. The problem here is that at step (3), the AST_CONTROL_ANSWER frame is queued onto Alice's channel, and the PBX thread moves the channel into the TalkDetect() application before step (4) is executed. Step (4) is important for setting formats on Alice's channel and setting appropriate translation paths as well. Since step (4) has not been completed yet, the attempt to play a file as part of TalkDetect() fails, and therefore the test fails. In most runs, however, step (4) does get completed before the PBX thread moves Alice's channel into TalkDetect(). The fix presented here is pretty simple. For transaction state changes, we are now hooked into step (5) instead of step (3) to call session supplements. This means that we do not call into session supplements until media has been negotiated on the channel. This eliminates the race condition entirely. Diffs - /branches/12/res/res_pjsip_session.c 421883 Diff: https://reviewboard.asterisk.org/r/3930/diff/ Testing --- I ran the tests/channels/pjsip/basic_calls/two_parties/nominal/alice_initiated/alice_hangs_up test in a loop on my console both before and after applying this patch. Before applying the patch, I would usually encounter a test failure within an hour or two. After applying this patch, I left the test running in a loop for over 24 hours and never had a test failure. 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
Re: [asterisk-dev] [Code Review] 3930: PJSIP: Resolve race condition regarding media handling when receiving 200 OK
On Aug. 25, 2014, 1:12 p.m., Joshua Colp wrote: Review your doxygen because I think you are repeating yourself, and also run through all the PJSIP tests. Do that and this is good to go. Mark Michelson wrote: I don't see the doxygen problem. One states that on transaction state changes, it is called before SDP negotiation and the other states that on transaction state changes it is called after SDP negotiation. * This gets called by the inv_session layer once it has processed a transaction state * change. This gets called into after the inv_session layer has completely processed * the state change. That is what caught me. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3930/#review13164 --- On Aug. 24, 2014, 11:37 p.m., Mark Michelson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3930/ --- (Updated Aug. 24, 2014, 11:37 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24212 https://issues.asterisk.org/jira/browse/ASTERISK-24212 Repository: Asterisk Description --- In /r/3927, I introduced a changeset that fixes a scheduler race condition. I noted on that issue that even though I was no longer seeing the same crashes when running tests, I was still seeing occasional test failures due to some media-related race condition. I have determined the cause of this race condition. PJSIP's inv_session API provides two callbacks, on_state_changed() and on_tsx_state_changed(). Both of these are called into when the state of a transaction changes, and the documentation is not particularly clear about what the difference is between these. Through some investigation, I've found that what happens is: 1) Transaction layer detects a state change (such as a request/response being sent/received, or a timeout) 2) Transaction layer informs the inv_session layer. 3) inv_session layer sets the new state and calls the on_state_changed() callback. 4) inv_session layer performs its processing of the transaction state change (including potential calls to the on_media_update() callback). 5) inv_session layer calls the on_tsx_state_changed() callback. res_pjsip_session is hooked into step (3) such that when a new SIP request or response is sent or received, we call into session supplements to let them react to the new message. One of these session supplements is in chan_pjsip and queues an AST_CONTROL_ANSWER frame when a 200 OK response is received. In the test where I am seeing the race condition occur, a call is originated to a PJSIP channel (Alice). Once Alice answers, her channel is placed into the dialplan to execute the TalkDetect() application. The problem here is that at step (3), the AST_CONTROL_ANSWER frame is queued onto Alice's channel, and the PBX thread moves the channel into the TalkDetect() application before step (4) is executed. Step (4) is important for setting formats on Alice's channel and setting appropriate translation paths as well. Since step (4) has not been completed yet, the attempt to play a file as part of TalkDetect() fails, and therefore the test fails. In most runs, however, step (4) does get completed before the PBX thread moves Alice's channel into TalkDetect(). The fix presented here is pretty simple. For transaction state changes, we are now hooked into step (5) instead of step (3) to call session supplements. This means that we do not call into session supplements until media has been negotiated on the channel. This eliminates the race condition entirely. Diffs - /branches/12/res/res_pjsip_session.c 421883 Diff: https://reviewboard.asterisk.org/r/3930/diff/ Testing --- I ran the tests/channels/pjsip/basic_calls/two_parties/nominal/alice_initiated/alice_hangs_up test in a loop on my console both before and after applying this patch. Before applying the patch, I would usually encounter a test failure within an hour or two. After applying this patch, I left the test running in a loop for over 24 hours and never had a test failure. 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
Re: [asterisk-dev] [Code Review] 3930: PJSIP: Resolve race condition regarding media handling when receiving 200 OK
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3930/#review13188 --- Ship it! Ship It! /branches/12/include/asterisk/res_pjsip_session.h https://reviewboard.asterisk.org/r/3930/#comment23664 Document the default value for this. - Joshua Colp On Aug. 26, 2014, 10:44 p.m., Mark Michelson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3930/ --- (Updated Aug. 26, 2014, 10:44 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24212 https://issues.asterisk.org/jira/browse/ASTERISK-24212 Repository: Asterisk Description --- In /r/3927, I introduced a changeset that fixes a scheduler race condition. I noted on that issue that even though I was no longer seeing the same crashes when running tests, I was still seeing occasional test failures due to some media-related race condition. I have determined the cause of this race condition. PJSIP's inv_session API provides two callbacks, on_state_changed() and on_tsx_state_changed(). Both of these are called into when the state of a transaction changes, and the documentation is not particularly clear about what the difference is between these. Through some investigation, I've found that what happens is: 1) Transaction layer detects a state change (such as a request/response being sent/received, or a timeout) 2) Transaction layer informs the inv_session layer. 3) inv_session layer sets the new state and calls the on_state_changed() callback. 4) inv_session layer performs its processing of the transaction state change (including potential calls to the on_media_update() callback). 5) inv_session layer calls the on_tsx_state_changed() callback. res_pjsip_session is hooked into step (3) such that when a new SIP request or response is sent or received, we call into session supplements to let them react to the new message. One of these session supplements is in chan_pjsip and queues an AST_CONTROL_ANSWER frame when a 200 OK response is received. In the test where I am seeing the race condition occur, a call is originated to a PJSIP channel (Alice). Once Alice answers, her channel is placed into the dialplan to execute the TalkDetect() application. The problem here is that at step (3), the AST_CONTROL_ANSWER frame is queued onto Alice's channel, and the PBX thread moves the channel into the TalkDetect() application before step (4) is executed. Step (4) is important for setting formats on Alice's channel and setting appropriate translation paths as well. Since step (4) has not been completed yet, the attempt to play a file as part of TalkDetect() fails, and therefore the test fails. In most runs, however, step (4) does get completed before the PBX thread moves Alice's channel into TalkDetect(). The fix presented here is pretty simple. For transaction state changes, we are now hooked into step (5) instead of step (3) to call session supplements. This means that we do not call into session supplements until media has been negotiated on the channel. This eliminates the race condition entirely. ***EDIT 26 Aug, 2014*** After running all PJSIP tests in the testsuite, I found that things weren't quite so cut and dry as they were initially presented here. Here are some additional findings: * For incoming 3XX responses, it's actually at step (2) that we get told about the redirection. There are some session supplements that need to be called back at this time. * Not all session supplements can safely wait until after media handling to be called into. For instance, the NAT handling code that rewrites contacts needs to be called into before media handling on an inbound 200 OK. * In something pretty much completely unrelated, I found that there was an undiscovered bug in res_pjsip_session.c that prevented incoming reinvites from reaching session supplements. With regards to the first two bullet points, I have added a response_priority field to session supplements. This allows for a session supplement to tell res_pjsip_session at what point during response handling it should be called into. All but two session supplements are called into at the same point that they were prior to writing this patch. However, two supplements now specify when they should be called into: 1) res_pjsip_diversion's supplement specifies to be called into before processing redirections. 2) One of chan_pjsip's session supplements specifies to be called into after media negotiation. I discovered the bug in the third bullet by accidentally fixing it. Because of this change, there are some session supplements
Re: [asterisk-dev] PJSIP insecure fonctionality
Yuriy Gorlichenko wrote: Hello. I use PJSIP as channel for my clients and CHAN_SIP as channel for my proiders. I user chan_sip because I can not find any analog of insecure option at PJSIP. Does IT have this analog of this function. Kia ora, Yes, but it is not grouped with endpoints. It is a separate configuration item that allows you to match IP addresses (or ranges) to an endpoint. When traffic comes in it is then associated. You can see an example of configuration for connecting to a VoIP provider here[1] and the specific part which does the IP address matching is the type=identify section. In the future for this type of help the asterisk-users mailing list would be more appropriate. [1] https://wiki.asterisk.org/wiki/display/AST/Configuring+res_pjsip Cheers, -- Joshua Colp Digium, Inc. | Senior Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - US Check us out at: www.digium.com www.asterisk.org -- _ -- 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] 12.5.0 cache_entry_compute_hash/app_meeme crash
Jared Mauch wrote: I have a crash here: snip It appears to be triggered when taking the meetme_statis_generate_msg of chan=NULL and passing it downstream to ast_channel_blob_create_from_cache #7 0x7f4d1e5036fe in meetme_stasis_generate_msg (meetme_conference=value optimized out, chan=0x0, user=0x0, message_type=0x34595a8, extras=value optimized out) at app_meetme.c:1382 1382msg = ast_channel_blob_create_from_cache(ast_channel_uniqueid(chan), message_type, json_object); seems there should be a check here in app_meetme.c for this. I also saw either this or something similar in asterisk-13.0.0-beta1. This issue was reported here[1] and has been fixed in SVN. The fix will be in the next releases. Cheers, [1] https://issues.asterisk.org/jira/browse/ASTERISK-24234 -- Joshua Colp Digium, Inc. | Senior Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - US Check us out at: www.digium.com www.asterisk.org -- _ -- 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] 3981: chan_rtp: Add unicast RTP support to chan_multicast_rtp.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3981/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- The chan_multicast_rtp module currently allows sending an RTP stream as multicast but not unicast. Using unicast allows the channel to become a thin protocol-less transport mechanism for media. To that end I've added unicast RTP support and moved both into a single channel driver (chan_rtp) as they shared most of the code. Cleanup of some code also occurred. Diffs - /trunk/channels/chan_rtp.c PRE-CREATION /trunk/channels/chan_multicast_rtp.c 422835 Diff: https://reviewboard.asterisk.org/r/3981/diff/ Testing --- Originated a call to a UnicastRTP channel and sent it to a Playback. Confirmed that RTP was sent to the provided IP address/port with the given format. 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
[asterisk-dev] [Code Review] 3982: res_rtp_asterisk: Fix a slew of TURN issues.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3982/ --- Review request for Asterisk Developers. Bugs: ASTERISK-23577 and ASTERISK-23634 https://issues.asterisk.org/jira/browse/ASTERISK-23577 https://issues.asterisk.org/jira/browse/ASTERISK-23634 Repository: Asterisk Description --- The TURN support in res_rtp_asterisk has been exercised by only a few, which has uncovered a slew of issues. 1. The number of file descriptors that an ioqueue instance in pjlib can support is a fixed number. Exceeding this causes an assertion. The code will now dynamically create/terminate threads as needed to ensure that this limit is not exceeded on ioqueue instances. 2. The API did not allow users of the TURN code to explicitly request a TURN session with details. This has been added. 3. The ICE code has a fixed size array of 4 for transports. As our transport identifiers started at 1 we were exceeding this causing an assertion. Our identifiers now start at 0. 4. The TURN client did not set up client binding causing needless bandwidth usage. Upon ICE completion if TURN is used channel binding is now established. 5. The code will no longer update address information on every sent packet. The remote address is now updated only once upon ICE completion to the target address. 6. When relaying was in use STUN traffic was getting looped back to res_rtp_asterisk due to it being handled on the normal socket. It is now handled in the TURN session callback instead. 7. Logging when a TURN relay is in use now uses the IP address that the TURN relay is sending/receiving to/from on our behalf. 8. Synchronization between the TURN session and the RTP instance now ensures that the session has been fully created or fully destroyed before continuing. 9. Some cleanup has occurred when tearing down the pj environment. Diffs - /branches/13/res/res_rtp_asterisk.c 422835 /branches/13/include/asterisk/rtp_engine.h 422835 Diff: https://reviewboard.asterisk.org/r/3982/diff/ Testing --- Sabotaged the code so the only candidates I sent were of the relay type. Confirmed bidirectional media flow using the TURN relay. 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] 3981: chan_rtp: Add unicast RTP support to chan_multicast_rtp.
Johann Steinwendtner wrote: On 2014-09-07 17:07, Joshua Colp wrote: This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3981/ Testing Originated a call to a UnicastRTP channel and sent it to a Playback. Confirmed that RTP was sent to the provided IP address/port with the given format. Hello, can you please explain what you mean by with the given format. There is a patch from John R. Covert which adds the capability of selecting the codec. Or is this not necessary in trunk. http://thread.gmane.org/gmane.comp.telephony.pbx.asterisk.devel/48495 The UnicastRTP dial string allows specifying the format. I did not touch MulticastRTP. -- Joshua Colp Digium, Inc. | Senior Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - US Check us out at: www.digium.com www.asterisk.org -- _ -- 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] 3982: res_rtp_asterisk: Fix a slew of TURN issues.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3982/ --- (Updated Sept. 10, 2014, 11:24 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-23577 and ASTERISK-23634 https://issues.asterisk.org/jira/browse/ASTERISK-23577 https://issues.asterisk.org/jira/browse/ASTERISK-23634 Repository: Asterisk Description --- The TURN support in res_rtp_asterisk has been exercised by only a few, which has uncovered a slew of issues. 1. The number of file descriptors that an ioqueue instance in pjlib can support is a fixed number. Exceeding this causes an assertion. The code will now dynamically create/terminate threads as needed to ensure that this limit is not exceeded on ioqueue instances. 2. The API did not allow users of the TURN code to explicitly request a TURN session with details. This has been added. 3. The ICE code has a fixed size array of 4 for transports. As our transport identifiers started at 1 we were exceeding this causing an assertion. Our identifiers now start at 0. 4. The TURN client did not set up client binding causing needless bandwidth usage. Upon ICE completion if TURN is used channel binding is now established. 5. The code will no longer update address information on every sent packet. The remote address is now updated only once upon ICE completion to the target address. 6. When relaying was in use STUN traffic was getting looped back to res_rtp_asterisk due to it being handled on the normal socket. It is now handled in the TURN session callback instead. 7. Logging when a TURN relay is in use now uses the IP address that the TURN relay is sending/receiving to/from on our behalf. 8. Synchronization between the TURN session and the RTP instance now ensures that the session has been fully created or fully destroyed before continuing. 9. Some cleanup has occurred when tearing down the pj environment. Diffs (updated) - /branches/13/res/res_rtp_asterisk.c 422898 /branches/13/include/asterisk/rtp_engine.h 422898 Diff: https://reviewboard.asterisk.org/r/3982/diff/ Testing --- Sabotaged the code so the only candidates I sent were of the relay type. Confirmed bidirectional media flow using the TURN relay. 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] 3985: realtime configuration: anything that goes through ast_destroy_realtime crashes if only a single key/value pair is used.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3985/#review13269 --- /branches/12/main/config.c https://reviewboard.asterisk.org/r/3985/#comment23754 It's still possible for this to occur under extreme circumstances (out of memory). - Joshua Colp On Sept. 9, 2014, 5:04 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3985/ --- (Updated Sept. 9, 2014, 5:04 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24231 https://issues.asterisk.org/jira/browse/ASTERISK-24231 Repository: Asterisk Description --- Attempting to use the following CLI command would cause a crash (regardless of whether a realtime database were in any way configured) realtime destroy (any table) field1 value1 This was due to attempting to create a variable list from a NULL name since we sent an empty va list to a function that creates variable lists from va lists. I've changed that function to be tolerant of empty va lists and I've changed the ast_realtime_destroy function to go ahead and call the realtime engine destroy functions even if the variable list that it gets back from that function is NULL. Diffs - /branches/12/main/config.c 422869 Diff: https://reviewboard.asterisk.org/r/3985/diff/ Testing --- I tested the changes against both a postgres database connected via res_config_pgsql and a mysql database connected via res_config_odbc. Of the two, res_config_odbc appeared to be the more likely source of trouble since it might use the variable list for something other than just iteration. It appears to set a string field to the variable list for whatever reason... in any event, both appeared to work fine and I didn't notice anything out of the ordinary happening due to deallocation or anything. I went ahead and checked all the other realtime engines to make sure their destroy functions looked like they would be compatible with NULL variable lists as well and they all checked out... they just add entries by iterating along the list and if it's NULL it just won't enter the loop in each case. 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] 3986: config: bug: fix truncation of included config files on permissions error
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3986/#review13270 --- Ship it! Ship It! - Joshua Colp On Sept. 10, 2014, 1:34 a.m., George Joseph wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3986/ --- (Updated Sept. 10, 2014, 1:34 a.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- ast_config_text_file_save() currently truncates include files as they are processed. If a subsequent include file or the main config file has a permissions error that prevents writing, earlier include files are left truncated resulting in a frantic search for backups. This patch causes ast_config_text_file_save to check for write access on all files before it truncates any of them. Will be applied 1.8 trunk. Diffs - branches/1.8/main/config.c 422882 Diff: https://reviewboard.asterisk.org/r/3986/diff/ Testing --- Tested normal access and verified that no files are truncated if any of the can't be written to. Thanks, George Joseph -- _ -- 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.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3954/#review13271 --- /branches/13/res/res_pjsip.c https://reviewboard.asterisk.org/r/3954/#comment23755 unsigned int /branches/13/res/res_pjsip.c https://reviewboard.asterisk.org/r/3954/#comment23757 I'm hesitant to say that this will always be true. I fear that due to the asynchronous nature of things (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. /branches/13/res/res_pjsip.c https://reviewboard.asterisk.org/r/3954/#comment23756 Magic number. Define this somewhere. /branches/13/res/res_pjsip/pjsip_options.c https://reviewboard.asterisk.org/r/3954/#comment23758 Same may or may not be true thing applies from above. - Joshua Colp On Sept. 4, 2014, 11:41 p.m., rmudgett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3954/ --- (Updated Sept. 4, 2014, 11:41 p.m.) Review request for Asterisk Developers. Bugs: AFS-155 and ASTERISK-24295 https://issues.asterisk.org/jira/browse/AFS-155 https://issues.asterisk.org/jira/browse/ASTERISK-24295 Repository: Asterisk Description --- The crash on the issues is a result of an invalid transport configuration change when asterisk is restarted. The attempt to send the qualify request fails and we cleaned up. However, the callback is also called which results in a double unref of the objects involved. * Fixed send_request_cb() and qualify_contact_cb() to not cleanup the token when the PJSIP event is PJSIP_EVENT_TRANSPORT_ERROR since the initial function call will do the clean up. * Made send_request_cb() able to handle repeated challenges (Up to 10). * Fix periodic endpoint qualify OPTIONS sched deletion race by avoiding it. The sched entry will no longer self stop and must be externally stopped. * Added REF_DEBUG description tags to struct sched_data in pjsip_options.c. * Fix some off-nominal ref leaks in schedule_qualify(), qualify_and_schedule(). * Reordered pjsip_options.c module start/stop code to cleanup better on error. Diffs - /branches/13/res/res_pjsip/pjsip_options.c 422583 /branches/13/res/res_pjsip.c 422583 Diff: https://reviewboard.asterisk.org/r/3954/diff/ Testing --- * With the qualify_frequency option enabled, added and removed a local_net= line in the transport section and restarted asterisk via core restart now. Before the latest patch version, asterisk 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 -- _ -- 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] 3978: Testsuite: Test for RLS large NOTIFY requests
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3978/#review13272 --- Ship it! Ship It! - Joshua Colp On Sept. 4, 2014, 8:11 p.m., Mark Michelson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3978/ --- (Updated Sept. 4, 2014, 8:11 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24181 https://issues.asterisk.org/jira/browse/ASTERISK-24181 Repository: testsuite Description --- This is a very simple test to ensure that Asterisk is capable of sending very large NOTIFY requests for RLS. This test has a SIPp scenario subscribe to a list of 20 presence resources. The resulting full state NOTIFY upon subscription clocks in around 17500 bytes. We don't care about the actual integrity of the NOTIFY that Asterisk sends for this test, just that Asterisk actually can send it. Diffs - /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/off_nominal/tests.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/off_nominal/large_notify/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/off_nominal/large_notify/sipp/subscribe.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/off_nominal/large_notify/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/off_nominal/large_notify/configs/ast1/extensions.conf PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3978/diff/ Testing --- Without the patch on /r/3977, this test fails since Asterisk cannot send the large NOTIFY. With the patch applied, this test succeeds. 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
Re: [asterisk-dev] [Code Review] 3985: realtime configuration: anything that goes through ast_destroy_realtime crashes if only a single key/value pair is used.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3985/#review13273 --- - Joshua Colp On Sept. 9, 2014, 5:04 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3985/ --- (Updated Sept. 9, 2014, 5:04 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24231 https://issues.asterisk.org/jira/browse/ASTERISK-24231 Repository: Asterisk Description --- Attempting to use the following CLI command would cause a crash (regardless of whether a realtime database were in any way configured) realtime destroy (any table) field1 value1 This was due to attempting to create a variable list from a NULL name since we sent an empty va list to a function that creates variable lists from va lists. I've changed that function to be tolerant of empty va lists and I've changed the ast_realtime_destroy function to go ahead and call the realtime engine destroy functions even if the variable list that it gets back from that function is NULL. Diffs - /branches/12/main/config.c 422869 Diff: https://reviewboard.asterisk.org/r/3985/diff/ Testing --- I tested the changes against both a postgres database connected via res_config_pgsql and a mysql database connected via res_config_odbc. Of the two, res_config_odbc appeared to be the more likely source of trouble since it might use the variable list for something other than just iteration. It appears to set a string field to the variable list for whatever reason... in any event, both appeared to work fine and I didn't notice anything out of the ordinary happening due to deallocation or anything. I went ahead and checked all the other realtime engines to make sure their destroy functions looked like they would be compatible with NULL variable lists as well and they all checked out... they just add entries by iterating along the list and if it's NULL it just won't enter the loop in each case. 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] 3954: pjsip_options.c: Fix race condition stopping periodic out of dialog OPTIONS request.
On Sept. 10, 2014, 11:51 a.m., Joshua Colp wrote: /branches/13/res/res_pjsip.c, lines 2478-2480 https://reviewboard.asterisk.org/r/3954/diff/3/?file=67221#file67221line2478 I'm hesitant to say that this will always be true. I fear that due to the asynchronous nature of things (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 know that pjsip_endpt_send_request() will return failure and call the callback for normal errors. I also know from looking at the pjproject code that pjsip_endpt_send_request() can return error and not call the callback. Either we assume that pjsip_endpt_send_request() will _always_ call the callback when it returns error and do what I did in version 2 of the patch or we go this way. One way 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. I think we have to do the mapping and know what will invoke the callback and what won't. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3954/#review13271 --- On Sept. 4, 2014, 11:41 p.m., rmudgett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3954/ --- (Updated Sept. 4, 2014, 11:41 p.m.) Review request for Asterisk Developers. Bugs: AFS-155 and ASTERISK-24295 https://issues.asterisk.org/jira/browse/AFS-155 https://issues.asterisk.org/jira/browse/ASTERISK-24295 Repository: Asterisk Description --- The crash on the issues is a result of an invalid transport configuration change when asterisk is restarted. The attempt to send the qualify request fails and we cleaned up. However, the callback is also called which results in a double unref of the objects involved. * Fixed send_request_cb() and qualify_contact_cb() to not cleanup the token when the PJSIP event is PJSIP_EVENT_TRANSPORT_ERROR since the initial function call will do the clean up. * Made send_request_cb() able to handle repeated challenges (Up to 10). * Fix periodic endpoint qualify OPTIONS sched deletion race by avoiding it. The sched entry will no longer self stop and must be externally stopped. * Added REF_DEBUG description tags to struct sched_data in pjsip_options.c. * Fix some off-nominal ref leaks in schedule_qualify(), qualify_and_schedule(). * Reordered pjsip_options.c module start/stop code to cleanup better on error. Diffs - /branches/13/res/res_pjsip/pjsip_options.c 422583 /branches/13/res/res_pjsip.c 422583 Diff: https://reviewboard.asterisk.org/r/3954/diff/ Testing --- * With the qualify_frequency option enabled, added and removed a local_net= line in the transport section and restarted asterisk via core restart now. Before the latest patch version, asterisk 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 -- _ -- 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] 3988: res_pjsip: Don't require a password when doing userpass authentication
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3988/#review13278 --- /branches/12/res/res_pjsip/config_auth.c https://reviewboard.asterisk.org/r/3988/#comment23764 Test this and make sure that the PJSIP digest authentication doesn't freak. :P - Joshua Colp On Sept. 10, 2014, 5:53 p.m., Sean Bright wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3988/ --- (Updated Sept. 10, 2014, 5:53 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- An empty password is valid for username/password authentication so we shouldn't barf on it. Diffs - /branches/12/res/res_pjsip/config_auth.c 422963 Diff: https://reviewboard.asterisk.org/r/3988/diff/ Testing --- Compiles. Thanks, Sean Bright -- _ -- 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] SIPit 31
Olivier wrote: snip Resource List Subscriptions ? This is definitely something I've wanted to test but forgot to add it to the list. Now added! Thanks, -- Joshua Colp Digium, Inc. | Senior Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - US Check us out at: www.digium.com www.asterisk.org -- _ -- 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] 3985: realtime configuration: anything that goes through ast_destroy_realtime crashes if only a single key/value pair is used.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3985/#review13281 --- Ship it! Ship It! - Joshua Colp On Sept. 10, 2014, 9:31 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3985/ --- (Updated Sept. 10, 2014, 9:31 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24231 https://issues.asterisk.org/jira/browse/ASTERISK-24231 Repository: Asterisk Description --- Attempting to use the following CLI command would cause a crash (regardless of whether a realtime database were in any way configured) realtime destroy (any table) field1 value1 This was due to attempting to create a variable list from a NULL name since we sent an empty va list to a function that creates variable lists from va lists. I've changed that function to be tolerant of empty va lists and I've changed the ast_realtime_destroy function to go ahead and call the realtime engine destroy functions even if the variable list that it gets back from that function is NULL. Diffs - /branches/12/main/config.c 422869 Diff: https://reviewboard.asterisk.org/r/3985/diff/ Testing --- I tested the changes against both a postgres database connected via res_config_pgsql and a mysql database connected via res_config_odbc. Of the two, res_config_odbc appeared to be the more likely source of trouble since it might use the variable list for something other than just iteration. It appears to set a string field to the variable list for whatever reason... in any event, both appeared to work fine and I didn't notice anything out of the ordinary happening due to deallocation or anything. I went ahead and checked all the other realtime engines to make sure their destroy functions looked like they would be compatible with NULL variable lists as well and they all checked out... they just add entries by iterating along the list and if it's NULL it just won't enter the loop in each case. 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] 3987: Bridging: Fix bouncing native bridge
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3987/#review13282 --- Ship it! Ship It! - Joshua Colp On Sept. 10, 2014, 3:56 p.m., opticron wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3987/ --- (Updated Sept. 10, 2014, 3:56 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24211 https://issues.asterisk.org/jira/browse/ASTERISK-24211 Repository: Asterisk Description --- This fixes a situation in Asterisk 1.8 and 11 where ast_channel_bridge could cause a bouncing native bridge. In the case of the dial_LS_options test, this was a remote RTP bridge which caused the audio path to continually cycle between Asterisk and the remote endpoints generating a large number of SIP messages and delaying the test long enough to cause it to fail (checking timing was part of the test). The root cause was that the code to decide whether to use native bridging was expecting a time-remaining value of 0 to be the default instead of the actual default value of -1. A value of 0 or negative numbers could also be generated by preceding code in some circumstances. Both issues are addressed in this patch. Diffs - branches/1.8/main/channel.c 422898 Diff: https://reviewboard.asterisk.org/r/3987/diff/ Testing --- Verified that the test (11-only) operated correctly with this patch. Thanks, opticron -- _ -- 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.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3954/#review13283 --- /branches/13/res/res_pjsip.c https://reviewboard.asterisk.org/r/3954/#comment23767 Looking at the pjproject code why wouldn't the following return values cover these cases for when the callback is not called: PJ_EINVAL PJ_EINVALIDOP PJ_ENOMEM Handling those as special would reduce the memory leak possibility. - Joshua Colp On Sept. 10, 2014, 10:18 p.m., rmudgett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3954/ --- (Updated Sept. 10, 2014, 10:18 p.m.) Review request for Asterisk Developers. Bugs: AFS-155 and ASTERISK-24295 https://issues.asterisk.org/jira/browse/AFS-155 https://issues.asterisk.org/jira/browse/ASTERISK-24295 Repository: Asterisk Description --- The crash on the issues is a result of an invalid transport configuration change when asterisk is restarted. The attempt to send the qualify request fails and we cleaned up. However, the callback is also called which results in a double unref of the objects involved. * Fixed send_request_cb() and qualify_contact_cb() to not cleanup the token when the PJSIP event is PJSIP_EVENT_TRANSPORT_ERROR since the initial function call will do the clean up. * Made send_request_cb() able to handle repeated challenges (Up to 10). * Fix periodic endpoint qualify OPTIONS sched deletion race by avoiding it. The sched entry will no longer self stop and must be externally stopped. * Added REF_DEBUG description tags to struct sched_data in pjsip_options.c. * Fix some off-nominal ref leaks in schedule_qualify(), qualify_and_schedule(). * Reordered pjsip_options.c module start/stop code to cleanup better on error. Diffs - /branches/13/res/res_pjsip/pjsip_options.c 422963 /branches/13/res/res_pjsip.c 422963 Diff: https://reviewboard.asterisk.org/r/3954/diff/ Testing --- * With the qualify_frequency option enabled, added and removed a local_net= line in the transport section and restarted asterisk via core restart now. Before the latest patch version, asterisk 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 -- _ -- 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] 3981: chan_rtp: Add unicast RTP support to chan_multicast_rtp.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3981/ --- (Updated Sept. 12, 2014, 12:42 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 423004 Repository: Asterisk Description --- The chan_multicast_rtp module currently allows sending an RTP stream as multicast but not unicast. Using unicast allows the channel to become a thin protocol-less transport mechanism for media. To that end I've added unicast RTP support and moved both into a single channel driver (chan_rtp) as they shared most of the code. Cleanup of some code also occurred. Diffs - /trunk/channels/chan_rtp.c PRE-CREATION /trunk/channels/chan_multicast_rtp.c 422835 Diff: https://reviewboard.asterisk.org/r/3981/diff/ Testing --- Originated a call to a UnicastRTP channel and sent it to a Playback. Confirmed that RTP was sent to the provided IP address/port with the given format. 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] 2964: res_pjsip_outbound_registration: Add virtual line support for automatic inbound matching
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/2964/ --- (Updated Sept. 13, 2014, 10:44 a.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- This patch adds virtual line support to the res_pjsip_outbound_registration module. This is an optional feature and simply adds a line URI parameter to the Contact we place in the outbound registration. If this line parameter is present on incoming requests we use it to establish a relationship to the outbound registration and match it to a user configured endpoint. This has the benefit that when registering to another server where it is supported you no longer have to do IP based matching for all of their potential servers. The downside (and why this is optional) is that if a third party got the line parameter they could send you calls as if they were the legit remote server. Diffs (updated) - /trunk/res/res_pjsip_outbound_registration.c 423063 /trunk/configs/samples/pjsip.conf.sample 423063 Diff: https://reviewboard.asterisk.org/r/2964/diff/ Testing --- Registered to an ITSP, placed an inbound call from them, confirmed matched using line parameter. Registered to a chan_sip instance, placed an inbound call from it, confirmed matched using line parameter. 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
[asterisk-dev] [Code Review] 3992: res_pjsip_sdp_rtp: Add optimistic SRTP support.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3992/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- When enabling SRTP support in PJSIP it is either forced on or disabled. This means that if you specify SRTP but the client does not support it the session will fail. For situations where this guarantee is not required this new functionality can be used to optimistically use SRTP if possible. This has the added benefit of encrypting the media when possible but does not guarantee it. This also fixes an issue where a client may offer SRTP using the normal transport but we reject it. Diffs - /trunk/res/res_pjsip_sdp_rtp.c 423064 /trunk/res/res_pjsip/pjsip_configuration.c 423064 /trunk/res/res_pjsip.c 423064 /trunk/include/asterisk/res_pjsip.h 423064 /trunk/configs/samples/pjsip.conf.sample 423064 Diff: https://reviewboard.asterisk.org/r/3992/diff/ Testing --- Used Blink to place calls with optimistic enabled and disabled on the PJSIP side. In Blink I alternated between disabled/mandatory/optional. Confirmed that for each scenario the expected outcome occurred. Blink Asterisk Result Disabled Optimistic Off Failed Disabled Optimistic On Success (Not encrypted) Mandatory Optimistic Off Success (Encrypted) Mandatory Optimistic On Success (Encrypted) Optional Optimistic Off Success (Encrypted) Optional Optimistic On Success (Encrypted) 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] udptl ports
James Cloos wrote: JC == Joshua Colpjc...@digium.com writes: In a report which I received, ast 1.8 used the same port which it had been using to originate rtp also to originate the udptl. (The other side used and advertised the same port in the re-INVITE negotiation which it had used and advertised in the initial INVITE.) JC Asterisk won't send from the same port. They are two different JC ranges. If it went through a NAT, though, then the external mapping JC COULD reuse the port depending on how it operates. The asterisk in the pcap is not in a nat. And it certainly did use the same src port for the t38 as it had been using for the rtp. The dst ports were different -- each was correctly sent to the other side's advertised port -- but both came from the same src. Unless stuff has been modified or something in between is doing things the code just won't allow it. The UDPTL and RTP stacks don't share sockets and the RTP side keeps the port open while UDPTL is going on so even if the UDPTL side wanted to use the same port it couldn't bind to it. Can you provide the trace of this with SDP and logging? -- Joshua Colp Digium, Inc. | Senior Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - US Check us out at: www.digium.com www.asterisk.org -- _ -- 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] 3992: res_pjsip_sdp_rtp: Add optimistic SRTP support.
On Sept. 15, 2014, 4:14 p.m., opticron wrote: This change appears to render the media_use_received_transport configuration option non-functional since it removes all checks relating to it. It'll still work, the difference is on received it is always done and the outgoing check has been relaxed to not rely on the option. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3992/#review13301 --- On Sept. 14, 2014, 12:25 a.m., Joshua Colp wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3992/ --- (Updated Sept. 14, 2014, 12:25 a.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- When enabling SRTP support in PJSIP it is either forced on or disabled. This means that if you specify SRTP but the client does not support it the session will fail. For situations where this guarantee is not required this new functionality can be used to optimistically use SRTP if possible. This has the added benefit of encrypting the media when possible but does not guarantee it. This also fixes an issue where a client may offer SRTP using the normal transport but we reject it. Diffs - /trunk/res/res_pjsip_sdp_rtp.c 423064 /trunk/res/res_pjsip/pjsip_configuration.c 423064 /trunk/res/res_pjsip.c 423064 /trunk/include/asterisk/res_pjsip.h 423064 /trunk/configs/samples/pjsip.conf.sample 423064 Diff: https://reviewboard.asterisk.org/r/3992/diff/ Testing --- Used Blink to place calls with optimistic enabled and disabled on the PJSIP side. In Blink I alternated between disabled/mandatory/optional. Confirmed that for each scenario the expected outcome occurred. Blink Asterisk Result Disabled Optimistic Off Failed Disabled Optimistic On Success (Not encrypted) Mandatory Optimistic Off Success (Encrypted) Mandatory Optimistic On Success (Encrypted) Optional Optimistic Off Success (Encrypted) Optional Optimistic On Success (Encrypted) 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] 3982: res_rtp_asterisk: Fix a slew of TURN issues.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3982/ --- (Updated Sept. 16, 2014, 6:08 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 423150 Bugs: ASTERISK-23577 and ASTERISK-23634 https://issues.asterisk.org/jira/browse/ASTERISK-23577 https://issues.asterisk.org/jira/browse/ASTERISK-23634 Repository: Asterisk Description --- The TURN support in res_rtp_asterisk has been exercised by only a few, which has uncovered a slew of issues. 1. The number of file descriptors that an ioqueue instance in pjlib can support is a fixed number. Exceeding this causes an assertion. The code will now dynamically create/terminate threads as needed to ensure that this limit is not exceeded on ioqueue instances. 2. The API did not allow users of the TURN code to explicitly request a TURN session with details. This has been added. 3. The ICE code has a fixed size array of 4 for transports. As our transport identifiers started at 1 we were exceeding this causing an assertion. Our identifiers now start at 0. 4. The TURN client did not set up client binding causing needless bandwidth usage. Upon ICE completion if TURN is used channel binding is now established. 5. The code will no longer update address information on every sent packet. The remote address is now updated only once upon ICE completion to the target address. 6. When relaying was in use STUN traffic was getting looped back to res_rtp_asterisk due to it being handled on the normal socket. It is now handled in the TURN session callback instead. 7. Logging when a TURN relay is in use now uses the IP address that the TURN relay is sending/receiving to/from on our behalf. 8. Synchronization between the TURN session and the RTP instance now ensures that the session has been fully created or fully destroyed before continuing. 9. Some cleanup has occurred when tearing down the pj environment. Diffs - /branches/13/res/res_rtp_asterisk.c 422898 /branches/13/include/asterisk/rtp_engine.h 422898 Diff: https://reviewboard.asterisk.org/r/3982/diff/ Testing --- Sabotaged the code so the only candidates I sent were of the relay type. Confirmed bidirectional media flow using the TURN relay. 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] 3999: chan_iax2: Jitterbuffer causes crash in Asterisk 13 on account of format changes
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3999/#review13329 --- /branches/13/channels/chan_iax2.c https://reviewboard.asterisk.org/r/3999/#comment23811 Despite it not changing behavior I'd still use 20 here to match 12. - Joshua Colp On Sept. 16, 2014, 9:28 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3999/ --- (Updated Sept. 16, 2014, 9:28 p.m.) Review request for Asterisk Developers, Matt Jordan and rmudgett. Bugs: ASTERISK-24265 https://issues.asterisk.org/jira/browse/ASTERISK-24265 Repository: Asterisk Description --- This only occurs when the chan_iax jitterbuffer options are set and no when setting jitterbuffers via diaplan or anything like that. The first time __get_from_jb is called, voiceformat has not been set on the IAX pvt. Trying to call ast_format_get_default_ms on a NULL pointer fails. This worked previously because Asterisk 12 and prior simply modified an ast_format on the stack, so when it used ast_codec_interp_len on that format pointer there was no possibility for it to be a NULL pointer... just one that doesn't have a real format associated with it. One thing I might not be doing right here is that I'm using an interpolation value of 0 for a NULL format. Previously Asterisk would just check to see if the format was ILBC and if it was, return 30 and otherwise return 20... so it might be more appropriate to use 20 instead of 0. It doesn't appear to make a difference for the sake of behavior. Diffs - /branches/13/channels/chan_iax2.c 423149 Diff: https://reviewboard.asterisk.org/r/3999/diff/ Testing --- Ran basic call from a PJSIP peer to an IAX peer with the following: [general] ; The important parts jitterbuffer=yes forcejitterbuffer=yes [deskbox] type=friend requirecalltoken = no username = deskbox secret = secret host = dynamic transfer = no dtmfmode = auto encryption = no qualify = 300 context = default disallow=all allow=ulaw allow=alaw ; Most of this is probably unnecessary for reproduction Without the patch this would crash in 13 but work fine in 12. With the patch, behavior strongly resembles 12 with the initial call into __get_from_jb attempting to jb_get and getting JB_OK back and then later, when the call was actually answered, the voice format would change and the function would call again with the proper format and the jitterbuffer would get started properly. 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] 3988: res_pjsip: Don't require a password when doing userpass authentication
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3988/#review1 --- Ship it! Ship It! - Joshua Colp On Sept. 15, 2014, 7:27 p.m., Sean Bright wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3988/ --- (Updated Sept. 15, 2014, 7:27 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- An empty password is valid for username/password authentication so we shouldn't barf on it. Diffs - /branches/12/res/res_pjsip/config_auth.c 422963 Diff: https://reviewboard.asterisk.org/r/3988/diff/ Testing --- Compiles. Registered a device with auth_type=userpass and no password set. Tested registration with a password which failed, and again without a password (an empty password) and it succeeds. Thanks, Sean Bright -- _ -- 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] 3999: chan_iax2: Jitterbuffer causes crash in Asterisk 13 on account of format changes
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3999/#review13337 --- Ship it! Ship It! - Joshua Colp On Sept. 16, 2014, 9:28 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3999/ --- (Updated Sept. 16, 2014, 9:28 p.m.) Review request for Asterisk Developers, Matt Jordan and rmudgett. Bugs: ASTERISK-24265 https://issues.asterisk.org/jira/browse/ASTERISK-24265 Repository: Asterisk Description --- This only occurs when the chan_iax jitterbuffer options are set and no when setting jitterbuffers via diaplan or anything like that. The first time __get_from_jb is called, voiceformat has not been set on the IAX pvt. Trying to call ast_format_get_default_ms on a NULL pointer fails. This worked previously because Asterisk 12 and prior simply modified an ast_format on the stack, so when it used ast_codec_interp_len on that format pointer there was no possibility for it to be a NULL pointer... just one that doesn't have a real format associated with it. One thing I might not be doing right here is that I'm using an interpolation value of 0 for a NULL format. Previously Asterisk would just check to see if the format was ILBC and if it was, return 30 and otherwise return 20... so it might be more appropriate to use 20 instead of 0. It doesn't appear to make a difference for the sake of behavior. Diffs - /branches/13/channels/chan_iax2.c 423149 Diff: https://reviewboard.asterisk.org/r/3999/diff/ Testing --- Ran basic call from a PJSIP peer to an IAX peer with the following: [general] ; The important parts jitterbuffer=yes forcejitterbuffer=yes [deskbox] type=friend requirecalltoken = no username = deskbox secret = secret host = dynamic transfer = no dtmfmode = auto encryption = no qualify = 300 context = default disallow=all allow=ulaw allow=alaw ; Most of this is probably unnecessary for reproduction Without the patch this would crash in 13 but work fine in 12. With the patch, behavior strongly resembles 12 with the initial call into __get_from_jb attempting to jb_get and getting JB_OK back and then later, when the call was actually answered, the voice format would change and the function would call again with the proper format and the jitterbuffer would get started properly. 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
[asterisk-dev] [Code Review] 4008: res_pjsip_session: Add additional checks to prevent session refresh in unpossible states.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4008/ --- Review request for Asterisk Developers and Mark Michelson. Repository: Asterisk Description --- Currently it is possible for ast_sip_session_refresh to be called at times where the state of the dialog and INVITE session does not allow it to send a request. Trying to send a request actually results in an assertion within PJSIP. This change adds additional checks for deferral of these, stops generating new SDP on COLP UPDATEs, makes it so deferral does not always result in SDP generation, and ensures that after a provisional response that any pending UPDATE occurs. * Note: Currently there is still a bug within pjproject which causes an UPDATE without SDP sent after a provisional response to cancel the pending SDP negotiation when it should not. This has been reported to Teluu and a fix is being worked on. Diffs - /branches/12/res/res_pjsip_session.c 423546 /branches/12/channels/chan_pjsip.c 423546 Diff: https://reviewboard.asterisk.org/r/4008/diff/ Testing --- Modified the dialing API to change callerID at certain points (after call but before handling responses, after handling responses). Confirmed that new code correctly defers sending COLP updates. 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] Opinions Needed: PJSIP Outboud Registration with multiple server_uris
George Joseph wrote: snip My proposal is to allow registration/server_uri to be specified as a comma separated list or to be specified multiple times just like aor/contact and identify/match. This would allow us to manage only 1 registration section in the same manner as aor and identify. A registration would be Registered if at least 1 server was registered or I can add a registration_state_registered_at variable similar to device_state_busy_at which would specify the minimum number of servers needed to be considered Registered. If you actually want 2 registrations, nothing stops you from creating them. It seems like a minor issue but for me (and other folks I'm betting) the transition from chan_sip to chan_pjsip rests on getting tools, GUIs, scripts, etc. migrated. That variable number of registrations is a pain to deal with. Josh had some issues with this approach on IRC and suggested bringing the proposal to the list for wider discussion. I'll elaborate on why I dislike this: 1. It's overloading the outbound registration object to actually mean outbound registrations. This complicates the implementation and from a user perspective becomes harder to explain. Someone doesn't have to use it, but if they see it... they will... potentially without knowing what it means. 2. From a first glance and seeing the type as outbound registration I would expect that someone would think of it as an ordered failover list. If I can't register to the first then I'll try the second and so on. This is what some phones do. That's not what this would do. 3. What it means to register to multiple URIs and how that should be interpreted is really environment specific. You've mentioned making what the cumulative result is configurable but knowing what failed and what succeeded individually is still important. It may not be for some ITSPs, it may be for others. If addressed individually you get this information already and it can be interpreted. 4. I'm hesitant to push this into the outbound registration module as the solution. I'm all for making things easier but having these lower level blocks as simple and direct as possible is valuable. Trying not to cross the line is hard. 5. The idea of higher level concept configuration has been thrown around as something to make this easier. I personally think this sort of thing belongs there. A type=trunk, itsp, phone, etc. Lower level blocks remain the same and additional logic on top can be added to handle this sort of thing. I look forward to seeing what others think! Cheers, -- Joshua Colp Digium, Inc. | Senior Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - US Check us out at: www.digium.com www.asterisk.org -- _ -- 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] Opinions Needed: PJSIP Outboud Registration with multiple server_uris
George Joseph wrote: snip 5. The idea of higher level concept configuration has been thrown around as something to make this easier. I personally think this sort of thing belongs there. A type=trunk, itsp, phone, etc. Lower level blocks remain the same and additional logic on top can be added to handle this sort of thing. Are you thinking like users.conf? I thought you guys wanted that to die a horrible death. :) Seriously though, are you thinking along the lines of a new composite pjsip configuration object that creates the base objects behind the scenes? If so, that'd solve a lot and I could start working on it right now. I just thought you guys were shying away from these types of things. As base objects it's a bad idea. As a single object to rule them all (a user) it's also a complicated/bad idea. As higher level concepts which represent things that people are familiar with they're fine. -- Joshua Colp Digium, Inc. | Senior Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - US Check us out at: www.digium.com www.asterisk.org -- _ -- 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] Opinions Needed: PJSIP Outboud Registration with multiple server_uris
George Joseph wrote: snip Or separate objects from a config file perspective but implemented in pjsip_configuration with endpoint. Completely separate. Mixing the two defeats the purpose of having a clear boundary. -- Joshua Colp Digium, Inc. | Senior Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - US Check us out at: www.digium.com www.asterisk.org -- _ -- 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] Opinions Needed: PJSIP Outboud Registration with multiple server_uris
George Joseph wrote: On Sat, Sep 20, 2014 at 1:10 PM, Joshua Colp jc...@digium.com mailto:jc...@digium.com wrote: George Joseph wrote: snip Or separate objects from a config file perspective but implemented in pjsip_configuration with endpoint. Completely separate. Mixing the two defeats the purpose of having a clear boundary. Ok, how about this... 2 new object types called composite and pattern (or whatever) implemented in a separate res_pjsip_* module [mytrunk] type = composite pattern = trunk etc... [trunk] type = pattern register = yes contacts = static outbound_auth = yes inbound_auth = no identify = yes I don't understand the naming or what they mean at first glance ^_^ -- Joshua Colp Digium, Inc. | Senior Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - US Check us out at: www.digium.com www.asterisk.org -- _ -- 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] Opinions Needed: PJSIP Outboud Registration with multiple server_uris
George Joseph wrote: snip I was thinking that we probably don't want to create hard coded objects called trunk, user, etc. Instead let the user define the patterns that suit them. It would be much more approachable for a user with specific types. These types result in underlying endpoint, identify, outbound registrations, etc existing but that fact is an underlying detail they don't worry or care about. -- Joshua Colp Digium, Inc. | Senior Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - US Check us out at: www.digium.com www.asterisk.org -- _ -- 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] Opinions Needed: PJSIP Outboud Registration with multiple server_uris
George Joseph wrote: On Sat, Sep 20, 2014 at 2:20 PM, Joshua Colp jc...@digium.com mailto:jc...@digium.com wrote: George Joseph wrote: snip I was thinking that we probably don't want to create hard coded objects called trunk, user, etc. Instead let the user define the patterns that suit them. It would be much more approachable for a user with specific types. Is this agreement on letting the user define the patterns (with samples provides) or are you saying we should hardcode types? There are enough variations in the patterns that I don't think we could create viable 'trunk', 'user', etc. objects. I'd make this a separate config (pjsip_express.conf or something) with a default set of pattern definitions. I'm saying for making it easier to configure PJSIP for users there could be hardcoded types which represent the common types that users need. If more control is required than the lower level detailed ones can be used. It is certainly possible to have 'phone' and 'trunk' types which are useful for a good percentage. Your pattern idea I would say is an alternative way for doing it, but is still more complicated than distinct types and requires explanation. Given the following (even without documentation) could someone coming from sip.conf understand it? [1000] type=phone secret=notverysecret context=trusted disallow=all allow=g722 mailbox=1000 I err on the side of yes. That's what I think is needed. Heck, it's hard enough to get people to realize they can use templates. -- Joshua Colp Digium, Inc. | Senior Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - US Check us out at: www.digium.com www.asterisk.org -- _ -- 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] Opinions Needed: PJSIP Outboud Registration with multiple server_uris
George Joseph wrote: snip How about we use the pattern approach but compile in patterns for trunk and user. There are lots of minor differences between ITSPs and phones and I just worry that in the quest to create something for everyone we create something that's useful to no one. If it does not impact any of the existing code and is easy for a user, then sure. That being said... get feedback any way you can before doing anything. This is a complicated area. Given the following (even without documentation) could someone coming from sip.conf understand it? [1000] type=phone secret=notverysecret context=trusted disallow=all allow=g722 mailbox=1000 I err on the side of yes. That's what I think is needed. Heck, it's hard enough to get people to realize they can use templates. I love templates so much that I enhanced manager and config so you read and write templates via AMI GetConfig and UpdateConfig. If we compile in basic patterns it could be as simple as [1000] type = composite ; ok, maybe composite and pattern aren't good names. pattern = phone ; built-in pattern incoming_password = notverysecret context = trusted disallow = all allow = g722 mailboxes = 1000 Are you OK with a separate config file? It would make manipulating it easier since there'd be no duplicate section names. Yes. -- Joshua Colp Digium, Inc. | Senior Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - US Check us out at: www.digium.com www.asterisk.org -- _ -- 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] beta2 compile failure
Paul Albrecht wrote: Asterisk 13 beta2 compile fails: . . . [CC] chan_pjsip.c - chan_pjsip.o [CC] pjsip/dialplan_functions.c - pjsip/dialplan_functions.o [LD] chan_pjsip.o pjsip/dialplan_functions.o - chan_pjsip.so /usr/lib/gcc/x86_64-pc-linux-gnu/4.5.3/../../../../x86_64-pc-linux-gnu/bin/ld: /usr/lib/gcc/x86_64-pc-linux-gnu/4.5.3/../../../../lib64/libpjsip-ua-x86_64-pc-linux-gnu.a(sip_inv.o): relocation R_X86_64_32S against `.rodata' can not be used when making a shared object; recompile with -fPIC /usr/lib/gcc/x86_64-pc-linux-gnu/4.5.3/../../../../lib64/libpjsip-ua-x86_64-pc-linux-gnu.a: could not read symbols: Bad value collect2: ld returned 1 exit status make[1]: *** [chan_pjsip.so] Error 1 make: *** [channels] Error 2 Your environment does not seem have a suitable pjproject. It should not be trying to link in any static. Did you follow the wiki to build pjproject? Do you have an old install as well as a new? -- Joshua Colp Digium, Inc. | Senior Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - US Check us out at: www.digium.com www.asterisk.org -- _ -- 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.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3954/#review13378 --- Ship it! Ship It! - Joshua Colp On Sept. 19, 2014, 10:02 p.m., rmudgett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3954/ --- (Updated Sept. 19, 2014, 10:02 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24295 https://issues.asterisk.org/jira/browse/ASTERISK-24295 Repository: Asterisk Description --- The crash on the issues is a result of an invalid transport configuration change when asterisk is restarted. The attempt to send the qualify request fails and we cleaned up. However, the callback is also called which results in a double unref of the objects involved. * Put a wrapper around pjsip_endpt_send_request() to detect when the passed in callback is called because of an error so callers can know to cleanup. * Made send_request_cb() able to handle repeated challenges (Up to 10). * Fix periodic endpoint qualify OPTIONS sched deletion race by avoiding it. The sched entry will no longer self stop and must be externally stopped. * Added REF_DEBUG description tags to struct sched_data in pjsip_options.c. * Fix some off-nominal ref leaks in schedule_qualify(), qualify_and_schedule(). * Reordered pjsip_options.c module start/stop code to cleanup better on error. Diffs - /branches/13/res/res_pjsip/pjsip_options.c 423616 /branches/13/res/res_pjsip.c 423616 Diff: https://reviewboard.asterisk.org/r/3954/diff/ Testing --- * With the qualify_frequency option enabled, added and removed a local_net= line in the transport section and restarted asterisk via core restart now. Before the latest patch version, asterisk 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 -- _ -- 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] 4016: chan_sip: Unref outbound proxy structure on dialog(pvt) struct
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4016/#review13379 --- /branches/1.8/channels/chan_sip.c https://reviewboard.asterisk.org/r/4016/#comment23871 This is actually incorrect. p-outboundproxy may point to a statically allocated outbound proxy. To ensure both cases are met use ref_proxy to set it to NULL. This will unref if it should and otherwise not unref. - Joshua Colp On Sept. 22, 2014, 8:30 p.m., wdoekes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4016/ --- (Updated Sept. 22, 2014, 8:30 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24063 https://issues.asterisk.org/jira/browse/ASTERISK-24063 Repository: Asterisk Description --- In ASTERISK-24063 Damian Ivereigh wants to add outboundproxy support to OPTIONS qualify requests. The addition clearly shows that dialog-set proxies were not unreffed, creating a (very slow) memory leak if proxies are frequently changed. This should fix things. Diffs - /branches/1.8/channels/chan_sip.c 423724 Diff: https://reviewboard.asterisk.org/r/4016/diff/ Testing --- Tested with the patch from: https://reviewboard.asterisk.org/r/3948/ because that patch makes sure a proxy is reffed for every OPTIONS. But without that patch, the problem applies to most other dialog creation as well (calling, mwi, ...). sip.conf: [general] [whoever] type=peer host=127.0.0.1 port=5062 qualify=yes outboundproxy=127.0.0.1:5065 Without this patch, 'memory atexit list' listed proxy_from_config(), with this patch, the atexit list was free of any chan_sip related stuff. Thanks, wdoekes -- _ -- 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] 4016: chan_sip: Unref outbound proxy structure on dialog(pvt) struct
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4016/#review13380 --- Ship it! Ship It! - Joshua Colp On Sept. 23, 2014, 12:18 p.m., wdoekes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4016/ --- (Updated Sept. 23, 2014, 12:18 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24063 https://issues.asterisk.org/jira/browse/ASTERISK-24063 Repository: Asterisk Description --- In ASTERISK-24063 Damian Ivereigh wants to add outboundproxy support to OPTIONS qualify requests. The addition clearly shows that dialog-set proxies were not unreffed, creating a (very slow) memory leak if proxies are frequently changed. This should fix things. Diffs - /branches/1.8/channels/chan_sip.c 423782 Diff: https://reviewboard.asterisk.org/r/4016/diff/ Testing --- Tested with the patch from: https://reviewboard.asterisk.org/r/3948/ because that patch makes sure a proxy is reffed for every OPTIONS. But without that patch, the problem applies to most other dialog creation as well (calling, mwi, ...). sip.conf: [general] [whoever] type=peer host=127.0.0.1 port=5062 qualify=yes outboundproxy=127.0.0.1:5065 Without this patch, 'memory atexit list' listed proxy_from_config(), with this patch, the atexit list was free of any chan_sip related stuff. Thanks, wdoekes -- _ -- 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] beta2 compile failure
Paul Albrecht wrote: On Sep 22, 2014, at 3:47 PM, Joshua Colp jc...@digium.com mailto:jc...@digium.com wrote: Paul Albrecht wrote: Asterisk 13 beta2 compile fails: . . . [CC] chan_pjsip.c - chan_pjsip.o [CC] pjsip/dialplan_functions.c - pjsip/dialplan_functions.o [LD] chan_pjsip.o pjsip/dialplan_functions.o - chan_pjsip.so /usr/lib/gcc/x86_64-pc-linux-gnu/4.5.3/../../../../x86_64-pc-linux-gnu/bin/ld: /usr/lib/gcc/x86_64-pc-linux-gnu/4.5.3/../../../../lib64/libpjsip-ua-x86_64-pc-linux-gnu.a(sip_inv.o): relocation R_X86_64_32S against `.rodata' can not be used when making a shared object; recompile with -fPIC /usr/lib/gcc/x86_64-pc-linux-gnu/4.5.3/../../../../lib64/libpjsip-ua-x86_64-pc-linux-gnu.a: could not read symbols: Bad value collect2: ld returned 1 exit status make[1]: *** [chan_pjsip.so] Error 1 make: *** [channels] Error 2 Your environment does not seem have a suitable pjproject. It should not be trying to link in any static. Did you follow the wiki to build pjproject? Do you have an old install as well as a new? Oops, my bad, I didn’t setup pjproject. However, the configure script didn’t complain so I assumed I was good to go and proceeded to make asterisk. Shouldn’t the configure script catch this and not allow someone to proceed to make? Thought that was a function of the configure script, that is, it verifies software dependancies before the build step. The configure script checks for pjproject but it is not currently specific enough to look for only shared. (Not sure off the top of my head how we could make it) Another thing … the README int the top level directory doesn’t seem to have been updated in a while and doesn’t mention pjproject. If there’s an external requirement like pjproject shouldn’t that go in the README? These days almost everything is documented on the wiki. The README itself only covers the basics to get Asterisk up and going. It could be extended to include dependencies for the various things. -- Joshua Colp Digium, Inc. | Senior Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - US Check us out at: www.digium.com www.asterisk.org -- _ -- 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] 4008: res_pjsip_session: Add additional checks to prevent session refresh in unpossible states.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4008/ --- (Updated Sept. 27, 2014, 7:42 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers and Mark Michelson. Changes --- Committed in revision 424056 Repository: Asterisk Description --- Currently it is possible for ast_sip_session_refresh to be called at times where the state of the dialog and INVITE session does not allow it to send a request. Trying to send a request actually results in an assertion within PJSIP. This change adds additional checks for deferral of these, stops generating new SDP on COLP UPDATEs, makes it so deferral does not always result in SDP generation, and ensures that after a provisional response that any pending UPDATE occurs. * Note: Currently there is still a bug within pjproject which causes an UPDATE without SDP sent after a provisional response to cancel the pending SDP negotiation when it should not. This has been reported to Teluu and a fix is being worked on. Diffs - /branches/12/res/res_pjsip_session.c 423546 /branches/12/channels/chan_pjsip.c 423546 Diff: https://reviewboard.asterisk.org/r/4008/diff/ Testing --- Modified the dialing API to change callerID at certain points (after call but before handling responses, after handling responses). Confirmed that new code correctly defers sending COLP updates. 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] 4032: PJSIP: Force transport on contact rewrite
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4032/#review13400 --- branches/12/res/res_pjsip_nat.c https://reviewboard.asterisk.org/r/4032/#comment23885 What will this do for UDP transport? For those the transport should be empty. - Joshua Colp On Sept. 29, 2014, 7:25 p.m., opticron wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4032/ --- (Updated Sept. 29, 2014, 7:25 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- If contact rewriting is enabled but the contact differs in transport from what is actually being used, messages after the initial INVITE transaction can be sent to an incorrect transport/port combination. In the case where this bug occurred the remote party never received a BYE since it was sent to the remote party's TCP port over UDP. Diffs - branches/12/res/res_pjsip_nat.c 424094 Diff: https://reviewboard.asterisk.org/r/4032/diff/ Testing --- Ensured that this patch allowed the BYE to be sent properly. Thanks, opticron -- _ -- 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] 4032: PJSIP: Force transport on contact rewrite
On Sept. 29, 2014, 7:47 p.m., Joshua Colp wrote: branches/12/res/res_pjsip_nat.c, line 49 https://reviewboard.asterisk.org/r/4032/diff/1/?file=67680#file67680line49 What will this do for UDP transport? For those the transport should be empty. opticron wrote: udp is also a valid transport along with tls, ws, and others. Is there a benefit to leaving it blank if the transport is UDP? Makes the message smaller? (I'm on a smaller message kick, it seems) Otherwise drop. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4032/#review13400 --- On Sept. 29, 2014, 7:25 p.m., opticron wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4032/ --- (Updated Sept. 29, 2014, 7:25 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- If contact rewriting is enabled but the contact differs in transport from what is actually being used, messages after the initial INVITE transaction can be sent to an incorrect transport/port combination. In the case where this bug occurred the remote party never received a BYE since it was sent to the remote party's TCP port over UDP. Diffs - branches/12/res/res_pjsip_nat.c 424094 Diff: https://reviewboard.asterisk.org/r/4032/diff/ Testing --- Ensured that this patch allowed the BYE to be sent properly. Thanks, opticron -- _ -- 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] PJSIP Configuration Wizard (Was: Opinions Needed: PJSIP Outboud Registration with multiple server_uris)
I'm down with proposal #2 but I have to ask: would this work with realtime, and how? -- Joshua Colp Digium, Inc. | Senior Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - US Check us out at: www.digium.com www.asterisk.org -- _ -- 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] 4039: res_rtp_asterisk: Crash if no candidates received for component
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4039/#review13462 --- I don't think this goes far enough. What if I receive no candidates for RTCP? It would still crash. - Joshua Colp On Oct. 1, 2014, 3:43 p.m., Kevin Harwell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4039/ --- (Updated Oct. 1, 2014, 3:43 p.m.) Review request for Asterisk Developers and Joshua Colp. Bugs: ASTERISK-24383 https://issues.asterisk.org/jira/browse/ASTERISK-24383 Repository: Asterisk Description --- When starting ice if there is not at least one remote ice candidate with an RTP component asterisk will crash. This is due to an assertion in pjnath as it expects at least one candidate with an RTP component. Added a check to make sure at least one candidate contains an RTP component. Diffs - branches/12/res/res_rtp_asterisk.c 424285 Diff: https://reviewboard.asterisk.org/r/4039/diff/ Testing --- Sent asterisk a list of ice candidates containing only RTCP components (no RTP ones) and observed the crash. After applying the patch ran the scenario again and it did not crash. 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
[asterisk-dev] [Code Review] 4057: bridge: Every time a bridge lies during the smart bridge operation I cry
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4057/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- Right now when a smart bridge operation occurs the bridge provided to the old technology is full of lies. This has the result of making technology implementations more complex because they can't assume the bridge is truthful, resulting in them storing more state than is needed or having additional code paths. The attached change makes the bridge provided to the old technology truthful. The bridge will contain all the channels that are actually in the old bridge instead of none. As a result the bridge_native_rtp module can be made a bit dumber and slimmed down. This makes the smart bridge operation, to the old technology, a series of normal leave calls. Diffs - /branches/12/main/bridge.c 424413 /branches/12/bridges/bridge_native_rtp.c 424413 Diff: https://reviewboard.asterisk.org/r/4057/diff/ Testing --- Performed calls and ran tests. 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] 4039: res_rtp_asterisk: Crash if no candidates received for component
On Oct. 7, 2014, 6:45 p.m., Joshua Colp wrote: I don't think this goes far enough. What if I receive no candidates for RTCP? It would still crash. Kevin Harwell wrote: I tested for this. The only time it crashes is when there are no RTP candidates in the list. If there are no RTCP candidates in the list it does not crash. I still think we should do it as that's relying on undocumented behavior. In fact the documentation says the opposite - that there must be at least one candidate per component. If that becomes true we are hosed. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4039/#review13462 --- On Oct. 1, 2014, 3:43 p.m., Kevin Harwell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4039/ --- (Updated Oct. 1, 2014, 3:43 p.m.) Review request for Asterisk Developers and Joshua Colp. Bugs: ASTERISK-24383 https://issues.asterisk.org/jira/browse/ASTERISK-24383 Repository: Asterisk Description --- When starting ice if there is not at least one remote ice candidate with an RTP component asterisk will crash. This is due to an assertion in pjnath as it expects at least one candidate with an RTP component. Added a check to make sure at least one candidate contains an RTP component. Diffs - branches/12/res/res_rtp_asterisk.c 424285 Diff: https://reviewboard.asterisk.org/r/4039/diff/ Testing --- Sent asterisk a list of ice candidates containing only RTCP components (no RTP ones) and observed the crash. After applying the patch ran the scenario again and it did not crash. 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] 4039: res_rtp_asterisk: Crash if no candidates received for component
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4039/#review13475 --- Ship it! Ship It! - Joshua Colp On Oct. 9, 2014, 3:44 p.m., Kevin Harwell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4039/ --- (Updated Oct. 9, 2014, 3:44 p.m.) Review request for Asterisk Developers and Joshua Colp. Bugs: ASTERISK-24383 https://issues.asterisk.org/jira/browse/ASTERISK-24383 Repository: Asterisk Description --- When starting ice if there is not at least one remote ice candidate with an RTP component asterisk will crash. This is due to an assertion in pjnath as it expects at least one candidate with an RTP component. Added a check to make sure at least one candidate contains an RTP component. Diffs - branches/12/res/res_rtp_asterisk.c 424940 Diff: https://reviewboard.asterisk.org/r/4039/diff/ Testing --- Sent asterisk a list of ice candidates containing only RTCP components (no RTP ones) and observed the crash. After applying the patch ran the scenario again and it did not crash. 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] 4057: bridge: Every time a bridge lies during the smart bridge operation I cry
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4057/ --- (Updated Oct. 9, 2014, 6:16 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- Right now when a smart bridge operation occurs the bridge provided to the old technology is full of lies. This has the result of making technology implementations more complex because they can't assume the bridge is truthful, resulting in them storing more state than is needed or having additional code paths. The attached change makes the bridge provided to the old technology truthful. The bridge will contain all the channels that are actually in the old bridge instead of none. As a result the bridge_native_rtp module can be made a bit dumber and slimmed down. This makes the smart bridge operation, to the old technology, a series of normal leave calls. Diffs (updated) - /branches/12/main/bridge.c 424850 /branches/12/bridges/bridge_native_rtp.c 424850 Diff: https://reviewboard.asterisk.org/r/4057/diff/ Testing --- Performed calls and ran tests. 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] 2964: res_pjsip_outbound_registration: Add virtual line support for automatic inbound matching
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/2964/ --- (Updated Oct. 10, 2014, 1:18 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- This patch adds virtual line support to the res_pjsip_outbound_registration module. This is an optional feature and simply adds a line URI parameter to the Contact we place in the outbound registration. If this line parameter is present on incoming requests we use it to establish a relationship to the outbound registration and match it to a user configured endpoint. This has the benefit that when registering to another server where it is supported you no longer have to do IP based matching for all of their potential servers. The downside (and why this is optional) is that if a third party got the line parameter they could send you calls as if they were the legit remote server. Diffs (updated) - /trunk/res/res_pjsip_outbound_registration.c 425156 /trunk/configs/samples/pjsip.conf.sample 425156 Diff: https://reviewboard.asterisk.org/r/2964/diff/ Testing --- Registered to an ITSP, placed an inbound call from them, confirmed matched using line parameter. Registered to a chan_sip instance, placed an inbound call from it, confirmed matched using line parameter. 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] 3992: res_pjsip_sdp_rtp: Add optimistic SRTP support.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3992/ --- (Updated Oct. 10, 2014, 8:13 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- When enabling SRTP support in PJSIP it is either forced on or disabled. This means that if you specify SRTP but the client does not support it the session will fail. For situations where this guarantee is not required this new functionality can be used to optimistically use SRTP if possible. This has the added benefit of encrypting the media when possible but does not guarantee it. This also fixes an issue where a client may offer SRTP using the normal transport but we reject it. Diffs (updated) - /trunk/res/res_pjsip_session.c 425156 /trunk/res/res_pjsip_sdp_rtp.c 425156 /trunk/res/res_pjsip/pjsip_configuration.c 425156 /trunk/res/res_pjsip.c 425156 /trunk/include/asterisk/res_pjsip_session.h 425156 /trunk/include/asterisk/res_pjsip.h 425156 /trunk/configs/samples/pjsip.conf.sample 425156 Diff: https://reviewboard.asterisk.org/r/3992/diff/ Testing --- Used Blink to place calls with optimistic enabled and disabled on the PJSIP side. In Blink I alternated between disabled/mandatory/optional. Confirmed that for each scenario the expected outcome occurred. Blink Asterisk Result Disabled Optimistic Off Failed Disabled Optimistic On Success (Not encrypted) Mandatory Optimistic Off Success (Encrypted) Mandatory Optimistic On Success (Encrypted) Optional Optimistic Off Success (Encrypted) Optional Optimistic On Success (Encrypted) 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] 4057: bridge: Every time a bridge lies during the smart bridge operation I cry
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4057/ --- (Updated Oct. 10, 2014, 3:47 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 425242 Repository: Asterisk Description --- Right now when a smart bridge operation occurs the bridge provided to the old technology is full of lies. This has the result of making technology implementations more complex because they can't assume the bridge is truthful, resulting in them storing more state than is needed or having additional code paths. The attached change makes the bridge provided to the old technology truthful. The bridge will contain all the channels that are actually in the old bridge instead of none. As a result the bridge_native_rtp module can be made a bit dumber and slimmed down. This makes the smart bridge operation, to the old technology, a series of normal leave calls. Diffs - /branches/12/main/bridge.c 424850 /branches/12/bridges/bridge_native_rtp.c 424850 Diff: https://reviewboard.asterisk.org/r/4057/diff/ Testing --- Performed calls and ran tests. 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] License issue
Russell Bryant wrote: It should be fine. Users have never been able to use reviewboard without an active contributor license agreement. (AFAIK) In the past this was manually done but these days it's automatic (yay automatic!). The gist being like Russell said: If a patch is on reviewboard then the person had a valid license agreement at the time it was uploaded. Cheers, -- Joshua Colp Digium, Inc. | Senior Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - US Check us out at: www.digium.com www.asterisk.org -- _ -- 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] 4073: res_pjsip: Add 'user_eq_phone' option for placing 'user=phone' parameter in request URI if user is number.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4073/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- This change adds a 'user_eq_phone' option to endpoints which, if set, will add a 'user=phone' parameter to the request URI if the user is determined to be a number. Diffs - /trunk/res/res_pjsip/pjsip_configuration.c 425395 /trunk/res/res_pjsip.c 425395 /trunk/include/asterisk/res_pjsip.h 425395 Diff: https://reviewboard.asterisk.org/r/4073/diff/ Testing --- Sent outgoing calls with various users (numbers, number+letters, letters) and confirmed that user=phone was set when it should be. 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] 4073: res_pjsip: Add 'user_eq_phone' option for placing 'user=phone' parameter in request URI if user is number.
On Oct. 13, 2014, 5:33 p.m., Matt Jordan wrote: /trunk/res/res_pjsip/pjsip_configuration.c, line 1735 https://reviewboard.asterisk.org/r/4073/diff/1/?file=68003#file68003line1735 Alembic! Thinking about it, we also need to update Alembic with your optimistic encryption patch. Also: UPGRADE notes. Wouldn't this belong in CHANGES? This isn't something that must be done when upgrading. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4073/#review13498 --- On Oct. 13, 2014, 4:54 p.m., Joshua Colp wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4073/ --- (Updated Oct. 13, 2014, 4:54 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- This change adds a 'user_eq_phone' option to endpoints which, if set, will add a 'user=phone' parameter to the request URI if the user is determined to be a number. Diffs - /trunk/res/res_pjsip/pjsip_configuration.c 425395 /trunk/res/res_pjsip.c 425395 /trunk/include/asterisk/res_pjsip.h 425395 Diff: https://reviewboard.asterisk.org/r/4073/diff/ Testing --- Sent outgoing calls with various users (numbers, number+letters, letters) and confirmed that user=phone was set when it should be. 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] 4073: res_pjsip: Add 'user_eq_phone' option for placing 'user=phone' parameter in request URI if user is number.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4073/ --- (Updated Oct. 13, 2014, 6:09 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- This change adds a 'user_eq_phone' option to endpoints which, if set, will add a 'user=phone' parameter to the request URI if the user is determined to be a number. Diffs (updated) - /trunk/res/res_pjsip_caller_id.c 425395 /trunk/res/res_pjsip/pjsip_configuration.c 425395 /trunk/res/res_pjsip.c 425395 /trunk/include/asterisk/res_pjsip.h 425395 /trunk/contrib/ast-db-manage/config/versions/371a3bf4143e_add_user_eq_phone_option_to_pjsip.py PRE-CREATION /trunk/CHANGES 425395 Diff: https://reviewboard.asterisk.org/r/4073/diff/ Testing --- Sent outgoing calls with various users (numbers, number+letters, letters) and confirmed that user=phone was set when it should be. 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] 3997: bridge_native_rtp: Fix odd audio issues when transitioning from native remote RTP bridge to softmix
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3997/#review13505 --- /branches/12/bridges/bridge_native_rtp.c https://reviewboard.asterisk.org/r/3997/#comment24024 To further elaborate: 1. There will always be at least one channel in the bridge when any of this is called 2. There will never be more than two channels in the bridge when any of this is called. Yay constraints which make sense! - Joshua Colp On Oct. 13, 2014, 5:32 p.m., Matt Jordan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3997/ --- (Updated Oct. 13, 2014, 5:32 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24327 https://issues.asterisk.org/jira/browse/ASTERISK-24327 Repository: Asterisk Description --- When a native RTP bridge that is remotely bridging its participants switches to a softmix bridge, it may not properly re-INVITE the media for one or both participants back to Asterisk. This is due to two factors: (1) The current bridge_native_rtp code only re-INVITEs if it believes the channel will survive the bridge operation. Currently, that code is failing, as it expects the channels to have a soft hangup flag set on it indicating that a redirect has occurred or that the channel is going to leave the bridge. (The code did not take into account a smart bridge operation). (2) When the bridge layer performs a smart bridge, it passes a dummy bridge down into the old mixing technology when it is stopped. That breaks the native RTP bridge, as it looks to bridge-channels to know which channels to re-INVITE back. That list has no entries, as the dummy bridge does not populate that value. This patch modifies bridge_native_rtp such that it keeps track of the channels itself. Given how tricky this code is - both smart bridging and native RTP bridging - this keeps the mixing technology insulated from changes in the core, which is probably a good thing. Diffs - /branches/12/bridges/bridge_native_rtp.c 425404 Diff: https://reviewboard.asterisk.org/r/3997/diff/ Testing --- The tests that extercised this code the most are the PJSIP blind transfer tests, as they change the bridge mixing technology from native_rtp to simple and back in various tests. Shocking the callee_with_hold/caller_with_hold tests worked right off the bat. The direct media tests still fail, but this is not surprising as the messages from Asterisk arrive interleaved, which is not something SIPp handles well (at all). Thanks, Matt Jordan -- _ -- 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] 3997: bridge_native_rtp: Fix odd audio issues when transitioning from native remote RTP bridge to softmix
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3997/#review13518 --- /branches/12/bridges/bridge_native_rtp.c https://reviewboard.asterisk.org/r/3997/#comment24038 No, AST_LIST_FIRST and AST_LIST_LAST would return the same thing since it's a linked list containing one channel. - Joshua Colp On Oct. 13, 2014, 5:32 p.m., Matt Jordan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3997/ --- (Updated Oct. 13, 2014, 5:32 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24327 https://issues.asterisk.org/jira/browse/ASTERISK-24327 Repository: Asterisk Description --- When a native RTP bridge that is remotely bridging its participants switches to a softmix bridge, it may not properly re-INVITE the media for one or both participants back to Asterisk. This is due to two factors: (1) The current bridge_native_rtp code only re-INVITEs if it believes the channel will survive the bridge operation. Currently, that code is failing, as it expects the channels to have a soft hangup flag set on it indicating that a redirect has occurred or that the channel is going to leave the bridge. (The code did not take into account a smart bridge operation). (2) When the bridge layer performs a smart bridge, it passes a dummy bridge down into the old mixing technology when it is stopped. That breaks the native RTP bridge, as it looks to bridge-channels to know which channels to re-INVITE back. That list has no entries, as the dummy bridge does not populate that value. This patch modifies bridge_native_rtp such that it keeps track of the channels itself. Given how tricky this code is - both smart bridging and native RTP bridging - this keeps the mixing technology insulated from changes in the core, which is probably a good thing. Diffs - /branches/12/bridges/bridge_native_rtp.c 425404 Diff: https://reviewboard.asterisk.org/r/3997/diff/ Testing --- The tests that extercised this code the most are the PJSIP blind transfer tests, as they change the bridge mixing technology from native_rtp to simple and back in various tests. Shocking the callee_with_hold/caller_with_hold tests worked right off the bat. The direct media tests still fail, but this is not surprising as the messages from Asterisk arrive interleaved, which is not something SIPp handles well (at all). Thanks, Matt Jordan -- _ -- 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] 4084: res_pjsip_keepalive: Add keepalive module for connection-oriented transports.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4084/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- Reusing connections is a good thing(tm). In the case of NAT it means that you have an actual way to exchange data back and forth. In practice, however, some things (such as PJSIP) close down the TCP connection after a short period of time (in the case of PJSIP for UAC it's 33 seconds). While PJSIP has a built in keepalive mechanism this is by default set to 90 seconds and can only be controlled at compile time. The attached module implements its own keepalive which is configurable at runtime and does not require configuration. This sends a lightweight keepalive to keep the TCP (or TLS) connection open. Diffs - /trunk/res/res_pjsip_keepalive.c PRE-CREATION /trunk/configs/samples/pjsip.conf.sample 425216 Diff: https://reviewboard.asterisk.org/r/4084/diff/ Testing --- Configured keepalives and used wireshark to verify that at the specific interval the message went out. 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] 3997: bridge_native_rtp: Fix odd audio issues when transitioning from native remote RTP bridge to softmix
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3997/#review13530 --- /branches/12/bridges/bridge_native_rtp.c https://reviewboard.asterisk.org/r/3997/#comment24060 glue1 will always exist so this can just become an else - Joshua Colp On Oct. 14, 2014, 7:11 p.m., Matt Jordan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3997/ --- (Updated Oct. 14, 2014, 7:11 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24327 https://issues.asterisk.org/jira/browse/ASTERISK-24327 Repository: Asterisk Description --- When a native RTP bridge that is remotely bridging its participants switches to a softmix bridge, it may not properly re-INVITE the media for one or both participants back to Asterisk. This is due to two factors: (1) The current bridge_native_rtp code only re-INVITEs if it believes the channel will survive the bridge operation. Currently, that code is failing, as it expects the channels to have a soft hangup flag set on it indicating that a redirect has occurred or that the channel is going to leave the bridge. (The code did not take into account a smart bridge operation). (2) When the bridge layer performs a smart bridge, it passes a dummy bridge down into the old mixing technology when it is stopped. That breaks the native RTP bridge, as it looks to bridge-channels to know which channels to re-INVITE back. That list has no entries, as the dummy bridge does not populate that value. This patch modifies bridge_native_rtp such that it keeps track of the channels itself. Given how tricky this code is - both smart bridging and native RTP bridging - this keeps the mixing technology insulated from changes in the core, which is probably a good thing. Diffs - /branches/12/bridges/bridge_native_rtp.c 425504 Diff: https://reviewboard.asterisk.org/r/3997/diff/ Testing --- The tests that extercised this code the most are the PJSIP blind transfer tests, as they change the bridge mixing technology from native_rtp to simple and back in various tests. Shocking the callee_with_hold/caller_with_hold tests worked right off the bat. The direct media tests still fail, but this is not surprising as the messages from Asterisk arrive interleaved, which is not something SIPp handles well (at all). Thanks, Matt Jordan -- _ -- 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] 3997: bridge_native_rtp: Fix odd audio issues when transitioning from native remote RTP bridge to softmix
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3997/#review13531 --- Ship it! Otherwise g2g - Joshua Colp On Oct. 14, 2014, 7:11 p.m., Matt Jordan wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3997/ --- (Updated Oct. 14, 2014, 7:11 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24327 https://issues.asterisk.org/jira/browse/ASTERISK-24327 Repository: Asterisk Description --- When a native RTP bridge that is remotely bridging its participants switches to a softmix bridge, it may not properly re-INVITE the media for one or both participants back to Asterisk. This is due to two factors: (1) The current bridge_native_rtp code only re-INVITEs if it believes the channel will survive the bridge operation. Currently, that code is failing, as it expects the channels to have a soft hangup flag set on it indicating that a redirect has occurred or that the channel is going to leave the bridge. (The code did not take into account a smart bridge operation). (2) When the bridge layer performs a smart bridge, it passes a dummy bridge down into the old mixing technology when it is stopped. That breaks the native RTP bridge, as it looks to bridge-channels to know which channels to re-INVITE back. That list has no entries, as the dummy bridge does not populate that value. This patch modifies bridge_native_rtp such that it keeps track of the channels itself. Given how tricky this code is - both smart bridging and native RTP bridging - this keeps the mixing technology insulated from changes in the core, which is probably a good thing. Diffs - /branches/12/bridges/bridge_native_rtp.c 425504 Diff: https://reviewboard.asterisk.org/r/3997/diff/ Testing --- The tests that extercised this code the most are the PJSIP blind transfer tests, as they change the bridge mixing technology from native_rtp to simple and back in various tests. Shocking the callee_with_hold/caller_with_hold tests worked right off the bat. The direct media tests still fail, but this is not surprising as the messages from Asterisk arrive interleaved, which is not something SIPp handles well (at all). Thanks, Matt Jordan -- _ -- 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