Add FEC producer fuzzing and a unittest for one of the issues found.

Add FEC producer fuzzing and a unittest for one of the issues found.

BUG=webrtc:4800
R=pbos@webrtc.org

Committed: https://crrev.com/4c1093b86f4d0a1c8ade68a4b6a411b2674deac8
Cr-Commit-Position: refs/heads/master@{#10990}

[stefan-webrtc, 2015-12-11 12:45:04]

Removed unused code

[stefan-webrtc, 2015-12-11 13:08:41]

Fixed fuzzer to have reasonable sequence numbers

[stefan-webrtc, 2015-12-11 13:08:55]

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

[pbos-webrtc, 2015-12-11 13:12:20]

Description was changed from

==========
Add FEC producer fuzzing and a unittest for one of the issues found.

BUG=4800
==========

to

==========
Add FEC producer fuzzing and a unittest for one of the issues found.

BUG=webrtc:4800
R=pbos@webrtc.org
==========

[pbos-webrtc, 2015-12-11 13:31:45]

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

https://codereview.webrtc.org/1522463002/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc:74: FuzzOneInput(data,
sizeof(data));
Could you unpack this test to have the actual meaning of the data? If you change
the fuzzer in any way you lose test coverage now.

Also it confuses me that this test even links with FuzzOneInput right now? I
don't think the test name makes sense either. Is this test case for a large
sequence-number gap? If so do that? :)

[stefan-webrtc, 2015-12-11 14:25:58]

Comment addressed, test updated.

[stefan-webrtc, 2015-12-11 14:26:27]

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

https://codereview.webrtc.org/1522463002/diff/40001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc:74: FuzzOneInput(data,
sizeof(data));
On 2015/12/11 13:31:45, pbos-webrtc wrote:
> Could you unpack this test to have the actual meaning of the data? If you
change
> the fuzzer in any way you lose test coverage now.
> 
> Also it confuses me that this test even links with FuzzOneInput right now? I
> don't think the test name makes sense either. Is this test case for a large
> sequence-number gap? If so do that? :)

Done.

[stefan-webrtc, 2015-12-11 14:26:41]

The CQ bit was checked by stefan@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2015-12-11 14:26:49]

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

[commit-bot: I haz the power, 2015-12-11 14:27:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-11 14:27:36]

Dry run: Try jobs failed on following builders:
  android_gn_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_gn_rel/builds/7631)
  ios_arm64_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/6252)
  ios_arm64_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_rel/builds/6182)
  ios_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/11418)
  ios_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_rel/builds/10075)
  mac_compile_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_compile_dbg/builds/...)
  mac_compile_x64_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_compile_x64_dbg/bui...)
  mac_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_rel/builds/11503)
  mac_x64_gn_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_x64_gn_dbg/builds/5904)
  mac_x64_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_x64_rel/builds/11125)

[stefan-webrtc, 2015-12-11 14:34:26]

merge

[stefan-webrtc, 2015-12-11 14:36:07]

The CQ bit was checked by stefan@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2015-12-11 14:36:17]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1522463002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1522463002/80001

[pbos-webrtc, 2015-12-11 14:42:04]

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

https://codereview.webrtc.org/1522463002/diff/80001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc:67:
protected_packets.push_back({15, 3, 41, 0});
Can this list be minimized? :)

https://codereview.webrtc.org/1522463002/diff/80001/webrtc/test/fuzzers/produ...
File webrtc/test/fuzzers/producer_fec_fuzzer.cc (right):

https://codereview.webrtc.org/1522463002/diff/80001/webrtc/test/fuzzers/produ...
webrtc/test/fuzzers/producer_fec_fuzzer.cc:45: const bool marker_bit =
(packet[1] & 0x80) ? true : false;
unused?

[stefan-webrtc, 2015-12-11 14:50:02]

Reduced test

[stefan-webrtc, 2015-12-11 14:50:50]

Remove unused variable

[stefan-webrtc, 2015-12-11 14:50:57]

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

https://codereview.webrtc.org/1522463002/diff/80001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc:67:
protected_packets.push_back({15, 3, 41, 0});
On 2015/12/11 14:42:04, pbos-webrtc wrote:
> Can this list be minimized? :)

Somewhat.

https://codereview.webrtc.org/1522463002/diff/80001/webrtc/test/fuzzers/produ...
File webrtc/test/fuzzers/producer_fec_fuzzer.cc (right):

https://codereview.webrtc.org/1522463002/diff/80001/webrtc/test/fuzzers/produ...
webrtc/test/fuzzers/producer_fec_fuzzer.cc:45: const bool marker_bit =
(packet[1] & 0x80) ? true : false;
On 2015/12/11 14:42:04, pbos-webrtc wrote:
> unused?

Done.

[stefan-webrtc, 2015-12-11 15:00:41]

Added comment

[pbos-webrtc, 2015-12-11 15:01:25]

lgtm

[stefan-webrtc, 2015-12-11 15:02:40]

The CQ bit was checked by stefan@webrtc.org

[commit-bot: I haz the power, 2015-12-11 15:02:51]

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

[commit-bot: I haz the power, 2015-12-11 17:03:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-11 17:03:24]

Try jobs failed on following builders:
  android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  linux_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  mac_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  win_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[pbos-webrtc, 2015-12-11 17:12:20]

The CQ bit was checked by pbos@webrtc.org

[commit-bot: I haz the power, 2015-12-11 17:12:30]

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

[stefan-webrtc, 2015-12-11 17:26:03]

Description was changed from

==========
Add FEC producer fuzzing and a unittest for one of the issues found.

BUG=webrtc:4800
R=pbos@webrtc.org
==========

to

==========
Add FEC producer fuzzing and a unittest for one of the issues found.

BUG=webrtc:4800
R=pbos@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/4c1093b86f4d0a1c8ade68a4b...
==========

[commit-bot: I haz the power, 2015-12-11 17:26:03]

Description was changed from

==========
Add FEC producer fuzzing and a unittest for one of the issues found.

BUG=webrtc:4800
R=pbos@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/4c1093b86f4d0a1c8ade68a4b...
==========

to

==========
Add FEC producer fuzzing and a unittest for one of the issues found.

BUG=webrtc:4800
R=pbos@webrtc.org

Committed: https://crrev.com/4c1093b86f4d0a1c8ade68a4b6a411b2674deac8
Cr-Commit-Position: refs/heads/master@{#10990}
==========

[commit-bot: I haz the power, 2015-12-11 17:26:04]

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

[stefan-webrtc, 2015-12-11 17:26:04]

Committed patchset #8 (id:140001) manually as
4c1093b86f4d0a1c8ade68a4b6a411b2674deac8 (presubmit successful).