date:20140313


Olle E. Johansson wrote:


On 12 Mar 2014, at 19:53, Joshua Colp reviewbo...@asterisk.org
mailto:reviewbo...@asterisk.org wrote:


This change adds a configuration option for setting nameservers to
be used by the PJSIP DNS client. If this option is not set then the
system nameservers are retrieved and used instead.

This also allows the nameservers to be changed by doing a reload.


Why is this a good thing?


PJLIB-Util (part of pjproject) provides a DNS client which can 
optionally (but is highly suggested) to be used with PJSIP. It provides 
asynchronous DNS, SRV lookups, multiple record support, etc. Right now 
this isn't enabled so we are simply doing A/ record lookups. The 
reason it's not enabled is that explicit nameservers *must* be provided 
to it when enabling it. It will not use the system ones by itself. The 
change up on reviewboard enables it by default using the system 
nameservers it finds, but with the ability to override or completely 
disable it if a user wants. The reason I also provide reload 
functionality is that people in #asterisk-dev expressed a concern that 
users may change nameservers but don't want to restart Asterisk, which 
is understandable.


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] 3343: res_pjsip: Enable DNS support.


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3343/
---

(Updated March 13, 2014, 10:42 a.m.)


Review request for Asterisk Developers.


Changes
---

Based on the question from Olle update description.


Bugs: ASTERISK-23435
https://issues.asterisk.org/jira/browse/ASTERISK-23435


Repository: Asterisk


Description (updated)
---

This change adds a configuration option for setting nameservers to be used by 
the PJSIP DNS client. If this option is not set then the system nameservers are 
retrieved and used instead.

This also allows the nameservers to be changed by doing a reload.

In case others are wondering as Olle was:

PJLIB-Util (part of pjproject) provides a DNS client which can optionally (but 
is highly suggested) to be used with PJSIP. It provides asynchronous DNS, SRV 
lookups, multiple record support, etc. Right now this isn't enabled so we are 
simply doing A/ record lookups. The reason it's not enabled is that 
explicit nameservers *must* be provided to it when enabling it. It will not use 
the system ones by itself. The change up on reviewboard enables it by default 
using the system nameservers it finds, but with the ability to override or 
completely disable it if a user wants. The reason I also provide reload 
functionality is that people in #asterisk-dev expressed a concern that users 
may change nameservers but don't want to restart Asterisk, which is 
understandable. 


Diffs
-

  /branches/12/res/res_pjsip/pjsip_configuration.c 410470 
  /branches/12/res/res_pjsip/include/res_pjsip_private.h 410470 
  /branches/12/res/res_pjsip/config_global.c 410470 
  /branches/12/res/res_pjsip.c 410470 
  /branches/12/main/dns.c 410470 
  /branches/12/include/asterisk/dns.h 410470 
  /branches/12/configs/pjsip.conf.sample 410470 
  /branches/12/CHANGES 410470 

Diff: https://reviewboard.asterisk.org/r/3343/diff/


Testing
---

Explicitly set nameservers and confirmed they were used by PJSIP. Disabled it 
and confirmed that the DNS client was disabled. Set to auto (explicitly and by 
default) and confirmed that the system nameservers were used.


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] 3343: res_pjsip: Enable DNS support.


Olle E. Johansson wrote:


On 12 Mar 2014, at 19:53, Joshua Colp reviewbo...@asterisk.org
mailto:reviewbo...@asterisk.org wrote:


This change adds a configuration option for setting nameservers to
be used by the PJSIP DNS client. If this option is not set then the
system nameservers are retrieved and used instead.

This also allows the nameservers to be changed by doing a reload.


Why is this a good thing?


I've now also placed my response on reviewboard in case anyone else 
comes along it and wants an extended explanation.


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] 3343: res_pjsip: Enable DNS support.

2014-03-13 Thread Olle E. Johansson


On 13 Mar 2014, at 11:42, Joshua Colp reviewbo...@asterisk.org wrote:

 In case others are wondering as Olle was:
 
 PJLIB-Util (part of pjproject) provides a DNS client which can optionally 
 (but is highly suggested) to be used with PJSIP. It provides asynchronous 
 DNS, SRV lookups, multiple record support, etc. Right now this isn't enabled 
 so we are simply doing A/ record lookups. The reason it's not enabled is 
 that explicit nameservers *must* be provided to it when enabling it. It will 
 not use the system ones by itself. The change up on reviewboard enables it by 
 default using the system nameservers it finds, but with the ability to 
 override or completely disable it if a user wants. The reason I also provide 
 reload functionality is that people in #asterisk-dev expressed a concern that 
 users may change nameservers but don't want to restart Asterisk, which is 
 understandable. 

Interesting to get answer in another channel... For both of us.

My question still stands - why would anyone want one part of Asterisk use other 
DNS servers than the rest of Asterisk and the rest of the system? If there is 
something wrong with the system resolver, that needs to be fixed.

I do not see the need for us to have a configuration option here. Someone else 
may have a good reason for it.

/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

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.


Olle E. Johansson wrote:


On 13 Mar 2014, at 11:42, Joshua Colp reviewbo...@asterisk.org
mailto:reviewbo...@asterisk.org wrote:


In case others are wondering as Olle was:

PJLIB-Util (part of pjproject) provides a DNS client which can
optionally (but is highly suggested) to be used with PJSIP. It
provides asynchronous DNS, SRV lookups, multiple record support,
etc. Right now this isn't enabled so we are simply doing A/
record lookups. The reason it's not enabled is that explicit
nameservers *must* be provided to it when enabling it. It will not
use the system ones by itself. The change up on reviewboard enables
it by default using the system nameservers it finds, but with the
ability to override or completely disable it if a user wants. The
reason I also provide reload functionality is that people in
#asterisk-dev expressed a concern that users may change nameservers
but don't want to restart Asterisk, which is understandable.


Interesting to get answer in another channel... For both of us.

My question still stands - why would anyone want one part of Asterisk
 use other DNS servers than the rest of Asterisk and the rest of the
 system? If there is something wrong with the system resolver, that
needs to be fixed.

I do not see the need for us to have a configuration option here.
Someone else may have a good reason for it.


I can't guarantee that the code which automatically determines and gets 
the nameservers from the system will work on all platforms. It's using 
res_init / res_ninit which, depending on the implementation, parses 
resolv.conf and stores the information. I think it *should* work but for 
cases where it won't the ability to manually set them is there. For most 
people you don't even need to know it exists or set it.


--
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] 3343: res_pjsip: Enable DNS support.

2014-03-13 Thread Sean Bright


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3343/#review11157
---

Ship it!


Looks good to me.  Now if only we could have it automatically update with 
/etc/resolv.conf is changed...


/branches/12/res/res_pjsip/config_global.c
https://reviewboard.asterisk.org/r/3343/#comment20797

The comment and function name are stale, but this was already resolved in 
your development branch.  Just pointing it out.


- Sean Bright


On March 13, 2014, 10:42 a.m., Joshua Colp wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3343/
 ---
 
 (Updated March 13, 2014, 10:42 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-23435
 https://issues.asterisk.org/jira/browse/ASTERISK-23435
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This change adds a configuration option for setting nameservers to be used by 
 the PJSIP DNS client. If this option is not set then the system nameservers 
 are retrieved and used instead.
 
 This also allows the nameservers to be changed by doing a reload.
 
 In case others are wondering as Olle was:
 
 PJLIB-Util (part of pjproject) provides a DNS client which can optionally 
 (but is highly suggested) to be used with PJSIP. It provides asynchronous 
 DNS, SRV lookups, multiple record support, etc. Right now this isn't enabled 
 so we are simply doing A/ record lookups. The reason it's not enabled is 
 that explicit nameservers *must* be provided to it when enabling it. It will 
 not use the system ones by itself. The change up on reviewboard enables it by 
 default using the system nameservers it finds, but with the ability to 
 override or completely disable it if a user wants. The reason I also provide 
 reload functionality is that people in #asterisk-dev expressed a concern that 
 users may change nameservers but don't want to restart Asterisk, which is 
 understandable. 
 
 
 Diffs
 -
 
   /branches/12/res/res_pjsip/pjsip_configuration.c 410470 
   /branches/12/res/res_pjsip/include/res_pjsip_private.h 410470 
   /branches/12/res/res_pjsip/config_global.c 410470 
   /branches/12/res/res_pjsip.c 410470 
   /branches/12/main/dns.c 410470 
   /branches/12/include/asterisk/dns.h 410470 
   /branches/12/configs/pjsip.conf.sample 410470 
   /branches/12/CHANGES 410470 
 
 Diff: https://reviewboard.asterisk.org/r/3343/diff/
 
 
 Testing
 ---
 
 Explicitly set nameservers and confirmed they were used by PJSIP. Disabled it 
 and confirmed that the DNS client was disabled. Set to auto (explicitly and 
 by default) and confirmed that the system nameservers were used.
 
 
 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] 3343: res_pjsip: Enable DNS support.


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3343/#review11158
---



/branches/12/res/res_pjsip.c
https://reviewboard.asterisk.org/r/3343/#comment20798

New parameter = alembic update


- Matt Jordan


On March 13, 2014, 5:42 a.m., Joshua Colp wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3343/
 ---
 
 (Updated March 13, 2014, 5:42 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-23435
 https://issues.asterisk.org/jira/browse/ASTERISK-23435
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This change adds a configuration option for setting nameservers to be used by 
 the PJSIP DNS client. If this option is not set then the system nameservers 
 are retrieved and used instead.
 
 This also allows the nameservers to be changed by doing a reload.
 
 In case others are wondering as Olle was:
 
 PJLIB-Util (part of pjproject) provides a DNS client which can optionally 
 (but is highly suggested) to be used with PJSIP. It provides asynchronous 
 DNS, SRV lookups, multiple record support, etc. Right now this isn't enabled 
 so we are simply doing A/ record lookups. The reason it's not enabled is 
 that explicit nameservers *must* be provided to it when enabling it. It will 
 not use the system ones by itself. The change up on reviewboard enables it by 
 default using the system nameservers it finds, but with the ability to 
 override or completely disable it if a user wants. The reason I also provide 
 reload functionality is that people in #asterisk-dev expressed a concern that 
 users may change nameservers but don't want to restart Asterisk, which is 
 understandable. 
 
 
 Diffs
 -
 
   /branches/12/res/res_pjsip/pjsip_configuration.c 410470 
   /branches/12/res/res_pjsip/include/res_pjsip_private.h 410470 
   /branches/12/res/res_pjsip/config_global.c 410470 
   /branches/12/res/res_pjsip.c 410470 
   /branches/12/main/dns.c 410470 
   /branches/12/include/asterisk/dns.h 410470 
   /branches/12/configs/pjsip.conf.sample 410470 
   /branches/12/CHANGES 410470 
 
 Diff: https://reviewboard.asterisk.org/r/3343/diff/
 
 
 Testing
 ---
 
 Explicitly set nameservers and confirmed they were used by PJSIP. Disabled it 
 and confirmed that the DNS client was disabled. Set to auto (explicitly and 
 by default) and confirmed that the system nameservers were used.
 
 
 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] 3344: PJSIP MWI: Prevent endpoints from subscribing to AORs that provide MWI for a mailbox for which they are already receiving unsolicited MWI.


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3344/#review11159
---

Ship it!


Ship It!

- Joshua Colp


On March 12, 2014, 9:52 p.m., Mark Michelson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3344/
 ---
 
 (Updated March 12, 2014, 9:52 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 The summary says it all: if an endpoint is receiving unsolicited MWI for a 
 particular mailbox, then prevent the endpoint from attempting to subscribe to 
 an AOR that provides MWI for that mailbox as well.
 
 
 Diffs
 -
 
   /branches/12/res/res_pjsip_mwi.c 410467 
 
 Diff: https://reviewboard.asterisk.org/r/3344/diff/
 
 
 Testing
 ---
 
 I have posted an accompanying test at /r/3345
 
 
 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] 3340: ARI: Bridge Playback needs to have PlaybackStarted and PlaybackFinished messages get relayed to the bridge topic

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3340/#review11161
---

/branches/12/res/ari/resource_bridges.c
https://reviewboard.asterisk.org/r/3340/#comment20799

Add a comment about why forwarding all messages doesn't result in channel
events going out.

The reason is because these types of channels are marked as internal, which
are sanitized by ARI and not allowed to go out as events.

In the case of the event you are forwarding it is unrelated to the channel
and allowed to pass.

- Joshua Colp

On March 12, 2014, 6:02 p.m., Jonathan Rose wrote:

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3340/
---

(Updated March 12, 2014, 6:02 p.m.)

Review request for Asterisk Developers, Joshua Colp and Matt Jordan.

Bugs: ASTERISK-23444
https://issues.asterisk.org/jira/browse/ASTERISK-23444

Repository: Asterisk

Description
---

This patch adds a stasis forwarder to put events from the playback channel
(the side of the unreal channel chain responsible for playing sounds to the
bridge) into the bridge stasis topic. This way if you are subscribed to the
bridge and play sounds to it using ARI, you'll know when those sounds start
and stop.

Diffs
-

/branches/12/res/ari/resource_bridges.c 410487

Diff: https://reviewboard.asterisk.org/r/3340/diff/

Testing
---

1. Had a websocket connect using stasis application 'hello'
2. Created a bridge using ARI
3. Subscribed 'hello' to the bridge created in (2) using ARI
4. Used bridge/play to play tt-wesels to the bridge

Before patch:

*crickets*

After patch:
RESPONSE:
{application:hello,type:PlaybackStarted,playback:{id:2f88583c-ccee-4340-9fe7-25352c1e6c5e,media_uri:sound:tt-weasels,target_uri:bridge:0ec1f7f4-62a3-49d1-9877-734ac987112e,language:en,state:playing}}
RESPONSE:
{application:hello,type:PlaybackFinished,playback:{id:2f88583c-ccee-4340-9fe7-25352c1e6c5e,media_uri:sound:tt-weasels,target_uri:bridge:0ec1f7f4-62a3-49d1-9877-734ac987112e,language:en,state:done}}

I also repeated this test with a PJSIP channel in the bridge to confirm that
the sounds were starting and stopping at the expected times.

I only receive the PlaybackStarted and PlaybackFinished events on the bridge
topic and not anything like BridgeEnter/Leave events or channel hangup
events. The reason for this I believe is because I'm pushing the opposite
(unreal;1 vs unreal;2) channel into the bridge from the one actually
executing the playback events and I'm canceling the forward before a hangup
stasis message is generated. At first I suspected I would receive channel
hangup notifications on account of hanging up the channel in the bridge prior
to canceling the stasis forward. I'm not sure if that would go against the
intent of this patch or not, but it doesn't appear to be the case... probably
a matter of the stasis forward being canceled before the playback channel
hangs up.

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] 3343: res_pjsip: Enable DNS support.

2014-03-13 Thread Olle E. Johansson


On 13 Mar 2014, at 12:06, Joshua Colp jc...@digium.com wrote:

 Olle E. Johansson wrote:
 
 On 13 Mar 2014, at 11:42, Joshua Colp reviewbo...@asterisk.org
 mailto:reviewbo...@asterisk.org wrote:
 
 In case others are wondering as Olle was:
 
 PJLIB-Util (part of pjproject) provides a DNS client which can
 optionally (but is highly suggested) to be used with PJSIP. It
 provides asynchronous DNS, SRV lookups, multiple record support,
 etc. Right now this isn't enabled so we are simply doing A/
 record lookups. The reason it's not enabled is that explicit
 nameservers *must* be provided to it when enabling it. It will not
 use the system ones by itself. The change up on reviewboard enables
 it by default using the system nameservers it finds, but with the
 ability to override or completely disable it if a user wants. The
 reason I also provide reload functionality is that people in
 #asterisk-dev expressed a concern that users may change nameservers
 but don't want to restart Asterisk, which is understandable.
 
 Interesting to get answer in another channel... For both of us.
 
 My question still stands - why would anyone want one part of Asterisk
 use other DNS servers than the rest of Asterisk and the rest of the
 system? If there is something wrong with the system resolver, that
 needs to be fixed.
 
 I do not see the need for us to have a configuration option here.
 Someone else may have a good reason for it.
 
 I can't guarantee that the code which automatically determines and gets the 
 nameservers from the system will work on all platforms. It's using res_init / 
 res_ninit which, depending on the implementation, parses resolv.conf and 
 stores the information. I think it *should* work but for cases where it won't 
 the ability to manually set them is there. For most people you don't even 
 need to know it exists or set it.

So we have yet another internal resolver? Is that a good thing? Why are we not 
using the system resolver?

We need to have some direction here. I think adding yet another DNS resolver is 
bad and will make it hard to add functions like DNSsec/DANE support. Making it 
possible for one piece of asterisk to use another DNS resolver is poor design, 
we should not open up for that.  The number of debug problems and runtime 
issues I see is reaching infinity. I can ping this server, but the sip channel 
can't reach it is just a start.

Is there a way we can either avoid using yet another resolver, or since this is 
propably better - switch all of Asterisk to use it and we'll get asynch DNS 
everywhere?

I can understand why a library like PJsip have support for this, in some cases 
it's the only way when writing clients like soft phones. That doesn't mean we 
have to expose it. We can still select and decide for ourselves about our 
design. 

