<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"><head><meta http-equiv=Content-Type content="text/html; charset=utf-8"><meta name=Generator content="Microsoft Word 12 (filtered medium)"><style><!--
/* Font Definitions */
@font-face
        {font-family:Wingdings;
        panose-1:5 0 0 0 0 0 0 0 0 0;}
@font-face
        {font-family:"Cambria Math";
        panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
        {font-family:Calibri;
        panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
        {font-family:Tahoma;
        panose-1:2 11 6 4 3 5 4 4 2 4;}
@font-face
        {font-family:Consolas;
        panose-1:2 11 6 9 2 2 4 3 2 4;}
@font-face
        {font-family:Verdana;
        panose-1:2 11 6 4 3 5 4 4 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
        {margin:0in;
        margin-bottom:.0001pt;
        font-size:12.0pt;
        font-family:"Times New Roman","serif";}
h1
        {mso-style-priority:9;
        mso-style-link:"Heading 1 Char";
        mso-margin-top-alt:auto;
        margin-right:0in;
        mso-margin-bottom-alt:auto;
        margin-left:0in;
        font-size:24.0pt;
        font-family:"Times New Roman","serif";
        font-weight:bold;}
a:link, span.MsoHyperlink
        {mso-style-priority:99;
        color:blue;
        text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
        {mso-style-priority:99;
        color:purple;
        text-decoration:underline;}
p
        {mso-style-priority:99;
        mso-margin-top-alt:auto;
        margin-right:0in;
        mso-margin-bottom-alt:auto;
        margin-left:0in;
        font-size:12.0pt;
        font-family:"Times New Roman","serif";}
pre
        {mso-style-priority:99;
        mso-style-link:"HTML Preformatted Char";
        margin:0in;
        margin-bottom:.0001pt;
        font-size:10.0pt;
        font-family:"Courier New";}
span.Heading1Char
        {mso-style-name:"Heading 1 Char";
        mso-style-priority:9;
        mso-style-link:"Heading 1";
        font-family:"Cambria","serif";
        color:#365F91;
        font-weight:bold;}
span.HTMLPreformattedChar
        {mso-style-name:"HTML Preformatted Char";
        mso-style-priority:99;
        mso-style-link:"HTML Preformatted";
        font-family:Consolas;}
span.EmailStyle21
        {mso-style-type:personal-reply;
        font-family:"Calibri","sans-serif";
        color:#1F497D;}
.MsoChpDefault
        {mso-style-type:export-only;
        font-size:10.0pt;}
@page WordSection1
        {size:8.5in 11.0in;
        margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
        {page:WordSection1;}
/* List Definitions */
@list l0
        {mso-list-id:1975066210;
        mso-list-template-ids:-1080887894;}
@list l0:level1
        {mso-level-number-format:bullet;
        mso-level-text:;
        mso-level-tab-stop:.5in;
        mso-level-number-position:left;
        text-indent:-.25in;
        mso-ansi-font-size:10.0pt;
        font-family:Symbol;}
ol
        {margin-bottom:0in;}
ul
        {margin-bottom:0in;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]--></head><body lang=EN-US link=blue vlink=purple><div class=WordSection1><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>I’d say it is a bug.  When packetization support was added it did <o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>prevent native bridging if the call legs had different requirements,<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>and the code looked much like what you have in the review.<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p>&nbsp;</o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Dan<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p>&nbsp;</o:p></span></p><div><div style='border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in'><p class=MsoNormal style='margin-left:.5in'><b><span style='font-size:10.0pt;font-family:"Tahoma","sans-serif"'>From:</span></b><span style='font-size:10.0pt;font-family:"Tahoma","sans-serif"'> asterisk-dev-bounces@lists.digium.com [mailto:asterisk-dev-bounces@lists.digium.com] <b>On Behalf Of </b>Mark Michelson<br><b>Sent:</b> Wednesday, February 06, 2013 11:53 AM<br><b>To:</b> Mark Michelson; Asterisk Developers; Mark Michelson<br><b>Subject:</b> [asterisk-dev] [Code Review] Disallow native RTP bridging when packetization differs between streams<o:p></o:p></span></p></div></div><p class=MsoNormal style='margin-left:.5in'><o:p>&nbsp;</o:p></p><div><table class=MsoNormalTable border=1 cellpadding=0 width="100%" style='width:100.0%;margin-left:.5in;background:#F9F3C9;border:solid #C9C399 1.0pt'><tr><td style='border:none;padding:6.0pt 6.0pt 6.0pt 6.0pt'><p class=MsoNormal>This is an automatically generated e-mail. To reply, visit: <a href="https://reviewboard.asterisk.org/r/2319/">https://reviewboard.asterisk.org/r/2319/</a> <o:p></o:p></p></td></tr></table><p class=MsoNormal style='margin-left:.5in'><span style='font-family:"Verdana","sans-serif"'><o:p>&nbsp;</o:p></span></p><table class=MsoNormalTable border=1 cellspacing=0 cellpadding=0 width="100%" style='width:100.0%;margin-left:.5in;background:#FEFADF;border:solid black 1.0pt;background-position-x:0%;background-position-y:0%'><tr><td style='border:none;padding:6.0pt 6.0pt 6.0pt 6.0pt'><div><p class=MsoNormal>Review request for Asterisk Developers.<o:p></o:p></p></div><div><p class=MsoNormal>By Mark Michelson.<o:p></o:p></p></div><h1 style='margin-top:.25in'><span style='font-size:10.0pt;color:#575012'>Description <o:p></o:p></span></h1><table class=MsoNormalTable border=1 cellspacing=0 cellpadding=0 width="100%" style='width:100.0%;background:white;border:solid #B8B5A0 1.0pt'><tr><td style='border:none;padding:7.5pt 7.5pt 7.5pt 7.5pt'><pre style='white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap: break-word'>In the linked issue, it is brought up that faxing was broken in Asterisk 1.8 due to the fact that Asterisk was not honoring the packetization specified in an SDP. As it turns out, the reason why things went awry was that a local RTP bridge was being used, thus bypassing any smoothers created on the RTP streams.<o:p></o:p></pre><pre><o:p>&nbsp;</o:p></pre><pre>In order to fix this, I propose this fix. With this, there can be no native RTP bridging (local or remote) if the packetization of the streams is not compatible. If packetization is not the same on both sides of the call, then the bridging must go through the core.<o:p></o:p></pre><pre><o:p>&nbsp;</o:p></pre><pre>I'm looking for three pieces of feedback here<o:p></o:p></pre><pre><o:p>&nbsp;</o:p></pre><pre>1) Is this approach correct? Should we prevent native RTP bridging if packetization of the streams differs?<o:p></o:p></pre><pre>2) Is the code implemented correctly?<o:p></o:p></pre><pre>3) Should this change be thrown into 1.8 as is? I think this is a bug myself, so I was planning to put this into 1.8. However, I would be interested in knowing if this sort of behavior change could cause unintended consequences.<o:p></o:p></pre></td></tr></table><h1 style='margin-top:.25in'><span style='font-size:10.0pt;color:#575012'>Testing <o:p></o:p></span></h1><table class=MsoNormalTable border=1 cellspacing=0 cellpadding=0 width="100%" style='width:100.0%;background:white;border:solid #B8B5A0 1.0pt'><tr><td style='border:none;padding:7.5pt 7.5pt 7.5pt 7.5pt'><pre style='white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap: break-word'>I tested this by placing a call from SIPp through Asterisk to a Polycom phone on my desk. The SIPp scenario specifies a ptime of 30. The leg between Asterisk and the phone specifies a ptime of 20. Using wireshark, I can see that before this patch is applied, the different packetizations allow for a local RTP bridge to be used. A packet capture shows that audio from Asterisk to SIPp comes in 20 ms segments. With the patch applied, local RTP bridging is prevented. A packet capture shows that audio from Asterisk to SIPp comes in 30 ms segments.<o:p></o:p></pre></td></tr></table><div style='margin-top:.25in'><p class=MsoNormal><b><span style='font-size:10.0pt;color:#575012'>Bugs: </span></b><a href="https://issues.asterisk.org/jira/browse/ASTERISK-20650">ASTERISK-20650</a> <o:p></o:p></p></div><h1 style='margin-top:.25in'><span style='font-size:10.0pt;color:#575012'>Diffs <o:p></o:p></span></h1><ul type=disc><li class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;mso-list:l0 level1 lfo1'>/branches/1.8/main/rtp_engine.c (380922)<o:p></o:p></li></ul><p><a href="https://reviewboard.asterisk.org/r/2319/diff/">View Diff</a><o:p></o:p></p></td></tr></table><p class=MsoNormal style='margin-left:.5in'><o:p>&nbsp;</o:p></p></div></div></body></html>