<p>Joshua Colp <strong>posted comments</strong> on this change.</p><p><a href="https://gerrit.asterisk.org/8773">View Change</a></p><p>Patch set 2:</p><p>(2 comments)</p><ul style="list-style: none; padding-left: 20px;"><li><p><a href="https://gerrit.asterisk.org/#/c/8773/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/8773/2/res/res_rtp_asterisk.c@5267">Patch Set #2, Line 5267:</a> <code style="font-family:monospace,monospace">      for (j = 3; j < length; j++) {</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Document why magic number 3 is used.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">On further investigation I think the FCI information starts at 1.</p><p style="white-space: pre-wrap; word-wrap: break-word;">The RTCP code automatically skips past the RTCP and packet source SSRC, so 0 would be the media source SSRC when we get here and therefore 1 would be the first FCI.</p></li><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/8773/2/res/res_rtp_asterisk.c@5269">Patch Set #2, Line 5269:</a> <code style="font-family:monospace,monospace">               pid = current_word >> 16;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I did some testing with this and I don't think the logic for getting the pid is correct:</p><p style="white-space: pre-wrap; word-wrap: break-word;">[Apr 16 11:23:54] NOTICE[19415][C-00000001]: res_rtp_asterisk.c:5270 ast_rtp_rtcp_handle_nack: Looking for seqno 7936, latest sent 20865<br>[Apr 16 11:23:54] NOTICE[19415][C-00000001]: res_rtp_asterisk.c:5276 ast_rtp_rtcp_handle_nack: Could not find 7936 to retransmit<br>[Apr 16 11:23:54] NOTICE[19415][C-00000001]: res_rtp_asterisk.c:5270 ast_rtp_rtcp_handle_nack: Looking for seqno 5888, latest sent 18042<br>[Apr 16 11:23:54] NOTICE[19415][C-00000001]: res_rtp_asterisk.c:5276 ast_rtp_rtcp_handle_nack: Could not find 5888 to retransmit<br>[Apr 16 11:24:03] NOTICE[19415][C-00000001]: res_rtp_asterisk.c:5270 ast_rtp_rtcp_handle_nack: Looking for seqno 7936, latest sent 18647<br>[Apr 16 11:24:03] NOTICE[19415][C-00000001]: res_rtp_asterisk.c:5276 ast_rtp_rtcp_handle_nack: Could not find 7936 to retransmit<br>[Apr 16 11:24:03] NOTICE[19425][C-00000003]: res_rtp_asterisk.c:5270 ast_rtp_rtcp_handle_nack: Looking for seqno 5376, latest sent 24711<br>[Apr 16 11:24:03] NOTICE[19425][C-00000003]: res_rtp_asterisk.c:5276 ast_rtp_rtcp_handle_nack: Could not find 5376 to retransmit<br>[Apr 16 11:24:12] NOTICE[19425][C-00000003]: res_rtp_asterisk.c:5270 ast_rtp_rtcp_handle_nack: Looking for seqno 4864, latest sent 26261<br>[Apr 16 11:24:12] NOTICE[19423][C-00000002]: res_rtp_asterisk.c:5270 ast_rtp_rtcp_handle_nack: Looking for seqno 4864, latest sent 15963<br>[Apr 16 11:24:12] NOTICE[19423][C-00000002]: res_rtp_asterisk.c:5276 ast_rtp_rtcp_handle_nack: Could not find 4864 to retransmit<br>[Apr 16 11:24:12] NOTICE[19425][C-00000003]: res_rtp_asterisk.c:5276 ast_rtp_rtcp_handle_nack: Could not find 4864 to retransmit</p><p style="white-space: pre-wrap; word-wrap: break-word;">The value it got for 'pid' was also 0 sometimes which is not what I'd expect.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/8773">change 8773</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/8773"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 15 </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: I7f7f124af3b9d5d2fd9cffc6ba8cb48a6fff06ec </div>
<div style="display:none"> Gerrit-Change-Number: 8773 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Benjamin Keith Ford <bford@digium.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-Comment-Date: Mon, 16 Apr 2018 11:42:29 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>