Re: [asterisk-dev] [Code Review] 4455: res_pjsip: conflicting endpoint identifiers

2015-03-02 Thread George Joseph

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


I think a cli command that lists the currently registered identifiers might be 
needed.  Otherwise how would you know, especially if the name of the module 
didn't start with res_pjsip_endpoint_identifier_*, or if you're the admin but 
not the compiler/installer and don't know what modules are available.

 pjsip list identifiers
Name. Module
anonymous   res_pjsip_endpoint_identifier_anonymous
ip  res_pjsip_endpoint_identifier_ip
usernameres_pjsip_endpoint_identifier_user





- George Joseph


On March 2, 2015, 1:04 p.m., Kevin Harwell wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4455/
 ---
 
 (Updated March 2, 2015, 1:04 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24840
 https://issues.asterisk.org/jira/browse/ASTERISK-24840
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 It's possible to have a scenario that will create a conflict between endpoint 
 identifiers. For instance an incoming call could be identified by two 
 different endpoint identifiers and the one chosen depended upon which 
 identifier module loaded first. This of course causes problems when, for 
 example, the incoming call is expected to be identified by username, but 
 instead is identified by ip. This patch adds a new 'global' option to 
 res_pjsip called 'identify_by_priority'. It is a comma separated list of 
 endpoint identifier names that specifies the order by which identifiers are 
 processed and checked.
 
 
 Diffs
 -
 
   branches/13/res/res_pjsip_endpoint_identifier_user.c 432422 
   branches/13/res/res_pjsip_endpoint_identifier_ip.c 432422 
   branches/13/res/res_pjsip_endpoint_identifier_anonymous.c 432422 
   branches/13/res/res_pjsip/config_global.c 432422 
   branches/13/res/res_pjsip.c 432422 
   branches/13/include/asterisk/res_pjsip.h 432422 
   
 branches/13/contrib/ast-db-manage/config/versions/45e3f47c6c44_add_pjsip_identify_by_priority.py
  PRE-CREATION 
   branches/13/configs/samples/pjsip.conf.sample 432422 
   branches/13/CHANGES 432422 
 
 Diff: https://reviewboard.asterisk.org/r/4455/diff/
 
 
 Testing
 ---
 
 Added a testsuite test: https://reviewboard.asterisk.org/r/4456/
 
 
 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] 4444: chan_dahdi/sig_analog: Fix distinctive ring detection to suck less.

2015-03-02 Thread rmudgett

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

(Updated March 2, 2015, 3:18 p.m.)


Review request for Asterisk Developers.


Changes
---

Made not stop searching for CID even if the distinctive ring pattern gets 
filled.  Some lengthy patterns fill the pattern and would abort detecting the 
caller id.


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


Repository: Asterisk


Description
---

The distinctive ring feature interferes with detecting Caller ID and
appears to have been broken for years.  What happens is if you have a
ring-ring cadence as used in the UK you get too many DAHDI events for the
distinctive ring pattern array and Caller ID detection is aborted.  I
think when Zapata/DAHDI added the ring begin event it broke distinctive
ring.  More events happen than before and the code does no filtering of
which event times are recorded in the pattern array.

* Made distinctive ring only record the ringt count when the ring ends
instead of on just any DAHDI event.  Distinctive ring can be ring,
ring-ring, ring-ring-ring, or different ring durations for the up to three
rings.

* Fixed the distinctive ring detection enable (chan_dahdi.conf option
usedistinctiveringdetection) to be per port instead of somewhat per port
and somewhat global.  This has been broken since v1.8.

* Fixed using the default distinctive ring context when the detected
pattern does not match any configured dringX patterns.  The default
context did not get set when the previous call was a matched distinctive
ring pattern and the current call is not matched.  This has been broken
since v1.8.

* Made distinctive ring have no effect on Caller ID detection when it is
disabled.  Caller ID detection just monitors for 10 seconds before giving
up.

* Fixed leak of struct callerid_state memory when a polarity reversal
during Caller ID detection causes the incoming call to be aborted.


Diffs (updated)
-

  /branches/11/channels/sig_analog.c 432422 
  /branches/11/channels/sig_analog.h 432422 
  /branches/11/channels/chan_dahdi.c 432422 
  /branches/11/UPGRADE.txt 432422 

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


Testing
---

Tested the following scenarios for US (ring) and UK (ring-ring) ring cadences:
1) usedistinctiveringdetection=no
2) usedistinctiveringdetection=yes with distinctiveringaftercid=no
3) usedistinctiveringdetection=yes with distinctiveringaftercid=yes

* Caller-ID was detected for each call
* The configured distinctive ring dringX pattern was detected and the specified 
context set.


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] Asterisk 13 and -march=native

2015-03-02 Thread Anatoli
Steve,

On virtualized systems -march=native sometimes doesn't work (a known bug 
unrelated to asterisk, qemu  gcc don't agree on processor type and required 
capabilities), so replace it with -march=x86-64.

