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