VEA unittest: remove quality test on chromeos temporarily

VEA unittest: remove quality test on chromeos temporarily

Currently encode result on chromeos is nondeterministic. Disable it
temporariliy to prevent it hide new regressions.

BUG=694131
TEST=SimpleEncode/VideoEncodeAcceleratorTest.TestSimpleEncode/1 no longer run on chromeos
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2758203002
Cr-Commit-Position: refs/heads/master@{#458027}
Committed: https://chromium.googlesource.com/chromium/src/+/f4292e56c05fc2ad1710c4b691923a40a4d115f3

[kcwu, 2017-03-20 02:54:16]

Description was changed from

==========
VEA unittest: remove quality test on chromeos temporarily

Currently encode result on chromeos is nondeterministic. Disable it
temporariliy to prevent it hide new regressions.

BUG=694131
TEST=SimpleEncode/VideoEncodeAcceleratorTest.TestSimpleEncode/1 no longer run on
chromeos
==========

to

==========
VEA unittest: remove quality test on chromeos temporarily

Currently encode result on chromeos is nondeterministic. Disable it
temporariliy to prevent it hide new regressions.

BUG=694131
TEST=SimpleEncode/VideoEncodeAcceleratorTest.TestSimpleEncode/1 no longer run on
chromeos
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[kcwu, 2017-03-20 02:55:27]

kcwu@chromium.org changed reviewers:
+ owenlin@chromium.org, posciak@chromium.org, wuchengli@chromium.org

[kcwu, 2017-03-20 02:55:27]

p.s. bitrate tests will be disabled in a separate CL.

[Owen Lin, 2017-03-20 03:14:57]

https://codereview.chromium.org/2758203002/diff/1/media/gpu/video_encode_acce...
File media/gpu/video_encode_accelerator_unittest.cc (right):

https://codereview.chromium.org/2758203002/diff/1/media/gpu/video_encode_acce...
media/gpu/video_encode_accelerator_unittest.cc:2146: // TODO(kcwu): add back
test of verify_output=true after crbug.com/694131 fixed.
How about file a new bug for adding this back?

https://codereview.chromium.org/2758203002/diff/1/media/gpu/video_encode_acce...
media/gpu/video_encode_accelerator_unittest.cc:2151: std::make_tuple(1, true, 0,
false, false, false, false, false, false)));
Why not filter out the test in autotest? My point is the test is still valid and
maybe we can use it for debugging/verification.

[kcwu, 2017-03-20 03:33:29]

https://codereview.chromium.org/2758203002/diff/1/media/gpu/video_encode_acce...
File media/gpu/video_encode_accelerator_unittest.cc (right):

https://codereview.chromium.org/2758203002/diff/1/media/gpu/video_encode_acce...
media/gpu/video_encode_accelerator_unittest.cc:2146: // TODO(kcwu): add back
test of verify_output=true after crbug.com/694131 fixed.
On 2017/03/20 03:14:57, Owen Lin wrote:
> How about file a new bug for adding this back?

My plan is keeping 694131 open, so unnecessary to file a new bug.

https://codereview.chromium.org/2758203002/diff/1/media/gpu/video_encode_acce...
media/gpu/video_encode_accelerator_unittest.cc:2151: std::make_tuple(1, true, 0,
false, false, false, false, false, false)));
On 2017/03/20 03:14:57, Owen Lin wrote:
> Why not filter out the test in autotest? My point is the test is still valid
and
> maybe we can use it for debugging/verification.

I am also considering filter out in autotest. My reasons to disable it here are:
1. This test is not named explicitly. Its name is "..../1" with index. The index
may be out of sync between veatest and autotest.
2. It seems nice (?) to fix the bug and enable the test later in the same CL.

What do you think?

[Owen Lin, 2017-03-20 06:53:02]

https://codereview.chromium.org/2758203002/diff/1/media/gpu/video_encode_acce...
File media/gpu/video_encode_accelerator_unittest.cc (right):