Regards,
Anatoli


From: asterisk-dev-boun...@lists.digium.com 
[mailto:asterisk-dev-boun...@lists.digium.com] On Behalf Of Steve Murphy
Sent: Monday, March 02, 2015 17:51
To: Asterisk Developers Mailing List
Subject: [asterisk-dev] Asterisk 13 and -march=native

Hello!
Found an interesting problem with Asterisk 13 on a cloud 
box running CentOS 6.6. It compiled, but would die with 
an illegal instruction on loading. Looking at the result, 
I saw that it was dying on the assembler instruction:
vcvtsi2sdq -0x98.
Apparently, this cloud box is AMD opteron 64-bit, but does
not have that particular opcode supported. I see that every
file compiles with -march=native, and that this can be turned
on/off via the 'BUILD_NATIVE' compiler option in menuselect.
I looked this up in the gcc man page, and for the i386/AMD 64-bit options, I 
see (under -mtune):

   native
   This selects the CPU to tune for at compilation time by 
determining the 
   processor type of the compiling machine.  Using -mtune=native 
will
   produce code optimized for the local machine under the 
constraints of the 
   selected instruction set.  Using -march=native will enable all
   instruction subsets supported by the local machine (hence the 
result 
   might not run on different machines).
And, in this case, the code generated didn't even run on my own machine.
Now, I know that one simple solution to this problem is to turn off the native
compile option in menuselect. (I tried it, and it works).  I tried again, and  
turned it
on,but changed the makefile rules file to use -mtune=native instead, and this 
also 
worked to solve this crash.

Output of cat /proc/cpuinfo

vendor_id: AuthenticAMD
cpu family: 21
model: 1
model name: AMD Opteron(TM) Processor 6272 
stepping: 2
cpu MHz: 2100.096
cache size: 2048 KB
physical id: 0
siblings: 1
core id: 4
cpu cores: 1
apicid: 0
initial apicid: 4
fpu: yes
fpu_exception: yes
cpuid level: 13
wp: yes
flags: fpu de tsc msr pae cx8 cmov pat clflush mmx fxsr sse sse2 ht 
syscall nx mmxext fxsr_opt lm up rep_good unfair_spinlock pni pclmulqdq ssse3 
cx16 sse4_1 sse4_2 popcnt aes hypervisor lahf_lm cmp_legacy extapic cr8_legacy 
abm sse4a misalignsse 3dnowprefetch xop fma4 perfctr_core perfctr_nb
bogomips: 4200.19
TLB size: 1536 4K pages
clflush size: 64
cache_alignment: 64
address sizes: 48 bits physical, 48 bits virtual
power management:

On my own AMD powered workstation, I see, in one of my centos 6.6 vm's:

vendor_id: AuthenticAMD
cpu family: 6
model: 6
model name: QEMU Virtual CPU version 2.0.0
stepping: 3
...
flags: fpu de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat 
pse36 clflush mmx fxsr sse sse2 syscall nx lm up unfair_spinlock pni cx16 
x2apic popcnt hypervisor lahf_lm svm abm sse4a
Now, I guess I'm suggesting that -mtune=native be used instead of 
-march=native, as
it will, by default, yield a working set of code on the current machine, no 
matter what
the possible options are, as the BUILD_NATIVE option is turned on by default 
(at least
in my environment), and it's not easily recognized when BUILD_NATIVE is 
appropriate 
and when it is not.  Running on VM's appear to give stripped-down CPU's; and 
such
appears to a fad that will not go away soon... But, I'm no expert. Am I being 
silly?
murf
-- 

Steve Murphy
ParseTree Corporation




-- 
_
-- 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] 4455: res_pjsip: conflicting endpoint identifiers

2015-03-02 Thread George Joseph

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



branches/13/configs/samples/pjsip.conf.sample
https://reviewboard.asterisk.org/r/4455/#comment25120

I think you need to specify how the identifiers are derived.  I.E. from the 
res_pjsip_endpoint_identifier_* modules.




branches/13/res/res_pjsip.c
https://reviewboard.asterisk.org/r/4455/#comment25121

I'd add a comment here as well as to where to look for the names.



- George Joseph


