Fix bug where rtcp::TransportFeedback may generate incorrect messages.

Fix bug where rtcp::TransportFeedback may generate incorrect messages.

In particular, if 14 short deltas were inserted (2 * capacity of status
vector chunk with 2bit items) followed by a large delta, that status
item would be dropped.

BUG=

Committed: https://crrev.com/49f9cdba02248d216dfc875dc0ab3c5ae187bc42
Cr-Commit-Position: refs/heads/master@{#10132}

[sprang_webrtc, 2015-09-25 14:49:46]

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

[sprang_webrtc, 2015-09-29 15:07:52]

Ping! This makes loopback tests sometimes crash :(

[stefan-webrtc, 2015-09-30 10:45:27]

https://codereview.webrtc.org/1367193002/diff/20001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback_unittest.cc
(right):

https://codereview.webrtc.org/1367193002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback_unittest.cc:457:
TEST(RtcpPacketTest, TransportFeedback_HeyWhatsGoingOn) {
There must be a better name than this? :)

https://codereview.webrtc.org/1367193002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback_unittest.cc:466:
for (int num_small_deltas = kTwoBitVectorCapacity - 1;
I don't follow why you don't start this counter on 0. Probably because I have
forgotten the details already... :/

https://codereview.webrtc.org/1367193002/diff/20001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc (right):

https://codereview.webrtc.org/1367193002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc:1042: // Send a
compound packet with a TransportFeedback followed by something else.
Where's the "followed by something else" part? I only see a transport feedback
being sent here?

https://codereview.webrtc.org/1367193002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc:1086:
&built_packet->MutableBuffer()[kStatusCountOffset], 42);
Sounds like we may want to fuzz this code. Maybe ask pbos if he could help out?

[sprang_webrtc, 2015-09-30 11:09:27]

https://codereview.webrtc.org/1367193002/diff/20001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback_unittest.cc
(right):

https://codereview.webrtc.org/1367193002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback_unittest.cc:457:
TEST(RtcpPacketTest, TransportFeedback_HeyWhatsGoingOn) {
On 2015/09/30 10:45:27, stefan-webrtc (holmer) wrote:
> There must be a better name than this? :)

Oops. Fixed. :)

https://codereview.webrtc.org/1367193002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback_unittest.cc:466:
for (int num_small_deltas = kTwoBitVectorCapacity - 1;
On 2015/09/30 10:45:27, stefan-webrtc (holmer) wrote:
> I don't follow why you don't start this counter on 0. Probably because I have
> forgotten the details already... :/

Nah, only that that range wasn't quite so suspect. Originally I only tested the
one case that triggered the crash bug. Can change it to 0 just to be sure.

https://codereview.webrtc.org/1367193002/diff/20001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc (right):

https://codereview.webrtc.org/1367193002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc:1042: // Send a
compound packet with a TransportFeedback followed by something else.
On 2015/09/30 10:45:27, stefan-webrtc (holmer) wrote:
> Where's the "followed by something else" part? I only see a transport feedback
> being sent here?

Copy/paste mistake. Removed comment.

https://codereview.webrtc.org/1367193002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc:1086:
&built_packet->MutableBuffer()[kStatusCountOffset], 42);
On 2015/09/30 10:45:27, stefan-webrtc (holmer) wrote:
> Sounds like we may want to fuzz this code. Maybe ask pbos if he could help
out?

Acknowledged.

[stefan-webrtc, 2015-10-01 08:52:39]

lgtm

[stefan-webrtc, 2015-10-01 08:52:40]

lgtm

lgtm

[sprang_webrtc, 2015-10-01 08:54:11]

The CQ bit was checked by sprang@webrtc.org

[commit-bot: I haz the power, 2015-10-01 08:54:18]

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

[commit-bot: I haz the power, 2015-10-01 10:07:02]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-10-01 10:07:11]

Patchset 3 (id:??) landed as
https://crrev.com/49f9cdba02248d216dfc875dc0ab3c5ae187bc42
Cr-Commit-Position: refs/heads/master@{#10132}