[rtp_rtcp] rtcp::Fir cleaned and got Parse function.

class doesn't rely on structures in RTCPUtility to store data.
supports several fci items in same packet.
got accessors to read data

BUG=webrtc:5260
R=asapersson@webrtc.org, åsapersson

Committed: https://chromium.googlesource.com/external/webrtc/+/32e590ec132ae1a22b92529c9ecca9b6563985ca

[danilchap, 2015-12-28 16:09:09]

Description was changed from

==========
[rtp_rtc] rtcp::Fir packet moved into own file and got Parse function.

BUG=webrtc:5260
R=åsapersson
==========

to

==========
[rtp_rtcp] rtcp::Fir packet moved into own file and got Parse function.

BUG=webrtc:5260
R=åsapersson
==========

[danilchap, 2016-01-15 15:07:50]

Description was changed from

==========
[rtp_rtcp] rtcp::Fir packet moved into own file and got Parse function.

BUG=webrtc:5260
R=åsapersson
==========

to

==========
[rtp_rtcp] rtcp::Fir packet cleaned and got Parse function.
class doesn't rely on structures in RTCPUtility to store data.
supports several fci items in same packet.
got accessors to read data 

BUG=webrtc:5260
R=åsapersson
==========

[danilchap, 2016-01-15 15:08:06]

Description was changed from

==========
[rtp_rtcp] rtcp::Fir packet cleaned and got Parse function.
class doesn't rely on structures in RTCPUtility to store data.
supports several fci items in same packet.
got accessors to read data 

BUG=webrtc:5260
R=åsapersson
==========

to

==========
class doesn't rely on structures in RTCPUtility to store data.
supports several fci items in same packet.
got accessors to read data 

BUG=webrtc:5260
R=åsapersson
==========

[åsapersson, 2016-01-21 16:04:35]

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

https://codereview.webrtc.org/1544403002/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet.h:41: //  fir.WithRequestTo(234,
123);
maybe change sequence number 123 to something else to not confuse with From(123)

https://codereview.webrtc.org/1544403002/diff/60001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/fir.cc (right):

https://codereview.webrtc.org/1544403002/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/fir.cc:22: // Full intra request
(FIR) (RFC 5104).
Move this comment to after line 36.

https://codereview.webrtc.org/1544403002/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/fir.cc:80:
RTC_CHECK(!items_.empty());
maybe use DCHECK?

https://codereview.webrtc.org/1544403002/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/fir.cc:88:
RTC_CHECK_EQ(Psfb::media_ssrc(), 0u);
ditto

https://codereview.webrtc.org/1544403002/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/fir.cc:99: RTC_CHECK_EQ(*index,
index_end);
ditto

https://codereview.webrtc.org/1544403002/diff/60001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/fir.h (right):

https://codereview.webrtc.org/1544403002/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/fir.h:57: void To(size_t ssrc);
size_t->uint32

https://codereview.webrtc.org/1544403002/diff/60001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/fir_unittest.cc (right):

https://codereview.webrtc.org/1544403002/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/fir_unittest.cc:40: bool
ParseFir(const uint8_t* buffer, size_t length, Fir* parsed) {
perhaps parsed->fir

[danilchap, 2016-01-21 16:48:13]

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

https://codereview.webrtc.org/1544403002/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet.h:41: //  fir.WithRequestTo(234,
123);
On 2016/01/21 16:04:35, åsapersson wrote:
> maybe change sequence number 123 to something else to not confuse with
From(123)

Done.

https://codereview.webrtc.org/1544403002/diff/60001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/fir.cc (right):

https://codereview.webrtc.org/1544403002/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/fir.cc:22: // Full intra request
(FIR) (RFC 5104).
On 2016/01/21 16:04:35, åsapersson wrote:
> Move this comment to after line 36.

Done.

https://codereview.webrtc.org/1544403002/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/fir.cc:80:
RTC_CHECK(!items_.empty());
On 2016/01/21 16:04:35, åsapersson wrote:
> maybe use DCHECK?
agree DCHECK is better: this line is more a documentation than a real check.

https://codereview.webrtc.org/1544403002/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/fir.cc:88:
RTC_CHECK_EQ(Psfb::media_ssrc(), 0u);
On 2016/01/21 16:04:35, åsapersson wrote:
> ditto

Done.

https://codereview.webrtc.org/1544403002/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/fir.cc:99: RTC_CHECK_EQ(*index,
index_end);
On 2016/01/21 16:04:35, åsapersson wrote:
> ditto

if *index != index_end I would rather crash right away:
*index > index_end might mean buffer overflow.
*index < index_end is not that harmful, but still indicate nasty mistake in the
code.

https://codereview.webrtc.org/1544403002/diff/60001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/fir.h (right):

https://codereview.webrtc.org/1544403002/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/fir.h:57: void To(size_t ssrc);
On 2016/01/21 16:04:35, åsapersson wrote:
> size_t->uint32

Oops.

https://codereview.webrtc.org/1544403002/diff/60001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/fir_unittest.cc (right):

https://codereview.webrtc.org/1544403002/diff/60001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/fir_unittest.cc:40: bool
ParseFir(const uint8_t* buffer, size_t length, Fir* parsed) {
On 2016/01/21 16:04:35, åsapersson wrote:
> perhaps parsed->fir

Done, fir is nicer.

[åsapersson, 2016-01-22 07:32:23]

lgtm

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

https://codereview.webrtc.org/1544403002/diff/80001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet.h:44: //  uint8_t
packet[kPacketSize];           // with sequence number 123.
please also change 123 here

[danilchap, 2016-01-22 10:03:42]

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

https://codereview.webrtc.org/1544403002/diff/80001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet.h:44: //  uint8_t
packet[kPacketSize];           // with sequence number 123.
On 2016/01/22 07:32:23, åsapersson wrote:
> please also change 123 here

Done.

[commit-bot: I haz the power, 2016-01-22 10:05:14]

Description was changed from

==========
class doesn't rely on structures in RTCPUtility to store data.
supports several fci items in same packet.
got accessors to read data 

BUG=webrtc:5260
R=åsapersson
==========

to

==========
class doesn't rely on structures in RTCPUtility to store data.
supports several fci items in same packet.
got accessors to read data

BUG=webrtc:5260
R=asapersson@webrtc.org, åsapersson

Committed: https://crrev.com/32e590ec132ae1a22b92529c9ecca9b6563985ca
Cr-Commit-Position: refs/heads/master@{#11354}
==========

[danilchap, 2016-01-22 10:05:14]

Description was changed from

==========
class doesn't rely on structures in RTCPUtility to store data.
supports several fci items in same packet.
got accessors to read data

BUG=webrtc:5260
R=asapersson@webrtc.org, åsapersson

Committed: https://crrev.com/32e590ec132ae1a22b92529c9ecca9b6563985ca
Cr-Commit-Position: refs/heads/master@{#11354}
==========

to

==========
class doesn't rely on structures in RTCPUtility to store data.
supports several fci items in same packet.
got accessors to read data

BUG=webrtc:5260
R=asapersson@webrtc.org, åsapersson

Committed:
https://chromium.googlesource.com/external/webrtc/+/32e590ec132ae1a22b92529c9...
==========

[commit-bot: I haz the power, 2016-01-22 10:05:14]

Patchset 6 (id:??) landed as
https://crrev.com/32e590ec132ae1a22b92529c9ecca9b6563985ca
Cr-Commit-Position: refs/heads/master@{#11354}

[danilchap, 2016-01-22 10:05:15]

Committed patchset #6 (id:100001) manually as
32e590ec132ae1a22b92529c9ecca9b6563985ca (presubmit successful).