date:20141219


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

Review request for Asterisk Developers.


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


Repository: Asterisk


Description
---

Valid payload type codes are between 0 and 127 to allow for being stored in 7 
bits.  During call setup, pjsip validates the SDP and will assert if it 
encounters an invalid payload type code (see pjmedia_sdp_validate() in 
pjmedia/src/pjmedia/sdp.c).  This assert will be hit if a call is placed to a 
pjsip endpoint with allow=all set.

To avoid this, the previous use 128 for the slin192 format has been changed to 
95.


Diffs
-

  /trunk/main/rtp_engine.c 429845 

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


Testing
---

Tested with pjsip calls to allow=all configured extensions.


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] 4279: chan_sip: Send CANCEL via proxy if CANCEL is to be sent after an UPDATE

2014-12-19 Thread kwemheuer


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

(Updated Dec. 19, 2014, 8:18 p.m.)


Review request for Asterisk Developers.


Changes
---

Updated considering comments by Mark and Scott.


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


Repository: Asterisk


Description
---

Given three SIP phones (A, B, C), all communicating via a proxy (in this case 
OpenSIPs) with asterisk. A call is established between A and B. B sets the call 
on hold (A is hearing MOH) and dials the extension of C. While phone C is 
ringing, B transfers the call (A is hearing ringing of phone C, B is idle). On 
SIP there is a REFER to transfer the call. In case "sendrpid=yes" asterisk 
sends an UPDATE to phone C. This update is sent directly (not through the 
proxy). If someone (e.g. B) is trying to get the call back (through a directed 
pickup), asterisk sends a CANCEL to C. But this CANCEL is sent directly to C 
not through the proxy. The phone ignores this CANCEL according to the RFC3261 
(Section 9.1).

In other situations asterisk ensures, that a CANCEL is sent the same way the 
INVITE has been sent. In this case the previously sent UPDATE overwrites the 
address used later for the CANCEL. On the other hand the UPDATE has to be sent 
via the proxy too (IMHO). The call is in the ringing state and other endpoints 
maybe have to been updated too.

The patch solves the issue. In function reqprep the UPDATE is handled the same 
way the CANCEL was (in case the state is in ringing state).


Diffs (updated)
-

  http://svn.asterisk.org/svn/asterisk/branches/11/channels/chan_sip.c 429824 

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


Testing
---

Patch is tested in the described scenario and solves the issue. Patched against 
11.15.0.


Thanks,

kwemheuer

-- 
_
-- 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] 4267: Testsuite: Add blind transfer tests for Stasis application interaction.

2014-12-19 Thread jbigelow


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

(Updated Dec. 19, 2014, 1:54 p.m.)


Review request for Asterisk Developers and Mark Michelson.


Changes
---

Implemented suggestion along with additional checks for the channels of the 
BridgeBlindTransfer event.


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


Repository: testsuite


Description
---

This adds the remaining blind transfer tests 1.9 & 1.10 as described on the 
StasisStart/StasisEnd Test Plan at: 
https://wiki.asterisk.org/wiki/pages/viewpage.action?pageId=30279826

This additionally updates the existing test 'stasis_bridge_to_non_stasis_app' 
(1.8) to verify the StasisEnd events of the channels per the test plan. An 
additional (dummy) channel was added for the test to prevent the test from 
ending when the channels involved in the test are hung up. This allows the 
StasisEnd events of all the other channels to be verified before the test has 
ended. The test description has also been updated to include more details about 
the test.


Diffs (updated)
-

  /asterisk/trunk/tests/rest_api/external_interaction/blind_transfer/tests.yaml 
6105 
  
/asterisk/trunk/tests/rest_api/external_interaction/blind_transfer/stasis_bridge_to_same_stasis_app/test-config.yaml
 PRE-CREATION 
  
/asterisk/trunk/tests/rest_api/external_interaction/blind_transfer/stasis_bridge_to_same_stasis_app/sipp/referer.xml
 PRE-CREATION 
  
/asterisk/trunk/tests/rest_api/external_interaction/blind_transfer/stasis_bridge_to_same_stasis_app/sipp/referee.xml
 PRE-CREATION 
  
/asterisk/trunk/tests/rest_api/external_interaction/blind_transfer/stasis_bridge_to_same_stasis_app/configs/ast1/pjsip.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/rest_api/external_interaction/blind_transfer/stasis_bridge_to_same_stasis_app/configs/ast1/extensions.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/rest_api/external_interaction/blind_transfer/stasis_bridge_to_non_stasis_app/test-config.yaml
 6105 
  
/asterisk/trunk/tests/rest_api/external_interaction/blind_transfer/stasis_bridge_to_non_stasis_app/configs/ast1/extensions.conf
 6105 
  
/asterisk/trunk/tests/rest_api/external_interaction/blind_transfer/stasis_bridge_to_non_stasis_app/blind_transfer.py
 6105 
  
/asterisk/trunk/tests/rest_api/external_interaction/blind_transfer/stasis_bridge_to_different_stasis_app/test-config.yaml
 PRE-CREATION 
  
/asterisk/trunk/tests/rest_api/external_interaction/blind_transfer/stasis_bridge_to_different_stasis_app/sipp/referer.xml
 PRE-CREATION 
  