Summary
- Don't add yet another DNS resolver randomly
- Don't allow using different DNS servers in parts of a server app and please 
don't allow anyone to configure anything else than the system resolvers in a 
server application.

/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

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

2014-03-13 Thread Michael L. Young

- Original Message -
 From: Olle E. Johansson o...@edvina.net
 To: Asterisk Developers Mailing List asterisk-dev@lists.digium.com

 So we have yet another internal resolver? Is that a good thing? Why
 are we not using the system resolver?

 We need to have some direction here. I think adding yet another DNS
 resolver is bad and will make it hard to add functions like
 DNSsec/DANE support. Making it possible for one piece of asterisk to
 use another DNS resolver is poor design, we should not open up for
 that.  The number of debug problems and runtime issues I see is
 reaching infinity. I can ping this server, but the sip channel
 can't reach it is just a start.

 Is there a way we can either avoid using yet another resolver, or
 since this is propably better - switch all of Asterisk to use it and
 we'll get asynch DNS everywhere?

 I can understand why a library like PJsip have support for this, in
 some cases it's the only way when writing clients like soft phones.
 That doesn't mean we have to expose it. We can still select and
 decide for ourselves about our design.

 Summary
 - Don't add yet another DNS resolver randomly
 - Don't allow using different DNS servers in parts of a server app
 and please don't allow anyone to configure anything else than the
 system resolvers in a server application.

I understand what Olle is getting at and I agree with it.  Why do we want 
res_pjsip using something different from the rest of Asterisk?  Everything 
should be using the same DNS resolver.  Otherwise, we are opening up a can of 
worms here when it comes to supporting Asterisk and trying to troubleshoot DNS 
issues.

This just doesn't seem like the right direction to go here.

Michael

-- 
_
-- 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] 3323: res_pjsip: Update PJSIP tests that evaluate ToS settings to go back to anticipating decimal values. Also fix another failure that is more recent.

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3323/#review11163
---

Ship it!

Ship It!

- Joshua Colp

On March 10, 2014, 7:13 p.m., Jonathan Rose wrote:

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3323/
---

(Updated March 10, 2014, 7:13 p.m.)

Review request for Asterisk Developers, Matt Jordan and Scott Griepentrog.

Repository: testsuite

Description
---

So in addition to ToS settings going back to using decimals for output after
the patch at https://reviewboard.asterisk.org/r/3324/, one of the PJSIP tests
was failing because the disallowed codecs field no longer appears in Sorcery
objects. This patch removes the check for the deny field... Just checking
the allow field gets the complete picture as far as I can tell.

This change in behavior was caused by r410190
pjsip: allow and disallow show same codecs

In order to prevent confusion over the allow and disallow
list of codecs being the same an option for registering a
field as an alias is added. The alias field will be read
from the configuration file, but afterwards is not listed
as a known field. With disallow set as an alias, the CLI
command pjsip show endpoint # will list the allow= field,
but not the disallow field.

Diffs
-

/asterisk/trunk/tests/channels/pjsip/dialplan_functions/pjsip_endpoint/configs/ast1/extensions.conf
4809
/asterisk/trunk/tests/channels/pjsip/ami/show_endpoint/test-config.yaml
4809

Diff: https://reviewboard.asterisk.org/r/3323/diff/

Testing
---

Ran tests against the patched version of Asterisk to check for success.
Success was achieved. Without the patch applied, tests still failed. Without
these changes to the testsuite, the tests failed in both versions. With just
the changes to the disallow check, this would pass without the patch to
change ToS values back to decimals.

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] 3324: res_pjsip: Make ToS values show up as decimals in sorcery object output


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3324/#review11164
---



/branches/12/res/res_pjsip/config_transport.c
https://reviewboard.asterisk.org/r/3324/#comment20800

Why not just use ast_asprintf for both of the to_str functions?


- Joshua Colp


On March 10, 2014, 7:04 p.m., Jonathan Rose wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3324/
 ---
 
 (Updated March 10, 2014, 7:04 p.m.)
 
 
 Review request for Asterisk Developers, George Joseph and Matt Jordan.
 
 
 Bugs: ASTERISK-23235
 https://issues.asterisk.org/jira/browse/ASTERISK-23235
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 ToS values were showing up as strings in the output for the sorcery objects 
 of res_pjsip endpoints and transports. This was causing test failures at one 
 time and is also just not a very useful way of using TOS values that aren't 
 from the predefined ToS names.  I also added a chart of named ToS values and 
 their decimal equivalents to 
 https://wiki.asterisk.org/wiki/display/AST/IP+Quality+of+Service This patch 
 makes it so that they are output as decimal values only in the sorcery 
 objects.
 
 
 Diffs
 -
 
   /branches/12/res/res_pjsip/pjsip_configuration.c 410367 
   /branches/12/res/res_pjsip/config_transport.c 410367 
   /branches/12/main/acl.c 410367 
   /branches/12/include/asterisk/acl.h 410367 
   /branches/12/CHANGES 410367 
 
 Diff: https://reviewboard.asterisk.org/r/3324/diff/
 
 
 Testing
 ---
 
 Tested output for all the named ToS values and some values without 
 equivalents.  Tested values above 255... since it uses the str2tos function, 
 this has a sort of wrap around behavior for anything above 255. Not sure how 
 appropriate that is, but it's been in place historically anyway. Updated 
 tests that were evaluating PJSIP tos settings.  That's in another review.
 
 
 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] 3340: ARI: Bridge Playback needs to have PlaybackStarted and PlaybackFinished messages get relayed to the bridge topic

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3340/#review11165
---

{quote}
I only receive the PlaybackStarted and PlaybackFinished events on the bridge
topic and not anything like BridgeEnter/Leave events or channel hangup events.
The reason for this I believe is because I'm pushing the opposite (unreal;1 vs
unreal;2) channel into the bridge from the one actually executing the playback
events and I'm canceling the forward before a hangup stasis message is
generated. At first I suspected I would receive channel hangup notifications on
account of hanging up the channel in the bridge prior to canceling the stasis
forward. I'm not sure if that would go against the intent of this patch or not,
but it doesn't appear to be the case... probably a matter of the stasis forward
being canceled before the playback channel hangs up.
{quote}

This is the expected behaviour.

The fact that we use a channel (which happens to be an unreal channel) to
inject media into the bridge is an internal implementation detail. From the
perspective of the ARI client, the media should be in relation to the bridge,
not some magical channel that shows up and then leaves that we never have
control over.

- Matt Jordan

On March 12, 2014, 1:02 p.m., Jonathan Rose wrote:

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3340/
---

(Updated March 12, 2014, 1:02 p.m.)

Review request for Asterisk Developers, Joshua Colp and Matt Jordan.

Bugs: ASTERISK-23444
https://issues.asterisk.org/jira/browse/ASTERISK-23444

Repository: Asterisk

Description
---

Diffs
-

/branches/12/res/ari/resource_bridges.c 410487

Diff: https://reviewboard.asterisk.org/r/3340/diff/

Testing
---

Before patch:

*crickets*

I also repeated this test with a PJSIP channel in the bridge to confirm that
the sounds were starting and stopping at the expected times.

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] 3331: Allows app_chanspy to whisper to a spyee's bridged peer (barge) even if the bridged party answers after initial spy invocation.


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3331/#review11167
---



/trunk/apps/app_chanspy.c
https://reviewboard.asterisk.org/r/3331/#comment20802

There's no need to assign 0 to bridge_connected here, as you already 
initialized it to 0 previously.



/trunk/apps/app_chanspy.c
https://reviewboard.asterisk.org/r/3331/#comment20803

Since the act of re-attaching the channel in barge mode uses the same code 
as the initial attachment, this could be refactored to place the act of 
attaching in a separate function.

Said function could return 0 on success or 1 on failure, which could be 
used to indicate whether or not the barger was previously attached.



/trunk/apps/app_chanspy.c
https://reviewboard.asterisk.org/r/3331/#comment20801

Coding guidelines:

if (!bridge_connected)

instead of:

if(bridge_connected == 0)

The space is necessary. Testing for negation using '!' is generally 
preferred over an explicit value.


- Matt Jordan


On March 11, 2014, 6:23 p.m., Robert Moss wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3331/
 ---
 
 (Updated March 11, 2014, 6:23 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-23381
 https://issues.asterisk.org/jira/browse/ASTERISK-23381
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Chanspy now can whisper to the spyee's bridged party (callee) (aka barging) 
 even if the Spy started before the bridged party answered.
 
 
 Diffs
 -
 
   /trunk/apps/app_chanspy.c 410469 
 
 Diff: https://reviewboard.asterisk.org/r/3331/diff/
 
 
 Testing
 ---
 
 Chanspy on an extension while the spyee's call is still ringing, after the 
 bridged party answers, both spied on parties can hear the monitor.
 
 
 Thanks,
 
 Robert Moss
 


-- 
_
-- 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] 3346: [res_config_pgsql] Correct handling of nullable int fields in update_realtime

2014-03-13 Thread zvision



 On March 13, 2014, 2:21 p.m., Matt Jordan wrote:
  Before this patch goes any further, it needs adequate testing.
  
  While posting a patch with 'it compiles' is a bare minimum requirement, for 
  it to be included seriously for inclusion the How this was tested field 
  should be filled in such that there is some confidence that the patch fixes 
  the reported problem.

Got it. That's why I put the explicit comment - to ensure someone interested at 
this issue will give it a try and report back.


- zvision


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3346/#review11166
---


On March 13, 2014, 10:06 a.m., zvision wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3346/
 ---
 
 (Updated March 13, 2014, 10:06 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-23351
 https://issues.asterisk.org/jira/browse/ASTERISK-23351
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This patch fixes handling of nullable int columns in update_realtime 
 function. It checks if a value is empty and sets the column to NULL instead 
 of '', which raises an error.
 Additionally, it checks for existence of the keyfield column instead of the 
 first parameter column.
 
 
 Diffs
 -
 
   http://svn.asterisk.org/svn/asterisk/branches/11/res/res_config_pgsql.c 
 410508 
 
 Diff: https://reviewboard.asterisk.org/r/3346/diff/
 
 
 Testing
 ---
 
 Only tested for successful compilation. Someone needs to confirm that the 
 patch works fine.
 
 
 Thanks,
 
 zvision
 


-- 
_
-- 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] 3338: ARI: Prevent corner cases where channel in bridge can wait forever for playback to finish


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3338/
---

(Updated March 13, 2014, 2:53 p.m.)


Review request for Asterisk Developers and rmudgett.


Changes
---

Get rid of some extra debug messages I left in.
Switch from a condition, lock, and boolean to a semaphore.


Repository: Asterisk


Description
---

ARI tests were occasionally showing a channel being stuck forever in Stasis. 
After looking at a live system where the problem was occurring, it became clear 
that the channel was stuck trying to wait for a playback to finish. However, it 
was also clear that the playback had been abandoned long ago.

When playing back a file to a channel that is in a bridge, the idea was to 
queue the playfile action onto the corresponding bridge channel, and then wait 
until the ARI custom playback function completed to signal that the playback 
had finished. There were several ways that this could fail:
* If the bridge channel were not found in the bridge, then we would never 
attempt to queue the playfile action, but we would still try to wait for it to 
finish happening.
* If queuing the playfile action did not succeed, then we would still attempt 
to wait for the action to occur.
* If the playback action was successfully queued, but the bridge channel were 
removed from the bridge before the playfile action could start, then the 
playback would never happen, and we'd wait forever.

The responsibility of knowing whether a playfile action has conpleted or ever 
will occur is the bridge's, since ARI can never fully know. With this in mind, 
I have created a new bridge channel API call to queue a playfile action 
synchronously. The way it is done, synchronous queuing operations could be 
created for other bridge actions quite easily. A new frametype, 
AST_FRAME_BRIDGE_ACTION_SYNC, contains a payload consisting of a 
synchronization ID and the actual payload of the bridge action that is to be 
queued. When the frame is queued, a synchronization structure is created and 
added to a linked list. The procedure then blocks until it is told that the 
frame has been disposed of.

When the frame gets freed (which will occur whether the playfile action 
succeeds or does not happen), the freer of the frame signals the waiting 
procedure that the playfile action has terminated, and control returns to 
whoever queued the playfile action in the first place.

From ARI's perspective, this greatly simplifies its code. Most of the 
res_stasis_playback changes are code removal. The bridge_channel changes, 
though, are pretty large, which is why I've included Richard on this issue.


Diffs (updated)
-

  /branches/12/res/res_stasis_playback.c 410467 
  /branches/12/main/channel.c 410467 
  /branches/12/main/bridge_channel.c 410467 
  /branches/12/include/asterisk/frame.h 410467 
  /branches/12/include/asterisk/bridge_channel.h 410467 
  /branches/12/funcs/func_frame_trace.c 410467 

Diff: https://reviewboard.asterisk.org/r/3338/diff/


Testing
---

Initially, I did some manual playbacks using Swagger-UI to a channel in a 
bridge to ensure that the specified file was actually being played as promised. 
I also queued up several files in quick succession to ensure that they played 
in series and not on top of each other.

In addition, I have created an automated test that is up for review at /r/3339


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] 3346: [res_config_pgsql] Correct handling of nullable int fields in update_realtime


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3346/#review11166
---


Before this patch goes any further, it needs adequate testing.

While posting a patch with 'it compiles' is a bare minimum requirement, for it 
to be included seriously for inclusion the How this was tested field should 
be filled in such that there is some confidence that the patch fixes the 
reported problem.

- Matt Jordan


On March 13, 2014, 5:06 a.m., zvision wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3346/
 ---
 
 (Updated March 13, 2014, 5:06 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-23351
 https://issues.asterisk.org/jira/browse/ASTERISK-23351
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This patch fixes handling of nullable int columns in update_realtime 
 function. It checks if a value is empty and sets the column to NULL instead 
 of '', which raises an error.
 Additionally, it checks for existence of the keyfield column instead of the 
 first parameter column.
 
 
 Diffs
 -
 
   http://svn.asterisk.org/svn/asterisk/branches/11/res/res_config_pgsql.c 
 410508 
 
 Diff: https://reviewboard.asterisk.org/r/3346/diff/
 
 
 Testing
 ---
 
 Only tested for successful compilation. Someone needs to confirm that the 
 patch works fine.
 
 
 Thanks,
 
 zvision
 


-- 
_
-- 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] 3343: res_pjsip: Enable DNS support.

2014-03-13 Thread Olle E Johansson


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3343/#review11170
---


We really should NOT add this code. Instead, we should clean up the code so all 
of Asterisk use the same resolver and relies on the system configuration. 
Having a separate DNS resolver, maybe even using a separate DNS server in the 
PJSIP channel is not a good solution and should be avoided at all cost. 

The good thing with this review is that it opened my eyes of some stuff in 
PJSIP that should not be there in the way we use it.

- Olle E Johansson


On March 13, 2014, 11:42 a.m., Joshua Colp wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3343/
 ---
 
 (Updated March 13, 2014, 11:42 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-23435
 https://issues.asterisk.org/jira/browse/ASTERISK-23435
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This change adds a configuration option for setting nameservers to be used by 
 the PJSIP DNS client. If this option is not set then the system nameservers 
 are retrieved and used instead.
 
 This also allows the nameservers to be changed by doing a reload.
 
 In case others are wondering as Olle was:
 
 PJLIB-Util (part of pjproject) provides a DNS client which can optionally 
 (but is highly suggested) to be used with PJSIP. It provides asynchronous 
 DNS, SRV lookups, multiple record support, etc. Right now this isn't enabled 
 so we are simply doing A/ record lookups. The reason it's not enabled is 
 that explicit nameservers *must* be provided to it when enabling it. It will 
 not use the system ones by itself. The change up on reviewboard enables it by 
 default using the system nameservers it finds, but with the ability to 
 override or completely disable it if a user wants. The reason I also provide 
 reload functionality is that people in #asterisk-dev expressed a concern that 
 users may change nameservers but don't want to restart Asterisk, which is 
 understandable. 
 
 
 Diffs
 -
 
   /branches/12/res/res_pjsip/pjsip_configuration.c 410470 
   /branches/12/res/res_pjsip/include/res_pjsip_private.h 410470 
   /branches/12/res/res_pjsip/config_global.c 410470 
   /branches/12/res/res_pjsip.c 410470 
   /branches/12/main/dns.c 410470 
   /branches/12/include/asterisk/dns.h 410470 
   /branches/12/configs/pjsip.conf.sample 410470 
   /branches/12/CHANGES 410470 
 
 Diff: https://reviewboard.asterisk.org/r/3343/diff/
 
 
 Testing
 ---
 
 Explicitly set nameservers and confirmed they were used by PJSIP. Disabled it 
 and confirmed that the DNS client was disabled. Set to auto (explicitly and 
 by default) and confirmed that the system nameservers were used.
 
 
 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] 3340: ARI: Bridge Playback needs to have PlaybackStarted and PlaybackFinished messages get relayed to the bridge topic

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3340/#review11171
---

The code changes themselves look good to me. I suggest taking your manual test
you did and translating it into a test for the testsuite.

- Mark Michelson

On March 12, 2014, 6:02 p.m., Jonathan Rose wrote:

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3340/
---

(Updated March 12, 2014, 6:02 p.m.)

