Added send stream test case for VP9 header.

Added send stream test case for VP9 header.

BUG=webrtc:4914
Committed: https://crrev.com/0f9af0145683b9021ffe14da20c175bfa9db5cab
Cr-Commit-Position: refs/heads/master@{#9831}

[philipel, 2015-08-13 12:44:52]

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

[sprang_webrtc, 2015-08-13 13:11:13]

Looks good. A few remarks.

https://codereview.webrtc.org/1288363002/diff/1/webrtc/video/video_send_strea...
File webrtc/video/video_send_stream_tests.cc (right):

https://codereview.webrtc.org/1288363002/diff/1/webrtc/video/video_send_strea...
webrtc/video/video_send_stream_tests.cc:1808: virtual void
InspectHeader(RTPVideoHeaderVP9* vp9videoHeader) {}
Would prefer to make this pure virtual.

https://codereview.webrtc.org/1288363002/diff/1/webrtc/video/video_send_strea...
webrtc/video/video_send_stream_tests.cc:1834: const uint8_t* vp9headerAt =
packet + header.headerLength;
Not sure about this name. vp9_data or vp9_packet perhaps?

https://codereview.webrtc.org/1288363002/diff/1/webrtc/video/video_send_strea...
webrtc/video/video_send_stream_tests.cc:1843: EXPECT_TRUE(is_vp9_codec_type);
I don't think these temporaries are really necessary. Sure, there may be an
extra failure message from the InspectHeader method, but that shouldn't be a big
deal. 

Just "EXPECT_TRUE(vp9depacketizer.Parse(&vp9payload, vp9headerAt,
payload_length));" is simpler and will on failure print the actual expression
that failed, without having to look up in the source code how exactly
parse_vp9header_successful was set.

https://codereview.webrtc.org/1288363002/diff/1/webrtc/video/video_send_strea...
webrtc/video/video_send_stream_tests.cc:1863:
EXPECT_TRUE(vp9videoHeader->flexible_mode == false);
EXPECT_FALSE(vp9videoHeader->flexible_mode);

https://codereview.webrtc.org/1288363002/diff/1/webrtc/video/video_send_strea...
webrtc/video/video_send_stream_tests.cc:1871: // TODO(philipel): enable once
flexible mode is implemeted
Nit: Capital E and end with a period.

https://codereview.webrtc.org/1288363002/diff/1/webrtc/video/video_send_strea...
webrtc/video/video_send_stream_tests.cc:1884:
EXPECT_TRUE(vp9videoHeader->flexible_mode == true);
EXPECT_TRUE(vp9videoHeader->flexible_mode);

[philipel, 2015-08-13 14:22:40]

https://codereview.webrtc.org/1288363002/diff/1/webrtc/video/video_send_strea...
File webrtc/video/video_send_stream_tests.cc (right):

https://codereview.webrtc.org/1288363002/diff/1/webrtc/video/video_send_strea...
webrtc/video/video_send_stream_tests.cc:1808: virtual void
InspectHeader(RTPVideoHeaderVP9* vp9videoHeader) {}
On 2015/08/13 13:11:13, språng wrote:
> Would prefer to make this pure virtual.

Done.

https://codereview.webrtc.org/1288363002/diff/1/webrtc/video/video_send_strea...
webrtc/video/video_send_stream_tests.cc:1834: const uint8_t* vp9headerAt =
packet + header.headerLength;
On 2015/08/13 13:11:13, språng wrote:
> Not sure about this name. vp9_data or vp9_packet perhaps?

Done.

https://codereview.webrtc.org/1288363002/diff/1/webrtc/video/video_send_strea...
webrtc/video/video_send_stream_tests.cc:1843: EXPECT_TRUE(is_vp9_codec_type);
On 2015/08/13 13:11:13, språng wrote:
> I don't think these temporaries are really necessary. Sure, there may be an
> extra failure message from the InspectHeader method, but that shouldn't be a
big
> deal. 
> 
> Just "EXPECT_TRUE(vp9depacketizer.Parse(&vp9payload, vp9headerAt,
> payload_length));" is simpler and will on failure print the actual expression
> that failed, without having to look up in the source code how exactly
> parse_vp9header_successful was set. 

If the result is not saved then it's not possible to check if the InspectHeader
function should be called or not. But maybe subsequent errors doesn't matter.

https://codereview.webrtc.org/1288363002/diff/1/webrtc/video/video_send_strea...
webrtc/video/video_send_stream_tests.cc:1863:
EXPECT_TRUE(vp9videoHeader->flexible_mode == false);
On 2015/08/13 13:11:13, språng wrote:
> EXPECT_FALSE(vp9videoHeader->flexible_mode);

Done.

https://codereview.webrtc.org/1288363002/diff/1/webrtc/video/video_send_strea...
webrtc/video/video_send_stream_tests.cc:1871: // TODO(philipel): enable once
flexible mode is implemeted
On 2015/08/13 13:11:13, språng wrote:
> Nit: Capital E and end with a period.

Done.

https://codereview.webrtc.org/1288363002/diff/1/webrtc/video/video_send_strea...
webrtc/video/video_send_stream_tests.cc:1884:
EXPECT_TRUE(vp9videoHeader->flexible_mode == true);
On 2015/08/13 13:11:13, språng wrote:
> EXPECT_TRUE(vp9videoHeader->flexible_mode);

Done.

[sprang_webrtc, 2015-08-13 16:30:16]

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

[sprang_webrtc, 2015-08-13 16:30:17]

lgtm
+mflodman, ptal

[mflodman, 2015-08-31 15:20:06]

LGTM

[philipel, 2015-09-01 12:34:12]

The CQ bit was checked by philipel@webrtc.org

[commit-bot: I haz the power, 2015-09-01 12:34:15]

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

[commit-bot: I haz the power, 2015-09-01 14:01:54]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-09-01 14:02:03]

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