rtcp::App moved into own file and got Parse function

rtcp::App moved into own file and got Parse function

Committed: https://crrev.com/0219c9b4bfcbb778137756210eb95f40d936cc66
Cr-Commit-Position: refs/heads/master@{#10688}

[danilchap, 2015-11-13 12:46:21]

Description was changed from

==========
rtcp::App moved into own file
rtcp::App got Parse function
==========

to

==========
rtcp::App moved into own file and got Parse function
==========

[danilchap, 2015-11-13 12:46:21]

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

[åsapersson, 2015-11-17 15:31:36]

https://codereview.webrtc.org/1437353003/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/app.cc (right):

https://codereview.webrtc.org/1437353003/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtcp_packet/app.cc:47: if (subtype > 0x1f) {
Should we use RTC_DCHECK?

https://codereview.webrtc.org/1437353003/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtcp_packet/app.cc:62: if (data_length % 4 != 0)
{
Should we use RTC_DCHECK?

https://codereview.webrtc.org/1437353003/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/app.h (right):

https://codereview.webrtc.org/1437353003/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtcp_packet/app.h:14: #include <vector>
needed?

[danilchap, 2015-11-17 17:12:36]

https://codereview.webrtc.org/1437353003/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/app.cc (right):

https://codereview.webrtc.org/1437353003/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtcp_packet/app.cc:47: if (subtype > 0x1f) {
On 2015/11/17 15:31:36, åsapersson wrote:
> Should we use RTC_DCHECK?

Done.

https://codereview.webrtc.org/1437353003/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtcp_packet/app.cc:62: if (data_length % 4 != 0)
{
On 2015/11/17 15:31:36, åsapersson wrote:
> Should we use RTC_DCHECK?

Done.

https://codereview.webrtc.org/1437353003/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/app.h (right):

https://codereview.webrtc.org/1437353003/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtcp_packet/app.h:14: #include <vector>
On 2015/11/17 15:31:36, åsapersson wrote:
> needed?

Done.

[åsapersson, 2015-11-18 10:36:55]

https://codereview.webrtc.org/1437353003/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/app.cc (right):

https://codereview.webrtc.org/1437353003/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtcp_packet/app.cc:47: if (subtype > 0x1f) {
On 2015/11/17 17:12:36, danilchap wrote:
> On 2015/11/17 15:31:36, åsapersson wrote:
> > Should we use RTC_DCHECK?
> 
> Done.

Maybe RTC_DCHECK_LE(subtype, 0x1f);

[danilchap, 2015-11-18 11:15:54]

https://codereview.webrtc.org/1437353003/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/app.cc (right):

https://codereview.webrtc.org/1437353003/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtcp_packet/app.cc:47: if (subtype > 0x1f) {
On 2015/11/18 10:36:55, åsapersson wrote:
> On 2015/11/17 17:12:36, danilchap wrote:
> > On 2015/11/17 15:31:36, åsapersson wrote:
> > > Should we use RTC_DCHECK?
> > 
> > Done.
> 
> Maybe RTC_DCHECK_LE(subtype, 0x1f);

Yes, got confused by RTC_DCHECK_EQ where it is common to put constant as first
parameter.
search through code shows RTC_DCHECK_LE/RTC_DCHECK_GE normally use constant as
second parameter.

[åsapersson, 2015-11-18 11:24:15]

lgtm

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

https://codereview.webrtc.org/1437353003/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/app_unittest.cc:19: using
webrtc::rtcp::App;
alphabetic order

[danilchap, 2015-11-18 12:12:14]

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

https://codereview.webrtc.org/1437353003/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/app_unittest.cc:19: using
webrtc::rtcp::App;
On 2015/11/18 11:24:15, åsapersson wrote:
> alphabetic order

Done.

[danilchap, 2015-11-18 13:07:38]

The CQ bit was checked by danilchap@webrtc.org

[danilchap, 2015-11-18 13:07:38]

The patchset sent to the CQ was uploaded after l-g-t-m from
asapersson@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1437353003/#ps60001 (title:
"merged with master")

[commit-bot: I haz the power, 2015-11-18 13:07:51]

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

[commit-bot: I haz the power, 2015-11-18 13:56:57]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-11-18 13:57:04]

Patchset 4 (id:??) landed as
https://crrev.com/0219c9b4bfcbb778137756210eb95f40d936cc66
Cr-Commit-Position: refs/heads/master@{#10688}