Review request for Asterisk Developers, Joshua Colp and Matt Jordan.

Bugs: ASTERISK-23444
https://issues.asterisk.org/jira/browse/ASTERISK-23444

Repository: Asterisk

Description
---

Diffs
-

/branches/12/res/ari/resource_bridges.c 410487

Diff: https://reviewboard.asterisk.org/r/3340/diff/

Testing
---

Before patch:

*crickets*

I also repeated this test with a PJSIP channel in the bridge to confirm that
the sounds were starting and stopping at the expected times.

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] 3335: [res_config_odbc] Fix for nullable integer columns and keyfield existence check in update_odbc


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3335/#review11173
---

Ship it!


Excellent job fixing the problem!

A note to whoever ends up committing this: This bug also exists in 1.8, 12, and 
trunk. The 1.8 fix is exactly the same as this one. The 12 and trunk fixes will 
be slightly different since the update_odbc() callback takes an ast_variable 
list instead of va_args. The fix is still very similar though.

- Mark Michelson


On March 13, 2014, 10:09 a.m., zvision wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3335/
 ---
 
 (Updated March 13, 2014, 10:09 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-23459
 https://issues.asterisk.org/jira/browse/ASTERISK-23459
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This patch fixes setting nullable integer columns to NULL instead of an empty 
 string, which fails for PostgreSQL, for example.
 The current code is supposed to do so, but the check is broken. The patch 
 also allows the first column in the list to be a nullable integer.
 Also, the check for existence of a mandatory column checked for the first 
 column in the list instead of the key field lookup column.
 
 
 Diffs
 -
 
   http://svn.asterisk.org/svn/asterisk/branches/11/res/res_config_odbc.c 
 410444 
 
 Diff: https://reviewboard.asterisk.org/r/3335/diff/
 
 
 Testing
 ---
 
 Tested by me. Use case scenario: Asterisk + res_odbc + PostgreSQL backend, 
 SIP realtime peers + regs.
 When a 'port' column in SIP regs (I assume this also applies when using 
 sippeers only) is a nullable integer,
 Asterisk tries to write an empty string here during SIP endpoint 
 deregistration.
 
 
 Thanks,
 
 zvision
 


-- 
_
-- 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] 3340: ARI: Bridge Playback needs to have PlaybackStarted and PlaybackFinished messages get relayed to the bridge topic

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3340/
---

(Updated March 13, 2014, 10:56 a.m.)

Review request for Asterisk Developers, Joshua Colp and Matt Jordan.

Changes
---

Add recording events as well. It's the exact same logic.

Tested it as well:
RESPONSE:
{application:hello,type:RecordingStarted,recording:{state:recording,format:wav,name:fishsauce,target_uri:bridge:0dcf0f67-4af8-46bb-9b81-8d72bf09b952}}
RESPONSE:
{application:hello,type:RecordingFinished,recording:{state:done,format:wav,name:fishsauce,target_uri:bridge:0dcf0f67-4af8-46bb-9b81-8d72bf09b952}}

Bugs: ASTERISK-23444
https://issues.asterisk.org/jira/browse/ASTERISK-23444

Repository: Asterisk

Description
---

This patch adds a stasis forwarder to put events from the playback channel (the
side of the unreal channel chain responsible for playing sounds to the bridge)
into the bridge stasis topic. This way if you are subscribed to the bridge and
play sounds to it using ARI, you'll know when those sounds start and stop.

Diffs (updated)
-

/branches/12/res/ari/resource_bridges.c 410487

Diff: https://reviewboard.asterisk.org/r/3340/diff/

Testing
---

Before patch:

*crickets*

I also repeated this test with a PJSIP channel in the bridge to confirm that
the sounds were starting and stopping at the expected times.

I only receive the PlaybackStarted and PlaybackFinished events on the bridge
topic and not anything like BridgeEnter/Leave events or channel hangup events.
The reason for this I believe is because I'm pushing the opposite (unreal;1 vs
unreal;2) channel into the bridge from the one actually executing the playback
events and I'm canceling the forward before a hangup stasis message is
generated. At first I suspected I would receive channel hangup notifications on
account of hanging up the channel in the bridge prior to canceling the stasis
forward. I'm not sure if that would go against the intent of this patch or not,
but it doesn't appear to be the case... probably a matter of the stasis forward
being canceled before the playback channel hangs up.

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] 3340: ARI: Bridge Playback needs to have PlaybackStarted and PlaybackFinished messages get relayed to the bridge topic (also recording)

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3340/#review11174
---

Ship it!

Ship It!

/branches/12/res/ari/resource_bridges.c
https://reviewboard.asterisk.org/r/3340/#comment20804

Speeling mistake.

- Joshua Colp

On March 13, 2014, 3:57 p.m., Jonathan Rose wrote:

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3340/
---

(Updated March 13, 2014, 3:57 p.m.)

Review request for Asterisk Developers, Joshua Colp and Matt Jordan.

Bugs: ASTERISK-23444
https://issues.asterisk.org/jira/browse/ASTERISK-23444

Repository: Asterisk

Description
---

Diffs
-

/branches/12/res/ari/resource_bridges.c 410487

Diff: https://reviewboard.asterisk.org/r/3340/diff/

Testing
---

Before patch:

*crickets*

I also repeated this test with a PJSIP channel in the bridge to confirm that
the sounds were starting and stopping at the expected times.

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] 3340: ARI: Bridge Playback needs to have PlaybackStarted and PlaybackFinished messages get relayed to the bridge topic (also recording)

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3340/
---

(Updated March 13, 2014, 10:57 a.m.)

Review request for Asterisk Developers, Joshua Colp and Matt Jordan.

Summary (updated)
-

ARI: Bridge Playback needs to have PlaybackStarted and PlaybackFinished
messages get relayed to the bridge topic (also recording)

Bugs: ASTERISK-23444
https://issues.asterisk.org/jira/browse/ASTERISK-23444

Repository: Asterisk

Description
---

This patch adds a stasis forwarder to put events from the playback channel (the
side of the unreal channel chain responsible for playing sounds to the bridge)
into the bridge stasis topic. This way if you are subscribed to the bridge and
play sounds to it using ARI, you'll know when those sounds start and stop.

Diffs
-

/branches/12/res/ari/resource_bridges.c 410487

Diff: https://reviewboard.asterisk.org/r/3340/diff/

Testing
---

Before patch:

*crickets*

I also repeated this test with a PJSIP channel in the bridge to confirm that
the sounds were starting and stopping at the expected times.

I only receive the PlaybackStarted and PlaybackFinished events on the bridge
topic and not anything like BridgeEnter/Leave events or channel hangup events.
The reason for this I believe is because I'm pushing the opposite (unreal;1 vs
unreal;2) channel into the bridge from the one actually executing the playback
events and I'm canceling the forward before a hangup stasis message is
generated. At first I suspected I would receive channel hangup notifications on
account of hanging up the channel in the bridge prior to canceling the stasis
forward. I'm not sure if that would go against the intent of this patch or not,
but it doesn't appear to be the case... probably a matter of the stasis forward
being canceled before the playback channel hangs up.

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] 3324: res_pjsip: Make ToS values show up as decimals in sorcery object output



 On March 13, 2014, 7:32 a.m., Joshua Colp wrote:
  /branches/12/res/res_pjsip/config_transport.c, lines 505-518
  https://reviewboard.asterisk.org/r/3324/diff/1/?file=55753#file55753line505
 
  Why not just use ast_asprintf for both of the to_str functions?

Because... sometimes I forget which string functions exist in which languages 
and which ones we have handlers for in our memory manager.
In short, _


- Jonathan


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3324/#review11164
---


On March 10, 2014, 2:04 p.m., Jonathan Rose wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3324/
 ---
 
 (Updated March 10, 2014, 2:04 p.m.)
 
 
 Review request for Asterisk Developers, George Joseph and Matt Jordan.
 
 
 Bugs: ASTERISK-23235
 https://issues.asterisk.org/jira/browse/ASTERISK-23235
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 ToS values were showing up as strings in the output for the sorcery objects 
 of res_pjsip endpoints and transports. This was causing test failures at one 
 time and is also just not a very useful way of using TOS values that aren't 
 from the predefined ToS names.  I also added a chart of named ToS values and 
 their decimal equivalents to 
 https://wiki.asterisk.org/wiki/display/AST/IP+Quality+of+Service This patch 
 makes it so that they are output as decimal values only in the sorcery 
 objects.
 
 
 Diffs
 -
 
   /branches/12/res/res_pjsip/pjsip_configuration.c 410367 
   /branches/12/res/res_pjsip/config_transport.c 410367 
   /branches/12/main/acl.c 410367 
   /branches/12/include/asterisk/acl.h 410367 
   /branches/12/CHANGES 410367 
 
 Diff: https://reviewboard.asterisk.org/r/3324/diff/
 
 
 Testing
 ---
 
 Tested output for all the named ToS values and some values without 
 equivalents.  Tested values above 255... since it uses the str2tos function, 
 this has a sort of wrap around behavior for anything above 255. Not sure how 
 appropriate that is, but it's been in place historically anyway. Updated 
 tests that were evaluating PJSIP tos settings.  That's in another review.
 
 
 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] 3324: res_pjsip: Make ToS values show up as decimals in sorcery object output


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3324/
---

(Updated March 13, 2014, 11:11 a.m.)


Review request for Asterisk Developers, George Joseph and Matt Jordan.


Changes
---

Use ast_asprintf


Bugs: ASTERISK-23235
https://issues.asterisk.org/jira/browse/ASTERISK-23235


Repository: Asterisk


Description
---

ToS values were showing up as strings in the output for the sorcery objects of 
res_pjsip endpoints and transports. This was causing test failures at one time 
and is also just not a very useful way of using TOS values that aren't from the 
predefined ToS names.  I also added a chart of named ToS values and their 
decimal equivalents to 
https://wiki.asterisk.org/wiki/display/AST/IP+Quality+of+Service This patch 
makes it so that they are output as decimal values only in the sorcery objects.


Diffs (updated)
-

  /branches/12/res/res_pjsip/pjsip_configuration.c 410367 
  /branches/12/res/res_pjsip/config_transport.c 410367 
  /branches/12/main/acl.c 410367 
  /branches/12/include/asterisk/acl.h 410367 
  /branches/12/CHANGES 410367 

Diff: https://reviewboard.asterisk.org/r/3324/diff/


Testing
---

Tested output for all the named ToS values and some values without equivalents. 
 Tested values above 255... since it uses the str2tos function, this has a sort 
of wrap around behavior for anything above 255. Not sure how appropriate that 
is, but it's been in place historically anyway. Updated tests that were 
evaluating PJSIP tos settings.  That's in another review.


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] 3324: res_pjsip: Make ToS values show up as decimals in sorcery object output


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3324/#review11176
---

Ship it!


Ship It!

- Joshua Colp


On March 13, 2014, 4:11 p.m., Jonathan Rose wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3324/
 ---
 
 (Updated March 13, 2014, 4:11 p.m.)
 
 
 Review request for Asterisk Developers, George Joseph and Matt Jordan.
 
 
 Bugs: ASTERISK-23235
 https://issues.asterisk.org/jira/browse/ASTERISK-23235
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 ToS values were showing up as strings in the output for the sorcery objects 
 of res_pjsip endpoints and transports. This was causing test failures at one 
 time and is also just not a very useful way of using TOS values that aren't 
 from the predefined ToS names.  I also added a chart of named ToS values and 
 their decimal equivalents to 
 https://wiki.asterisk.org/wiki/display/AST/IP+Quality+of+Service This patch 
 makes it so that they are output as decimal values only in the sorcery 
 objects.
 
 
 Diffs
 -
 
   /branches/12/res/res_pjsip/pjsip_configuration.c 410367 
   /branches/12/res/res_pjsip/config_transport.c 410367 
   /branches/12/main/acl.c 410367 
   /branches/12/include/asterisk/acl.h 410367 
   /branches/12/CHANGES 410367 
 
 Diff: https://reviewboard.asterisk.org/r/3324/diff/
 
 
 Testing
 ---
 
 Tested output for all the named ToS values and some values without 
 equivalents.  Tested values above 255... since it uses the str2tos function, 
 this has a sort of wrap around behavior for anything above 255. Not sure how 
 appropriate that is, but it's been in place historically anyway. Updated 
 tests that were evaluating PJSIP tos settings.  That's in another review.
 
 
 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] 3339: Testsuite: ARI test for playback of audio to a channel in a bridge.

2014-03-13 Thread Kevin Harwell


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3339/#review11178
---



/asterisk/trunk/tests/rest_api/playback/to_channel_in_bridge/play_file.py
https://reviewboard.asterisk.org/r/3339/#comment20816

The debug statement might not be bad for all tests and could be added to 
the ari test object alleviating the need for this function and associated 
decorators.



/asterisk/trunk/tests/rest_api/playback/to_channel_in_bridge/test-config.yaml
https://reviewboard.asterisk.org/r/3339/#comment20817

Remove commented settings if not needed.


- Kevin Harwell


On March 12, 2014, 12:31 p.m., Mark Michelson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3339/
 ---
 
 (Updated March 12, 2014, 12:31 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: testsuite
 
 
 Description
 ---
 
 This was created to test the changes in /r/3338
 
 This test is pretty simple. A channel is placed into an ARI bridge, and two 
 playbacks are queued up in quick succession. The test ensures that the files 
 play and that the first file plays and finishes before the second file plays 
 and finishes.
 
 
 Diffs
 -
 
   
 /asterisk/trunk/tests/rest_api/playback/to_channel_in_bridge/test-config.yaml 
 PRE-CREATION 
   /asterisk/trunk/tests/rest_api/playback/to_channel_in_bridge/play_file.py 
 PRE-CREATION 
   /asterisk/trunk/tests/rest_api/playback/tests.yaml 4836 
 
 Diff: https://reviewboard.asterisk.org/r/3339/diff/
 
 
 Testing
 ---
 
 
 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

[asterisk-dev] [Code Review] 3348: Test Suite: MWI subscription test for PJSIP

2014-03-13 Thread jbigelow


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3348/
---

Review request for Asterisk Developers.


Bugs: ASTERISK-23343
https://issues.asterisk.org/jira/browse/ASTERISK-23343


Repository: testsuite


Description
---

This test depends on some changes in the PJSIP python library pjsua.py. The 
plan is to submit this upstream. See in-line with this description below.

This basic nominal test ensures that a mailbox on an AOR for an enpoint can be 
subscribed to for MWI and that a NOTIFY is received. It creates endpoint 
alice with subscribing to MWI. Upon receiving a notify for message summary 
indicating no messages the test will be marked as passed otherwise it will be 
failed.

The other two tests on ASTERISK-23343 aren't currently possible with the PJSUA 
API v1.

Note: the following patch for pjsua.py adds modify_account() which isn't 
currently used by the testsuite but was added for possible future use.

Index: pjsip-apps/src/python/pjsua.py
===
--- pjsip-apps/src/python/pjsua.py  (revision 4791)
+++ pjsip-apps/src/python/pjsua.py  (working copy)
@@ -775,6 +775,7 @@
 use_srtp = 0
 srtp_secure_signaling = 1
 rtp_transport_cfg = None
+mwi_enabled = False
 
 def __init__(self, domain=, username=, password=, 
  display=, registrar=, proxy=):
@@ -865,6 +866,7 @@
 self.ka_data = cfg.ka_data
 self.use_srtp = cfg.use_srtp
 self.srtp_secure_signaling = cfg.srtp_secure_signaling
+self.mwi_enabled = cfg.mwi_enabled
 if (self.rtp_transport_cfg is not None):
 self.rtp_transport_cfg._cvt_from_pjsua(cfg.rtp_transport_cfg)
 
@@ -896,6 +898,7 @@
 cfg.ka_data = self.ka_data
 cfg.use_srtp = self.use_srtp
 cfg.srtp_secure_signaling = self.srtp_secure_signaling
+cfg.mwi_enabled = self.mwi_enabled
 
 if (self.rtp_transport_cfg is not None):
 cfg.rtp_transport_cfg = self.rtp_transport_cfg._cvt_to_pjsua()
@@ -2337,6 +2340,18 @@
 self._err_check(create_account_for_transport(), self, err)
 return Account(self, acc_id, cb)
 
+def modify_account(self, acc_id, acc_config):
+Modify configuration of a pjsua account.
+
+Keyword arguments:
+acc_id  -- ID of the account to be modified.
+acc_config  -- New account configuration.
+
+
+lck = self.auto_lock()
+err = _pjsua.acc_modify(acc_id, acc_config._cvt_to_pjsua())
+self._err_check(modify_account(), self, err)
+
 def hangup_all(self):
 Hangup all calls.


Diffs
-

  /asterisk/trunk/tests/channels/pjsip/subscriptions/mwi/tests.yaml 4836 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/mwi/basic_mailbox_subscribe/test-config.yaml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/mwi/basic_mailbox_subscribe/subscribe.py
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/mwi/basic_mailbox_subscribe/configs/ast1/pjsip.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/mwi/basic_mailbox_subscribe/configs/ast1/modules.conf
 PRE-CREATION 
  /asterisk/trunk/lib/python/asterisk/pjsua_mod.py 4836 

Diff: https://reviewboard.asterisk.org/r/3348/diff/


Testing
---

* Ensured tests pass on multiple executions
* Ensured the testsuite  Asterisk logs looked good.


Thanks,

jbigelow

-- 
_
-- 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] 3325: res_mwi_external: Clear the stasis cache entry when the external MWI is deleted.


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3325/#review11179
---

