[prev in list] [next in list] [prev in thread] [next in thread]
List: asterisk-commits
Subject: [asterisk-commits] =?utf-8?q?res_rtp_asterisk=3A_Trim_trailing_by?= =?utf-8?q?te_off_of_SDES_packet_
From: SVN commits to the Asterisk project <asterisk-commits () lists ! digium ! com>
Date: 2017-09-28 11:58:43
Message-ID: mailman.59809.1506624568.27840.asterisk-commits () lists ! digium ! com
[Download RAW message or body]
Joshua Colp has submitted this change and it was merged. ( \
https://gerrit.asterisk.org/6601 )
Change subject: res_rtp_asterisk: Trim trailing byte off of SDES packet
......................................................................
res_rtp_asterisk: Trim trailing byte off of SDES packet
This could have been fixed by subtracting 1 from the final value of
'len' but the way the packet was being constructed was confusing so I
took the opportunity to (I think) make it more clear.
We were sending 1 extra byte at the end of the SDES RTCP packet which
caused Chrome to complain (in its debug log):
Too little data (1 byte) remaining in buffer to parse
RTCP header (4 bytes).
We now send the correct number of bytes.
Change-Id: I9dcf087cdaf97da0374ae0acb7d379746a71e81b
---
M res/res_rtp_asterisk.c
1 file changed, 29 insertions(+), 5 deletions(-)
Approvals:
Joshua Colp: Looks good to me, but someone else must approve; Approved for Submit
George Joseph: Looks good to me, approved
diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index 8f66a0a..7bb9152 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -3763,6 +3763,7 @@
RAII_VAR(struct ast_json *, message_blob, NULL, ast_json_unref);
int res;
int len = 0;
+ uint16_t sdes_packet_len_bytes, sdes_packet_len_rounded;
struct timeval now;
unsigned int now_lsw;
unsigned int now_msw;
@@ -3770,7 +3771,7 @@
unsigned int lost_packets;
int fraction_lost;
struct timeval dlsr = { 0, };
- unsigned char bdata[512] = "";
+ unsigned char bdata[AST_UUID_STR_LEN + 128] = ""; /* More than enough */
int rate = rtp_get_rate(rtp->f.subclass.format);
int ice;
struct ast_sockaddr remote_address = { { 0, } };
@@ -3850,12 +3851,35 @@
put_unaligned_uint32(rtcpheader, htonl((2 << 30) | \
(rtcp_report->reception_report_count << 24) | ((sr ? RTCP_PT_SR : RTCP_PT_RR) << 16) \
| ((len/4)-1)));
- put_unaligned_uint32(rtcpheader + len, htonl((2 << 30) | (1 << 24) | (RTCP_PT_SDES \
<< 16) | (2 + (AST_UUID_STR_LEN / 4)))); + sdes_packet_len_bytes =
+ 4 + /* RTCP Header */
+ 4 + /* SSRC */
+ 1 + /* Type (CNAME) */
+ 1 + /* Text Length */
+ AST_UUID_STR_LEN /* Text and NULL terminator */
+ ;
+
+ /* Round to 32 bit boundary */
+ sdes_packet_len_rounded = (sdes_packet_len_bytes + 3) & ~0x3;
+
+ put_unaligned_uint32(rtcpheader + len, htonl((2 << 30) | (1 << 24) | (RTCP_PT_SDES \
<< 16) | ((sdes_packet_len_rounded / 4) - 1))); put_unaligned_uint32(rtcpheader + \
len + 4, htonl(rtcp_report->ssrc));
- put_unaligned_uint16(rtcpheader + len + 8, htonl(0x01 << 24));
- put_unaligned_uint16(rtcpheader + len + 9, htonl(AST_UUID_STR_LEN << 24));
+ rtcpheader[len + 8] = 0x01; /* CNAME */
+ rtcpheader[len + 9] = AST_UUID_STR_LEN - 1; /* Number of bytes of text */
memcpy(rtcpheader + len + 10, rtp->cname, AST_UUID_STR_LEN);
- len += 12 + AST_UUID_STR_LEN;
+ len += 10 + AST_UUID_STR_LEN;
+
+ /* Padding - Note that we don't set the padded bit on the packet. From
+ * RFC 3550 Section 6.5:
+ *
+ * No length octet follows the null item type octet, but additional null
+ * octets MUST be included if needed to pad until the next 32-bit
+ * boundary. Note that this padding is separate from that indicated by
+ * the P bit in the RTCP header.
+ *
+ * These bytes will already be zeroed out during array initialization.
+ */
+ len += (sdes_packet_len_rounded - sdes_packet_len_bytes);
if (rtp->bundled) {
ast_rtp_instance_get_remote_address(instance, &remote_address);
--
To view, visit https://gerrit.asterisk.org/6601
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 15.0
Gerrit-MessageType: merged
Gerrit-Change-Id: I9dcf087cdaf97da0374ae0acb7d379746a71e81b
Gerrit-Change-Number: 6601
Gerrit-PatchSet: 1
Gerrit-Owner: Sean Bright <sean.bright@gmail.com>
Gerrit-Reviewer: George Joseph <gjoseph@digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp@digium.com>
[Attachment #3 (text/html)]
<p>Joshua Colp <strong>merged</strong> this change.</p><p><a \
href="https://gerrit.asterisk.org/6601">View Change</a></p><div \
style="white-space:pre-wrap">Approvals: Joshua Colp: Looks good to me, but someone \
else must approve; Approved for Submit George Joseph: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: \
pre-wrap;">res_rtp_asterisk: Trim trailing byte off of SDES packet<br><br>This could \
have been fixed by subtracting 1 from the final value of<br>'len' but the way \
the packet was being constructed was confusing so I<br>took the opportunity to (I \
think) make it more clear.<br><br>We were sending 1 extra byte at the end of the SDES \
RTCP packet which<br>caused Chrome to complain (in its debug log):<br><br> Too \
little data (1 byte) remaining in buffer to parse<br> RTCP header (4 \
bytes).<br><br>We now send the correct number of bytes.<br><br>Change-Id: \
I9dcf087cdaf97da0374ae0acb7d379746a71e81b<br>---<br>M res/res_rtp_asterisk.c<br>1 \
file changed, 29 insertions(+), 5 deletions(-)<br><br></pre><pre style="font-family: \
monospace,monospace; white-space: pre-wrap;">diff --git a/res/res_rtp_asterisk.c \
b/res/res_rtp_asterisk.c<br>index 8f66a0a..7bb9152 100644<br>--- \
a/res/res_rtp_asterisk.c<br>+++ b/res/res_rtp_asterisk.c<br>@@ -3763,6 +3763,7 @@<br> \
RAII_VAR(struct ast_json *, message_blob, NULL, ast_json_unref);<br> int res;<br> \
int len = 0;<br>+ uint16_t sdes_packet_len_bytes, sdes_packet_len_rounded;<br> \
struct timeval now;<br> unsigned int now_lsw;<br> unsigned int now_msw;<br>@@ \
-3770,7 +3771,7 @@<br> unsigned int lost_packets;<br> int fraction_lost;<br> \
struct timeval dlsr = { 0, };<br>- unsigned char bdata[512] = \
"";<br>+ unsigned char bdata[AST_UUID_STR_LEN + 128] = ""; /* \
More than enough */<br> int rate = rtp_get_rate(rtp->f.subclass.format);<br> int \
ice;<br> struct ast_sockaddr remote_address = { { 0, } };<br>@@ -3850,12 +3851,35 \
@@<br> put_unaligned_uint32(rtcpheader, htonl((2 << 30) | \
(rtcp_report->reception_report_count << 24)<br> | ((sr ? RTCP_PT_SR : \
RTCP_PT_RR) << 16) | ((len/4)-1)));<br> <br>- put_unaligned_uint32(rtcpheader + \
len, htonl((2 << 30) | (1 << 24) | (RTCP_PT_SDES << 16) | (2 + \
(AST_UUID_STR_LEN / 4))));<br>+ sdes_packet_len_bytes =<br>+ 4 + /* RTCP Header \
*/<br>+ 4 + /* SSRC */<br>+ 1 + /* Type (CNAME) */<br>+ 1 + /* Text Length \
*/<br>+ AST_UUID_STR_LEN /* Text and NULL terminator */<br>+ ;<br>+<br>+ /* Round \
to 32 bit boundary */<br>+ sdes_packet_len_rounded = (sdes_packet_len_bytes + 3) \
& ~0x3;<br>+<br>+ put_unaligned_uint32(rtcpheader + len, htonl((2 << 30) | \
(1 << 24) | (RTCP_PT_SDES << 16) | ((sdes_packet_len_rounded / 4) - \
1)));<br> put_unaligned_uint32(rtcpheader + len + 4, \
htonl(rtcp_report->ssrc));<br>- put_unaligned_uint16(rtcpheader + len + 8, \
htonl(0x01 << 24));<br>- put_unaligned_uint16(rtcpheader + len + 9, \
htonl(AST_UUID_STR_LEN << 24));<br>+ rtcpheader[len + 8] = 0x01; /* CNAME \
*/<br>+ rtcpheader[len + 9] = AST_UUID_STR_LEN - 1; /* Number of bytes of text */<br> \
memcpy(rtcpheader + len + 10, rtp->cname, AST_UUID_STR_LEN);<br>- len += 12 + \
AST_UUID_STR_LEN;<br>+ len += 10 + AST_UUID_STR_LEN;<br>+<br>+ /* Padding - Note that \
we don't set the padded bit on the packet. From<br>+ * RFC 3550 Section \
6.5:<br>+ *<br>+ * No length octet follows the null item type octet, but \
additional null<br>+ * octets MUST be included if needed to pad until the next \
32-bit<br>+ * boundary. Note that this padding is separate from that indicated \
by<br>+ * the P bit in the RTCP header.<br>+ *<br>+ * These bytes will already \
be zeroed out during array initialization.<br>+ */<br>+ len += \
(sdes_packet_len_rounded - sdes_packet_len_bytes);<br> <br> if (rtp->bundled) \
{<br> ast_rtp_instance_get_remote_address(instance, \
&remote_address);<br></pre><p>To view, visit <a \
href="https://gerrit.asterisk.org/6601">change 6601</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/6601"/><meta itemprop="name" content="View \
Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 15.0 </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: \
I9dcf087cdaf97da0374ae0acb7d379746a71e81b </div> <div style="display:none"> \
Gerrit-Change-Number: 6601 </div> <div style="display:none"> Gerrit-PatchSet: 1 \
</div> <div style="display:none"> Gerrit-Owner: Sean Bright \
<sean.bright@gmail.com> </div> <div style="display:none"> Gerrit-Reviewer: \
George Joseph <gjoseph@digium.com> </div> <div style="display:none"> \
Gerrit-Reviewer: Jenkins2 </div> <div style="display:none"> Gerrit-Reviewer: Joshua \
Colp <jcolp@digium.com> </div>
--
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-commits mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-commits
--===============1502494457452638971==--
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic