rtcp::ReportBlock refactored to contain parsing

rtcp::ReportBlock refactored to contain parsing

Committed: https://crrev.com/df948f03b34dc652c2b3a944535fc01ec22395ce
Cr-Commit-Position: refs/heads/master@{#10633}

[danilchap, 2015-11-09 14:38:08]

Description was changed from

==========
rtcp::ReportBlock refactored to contain parsing

BUG=
==========

to

==========
rtcp::ReportBlock refactored to contain parsing
==========

[danilchap, 2015-11-09 14:38:08]

danilchap@webrtc.org changed reviewers:
+ sprang@webrtc.org

[sprang_webrtc, 2015-11-09 16:28:39]

sprang@webrtc.org changed reviewers:
+ asapersson@webrtc.org

[sprang_webrtc, 2015-11-09 16:28:41]

+asapersson
FYI, I told Danil to take on an Rtcp packet class at a time and add parsing.
Then go for refactoring RtcpReceiver.

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet.cc (right):

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet.cc:161: size_t consumed =
block.Create(buffer + *pos);
perhaps bytes_consumed to be clear with units?

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet.cc:162: assert(consumed > 0);
RTC_CHECK_GT(consumed, 0);

(#include "webrtc/base/checks.h")

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h (right):

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h:2: *  Copyright (c)
2014 The WebRTC project authors. All Rights Reserved.
2015

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h:16: #include
"webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h"  // RTCPReportBlock
Don't usually comment what is used from an include, unless the import seems
totally uncalled for.
Also, line too long. Rung "git cl format" automatically get a bunch of
formatting fixed for you.

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h:25: class
ReportBlockParser {
Is this class intended to be used somewhere else? Otherwise I think it would be
simpler to just read the data directly in the Parse() method.

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h:76: }
Would prefer not to do this inline.

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h:99: uint32_t
source_ssrc() const { return report_block_.sourceSSRC; }
These should be preferably be in CamelCase as well.

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block_unittest.cc
(right):

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block_unittest.cc:37: // fill
buffer with semi-random data
Nit: All comments should start with a capital and end with a period.

[danilchap, 2015-11-09 16:59:45]

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet.cc (right):

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet.cc:161: size_t consumed =
block.Create(buffer + *pos);
On 2015/11/09 16:28:40, språng wrote:
> perhaps bytes_consumed to be clear with units?

Acknowledged.

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet.cc:162: assert(consumed > 0);
On 2015/11/09 16:28:40, språng wrote:
> RTC_CHECK_GT(consumed, 0);
> 
> (#include "webrtc/base/checks.h")

Acknowledged.

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h (right):

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h:2: *  Copyright (c)
2014 The WebRTC project authors. All Rights Reserved.
On 2015/11/09 16:28:41, språng wrote:
> 2015

Acknowledged.

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h:16: #include
"webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h"  // RTCPReportBlock
On 2015/11/09 16:28:40, språng wrote:
> Don't usually comment what is used from an include, unless the import seems
> totally uncalled for.
> Also, line too long. Rung "git cl format" automatically get a bunch of
> formatting fixed for you.

git cl format skipped it.

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h:16: #include
"webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h"  // RTCPReportBlock
On 2015/11/09 16:28:40, språng wrote:
> Don't usually comment what is used from an include, unless the import seems
> totally uncalled for.
> Also, line too long. Rung "git cl format" automatically get a bunch of
> formatting fixed for you.

Done.

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h:25: class
ReportBlockParser {
On 2015/11/09 16:28:40, språng wrote:
> Is this class intended to be used somewhere else? Otherwise I think it would
be
> simpler to just read the data directly in the Parse() method.

Yes, the idea to have similar class for all rtcp packets. SenderReportParser
would use this class for parsing instead of ReportBlock. (SenderReport would use
ReportBlock, like it is now)
The final parsing normally read each value once, having this set of classes will
allow to read the value directly from the source buffer (using nice name)
without copying it into structure in the middle.
More importantly, this set of classes would avoid creating/destroying containers
while parsing.

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h:76: }
On 2015/11/09 16:28:40, språng wrote:
> Would prefer not to do this inline.

Acknowledged.

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h:99: uint32_t
source_ssrc() const { return report_block_.sourceSSRC; }
On 2015/11/09 16:28:40, språng wrote:
> These should be preferably be in CamelCase as well.

snake_case hints it is a cheap accessor, not an heavy function

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block_unittest.cc
(right):

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block_unittest.cc:37: // fill
buffer with semi-random data
On 2015/11/09 16:28:41, språng wrote:
> Nit: All comments should start with a capital and end with a period.

Acknowledged.

[sprang_webrtc, 2015-11-10 10:36:34]

Looks good, just a few comments.

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h (right):

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h:25: class
ReportBlockParser {
On 2015/11/09 16:59:45, danilchap wrote:
> On 2015/11/09 16:28:40, språng wrote:
> > Is this class intended to be used somewhere else? Otherwise I think it would
> be
> > simpler to just read the data directly in the Parse() method.
> 
> Yes, the idea to have similar class for all rtcp packets. SenderReportParser
> would use this class for parsing instead of ReportBlock. (SenderReport would
use
> ReportBlock, like it is now)
> The final parsing normally read each value once, having this set of classes
will
> allow to read the value directly from the source buffer (using nice name)
> without copying it into structure in the middle.
> More importantly, this set of classes would avoid creating/destroying
containers
> while parsing.

OK, gotcha.
Not sure why you want to pass this by value though? I'd be more comfortable
using
DISALLOW_COPY_AND_ASSIGN and always passing a reference, just so the scope is
clear. Are you planning to use it in a different way later?

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h:99: uint32_t
source_ssrc() const { return report_block_.sourceSSRC; }
On 2015/11/09 16:59:45, danilchap wrote:
> On 2015/11/09 16:28:40, språng wrote:
> > These should be preferably be in CamelCase as well.
> 
> snake_case hints it is a cheap accessor, not an heavy function

Right, that's technically correct. In our code base it also seems
to indicate legacy code though :)
But I'm fine either way.

[danilchap, 2015-11-10 11:14:26]

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h (right):

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h:25: class
ReportBlockParser {
On 2015/11/10 10:36:34, språng wrote:
> On 2015/11/09 16:59:45, danilchap wrote:
> > On 2015/11/09 16:28:40, språng wrote:
> > > Is this class intended to be used somewhere else? Otherwise I think it
would
> > be
> > > simpler to just read the data directly in the Parse() method.
> > 
> > Yes, the idea to have similar class for all rtcp packets. SenderReportParser
> > would use this class for parsing instead of ReportBlock. (SenderReport would
> use
> > ReportBlock, like it is now)
> > The final parsing normally read each value once, having this set of classes
> will
> > allow to read the value directly from the source buffer (using nice name)
> > without copying it into structure in the middle.
> > More importantly, this set of classes would avoid creating/destroying
> containers
> > while parsing.
> 
> OK, gotcha.
> Not sure why you want to pass this by value though? I'd be more comfortable
> using
> DISALLOW_COPY_AND_ASSIGN and always passing a reference, just so the scope is
> clear. Are you planning to use it in a different way later?
Here is the plan how to use it:
class SenderReportParser {
  ReportBlockParser block(size_t index) { return ReportBlockParser(buffer_ +
kSenderReportHeaderSize + kReportBlockSize*index); } 
};

[sprang_webrtc, 2015-11-10 13:21:46]

lgtm
åsa?

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h (right):

https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h:25: class
ReportBlockParser {
On 2015/11/10 11:14:26, danilchap wrote:
> On 2015/11/10 10:36:34, språng wrote:
> > On 2015/11/09 16:59:45, danilchap wrote:
> > > On 2015/11/09 16:28:40, språng wrote:
> > > > Is this class intended to be used somewhere else? Otherwise I think it
> would
> > > be
> > > > simpler to just read the data directly in the Parse() method.
> > > 
> > > Yes, the idea to have similar class for all rtcp packets.
SenderReportParser
> > > would use this class for parsing instead of ReportBlock. (SenderReport
would
> > use
> > > ReportBlock, like it is now)
> > > The final parsing normally read each value once, having this set of
classes
> > will
> > > allow to read the value directly from the source buffer (using nice name)
> > > without copying it into structure in the middle.
> > > More importantly, this set of classes would avoid creating/destroying
> > containers
> > > while parsing.
> > 
> > OK, gotcha.
> > Not sure why you want to pass this by value though? I'd be more comfortable
> > using
> > DISALLOW_COPY_AND_ASSIGN and always passing a reference, just so the scope
is
> > clear. Are you planning to use it in a different way later?
> Here is the plan how to use it:
> class SenderReportParser {
>   ReportBlockParser block(size_t index) { return ReportBlockParser(buffer_ +
> kSenderReportHeaderSize + kReportBlockSize*index); } 
> };

OK, fair enough. I'd probably have used an output parameter but I'm OK with this
too. Still, the static_assert seems a bit overkill just for a minor performance
check, no?

[danilchap, 2015-11-10 16:10:49]

On 2015/11/10 13:21:46, språng wrote:
> lgtm
> åsa?
> 
>
https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
> File webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h (right):
> 
>
https://codereview.webrtc.org/1420283022/diff/40001/webrtc/modules/rtp_rtcp/s...
> webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h:25: class
> ReportBlockParser {
> On 2015/11/10 11:14:26, danilchap wrote:
> > On 2015/11/10 10:36:34, språng wrote:
> > > On 2015/11/09 16:59:45, danilchap wrote:
> > > > On 2015/11/09 16:28:40, språng wrote:
> > > > > Is this class intended to be used somewhere else? Otherwise I think it
> > would
> > > > be
> > > > > simpler to just read the data directly in the Parse() method.
> > > > 
> > > > Yes, the idea to have similar class for all rtcp packets.
> SenderReportParser
> > > > would use this class for parsing instead of ReportBlock. (SenderReport
> would
> > > use
> > > > ReportBlock, like it is now)
> > > > The final parsing normally read each value once, having this set of
> classes
> > > will
> > > > allow to read the value directly from the source buffer (using nice
name)
> > > > without copying it into structure in the middle.
> > > > More importantly, this set of classes would avoid creating/destroying
> > > containers
> > > > while parsing.
> > > 
> > > OK, gotcha.
> > > Not sure why you want to pass this by value though? I'd be more
comfortable
> > > using
> > > DISALLOW_COPY_AND_ASSIGN and always passing a reference, just so the scope
> is
> > > clear. Are you planning to use it in a different way later?
> > Here is the plan how to use it:
> > class SenderReportParser {
> >   ReportBlockParser block(size_t index) { return ReportBlockParser(buffer_ +
> > kSenderReportHeaderSize + kReportBlockSize*index); } 
> > };
> 
> OK, fair enough. I'd probably have used an output parameter but I'm OK with
this
> too. Still, the static_assert seems a bit overkill just for a minor
performance
> check, no?

I see static_assert as a cheap test and hint how this class would be used. Do
not see it as an overkill.

But set of parser classes is likely an overkill (tried it with bye packet -
cl1419573016, looks too ugly for a small perfomance gain in the code that is not
perfomance critical).
Will remove ReportBlockParser and abandon its idea.

[åsapersson, 2015-11-12 14:26:25]

Looks good, a few minor comments.

https://codereview.webrtc.org/1420283022/diff/80001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet.cc (right):

https://codereview.webrtc.org/1420283022/diff/80001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet.cc:162: RTC_CHECK_GT(bytes_consumed,
0u);
Maybe move check to when setting the field cum loss.

It does not seem to be any check that the loss can't be greater than 0xFFFFFF in
receive_statistics_impl.cc where it is calculated. Should maybe be added.

https://codereview.webrtc.org/1420283022/diff/80001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet.h (right):

https://codereview.webrtc.org/1420283022/diff/80001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet.h:23: #include
"webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h"
nit: alphabetic order

https://codereview.webrtc.org/1420283022/diff/80001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.cc (right):

https://codereview.webrtc.org/1420283022/diff/80001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.cc:48: if (buffer ==
nullptr) {
Can RTC_CHECK be used?

https://codereview.webrtc.org/1420283022/diff/80001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block_unittest.cc
(right):

https://codereview.webrtc.org/1420283022/diff/80001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block_unittest.cc:34:
TEST(RtcpPacketTest, ReportBlock_InvalidParse) {
maybe rename RtcpPacketTest to RtcpPacketReportBlockTest (similar to the other
new classes)

https://codereview.webrtc.org/1420283022/diff/80001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block_unittest.cc:44: // Any
data is valid for parsing, as long as there is some.
Maybe move to separate test or rename test.

[danilchap, 2015-11-12 15:30:46]

https://codereview.webrtc.org/1420283022/diff/80001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet.cc (right):

https://codereview.webrtc.org/1420283022/diff/80001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet.cc:162: RTC_CHECK_GT(bytes_consumed,
0u);
On 2015/11/12 14:26:24, åsapersson wrote:
> Maybe move check to when setting the field cum loss.
> 
> It does not seem to be any check that the loss can't be greater than 0xFFFFFF
in
> receive_statistics_impl.cc where it is calculated. Should maybe be added. 

Moved check for CumulativeLost to ReportBlock::WithCumulativeLost
Adjusted RtcpSender::AddReport to propogate that error, but it is still ignored
in the RtcpSender::PrepareRTCP

https://codereview.webrtc.org/1420283022/diff/80001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet.h (right):

https://codereview.webrtc.org/1420283022/diff/80001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet.h:23: #include
"webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h"
On 2015/11/12 14:26:25, åsapersson wrote:
> nit: alphabetic order

Done.

https://codereview.webrtc.org/1420283022/diff/80001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.cc (right):

https://codereview.webrtc.org/1420283022/diff/80001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.cc:48: if (buffer ==
nullptr) {
On 2015/11/12 14:26:25, åsapersson wrote:
> Can RTC_CHECK be used?
yes, RTC_DCHECK make even more sense

https://codereview.webrtc.org/1420283022/diff/80001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block_unittest.cc
(right):

https://codereview.webrtc.org/1420283022/diff/80001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block_unittest.cc:34:
TEST(RtcpPacketTest, ReportBlock_InvalidParse) {
On 2015/11/12 14:26:25, åsapersson wrote:
> maybe rename RtcpPacketTest to RtcpPacketReportBlockTest (similar to the other
> new classes)

Done.

https://codereview.webrtc.org/1420283022/diff/80001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block_unittest.cc:44: // Any
data is valid for parsing, as long as there is some.
On 2015/11/12 14:26:25, åsapersson wrote:
> Maybe move to separate test or rename test. 

Done, other tests are also renamed to better match what they test

[åsapersson, 2015-11-13 08:26:33]

lgtm

https://codereview.webrtc.org/1420283022/diff/100001/webrtc/modules/rtp_rtcp/...
File webrtc/modules/rtp_rtcp/rtp_rtcp.gypi (right):

https://codereview.webrtc.org/1420283022/diff/100001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/rtp_rtcp.gypi:47: 'source/rtcp_packet/report_block.h',
nit: alphabetic order

https://codereview.webrtc.org/1420283022/diff/100001/webrtc/modules/rtp_rtcp/...
File webrtc/modules/rtp_rtcp/source/rtcp_packet.cc (right):

https://codereview.webrtc.org/1420283022/diff/100001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/source/rtcp_packet.cc:161: block.Create(buffer + *pos);
maybe let Create advance pos?

[danilchap, 2015-11-13 10:12:55]

The CQ bit was checked by danilchap@webrtc.org

[danilchap, 2015-11-13 10:12:55]

The patchset sent to the CQ was uploaded after l-g-t-m from sprang@webrtc.org,
asapersson@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1420283022/#ps120001 (title:
" ")

[commit-bot: I haz the power, 2015-11-13 10:12:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1420283022/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1420283022/120001

[danilchap, 2015-11-13 10:13:39]

https://codereview.webrtc.org/1420283022/diff/100001/webrtc/modules/rtp_rtcp/...
File webrtc/modules/rtp_rtcp/rtp_rtcp.gypi (right):

https://codereview.webrtc.org/1420283022/diff/100001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/rtp_rtcp.gypi:47: 'source/rtcp_packet/report_block.h',
On 2015/11/13 08:26:33, åsapersson wrote:
> nit: alphabetic order

Done.

https://codereview.webrtc.org/1420283022/diff/100001/webrtc/modules/rtp_rtcp/...
File webrtc/modules/rtp_rtcp/source/rtcp_packet.cc (right):

https://codereview.webrtc.org/1420283022/diff/100001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/source/rtcp_packet.cc:161: block.Create(buffer + *pos);
On 2015/11/13 08:26:33, åsapersson wrote:
> maybe let Create advance pos?
Ack.
That would require Create take additional parameter just to increment it by
known value.
Caller already expect to know size of ReportBlock to ensure buffer has enough
space.
Moving that check from Caller to ReportBlock would require check per block
instead of single batch check.

[commit-bot: I haz the power, 2015-11-13 11:03:16]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-11-13 11:03:28]

Patchset 7 (id:??) landed as
https://crrev.com/df948f03b34dc652c2b3a944535fc01ec22395ce
Cr-Commit-Position: refs/heads/master@{#10633}