Ship it!


My only issues are with documentation. Otherwise, it's good to go.


/branches/12/include/asterisk/app.h
https://reviewboard.asterisk.org/r/3325/#comment20820

Mailbox is not necessarily a number. It can be any string.

(same comment applies to the doxygen for mwi_state_create_message() in 
app.c)



/branches/12/main/app.c
https://reviewboard.asterisk.org/r/3325/#comment20819

Description is incomplete.


- Mark Michelson


On March 10, 2014, 9:24 p.m., rmudgett wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3325/
 ---
 
 (Updated March 10, 2014, 9:24 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 One of the things missing when external MWI support was added was the ability 
 to clear the stasis cache entry of deleted external MWI mailboxes.
 
 
 Diffs
 -
 
   /branches/12/res/res_mwi_external.c 410444 
   /branches/12/main/app.c 410444 
   /branches/12/include/asterisk/app.h 410444 
 
 Diff: https://reviewboard.asterisk.org/r/3325/diff/
 
 
 Testing
 ---
 
 Added temporary debug messages and observed that the stasis cache entry was 
 removed when the external MWI mailbox was deleted.
 
 
 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] 3338: ARI: Prevent corner cases where channel in bridge can wait forever for playback to finish


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3338/#review11177
---


You need to add cases to the switches that handle AST_FRAME_BRIDGE_ACTION in 
frame.c and bridge_softmix.c.  It is probably best if the 
AST_FRAME_BRIDGE_ACTION_SYNC case is next to the AST_BRIDGE_ACTION case.

The main difficulty with this patch is that you are adding an allocated 
resource to ast_frame's which inherently doesn't support this.


/branches/12/main/bridge_channel.c
https://reviewboard.asterisk.org/r/3338/#comment20814

Should this be static?



/branches/12/main/bridge_channel.c
https://reviewboard.asterisk.org/r/3338/#comment20806

This is a comment to what?



/branches/12/main/bridge_channel.c
https://reviewboard.asterisk.org/r/3338/#comment20815

Should be made static.

Just because _STATIC is in the name doesn't mean that the macro includes 
the static keyword. :)



/branches/12/main/bridge_channel.c
https://reviewboard.asterisk.org/r/3338/#comment20808

Moving this struct defintion looks to be a left-over before you made the 
generic sync action frame type.



/branches/12/main/bridge_channel.c
https://reviewboard.asterisk.org/r/3338/#comment20807

If you're going to break long lines, I would prefer the continuation line 
only indent by one level and not more.  Horizontal space is a precious 
commodity.



/branches/12/main/bridge_channel.c
https://reviewboard.asterisk.org/r/3338/#comment20809

Another lefover change.



/branches/12/main/bridge_channel.c
https://reviewboard.asterisk.org/r/3338/#comment20810

The \param must match the function param name.



/branches/12/main/channel.c
https://reviewboard.asterisk.org/r/3338/#comment20811

Astetically, it is probably best if the AST_FRAME_BRIDGE_ACTION_SYNC case 
is next to the AST_BRIDGE_ACTION case.



/branches/12/main/channel.c
https://reviewboard.asterisk.org/r/3338/#comment20812

idem



/branches/12/res/res_stasis_playback.c
https://reviewboard.asterisk.org/r/3338/#comment20813

bridge_chan is only valid while the bridge is locked here.  
bridge_find_channel() does not give you a reference to bridge_chan so it can 
dissapear on you.


- rmudgett


On March 13, 2014, 9:53 a.m., Mark Michelson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3338/
 ---
 
 (Updated March 13, 2014, 9:53 a.m.)
 
 
 Review request for Asterisk Developers and rmudgett.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 ARI tests were occasionally showing a channel being stuck forever in Stasis. 
 After looking at a live system where the problem was occurring, it became 
 clear that the channel was stuck trying to wait for a playback to finish. 
 However, it was also clear that the playback had been abandoned long ago.
 
 When playing back a file to a channel that is in a bridge, the idea was to 
 queue the playfile action onto the corresponding bridge channel, and then 
 wait until the ARI custom playback function completed to signal that the 
 playback had finished. There were several ways that this could fail:
 * If the bridge channel were not found in the bridge, then we would never 
 attempt to queue the playfile action, but we would still try to wait for it 
 to finish happening.
 * If queuing the playfile action did not succeed, then we would still attempt 
 to wait for the action to occur.
 * If the playback action was successfully queued, but the bridge channel were 
 removed from the bridge before the playfile action could start, then the 
 playback would never happen, and we'd wait forever.
 
 The responsibility of knowing whether a playfile action has conpleted or ever 
 will occur is the bridge's, since ARI can never fully know. With this in 
 mind, I have created a new bridge channel API call to queue a playfile action 
 synchronously. The way it is done, synchronous queuing operations could be 
 created for other bridge actions quite easily. A new frametype, 
 AST_FRAME_BRIDGE_ACTION_SYNC, contains a payload consisting of a 
 synchronization ID and the actual payload of the bridge action that is to be 
 queued. When the frame is queued, a synchronization structure is created and 
 added to a linked list. The procedure then blocks until it is told that the 
 frame has been disposed of.
 
 When the frame gets freed (which will occur whether the playfile action 
 succeeds or does not happen), the freer of the frame signals the waiting 
 procedure that the playfile action has terminated, and control returns to 
 whoever queued the playfile action in the first place.
 
 From ARI's perspective, this greatly simplifies its code. Most of the 
 res_stasis_playback changes are code removal.

[asterisk-dev] [Code Review] 3350: Add AES-GCM support for SRTP

2014-03-13 Thread Kristian Kielhofner


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3350/
---

Review request for Asterisk Developers.


Bugs: ASTERISK-22832
https://issues.asterisk.org/jira/browse/ASTERISK-22832


Repository: Asterisk


Description
---

There is a version of libsrtp that supports AES-NI and AES-GCM mode:
https://github.com/cisco/libsrtp/pull/34

More on AES-GCM mode:
http://tools.ietf.org/html/draft-ietf-avtcore-srtp-aes-gcm-10
http://2013.diac.cr.yp.to/slides/gueron.pdf

AES-GCM mode improves the performance of SRTP on systems with and without 
support for the AES-NI instruction set.

This patch implements 128 bit AES GCM mode with SRTP. Significantly more work 
will be required to support 192 and 256 bit AES regardless of mode. Various 
build stuffs will also need to be updated with the required checks for AES-GCM 
support in libsrtp and OpenSSL.

Big AES (including 256 GCM) should probably be implemented with a separate 
patch/bug/review:

http://tools.ietf.org/html/rfc6188


Diffs
-

  /trunk/res/res_srtp.c 402525 
  /trunk/main/sdp_srtp.c 402525 
  /trunk/include/asterisk/sdp_srtp.h 402525 
  /trunk/include/asterisk/res_srtp.h 402525 

Diff: https://reviewboard.asterisk.org/r/3350/diff/


Testing
---

Successfully tested call setup and audio exchange with patched pjsip client and 
FreeSWITCH.


Thanks,

Kristian Kielhofner

-- 
_
-- 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] 3325: res_mwi_external: Clear the stasis cache entry when the external MWI is deleted.


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3325/
---

(Updated March 13, 2014, 1:23 p.m.)


Review request for Asterisk Developers.


Changes
---

Addressed Mark's doxygen comments.


Repository: Asterisk


Description
---

One of the things missing when external MWI support was added was the ability 
to clear the stasis cache entry of deleted external MWI mailboxes.


Diffs (updated)
-

  /branches/12/res/res_mwi_external.c 410525 
  /branches/12/main/app.c 410525 
  /branches/12/include/asterisk/app.h 410525 

Diff: https://reviewboard.asterisk.org/r/3325/diff/


Testing
---

Added temporary debug messages and observed that the stasis cache entry was 
removed when the external MWI mailbox was deleted.


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] 3349: Implement RFC-3966 TEL URI incoming INVITE

2014-03-13 Thread Geert Van Pamel

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3349/
---

(Updated March 13, 2014, 7:41 p.m.)

Review request for Asterisk Developers, Corey Farrell, lmadensen, Matt Jordan,
and wdoekes.

Changes
---

Correct reference to JIRA + Reviewer asterisk-dev group

Bugs: ASTERISK-17179
https://issues.asterisk.org/jira/browse/ASTERISK-17179

Repository: Asterisk

Description
---

Implements RFC-3966 TEL URI incoming INVITE.

See https://issues.asterisk.org/jira/browse/ASTERISK-17179 for a description of
the original isssue.

I have been patching all versions since Asterisk 1.6. I would like to include
the code into the main trunk for version 13.

Previously Asterisk was failing with error on incoming IMS call:

Nov 13 17:52:05 NOTICE[27459]: chan_sip.c:6973 check_user_full: From address
missing 'sip:', using it anyway

Nov 13 17:52:05 WARNING[27459]: chan_sip.c:6525 get_destination: Huh? Not a SIP
header (tel:0987654321;phone-context=+32987654321)?

Reason: tel: protocol was not recognized.

Diffs
-

/trunk/channels/sip/reqresp_parser.c 410429
/trunk/channels/chan_sip.c 410429

Diff: https://reviewboard.asterisk.org/r/3349/diff/

Testing
---

Executed an incoming TEL URI INVITE connection.
CLI was present on the display and in the CDR file.
No errors on SIP debug output.

File Attachments

RFC-3966 tel URI patch

https://reviewboard.asterisk.org/media/uploaded/files/2014/03/13/cad7a996-88c1-47fe-a2a9-cc6987af3b75__rfc-3966-tel-uri-patch-diff.txt

Thanks,

Geert Van Pamel

--
_
-- 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] 3209: Crash in ast_format_cmp on shutdown


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3209/#review11180
---

Ship it!


Short of protecting the global interfaces container pointer with ao2_global_obj 
which can be expensive CPU wise or leaking it on shutdown there is no way to 
completely prevent a crash from a NULL interfaces pointer.  All this patch can 
do is minimize the chance of a crash.


/branches/11/main/format.c
https://reviewboard.asterisk.org/r/3209/#comment20821

Nit.  Please put a blank line after every variable declaration section to 
separate variable declaration from normal code.


- rmudgett


On March 13, 2014, 12:37 p.m., Scott Griepentrog wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3209/
 ---
 
 (Updated March 13, 2014, 12:37 p.m.)
 
 
 Review request for Asterisk Developers, Corey Farrell and n8ideas.
 
 
 Bugs: ASTERISK-23103
 https://issues.asterisk.org/jira/browse/ASTERISK-23103
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Alternate method to more safely shutdown interfaces container.  Set interface 
 global to null first to avoid possible race condition, and also double check 
 interfaces prior to all uses.
 
 
 Diffs
 -
 
   /branches/11/main/format.c 410525 
 
 Diff: https://reviewboard.asterisk.org/r/3209/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Scott Griepentrog
 


-- 
_
-- 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] 3338: ARI: Prevent corner cases where channel in bridge can wait forever for playback to finish



 On March 13, 2014, 5:43 p.m., rmudgett wrote:
  You need to add cases to the switches that handle AST_FRAME_BRIDGE_ACTION 
  in frame.c and bridge_softmix.c.  It is probably best if the 
  AST_FRAME_BRIDGE_ACTION_SYNC case is next to the AST_BRIDGE_ACTION case.
  
  The main difficulty with this patch is that you are adding an allocated 
  resource to ast_frame's which inherently doesn't support this.

I don't understand the comment about adding an allocated resource to ast_frame. 
The frame payload is allocated on the stack, but that's the same strategy used 
for all bridge_playfile payloads right now. The bridge_playfile is memcpy'd 
into the sync_payload, and the sync_payload's data length is adjusted to 
account for this. When ast_frdup() copies the frame data to another frame, the 
entire sync_payload and encased bridge_playfile is copied to the other frame, 
so I'm not sure what the problem here is.


- Mark


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3338/#review11177
---


On March 13, 2014, 2:53 p.m., Mark Michelson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3338/
 ---
 
 (Updated March 13, 2014, 2:53 p.m.)
 
 
 Review request for Asterisk Developers and rmudgett.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 ARI tests were occasionally showing a channel being stuck forever in Stasis. 
 After looking at a live system where the problem was occurring, it became 
 clear that the channel was stuck trying to wait for a playback to finish. 
 However, it was also clear that the playback had been abandoned long ago.
 
 When playing back a file to a channel that is in a bridge, the idea was to 
 queue the playfile action onto the corresponding bridge channel, and then 
 wait until the ARI custom playback function completed to signal that the 
 playback had finished. There were several ways that this could fail:
 * If the bridge channel were not found in the bridge, then we would never 
 attempt to queue the playfile action, but we would still try to wait for it 
 to finish happening.
 * If queuing the playfile action did not succeed, then we would still attempt 
 to wait for the action to occur.
 * If the playback action was successfully queued, but the bridge channel were 
 removed from the bridge before the playfile action could start, then the 
 playback would never happen, and we'd wait forever.
 
 The responsibility of knowing whether a playfile action has conpleted or ever 
 will occur is the bridge's, since ARI can never fully know. With this in 
 mind, I have created a new bridge channel API call to queue a playfile action 
 synchronously. The way it is done, synchronous queuing operations could be 
 created for other bridge actions quite easily. A new frametype, 
 AST_FRAME_BRIDGE_ACTION_SYNC, contains a payload consisting of a 
 synchronization ID and the actual payload of the bridge action that is to be 
 queued. When the frame is queued, a synchronization structure is created and 
 added to a linked list. The procedure then blocks until it is told that the 
 frame has been disposed of.
 
 When the frame gets freed (which will occur whether the playfile action 
 succeeds or does not happen), the freer of the frame signals the waiting 
 procedure that the playfile action has terminated, and control returns to 
 whoever queued the playfile action in the first place.
 
 From ARI's perspective, this greatly simplifies its code. Most of the 
 res_stasis_playback changes are code removal. The bridge_channel changes, 
 though, are pretty large, which is why I've included Richard on this issue.
 
 
 Diffs
 -
 
   /branches/12/res/res_stasis_playback.c 410467 
   /branches/12/main/channel.c 410467 
   /branches/12/main/bridge_channel.c 410467 
   /branches/12/include/asterisk/frame.h 410467 
   /branches/12/include/asterisk/bridge_channel.h 410467 
   /branches/12/funcs/func_frame_trace.c 410467 
 
 Diff: https://reviewboard.asterisk.org/r/3338/diff/
 
 
 Testing
 ---
 
 Initially, I did some manual playbacks using Swagger-UI to a channel in a 
 bridge to ensure that the specified file was actually being played as 
 promised. I also queued up several files in quick succession to ensure that 
 they played in series and not on top of each other.
 
 In addition, I have created an automated test that is up for review at /r/3339
 
 
 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] 3338: ARI: Prevent corner cases where channel in bridge can wait forever for playback to finish


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3338/
---

(Updated March 13, 2014, 7 p.m.)


Review request for Asterisk Developers and rmudgett.


Changes
---

Address Richard's feedback


Repository: Asterisk


Description
---

ARI tests were occasionally showing a channel being stuck forever in Stasis. 
After looking at a live system where the problem was occurring, it became clear 
that the channel was stuck trying to wait for a playback to finish. However, it 
was also clear that the playback had been abandoned long ago.

When playing back a file to a channel that is in a bridge, the idea was to 
queue the playfile action onto the corresponding bridge channel, and then wait 
until the ARI custom playback function completed to signal that the playback 
had finished. There were several ways that this could fail:
* If the bridge channel were not found in the bridge, then we would never 
attempt to queue the playfile action, but we would still try to wait for it to 
finish happening.
* If queuing the playfile action did not succeed, then we would still attempt 
to wait for the action to occur.
* If the playback action was successfully queued, but the bridge channel were 
removed from the bridge before the playfile action could start, then the 
playback would never happen, and we'd wait forever.

The responsibility of knowing whether a playfile action has conpleted or ever 
will occur is the bridge's, since ARI can never fully know. With this in mind, 
I have created a new bridge channel API call to queue a playfile action 
synchronously. The way it is done, synchronous queuing operations could be 
created for other bridge actions quite easily. A new frametype, 
AST_FRAME_BRIDGE_ACTION_SYNC, contains a payload consisting of a 
synchronization ID and the actual payload of the bridge action that is to be 
queued. When the frame is queued, a synchronization structure is created and 
added to a linked list. The procedure then blocks until it is told that the 
frame has been disposed of.

When the frame gets freed (which will occur whether the playfile action 
succeeds or does not happen), the freer of the frame signals the waiting 
procedure that the playfile action has terminated, and control returns to 
whoever queued the playfile action in the first place.

From ARI's perspective, this greatly simplifies its code. Most of the 
res_stasis_playback changes are code removal. The bridge_channel changes, 
though, are pretty large, which is why I've included Richard on this issue.


