<p>Joshua Colp <strong>submitted</strong> this change.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/15641">View Change</a></p><div style="white-space:pre-wrap">Approvals:
George Joseph: Looks good to me, but someone else must approve
Benjamin Keith Ford: Looks good to me, but someone else must approve
Kevin Harwell: Looks good to me, approved
Joshua Colp: Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res_rtp_asterisk: Force resync on SSRC change.<br><br>When an SSRC change occurs the timestamps are likely<br>to change as well. As a result we need to reset the<br>timestamp mapping done in the calc_rxstamp function<br>so that they map properly from timestamp to real<br>time.<br><br>This previously occurred but due to packet<br>retransmission support the explicit setting<br>of the marker bit was not effective.<br><br>ASTERISK-29352<br><br>Change-Id: I2d4c8f93ea24abc1030196706de2d70facf05a5a<br>---<br>M res/res_rtp_asterisk.c<br>1 file changed, 51 insertions(+), 47 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c</span><br><span>index 21eff54..b0a3f2a 100644</span><br><span>--- a/res/res_rtp_asterisk.c</span><br><span>+++ b/res/res_rtp_asterisk.c</span><br><span>@@ -7203,12 +7203,13 @@</span><br><span> }</span><br><span> </span><br><span> static struct ast_frame *ast_rtp_interpret(struct ast_rtp_instance *instance, struct ast_srtp *srtp,</span><br><span style="color: hsl(0, 100%, 40%);">- const struct ast_sockaddr *remote_address, unsigned char *read_area, int length, int prev_seqno)</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct ast_sockaddr *remote_address, unsigned char *read_area, int length, int prev_seqno,</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int bundled)</span><br><span> {</span><br><span> unsigned int *rtpheader = (unsigned int*)(read_area);</span><br><span> struct ast_rtp *rtp = ast_rtp_instance_get_data(instance);</span><br><span> struct ast_rtp_instance *instance1;</span><br><span style="color: hsl(0, 100%, 40%);">- int res = length, hdrlen = 12, seqno, payloadtype, padding, mark, ext, cc;</span><br><span style="color: hsl(120, 100%, 40%);">+ int res = length, hdrlen = 12, ssrc, seqno, payloadtype, padding, mark, ext, cc;</span><br><span> unsigned int timestamp;</span><br><span> RAII_VAR(struct ast_rtp_payload_type *, payload, NULL, ao2_cleanup);</span><br><span> struct frame_list frames;</span><br><span>@@ -7225,6 +7226,7 @@</span><br><span> }</span><br><span> </span><br><span> /* Pull out the various other fields we will need */</span><br><span style="color: hsl(120, 100%, 40%);">+ ssrc = ntohl(rtpheader[2]);</span><br><span> seqno = ntohl(rtpheader[0]);</span><br><span> payloadtype = (seqno & 0x7f0000) >> 16;</span><br><span> padding = seqno & (1 << 29);</span><br><span>@@ -7274,6 +7276,42 @@</span><br><span> return AST_LIST_FIRST(&frames) ? AST_LIST_FIRST(&frames) : &ast_null_frame;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ /* Only non-bundled instances can change/learn the remote's SSRC implicitly. */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!bundled) {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Force a marker bit and change SSRC if the SSRC changes */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rtp->themssrc_valid && rtp->themssrc != ssrc) {</span><br><span style="color: hsl(120, 100%, 40%);">+ struct ast_frame *f, srcupdate = {</span><br><span style="color: hsl(120, 100%, 40%);">+ AST_FRAME_CONTROL,</span><br><span style="color: hsl(120, 100%, 40%);">+ .subclass.integer = AST_CONTROL_SRCCHANGE,</span><br><span style="color: hsl(120, 100%, 40%);">+ };</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!mark) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ast_debug_rtp_packet_is_allowed) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_debug(0, "(%p) RTP forcing Marker bit, because SSRC has changed\n", instance);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ mark = 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ f = ast_frisolate(&srcupdate);</span><br><span style="color: hsl(120, 100%, 40%);">+ AST_LIST_INSERT_TAIL(&frames, f, frame_list);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ rtp->seedrxseqno = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ rtp->rxcount = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ rtp->rxoctetcount = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ rtp->cycles = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ prev_seqno = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ rtp->last_seqno = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ rtp->last_end_timestamp = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rtp->rtcp) {</span><br><span style="color: hsl(120, 100%, 40%);">+ rtp->rtcp->expected_prior = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ rtp->rtcp->received_prior = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ rtp->themssrc = ssrc; /* Record their SSRC to put in future RR */</span><br><span style="color: hsl(120, 100%, 40%);">+ rtp->themssrc_valid = 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> rtp->rxcount++;</span><br><span> rtp->rxoctetcount += (res - hdrlen);</span><br><span> if (rtp->rxcount == 1) {</span><br><span>@@ -7498,13 +7536,14 @@</span><br><span> struct ast_srtp *srtp;</span><br><span> RAII_VAR(struct ast_rtp_instance *, child, NULL, rtp_instance_unlock);</span><br><span> struct ast_sockaddr addr;</span><br><span style="color: hsl(0, 100%, 40%);">- int res, hdrlen = 12, version, payloadtype, mark;</span><br><span style="color: hsl(120, 100%, 40%);">+ int res, hdrlen = 12, version, payloadtype;</span><br><span> unsigned char *read_area = rtp->rawdata + AST_FRIENDLY_OFFSET;</span><br><span> size_t read_area_size = sizeof(rtp->rawdata) - AST_FRIENDLY_OFFSET;</span><br><span> unsigned int *rtpheader = (unsigned int*)(read_area), seqno, ssrc, timestamp, prev_seqno;</span><br><span> struct ast_sockaddr remote_address = { {0,} };</span><br><span> struct frame_list frames;</span><br><span> struct ast_frame *frame;</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int bundled;</span><br><span> </span><br><span> /* If this is actually RTCP let's hop on over and handle it */</span><br><span> if (rtcp) {</span><br><span>@@ -7781,7 +7820,6 @@</span><br><span> </span><br><span> /* Pull out the various other fields we will need */</span><br><span> payloadtype = (seqno & 0x7f0000) >> 16;</span><br><span style="color: hsl(0, 100%, 40%);">- mark = seqno & (1 << 23);</span><br><span> seqno &= 0xffff;</span><br><span> timestamp = ntohl(rtpheader[1]);</span><br><span> </span><br><span>@@ -7793,48 +7831,14 @@</span><br><span> </span><br><span> AST_LIST_HEAD_INIT_NOLOCK(&frames);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* Only non-bundled instances can change/learn the remote's SSRC implicitly. */</span><br><span style="color: hsl(0, 100%, 40%);">- if (!child && !AST_VECTOR_SIZE(&rtp->ssrc_mapping)) {</span><br><span style="color: hsl(0, 100%, 40%);">- /* Force a marker bit and change SSRC if the SSRC changes */</span><br><span style="color: hsl(0, 100%, 40%);">- if (rtp->themssrc_valid && rtp->themssrc != ssrc) {</span><br><span style="color: hsl(0, 100%, 40%);">- struct ast_frame *f, srcupdate = {</span><br><span style="color: hsl(0, 100%, 40%);">- AST_FRAME_CONTROL,</span><br><span style="color: hsl(0, 100%, 40%);">- .subclass.integer = AST_CONTROL_SRCCHANGE,</span><br><span style="color: hsl(0, 100%, 40%);">- };</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (!mark) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (ast_debug_rtp_packet_is_allowed) {</span><br><span style="color: hsl(0, 100%, 40%);">- ast_debug(0, "(%p) RTP forcing Marker bit, because SSRC has changed\n", instance);</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- mark = 1;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- f = ast_frisolate(&srcupdate);</span><br><span style="color: hsl(0, 100%, 40%);">- AST_LIST_INSERT_TAIL(&frames, f, frame_list);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- rtp->seedrxseqno = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- rtp->rxcount = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- rtp->rxoctetcount = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- rtp->cycles = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- rtp->lastrxseqno = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- rtp->last_seqno = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- rtp->last_end_timestamp = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- if (rtp->rtcp) {</span><br><span style="color: hsl(0, 100%, 40%);">- rtp->rtcp->expected_prior = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- rtp->rtcp->received_prior = 0;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- rtp->themssrc = ssrc; /* Record their SSRC to put in future RR */</span><br><span style="color: hsl(0, 100%, 40%);">- rtp->themssrc_valid = 1;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(120, 100%, 40%);">+ bundled = (child || AST_VECTOR_SIZE(&rtp->ssrc_mapping)) ? 1 : 0;</span><br><span> </span><br><span> prev_seqno = rtp->lastrxseqno;</span><br><span> rtp->lastrxseqno = seqno;</span><br><span> </span><br><span> if (!rtp->recv_buffer) {</span><br><span> /* If there is no receive buffer then we can pass back the frame directly */</span><br><span style="color: hsl(0, 100%, 40%);">- frame = ast_rtp_interpret(instance, srtp, &addr, read_area, res, prev_seqno);</span><br><span style="color: hsl(120, 100%, 40%);">+ frame = ast_rtp_interpret(instance, srtp, &addr, read_area, res, prev_seqno, bundled);</span><br><span> AST_LIST_INSERT_TAIL(&frames, frame, frame_list);</span><br><span> return AST_LIST_FIRST(&frames);</span><br><span> } else if (rtp->expectedrxseqno == -1 || seqno == rtp->expectedrxseqno) {</span><br><span>@@ -7849,7 +7853,7 @@</span><br><span> * return it directly without duplicating it.</span><br><span> */</span><br><span> if (!ast_data_buffer_count(rtp->recv_buffer)) {</span><br><span style="color: hsl(0, 100%, 40%);">- frame = ast_rtp_interpret(instance, srtp, &addr, read_area, res, prev_seqno);</span><br><span style="color: hsl(120, 100%, 40%);">+ frame = ast_rtp_interpret(instance, srtp, &addr, read_area, res, prev_seqno, bundled);</span><br><span> AST_LIST_INSERT_TAIL(&frames, frame, frame_list);</span><br><span> return AST_LIST_FIRST(&frames);</span><br><span> }</span><br><span>@@ -7864,7 +7868,7 @@</span><br><span> * chance it will be overwritten.</span><br><span> */</span><br><span> if (!ast_data_buffer_get(rtp->recv_buffer, rtp->expectedrxseqno)) {</span><br><span style="color: hsl(0, 100%, 40%);">- frame = ast_rtp_interpret(instance, srtp, &addr, read_area, res, prev_seqno);</span><br><span style="color: hsl(120, 100%, 40%);">+ frame = ast_rtp_interpret(instance, srtp, &addr, read_area, res, prev_seqno, bundled);</span><br><span> AST_LIST_INSERT_TAIL(&frames, frame, frame_list);</span><br><span> return AST_LIST_FIRST(&frames);</span><br><span> }</span><br><span>@@ -7874,7 +7878,7 @@</span><br><span> * to the head of the frames list and avoid having to duplicate it but this would result in out</span><br><span> * of order packet processing by libsrtp which we are trying to avoid.</span><br><span> */</span><br><span style="color: hsl(0, 100%, 40%);">- frame = ast_frdup(ast_rtp_interpret(instance, srtp, &addr, read_area, res, prev_seqno));</span><br><span style="color: hsl(120, 100%, 40%);">+ frame = ast_frdup(ast_rtp_interpret(instance, srtp, &addr, read_area, res, prev_seqno, bundled));</span><br><span> if (frame) {</span><br><span> AST_LIST_INSERT_TAIL(&frames, frame, frame_list);</span><br><span> prev_seqno = seqno;</span><br><span>@@ -7889,7 +7893,7 @@</span><br><span> break;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- frame = ast_frdup(ast_rtp_interpret(instance, srtp, &addr, payload->buf, payload->size, prev_seqno));</span><br><span style="color: hsl(120, 100%, 40%);">+ frame = ast_frdup(ast_rtp_interpret(instance, srtp, &addr, payload->buf, payload->size, prev_seqno, bundled));</span><br><span> ast_free(payload);</span><br><span> </span><br><span> if (!frame) {</span><br><span>@@ -7935,7 +7939,7 @@</span><br><span> </span><br><span> /* If the packet we received is the one we are expecting at this point then add it in */</span><br><span> if (rtp->expectedrxseqno == seqno) {</span><br><span style="color: hsl(0, 100%, 40%);">- frame = ast_frdup(ast_rtp_interpret(instance, srtp, &addr, read_area, res, prev_seqno));</span><br><span style="color: hsl(120, 100%, 40%);">+ frame = ast_frdup(ast_rtp_interpret(instance, srtp, &addr, read_area, res, prev_seqno, bundled));</span><br><span> if (frame) {</span><br><span> AST_LIST_INSERT_TAIL(&frames, frame, frame_list);</span><br><span> prev_seqno = seqno;</span><br><span>@@ -7960,7 +7964,7 @@</span><br><span> </span><br><span> payload = (struct ast_rtp_rtcp_nack_payload *)ast_data_buffer_remove(rtp->recv_buffer, rtp->expectedrxseqno);</span><br><span> if (payload) {</span><br><span style="color: hsl(0, 100%, 40%);">- frame = ast_frdup(ast_rtp_interpret(instance, srtp, &addr, payload->buf, payload->size, prev_seqno));</span><br><span style="color: hsl(120, 100%, 40%);">+ frame = ast_frdup(ast_rtp_interpret(instance, srtp, &addr, payload->buf, payload->size, prev_seqno, bundled));</span><br><span> if (frame) {</span><br><span> AST_LIST_INSERT_TAIL(&frames, frame, frame_list);</span><br><span> prev_seqno = rtp->expectedrxseqno;</span><br><span>@@ -7982,7 +7986,7 @@</span><br><span> * to be the last packet processed right now and it is also guaranteed that it will always return</span><br><span> * non-NULL.</span><br><span> */</span><br><span style="color: hsl(0, 100%, 40%);">- frame = ast_rtp_interpret(instance, srtp, &addr, read_area, res, prev_seqno);</span><br><span style="color: hsl(120, 100%, 40%);">+ frame = ast_rtp_interpret(instance, srtp, &addr, read_area, res, prev_seqno, bundled);</span><br><span> AST_LIST_INSERT_TAIL(&frames, frame, frame_list);</span><br><span> rtp->expectedrxseqno = seqno + 1;</span><br><span> if (rtp->expectedrxseqno == SEQNO_CYCLE_OVER) {</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/15641">change 15641</a>. To unsubscribe, or for help writing mail filters, 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/c/asterisk/+/15641"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 18 </div>
<div style="display:none"> Gerrit-Change-Id: I2d4c8f93ea24abc1030196706de2d70facf05a5a </div>
<div style="display:none"> Gerrit-Change-Number: 15641 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Reviewer: Benjamin Keith Ford <bford@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>