/asterisk/trunk/tests/rest_api/external_interaction/blind_transfer/stasis_bridge_to_different_stasis_app/sipp/referee.xml
 PRE-CREATION 
  
/asterisk/trunk/tests/rest_api/external_interaction/blind_transfer/stasis_bridge_to_different_stasis_app/configs/ast1/pjsip.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/rest_api/external_interaction/blind_transfer/stasis_bridge_to_different_stasis_app/configs/ast1/extensions.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/rest_api/external_interaction/blind_transfer/blind_transfer.py
 PRE-CREATION 

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


Testing
---

* Executed tests multiple times
* Reviewed logs to manually verify StasisStart/StasisEnd events occurred.


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] 4259: DTMF atxfer: Setup recall channels as if the original transferrer originated the call.


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

Ship it!


Ship It!

- Matt Jordan


On Dec. 15, 2014, 12:51 p.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4259/
> ---
> 
> (Updated Dec. 15, 2014, 12:51 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23841
> https://issues.asterisk.org/jira/browse/ASTERISK-23841
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> After the initial DTMF atxfer call attempt to the transfer target fails to
> answer during a blonde transfer, the recall callback channels do not get
> setup with information from the initial transferrer channel.  As a result,
> the recall callback to the transferrer does not have callid, channel
> variables, datastores, accountcode, peeraccount, COLP, and CLID setup.  A
> similar situation happens with the recall callback to the transfer target
> but it is less visible.  The recall callback to the transfer target does
> not have callid, channel variables, datastores, accountcode, peeraccount,
> and COLP setup.
> 
> * Added missing information to the recall callback channels before
> initiating the call.  callid, channel variables, datastores, accountcode,
> peeraccount, COLP, and CLID
> 
> * Set callid of the transferrer channel on the DTMF atxfer controller
> thread attended_transfer_monitor_thread().
> 
> * Added missing channel unlocks and props unref to off nominal paths in
> attended_transfer_properties_alloc().
> 
> 
> Diffs
> -
> 
>   /branches/13/main/bridge_basic.c 429611 
> 
> Diff: https://reviewboard.asterisk.org/r/4259/diff/
> 
> 
> Testing
> ---
> 
> A calls B
> B initiates a DTMF blonde transfer to C but C doesn't answer
> 
> * When B is recalled, B sees A's CLID with the patch and a UUID without
> the patch.
> 
> * If B answers the recall call, A gets the original B channel COLP
> information with the patch.  Without the patch A loses COLP information
> such as B's name.  Also "core show channel B" shows channel variables that
> wouldn't be there without the patch.
> 
> * The debug log now has a callid for the recall calls where before the
> recall calls didn't have any callid.
> 
> 
> 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] 4282: queue_log: Post QUEUESTART entry when Asterisk fully boots.


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

Ship it!


Ew. Ew ew ew.

On the other hand, I can't think of another way to do this and restore the 
behaviour that used to exist. So, blech.

- Matt Jordan


On Dec. 17, 2014, 4:55 p.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4282/
> ---
> 
> (Updated Dec. 17, 2014, 4:55 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> The QUEUESTART log entry has historically acted like a fully booted event for 
> the queue_log file.  When the QUEUESTART entry was posted to the log was 
> broken by the change made by ASTERISK-15863.
> 
> * Made post the QUEUESTART queue_log entry when Asterisk fully boots.  This 
> restores the intent of that log entry and happens after realtime has had a 
> chance to load.
> 
> 
> Diffs
> -
> 
>   /branches/11/main/logger.c 429698 
>   /branches/11/main/asterisk.c 429698 
>   /branches/11/include/asterisk/_private.h 429698 
> 
> Diff: https://reviewboard.asterisk.org/r/4282/diff/
> 
> 
> Testing
> ---
> 
> The QUEUESTART entry is now written to the queue_log file when Asterisk boots 
> rather than waiting for the first event to happen.
> 
> 
> 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] 4264: PJSIP: Update transport method documentation


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

Ship it!


Ship It!

- Matt Jordan


On Dec. 15, 2014, 9:43 a.m., opticron wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4264/
> ---
> 
> (Updated Dec. 15, 2014, 9:43 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This updates the documentation for the 'method' configuration option to be 
> more verbose about the behaviors of values 'unspecified' and 'default'. They 
> do exactly the same thing which is to select the default as defined by PJSIP 
> which is currently TLSv1.
> 
> 
> Diffs
> -
> 
>   branches/13/res/res_pjsip.c 429431 
> 
> Diff: https://reviewboard.asterisk.org/r/4264/diff/
> 
> 
> Testing
> ---
> 
> Ensured that Asterisk compiled and that the XML schema verification passed 
> and that the new documentation was visible on the CLI.
> 
> 
> 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] 4279: chan_sip: Send CANCEL via proxy if CANCEL is to be sent after an UPDATE



