Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(110)

Issue 1165113002: Add support for fragmentation in RtcpPacket. (Closed)

Created:
5 years, 6 months ago by sprang_webrtc
Modified:
5 years, 6 months ago
Reviewers:
åsapersson
CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com, stefan-webrtc, mflodman
Base URL:
https://chromium.googlesource.com/external/webrtc.git@master
Target Ref:
refs/pending/heads/master
Project:
webrtc
Visibility:
Public.

Description

Add support for fragmentation in RtcpPacket. If the buffer becomes full an OnPacketReady callback will be used to send the packets created so far. On success the buffer can be reused. The same callback will be called when the last packet has beed created. Also made some changes to RawPacket. Buffer will now be heap-allocated rather than (potentially) stack-allocated, but on the plus side it can now be allocted with variable size and also avoids one memcpy. BUG= patch from issue 56429004 at patchset 160001 (http://crrev.com/56429004#ps160001) R=asapersson@webrtc.org Committed: https://crrev.com/c1b9d4e686c184e4b1779d442d447128477d3b8b Cr-Commit-Position: refs/heads/master@{#9390}

Patch Set 1 #

Patch Set 2 : Removed stack allocated variable length array #

Total comments: 2

Patch Set 3 : Fixed nit, plus buffer overflow in nack test #

Unified diffs Side-by-side diffs Delta from patch set Stats (+864 lines, -481 lines) Patch
M webrtc/modules/rtp_rtcp/source/rtcp_packet.h View 1 22 chunks +143 lines, -96 lines 0 comments Download
M webrtc/modules/rtp_rtcp/source/rtcp_packet.cc View 1 11 chunks +288 lines, -138 lines 0 comments Download
M webrtc/modules/rtp_rtcp/source/rtcp_packet_unittest.cc View 1 2 44 chunks +342 lines, -156 lines 0 comments Download
M webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc View 40 chunks +88 lines, -88 lines 0 comments Download
M webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc View 1 chunk +3 lines, -3 lines 0 comments Download

Messages

Total messages: 7 (1 generated)
sprang_webrtc
Patched CL over to new rietveld.
5 years, 6 months ago (2015-06-05 13:24:40 UTC) #2
åsapersson
https://codereview.webrtc.org/1165113002/diff/20001/webrtc/modules/rtp_rtcp/source/rtcp_packet_unittest.cc File webrtc/modules/rtp_rtcp/source/rtcp_packet_unittest.cc (right): https://codereview.webrtc.org/1165113002/diff/20001/webrtc/modules/rtp_rtcp/source/rtcp_packet_unittest.cc#newcode452 webrtc/modules/rtp_rtcp/source/rtcp_packet_unittest.cc:452: const size_t buffer_size = 12 + (3 * 4); ...
5 years, 6 months ago (2015-06-05 14:09:59 UTC) #3
åsapersson
On 2015/06/05 14:09:59, asapersson wrote: > https://codereview.webrtc.org/1165113002/diff/20001/webrtc/modules/rtp_rtcp/source/rtcp_packet_unittest.cc > File webrtc/modules/rtp_rtcp/source/rtcp_packet_unittest.cc (right): > > https://codereview.webrtc.org/1165113002/diff/20001/webrtc/modules/rtp_rtcp/source/rtcp_packet_unittest.cc#newcode452 > ...
5 years, 6 months ago (2015-06-05 14:57:55 UTC) #4
sprang_webrtc
Committed patchset #3 (id:40001) manually as c1b9d4e686c184e4b1779d442d447128477d3b8b (presubmit successful).
5 years, 6 months ago (2015-06-08 07:54:30 UTC) #5
commit-bot: I haz the power
Patchset 3 (id:??) landed as https://crrev.com/c1b9d4e686c184e4b1779d442d447128477d3b8b Cr-Commit-Position: refs/heads/master@{#9390}
5 years, 6 months ago (2015-06-08 07:54:38 UTC) #6
sprang_webrtc
5 years, 6 months ago (2015-06-11 08:22:01 UTC) #7
Message was sent while issue was closed.
https://codereview.webrtc.org/1165113002/diff/20001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet_unittest.cc (right):

https://codereview.webrtc.org/1165113002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet_unittest.cc:452: const size_t
buffer_size = 12 + (3 * 4);  // Fits common header + 3 nack items
On 2015/06/05 14:09:59, asapersson wrote:
> buffer_size->kBufferSize

Done.

Powered by Google App Engine
This is Rietveld 408576698