Re: [asterisk-dev] [Code Review] 3349: Implement RFC-3966 TEL URI incoming INVITE
On March 22, 2014, 4:39 p.m., Olle E Johansson wrote:
I don't see what happens with the phone-context argument. Shouldn't we pass
that on as a channel variable?
Geert Van Pamel wrote:
We return this into the hostport.
Geert Van Pamel wrote:
According to RFC 3966 phone-context is either a domain-name, or (part of)
an international telephone number (indicated with +prefix).
It is used by a gateway to know how to dial the local number... the
local number must be unique within its context...
Olle E Johansson wrote:
So it ends up in the SIPDOMAIN variable in the dial plan? It has to be
reachable in the dial plan somehow.
Geert Van Pamel wrote:
The variable ${SIPDOMAIN} contains the local IP address of the Asterisk
server.
The userinfo arrives in ${CALLERID} and is displayed on the display of
the called device, and arrives in the CDR file.
Actually I do not know into which variable the incoming hostport info is
copied to?
Could somebody else answer this question?
Olle E Johansson wrote:
If I place a normal call to sip:ge...@example.com to my Asterisk server.
geert will be the extension I'm looking for, example.com ends up in
SIPDOMAIN. It's not the local IP address, it's the domain/host part of the
request URI in the INVITE.
I would prefer if phone context ended up in TELPHONECONTEXT so I could
use it the same way as SIPDOMAIN in the dial plan. It should not end up in
SIPDOMAIN as it is not a SIP uri. That way an extension in a local context
can be routed differently than an extension in a global context.
Olle E Johansson wrote:
Just to make sure it's documented: The phone-context should NOT be
matched to a context in the dial plan. From a security standpoint, that would
be horrific. We need the information to be able to route correctly in the
dial plan, but no automatic selection. (I am not suggesting you where going
down that path, Geert. Just wanted to close that way of thinking since the
word context could lead there.)
Geert Van Pamel wrote:
So I understand that Olle proposes to create a new variable
${TELPHONECONTEXT} that contains the TEL URI phone-context which can be
either the global number, or a global number prefix, or the related routing
domain or any other unique routing identifier, or would be empty in case
there is just a local number (as specified in RFC 3966).
Currently this variable is not available yet in Asterisk. In the dialplan
treating incoming calls currently I do not use nor do not need this
information, as the local number in ${CALLERID} is sufficient (for the time
being)... Still this phone context identifier could be needed for subsequent
outgoing calls (return calls, callbacks, etc.).
I agree that ${SIPDOMAIN} will remain reserved for SIP invites, and is
untouched for TEL URI invites.
I perfectly understand that this TEL URI context has nothing to do with
dialplan context.
Who could us further advise how to create the new variable
${TELPHONECONTEXT} ?
Olle E Johansson wrote:
Just grep for SIPDOMAIN in the chan_sip source code :-)
Matt Jordan wrote:
So I'd hate for this patch to stall out on the channel variable - which
would be a very helpful addition but is not strictly necessary for Geert's
functionality.
Olle - what do you think if this goes in as is, and we add the
TELPHONECONTEXT channel variable in a subsequent patch? It should be
relatively trivial, and I don't mind helping Geert with that work (so long as
Geert is willing to test said patch, as I don't have anything that emulates
tel URIs handy).
Geert - what do you think?
Geert Van Pamel wrote:
Matt, I agree with you... the variable ${TELPHONECONTEXT} is indeed not
needed for TEL URI INVITE to work. Actually, I a uncertain how to implement
this variable. If anybody else can code the variable later, I am willing to
test it...
Matt Jordan wrote:
Geert - I can take a look at adding that variable in a subsequent patch.
I'll definitely need you to test it!
I'll confirm with Olle that he's okay with this - if so, we'll get this
committed.
wdoekes wrote:
That's a negative:
14:26 wdoekes whether we can ship it without the TELURICONTEXT var
14:26 @oej Did they get the channel variable?
14:26 wdoekes for another rainy day, it was said
14:28 @oej Why is that? Only sending incomplete information to the
dialplan is not a good implementation.
14:28 @oej It's a simple change, nothing much to discuss. Many examples
in the source code of chan_sip
14:29 @oej I guess I should have discussed this with Matt last week.
14:30 wdoekes not sure it's that trivial, since the info has to get
out of the parser functions: a bit of refactoring
14:31 @oej It can't be that hard. Ignoring it would be bad and an
[asterisk-dev] tcptls.c
Looking at tcptls.c we have a lot of error messages sent to LOG_WARNING and some stuff sent to the verbose channel that should not be there. Do I need to go through reviewboard to just change these to LOG_ERROR or can I just fix this straight into the subversion repo? /O -- _ -- 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] 3437: chan_sip: Add support for a few more 4xx error responses
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3437/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- Adding improved support for 400, 414 and 493 response codes. I would like to add this as a bug fix to 1.8 and up. Diffs - /trunk/channels/chan_sip.c 412166 Diff: https://reviewboard.asterisk.org/r/3437/diff/ Testing --- A lot during interoperability tests. Thanks, Olle E Johansson -- _ -- 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] 3438: Implement SIP TImer C in Asterisk
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3438/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- SIP Timer C is defined for proxys that forward messages. In some ways, we forward calls. It is activated when we receive a 100 trying and wait for any other message. If that's not received, timer C triggers and cancels the call attempt. This is required in an interoperability test I'm working with. Red dots will be handled in the way they deserve. Diffs - /trunk/configs/sip.conf.sample 412166 /trunk/channels/sip/include/sip.h 412166 /trunk/channels/chan_sip.c 412166 Diff: https://reviewboard.asterisk.org/r/3438/diff/ Testing --- Passed interoperability testing with funky test tool. Thanks, Olle E Johansson -- _ -- 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] 3430: [channels/chan_unistim.c]: Improvements and bugfixes to chan_unistim.c
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3430/ --- (Updated April 11, 2014, 10:51 a.m.) Review request for Asterisk Developers. Changes --- Updated following Matt Jordan's comments. Repository: Asterisk Description --- I have been actively testing Unistim devices (i2002 i2004) and have encountered issues with 1) dialtone wrong 2) inability to select DTMF playback duration 3) inability to set date to match than of a Nortel CS1000 4) mute causing playback to be muted as well as microphone 5) Timer displayed in French (Duree) The attached patch fixes as follows: 1) The modulation should not be referenced for tone+tone as it refers to the on-off characteristic I believe - this often resulted in a single tone rather than the multitone as in the UK. 2) I have added the unistim.conf variable dtmf_duration which can select the DTMF playback duration from 0ms to 150ms (0 is off and is the new default) 3) I have enabled the transmission of MonthLabels (in English) which are sent with the date and changed the dateformat variable to accept the values 0-3 as per the UNISTIM standard (2 3 match the previous 1 2 formats). 4) I have enabled the Mute packet and merged it into the previous code. It now only mutes the microphone. (Improvements welcome) 5) Changed Duree to Timer on i2004 display Diffs (updated) - /trunk/configs/unistim.conf.sample 412191 /trunk/channels/chan_unistim.c 412191 /trunk/CHANGES 412191 Diff: https://reviewboard.asterisk.org/r/3430/diff/ Testing --- In use here Thanks, Peter Whisker -- _ -- 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] 3379: ARI: Make bridges/{bridgeId}/play queue sound files if sounds are already playing on the bridge instead of playing them simultaneously as they are called
On April 10, 2014, 1:44 p.m., Mark Michelson wrote: /branches/12/res/ari/resource_bridges.h, lines 282-292 https://reviewboard.asterisk.org/r/3379/diff/5/?file=56855#file56855line282 There's no need to put \brief for one-line doxygen comments. Doxygen automatically treats these as brief summaries. Jonathan Rose wrote: This one's on the automatic documentation generation. We aren't supposed to touch these headers directly. In a separate patch, update the template generation code. - Matt --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3379/#review11552 --- On April 10, 2014, 4:59 p.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3379/ --- (Updated April 10, 2014, 4:59 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-22677 https://issues.asterisk.org/jira/browse/ASTERISK-22677 Repository: Asterisk Description --- Previously, if you played an audio file and then played another before the first finished, the second audio file would start playing immediately as it was called overlapping the previous sound. Apparently people don't like that. This patch changes that behavior so that the sound will be queued at the end of any existing controls if they are running. Diffs - /branches/12/rest-api/api-docs/bridges.json 412087 /branches/12/res/stasis/control.c 412087 /branches/12/res/stasis/control.h 412087 /branches/12/res/res_stasis_playback.c 412087 /branches/12/res/res_stasis.c 412087 /branches/12/res/res_ari_bridges.c 412087 /branches/12/res/ari/resource_channels.c 412087 /branches/12/res/ari/resource_bridges.c 412087 /branches/12/res/ari/resource_bridges.h 412087 /branches/12/include/asterisk/stasis_app.h 412087 /branches/12/CHANGES 412087 Diff: https://reviewboard.asterisk.org/r/3379/diff/ Testing --- Tested for playback channel wrapper leaks, tested to make sure control objects were being destroyed when they fell out of use. Tested playing of a single file. Tested playing of multiple files in a row. Tested playing of multiple files in a row and then after a sequence finished, playing additional files so that new channels would have to be created. Tested playing sounds right as other sounds were concluding. I wasn't able to break it (although I wouldn't be surprised if there is a possible condition where you can grab a control as it is finishing up its queue and then attempting to add a sound to a finished queue causing the playback to fail. I don't think this would break things in a profound way, it just might possibly make one sound fail to queue under extremely unlikely conditions). 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] 3415: bridging: Ensure proper locking during snapshot creation
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3415/ --- (Updated April 11, 2014, 7:35 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 412193 Bugs: ASTERISK-22904 https://issues.asterisk.org/jira/browse/ASTERISK-22904 Repository: Asterisk Description --- While the vast majority of bridge snapshot creation is locked properly, there are currently some instances that are not. This adds the missing locking to ensure bridge state is not malleable during snapshot creation. Diffs - branches/12/tests/test_cel.c 411668 branches/12/res/ari/resource_bridges.c 411668 branches/12/main/bridge_basic.c 411668 branches/12/main/bridge.c 411668 branches/12/include/asterisk/stasis_bridges.h 411668 branches/12/apps/app_confbridge.c 411668 Diff: https://reviewboard.asterisk.org/r/3415/diff/ Testing --- Ran the testsuite to ensure that it did not cause failures. 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] 3349: Implement RFC-3966 TEL URI incoming INVITE
On March 22, 2014, 10:39 a.m., Olle E Johansson wrote:
I don't see what happens with the phone-context argument. Shouldn't we pass
that on as a channel variable?
Geert Van Pamel wrote:
We return this into the hostport.
Geert Van Pamel wrote:
According to RFC 3966 phone-context is either a domain-name, or (part of)
an international telephone number (indicated with +prefix).
It is used by a gateway to know how to dial the local number... the
local number must be unique within its context...
Olle E Johansson wrote:
So it ends up in the SIPDOMAIN variable in the dial plan? It has to be
reachable in the dial plan somehow.
Geert Van Pamel wrote:
The variable ${SIPDOMAIN} contains the local IP address of the Asterisk
server.
The userinfo arrives in ${CALLERID} and is displayed on the display of
the called device, and arrives in the CDR file.
Actually I do not know into which variable the incoming hostport info is
copied to?
Could somebody else answer this question?
Olle E Johansson wrote:
If I place a normal call to sip:ge...@example.com to my Asterisk server.
geert will be the extension I'm looking for, example.com ends up in
SIPDOMAIN. It's not the local IP address, it's the domain/host part of the
request URI in the INVITE.
I would prefer if phone context ended up in TELPHONECONTEXT so I could
use it the same way as SIPDOMAIN in the dial plan. It should not end up in
SIPDOMAIN as it is not a SIP uri. That way an extension in a local context
can be routed differently than an extension in a global context.
Olle E Johansson wrote:
Just to make sure it's documented: The phone-context should NOT be
matched to a context in the dial plan. From a security standpoint, that would
be horrific. We need the information to be able to route correctly in the
dial plan, but no automatic selection. (I am not suggesting you where going
down that path, Geert. Just wanted to close that way of thinking since the
word context could lead there.)
Geert Van Pamel wrote:
So I understand that Olle proposes to create a new variable
${TELPHONECONTEXT} that contains the TEL URI phone-context which can be
either the global number, or a global number prefix, or the related routing
domain or any other unique routing identifier, or would be empty in case
there is just a local number (as specified in RFC 3966).
Currently this variable is not available yet in Asterisk. In the dialplan
treating incoming calls currently I do not use nor do not need this
information, as the local number in ${CALLERID} is sufficient (for the time
being)... Still this phone context identifier could be needed for subsequent
outgoing calls (return calls, callbacks, etc.).
I agree that ${SIPDOMAIN} will remain reserved for SIP invites, and is
untouched for TEL URI invites.
I perfectly understand that this TEL URI context has nothing to do with
dialplan context.
Who could us further advise how to create the new variable
${TELPHONECONTEXT} ?
Olle E Johansson wrote:
Just grep for SIPDOMAIN in the chan_sip source code :-)
Matt Jordan wrote:
So I'd hate for this patch to stall out on the channel variable - which
would be a very helpful addition but is not strictly necessary for Geert's
functionality.
Olle - what do you think if this goes in as is, and we add the
TELPHONECONTEXT channel variable in a subsequent patch? It should be
relatively trivial, and I don't mind helping Geert with that work (so long as
Geert is willing to test said patch, as I don't have anything that emulates
tel URIs handy).
Geert - what do you think?
Geert Van Pamel wrote:
Matt, I agree with you... the variable ${TELPHONECONTEXT} is indeed not
needed for TEL URI INVITE to work. Actually, I a uncertain how to implement
this variable. If anybody else can code the variable later, I am willing to
test it...
Matt Jordan wrote:
Geert - I can take a look at adding that variable in a subsequent patch.
I'll definitely need you to test it!
I'll confirm with Olle that he's okay with this - if so, we'll get this
committed.
wdoekes wrote:
That's a negative:
14:26 wdoekes whether we can ship it without the TELURICONTEXT var
14:26 @oej Did they get the channel variable?
14:26 wdoekes for another rainy day, it was said
14:28 @oej Why is that? Only sending incomplete information to the
dialplan is not a good implementation.
14:28 @oej It's a simple change, nothing much to discuss. Many examples
in the source code of chan_sip
14:29 @oej I guess I should have discussed this with Matt last week.
14:30 wdoekes not sure it's that trivial, since the info has to get
out of the parser functions: a bit of refactoring
14:31 @oej It can't be that hard. Ignoring it would be bad and an
[asterisk-dev] [Code Review] 3439: chan_sip: Support a=rtcp attribute in SDP
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3439/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- the A=rtcp attribute in SDP points out a different port than the mediaport+1 to receive RTCP on. This patch adds a new api to rtpengine and res_rtp_asterisk and updates chan_sip to use it. Diffs - /trunk/res/res_rtp_asterisk.c 412166 /trunk/main/rtp_engine.c 412166 /trunk/include/asterisk/rtp_engine.h 412166 /trunk/channels/chan_sip.c 412166 Diff: https://reviewboard.asterisk.org/r/3439/diff/ Testing --- A massive amount of testing with a test tool for interoperability. Thanks, Olle E Johansson -- _ -- 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] 2478: Support multiple Require: and Supported: headers in the same request
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/2478/ --- (Updated April 11, 2014, 4:36 p.m.) Review request for Asterisk Developers. Changes --- Updated according to reviews. Bugs: ASTERISK-21721 https://issues.asterisk.org/jira/browse/ASTERISK-21721 Repository: Asterisk Description --- We have servers sending multiple Supported: and Require: options in separate headers in the same message. This patch fixes that support so that we parse more than the first header found. Diffs (updated) - /trunk/channels/sip/reqresp_parser.c 412208 /trunk/channels/chan_sip.c 412208 Diff: https://reviewboard.asterisk.org/r/2478/diff/ Testing --- Tested with the server that caused the issue. Problem solved and we did reach interoperability! Thanks, Olle E Johansson -- _ -- 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] 2872: Pre-review of work to handle SRTP lifetime and MKI in a less bad way, but not the best way
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/2872/ --- Review request for Asterisk Developers and mattjordan. Bugs: ASTERISK-17899 https://issues.asterisk.org/jira/browse/ASTERISK-17899 Repository: Asterisk Description --- Asterisk 1.8 and later currently just refuse calls that have an SDP lifetime parameter, even if it's ridiculously high. This patch adds a treshold of 10 hours (that could be made configurable) and refuse calls with a shorter lifetime. It also parses MKI/Lifetime properly. This is not ready for commit anywhere, but I wanted to know if we can agree that this is a good path moving forward. Diffs - Diff: https://reviewboard.asterisk.org/r/2872/diff/ Testing --- I've sent all kind of weird SDES keys from the README.lingon and while it failed previously it now succeeds where I want to and fail when I want it to fail. Thanks, Olle E Johansson -- _ -- 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] tcptls.c
On Fri, Apr 11, 2014 at 1:38 AM, Olle E. Johansson o...@edvina.net wrote: Looking at tcptls.c we have a lot of error messages sent to LOG_WARNING and some stuff sent to the verbose channel that should not be there. Do I need to go through reviewboard to just change these to LOG_ERROR or can I just fix this straight into the subversion repo? If you're just cleaning up WARNING/VERBOSE messages (and making certain ones ERRORs that weren't before, such as some of those ast_verb(0, ...) messages), I don't think that has to go up to Review Board. If there's something about the commit that someone doesn't like, we can always discuss it on the -dev list :-) -- Matthew Jordan Digium, Inc. | Engineering Manager 445 Jan Davis Drive NW - Huntsville, AL 35806 - USA Check us out at: http://digium.com http://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] 2227: Manage translation table between SIP and ISDN hangup causes
On Dec. 6, 2012, 11 a.m., Matt Jordan wrote:
So I have no idea why the svn merge didn't pull over the new files. I
ended up just checking out earl-grey and looking over sip2cause.c.
Line 41, sip2causestruct definition - right now you have this structured as
a list of sip2causestruct objects. This means to find a specific entry,
you have to iterate over the entire list of sip2causestruct definitions in
a table.
A possible faster way of structuring this data would be to use an ao2
object with the same definition of sip2causestruct, minus the 'next'
pointer. You could then modify sip2causetable to contain a container of
those objects:
struct sip2causetable {
struct ao2_container *table;
int default_code;
AST_DECLARE_STRING_FIELDS(
AST_STRING_FIELD(default_reason);
);
};
You would then create two different instances of sip2causetable - one would
be for your SIP to ISDN mappings, and one would be for your ISDN to SIP
mappings. They would differ in their key lookups - one would key off of
sip2causestruct's sip field, the other would key off of sipcausestruct's
cause field. This lets you do O(1) lookups of the appropriate mappings
instead of O(n) lookups, which should dramatically speed up
hangup_sip2cause and hangup_cause2sip.
As an aside, I don't see where defaultcode/defaultreason are used in
sip2causetable. If they aren't needed, this simplifies things even more -
you can get rid of the sip2causetable struct completely and replace it with
two ao2_containers:
static struct ao2_container *sip2cause_lookups;
static struct ao2_container *cause2sip_lookups;
(As an aside, more use of the '_' please :-))
Olle E Johansson wrote:
I had an idea for the defaultcode, but seems to have forgotten it during
the way forward. I'll review that again.
I don't think speed matters and will change significantly since there's
only a handful of entries in the table. It's such a simple function and today
we have a switch...
I will look for a container filled with underscores and apply it :-)
Thanks for the review. Seems like you agree with creating the new .c and
.h files and the config file, which is good.
Joshua Colp wrote:
Presumably this has to be locked when iterated which could become a
contention point on a system with many call teardowns so imo speed does
indeed matter. Previously since it never changed it did not need to be
protected.
Olle E Johansson wrote:
It can only change during a sip load/reload. During operations it doesn't
change. We propably need a lock for the reload, but not for lookups. So there
is no need for a lock during iteration - right?
Joshua Colp wrote:
You'd still have to lock it to make sure it doesn't change, but a
read/write lock would work.
How could it possibly change other than reload?
- Olle E
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2227/#review7498
---
On Dec. 5, 2012, 6:34 a.m., Olle E Johansson wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2227/
---
(Updated Dec. 5, 2012, 6:34 a.m.)
Review request for Asterisk Developers.
Bugs: ASTERISK-20759
https://issues.asterisk.org/jira/browse/ASTERISK-20759
Repository: Asterisk
Description
---
The SIP2CAUSE hangup code conversion tables has up to now been hard-coded in
Asterisk. In some cases, like when building in-house ISDN/Q.SIG to SIP
gateways, there's a need to manipulate this conversion.
With this code, advanced users can add a private conversion. This is added
in front of the built-in conversions.
Asterisk conversion tables does not change in this patch. Everything should
work as before. To shrink the chan_sip.c file a small bit I decided to move
this functionality into a new source code file.
Adding:
- new source code file sip2cause.c and include file sip2cause.h
- new configuration file sip2cause.conf
Reviewboard doesn't seem accept the new files, so they have to be found in
the branch itself.
http://svn.digium.com/svn/asterisk/team/oej/earl-grey-sip2cause-configurable-trunk
The new files are:
*
http://svnview.digium.com/svn/asterisk/team/oej/earl-grey-sip2cause-configurable-trunk/configs/sip2cause.conf.sample
*
http://svnview.digium.com/svn/asterisk/team/oej/earl-grey-sip2cause-configurable-trunk/channels/sip/sip2cause.c
*
http://svnview.digium.com/svn/asterisk/team/oej/earl-grey-sip2cause-configurable-trunk/channels/sip/include/sip2cause.h
Diffs
-
/trunk/channels/sip/include/sip_utils.h 377205
Re: [asterisk-dev] [Code Review] 2227: Manage translation table between SIP and ISDN hangup causes
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/2227/ --- (Updated April 11, 2014, 4:57 p.m.) Review request for Asterisk Developers. Changes --- Adding alert of new version. Bugs: ASTERISK-20759 https://issues.asterisk.org/jira/browse/ASTERISK-20759 Repository: Asterisk Description (updated) --- The SIP2CAUSE hangup code conversion tables has up to now been hard-coded in Asterisk. In some cases, like when building in-house ISDN/Q.SIG to SIP gateways, there's a need to manipulate this conversion. With this code, advanced users can add a private conversion. This is added in front of the built-in conversions. Asterisk conversion tables does not change in this patch. Everything should work as before. To shrink the chan_sip.c file a small bit I decided to move this functionality into a new source code file. Adding: - new source code file sip2cause.c and include file sip2cause.h - new configuration file sip2cause.conf Reviewboard doesn't seem accept the new files, so they have to be found in the branch itself. http://svn.digium.com/svn/asterisk/team/oej/earl-grey-sip2cause-configurable-trunk The new files are: * http://svnview.digium.com/svn/asterisk/team/oej/earl-grey-sip2cause-configurable-trunk/configs/sip2cause.conf.sample * http://svnview.digium.com/svn/asterisk/team/oej/earl-grey-sip2cause-configurable-trunk/channels/sip/sip2cause.c * http://svnview.digium.com/svn/asterisk/team/oej/earl-grey-sip2cause-configurable-trunk/channels/sip/include/sip2cause.h 2014-04: A new version will be coming soon with a new function - custom hangupcauses outside of the ISDN range (as discussed on asterisk-dev a while ago). Diffs - /trunk/channels/sip/include/sip_utils.h 377205 /trunk/channels/chan_sip.c 377205 Diff: https://reviewboard.asterisk.org/r/2227/diff/ Testing --- Tested all kinds of weird translations. This file should cause some errors (AST_CAUSE_SKREP doesn't exist, 903 is not a valid SIP reason code etc etc. [sip2cause] 604 = AST_CAUSE_SKREP 404 = UNALLOCATED 599 Bad = USER_BUSY 486 = NORMAL_CLEARING 603 = UNALLOCATED [cause2sip] SKREP = 503 Service Failure UNALLOCATED = 903 Go to hell UNALLOCATED = 499 I don't want to do that. USER_BUSY = 503 I am not feeling well Thanks, Olle E Johansson -- _ -- 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] 488: Add AMI actions for changing custom device state and generate manager events when device states changes
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/488/ --- (Updated April 11, 2014, 4:59 p.m.) Status -- This change has been discarded. Review request for Asterisk Developers. Bugs: 16732 https://issues.asterisk.org/jira/browse/16732 Repository: Asterisk Description --- Patch by Haakon This patch adds a DeviceStateChanged event to AMI. And also introduces AMI commands DeviceStateSet and DeviceStateGet to controll device states from AMI, if func_devicestate is compiled in. Diffs - /trunk/configs/manager.conf.sample 244686 /trunk/CHANGES 244686 /trunk/funcs/func_devstate.c 244686 /trunk/main/manager.c 244686 /trunk/main/devicestate.c 244686 /trunk/include/asterisk/manager.h 244686 Diff: https://reviewboard.asterisk.org/r/488/diff/ Testing --- Tested by contributor. Thanks, Olle E Johansson -- _ -- 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] 1164: [patch] Improve debug of ast_hangup
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/1164/ --- (Updated April 11, 2014, 5 p.m.) Status -- This change has been discarded. Review request for Asterisk Developers. Bugs: 19080 https://issues.asterisk.org/jira/browse/19080 Repository: Asterisk Description --- In many cases when I develop crazy code, I get weird hangups. It may only happen to me, but it does happen. I need to know when a hangup process is initiated and by whom. This small patch is a first step towards that goal. Diffs - /trunk/main/channel.c 313005 /trunk/include/asterisk/channel.h 313005 Diff: https://reviewboard.asterisk.org/r/1164/diff/ Testing --- Tested in various versions of Asterisk. Seems to give me the output I need. Thanks, Olle E Johansson -- _ -- 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] 1421: AMI :: Debug manager actions in the CLI
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/1421/#review11580 --- I've used this for a very long time in my own version of Asterisk and still find it useful. Shouldn't we try to make something along these lines happen? - Olle E Johansson On Sept. 19, 2011, 8:45 a.m., Olle E Johansson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/1421/ --- (Updated Sept. 19, 2011, 8:45 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-18447 https://issues.asterisk.org/jira/browse/ASTERISK-18447 Repository: Asterisk Description --- Adds a new option to manager.conf. When enabled, all manager actions will be output in the CLI session, in order to be able to debug a system controlled by AMI connections. Red dots will be removed before commit Diffs - /team/oej/rana-manager-debug-trunk/main/manager.c 334684 /team/oej/rana-manager-debug-trunk/configs/manager.conf.sample 334684 Diff: https://reviewboard.asterisk.org/r/1421/diff/ Testing --- Works fine on my system and a system at customer site in 1.4 version. The 1.8 version is not much different. Thanks, Olle E Johansson -- _ -- 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] tcptls.c
On 11 Apr 2014, at 16:53, Matthew Jordan mjor...@digium.com wrote: On Fri, Apr 11, 2014 at 1:38 AM, Olle E. Johansson o...@edvina.net wrote: Looking at tcptls.c we have a lot of error messages sent to LOG_WARNING and some stuff sent to the verbose channel that should not be there. Do I need to go through reviewboard to just change these to LOG_ERROR or can I just fix this straight into the subversion repo? If you're just cleaning up WARNING/VERBOSE messages (and making certain ones ERRORs that weren't before, such as some of those ast_verb(0, ...) messages), I don't think that has to go up to Review Board. If there's something about the commit that someone doesn't like, we can always discuss it on the -dev list :-) No, nothing else. Just changing the default cipher to the null cipher... ;-) Thanks, Matt. /O-- _ -- 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] 3440: ARI: Get rid of \brief on ARI resource mustache struct documentation comments
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3440/ --- Review request for Asterisk Developers, David Lee and Matt Jordan. Repository: Asterisk Description --- This was pointed out on https://reviewboard.asterisk.org/r/3379/ -- the real change here was a very minor adjustment to the ARI resource mustache template... the rest is all autogenerated. Diffs - /branches/12/rest-api-templates/ari_resource.h.mustache 412208 /branches/12/res/ari/resource_sounds.h 412208 /branches/12/res/ari/resource_recordings.h 412208 /branches/12/res/ari/resource_playbacks.h 412208 /branches/12/res/ari/resource_mailboxes.h 412208 /branches/12/res/ari/resource_endpoints.h 412208 /branches/12/res/ari/resource_device_states.h 412208 /branches/12/res/ari/resource_channels.h 412208 /branches/12/res/ari/resource_bridges.h 412208 /branches/12/res/ari/resource_asterisk.h 412208 /branches/12/res/ari/resource_applications.h 412208 Diff: https://reviewboard.asterisk.org/r/3440/diff/ Testing --- Made sure the struct fields lost their \briefs, checked to make sure nothing else did in the file I was looking at at least. 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] 3440: ARI: Get rid of \brief on ARI resource mustache struct documentation comments
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3440/#review11581 --- /branches/12/rest-api-templates/ari_resource.h.mustache https://reviewboard.asterisk.org/r/3440/#comment21338 I think you should remove \brief from all of these lines for the same reason. - rmudgett On April 11, 2014, 11:33 a.m., Jonathan Rose wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3440/ --- (Updated April 11, 2014, 11:33 a.m.) Review request for Asterisk Developers, David Lee and Matt Jordan. Repository: Asterisk Description --- This was pointed out on https://reviewboard.asterisk.org/r/3379/ -- the real change here was a very minor adjustment to the ARI resource mustache template... the rest is all autogenerated. Diffs - /branches/12/rest-api-templates/ari_resource.h.mustache 412208 /branches/12/res/ari/resource_sounds.h 412208 /branches/12/res/ari/resource_recordings.h 412208 /branches/12/res/ari/resource_playbacks.h 412208 /branches/12/res/ari/resource_mailboxes.h 412208 /branches/12/res/ari/resource_endpoints.h 412208 /branches/12/res/ari/resource_device_states.h 412208 /branches/12/res/ari/resource_channels.h 412208 /branches/12/res/ari/resource_bridges.h 412208 /branches/12/res/ari/resource_asterisk.h 412208 /branches/12/res/ari/resource_applications.h 412208 Diff: https://reviewboard.asterisk.org/r/3440/diff/ Testing --- Made sure the struct fields lost their \briefs, checked to make sure nothing else did in the file I was looking at at least. 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] 1164: [patch] Improve debug of ast_hangup
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/1164/#review11582 --- Looks fine to me at first glance. - Jared Smith On April 11, 2014, 3 p.m., Olle E Johansson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/1164/ --- (Updated April 11, 2014, 3 p.m.) Review request for Asterisk Developers. Bugs: 19080 https://issues.asterisk.org/jira/browse/19080 Repository: Asterisk Description --- In many cases when I develop crazy code, I get weird hangups. It may only happen to me, but it does happen. I need to know when a hangup process is initiated and by whom. This small patch is a first step towards that goal. Diffs - /trunk/main/channel.c 313005 /trunk/include/asterisk/channel.h 313005 Diff: https://reviewboard.asterisk.org/r/1164/diff/ Testing --- Tested in various versions of Asterisk. Seems to give me the output I need. Thanks, Olle E Johansson -- _ -- 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] 3433: bridge_unreal: An alternative implementation for optimizing Unreal/Local channels.
On April 10, 2014, 6:20 p.m., rmudgett wrote: I'm not seeing any protection from loss of frames when the channels optimize out. Losing media frames isn't nice but is tollerable. Losing control frames is unacceptable. Joshua Colp wrote: 1. Can you explain the scenario and how I would lose frames. 2. Since I'm using standard APIs we now provide if I have to do special stuff shouldn't every user? 1) frame loss The unreal_bridge_optimize_task() is effectively a third party outside the chain. It cannot know for certain what is happening within the chain when the channels actually get moved. Thus it cannot prevent the potential loss of frames in the local channels being optimized without some cooperation of the bridges and local channels involved. See the note in bridge_channel_write_frame() about a deferred write queue that could be useful to do this. The deferred write queue would need to go with the surviving channel of the optimization that is being moved to the new bridge. You can only optimize if the local channels involved are processing media frames at the time. The current optimization code checks: 1) is local channel optimization enabled. 2) are the read and write queues of the local channels empty. 3) is DTMF not in progress. 4) are there any active monitors, audiohooks, or framehooks. 5) are the two local channels in bridges. 6) are the local channel threads in a safe position for the optimization. Holding the locks blocks the local channel threads at certain points so the bridge_channel-activity and bridge_channel-state can be checked to find out what the local channel threads are up to. 7) do the bridges allow optimization and which way. One thing I did see when trying to fix the masquerade supertest was that there is almost no contention for the seven locks needed. The main preventer of optimization was the queues were not empty. 2) bridge API calls The answer depends on which calls and why they are being made. Optimization has a unique requirement when restructuring the bridges and channels. Optimization should not be noticeable to the parties involved so losing frames is not good. The other API users are moving channels to/from parking, to/from the agent holding bridge, or transferring calls. The frame stream is expected to be interrupted and the frame source is expected to be changed. - rmudgett --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3433/#review11572 --- On April 9, 2014, 2:49 p.m., Joshua Colp wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3433/ --- (Updated April 9, 2014, 2:49 p.m.) Review request for Asterisk Developers, Matt Jordan, Mark Michelson, and rmudgett. Repository: Asterisk Description --- This change removes Unreal/Local channel optimization from the core and isolates it in a bridging technology implementation. The implementation attempts to optimize when channels join a bridge using it, instead of attempting to optimize constantly as frames pass. This review is not just for the code itself but the approach in general. Is this something we'd like? Is it viable? Now the con... Since this is implemented as a bridging technology and not as part of the bridging core it can not optimize completely when multi-party bridges are involved. If you have a chain in the middle that'll go anyway but no merging of bridges will occur. Diffs - /branches/12/main/core_unreal.c 411867 /branches/12/main/core_local.c 411867 /branches/12/main/bridge.c 411867 /branches/12/include/asterisk/core_unreal.h 411867 /branches/12/bridges/bridge_unreal.c PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3433/diff/ Testing --- Since this has been an idea and side project I've only done some tests myself with large numbers of Local channels in chains (100, 200, 300) and confirmed that stuff works as expected. 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] 3357: testsuite: Add off-nominal subscription tests for PJSIP.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3357/ --- (Updated April 11, 2014, 5:12 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers, Kevin Harwell and Matt Jordan. Changes --- Committed in revision 4952 Bugs: ASTERISK-23342 https://issues.asterisk.org/jira/browse/ASTERISK-23342 Repository: testsuite Description --- No Accept header This would set up the subscription, but use the default type for the event package being subscribed for Disallowed subscriptions A SIP UA subscribes for a valid event package with Asterisk, but the endpoint doesn't allow subscriptions Asterisk responds with a 603 MinExpiry not met A SIP UA sends a subscription with an expiration time that is less than the configured minexpiry for the endpoint Asterisk responds with a 423 No Event Header A SIP UA sends a subscription but fails to provide an Event header Asterisk responds with a 489 Unknown Event Package A SIP UA sends a subscription for an unknown event package Asterisk responds with a 489 Each of these tests is based on kharwell's Digium Presence test. As such, the No Accept Header test does require some digium phone specific stuff to be loaded in order to work. For all the other tests though, the tests are fairly general and will just fail for the reasons you would expect. Diffs - /asterisk/trunk/tests/channels/pjsip/subscriptions/unknown_event_package/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/unknown_event_package/sipp/subscribe.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/unknown_event_package/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/unknown_event_package/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/unallowed/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/unallowed/sipp/subscribe.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/unallowed/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/unallowed/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/tests.yaml 4836 /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/tests.yaml 4836 /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/no_accept_header/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/no_accept_header/sipp/subscribe.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/no_accept_header/run-test PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/no_accept_header/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/presence/no_accept_header/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/no_event_header/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/no_event_header/sipp/subscribe.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/no_event_header/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/no_event_header/configs/ast1/extensions.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/mwi/tests.yaml 4836 /asterisk/trunk/tests/channels/pjsip/subscriptions/mwi/no_accept_header/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/mwi/no_accept_header/sipp/subscribe.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/mwi/no_accept_header/run-test PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/mwi/no_accept_header/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/mwi/no_accept_header/configs/ast1/modules.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/below_min_expiry/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/below_min_expiry/sipp/subscribe.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/below_min_expiry/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/subscriptions/below_min_expiry/configs/ast1/extensions.conf PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3357/diff/ Testing --- Ran tests to determine that the sipp scenarios were received the expected responses for every scenario. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE
Re: [asterisk-dev] [Code Review] 3424: mixmonitor: Add option to enable a periodic beep
On April 10, 2014, 3:35 p.m., Mark Michelson wrote: /trunk/apps/app_mixmonitor.c, line 1067 https://reviewboard.asterisk.org/r/3424/diff/2/?file=57155#file57155line1067 Suggestion: Instead of doing ast_module_check(), use ast_custom_function_find() to find the PERIODIC_HOOK function. I have a couple of reasons for this: 1) Currently, if func_periodic_hook.so is loaded, it also means that PERIODIC_HOOK is registered, but that assumption may not always hold. 2) More importantly, getting the ast_custom_function allows for you to bump the module refcount for func_periodic_hook.so so that the module cannot be unloaded while there is an active mixmonitor. You can decrement the module refcount when the mixmonitor is destroyed. I started looking at this, but I'm not sure the alternative is better. They both have downsides. ast_custom_function_find() may be worse. Re: #1, you're right, but ... Re: #2, the module ref count is now always incremented on func_periodic_hook.so when PERIODIC_HOOK() is active. Also, it looks like ast_custom_function_find() isn't safe. There is a race condition between finding a function and incrementing the module ref count where the module could be unloaded and cause a crash. At least with ast_module_check(), ast_func_read() may fail, but it's handled gracefully without crashing. - Russell --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3424/#review11562 --- On April 8, 2014, 7:49 p.m., Russell Bryant wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3424/ --- (Updated April 8, 2014, 7:49 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- Add an option to enable a periodic beep to be played into a call if it is being recorded. If enabled, it uses the PERIODIC_HOOK() function internally to play the 'beep' prompt into the call at a specified interval. Diffs - /trunk/main/app.c 412023 /trunk/include/asterisk/app.h 412023 /trunk/funcs/func_periodic_hook.c 412023 /trunk/apps/app_mixmonitor.c 412023 /trunk/CHANGES 412023 Diff: https://reviewboard.asterisk.org/r/3424/diff/ Testing --- exten = 103,1,Answer() same = n,MixMonitor(test.gsm,B(5)) same = n,MusicOnHold() exten = 104,1,Answer() same = n,MixMonitor(test.gsm,B) same = n,MusicOnHold() exten = 105,1,Answer() same = n,MixMonitor(test.gsm,B(3)) same = n,StartMusicOnHold() same = n,Wait(15) same = n,StopMusicOnHold() same = n,StopMixMonitor() same = n,Wait(5) same = n,Hangup() Thanks, Russell Bryant -- _ -- 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] 3438: Implement SIP TImer C in Asterisk
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3438/#review11585 --- /trunk/channels/chan_sip.c https://reviewboard.asterisk.org/r/3438/#comment21349 I am concerned about a repeat of https://reviewboard.asterisk.org/r/3368/. It seems like the timer can be rescheduled or cancelled from another thread while we wait for the pvt lock. - Corey Farrell On April 11, 2014, 4:41 a.m., Olle E Johansson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3438/ --- (Updated April 11, 2014, 4:41 a.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- SIP Timer C is defined for proxys that forward messages. In some ways, we forward calls. It is activated when we receive a 100 trying and wait for any other message. If that's not received, timer C triggers and cancels the call attempt. This is required in an interoperability test I'm working with. Red dots will be handled in the way they deserve. Diffs - /trunk/configs/sip.conf.sample 412166 /trunk/channels/sip/include/sip.h 412166 /trunk/channels/chan_sip.c 412166 Diff: https://reviewboard.asterisk.org/r/3438/diff/ Testing --- Passed interoperability testing with funky test tool. Thanks, Olle E Johansson -- _ -- 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] 3424: mixmonitor: Add option to enable a periodic beep
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3424/ --- (Updated April 11, 2014, 10:32 p.m.) Review request for Asterisk Developers. Changes --- Updated to use optional_api Repository: Asterisk Description --- Add an option to enable a periodic beep to be played into a call if it is being recorded. If enabled, it uses the PERIODIC_HOOK() function internally to play the 'beep' prompt into the call at a specified interval. Diffs (updated) - /trunk/include/asterisk/beep.h PRE-CREATION /trunk/funcs/func_periodic_hook.c 412280 /trunk/apps/app_mixmonitor.c 412278 /trunk/CHANGES 412278 Diff: https://reviewboard.asterisk.org/r/3424/diff/ Testing --- exten = 103,1,Answer() same = n,MixMonitor(test.gsm,B(5)) same = n,MusicOnHold() exten = 104,1,Answer() same = n,MixMonitor(test.gsm,B) same = n,MusicOnHold() exten = 105,1,Answer() same = n,MixMonitor(test.gsm,B(3)) same = n,StartMusicOnHold() same = n,Wait(15) same = n,StopMusicOnHold() same = n,StopMixMonitor() same = n,Wait(5) same = n,Hangup() Thanks, Russell Bryant -- _ -- 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] 3441: testsuite: Add a test for TEL URI
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3441/ --- Review request for Asterisk Developers and Geert Van Pamel. Bugs: ASTERISK-17179 https://issues.asterisk.org/jira/browse/ASTERISK-17179 Repository: testsuite Description --- This test verifies support for TEL URIs in basic incoming calls. An INVITE request with a TEL URI sends a request to Asterisk. The phone-context specifies a domain for the global number. The From header contains a local number with a phone-context that contains the prefix of a global number. If the INVITE request is handled properly, the TELPHONECONTEXT channel variable will be set properly. If not set properly, the test will fail as the channel will not be answered and hungup prematurely. Diffs - /asterisk/trunk/tests/channels/SIP/tests.yaml 4951 /asterisk/trunk/tests/channels/SIP/tel_uri/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/SIP/tel_uri/sipp/tel_uac.xml PRE-CREATION /asterisk/trunk/tests/channels/SIP/tel_uri/configs/ast1/sip.conf PRE-CREATION /asterisk/trunk/tests/channels/SIP/tel_uri/configs/ast1/extensions.conf PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3441/diff/ Testing --- 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] 3441: testsuite: Add a test for TEL URI
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3441/ --- (Updated April 11, 2014, 9:35 p.m.) Review request for Asterisk Developers and Geert Van Pamel. Bugs: ASTERISK-17179 https://issues.asterisk.org/jira/browse/ASTERISK-17179 Repository: testsuite Description --- This test verifies support for TEL URIs in basic incoming calls. An INVITE request with a TEL URI sends a request to Asterisk. The phone-context specifies a domain for the global number. The From header contains a local number with a phone-context that contains the prefix of a global number. If the INVITE request is handled properly, the TELPHONECONTEXT channel variable will be set properly. If not set properly, the test will fail as the channel will not be answered and hungup prematurely. Diffs - /asterisk/trunk/tests/channels/SIP/tests.yaml 4951 /asterisk/trunk/tests/channels/SIP/tel_uri/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/SIP/tel_uri/sipp/tel_uac.xml PRE-CREATION /asterisk/trunk/tests/channels/SIP/tel_uri/configs/ast1/sip.conf PRE-CREATION /asterisk/trunk/tests/channels/SIP/tel_uri/configs/ast1/extensions.conf PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3441/diff/ Testing --- 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] 3442: chan_sip: Add TELPHONECONTEXT channel variable for Request TEL URIs
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3442/ --- Review request for Asterisk Developers, Geert Van Pamel and Olle E Johansson. Bugs: ASTERISK-17179 https://issues.asterisk.org/jira/browse/ASTERISK-17179 Repository: Asterisk Description --- This patch is a continuation of 3349 (https://reviewboard.asterisk.org/r/3349/) It resolves a finding oej had that the phone-context be available in a TELPHONECONTEXT channel variable. This patch adds that variable. It also allows a local number (or global number specified in the TEL URI) to be used to look up as a peer. Granted; not super useful. But hey. Diffs - /trunk/channels/sip/include/sip.h 412292 /trunk/channels/chan_sip.c 412292 Diff: https://reviewboard.asterisk.org/r/3442/diff/ Testing --- See review https://reviewboard.asterisk.org/r/3441 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] 3424: mixmonitor: Add option to enable a periodic beep
On April 10, 2014, 3:35 p.m., Mark Michelson wrote: /trunk/apps/app_mixmonitor.c, line 1067 https://reviewboard.asterisk.org/r/3424/diff/2/?file=57155#file57155line1067 Suggestion: Instead of doing ast_module_check(), use ast_custom_function_find() to find the PERIODIC_HOOK function. I have a couple of reasons for this: 1) Currently, if func_periodic_hook.so is loaded, it also means that PERIODIC_HOOK is registered, but that assumption may not always hold. 2) More importantly, getting the ast_custom_function allows for you to bump the module refcount for func_periodic_hook.so so that the module cannot be unloaded while there is an active mixmonitor. You can decrement the module refcount when the mixmonitor is destroyed. Russell Bryant wrote: I started looking at this, but I'm not sure the alternative is better. They both have downsides. ast_custom_function_find() may be worse. Re: #1, you're right, but ... Re: #2, the module ref count is now always incremented on func_periodic_hook.so when PERIODIC_HOOK() is active. Also, it looks like ast_custom_function_find() isn't safe. There is a race condition between finding a function and incrementing the module ref count where the module could be unloaded and cause a crash. At least with ast_module_check(), ast_func_read() may fail, but it's handled gracefully without crashing. And actually, converting this over to optional_api removed the module check anyway - Russell --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3424/#review11562 --- On April 11, 2014, 10:32 p.m., Russell Bryant wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3424/ --- (Updated April 11, 2014, 10:32 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- Add an option to enable a periodic beep to be played into a call if it is being recorded. If enabled, it uses the PERIODIC_HOOK() function internally to play the 'beep' prompt into the call at a specified interval. Diffs - /trunk/include/asterisk/beep.h PRE-CREATION /trunk/funcs/func_periodic_hook.c 412280 /trunk/apps/app_mixmonitor.c 412278 /trunk/CHANGES 412278 Diff: https://reviewboard.asterisk.org/r/3424/diff/ Testing --- exten = 103,1,Answer() same = n,MixMonitor(test.gsm,B(5)) same = n,MusicOnHold() exten = 104,1,Answer() same = n,MixMonitor(test.gsm,B) same = n,MusicOnHold() exten = 105,1,Answer() same = n,MixMonitor(test.gsm,B(3)) same = n,StartMusicOnHold() same = n,Wait(15) same = n,StopMusicOnHold() same = n,StopMixMonitor() same = n,Wait(5) same = n,Hangup() Thanks, Russell Bryant -- _ -- 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