> On Dec. 19, 2014, 9:59 a.m., Scott Griepentrog wrote:
> > http://svn.asterisk.org/svn/asterisk/branches/11/channels/chan_sip.c, lines 
> > 11834-11842
> > 
> >
> > I would recommend appending to the comment block something like this:
> > 
> > ASTERISK-24628: Send UPDATE to the same destination as CANCEL
> 
> kwemheuer wrote:
> OK. I think in the comment is a little typo ("unless this is a CANCEL 
> _on_ an ACK.."). I think, it should have been "unless this is a CANCEL _or_ 
> an ACK..". At least this is, what the code is doing. Should I fix this, when 
> adding the new comment?

We don't typically put issue numbers in comments, as the commit message should 
have the issue number referenced in it. The output 'svn blame' should help 
explain the issue that caused the code change.


> On Dec. 19, 2014, 9:59 a.m., Scott Griepentrog wrote:
> > http://svn.asterisk.org/svn/asterisk/branches/11/channels/chan_sip.c, lines 
> > 11840-11842
> > 
> >
> > Purely for readability, the sipmethod == SIP_UPDATE condition should be 
> > indented to the same level as SIP_ACK, since it is similarly nested under 
> > the same negation started on the line containing SIP_CANCEL.
> 
> kwemheuer wrote:
> OK. Shall I update my patch?

Yes! :-)


- Matt


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


On Dec. 17, 2014, 12:11 p.m., kwemheuer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4279/
> ---
> 
> (Updated Dec. 17, 2014, 12:11 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24628
> https://issues.asterisk.org/jira/browse/ASTERISK-24628
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Given three SIP phones (A, B, C), all communicating via a proxy (in this case 
> OpenSIPs) with asterisk. A call is established between A and B. B sets the 
> call on hold (A is hearing MOH) and dials the extension of C. While phone C 
> is ringing, B transfers the call (A is hearing ringing of phone C, B is 
> idle). On SIP there is a REFER to transfer the call. In case "sendrpid=yes" 
> asterisk sends an UPDATE to phone C. This update is sent directly (not 
> through the proxy). If someone (e.g. B) is trying to get the call back 
> (through a directed pickup), asterisk sends a CANCEL to C. But this CANCEL is 
> sent directly to C not through the proxy. The phone ignores this CANCEL 
> according to the RFC3261 (Section 9.1).
> 
> In other situations asterisk ensures, that a CANCEL is sent the same way the 
> INVITE has been sent. In this case the previously sent UPDATE overwrites the 
> address used later for the CANCEL. On the other hand the UPDATE has to be 
> sent via the proxy too (IMHO). The call is in the ringing state and other 
> endpoints maybe have to been updated too.
> 
> The patch solves the issue. In function reqprep the UPDATE is handled the 
> same way the CANCEL was (in case the state is in ringing state).
> 
> 
> Diffs
> -
> 
>   http://svn.asterisk.org/svn/asterisk/branches/11/channels/chan_sip.c 429698 
> 
> Diff: https://reviewboard.asterisk.org/r/4279/diff/
> 
> 
> Testing
> ---
> 
> Patch is tested in the described scenario and solves the issue. Patched 
> against 11.15.0.
> 
> 
> Thanks,
> 
> kwemheuer
> 
>

-- 
_
-- 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] 4281: Testsuite: Single channel redirect tests



> On Dec. 17, 2014, 11:55 p.m., Mark Michelson wrote:
> > ./redirect/single/bridge/test-config.yaml, line 41
> > 
> >
> > Hm, I'd expect Local/alice-;2 to be bridged, not 
> > Local/alice-;1.
> 
> Kevin Harwell wrote:
> ;1 is the one that enters the bridge.  Changing it fails the test.

You can discard this issue.


- Mark


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


On Dec. 19, 2014, 4:57 p.m., Kevin Harwell wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4281/
> ---
> 
> (Updated Dec. 19, 2014, 4:57 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24536
> https://issues.asterisk.org/jira/browse/ASTERISK-24536
> 
> 
> Repository: testsuite
> 
> 
> Description
> ---
> 
> Wrote two tests:
> 
> 1) Redirect a single channel that is in the Echo application, verify that it 
> gets redirected to the appropriate dialplan location, and make sure no 
> surrogate channels are hanging around.
> 
> 2) Redirect a single channel that is in a bridge with another channel, verify 
> that the channel gets redirected to the appropriate dialplan location, and 
> make sure the channel it was bridged with is hung up.
> 
> 
> Diffs
> -
> 
>   /asterisk/trunk/tests/manager/tests.yaml 6126 
>   /asterisk/trunk/tests/manager/redirect/tests.yaml PRE-CREATION 
>   /asterisk/trunk/tests/manager/redirect/single/tests.yaml PRE-CREATION 
>   /asterisk/trunk/tests/manager/redirect/single/bridge/test-config.yaml 
> PRE-CREATION 
>   
> /asterisk/trunk/tests/manager/redirect/single/bridge/configs/ast1/extensions.conf
>  PRE-CREATION 
>   /asterisk/trunk/tests/manager/redirect/single/app/test-config.yaml 
> PRE-CREATION 
>   
> /asterisk/trunk/tests/manager/redirect/single/app/configs/ast1/extensions.conf
>  PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/4281/diff/
> 
> 
> Testing
> ---
> 
> Ran tests and they passed.
> 
> 
> Thanks,
> 
> Kevin Harwell
> 
>

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4281: Testsuite: Single channel redirect tests

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

/asterisk/trunk/tests/manager/redirect/single/bridge/test-config.yaml

