<p>Richard Mudgett <strong>posted comments</strong> on this change.</p><p><a href="https://gerrit.asterisk.org/8948">View Change</a></p><p>Patch set 2:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4;">Code-Review -1</span></p><p>(1 comment)</p><ul style="list-style: none; padding-left: 20px;"><li><p><a href="https://gerrit.asterisk.org/#/c/8948/2/res/res_rtp_asterisk.c">File res/res_rtp_asterisk.c:</a></p><ul style="list-style: none; padding-left: 20px;"><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/8948/2/res/res_rtp_asterisk.c@5244">Patch Set #2, Line 5244:</a> <code style="font-family:monospace,monospace">                bridged->ssrc = ssrc;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Changing the ssrc is going to mess up the RTCP sender report.  In addition on v15 it is likely to mess up the bundled ssrc mapping, the SSRC reported in SDP for bundling, and DTLS.</p><p style="white-space: pre-wrap; word-wrap: break-word;">I think to get a similar effect we need a new flag value (FLAG_REQ_LOCAL_BRIDGE_BIT) to also be set by ast_rtp_local_bridge().  We treat this flag similar to the FLAG_NEED_MARKER_BIT only in bridge_p2p_rtp_write().</p><p style="white-space: pre-wrap; word-wrap: break-word;">#define FLAG_NEED_MARKER_BIT            (1 << 3)<br>...<br>#define FLAG_REQ_LOCAL_BRIDGE_BIT            (1 << 5)<br>...</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">bridge_p2p_rtp_write()<br>{<br>...<br>   /* If the marker bit has been explicitly set turn it on */<br>    if (ast_test_flag(bridged, FLAG_NEED_MARKER_BIT)) {<br>           mark = 1;<br>             ast_clear_flag(bridged, FLAG_NEED_MARKER_BIT);<br>        }<br>     /* Set the marker bit for the first local bridged packet which has the first bridged peer's SSRC. */<br>      if (ast_test_flag(bridged, FLAG_REQ_LOCAL_BRIDGE_BIT)) {<br>              mark = 1;<br>             ast_clear_flag(bridged, FLAG_REQ_LOCAL_BRIDGE_BIT);<br>   }<br>...<br>}</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><br>ast_rtp_local_bridge()<br>{<br>...<br>   ao2_lock(instance0);<br>  ast_set_flag(rtp, FLAG_NEED_MARKER_BIT | FLAG_REQ_LOCAL_BRIDGE_BIT);<br>  ao2_unlock(instance0);</pre><p style="white-space: pre-wrap; word-wrap: break-word;">}</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/8948">change 8948</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/8948"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 13 </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: I01858bd0235f1e5e629e20de71b422b16f55759b </div>
<div style="display:none"> Gerrit-Change-Number: 8948 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Torrey Searle <tsearle@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Matthew Fredrickson <creslin@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Fri, 18 May 2018 18:08:11 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>