<p>Joshua Colp <strong>posted comments</strong> on this change.</p><p><a href="https://gerrit.asterisk.org/9224">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>(5 comments)</p><ul style="list-style: none; padding-left: 20px;"><li><p><a href="https://gerrit.asterisk.org/#/c/9224/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/9224/2/res/res_rtp_asterisk.c@3626">Patch Set #2, Line 3626:</a> <code style="font-family:monospace,monospace"> rtp->expectedrxseqno = -1;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">If we're told that the source has changed (ast_rtp_change_source) we should probably reset this to -1 as well since they likely won't follow the same seqnos.</p></li><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/9224/2/res/res_rtp_asterisk.c@6749">Patch Set #2, Line 6749:</a> <code style="font-family:monospace,monospace"> /* Define our starting point, or recover from a large gap of lost packets */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">If that happened for the large gap case is it possible that there would be payloads still in the data buffer?</p></li><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/9224/2/res/res_rtp_asterisk.c@6777">Patch Set #2, Line 6777:</a> <code style="font-family:monospace,monospace"> ast_data_buffer_put(rtp->recv_buffer, seqno, f);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">What happens if we can't dupe the frame here? Will things pile up until we hit the max data buffer size and then it'll all get flushed? (Since this is a packet we would never get again)</p></li><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/9224/2/res/res_rtp_asterisk.c@6785">Patch Set #2, Line 6785:</a> <code style="font-family:monospace,monospace"> /* We don't want missing sequence number duplicates */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Can you extend the comment to explain the scenario where this would happen?</p></li><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/9224/2/res/res_rtp_asterisk.c@6844">Patch Set #2, Line 6844:</a> <code style="font-family:monospace,monospace"> /* Things are a little out of hand. Salvage what we can and start fresh */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">If this is carrying video (as it always will right now) it may be wise to preemptively call rtp_write_rtcp_fir - this will tell the remote side to send a full frame, allowing video to be restored faster in the case of this messy situation.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/9224">change 9224</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/9224"/><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: Idab644b08a1593659c92cda64132ccc203fe991d </div>
<div style="display:none"> Gerrit-Change-Number: 9224 </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: 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-Reviewer: Sean Bright <sean.bright@gmail.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Thu, 21 Jun 2018 10:09:31 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>