On my last review, I noted that I thought that this should be 
Local/alice.*2$, but you mentioned that this was not correct. I ran the test 
and figured out what's up.

Origination operates on the ;1 channel, so origination creates the ;1 
channel and waits for an answer. The ;2 channel is created to run dialplan. In 
this case, the ;2 channel goes into alice@default and answers. Origination gets 
the answer and places the ;1 channel into the call-bob extension. So it makes 
sense that the ;1 channel is the one to get bridged...

/asterisk/trunk/tests/manager/redirect/single/bridge/test-config.yaml

...which leads me to an actual finding. The purpose of this test is to 
redirect a channel that's in a bridge. Right now, you're redirecting a channel 
that is in the Echo application, and that channel happens to be linked via 
local channel magic to a channel in a bridge.

Change the redirect to move Local/alice@default-;1 instead. You'll 
also need to change your UserEvent expectation later.

- Mark Michelson

On Dec. 19, 2014, 4:57 p.m., Kevin Harwell wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4281/
> ---
> 
> (Updated Dec. 19, 2014, 4:57 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24536
> https://issues.asterisk.org/jira/browse/ASTERISK-24536
> 
> 
> Repository: testsuite
> 
> 
> Description
> ---
> 
> Wrote two tests:
> 
> 1) Redirect a single channel that is in the Echo application, verify that it 
> gets redirected to the appropriate dialplan location, and make sure no 
> surrogate channels are hanging around.
> 
> 2) Redirect a single channel that is in a bridge with another channel, verify 
> that the channel gets redirected to the appropriate dialplan location, and 
> make sure the channel it was bridged with is hung up.
> 
> 
> Diffs
> -
> 
>   /asterisk/trunk/tests/manager/tests.yaml 6126 
>   /asterisk/trunk/tests/manager/redirect/tests.yaml PRE-CREATION 
>   /asterisk/trunk/tests/manager/redirect/single/tests.yaml PRE-CREATION 
>   /asterisk/trunk/tests/manager/redirect/single/bridge/test-config.yaml 
> PRE-CREATION 
>   
> /asterisk/trunk/tests/manager/redirect/single/bridge/configs/ast1/extensions.conf
>  PRE-CREATION 
>   /asterisk/trunk/tests/manager/redirect/single/app/test-config.yaml 
> PRE-CREATION 
>   
> /asterisk/trunk/tests/manager/redirect/single/app/configs/ast1/extensions.conf
>  PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/4281/diff/
> 
> 
> Testing
> ---
> 
> Ran tests and they passed.
> 
> 
> Thanks,
> 
> Kevin Harwell
> 
>

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4284: Testsuite: Update ARI test for continuation and add new ARI test for origination to dialplan location

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

(Updated Dec. 19, 2014, 6:18 p.m.)

Review request for Asterisk Developers.

Changes
---

Fix formatting of lines in extensions.conf in the continuation tests. Some
lines began with a tab and others began with four spaces. All indented lines
now start with a tab.

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

Repository: testsuite

Description
---

Because of label support being added to ARI continuation/origination, I decided
to make sure the new feature was well-tested in order to get the feature into
Asterisk 13.

The continuation test as it exists only attempts to ensure that continuing to
the next priority in the dialplan works correctly. The continuation test has
been overhauled to instead test 24 combinations of specifying varying contexts,
extensions, priorities, and labels to ensure that all combinations direct the
channel to the expected location.

There were no existing tests for originating a channel to a dialplan location,
so I have added one. This test performs 12 originations that test various
combinations of context, extension, priority, and label. This has half as many
test cases as continuation because origination mandates that an extension must
be specified. So that eliminates the other 12 test cases that continuation has
where we do not specify an extension.

There is one potentially controversial change made to lib/python/ari.py. I
found it kind of annoying that I could not originate a channel and specify
variables on it from my test-config.yaml. So I modified the post and put
functions to be able to do this. If the 'json' key exists in the keyword
parameters, it is popped out of the parameters and passed to the requests
library function as its json parameter. This way, I could do the entire
origination test in test-config.yaml.

The big thing I'm looking for on this test is to make sure that the assumptions
about where to continue/originate when certain pieces of information are
omitted makes sense.

Diffs (updated)
-

/asterisk/trunk/tests/rest_api/continue/test-config.yaml 6075
/asterisk/trunk/tests/rest_api/continue/rest_continue.py 6075
/asterisk/trunk/tests/rest_api/continue/configs/ast1/extensions.conf 6075
/asterisk/trunk/tests/rest_api/channels/tests.yaml 6076

/asterisk/trunk/tests/rest_api/channels/originate_to_dialplan/test-config.yaml
PRE-CREATION

/asterisk/trunk/tests/rest_api/channels/originate_to_dialplan/configs/ast1/extensions.conf
PRE-CREATION
/asterisk/trunk/lib/python/asterisk/ari.py 6075

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

Testing
---

The tests send channels to all the expected locations. If I modify locations in
tests, I have verified that the channels go to the modified locations instead.

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] 4279: chan_sip: Send CANCEL via proxy if CANCEL is to be sent after an UPDATE

2014-12-19 Thread kwemheuer