On March 2, 2015, 1:04 p.m., Kevin Harwell wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4455/
 ---
 
 (Updated March 2, 2015, 1:04 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24840
 https://issues.asterisk.org/jira/browse/ASTERISK-24840
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 It's possible to have a scenario that will create a conflict between endpoint 
 identifiers. For instance an incoming call could be identified by two 
 different endpoint identifiers and the one chosen depended upon which 
 identifier module loaded first. This of course causes problems when, for 
 example, the incoming call is expected to be identified by username, but 
 instead is identified by ip. This patch adds a new 'global' option to 
 res_pjsip called 'identify_by_priority'. It is a comma separated list of 
 endpoint identifier names that specifies the order by which identifiers are 
 processed and checked.
 
 
 Diffs
 -
 
   branches/13/res/res_pjsip_endpoint_identifier_user.c 432422 
   branches/13/res/res_pjsip_endpoint_identifier_ip.c 432422 
   branches/13/res/res_pjsip_endpoint_identifier_anonymous.c 432422 
   branches/13/res/res_pjsip/config_global.c 432422 
   branches/13/res/res_pjsip.c 432422 
   branches/13/include/asterisk/res_pjsip.h 432422 
   
 branches/13/contrib/ast-db-manage/config/versions/45e3f47c6c44_add_pjsip_identify_by_priority.py
  PRE-CREATION 
   branches/13/configs/samples/pjsip.conf.sample 432422 
   branches/13/CHANGES 432422 
 
 Diff: https://reviewboard.asterisk.org/r/4455/diff/
 
 
 Testing
 ---
 
 Added a testsuite test: https://reviewboard.asterisk.org/r/4456/
 
 
 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] 4442: chan_sip: Asterisk fails to re-activate an inactive media session when an offer does not contain a=sendrecv

2015-03-02 Thread Ashley Sanders

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

(Updated March 2, 2015, 10:13 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 6483


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


Repository: testsuite


Description
---

This test is to ensure that Asterisk correctly applies the direction of the 
media stream when a=sendonly|recvonly|inactive|sendrecv is missing from the 
offer's SDP. The expected behavior is for Asterisk to apply sendrecv as the 
direction of the media stream when no direction attribute is present in an 
offer's SDP. According to RFC 4566 (Section 6. SDP Attributes): If none of the 
attributes sendonly, recvonly, inactive, and sendrecv is present, 
sendrecv SHOULD be assumed as the default for sessions that are not of the 
conference type broadcast or H332 [...]

The test scenario:

1. From Phone A, send an offer to Phone B to establish a call
2. From Phone B, send an offer to Phone A to put the call on hold. 
3. Observe that the MOH start event occurs.
4. From Phone B, send an offer to Phone A to 'un-hold' the call (ensure that 
the direction attribute from the offer's SDP is omitted)
5. Observe that the MOH stop event occurs.

***Note*** This is a test. It is only a test.


Diffs
-

  ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_no_direction.xml 
PRE-CREATION 
  ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_media_restrict.xml 
6458 
  ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_IP_restrict.xml 
6458 
  
./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_IP_media_restrict.xml 
6458 
  ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_A_no_direction.xml 
PRE-CREATION 
  ./asterisk/trunk/tests/channels/SIP/sip_hold/run-test 6458 

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


Testing
---


Thanks,

Ashley Sanders

-- 
_
-- 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] Asterisk 13 and -march=native

2015-03-02 Thread Steve Murphy
Hello!

Found an interesting problem with Asterisk 13 on a cloud
box running CentOS 6.6. It compiled, but would die with
an illegal instruction on loading. Looking at the result,
I saw that it was dying on the assembler instruction:

vcvtsi2sdq -0x98.

Apparently, this cloud box is AMD opteron 64-bit, but does
not have that particular opcode supported. I see that every
file compiles with -march=native, and that this can be turned
on/off via the 'BUILD_NATIVE' compiler option in menuselect.

I looked this up in the gcc man page, and for the i386/AMD 64-bit options,
I see (under -mtune):

   native
   This selects the CPU to tune for at compilation time by
determining the
   processor type of the compiling machine.  Using
-mtune=native will
   produce code optimized for the local machine under the
constraints of the
   selected instruction set.  Using -march=native will enable
all
   instruction subsets supported by the local machine (hence
the result
   might not run on different machines).

And, in this case, the code generated didn't even run on my own machine.
Now, I know that one simple solution to this problem is to turn off the
native
compile option in menuselect. (I tried it, and it works).  I tried again,
and  turned it
on,but changed the makefile rules file to use -mtune=native instead, and
this also
worked to solve this crash.

Output of cat /proc/cpuinfo

vendor_id: AuthenticAMD
cpu family: 21
model: 1
model name: AMD Opteron(TM) Processor 6272
stepping: 2
cpu MHz: 2100.096
cache size: 2048 KB
physical id: 0
siblings: 1
core id: 4
cpu cores: 1
apicid: 0
initial apicid: 4
fpu: yes
fpu_exception: yes
cpuid level: 13
wp: yes
flags: fpu de tsc msr pae cx8 cmov pat clflush mmx fxsr sse sse2 ht
syscall nx mmxext fxsr_opt lm up rep_good unfair_spinlock pni pclmulqdq
ssse3 cx16 sse4_1 sse4_2 popcnt aes hypervisor lahf_lm cmp_legacy extapic
cr8_legacy abm sse4a misalignsse 3dnowprefetch xop fma4 perfctr_core
perfctr_nb
bogomips: 4200.19
TLB size: 1536 4K pages
clflush size: 64
cache_alignment: 64
address sizes: 48 bits physical, 48 bits virtual
power management:


On my own AMD powered workstation, I see, in one of my centos 6.6 vm's:

vendor_id: AuthenticAMD
cpu family: 6
model: 6
model name: QEMU Virtual CPU version 2.0.0
stepping: 3
...
flags: fpu de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov
pat pse36 clflush mmx fxsr sse sse2 syscall nx lm up unfair_spinlock pni
cx16 x2apic popcnt hypervisor lahf_lm svm abm sse4a

Now, I guess I'm suggesting that -mtune=native be used instead of
-march=native, as
it will, by default, yield a working set of code on the current machine, no
matter what
the possible options are, as the BUILD_NATIVE option is turned on by
default (at least
in my environment), and it's not easily recognized when BUILD_NATIVE is
appropriate
and when it is not.  Running on VM's appear to give stripped-down CPU's;
and such
appears to a fad that will not go away soon... But, I'm no expert. Am I
being silly?

murf

-- 

Steve Murphy
ParseTree Corporation
-- 
_
-- 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] 4453: Asterisk 14: RTP improvements

