Separating video settings in VideoQualityTest.

Separating video settings in VideoQualityTest.

This is a simple refactoring of VideoQualityTest. It will help in adding audio related settings to VideoQualityTest.

BUG=

Committed: https://crrev.com/f07fb0013164bdb031dcc88dc83365a27643b2d9
Cr-Commit-Position: refs/heads/master@{#14139}

[minyue-webrtc, 2016-09-05 12:43:45]

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

[commit-bot: I haz the power, 2016-09-05 12:43:56]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[minyue-webrtc, 2016-09-05 12:46:02]

Description was changed from

==========
refactoring video_quality_test

BUG=
==========

to

==========
Separating video settings in VideoQualityTest.

This is a refactoring of VideoQualityTest. It will help in adding audio related
settings to VideoQualityTest.

BUG=
==========

[minyue-webrtc, 2016-09-05 12:46:03]

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

[minyue-webrtc, 2016-09-05 12:46:22]

Description was changed from

==========
Separating video settings in VideoQualityTest.

This is a refactoring of VideoQualityTest. It will help in adding audio related
settings to VideoQualityTest.

BUG=
==========

to

==========
Separating video settings in VideoQualityTest.

This is a simple refactoring of VideoQualityTest. It will help in adding audio
related settings to VideoQualityTest.

BUG=
==========

[minyue-webrtc, 2016-09-05 12:55:21]

Hi Stefan,

This is a simple CL on the way toward adding audio only mode to video quality
test. PTAL. Thanks!

[commit-bot: I haz the power, 2016-09-05 14:27:47]

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

[commit-bot: I haz the power, 2016-09-05 14:27:47]

Dry run: This issue passed the CQ dry run.

[stefan-webrtc, 2016-09-07 13:33:33]

lgtm % nits.

https://codereview.webrtc.org/2312613003/diff/1/webrtc/video/full_stack.cc
File webrtc/video/full_stack.cc (right):

https://codereview.webrtc.org/2312613003/diff/1/webrtc/video/full_stack.cc#ne...
webrtc/video/full_stack.cc:28: foreman_cif.video = {true,   352,    288,   30,  
        700000,
Is this the formatting you get from git cl format? It doesn't look like the
alignment helps much.

https://codereview.webrtc.org/2312613003/diff/1/webrtc/video/video_quality_te...
File webrtc/video/video_quality_test.h (right):

https://codereview.webrtc.org/2312613003/diff/1/webrtc/video/video_quality_te...
webrtc/video/video_quality_test.h:33: } common;
I think common should either be called "call" or "bwe" to match the code
structure.

https://codereview.webrtc.org/2312613003/diff/1/webrtc/video/video_quality_te...
webrtc/video/video_quality_test.h:34: struct {  // Video-specific settings.
Remove comment, I think we're fine without it.

https://codereview.webrtc.org/2312613003/diff/1/webrtc/video/video_quality_te...
webrtc/video/video_quality_test.h:50: struct {  // Audio-specific
This comment isn't needed.

[minyue-webrtc, 2016-09-08 09:38:40]

https://codereview.webrtc.org/2312613003/diff/1/webrtc/video/full_stack.cc
File webrtc/video/full_stack.cc (right):

https://codereview.webrtc.org/2312613003/diff/1/webrtc/video/full_stack.cc#ne...
webrtc/video/full_stack.cc:28: foreman_cif.video = {true,   352,    288,   30,  
        700000,
On 2016/09/07 13:33:32, stefan-webrtc (holmer) wrote:
> Is this the formatting you get from git cl format? It doesn't look like the
> alignment helps much.

Right, git cl format did it. I'd manually reformat it. Thanks!

https://codereview.webrtc.org/2312613003/diff/1/webrtc/video/video_quality_te...
File webrtc/video/video_quality_test.h (right):

https://codereview.webrtc.org/2312613003/diff/1/webrtc/video/video_quality_te...
webrtc/video/video_quality_test.h:33: } common;
On 2016/09/07 13:33:32, stefan-webrtc (holmer) wrote:
> I think common should either be called "call" or "bwe" to match the code
> structure.

Done.

https://codereview.webrtc.org/2312613003/diff/1/webrtc/video/video_quality_te...
webrtc/video/video_quality_test.h:34: struct {  // Video-specific settings.
On 2016/09/07 13:33:32, stefan-webrtc (holmer) wrote:
> Remove comment, I think we're fine without it.

Done.

https://codereview.webrtc.org/2312613003/diff/1/webrtc/video/video_quality_te...
webrtc/video/video_quality_test.h:50: struct {  // Audio-specific
On 2016/09/07 13:33:32, stefan-webrtc (holmer) wrote:
> This comment isn't needed.

Done.

[minyue-webrtc, 2016-09-08 10:02:20]

The CQ bit was checked by minyue@webrtc.org

[minyue-webrtc, 2016-09-08 10:02:21]

The patchset sent to the CQ was uploaded after l-g-t-m from stefan@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2312613003/#ps20001 (title:
"on Stefan's comments")

[commit-bot: I haz the power, 2016-09-08 10:02:25]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-09-08 12:02:47]

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

[commit-bot: I haz the power, 2016-09-08 12:02:48]

Try jobs failed on following builders:
  android_arm64_rel on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  android_dbg on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  android_rel on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[minyue-webrtc, 2016-09-08 12:16:40]

The CQ bit was checked by minyue@webrtc.org

[commit-bot: I haz the power, 2016-09-08 12:16:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-09-08 14:17:10]

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

[commit-bot: I haz the power, 2016-09-08 14:17:10]

Exceeded global retry quota

[minyue-webrtc, 2016-09-08 15:03:06]

The CQ bit was checked by minyue@webrtc.org

[commit-bot: I haz the power, 2016-09-08 15:03:15]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-09-08 15:20:10]

Description was changed from

==========
Separating video settings in VideoQualityTest.

This is a simple refactoring of VideoQualityTest. It will help in adding audio
related settings to VideoQualityTest.

BUG=
==========

to

==========
Separating video settings in VideoQualityTest.

This is a simple refactoring of VideoQualityTest. It will help in adding audio
related settings to VideoQualityTest.

BUG=
==========

[commit-bot: I haz the power, 2016-09-08 15:20:11]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-08 15:20:19]

Description was changed from

==========
Separating video settings in VideoQualityTest.

This is a simple refactoring of VideoQualityTest. It will help in adding audio
related settings to VideoQualityTest.

BUG=
==========

to

==========
Separating video settings in VideoQualityTest.

This is a simple refactoring of VideoQualityTest. It will help in adding audio
related settings to VideoQualityTest.

BUG=

Committed: https://crrev.com/f07fb0013164bdb031dcc88dc83365a27643b2d9
Cr-Commit-Position: refs/heads/master@{#14139}
==========

[commit-bot: I haz the power, 2016-09-08 15:20:20]

Patchset 2 (id:??) landed as
https://crrev.com/f07fb0013164bdb031dcc88dc83365a27643b2d9
Cr-Commit-Position: refs/heads/master@{#14139}

[kjellander_webrtc, 2016-09-08 17:52:23]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.webrtc.org/2325723002/ by kjellander@webrtc.org.

The reason for reverting is: Breaks webrtc_perf_tests on Windows, Mac and Linux
(that test don't run on trybots):
https://build.chromium.org/p/client.webrtc/builders/Linux64%20Release%20%5Bla...

Example:
[ RUN      ] FullStackTest.ForemanCifWithoutPacketLossVp9

# Fatal error in ../../webrtc/video/video_quality_test.cc, line 1056
# last system error: 34
# Check failed: !params_.audio.enabled
.