> On Dec. 19, 2014, 3:59 p.m., Scott Griepentrog wrote:
> > http://svn.asterisk.org/svn/asterisk/branches/11/channels/chan_sip.c, lines 
> > 11834-11842
> > 
> >
> > I would recommend appending to the comment block something like this:
> > 
> > ASTERISK-24628: Send UPDATE to the same destination as CANCEL

OK. I think in the comment is a little typo ("unless this is a CANCEL _on_ an 
ACK.."). I think, it should have been "unless this is a CANCEL _or_ an ACK..". 
At least this is, what the code is doing. Should I fix this, when adding the 
new comment?


> On Dec. 19, 2014, 3:59 p.m., Scott Griepentrog wrote:
> > http://svn.asterisk.org/svn/asterisk/branches/11/channels/chan_sip.c, lines 
> > 11840-11842
> > 
> >
> > Purely for readability, the sipmethod == SIP_UPDATE condition should be 
> > indented to the same level as SIP_ACK, since it is similarly nested under 
> > the same negation started on the line containing SIP_CANCEL.

OK. Shall I update my patch?


- kwemheuer


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


On Dec. 17, 2014, 6:11 p.m., kwemheuer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4279/
> ---
> 
> (Updated Dec. 17, 2014, 6:11 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24628
> https://issues.asterisk.org/jira/browse/ASTERISK-24628
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Given three SIP phones (A, B, C), all communicating via a proxy (in this case 
> OpenSIPs) with asterisk. A call is established between A and B. B sets the 
> call on hold (A is hearing MOH) and dials the extension of C. While phone C 
> is ringing, B transfers the call (A is hearing ringing of phone C, B is 
> idle). On SIP there is a REFER to transfer the call. In case "sendrpid=yes" 
> asterisk sends an UPDATE to phone C. This update is sent directly (not 
> through the proxy). If someone (e.g. B) is trying to get the call back 
> (through a directed pickup), asterisk sends a CANCEL to C. But this CANCEL is 
> sent directly to C not through the proxy. The phone ignores this CANCEL 
> according to the RFC3261 (Section 9.1).
> 
> In other situations asterisk ensures, that a CANCEL is sent the same way the 
> INVITE has been sent. In this case the previously sent UPDATE overwrites the 
> address used later for the CANCEL. On the other hand the UPDATE has to be 
> sent via the proxy too (IMHO). The call is in the ringing state and other 
> endpoints maybe have to been updated too.
> 
> The patch solves the issue. In function reqprep the UPDATE is handled the 
> same way the CANCEL was (in case the state is in ringing state).
> 
> 
> Diffs
> -
> 
>   http://svn.asterisk.org/svn/asterisk/branches/11/channels/chan_sip.c 429698 
> 
> Diff: https://reviewboard.asterisk.org/r/4279/diff/
> 
> 
> Testing
> ---
> 
> Patch is tested in the described scenario and solves the issue. Patched 
> against 11.15.0.
> 
> 
> Thanks,
> 
> kwemheuer
> 
>

-- 
_
-- 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] 4101: Channel Originate/Continue via ARI support for labels in dialplan is incomplete

2014-12-19 Thread greenfieldtech


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

(Updated Dec. 19, 2014, 5:46 p.m.)


Status
--

This change has been discarded.


Review request for Asterisk Developers.


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


Repository: Asterisk


Description
---

This patch changes the current behavior of ARI, to allow channel 
originate/continue requests to be performed with labels as the priority, not 
only integer values.


Diffs
-

  /trunk/rest-api/api-docs/channels.json 429698 
  /trunk/res/res_ari_channels.c 429698 
  /trunk/res/ari/resource_channels.c 429698 
  /trunk/res/ari/resource_channels.h 429698 

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


Testing
---

Testing was performed by testing the following scenarios:
1. Originating a call to a numeric priority - works
2. Originating a call to a null priority - works
3. Originating a call to a label - works
4. Continue a call to a label - not tested yet


Thanks,

greenfieldtech

-- 
_
-- 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] 4101: Channel Originate/Continue via ARI support for labels in dialplan is incomplete


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


When I was writing tests for this new functionality, I came across some bugs 
(some introduced by the patch, and some pre-existing). So I have uploaded a new 
patch onto review board at /r/4285. I'm closing this review so focus can shift 
to the new one.

- Mark Michelson


On Dec. 17, 2014, 6:59 p.m., greenfieldtech wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4101/
> ---
> 
> (Updated Dec. 17, 2014, 6:59 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24412
> https://issues.asterisk.org/jira/browse/ASTERISK-24412
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This patch changes the current behavior of ARI, to allow channel 
> originate/continue requests to be performed with labels as the priority, not 
> only integer values.
> 
> 
> Diffs
> -
> 
>   /trunk/rest-api/api-docs/channels.json 429698 
>   /trunk/res/res_ari_channels.c 429698 
>   /trunk/res/ari/resource_channels.c 429698 
>   /trunk/res/ari/resource_channels.h 429698 
> 
> Diff: https://reviewboard.asterisk.org/r/4101/diff/
> 
> 
> Testing
> ---
> 
> Testing was performed by testing the following scenarios:
> 1. Originating a call to a numeric priority - works
> 2. Originating a call to a null priority - works
> 3. Originating a call to a label - works
> 4. Continue a call to a label - not tested yet
> 
> 
> Thanks,
> 
> greenfieldtech
> 
>

-- 
_
-- 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] 4262: bridge: channel ref leak on blond_nonfinal_enter


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