2015-03-02 Thread Matt Jordan

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


Review of https://wiki.asterisk.org/wiki/display/AST/RTP+engine+replacement

-- Section: A glossary of terms

{quote}
* RTP stream: RTP instances created by Steel Zebra will be referred to as RTP 
streams.
* RTP session: A structure created by Steel Zebra that contains related RTP 
streams and coordinates activities between streams where necessary.
{quote}

Since RTP instances are created separately by a channel driver, how will the 
RTP engine be notified that multiple RTP instances (that is, streams) are 
related? This may be answered in another section, but it might be good to know 
that this will require changes in the RTP engine here.

-- Section: Media Flow: Incoming Media

{quote}
Buffering/reordering

RTP may be received in bursts, out of order, or in other less-than-ideal ways. 
Asterisk will implement reception buffers to place incoming RTP traffic into, 
potentially reordering packets as necessary if they arrive out of order.
{quote}

While I'm not against buffering in the RTP stack, have you given any thought 
how that would be set up? As it adds delay, I would expect that not every RTP 
stream should be buffered; consequently, this would need to be driven by 
configuration or by some dialplan construct. Configuration may work in some 
cases (for example, when you know that some endpoint is always jittery); in 
other cases, dialplan is probably a better approach. In both cases however, 
these would require manipulation at a layer higher than the RTP stack itself, 
which would mean drilling down through the RTP engine into the RTP instance - 
which ends up sound something like our current jitter buffer approaches. What 
advantages are there to buffering in the stack itself, versus simply expanding 
the jitter buffers to handle more than just VOICE frames? Would we want to 
provide buffering in native RTP bridges, or let the far endpoints handle the 
re-ordering?

-- Section: Other Stuff

{quote}
Native local RTP bridges

Native local RTP bridges have a few considerations when implementing a new RTP 
engine.

First, bridge_native_rtp requires that the RTP engine's local_bridge method has 
to be the same for each of the bridged RTP instances. If we create a new RTP 
engine, it will not have the same local_bridge method as res_rtp_asterisk. This 
means that calls that use res_rtp_asterisk will not be able to be locally 
bridged with calls that use the new RTP engine. I think it is possible to 
rework the inner workings of native local bridges such that they can be between 
different RTP engines. However, if the goal is the total removal of 
res_rtp_asterisk from the codebase, then such considerations are not as 
necessary.

Second, native local RTP bridging is performed at the main RTP API layer by 
having the bridged RTP instances point at each other. It is up to the 
individual RTP instances to detect that this has occurred and act accordingly. 
It might work better if the job of setting bridges on RTP instances were passed 
down to the engines themselves in case they want to perform other side effects 
besides changing a pointer.
{quote}

I think it is arguable whether or not the local_bridge code should be in 
res_rtp_asterisk still. Ideally, an RTP implementation would simply have a 
direct write/direct read callbacks that the bridge itself would call into, 
rather than let the RTP implementation do the actual bridging. This has a few 
advantages:
(1) We could implement some other more interesting RTP bridges (such as a 
multi-party RTP forwarding bridge or a RTP bridge w/ RTP recording)
(2) It simplifies the thread boundaries. Right now, it's a little tricky 
managing the safety of calling into an RTP implementation from 
bridge_native_rtp. Having a more concrete boundary between the bridge and the 
RTP implementations would be advantageous.

Of course, if res_rtp_asterisk is refactored as opposed to replaced, that makes 
altering and/or supporting the native bridging in any fashion easier.

I think you might be referring to this in your second point, but I'm not 
entirely sure if this is what you meant in it.

- Matt Jordan