Diffs (updated)
-

  /branches/12/res/res_stasis_playback.c 410467 
  /branches/12/main/frame.c 410467 
  /branches/12/main/channel.c 410467 
  /branches/12/main/bridge_channel.c 410467 
  /branches/12/include/asterisk/frame.h 410467 
  /branches/12/include/asterisk/bridge_channel.h 410467 
  /branches/12/funcs/func_frame_trace.c 410467 
  /branches/12/bridges/bridge_softmix.c 410467 

Diff: https://reviewboard.asterisk.org/r/3338/diff/


Testing
---

Initially, I did some manual playbacks using Swagger-UI to a channel in a 
bridge to ensure that the specified file was actually being played as promised. 
I also queued up several files in quick succession to ensure that they played 
in series and not on top of each other.

In addition, I have created an automated test that is up for review at /r/3339


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] 3343: res_pjsip: Enable DNS support.



 On March 13, 2014, 10:31 a.m., Olle E Johansson wrote:
  We really should NOT add this code. Instead, we should clean up the code so 
  all of Asterisk use the same resolver and relies on the system 
  configuration. Having a separate DNS resolver, maybe even using a separate 
  DNS server in the PJSIP channel is not a good solution and should be 
  avoided at all cost. 
  
  The good thing with this review is that it opened my eyes of some stuff in 
  PJSIP that should not be there in the way we use it.

DNS support in Asterisk has had problems for a long time and is something that 
I think we would all like to see improved upon. The state of such support as it 
exists in the core today is limited: modules and the core can either use dnsmgr 
– which only gives you a single IP address result but does make an attempt at 
querying and managing the DNS records – or they can use ast_gethostbyname, 
which is a wrapper around a reentrant version of gethostbyname. In the case of 
the former, users get a small (albeit limited) benefit of having the records be 
managed internally; in the case of the latter users only get the standard 
gethostbyname behavior. Neither option provides asynchronous DNS querying, nor 
does either option providethe ability to handle multiple SRV records.

The situation is the way it is today not because parsing and returning multiple 
DNS records is particularly hard, or because properly weighting the results is 
extremely difficult. Even making the querying of records asynchronous is 
doable. While not trivial changes, they are well understood problems and the 
API in dns.h/srv.h/dnsmgr.h could be modified accordingly. The hard part is 
making use of the DNS information.

The legacy IP based channel drivers in Asterisk are not structured in such a 
fashion that the location of the thing that Asterisk communicates with is 
separate from the IP address. This was an unfortunate design decision; 
separating out the concept of location from the IP address is now extremely 
non-trivial. I can't stress this enough: it is an incredibly large task to go 
do this work – particularly when we consider the size and complexity of those 
channel drivers. Unfortunately, chan_sip and chan_iax2 are both places where 
this would be beneficial, but also cases where making use of multiple SRV 
records or asynchronous DNS would be extremely challenging.

That gets to the crux of the problem: even if we had the world's best DNS 
resolver in the Asterisk core, the vast majority of places in Asterisk that you 
would like to make use of it would not easily be able to do so.

There is, of course, benefit to having better DNS support in the core of 
Asterisk outside of those channel drivers. New things would be able to make use 
of it easier, and improvements in the DNS support would be shared by all things 
that use it. I think it would be great to see patches that improve the DNS 
support in the core. This should include asynchronous querying, multiple SRV 
records, as well as other cool kinds of things such as DANE. At the same time, 
since the vast majority of existing channel drivers can't make use of that 
information, it is of admittedly limited benefit unless other changes take 
place.

This is, unfortunately, one of those places where making use of a third party 
stack does make things more difficult for us. Despite all the benefits PJSIP 
brings - and it is a lot! - there are bound to be some downsides, and its 
integration with its DNS resolver could be considered one of them. I can see 
two possible approaches available for us to go make use of a core DNS resolver:

(1) Modify PJSIP to support a pluggable DNS resolver. The PJSIP endpoint does 
support having a DNS resolver be registered to it; unfortunately, today this is 
an opaque structure that the endpoint uses with pjlib-util/resolver.h. We would 
have to provide a shim between pj_dns_resolver and the core resolver, and then 
update PJSIP to directly use the shim – with callbacks between a PJSIP version 
of the shim and pj_dns_resolver used internally in that project by default. 
This is doable, but again, not a trivial amount of work. Since the changes 
would be substantial, we would be forking PJSIP (again) and pushing our changes 
up-stream (again). This is not something we should shy away from when 
necessary, but it is something we should keep in mind when making substantial 
changes in that project.
(2) We could eschew any usage of PJSIP's DNS resolution by simply performing 
all of the resolution ourselves and pre-populating the PJSIP message structs in 
the res_pjsip* modules with the data. We _think_ that this would work: PJSIP 
should detect that it already has an address and not bother with any resolution 
if we did that. This has several obvious drawbacks however: (a) we take the 
burden of when and how to do resolution out of the library, which often has 
better knowledge of when to do this action than the res_pjsip* modules

Re: [asterisk-dev] [Code Review] 3331: Allows app_chanspy to whisper to a spyee's bridged peer (barge) even if the bridged party answers after initial spy invocation.

2014-03-13 Thread Robert Moss


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3331/
---

(Updated March 13, 2014, 7:24 p.m.)


Review request for Asterisk Developers.


Changes
---

Put duplicate code in a function.
Changed to negation test, removing 'magic number' 0
Removed unneeded assignment.


Bugs: ASTERISK-23381
https://issues.asterisk.org/jira/browse/ASTERISK-23381


Repository: Asterisk


Description
---

Chanspy now can whisper to the spyee's bridged party (callee) (aka barging) 
even if the Spy started before the bridged party answered.


Diffs (updated)
-

  /trunk/apps/app_chanspy.c 410469 

Diff: https://reviewboard.asterisk.org/r/3331/diff/


Testing
---

Chanspy on an extension while the spyee's call is still ringing, after the 
bridged party answers, both spied on parties can hear the monitor.


Thanks,

Robert Moss

-- 
_
-- 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] 3338: ARI: Prevent corner cases where channel in bridge can wait forever for playback to finish



 On March 13, 2014, 12:43 p.m., rmudgett wrote:
  You need to add cases to the switches that handle AST_FRAME_BRIDGE_ACTION 
  in frame.c and bridge_softmix.c.  It is probably best if the 
  AST_FRAME_BRIDGE_ACTION_SYNC case is next to the AST_BRIDGE_ACTION case.
  
  The main difficulty with this patch is that you are adding an allocated 
  resource to ast_frame's which inherently doesn't support this.
 
 Mark Michelson wrote:
 I don't understand the comment about adding an allocated resource to 
 ast_frame. The frame payload is allocated on the stack, but that's the same 
 strategy used for all bridge_playfile payloads right now. The bridge_playfile 
 is memcpy'd into the sync_payload, and the sync_payload's data length is 
 adjusted to account for this. When ast_frdup() copies the frame data to 
 another frame, the entire sync_payload and encased bridge_playfile is copied 
 to the other frame, so I'm not sure what the problem here is.

The allocated resource is referring to the fact that you have to explicitly 
keep track of the frame to manage the semaphore.  If that frame is simply 
destroyed, you will wait forever; which is a resource leak.


- rmudgett


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3338/#review11177
---


On March 13, 2014, 2 p.m., Mark Michelson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3338/
 ---
 
 (Updated March 13, 2014, 2 p.m.)
 
 
 Review request for Asterisk Developers and rmudgett.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 ARI tests were occasionally showing a channel being stuck forever in Stasis. 
 After looking at a live system where the problem was occurring, it became 
 clear that the channel was stuck trying to wait for a playback to finish. 
 However, it was also clear that the playback had been abandoned long ago.
 
 When playing back a file to a channel that is in a bridge, the idea was to 
 queue the playfile action onto the corresponding bridge channel, and then 
 wait until the ARI custom playback function completed to signal that the 
 playback had finished. There were several ways that this could fail:
 * If the bridge channel were not found in the bridge, then we would never 
 attempt to queue the playfile action, but we would still try to wait for it 
 to finish happening.
 * If queuing the playfile action did not succeed, then we would still attempt 
 to wait for the action to occur.
 * If the playback action was successfully queued, but the bridge channel were 
 removed from the bridge before the playfile action could start, then the 
 playback would never happen, and we'd wait forever.
 
 The responsibility of knowing whether a playfile action has conpleted or ever 
 will occur is the bridge's, since ARI can never fully know. With this in 
 mind, I have created a new bridge channel API call to queue a playfile action 
 synchronously. The way it is done, synchronous queuing operations could be 
 created for other bridge actions quite easily. A new frametype, 
 AST_FRAME_BRIDGE_ACTION_SYNC, contains a payload consisting of a 
 synchronization ID and the actual payload of the bridge action that is to be 
 queued. When the frame is queued, a synchronization structure is created and 
 added to a linked list. The procedure then blocks until it is told that the 
 frame has been disposed of.
 
 When the frame gets freed (which will occur whether the playfile action 
 succeeds or does not happen), the freer of the frame signals the waiting 
 procedure that the playfile action has terminated, and control returns to 
 whoever queued the playfile action in the first place.
 
 From ARI's perspective, this greatly simplifies its code. Most of the 
 res_stasis_playback changes are code removal. The bridge_channel changes, 
 though, are pretty large, which is why I've included Richard on this issue.
 
 
 Diffs
 -
 
   /branches/12/res/res_stasis_playback.c 410467 
   /branches/12/main/frame.c 410467 
   /branches/12/main/channel.c 410467 
   /branches/12/main/bridge_channel.c 410467 
   /branches/12/include/asterisk/frame.h 410467 
   /branches/12/include/asterisk/bridge_channel.h 410467 
   /branches/12/funcs/func_frame_trace.c 410467 
   /branches/12/bridges/bridge_softmix.c 410467 
 
 Diff: https://reviewboard.asterisk.org/r/3338/diff/
 
 
 Testing
 ---
 
 Initially, I did some manual playbacks using Swagger-UI to a channel in a 
 bridge to ensure that the specified file was actually being played as 
 promised. I also queued up several files in quick succession to ensure that 
 they played in series and not on top of each other.
 
 In addition, I have created an automated test that is up for review

Re: [asterisk-dev] [Code Review] 3336: ARI: Ensure ChannelEnteredBridge messages get to the managing application

2014-03-13 Thread opticron


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3336/
---

(Updated March 13, 2014, 2:30 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 410527


Bugs: ASTERISK-23295
https://issues.asterisk.org/jira/browse/ASTERISK-23295


Repository: Asterisk


Description
---

This fixes an issue where a Stasis application running over ARI and subscribed 
to ari/events could miss the ChannelEnteredBridge event because it did not 
subscribe to the new bridge fast enough.

To accomplish this, it subscribes the application controlling the channel to 
the new bridge before adding it to that bridge which required the 
stasis_app_control structure to maintain a reference to the stasis_app.


Diffs
-

  branches/12/res/stasis/control.c 410448 
  branches/12/res/stasis/control.h 410448 
  branches/12/res/res_stasis.c 410448 

Diff: https://reviewboard.asterisk.org/r/3336/diff/


Testing
---

Ensured that the ARI tests still functioned since I was unable to reproduce the 
bug this fixes.


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] 3338: ARI: Prevent corner cases where channel in bridge can wait forever for playback to finish


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3338/#review11187
---



/branches/12/main/bridge_channel.c
https://reviewboard.asterisk.org/r/3338/#comment20826

You added this field but it is not used anywhere.




/branches/12/bridges/bridge_softmix.c
https://reviewboard.asterisk.org/r/3338/#comment20825

Doing this will require you to deal with the action running on all other 
channels in the bridge.  You are currently setup to handle only one channel.


- rmudgett


On March 13, 2014, 2 p.m., Mark Michelson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3338/
 ---
 
 (Updated March 13, 2014, 2 p.m.)
 
 
 Review request for Asterisk Developers and rmudgett.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 ARI tests were occasionally showing a channel being stuck forever in Stasis. 
 After looking at a live system where the problem was occurring, it became 
 clear that the channel was stuck trying to wait for a playback to finish. 
 However, it was also clear that the playback had been abandoned long ago.
 
 When playing back a file to a channel that is in a bridge, the idea was to 
 queue the playfile action onto the corresponding bridge channel, and then 
 wait until the ARI custom playback function completed to signal that the 
 playback had finished. There were several ways that this could fail:
 * If the bridge channel were not found in the bridge, then we would never 
 attempt to queue the playfile action, but we would still try to wait for it 
 to finish happening.
 * If queuing the playfile action did not succeed, then we would still attempt 
 to wait for the action to occur.
 * If the playback action was successfully queued, but the bridge channel were 
 removed from the bridge before the playfile action could start, then the 
 playback would never happen, and we'd wait forever.
 
 The responsibility of knowing whether a playfile action has conpleted or ever 
 will occur is the bridge's, since ARI can never fully know. With this in 
 mind, I have created a new bridge channel API call to queue a playfile action 
 synchronously. The way it is done, synchronous queuing operations could be 
 created for other bridge actions quite easily. A new frametype, 
 AST_FRAME_BRIDGE_ACTION_SYNC, contains a payload consisting of a 
 synchronization ID and the actual payload of the bridge action that is to be 
 queued. When the frame is queued, a synchronization structure is created and 
 added to a linked list. The procedure then blocks until it is told that the 
 frame has been disposed of.
 
 When the frame gets freed (which will occur whether the playfile action 
 succeeds or does not happen), the freer of the frame signals the waiting 
 procedure that the playfile action has terminated, and control returns to 
 whoever queued the playfile action in the first place.
 
 From ARI's perspective, this greatly simplifies its code. Most of the 
 res_stasis_playback changes are code removal. The bridge_channel changes, 
 though, are pretty large, which is why I've included Richard on this issue.
 
 
 Diffs
 -
 
   /branches/12/res/res_stasis_playback.c 410467 
   /branches/12/main/frame.c 410467 
   /branches/12/main/channel.c 410467 
   /branches/12/main/bridge_channel.c 410467 
   /branches/12/include/asterisk/frame.h 410467 
   /branches/12/include/asterisk/bridge_channel.h 410467 
   /branches/12/funcs/func_frame_trace.c 410467 
   /branches/12/bridges/bridge_softmix.c 410467 
 
 Diff: https://reviewboard.asterisk.org/r/3338/diff/
 
 
 Testing
 ---
 
 Initially, I did some manual playbacks using Swagger-UI to a channel in a 
 bridge to ensure that the specified file was actually being played as 
 promised. I also queued up several files in quick succession to ensure that 
 they played in series and not on top of each other.
 
 In addition, I have created an automated test that is up for review at /r/3339
 
 
 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] 3343: res_pjsip: Enable DNS support.

2014-03-13 Thread Olle E Johansson



 On March 13, 2014, 4:31 p.m., Olle E Johansson wrote:
  We really should NOT add this code. Instead, we should clean up the code so 
  all of Asterisk use the same resolver and relies on the system 
  configuration. Having a separate DNS resolver, maybe even using a separate 
  DNS server in the PJSIP channel is not a good solution and should be 
  avoided at all cost. 
  
  The good thing with this review is that it opened my eyes of some stuff in 
  PJSIP that should not be there in the way we use it.
 
 Matt Jordan wrote:
 DNS support in Asterisk has had problems for a long time and is something 
 that I think we would all like to see improved upon. The state of such 
 support as it exists in the core today is limited: modules and the core can 
 either use dnsmgr – which only gives you a single IP address result but does 
 make an attempt at querying and managing the DNS records – or they can use 
 ast_gethostbyname, which is a wrapper around a reentrant version of 
 gethostbyname. In the case of the former, users get a small (albeit limited) 
 benefit of having the records be managed internally; in the case of the 
 latter users only get the standard gethostbyname behavior. Neither option 
 provides asynchronous DNS querying, nor does either option providethe ability 
 to handle multiple SRV records.
 
 The situation is the way it is today not because parsing and returning 
 multiple DNS records is particularly hard, or because properly weighting the 
 results is extremely difficult. Even making the querying of records 
 asynchronous is doable. While not trivial changes, they are well understood 
 problems and the API in dns.h/srv.h/dnsmgr.h could be modified accordingly. 
 The hard part is making use of the DNS information.
 
 The legacy IP based channel drivers in Asterisk are not structured in 
 such a fashion that the location of the thing that Asterisk communicates with 
 is separate from the IP address. This was an unfortunate design decision; 
 separating out the concept of location from the IP address is now extremely 
 non-trivial. I can't stress this enough: it is an incredibly large task to go 
 do this work – particularly when we consider the size and complexity of those 
 channel drivers. Unfortunately, chan_sip and chan_iax2 are both places where 
 this would be beneficial, but also cases where making use of multiple SRV 
 records or asynchronous DNS would be extremely challenging.
 
 That gets to the crux of the problem: even if we had the world's best DNS 
 resolver in the Asterisk core, the vast majority of places in Asterisk that 
 you would like to make use of it would not easily be able to do so.
 
 There is, of course, benefit to having better DNS support in the core of 
 Asterisk outside of those channel drivers. New things would be able to make 
 use of it easier, and improvements in the DNS support would be shared by all 
 things that use it. I think it would be great to see patches that improve the 
 DNS support in the core. This should include asynchronous querying, multiple 
 SRV records, as well as other cool kinds of things such as DANE. At the same 
 time, since the vast majority of existing channel drivers can't make use of 
 that information, it is of admittedly limited benefit unless other changes 
 take place.
 
 This is, unfortunately, one of those places where making use of a third 
 party stack does make things more difficult for us. Despite all the benefits 
 PJSIP brings - and it is a lot! - there are bound to be some downsides, and 
 its integration with its DNS resolver could be considered one of them. I can 
 see two possible approaches available for us to go make use of a core DNS 
 resolver:
 
 (1) Modify PJSIP to support a pluggable DNS resolver. The PJSIP endpoint 
 does support having a DNS resolver be registered to it; unfortunately, today 
 this is an opaque structure that the endpoint uses with 
 pjlib-util/resolver.h. We would have to provide a shim between 
 pj_dns_resolver and the core resolver, and then update PJSIP to directly use 
 the shim – with callbacks between a PJSIP version of the shim and 
 pj_dns_resolver used internally in that project by default. This is doable, 
 but again, not a trivial amount of work. Since the changes would be 
 substantial, we would be forking PJSIP (again) and pushing our changes 
 up-stream (again). This is not something we should shy away from when 
 necessary, but it is something we should keep in mind when making substantial 
 changes in that project.
 (2) We could eschew any usage of PJSIP's DNS resolution by simply 
 performing all of the resolution ourselves and pre-populating the PJSIP 
 message structs in the res_pjsip* modules with the data. We _think_ that this 
 would work: PJSIP should detect that it already has an address and not bother 
 with any resolution if we did that. This has several obvious drawbacks 
 however: (a) we take the burden