(Updated Dec. 19, 2014, 11:25 a.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 429826


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


Repository: Asterisk


Description
---

After a blond transfer to a destination that then hangs up, the Local;1 channel 
would suffer a reference leak.  This was due to blond_nonfinal_enter() 
resetting the props->transfer_target to NULL as it unref'd the channel for 
itself, later preventing the attended_transfer_properties_destructor() from 
releasing it's own reference, which was inherited during ast_bridge_impart with 
INDEPDENDENT.

This change prevents props->transfer_target from being set to NULL, and thus 
eliminates the leak.


Diffs
-

  /branches/12/main/bridge_basic.c 429516 

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


Testing
---

Newly created test 'atxfer_fail_blonde' 
(https://reviewboard.asterisk.org/r/4256/) now passes.  No other new test 
failures encountered.


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

[asterisk-dev] [Code Review] 4285: Bug fixes for ARI Originate/Continue with label support (Continuation of /r/4101)


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

Review request for Asterisk Developers.


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


Repository: Asterisk


Description
---

See /r/4101 for the initial review uploaded by Nir Simionivich to understand 
the need for the ability to originate/continue to a labeled dialplan priority.

When writing the tests in /r/4284 for continuation/origination, I ran into some 
problems, and so I have updated the original patch to fix these issues. Here 
are the modifications to the original patch:

* In continuation code, we get a channel snapshot from the stasis_app_control 
instead of getting an actual channel instance. This is because 
pbx_findlabel_extension doesn't actually use the channel for anything, so the 
channel isn't necessary.
* In both continuation and origination code, do not pass the input context to 
pbx_findlabel_extension. If no context is specified, this causes a crash. 
Instead, determine the intended context beforehand and pass that context into 
pbx_findlabel_extension.
* This is not a fault of the previous patch, but there were code paths that 
resulted in some unexpected behavior in continuation. Since continuation states 
that context, extension, priority, and label are all optional, but didn't 
really do anything to make sense of what should happen when one or more are 
omitted, the code has been updated to make sure that a sane default is used 
when one or more of these are omitted.
* I have updated the apiVersion in rest-api/resources.json to "1.7.0" since 
this change introduces backwards-compatible new functionality. QUESTION: Is 
this version bump required in other .json files as well?
* I have updated CHANGES to indicate the new functionality


Diffs
-

  /branches/13/rest-api/resources.json 429698 
  /branches/13/rest-api/api-docs/channels.json 429698 
  /branches/13/res/res_ari_channels.c 429698 
  /branches/13/res/ari/resource_channels.c 429698 
  /branches/13/res/ari/resource_channels.h 429698 
  /branches/13/CHANGES 429698 

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


Testing
---

See /r/4284 for tests.


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] 4284: Testsuite: Update ARI test for continuation and add new ARI test for origination to dialplan location

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

Review request for Asterisk Developers.

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

Repository: testsuite

Description
---

Because of label support being added to ARI continuation/origination, I decided
to make sure the new feature was well-tested in order to get the feature into
Asterisk 13.

The big thing I'm looking for on this test is to make sure that the assumptions
about where to continue/originate when certain pieces of information are
omitted makes sense.

Diffs
-

/asterisk/trunk/tests/rest_api/channels/originate_to_dialplan/test-config.yaml
PRE-CREATION

/asterisk/trunk/tests/rest_api/channels/originate_to_dialplan/configs/ast1/extensions.conf
PRE-CREATION
/asterisk/trunk/lib/python/asterisk/ari.py 6075

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

Testing
---

The tests send channels to all the expected locations. If I modify locations in
tests, I have verified that the channels go to the modified locations instead.

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] 4281: Testsuite: Single channel redirect tests

2014-12-19 Thread Kevin Harwell


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

(Updated Dec. 19, 2014, 10:57 a.m.)


Review request for Asterisk Developers.


Changes
---

Updated based review feedback.  Also was finally able to get reviewboard to 
include the modified parent dir's tests.yaml file.


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


Repository: testsuite


Description
---

Wrote two tests:

1) Redirect a single channel that is in the Echo application, verify that it 
gets redirected to the appropriate dialplan location, and make sure no 
surrogate channels are hanging around.

2) Redirect a single channel that is in a bridge with another channel, verify 
that the channel gets redirected to the appropriate dialplan location, and make 
sure the channel it was bridged with is hung up.


Diffs (updated)
-

  /asterisk/trunk/tests/manager/tests.yaml 6126 
  /asterisk/trunk/tests/manager/redirect/tests.yaml PRE-CREATION 
  /asterisk/trunk/tests/manager/redirect/single/tests.yaml PRE-CREATION 
  /asterisk/trunk/tests/manager/redirect/single/bridge/test-config.yaml 
PRE-CREATION 
  
/asterisk/trunk/tests/manager/redirect/single/bridge/configs/ast1/extensions.conf
 PRE-CREATION 
  /asterisk/trunk/tests/manager/redirect/single/app/test-config.yaml 
PRE-CREATION 
  
/asterisk/trunk/tests/manager/redirect/single/app/configs/ast1/extensions.conf 
PRE-CREATION 

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