On Feb. 27, 2015, 12:47 p.m., Mark Michelson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4453/
 ---
 
 (Updated Feb. 27, 2015, 12:47 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Description
 ---
 
 I've created a series of wiki pages that discuss the idea of writing an 
 improved RTP architecture in Asterisk 14.
 
 To regurgitate some details from the linked page, the current RTP engine in 
 Asterisk (res_rtp_asterisk) gets the job done but has 

Re: [asterisk-dev] [Code Review] 4453: Asterisk 14: RTP improvements

2015-03-02 Thread Olle E. Johansson

On 02 Mar 2015, at 16:54, Matt Jordan reviewbo...@asterisk.org wrote:

 {quote}
 Buffering/reordering
 
 RTP may be received in bursts, out of order, or in other less-than-ideal 
 ways. Asterisk will implement reception buffers to place incoming RTP traffic 
 into, potentially reordering packets as necessary if they arrive out of order.
 {quote}
 
By default, asterisk should forward RTP packets without any buffer, without 
reordering or doing anything. Today Asterisk renumbers packets - thus hiding 
packet loss and reordering. This is bad.

Forwarding with packet loss and reordering is quite ok as default. The endpoint 
in the call will sort it up with a jitter buffer.

In some cases Asterisk is the endpoint of the RTP stream (IVR, protocol 
conversion). In that case we can apply a buffer like any endpoint. But not by 
default.

I guess cases with transcoding involved also requires a jitter buffer.

/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] 4453: Asterisk 14: RTP improvements -session

2015-03-02 Thread Olle E. Johansson
Mark,
One thing I think we have to look into is grouping of media sessions. You do 
not mention that, but you point to the bundle RFC. There are a few cases, like 
video conferences with left/right video and different options for audio - from 
high bandwidth to low bandwidth, lossy compression. Alternatives in the group 
and stuff that belongs together.

Sorry for making your hair turn grey, but I think it's time. And this will of 
course go beyond RTP.

/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] 4453: Asterisk 14: RTP improvements

2015-03-02 Thread Mark Michelson


 On March 2, 2015, 3:54 p.m., Matt Jordan wrote:
  Review of https://wiki.asterisk.org/wiki/display/AST/RTP+engine+replacement
  
  -- Section: A glossary of terms
  
  {quote}
  * RTP stream: RTP instances created by Steel Zebra will be referred to as 
  RTP streams.
  * RTP session: A structure created by Steel Zebra that contains related RTP 
  streams and coordinates activities between streams where necessary.
  {quote}
  
  Since RTP instances are created separately by a channel driver, how will 
  the RTP engine be notified that multiple RTP instances (that is, streams) 
  are related? This may be answered in another section, but it might be good 
  to know that this will require changes in the RTP engine here.
  
  -- Section: Media Flow: Incoming Media
  
  {quote}
  Buffering/reordering
  
  RTP may be received in bursts, out of order, or in other less-than-ideal 
  ways. Asterisk will implement reception buffers to place incoming RTP 
  traffic into, potentially reordering packets as necessary if they arrive 
  out of order.
  {quote}
  
  While I'm not against buffering in the RTP stack, have you given any 
  thought how that would be set up? As it adds delay, I would expect that not 
  every RTP stream should be buffered; consequently, this would need to be 
  driven by configuration or by some dialplan construct. Configuration may 
  work in some cases (for example, when you know that some endpoint is always 
  jittery); in other cases, dialplan is probably a better approach. In both 
  cases however, these would require manipulation at a layer higher than the 
  RTP stack itself, which would mean drilling down through the RTP engine 
  into the RTP instance - which ends up sound something like our current 
  jitter buffer approaches. What advantages are there to buffering in the 
  stack itself, versus simply expanding the jitter buffers to handle more 
  than just VOICE frames? Would we want to provide buffering in native RTP 
  bridges, or let the far endpoints handle the re-ordering?
  
  -- Section: Other Stuff
  
  {quote}
  Native local RTP bridges
  
  Native local RTP bridges have a few considerations when implementing a new 
  RTP engine.
  
  First, bridge_native_rtp requires that the RTP engine's local_bridge method 
  has to be the same for each of the bridged RTP instances. If we create a 
  new RTP engine, it will not have the same local_bridge method as 
  res_rtp_asterisk. This means that calls that use res_rtp_asterisk will not 
  be able to be locally bridged with calls that use the new RTP engine. I 
  think it is possible to rework the inner workings of native local bridges 
  such that they can be between different RTP engines. However, if the goal 
  is the total removal of res_rtp_asterisk from the codebase, then such 
  considerations are not as necessary.
  
  Second, native local RTP bridging is performed at the main RTP API layer by 
  having the bridged RTP instances point at each other. It is up to the 
  individual RTP instances to detect that this has occurred and act 
  accordingly. It might work better if the job of setting bridges on RTP 
  instances were passed down to the engines themselves in case they want to 
  perform other side effects besides changing a pointer.
  {quote}
  
  I think it is arguable whether or not the local_bridge code should be in 
  res_rtp_asterisk still. Ideally, an RTP implementation would simply have a 
  direct write/direct read callbacks that the bridge itself would call 
  into, rather than let the RTP implementation do the actual bridging. This 
  has a few advantages:
  (1) We could implement some other more interesting RTP bridges (such as a 
  multi-party RTP forwarding bridge or a RTP bridge w/ RTP recording)
  (2) It simplifies the thread boundaries. Right now, it's a little tricky 
  managing the safety of calling into an RTP implementation from 
  bridge_native_rtp. Having a more concrete boundary between the bridge and 
  the RTP implementations would be advantageous.
  
  Of course, if res_rtp_asterisk is refactored as opposed to replaced, that 
  makes altering and/or supporting the native bridging in any fashion easier.
  
  I think you might be referring to this in your second point, but I'm not 
  entirely sure if this is what you meant in it.