Re: [asterisk-dev] [Code Review] 3343: res_pjsip: Enable DNS support.

2014-03-13 Thread Dan Austin

Matt wrote:

Not including this change does not seem to buy us anything, save for some 
semblance of architectural purity. While I would love for there to be only one 
way to perform DNS resolution, that feels like a long term goal – and 
sacrificing the practicality of delivering a feature that a large number of 
Asterisk users have wanted for an extremely long time doesn't feel worth it to 
me.

I am indifferent to PJSIP and have no intent to use it any time soon, so my 
critic is not of
that channel driver.  In times not so long past if a developer offered a new 
feature for one
of the second-class channels or apps they stood a good chance being told to 
rewrite it to
be channel agnostic,  that it should (had to) be coded in such a way that all 
channels would
benefit.  It is kind of amusing to see that turned around to not apply to the 
new kid on the block.

Dan
-- 
_
-- 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] 3322: media_formats: Move resource modules over.

2014-03-13 Thread Kevin Harwell


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3322/#review11189
---



/team/group/media_formats-reviewed/res/res_musiconhold.c
https://reviewboard.asterisk.org/r/3322/#comment20827

I think there is a chance of a leak here.  origwfmt is not NULL and if 
mohwfmt is NULL then origwfmt doesn't get its ref count decremented before 
being overwritten.


- Kevin Harwell


On March 8, 2014, 11:36 a.m., Joshua Colp wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3322/
 ---
 
 (Updated March 8, 2014, 11:36 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This moves the resource modules over to the new media formats API.
 
 
 Diffs
 -
 
   /team/group/media_formats-reviewed/res/res_stasis_snoop.c 409286 
   /team/group/media_formats-reviewed/res/res_stasis.c 409286 
   /team/group/media_formats-reviewed/res/res_speech.c 409286 
   /team/group/media_formats-reviewed/res/res_musiconhold.c 409286 
   /team/group/media_formats-reviewed/res/res_fax_spandsp.c 409286 
   /team/group/media_formats-reviewed/res/res_fax.c 410186 
   /team/group/media_formats-reviewed/res/res_clioriginate.c 409286 
   /team/group/media_formats-reviewed/res/res_calendar.c 409286 
   /team/group/media_formats-reviewed/res/res_agi.c 409286 
   /team/group/media_formats-reviewed/res/res_adsi.c 409286 
   /team/group/media_formats-reviewed/res/parking/parking_applications.c 
 409286 
   /team/group/media_formats-reviewed/res/ari/resource_sounds.c 409286 
   /team/group/media_formats-reviewed/res/ari/resource_channels.c 409286 
   /team/group/media_formats-reviewed/res/ari/resource_bridges.c 409286 
 
 Diff: https://reviewboard.asterisk.org/r/3322/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] 3321: media_formats: Move dialplan functions over.

2014-03-13 Thread Kevin Harwell


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3321/#review11190
---

Ship it!


Ship It!

- Kevin Harwell


On March 8, 2014, 11:36 a.m., Joshua Colp wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3321/
 ---
 
 (Updated March 8, 2014, 11:36 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This moves dialplan functions over to the new media format API.
 
 
 Diffs
 -
 
   /team/group/media_formats-reviewed/funcs/func_speex.c 409286 
   /team/group/media_formats-reviewed/funcs/func_pitchshift.c 409286 
   /team/group/media_formats-reviewed/funcs/func_frame_trace.c 409286 
   /team/group/media_formats-reviewed/funcs/func_channel.c 409286 
 
 Diff: https://reviewboard.asterisk.org/r/3321/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] 3209: Crash in ast_format_cmp on shutdown

2014-03-13 Thread Corey Farrell


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3209/#review11181
---



/branches/11/main/format.c
https://reviewboard.asterisk.org/r/3209/#comment20824

I think this condition still needs !ast_undestroyed_channels().  If we have 
undestroyed channels then find_interface will be called many times per second.  
format_attr_shutdown could free interfaces after find_interface has already 
checked it, allowing ao2_find on either an already freed or NULL pointer.


- Corey Farrell


On March 13, 2014, 1:37 p.m., Scott Griepentrog wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3209/
 ---
 
 (Updated March 13, 2014, 1:37 p.m.)
 
 
 Review request for Asterisk Developers, Corey Farrell and n8ideas.
 
 
 Bugs: ASTERISK-23103
 https://issues.asterisk.org/jira/browse/ASTERISK-23103
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Alternate method to more safely shutdown interfaces container.  Set interface 
 global to null first to avoid possible race condition, and also double check 
 interfaces prior to all uses.
 
 
 Diffs
 -
 
   /branches/11/main/format.c 410525 
 
 Diff: https://reviewboard.asterisk.org/r/3209/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Scott Griepentrog
 


-- 
_
-- 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] 3343: res_pjsip: Enable DNS support.

2014-03-13 Thread Paul Belanger

On Thu, Mar 13, 2014 at 4:15 PM, Dan Austin dan_aus...@phoenix.com wrote:
 Matt wrote:

 Not including this change does not seem to buy us anything, save for some
 semblance of architectural purity. While I would love for there to be only
 one way to perform DNS resolution, that feels like a long term goal - and
 sacrificing the practicality of delivering a feature that a large number of
 Asterisk users have wanted for an extremely long time doesn't feel worth it
 to me.



 I am indifferent to PJSIP and have no intent to use it any time soon, so my
 critic is not of

 that channel driver.  In times not so long past if a developer offered a new
 feature for one

 of the second-class channels or apps they stood a good chance being told to
 rewrite it to

 be channel agnostic,  that it should (had to) be coded in such a way that
 all channels would

 benefit.  It is kind of amusing to see that turned around to not apply to
 the new kid on the block.

+1 with Dan.  Comments aside on DNS functionality (I have opinions but
sitting this one out). Any functionality should be channel agnostic.
I too am a little concern'd that statement seems to have changed.

-- 
Paul Belanger | PolyBeacon, Inc.
Jabber: paul.belan...@polybeacon.com | IRC: pabelanger (Freenode)
Github: https://github.com/pabelanger | Twitter: https://twitter.com/pabelanger

-- 
_
-- 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] 3326: Sorcery: Do not apply the same wizard to an object type twice; Automatically apply sorcery configuration when sorcery is opened.


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3326/#review11191
---


test_sorcery.c also calls ast_sorcery_apply_config() and one of them is the 
module name.

- rmudgett


On March 12, 2014, 12:54 p.m., Mark Michelson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3326/
 ---
 
 (Updated March 12, 2014, 12:54 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 When performing some realtime tests, I noticed that the AMI command 
 PJSIPShowEndpoints was listing all of my endpoints twice. This is because 
 ast_sorcery_apply_config() was being called twice from res_pjsip code, once 
 when initializing system configuration, and once again when initializing the 
 rest of the configuration data. This patch aims to fix the problem on two 
 fronts:
 
 1) Remove the ast_sorcery_apply_config() calls from the PJSIP code entirely 
 in favor of having sorcery automatically apply configuration for the module 
 when sorcery is opened. This reduces the chance of accidentally attempting to 
 apply the same configuration twice. I also removed the call to 
 ast_sorcery_apply_config from res_mwi_external since it is no longer 
 necessary either.
 
 2) Adjust sorcery_apply_wizard_mapping() not to apply the same wizard to an 
 object type more than once, just in case someone does make the error of 
 calling ast_sorcery_apply_config() multiple times for the same object type.
 
 
 Diffs
 -
 
   /branches/12/res/res_pjsip/pjsip_configuration.c 410467 
   /branches/12/res/res_pjsip/config_system.c 410467 
   /branches/12/res/res_mwi_external.c 410467 
   /branches/12/main/sorcery.c 410467 
   /branches/12/include/asterisk/sorcery.h 410467 
 
 Diff: https://reviewboard.asterisk.org/r/3326/diff/
 
 
 Testing
 ---
 
 My tests of retrieving data from realtime now get the expected objects. I 
 don't have any automated tests to post yet because the realtime testsuite is 
 a work in progress.
 
 
 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

[asterisk-dev] version.c generation

2014-03-13 Thread Michal Rybárik


Hello,

how and when is main/version.c generated ? I checked out branch-11 svn, 
compiled, packaged for debian, and now I realized that core show 
version is diffent than version that I got from SVN.


main/version.c
static const char asterisk_version[] = SVN-branch-11-r410490M;

ipfon-test*CLI core show version
Asterisk SVN-branch-11-r410490M built by root @ fero on a i686 running 
Linux on 2014-03-13 20:44:31 UTC


SVN checkout:
Checked out revision 410540.

How this works?

Thanks,
Michal Rybarik

--
_
-- 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] 3343: res_pjsip: Enable DNS support.

2014-03-13 Thread Sean Bright


On 3/13/2014 4:42 PM, Paul Belanger wrote:

+1 with Dan.  Comments aside on DNS functionality (I have opinions but
sitting this one out). Any functionality should be channel agnostic.
I too am a little concern'd that statement seems to have changed.


In order to make this channel agnostic you have three (equally bad) 
options:


1. Replace Asterisk's internal DNS facilities with PJLIB's, creating a
   mandatory dependency on PJSIP.
2. Roll a shiny new DNS API into Asterisk that supports all address
   types (multiple results, weighting, etc.).  Bear in mind that PJSIP
   would not use this new API at all, you would still need to create a
   PJLIB DNS resolver and feed it the nameservers to use.
3. Use PJLIB's DNS interface if it is available, otherwise fall back to
   Asterisk's current DNS interface.  This means that you are now
   maintaining two separate interfaces and have to throw a layer of
   abstraction in while you're at it.  In fact, by adding an
   abstraction layer you would force res_pjsip to then unwrap and then
   re-wrap the abstraction just to get at the necessary PJLIB data
   structures.

Frankly, I don't see what all the hubbub is about.  99.9% of users will 
never touch the nameservers configuration option and it will behave 
exactly as if the system resolver was being used.


-- 
_
-- 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] 3328: Agent Acknowledgement Nominal and Error Tests


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3328/#review11194
---

Ship it!


Ship It!

- Mark Michelson


On March 11, 2014, 7:48 p.m., Benjamin Keith Ford wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3328/
 ---
 
 (Updated March 11, 2014, 7:48 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-23263
 https://issues.asterisk.org/jira/browse/ASTERISK-23263
 
 
 Repository: testsuite
 
 
 Description
 ---
 
 This test covers both the nominal agent acknowledgement and the agent 
 acknowledgement errors that can occur.
 
 Nominal scenarios:
 1. Tests an agent that acknowledges the request with the '#' key.
 2. Tests an agent that acknowledges the request with the '*' key.
 3. The same as scenario 1, but instead of hearing a beep, a custom sound file 
 is played.
 
 Error scenarios:
 1. Tests an agent that enters an incorrect DTMF to acknowledge the request.
 2. Tests an agent that does nothing when a request is received.
 
 
 Diffs
 -
 
   ./asterisk/trunk/tests/apps/tests.yaml 4675 
   ./asterisk/trunk/tests/apps/agents/tests.yaml PRE-CREATION 
   ./asterisk/trunk/tests/apps/agents/agent_acknowledge/tests.yaml 
 PRE-CREATION 
   
 ./asterisk/trunk/tests/apps/agents/agent_acknowledge/nominal/test-config.yaml 
 PRE-CREATION 
   
 ./asterisk/trunk/tests/apps/agents/agent_acknowledge/nominal/configs/ast1/extensions.conf
  PRE-CREATION 
   
 ./asterisk/trunk/tests/apps/agents/agent_acknowledge/nominal/configs/ast1/agents.conf
  PRE-CREATION 
   
 ./asterisk/trunk/tests/apps/agents/agent_acknowledge/agent_acknowledge_error/test-config.yaml
  PRE-CREATION 
   
 ./asterisk/trunk/tests/apps/agents/agent_acknowledge/agent_acknowledge_error/configs/ast1/extensions.conf
  PRE-CREATION 
   
 ./asterisk/trunk/tests/apps/agents/agent_acknowledge/agent_acknowledge_error/configs/ast1/agents.conf
  PRE-CREATION 
   ./asterisk/trunk/sample-yaml/apptest-config.yaml.sample 4675 
   ./asterisk/trunk/lib/python/asterisk/apptest.py 4675 
 
 Diff: https://reviewboard.asterisk.org/r/3328/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Benjamin Keith Ford
 


-- 
_
-- 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] 3330: Testsuite: e( ) Options BridgeWait Application


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3330/#review11195
---

Ship it!


Ship It!

- Mark Michelson


On March 11, 2014, 11:10 p.m., Scott Emidy wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3330/
 ---
 
 (Updated March 11, 2014, 11:10 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-23355
 https://issues.asterisk.org/jira/browse/ASTERISK-23355
 
 
 Repository: testsuite
 
 
 Description
 ---
 
 This test verifies that the various ways to entertain a holding bridge 
 receive the correct events, and ultimately work properly. The tests I covered 
 involve:
 1) e(m) = This option just needed justification that the participant in the 
 holding bridge was receiving MoH events.
 2) m(class) = This scenario was to justify that not only the MoH events 
 showed up, but also that it was reading the class correctly.
 3) e(n) = This scenario calls for a fail-test action-type if either a Hold 
 event or MoH event were to show up.
 4) e(s) = This scenario was basically identical to e(n), only the participant 
 receives silent audio instead of no sound at all.
 5) e(r) = This scenario verifies that the channel state changed to ringing 
 through checking a Newstate event.
 6) e(h) = This scenario puts the participant on hold, and was justified 
 through MoH events that were received.
 
 
 Diffs
 -
 
   ./asterisk/trunk/tests/apps/bridge/tests.yaml 4791 
   ./asterisk/trunk/tests/apps/bridge/bridge_wait/tests.yaml PRE-CREATION 
   
 ./asterisk/trunk/tests/apps/bridge/bridge_wait/bridge_wait_e_options/test-config.yaml
  PRE-CREATION 
   
 ./asterisk/trunk/tests/apps/bridge/bridge_wait/bridge_wait_e_options/configs/ast1/musiconhold.conf
  PRE-CREATION 
   
 ./asterisk/trunk/tests/apps/bridge/bridge_wait/bridge_wait_e_options/configs/ast1/extensions.conf
  PRE-CREATION 
 
 Diff: https://reviewboard.asterisk.org/r/3330/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Scott Emidy
 


-- 
_
-- 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] 3338: ARI: Prevent corner cases where channel in bridge can wait forever for playback to finish


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3338/#review11193
---



/branches/12/include/asterisk/frame.h
https://reviewboard.asterisk.org/r/3338/#comment20829

This block belongs with the other enum value. :)



/branches/12/main/bridge_channel.c
https://reviewboard.asterisk.org/r/3338/#comment20830

You should do this after you remove the references to it in the list.



/branches/12/main/bridge_channel.c
https://reviewboard.asterisk.org/r/3338/#comment20831

Doxegen the units and purpose.

You should be in the habbit of adding isolating parentheses around macro 
values that have more than one token.



/branches/12/main/bridge_channel.c
https://reviewboard.asterisk.org/r/3338/#comment20832

This must be done before you unlock because sync is only valid while the 
list is locked.
It being a stack variable on another thread would make the crashes truly 
baffling.


- rmudgett