Testing
---

Ran tests and they passed.


Thanks,

Kevin Harwell

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4279: chan_sip: Send CANCEL via proxy if CANCEL is to be sent after an UPDATE


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

Ship it!


Other than a few non-code issues, I ship thee.  Did not trigger any SIP tagged 
test failures.


http://svn.asterisk.org/svn/asterisk/branches/11/channels/chan_sip.c


I would recommend appending to the comment block something like this:

ASTERISK-24628: Send UPDATE to the same destination as CANCEL



http://svn.asterisk.org/svn/asterisk/branches/11/channels/chan_sip.c


Purely for readability, the sipmethod == SIP_UPDATE condition should be 
indented to the same level as SIP_ACK, since it is similarly nested under the 
same negation started on the line containing SIP_CANCEL.


- Scott Griepentrog


On Dec. 17, 2014, 12:11 p.m., kwemheuer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4279/
> ---
> 
> (Updated Dec. 17, 2014, 12:11 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24628
> https://issues.asterisk.org/jira/browse/ASTERISK-24628
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Given three SIP phones (A, B, C), all communicating via a proxy (in this case 
> OpenSIPs) with asterisk. A call is established between A and B. B sets the 
> call on hold (A is hearing MOH) and dials the extension of C. While phone C 
> is ringing, B transfers the call (A is hearing ringing of phone C, B is 
> idle). On SIP there is a REFER to transfer the call. In case "sendrpid=yes" 
> asterisk sends an UPDATE to phone C. This update is sent directly (not 
> through the proxy). If someone (e.g. B) is trying to get the call back 
> (through a directed pickup), asterisk sends a CANCEL to C. But this CANCEL is 
> sent directly to C not through the proxy. The phone ignores this CANCEL 
> according to the RFC3261 (Section 9.1).
> 
> In other situations asterisk ensures, that a CANCEL is sent the same way the 
> INVITE has been sent. In this case the previously sent UPDATE overwrites the 
> address used later for the CANCEL. On the other hand the UPDATE has to be 
> sent via the proxy too (IMHO). The call is in the ringing state and other 
> endpoints maybe have to been updated too.
> 
> The patch solves the issue. In function reqprep the UPDATE is handled the 
> same way the CANCEL was (in case the state is in ringing state).
> 
> 
> Diffs
> -
> 
>   http://svn.asterisk.org/svn/asterisk/branches/11/channels/chan_sip.c 429698 
> 
> Diff: https://reviewboard.asterisk.org/r/4279/diff/
> 
> 
> Testing
> ---
> 
> Patch is tested in the described scenario and solves the issue. Patched 
> against 11.15.0.
> 
> 
> Thanks,
> 
> kwemheuer
> 
>

-- 
_
-- 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] 4283: Testsuite: Dual channel redirect tests

2014-12-19 Thread Kevin Harwell


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

Review request for Asterisk Developers.


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


Repository: testsuite


Description
---

Wrote three tests:

1) Redirect two channels that are in the Echo application, verify that they are 
both redirected to the appropriate dialplan location, make sure no surrogate 
channels are hanging around.

2) Redirect two channels that are bridged, verify that both channels are 
redirected to the same dialplan location, the bridge they were in gets 
destroyed, and neither channel is hung up by the process.

3) Redirect two channels that are in two different bridges, verify that both 
channels are redirected to the same dialplan location, the bridges they were in 
get destroyed, and neither channel is hung up by the process.

Also, added an option to the event action pluggable module instance that when 
set will not trigger the action until the count has been met on the event.


Diffs
-

  /asterisk/trunk/tests/manager/tests.yaml 6126 
  /asterisk/trunk/tests/manager/redirect/tests.yaml PRE-CREATION 
  /asterisk/trunk/tests/manager/redirect/dual/tests.yaml PRE-CREATION 
  /asterisk/trunk/tests/manager/redirect/dual/bridge/tests.yaml PRE-CREATION 
  /asterisk/trunk/tests/manager/redirect/dual/bridge/same/test-config.yaml 
PRE-CREATION 
  
/asterisk/trunk/tests/manager/redirect/dual/bridge/same/configs/ast1/extensions.conf
 PRE-CREATION 
  /asterisk/trunk/tests/manager/redirect/dual/bridge/different/test-config.yaml 
PRE-CREATION 
  
/asterisk/trunk/tests/manager/redirect/dual/bridge/different/configs/ast1/extensions.conf
 PRE-CREATION 
  /asterisk/trunk/tests/manager/redirect/dual/app/test-config.yaml PRE-CREATION 
  /asterisk/trunk/tests/manager/redirect/dual/app/configs/ast1/extensions.conf 
PRE-CREATION 
  /asterisk/trunk/lib/python/asterisk/ami.py 6126 

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


Testing
---

Ran tests and they passed.


Thanks,

Kevin Harwell

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] Adding the support for NACK in asterisk

2014-12-19 Thread Matthew Jordan