{quote}
Since RTP instances are created separately by a channel driver, how will the 
RTP engine be notified that multiple RTP instances (that is, streams) are 
related? This may be answered in another section, but it might be good to know 
that this will require changes in the RTP engine here.
{quote}

This is addressed in the Media Setup section, subsection The actual Setup . 
{{ast_rtp_instance_new()}} takes an engine-specific void pointer as the final 
parameter. The new RTP engine will take a structure that contains the channel 
that the RTP instance is being created for. This channel is used as a key to 
determine if a session has been set up yet.

{quote}
Stuff 

Re: [asterisk-dev] [Code Review] 4453: Asterisk 14: RTP improvements

2015-03-02 Thread Mark Michelson

On 03/02/2015 10:22 AM, Olle E. Johansson wrote:


On 02 Mar 2015, at 16:54, Matt Jordan reviewbo...@asterisk.org 
mailto:reviewbo...@asterisk.org wrote:



{quote}
Buffering/reordering

RTP may be received in bursts, out of order, or in other less-than-ideal ways. 
Asterisk will implement reception buffers to place incoming RTP traffic into, 
potentially reordering packets as necessary if they arrive out of order.
{quote}

By default, asterisk should forward RTP packets without any buffer, 
without reordering or doing anything. Today Asterisk renumbers packets 
- thus hiding packet loss and reordering. This is bad.


Forwarding with packet loss and reordering is quite ok as default. The 
endpoint in the call will sort it up with a jitter buffer.


In some cases Asterisk is the endpoint of the RTP stream (IVR, 
protocol conversion). In that case we can apply a buffer like any 
endpoint. But not by default.


I guess cases with transcoding involved also requires a jitter buffer.

/O


After reading this response and thinking about it for a while, I think 
you're 100% correct here. In fact, I think the best course of action is 
not to perform buffering or reordering at the RTP layer under any 
circumstances. Instead, the application can be responsible for deciding 
what is appropriate. When I say application here, I don't mean the 
dialplan application Asterisk is running, but a high-level description 
of what is happening, media-wise.


So for instance, if a native local RTP bridge is being used, then RTP 
comes in one side and is directly sent out the other side. We can make 
the assumption that the RTP-capable endpoints will perform their own 
buffering, re-ordering, and synchronizing. We just need to make sure 
that we provide proper timestamps so that a receiver can accurately 
reconstruct the stream.


If other types of bridges are being used, then it is dependent on the 
channel technology and the number of parties in the bridge whether we 
want to buffer/synchronize. And if we do want to perform buffering or 
synchronization of streams, that should be taken care of at the channel 
or bridge level, not at the RTP level.


For IVRs or call recording, then like you mentioned, 
buffering/synchronizing is always a good idea. But again, this doesn't 
need to happen at the RTP layer.


My plan here is to remove the language about buffering and 
synchronization from the RTP layer. Does anyone object to this?
-- 
_
-- 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] 4453: Asterisk 14: RTP improvements

2015-03-02 Thread Matt Jordan

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


Review of https://wiki.asterisk.org/wiki/display/AST/Notes+on+RTP+standards

-- Section SRTP (RFC 3711)

{quote}
This document creates a new RTP profile, called RTP/SAVP. From section section 
3:
{quote}

section repeated twice. The formatting of section 3 is also a little wonky.

-- Section RTP/AVPF (RFC 4585)

{quote}
In section 3.2, it outlines the rule changes from RFC 3550 with regards to RTCP 
transmission intervals. The minimum 5 second interval is not enforced and 
instead the bandwidth of the connection is used to determine the interval 
(though a minimum may still be optionally selected). Sending packets early is 
referred to, appropriately, as early RTCP. Early RTCP follows its own rules 
about what types of RTCP packets can make up the compound RTCP packet. 
Honestly, it's a bit of a mess. The rules are:

I decide I need to send a feedback packet, and I want to send it NOW, not 
at the next interval
I wait a brief period to make sure no one else that noticed the event has 
already sent their own feedback packet.
I have to check if early feedback is permitted.
I can send an an early feedback packet if all the above are fulfilled.