On March 13, 2014, 4:34 p.m., Mark Michelson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3338/
 ---
 
 (Updated March 13, 2014, 4:34 p.m.)
 
 
 Review request for Asterisk Developers and rmudgett.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 ARI tests were occasionally showing a channel being stuck forever in Stasis. 
 After looking at a live system where the problem was occurring, it became 
 clear that the channel was stuck trying to wait for a playback to finish. 
 However, it was also clear that the playback had been abandoned long ago.
 
 When playing back a file to a channel that is in a bridge, the idea was to 
 queue the playfile action onto the corresponding bridge channel, and then 
 wait until the ARI custom playback function completed to signal that the 
 playback had finished. There were several ways that this could fail:
 * If the bridge channel were not found in the bridge, then we would never 
 attempt to queue the playfile action, but we would still try to wait for it 
 to finish happening.
 * If queuing the playfile action did not succeed, then we would still attempt 
 to wait for the action to occur.
 * If the playback action was successfully queued, but the bridge channel were 
 removed from the bridge before the playfile action could start, then the 
 playback would never happen, and we'd wait forever.
 
 The responsibility of knowing whether a playfile action has conpleted or ever 
 will occur is the bridge's, since ARI can never fully know. With this in 
 mind, I have created a new bridge channel API call to queue a playfile action 
 synchronously. The way it is done, synchronous queuing operations could be 
 created for other bridge actions quite easily. A new frametype, 
 AST_FRAME_BRIDGE_ACTION_SYNC, contains a payload consisting of a 
 synchronization ID and the actual payload of the bridge action that is to be 
 queued. When the frame is queued, a synchronization structure is created and 
 added to a linked list. The procedure then blocks until it is told that the 
 frame has been disposed of.
 
 When the frame gets freed (which will occur whether the playfile action 
 succeeds or does not happen), the freer of the frame signals the waiting 
 procedure that the playfile action has terminated, and control returns to 
 whoever queued the playfile action in the first place.
 
 From ARI's perspective, this greatly simplifies its code. Most of the 
 res_stasis_playback changes are code removal. The bridge_channel changes, 
 though, are pretty large, which is why I've included Richard on this issue.
 
 
 Diffs
 -
 
   /branches/12/res/res_stasis_playback.c 410467 
   /branches/12/main/frame.c 410467 
   /branches/12/main/channel.c 410467 
   /branches/12/main/bridge_channel.c 410467 
   /branches/12/include/asterisk/frame.h 410467 
   /branches/12/include/asterisk/bridge_channel.h 410467 
   /branches/12/funcs/func_frame_trace.c 410467 
   /branches/12/bridges/bridge_softmix.c 410467 
 
 Diff: https://reviewboard.asterisk.org/r/3338/diff/
 
 
 Testing
 ---
 
 Initially, I did some manual playbacks using Swagger-UI to a channel in a 
 bridge to ensure that the specified file was actually being played as 
 promised. I also queued up several files in quick succession to ensure that 
 they played in series and not on top of each other.
 
 In addition, I have created an automated test that is up for review at /r/3339
 
 
 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

[asterisk-dev] magic number 128- for concurrent meetme monitoring calls.

2014-03-13 Thread beiyan jin

In my load test calls,
1. each call has two parties connected by meetme conference.
2. Each call is recorded by monitor.


For every load test, before the number of concurrent calls reach 128,
everything is fine. But after 128, newly started calls get dropped.
Both CPU and memory are ok in the linux box hosting asterisk.

The behavior is like asterisk is configured to only allow 128 concurrent 

calls or 256 concurrent channels.

If this is configured like this by default, where can I change the 
configuration?
If it is not configured, then why it only allows 128 concurrent calls?

Thanks in advance for your help!

Beiyan-- 
_
-- 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] 3338: ARI: Prevent corner cases where channel in bridge can wait forever for playback to finish


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3338/
---

(Updated March 13, 2014, 9:34 p.m.)


Review request for Asterisk Developers and rmudgett.


Changes
---

Addressed all feedback again.


Repository: Asterisk


Description
---

ARI tests were occasionally showing a channel being stuck forever in Stasis. 
After looking at a live system where the problem was occurring, it became clear 
that the channel was stuck trying to wait for a playback to finish. However, it 
was also clear that the playback had been abandoned long ago.

When playing back a file to a channel that is in a bridge, the idea was to 
queue the playfile action onto the corresponding bridge channel, and then wait 
until the ARI custom playback function completed to signal that the playback 
had finished. There were several ways that this could fail:
* If the bridge channel were not found in the bridge, then we would never 
attempt to queue the playfile action, but we would still try to wait for it to 
finish happening.
* If queuing the playfile action did not succeed, then we would still attempt 
to wait for the action to occur.
* If the playback action was successfully queued, but the bridge channel were 
removed from the bridge before the playfile action could start, then the 
playback would never happen, and we'd wait forever.

The responsibility of knowing whether a playfile action has conpleted or ever 
will occur is the bridge's, since ARI can never fully know. With this in mind, 
I have created a new bridge channel API call to queue a playfile action 
synchronously. The way it is done, synchronous queuing operations could be 
created for other bridge actions quite easily. A new frametype, 
AST_FRAME_BRIDGE_ACTION_SYNC, contains a payload consisting of a 
synchronization ID and the actual payload of the bridge action that is to be 
queued. When the frame is queued, a synchronization structure is created and 
added to a linked list. The procedure then blocks until it is told that the 
frame has been disposed of.

When the frame gets freed (which will occur whether the playfile action 
succeeds or does not happen), the freer of the frame signals the waiting 
procedure that the playfile action has terminated, and control returns to 
whoever queued the playfile action in the first place.

From ARI's perspective, this greatly simplifies its code. Most of the 
res_stasis_playback changes are code removal. The bridge_channel changes, 
though, are pretty large, which is why I've included Richard on this issue.


Diffs (updated)
-

  /branches/12/res/res_stasis_playback.c 410467 
  /branches/12/main/frame.c 410467 
  /branches/12/main/channel.c 410467 
  /branches/12/main/bridge_channel.c 410467 
  /branches/12/include/asterisk/frame.h 410467 
  /branches/12/include/asterisk/bridge_channel.h 410467 
  /branches/12/funcs/func_frame_trace.c 410467 
  /branches/12/bridges/bridge_softmix.c 410467 

Diff: https://reviewboard.asterisk.org/r/3338/diff/


Testing
---

Initially, I did some manual playbacks using Swagger-UI to a channel in a 
bridge to ensure that the specified file was actually being played as promised. 
I also queued up several files in quick succession to ensure that they played 
in series and not on top of each other.

In addition, I have created an automated test that is up for review at /r/3339


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] version.c generation



   	   
   	Michal Rybrik  
  Thursday, March 
13, 2014 4:03 PM
  Hello,

how and when is main/version.c generated ? I checked out branch-11 
svn, 
compiled, packaged for debian, and now I realized that "core show 
version" is diffent than version that I got from SVN.

main/version.c
static const char asterisk_version[] = "SVN-branch-11-r410490M";

ipfon-test*CLI core show version
Asterisk SVN-branch-11-r410490M built by root @ fero on a i686 
running 
Linux on 2014-03-13 20:44:31 UTC

SVN checkout:
Checked out revision 410540.

How this works?

Thanks,
Michal Rybarik

 

The version number is determined by the build_tools/make_version script.
 It uses an svn property (by default svnmerge-integrated) in order to 
determine the version.

The reason this is different from what "svn info" shows is that the 
Asterisk repository has commits being made to many different branches. 
It is possible (and likely) that the branch you are currently using was 
last updated by a commit made many revisions ago. The version you see 
from "core show version" in Asterisk should show you the latest revision
 made in the branch you currently are using. "svn info" just shows the 
latest revision committed to the Asterisk repository, no matter the 
branch you currently have checked out.

-- 
_
-- 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] 3338: ARI: Prevent corner cases where channel in bridge can wait forever for playback to finish



 On March 13, 2014, 7:32 p.m., rmudgett wrote:
  /branches/12/bridges/bridge_softmix.c, line 662
  https://reviewboard.asterisk.org/r/3338/diff/3/?file=55895#file55895line662
 
  Doing this will require you to deal with the action running on all 
  other channels in the bridge.  You are currently setup to handle only one 
  channel.

Richard and I discussed this in person.

Synchronous bridge actions may only be queued onto bridge channels. They may 
not be written into bridges. I have documented this in frame.h, and I have 
added an assertion into bridge_softmix if this type of frame somehow gets in 
there.


- Mark


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3338/#review11187
---


On March 13, 2014, 7 p.m., Mark Michelson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3338/
 ---
 
 (Updated March 13, 2014, 7 p.m.)
 
 
 Review request for Asterisk Developers and rmudgett.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 ARI tests were occasionally showing a channel being stuck forever in Stasis. 
 After looking at a live system where the problem was occurring, it became 
 clear that the channel was stuck trying to wait for a playback to finish. 
 However, it was also clear that the playback had been abandoned long ago.
 
 When playing back a file to a channel that is in a bridge, the idea was to 
 queue the playfile action onto the corresponding bridge channel, and then 
 wait until the ARI custom playback function completed to signal that the 
 playback had finished. There were several ways that this could fail:
 * If the bridge channel were not found in the bridge, then we would never 
 attempt to queue the playfile action, but we would still try to wait for it 
 to finish happening.
 * If queuing the playfile action did not succeed, then we would still attempt 
 to wait for the action to occur.
 * If the playback action was successfully queued, but the bridge channel were 
 removed from the bridge before the playfile action could start, then the 
 playback would never happen, and we'd wait forever.
 
 The responsibility of knowing whether a playfile action has conpleted or ever 
 will occur is the bridge's, since ARI can never fully know. With this in 
 mind, I have created a new bridge channel API call to queue a playfile action 
 synchronously. The way it is done, synchronous queuing operations could be 
 created for other bridge actions quite easily. A new frametype, 
 AST_FRAME_BRIDGE_ACTION_SYNC, contains a payload consisting of a 
 synchronization ID and the actual payload of the bridge action that is to be 
 queued. When the frame is queued, a synchronization structure is created and 
 added to a linked list. The procedure then blocks until it is told that the 
 frame has been disposed of.
 
 When the frame gets freed (which will occur whether the playfile action 
 succeeds or does not happen), the freer of the frame signals the waiting 
 procedure that the playfile action has terminated, and control returns to 
 whoever queued the playfile action in the first place.
 
 From ARI's perspective, this greatly simplifies its code. Most of the 
 res_stasis_playback changes are code removal. The bridge_channel changes, 
 though, are pretty large, which is why I've included Richard on this issue.
 
 
 Diffs
 -
 
   /branches/12/res/res_stasis_playback.c 410467 
   /branches/12/main/frame.c 410467 
   /branches/12/main/channel.c 410467 
   /branches/12/main/bridge_channel.c 410467 
   /branches/12/include/asterisk/frame.h 410467 
   /branches/12/include/asterisk/bridge_channel.h 410467 
   /branches/12/funcs/func_frame_trace.c 410467 
   /branches/12/bridges/bridge_softmix.c 410467 
 
 Diff: https://reviewboard.asterisk.org/r/3338/diff/
 
 
 Testing
 ---
 
 Initially, I did some manual playbacks using Swagger-UI to a channel in a 
 bridge to ensure that the specified file was actually being played as 
 promised. I also queued up several files in quick succession to ensure that 
 they played in series and not on top of each other.
 
 In addition, I have created an automated test that is up for review at /r/3339
 
 
 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] 3348: Test Suite: MWI subscription test for PJSIP

2014-03-13 Thread jbigelow


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3348/
---

(Updated March 13, 2014, 4:27 p.m.)


Review request for Asterisk Developers.


Changes
---

Updated to handle the scenario of the on_mwi_info() callback not be called.


Bugs: ASTERISK-23343
https://issues.asterisk.org/jira/browse/ASTERISK-23343


Repository: testsuite


Description
---

This test depends on some changes in the PJSIP python library pjsua.py. The 
plan is to submit this upstream. See in-line with this description below.

This basic nominal test ensures that a mailbox on an AOR for an enpoint can be 
subscribed to for MWI and that a NOTIFY is received. It creates endpoint 
alice with subscribing to MWI. Upon receiving a notify for message summary 
indicating no messages the test will be marked as passed otherwise it will be 
failed.

The other two tests on ASTERISK-23343 aren't currently possible with the PJSUA 
API v1.

Note: the following patch for pjsua.py adds modify_account() which isn't 
currently used by the testsuite but was added for possible future use.

Index: pjsip-apps/src/python/pjsua.py
===
--- pjsip-apps/src/python/pjsua.py  (revision 4791)
+++ pjsip-apps/src/python/pjsua.py  (working copy)
@@ -775,6 +775,7 @@
 use_srtp = 0
 srtp_secure_signaling = 1
 rtp_transport_cfg = None
+mwi_enabled = False
 
 def __init__(self, domain=, username=, password=, 
  display=, registrar=, proxy=):
@@ -865,6 +866,7 @@
 self.ka_data = cfg.ka_data
 self.use_srtp = cfg.use_srtp
 self.srtp_secure_signaling = cfg.srtp_secure_signaling
+self.mwi_enabled = cfg.mwi_enabled
 if (self.rtp_transport_cfg is not None):
 self.rtp_transport_cfg._cvt_from_pjsua(cfg.rtp_transport_cfg)
 
@@ -896,6 +898,7 @@
 cfg.ka_data = self.ka_data
 cfg.use_srtp = self.use_srtp
 cfg.srtp_secure_signaling = self.srtp_secure_signaling
+cfg.mwi_enabled = self.mwi_enabled
 
 if (self.rtp_transport_cfg is not None):
 cfg.rtp_transport_cfg = self.rtp_transport_cfg._cvt_to_pjsua()
@@ -2337,6 +2340,18 @@
 self._err_check(create_account_for_transport(), self, err)
 return Account(self, acc_id, cb)
 
+def modify_account(self, acc_id, acc_config):
+Modify configuration of a pjsua account.
+
+Keyword arguments:
+acc_id  -- ID of the account to be modified.
+acc_config  -- New account configuration.
+
+
+lck = self.auto_lock()
+err = _pjsua.acc_modify(acc_id, acc_config._cvt_to_pjsua())
+self._err_check(modify_account(), self, err)
+
 def hangup_all(self):
 Hangup all calls.


Diffs (updated)
-

  /asterisk/trunk/tests/channels/pjsip/subscriptions/mwi/tests.yaml 4836 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/mwi/basic_mailbox_subscribe/test-config.yaml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/mwi/basic_mailbox_subscribe/subscribe.py
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/mwi/basic_mailbox_subscribe/configs/ast1/pjsip.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/mwi/basic_mailbox_subscribe/configs/ast1/modules.conf
 PRE-CREATION 
  /asterisk/trunk/lib/python/asterisk/pjsua_mod.py 4836 

Diff: https://reviewboard.asterisk.org/r/3348/diff/


Testing
---

* Ensured tests pass on multiple executions
* Ensured the testsuite  Asterisk logs looked good.


Thanks,

jbigelow

-- 
_
-- 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] 3343: res_pjsip: Enable DNS support.

2014-03-13 Thread Matthew Jordan

On Thu, Mar 13, 2014 at 4:13 PM, Sean Bright sean.bri...@gmail.com wrote:
 On 3/13/2014 4:42 PM, Paul Belanger wrote:

 +1 with Dan.  Comments aside on DNS functionality (I have opinions but
 sitting this one out). Any functionality should be channel agnostic.
 I too am a little concern'd that statement seems to have changed.


 In order to make this channel agnostic you have three (equally bad)
 options:

 Replace Asterisk's internal DNS facilities with PJLIB's, creating a
 mandatory dependency on PJSIP.
 Roll a shiny new DNS API into Asterisk that supports all address types
 (multiple results, weighting, etc.).  Bear in mind that PJSIP would not use
 this new API at all, you would still need to create a PJLIB DNS resolver and
 feed it the nameservers to use.
 Use PJLIB's DNS interface if it is available, otherwise fall back to
 Asterisk's current DNS interface.  This means that you are now maintaining
 two separate interfaces and have to throw a layer of abstraction in while
 you're at it.  In fact, by adding an abstraction layer you would force
 res_pjsip to then unwrap and then re-wrap the abstraction just to get at the
 necessary PJLIB data structures.

 Frankly, I don't see what all the hubbub is about.  99.9% of users will
 never touch the nameservers configuration option and it will behave exactly
 as if the system resolver was being used.


To build on Sean's sentiments:

(1) This is not new functionality - in the sense that we have not
written some amazing new functionality in either the core or a
res_pjsip* module and only want to use it one place. PJSIP itself
already provides DNS resolution. We just want to turn it on.

(2) Disregarding obtaining the nameservers, enabling the DNS
resolution as we want it to be used could be all of the following
code:

pj_dns_resolver *resolver;

if (!pjsip_endpt_get_resolver(ast_sip_get_pjsip_endpoint())) {
pjsip_endpt_create_resolver(ast_sip_get_pjsip_endpoint(), resolver);
pjsip_endpt_set_resolver(ast_sip_get_pjsip_endpoint(), resolver);
}

Josh did more of this because he's doing a Good Job. It's
certainly fine to look at how we get to those lines of code; maybe we
should not allow the user to specify the nameservers. At the end of
the day, however, those few lines of code enable asynchronous DNS,
multiple SRV records, and all the fanciness that comes with them.
Achieving the same functionality in any other channel driver is
thousands of lines of code. The scale of the problem is not the same
across the code base. Often, when people have proposed some generic
new functionality and only implemented it in a single channel driver,
we do push for its usage everywhere when the level of effort is the
same across all channel drivers: that isn't the case here.