https://codereview.chromium.org/2758203002/diff/1/media/gpu/video_encode_acce...
media/gpu/video_encode_accelerator_unittest.cc:2146: // TODO(kcwu): add back
test of verify_output=true after crbug.com/694131 fixed.
On 2017/03/20 03:33:29, kcwu wrote:
> On 2017/03/20 03:14:57, Owen Lin wrote:
> > How about file a new bug for adding this back?
> 
> My plan is keeping 694131 open, so unnecessary to file a new bug.

It just makes things clear. Maybe we will assign the bug to partner.

I mean, if you're going to fix the issue, I am fine with that. If not, I would
suggest to have a dedicate bug for adding back.

https://codereview.chromium.org/2758203002/diff/1/media/gpu/video_encode_acce...
media/gpu/video_encode_accelerator_unittest.cc:2151: std::make_tuple(1, true, 0,
false, false, false, false, false, false)));
On 2017/03/20 03:33:29, kcwu wrote:
> On 2017/03/20 03:14:57, Owen Lin wrote:
> > Why not filter out the test in autotest? My point is the test is still valid
> and
> > maybe we can use it for debugging/verification.
> 
> I am also considering filter out in autotest. My reasons to disable it here
are:
> 1. This test is not named explicitly. Its name is "..../1" with index. The
index
> may be out of sync between veatest and autotest.
> 2. It seems nice (?) to fix the bug and enable the test later in the same CL.
> 
> What do you think?

Acknowledged. I think (1) is a good reason. But about (2), do we know the root
cause to the issue, maybe the root cause is not in chromium source code.

[kcwu, 2017-03-20 07:11:45]

https://codereview.chromium.org/2758203002/diff/1/media/gpu/video_encode_acce...
File media/gpu/video_encode_accelerator_unittest.cc (right):

https://codereview.chromium.org/2758203002/diff/1/media/gpu/video_encode_acce...
media/gpu/video_encode_accelerator_unittest.cc:2146: // TODO(kcwu): add back
test of verify_output=true after crbug.com/694131 fixed.
On 2017/03/20 06:53:02, Owen Lin wrote:
> On 2017/03/20 03:33:29, kcwu wrote:
> > On 2017/03/20 03:14:57, Owen Lin wrote:
> > > How about file a new bug for adding this back?
> > 
> > My plan is keeping 694131 open, so unnecessary to file a new bug.
> 
> It just makes things clear. Maybe we will assign the bug to partner.
> 
> I mean, if you're going to fix the issue, I am fine with that. If not, I would
> suggest to have a dedicate bug for adding back.

I filed a bug to add it back.
https://bugs.chromium.org/p/chromium/issues/detail?id=703039

[Owen Lin, 2017-03-20 07:27:22]

lgtm

[wuchengli, 2017-03-20 07:56:19]

lgtm

[kcwu, 2017-03-20 07:56:55]

The CQ bit was checked by kcwu@chromium.org

[commit-bot: I haz the power, 2017-03-20 07:57:04]

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

[commit-bot: I haz the power, 2017-03-20 08:58:16]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1489996615908380, "parent_rev":
"d8cc69507968d87cea7eeefc39c0dae78f960879", "commit_rev":
"f4292e56c05fc2ad1710c4b691923a40a4d115f3"}

[commit-bot: I haz the power, 2017-03-20 08:59:28]

Description was changed from

==========
VEA unittest: remove quality test on chromeos temporarily

Currently encode result on chromeos is nondeterministic. Disable it
temporariliy to prevent it hide new regressions.

BUG=694131
TEST=SimpleEncode/VideoEncodeAcceleratorTest.TestSimpleEncode/1 no longer run on
chromeos
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
VEA unittest: remove quality test on chromeos temporarily

Currently encode result on chromeos is nondeterministic. Disable it
temporariliy to prevent it hide new regressions.

BUG=694131
TEST=SimpleEncode/VideoEncodeAcceleratorTest.TestSimpleEncode/1 no longer run on
chromeos
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2758203002
Cr-Commit-Position: refs/heads/master@{#458027}
Committed:
https://chromium.googlesource.com/chromium/src/+/f4292e56c05fc2ad1710c4b69192...
==========

[commit-bot: I haz the power, 2017-03-20 08:59:28]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/f4292e56c05fc2ad1710c4b69192...