So in order to send RTCP feedback, I modify timing with regards to sending 
RTCP, I can violate those timers and send early feedback, I have to insert 
small intervals in timing to allow for other feedback senders to send their 
feedback first, I have to determine permissiveness of sending early feedback.

Or I can just add feedback to my compound RTCP packets I'm already sending. I 
think I know how I'd implement this if given the choice...
{quote}

I think it's fine to just make a recommendation that we add feedback to the 
compound RTCP packets. That being said, the scheduling algorithms for this and 
other feedback mechanisms (such as NACK) are relatively difficult and may end 
up conflicting with each other. Have you thought about how scheduling different 
types of RTCP reports should be handled?

- Matt Jordan


On Feb. 27, 2015, 12:47 p.m., Mark Michelson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4453/
 ---
 
 (Updated Feb. 27, 2015, 12:47 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Description
 ---
 
 I've created a series of wiki pages that discuss the idea of writing an 
 improved RTP architecture in Asterisk 14.
 
 To regurgitate some details from the linked page, the current RTP engine in 
 Asterisk (res_rtp_asterisk) gets the job done but has some issues. It is not 
 architected in a way that allows for easy insertion of new features. It has 
 dead code (or code that might as well be dead). And it has some general flaws 
 in it with regards to following rules defined by fundamental RFCs.
 
 I have approached these wiki pages with the idea of writing a replacement for 
 res_rtp_asterisk.c. The reason for this is that there are interesting 
 media-related IETF drafts (trickle ICE and BUNDLE, to name two) that would be 
 difficult to implement in the current res_rtp_asterisk.c code correctly. 
 Taking the opportunity to re-engineer the underlying architecture into 
 something more layered and extendable would help in this regard. The goal 
 also is to not disturb the high-level RTP engine API wherever possible, 
 meaning that channel drivers will not be touched at all by this set of 
 changes.
 
 The main page where this is discussed is here: 
 https://wiki.asterisk.org/wiki/display/AST/RTP+engine+replacement . This page 
 has a subpage that has my informal rambling notes regarding a sampling of RTP 
 and media-related RFCs and drafts I read. It also has a subpage with more 
 informal and rambling notes about the current state of RTP in Asterisk. While 
 these pages are not really part of the review, you may want to read them 
 anyway just so you might have some idea of where I'm coming from when drawing 
 up the ideas behind a new architecture.
 
 I also have a task list page that details a list of high-level tasks that 
 would need to be performed if a new RTP engine were to be written: 
 https://wiki.asterisk.org/wiki/display/AST/RTP+task+list . This should give 
 some idea of the amount of work required to make a new RTP engine a reality. 
 The tasks with (?) around them are tasks that add new features to Asterisk's 
 RTP support, and it is therefore questionable whether they fit in the scope 
 of this work at this time.
 
 Some things to consider when reading through this:
 * Refactor or rewrite? When considering current issues with RTP/RTCP in 
 Asterisk, and considering the types of features that are coming 

Re: [asterisk-dev] [Code Review] 4453: Asterisk 14: RTP improvements

2015-03-02 Thread Matt Jordan

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


Review: https://wiki.asterisk.org/wiki/display/AST/RTP+task+list

I think it is worth putting somewhere on the task list the notion of testing 
RTP independent of a channel driver. That is, I'd like to be able to pass RTP 
packets into Asterisk, have them echo'd back to me (or otherwise transformed 
and sent back to something that can receive RTP), and not have to go through 
the SIP/PJSIP/Motif/etc. layers.

This could be a simple extension to chan_rtp, or it could be signalling set up 
through ARI. The goal should be to let the Asterisk Test Suite inject RTP into 
Asterisk and verify that things behave appropriately.

It may be worthwhile writing a tool that does this completely as a separate 
project outside of the Asterisk Test Suite (ala SIPp - RTPp?) to make it easier 
for others to contribute to. Either way, I think being able to write a lot of 
tests that exercise the new RTP implementation will be key to getting this work 
done.

-- Section: Bare-Bones calls

{quote}
The first milestone will be to get the RTP engine written to the point that we 
can successfully pass media through Asterisk. This means successfully 
constructing the media flows as described in the parent page and putting some 
basic setup in place. For this phase, it is encouraged to use hard-coding in 
place of pluggable elements. The sooner we can get to a point where we have 
successful calls, the easier it is to rapidly develop new features (and tests!) 
with confidence that they work as intended.
{quote}

I think we're assuming here that we are writing something from scratch. After 
reading through your state of the world and the state of current RFCs, how do 
you feel about refactoring res_rtp_asterisk?

If we went with that approach, the first task may be:
(1) Write tests that exercise the current implementation with basic capabilities
(2) Refactor res_rtp_asterisk into different units (modules and/or separate 
files)
(3) Prove out the validity of the threading model, using the tests to exercise 
both functionality as well as the performance impact