(3) Even if this were a general purpose change - and as Sean pointed
out, it is not - many other general purpose frameworks have been
introduced in Asterisk and have not been globally applied to every
channel driver - the Configuration Framework, Sorcery, and others come
to mind. This is for good reason: the act of introducing them into an
existing channel driver would create more problems, i.e., regressions,
than they would solve. Going forward, we attempt to use these
frameworks where possible - but not at the sake of hurting existing
users of Asterisk. This is clearly one of those cases: as I pointed
out, implementing the kind of DNS resolution this provides (assuming
we had something that provided it) in chan_sip or chan_iax2 is a gut
and rewrite of those channel drivers. There is _no_ way such an
implementation wouldn't be a substantial risk to users of those
drivers.

(4) Applying a new feature everywhere is not a policy [1]. It *is* a
guideline (of a type which until recently, wasn't even written down
[2]). Policies are great; guidelines are great. They should be
followed - but not to the overall detriment of the project. Adhering
to the letter of the law while ignoring the spirit is not something
I'll ever advocate.

[1] https://wiki.asterisk.org/wiki/display/AST/Coding+Guidelines
[2] https://wiki.asterisk.org/wiki/display/AST/Code+Review+Checklist

-- 
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

[asterisk-dev] [Code Review] 3353: testsuite: Test for receiving Play/Record start and stop events on ARI bridge playback/recording.


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3353/
---

Review request for Asterisk Developers, Matt Jordan and Mark Michelson.


Bugs: ASTERISK-23444
https://issues.asterisk.org/jira/browse/ASTERISK-23444


Repository: testsuite


Description
---

As mmichelson suggested, here are a couple automated tests for replicating the 
tests I did by hand in https://reviewboard.asterisk.org/r/3340/

The mechanism is fairly simple...

Channel enters stasis and starts the whole process
Mixing bridge gets created via ARI
An application subscribes to bridge stasis messages for the newly created bridge
Channel gets pushed into the bridge via ARI
Bridge is recorded via ARI / Bridge playback with sound:tt-weasels is executed 
via ARI
The sound comes naturally to an end / the recording is stopped manually upon 
receiving its startup event
The test closes in the same way as a normal bridge subscription test


Diffs
-

  /asterisk/trunk/tests/rest_api/bridges/tests.yaml 4836 
  /asterisk/trunk/tests/rest_api/bridges/bridge_record/test-config.yaml 
PRE-CREATION 
  /asterisk/trunk/tests/rest_api/bridges/bridge_record/subscribe_bridge.py 
PRE-CREATION 
  
/asterisk/trunk/tests/rest_api/bridges/bridge_record/configs/ast1/extensions.conf
 PRE-CREATION 
  /asterisk/trunk/tests/rest_api/bridges/bridge_play/test-config.yaml 
PRE-CREATION 
  /asterisk/trunk/tests/rest_api/bridges/bridge_play/subscribe_bridge.py 
PRE-CREATION 
  
/asterisk/trunk/tests/rest_api/bridges/bridge_play/configs/ast1/extensions.conf 
PRE-CREATION 

Diff: https://reviewboard.asterisk.org/r/3353/diff/


Testing
---

ran both tests some number of times to ensure success was repeatable.
Varied some expectations to make sure everything I was looking for was actually 
being checked appropriately.


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] 3343: res_pjsip: Enable DNS support.



 On March 13, 2014, 10:31 a.m., Olle E Johansson wrote:
  We really should NOT add this code. Instead, we should clean up the code so 
  all of Asterisk use the same resolver and relies on the system 
  configuration. Having a separate DNS resolver, maybe even using a separate 
  DNS server in the PJSIP channel is not a good solution and should be 
  avoided at all cost. 
  
  The good thing with this review is that it opened my eyes of some stuff in 
  PJSIP that should not be there in the way we use it.
 
 Matt Jordan wrote:
 DNS support in Asterisk has had problems for a long time and is something 
 that I think we would all like to see improved upon. The state of such 
 support as it exists in the core today is limited: modules and the core can 
 either use dnsmgr – which only gives you a single IP address result but does 
 make an attempt at querying and managing the DNS records – or they can use 
 ast_gethostbyname, which is a wrapper around a reentrant version of 
 gethostbyname. In the case of the former, users get a small (albeit limited) 
 benefit of having the records be managed internally; in the case of the 
 latter users only get the standard gethostbyname behavior. Neither option 
 provides asynchronous DNS querying, nor does either option providethe ability 
 to handle multiple SRV records.
 
 The situation is the way it is today not because parsing and returning 
 multiple DNS records is particularly hard, or because properly weighting the 
 results is extremely difficult. Even making the querying of records 
 asynchronous is doable. While not trivial changes, they are well understood 
 problems and the API in dns.h/srv.h/dnsmgr.h could be modified accordingly. 
 The hard part is making use of the DNS information.
 
 The legacy IP based channel drivers in Asterisk are not structured in 
 such a fashion that the location of the thing that Asterisk communicates with 
 is separate from the IP address. This was an unfortunate design decision; 
 separating out the concept of location from the IP address is now extremely 
 non-trivial. I can't stress this enough: it is an incredibly large task to go 
 do this work – particularly when we consider the size and complexity of those 
 channel drivers. Unfortunately, chan_sip and chan_iax2 are both places where 
 this would be beneficial, but also cases where making use of multiple SRV 
 records or asynchronous DNS would be extremely challenging.
 
 That gets to the crux of the problem: even if we had the world's best DNS 
 resolver in the Asterisk core, the vast majority of places in Asterisk that 
 you would like to make use of it would not easily be able to do so.
 
 There is, of course, benefit to having better DNS support in the core of 
 Asterisk outside of those channel drivers. New things would be able to make 
 use of it easier, and improvements in the DNS support would be shared by all 
 things that use it. I think it would be great to see patches that improve the 
 DNS support in the core. This should include asynchronous querying, multiple 
 SRV records, as well as other cool kinds of things such as DANE. At the same 
 time, since the vast majority of existing channel drivers can't make use of 
 that information, it is of admittedly limited benefit unless other changes 
 take place.
 
 This is, unfortunately, one of those places where making use of a third 
 party stack does make things more difficult for us. Despite all the benefits 
 PJSIP brings - and it is a lot! - there are bound to be some downsides, and 
 its integration with its DNS resolver could be considered one of them. I can 
 see two possible approaches available for us to go make use of a core DNS 
 resolver:
 
 (1) Modify PJSIP to support a pluggable DNS resolver. The PJSIP endpoint 
 does support having a DNS resolver be registered to it; unfortunately, today 
 this is an opaque structure that the endpoint uses with 
 pjlib-util/resolver.h. We would have to provide a shim between 
 pj_dns_resolver and the core resolver, and then update PJSIP to directly use 
 the shim – with callbacks between a PJSIP version of the shim and 
 pj_dns_resolver used internally in that project by default. This is doable, 
 but again, not a trivial amount of work. Since the changes would be 
 substantial, we would be forking PJSIP (again) and pushing our changes 
 up-stream (again). This is not something we should shy away from when 
 necessary, but it is something we should keep in mind when making substantial 
 changes in that project.
 (2) We could eschew any usage of PJSIP's DNS resolution by simply 
 performing all of the resolution ourselves and pre-populating the PJSIP 
 message structs in the res_pjsip* modules with the data. We _think_ that this 
 would work: PJSIP should detect that it already has an address and not bother 
 with any resolution if we did that. This has several obvious drawbacks 
 however: (a) we take the

Re: [asterisk-dev] magic number 128- for concurrent meetme monitoring calls.

2014-03-13 Thread Richard Mudgett

On Thu, Mar 13, 2014 at 5:07 PM, beiyan jin jinbeiyan2...@yahoo.com wrote:

 In my load test calls,
 1. each call has two parties connected by meetme conference.
 2. Each call is recorded by monitor.

 For every load test, before the number of concurrent calls reach 128,
 everything is fine. But after 128, newly started calls get dropped.
 Both CPU and memory are ok in the linux box hosting asterisk.

 The behavior is like asterisk is configured to only allow 128 concurrent
 calls or 256 concurrent channels.

 If this is configured like this by default, where can I change the
 configuration?
 If it is not configured, then why it only allows 128 concurrent calls?


Sounds like the ulimit is at the default 1024.  You need to increase it
because
Asterisk needs a lot of file descriptors.

This kind of question is better asked on the asterisk-users list.

Richard
-- 
_
-- 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] 3329: AGI Exit Status Test


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3329/#review11196
---


One thing I notice about this test is that it is structured to call the first 
extension, then when that finishes, call the second, then when that finishes, 
call the third, etc.

Since no call depends on any of the previous call results, I think this test 
could be changed to originate all the calls at the same time and evaluate the 
UserEvents and Hangups as they arrive. This would have a few benefits:

1) The test will execute more quickly
2) You can get rid of the can_call member of your test class.
3) Your AMI hangup handler will be simplified greatly since you won't need to 
have a big if-else ladder to figure out what to do next. In fact, you may be 
able to just get rid of it altogether.

Doing it this way kind of screws up your detected member, though. As a simple 
workaround, in your new_exten_event_handler, you can determine whether to send 
the AMI hangup by checking event['AppData'] for the name of the AGI script. If 
event['AppData'] is 'waiting.agi' or 'executing.agi' then you issue the AMI 
hangup.


./asterisk/trunk/tests/agi/exit_status/configs/ast1/extensions.conf
https://reviewboard.asterisk.org/r/3329/#comment20835

I recommend sending a UserEvent if the AGISTATUS is not what is expected. 
Something like the following:

same = 
n,UserEvent(TestResult,result:${IF($[${AGISTATUS}=NOTFOUND]?pass:fail)})

Since your test script prints a message when the result is not pass, 
making this alteration can help to more easily debug which test cases are 
misbehaving.



./asterisk/trunk/tests/agi/exit_status/run-test
https://reviewboard.asterisk.org/r/3329/#comment20833

This sentence is incomplete.



./asterisk/trunk/tests/agi/exit_status/run-test
https://reviewboard.asterisk.org/r/3329/#comment20839

Even though I'm advocating to just get rid of this member altogether, I 
think it's worth mentioning that the name could be improved. Since this is 
intended to indicated that the testsuite is supposed to interrupt the current 
AGI, something like interrupt_agi would probably be more clear.



./asterisk/trunk/tests/agi/exit_status/run-test
https://reviewboard.asterisk.org/r/3329/#comment20837

Change the else to explicitly check if event['context'] == 
'executing-extens'

Add a final else clause that logs an error if an unexpected extension ends 
up calling the userevent from the h extension. Structure this so that 
success_count does not get incremented if this final else case is executed.



./asterisk/trunk/tests/agi/exit_status/run-test
https://reviewboard.asterisk.org/r/3329/#comment20836

Use %d since self.success_count is an integer.



./asterisk/trunk/tests/agi/exit_status/run-test
https://reviewboard.asterisk.org/r/3329/#comment20838

This use of 7 here is what is sometimes referred to as a magic number in 
programming, and it's not really a good thing. If this test were to be altered 
later to test some more situations or to remove some test cases, then whoever 
made the modifications would have to go through the source and change all of 
the 7's to 8's or 6's or whatever.

Instead, at the top of the file, declare a global variable, something like:

EXPECTED_SUCCESSES = 7

Then, everywhere where you are currently comparing to 7, just compare to 
EXPECTED_SUCCESSES instead.

In addition to the code maintainability I mentioned earlier, it also is 
more expressive; readers of the code will easily understand that you are 
comparing to the number of expected successes rather than a number they have to 
figure out the significance of.

In general, prefer symbolic names for things over bare numerals. Obviously, 
as with every rule, there are exceptions to this.



./asterisk/trunk/tests/agi/exit_status/run-test
https://reviewboard.asterisk.org/r/3329/#comment20834

Change this to an error instead of info, and print the value of 
test.success_count.


- Mark Michelson


On March 11, 2014, 7:54 p.m., Benjamin Keith Ford wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3329/
 ---
 
 (Updated March 11, 2014, 7:54 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-19167
 https://issues.asterisk.org/jira/browse/ASTERISK-19167
 
 
 Repository: testsuite
 
 
 Description
 ---
 
 Notes:
 - Is a sub-task of ASTERISK-19167
 - Ignore userA directory; will be removed from repository
 
 This test runs through a few different AGI scripts to verify that AGISTATUS 
 returns the correct values:
 1. Attempts to run an AGI script that does not exist. AGISTATUS returns 
 NOTFOUND.
 2. Attempts

Re: [asterisk-dev] magic number 128- for concurrent meetme monitoring calls.

2014-03-13 Thread Paul Belanger

On Thu, Mar 13, 2014 at 6:54 PM, Richard Mudgett rmudg...@digium.com wrote:



 On Thu, Mar 13, 2014 at 5:07 PM, beiyan jin jinbeiyan2...@yahoo.com wrote:

 In my load test calls,
 1. each call has two parties connected by meetme conference.
 2. Each call is recorded by monitor.

 For every load test, before the number of concurrent calls reach 128,
 everything is fine. But after 128, newly started calls get dropped.
 Both CPU and memory are ok in the linux box hosting asterisk.

 The behavior is like asterisk is configured to only allow 128 concurrent
 calls or 256 concurrent channels.

 If this is configured like this by default, where can I change the
 configuration?
 If it is not configured, then why it only allows 128 concurrent calls?


 Sounds like the ulimit is at the default 1024.  You need to increase it
 because
 Asterisk needs a lot of file descriptors.

 This kind of question is better asked on the asterisk-users list.

Yup, next limit you'll hit is dahdi pseudo channels, which is 512.

-- 
Paul Belanger | PolyBeacon, Inc.
Jabber: paul.belan...@polybeacon.com | IRC: pabelanger (Freenode)
Github: https://github.com/pabelanger | Twitter: https://twitter.com/pabelanger

-- 
_
-- 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] 3329: AGI Exit Status Test



 On March 13, 2014, 6:13 p.m., Mark Michelson wrote:
  One thing I notice about this test is that it is structured to call the 
  first extension, then when that finishes, call the second, then when that 
  finishes, call the third, etc.
  
  Since no call depends on any of the previous call results, I think this 
  test could be changed to originate all the calls at the same time and 
  evaluate the UserEvents and Hangups as they arrive. This would have a few 
  benefits:
  
  1) The test will execute more quickly
  2) You can get rid of the can_call member of your test class.
  3) Your AMI hangup handler will be simplified greatly since you won't need 
  to have a big if-else ladder to figure out what to do next. In fact, you 
  may be able to just get rid of it altogether.
  
  Doing it this way kind of screws up your detected member, though. As a 
  simple workaround, in your new_exten_event_handler, you can determine 
  whether to send the AMI hangup by checking event['AppData'] for the name of 
  the AGI script. If event['AppData'] is 'waiting.agi' or 'executing.agi' 
  then you issue the AMI hangup.

Doing tests serially allows them to be debugged easier.  Having all the tests 
run simultaneously makes it difficult to figure out which event is a result of 
which test.  This is especially true if unexpected events happen that are a 
clue to what is going wrong.


- rmudgett


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3329/#review11196
---


On March 11, 2014, 2:54 p.m., Benjamin Keith Ford wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3329/
 ---
 
 (Updated March 11, 2014, 2:54 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-19167
 https://issues.asterisk.org/jira/browse/ASTERISK-19167
 
 
 Repository: testsuite
 
 
 Description
 ---
 
 Notes:
 - Is a sub-task of ASTERISK-19167
 - Ignore userA directory; will be removed from repository
 
 This test runs through a few different AGI scripts to verify that AGISTATUS 
 returns the correct values:
 1. Attempts to run an AGI script that does not exist. AGISTATUS returns 
 NOTFOUND.
 2. Attempts to run an AGI script that has an invalid path. AGISTATUS returns 
 FAILURE.
 3. Attempts to run an AGI script that has a non-executable interpreter. 
 AGISTATUS returns FAILURE.
 4. Attempts to run an AGI script that is non-executable. AGISTATUS returns 
 FAILURE.
 5. Runs an AGI script that will be hung up on while waiting for a command. 
 AGISTATUS returns HANGUP.
 6. Runs an AGI script that will be hung up on while executing a command. 
 AGISTATUS returns HANGUP.
 7. Runs an AGI script that exits normally. AGISTATUS returns SUCCESS.
 
 
 Diffs
 -
 
   ./asterisk/trunk/tests/agi/exit_status/waiting.agi PRE-CREATION 
   ./asterisk/trunk/tests/agi/exit_status/test-config.yaml 4749 
   ./asterisk/trunk/tests/agi/exit_status/run-test 4749 
   ./asterisk/trunk/tests/agi/exit_status/executing.agi PRE-CREATION 
   ./asterisk/trunk/tests/agi/exit_status/donothing.agi PRE-CREATION 
   ./asterisk/trunk/tests/agi/exit_status/configs/ast1/extensions.conf 
 PRE-CREATION 
   ./asterisk/trunk/tests/agi/exit_status/badinterpreter3.agi PRE-CREATION 
   ./asterisk/trunk/tests/agi/exit_status/badinterpreter2.agi PRE-CREATION 
   ./asterisk/trunk/tests/agi/exit_status/badinterpreter.agi PRE-CREATION 
 
 Diff: https://reviewboard.asterisk.org/r/3329/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Benjamin Keith Ford
 


-- 
_
-- 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] 3350: Add AES-GCM support for SRTP