On Thu, Dec 18, 2014 at 5:06 AM, Nitesh Bansal  wrote:
>
> Hello,
>
> I am talking about NACK in the context of RTCP feedback (a=rtcp-fb:* nack) to 
> request retransmission of lost frames.
> I would like to advertise in my SDP that asterisk supports NACK, so WebRTC 
> browsers can
> request for lost packets, lost FIR or PLI messages via RTCP feedback.
>
> Since asterisk isn't source of the video, it will have to buffer the RTP 
> messages received from the remote peer and retransmit
> them when requested by WebRTC browsers.

Short answer: complicated and hard. :-)

For the sake of the explanation, I'm going to assume you want to
implement RFC 4585 (https://tools.ietf.org/html/rfc4585)

Part of this will be because you will be fighting inside
res_rtp_asterisk - which is a module that is long overdue for a
serious overhaul. Much like chan_sip, it has grown organically over
the years, and suffers from a lack of structure in certain parts (and
some dead code, and strange hacks for equipment that probably not
longer exists, etc.) Unlike chan_sip, it's not nearly so large that
it's impossible to work inside of - but it is challenging.

That isn't a reason _not_ to do this work, but it should be a
consideration - you'll want to think through how you can modify
res_rtp_asterisk such that the feedback mechanisms don't interfere
with the 'normal' operation of res_rtp_asterisk. A decent portion of
the work may just be simply restructuring res_rtp_asterisk to easily
handle the addition.

As for the work itself:

(1) You'll need to modify rtp_engine and res_rtp_asterisk to
understand compound RTCP packets properly. Currently, we always assume
that the RTCP packets contain a single SR/RR: that means modifications
to the parsing in ast_rtcp_read, as well as the conversion of received
RTCP reports in rtp_engine into JSON for delivery to RTP Stasis topic
consumers. Note that this would be an opportune time to make the
parsing in ast_rtcp_read not be quite so terrible as it is today.

(2) Going along with #1, you'll need to add additional RTCP report
types - both when reading and when writing. Currently, Asterisk only
supports SR/RR. The RFC mandates FR reports, along with SDES packets.
You'll also need to change the algorithm for when reports are
scheduled. Since RFC 4585 modifies the scheduling algorithm, you'll
pretty much have to abandon how scheduling is set up currently in
ast_rtp_read. There are different modes of operation, each with slight
variations to the algorithm for how/when reports should be sent. A
state machine would probably be handy for this.

(3) NACK mode. I would be very careful here: as a B2BUA, Asterisk
*always* assumes there are only two participants in an RTP session -
and making Asterisk realize that this isn't the case is going to be
more challenging than you may think. Asterisk has a lot of
implementation 'shortcuts' it takes in RTCP statistic management that
it takes because it knows that there is only one other RTP participant
other than itself: if you go down the road of wanting to maintain
additional statistics beyond the RTP instance and whoever it is
talking to, the scope of this would expand dramatically.

(4) If I'm reading the RFC correctly, you _may_ not need to buffer the
RTP packets themselves; rather, you need to inform the other side of
the RTP packets you've lost. Generally, this means keeping a limited
history of which RTP sequence numbers you've received, and which ones
are missed. When constructing the NACK message (in 5a), you'd see if
you had a missed RTP sequence number, then look to see if any of the
16 RTP packets after that one were also missed. After reporting this,
the buffer could be flushed - how long the buffer is probably depends
on the frequency of report sending, which is variable as per (2).

(5) RTCP feedback messages. The transport layer feedback messages
would obviously be provided by the RTP engine and/or res_rtp_asterisk.
However, the other layers - payload/application - should have
corresponding APIs in rtp_engine. In particular, the codec layer
(payload) would be interesting, as it would require the ability for
codec modules (or something else in the media 'section' of Asterisk)
to take an RTP instance and register a handler for providing feedback
messages at opportune times. The application layer would be similar,
except that currently, we probably wouldn't have an implementation.
You may want to consider that as 'future work', as it is probably
sufficient to be a project in its own right.
  (a) Generic NACK: these messages should be implemented in such a
fashion that res_rtp_asterisk can have one be constructed based on the
statistics in (4). A part of me wishes all of this were implemented in
RTP engine, so that other RTP implementations could take advantage of
the RTCP work. That's a consideration that should be given some
thought, given how much of the RTCP implementation would have to be
ripped apart by this work anyway.
  (b) PLI/SLI/RPSI: I'm not

Re: [asterisk-dev] [Code Review] 4280: sip_to_pjsip: improved ability to parse input without exception


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

(Updated Dec. 19, 2014, 7:52 a.m.)


Review request for Asterisk Developers.


Changes
---

Change default binding and improved help.


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


Repository: Asterisk


Description
---

General improvements to reliability of conversion utility:

1) track default section of input file to allow parsing an include file that 
doesn't specify a [section]

2) informatively handle case of assignment with no section

3) correctly handle getting sections from included files

4) assume default bind of 0.0.0.0

5) gracefully handle missing portions of registration string

6) Denote steps of operation and confirm top level conf files being 
read/written as a convenience


Diffs (updated)
-

  /branches/12/contrib/scripts/sip_to_pjsip/sip_to_pjsip.py 429613 
  /branches/12/contrib/scripts/sip_to_pjsip/astconfigparser.py 429613 

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


Testing
---

Ran on config files from various sources to insure no exceptions occurred.  
Perused output to confirm appearance of converted input values.


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] 4280: sip_to_pjsip: improved ability to parse input without exception