I'm not sure on the correct way to go here - and I'd expect that most of the 
RTP/RTCP code to get heavily re-worked/re-written - but given that 
res_rtp_asterisk is substantially smaller than our other legacy modules, it 
may be possible to simply refactor it as opposed to re-writing the whole thing 
from scratch.

-- Section: Destruction

{quote}
Once a call is hung up, we need to be able to clean up the RTP
{quote}

This is missing the end of its phrase :-)



- Matt Jordan


On Feb. 27, 2015, 12:47 p.m., Mark Michelson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4453/
 ---
 
 (Updated Feb. 27, 2015, 12:47 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Description
 ---
 
 I've created a series of wiki pages that discuss the idea of writing an 
 improved RTP architecture in Asterisk 14.
 
 To regurgitate some details from the linked page, the current RTP engine in 
 Asterisk (res_rtp_asterisk) gets the job done but has some issues. It is not 
 architected in a way that allows for easy insertion of new features. It has 
 dead code (or code that might as well be dead). And it has some general flaws 
 in it with regards to following rules defined by fundamental RFCs.
 
 I have approached these wiki pages with the idea of writing a replacement for 
 res_rtp_asterisk.c. The reason for this is that there are interesting 
 media-related IETF drafts (trickle ICE and BUNDLE, to name two) that would be 
 difficult to implement in the current res_rtp_asterisk.c code correctly. 
 Taking the opportunity to re-engineer the underlying architecture into 
 something more layered and extendable would help in this regard. The goal 
 also is to not disturb the high-level RTP engine API wherever possible, 
 meaning that channel drivers will not be touched at all by this set of 
 changes.
 
 The main page where this is discussed is here: 
 https://wiki.asterisk.org/wiki/display/AST/RTP+engine+replacement . This page 
 has a subpage that has my informal rambling notes regarding a sampling of RTP 
 and media-related RFCs and drafts I read. It also has a subpage with more 
 informal and rambling notes about the current state of RTP in Asterisk. While 
 these pages are not really part of the review, you may want to read them 
 anyway just so you might have some idea of where I'm coming from when drawing 
 up the ideas behind a new architecture.
 
 I also have a task list page that details a list of high-level tasks that 
 would need to be performed if a new RTP engine were to be 

[asterisk-dev] [Code Review] 4456: testsuite: res_pjsip - conflicting endpoint identifiers test

2015-03-02 Thread Kevin Harwell

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

Review request for Asterisk Developers.


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


Repository: testsuite


Description
---

It is possible that two or more endpoint identifiers could match against an 
incoming call. This test makes sure that no matter what order the endpoint 
identifier modules were loaded priority is given based on the ones specified in 
the global identify_by_priority option. See more details see 
https://reviewboard.asterisk.org/r/4455/


Diffs
-

  /asterisk/trunk/tests/channels/pjsip/tests.yaml 6482 
  /asterisk/trunk/tests/channels/pjsip/endpoint_identify/test-config.yaml 
PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/endpoint_identify/configs/ast2/pjsip.conf 
PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/endpoint_identify/configs/ast2/extensions.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/endpoint_identify/configs/ast1/pjsip.conf 
PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/endpoint_identify/configs/ast1/extensions.conf
 PRE-CREATION 

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


Testing
---

Ran the test without the code fix in and it would fail. Patched the code and it 
passed. Also modified the identify order and re-ran the test and it would fail 
as expected.


Thanks,

Kevin Harwell

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

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

[asterisk-dev] [Code Review] 4455: res_pjsip: conflicting endpoint identifiers

2015-03-02 Thread Kevin Harwell

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

Review request for Asterisk Developers.


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


Repository: Asterisk


Description
---

It's possible to have a scenario that will create a conflict between endpoint 
identifiers. For instance an incoming call could be identified by two different 
endpoint identifiers and the one chosen depended upon which identifier module 
loaded first. This of course causes problems when, for example, the incoming 
call is expected to be identified by username, but instead is identified by ip. 
This patch adds a new 'global' option to res_pjsip called 
'identify_by_priority'. It is a comma separated list of endpoint identifier 
names that specifies the order by which identifiers are processed and checked.


Diffs
-

  branches/13/res/res_pjsip_endpoint_identifier_user.c 432422 
  branches/13/res/res_pjsip_endpoint_identifier_ip.c 432422 
  branches/13/res/res_pjsip_endpoint_identifier_anonymous.c 432422 
  branches/13/res/res_pjsip/config_global.c 432422 
  branches/13/res/res_pjsip.c 432422 
  branches/13/include/asterisk/res_pjsip.h 432422 
  
branches/13/contrib/ast-db-manage/config/versions/45e3f47c6c44_add_pjsip_identify_by_priority.py
 PRE-CREATION 
  branches/13/configs/samples/pjsip.conf.sample 432422 
  branches/13/CHANGES 432422 

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


Testing
---

Added a testsuite test: https://reviewboard.asterisk.org/r/4456/


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