Add Gpu blacklist entry for video encoding on Linux.

Add Gpu blacklist entry for video encoding on Linux.

On Linux, Video encoding is currently erroneously reported on chrome://gpu as "Hardware 
accelerated". This CL fixes this by adding a blacklist entry.

BUG=641485
TEST=Open chrome://gpu on Linux and check that Video Encode is reported as
     Software only, hardware acceleration unavailable
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/2744683002
Cr-Commit-Position: refs/heads/master@{#456153}
Committed: https://chromium.googlesource.com/chromium/src/+/31ec638091fc05bd13c41f8cd19cae7479cd5a1c

[chfremer, 2017-03-09 19:37:50]

Description was changed from

==========
Add Gpu blacklist entry for video encoding on Linux.

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

to

==========
Add Gpu blacklist entry for video encoding on Linux.

BUG=
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
==========

[chfremer, 2017-03-09 19:38:37]

Description was changed from

==========
Add Gpu blacklist entry for video encoding on Linux.

BUG=
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

==========
Add Gpu blacklist entry for video encoding on Linux.

BUG=641485
TEST=Open chrome://gpu on Linux and check that Video Encode is reported as
     Software only, hardware acceleration unavailable
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
==========

[chfremer, 2017-03-09 19:43:42]

Description was changed from

==========
Add Gpu blacklist entry for video encoding on Linux.

BUG=641485
TEST=Open chrome://gpu on Linux and check that Video Encode is reported as
     Software only, hardware acceleration unavailable
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

==========
Add Gpu blacklist entry for video encoding on Linux.

On Linux, Video encoding is currently erroneously reported on chrome://gpu as
"Hardware 
accelerated". This CL fixes this by adding a blacklist entry.

BUG=641485
TEST=Open chrome://gpu on Linux and check that Video Encode is reported as
     Software only, hardware acceleration unavailable
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
==========

[chfremer, 2017-03-09 19:43:48]

The CQ bit was checked by chfremer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-09 19:44:36]

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

[chfremer, 2017-03-09 19:45:01]

chfremer@chromium.org changed reviewers:
+ emircan@chromium.org, zmo@chromium.org

[chfremer, 2017-03-09 19:45:02]

zmo@: Please do an owner's review
emircan@: FYI

[emircan, 2017-03-09 20:21:43]

lgtm

[commit-bot: I haz the power, 2017-03-09 20:38:22]

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

[commit-bot: I haz the power, 2017-03-09 20:38:23]

Dry run: This issue passed the CQ dry run.

[Zhenyao Mo, 2017-03-09 21:49:33]

My question is: we report it wrong, but still act correctly.  So where is the
logic?  We should remove that logic and hook it up with blacklist decision.

https://codereview.chromium.org/2744683002/diff/1/gpu/config/software_renderi...
File gpu/config/software_rendering_list_json.cc (right):

https://codereview.chromium.org/2744683002/diff/1/gpu/config/software_renderi...
gpu/config/software_rendering_list_json.cc:409: "id": 49,
No, please don't reuse the ID, that will mess up histograms. Please move this to
the end of file and use the last id + 1.

[chfremer, 2017-03-09 22:25:10]

On 2017/03/09 21:49:33, Zhenyao Mo wrote:
> My question is: we report it wrong, but still act correctly.  So where is the
> logic?  We should remove that logic and hook it up with blacklist decision.

Good point. The logic that determines that accelerated video encode is not
supported on Linux is located at 
https://cs.chromium.org/chromium/src/media/gpu/ipc/service/gpu_video_encode_a...

Note, that it is the non-existence of a factory method rather than a decision to
actively suppress something that determines how we act. Therefore, we cannot
simply replace this logic with the blacklist decision.

IMHO, the clean solution would be to have the reporting module ask the feature
implementing module whether or not the feature is supported. But it looks like
that would be a significant departure from how the reporting feature appears to
be designed to be used, i.e. it would be a hack unless we changed the overall
design of the reporting feature. Adding a blacklist entry, on the other hand,
appears to be in line with how the feature was designed to be used.

[chfremer, 2017-03-09 22:25:35]

https://codereview.chromium.org/2744683002/diff/1/gpu/config/software_renderi...
File gpu/config/software_rendering_list_json.cc (right):

https://codereview.chromium.org/2744683002/diff/1/gpu/config/software_renderi...
gpu/config/software_rendering_list_json.cc:409: "id": 49,
On 2017/03/09 21:49:32, Zhenyao Mo wrote:
> No, please don't reuse the ID, that will mess up histograms. Please move this
to
> the end of file and use the last id + 1.

Done.

A question I was having while doing so is if there is any guarantee that last id
+ 1 has not been used before. Even though quite unlikely, could it happen that
someone removes the last entry and then that id gets reused the next time
someone adds something?

[Zhenyao Mo, 2017-03-09 22:30:02]

On 2017/03/09 22:25:10, chfremer wrote:
> On 2017/03/09 21:49:33, Zhenyao Mo wrote:
> > My question is: we report it wrong, but still act correctly.  So where is
the
> > logic?  We should remove that logic and hook it up with blacklist decision.
> 
> Good point. The logic that determines that accelerated video encode is not
> supported on Linux is located at 
>
https://cs.chromium.org/chromium/src/media/gpu/ipc/service/gpu_video_encode_a...
> 
> Note, that it is the non-existence of a factory method rather than a decision
to
> actively suppress something that determines how we act. Therefore, we cannot
> simply replace this logic with the blacklist decision.
> 
> IMHO, the clean solution would be to have the reporting module ask the feature
> implementing module whether or not the feature is supported. But it looks like
> that would be a significant departure from how the reporting feature appears
to
> be designed to be used, i.e. it would be a hack unless we changed the overall
> design of the reporting feature. Adding a blacklist entry, on the other hand,
> appears to be in line with how the feature was designed to be used.

I see. Thanks.

[Zhenyao Mo, 2017-03-09 22:30:55]

On 2017/03/09 22:25:35, chfremer wrote:
>
https://codereview.chromium.org/2744683002/diff/1/gpu/config/software_renderi...
> File gpu/config/software_rendering_list_json.cc (right):
> 
>
https://codereview.chromium.org/2744683002/diff/1/gpu/config/software_renderi...
> gpu/config/software_rendering_list_json.cc:409: "id": 49,
> On 2017/03/09 21:49:32, Zhenyao Mo wrote:
> > No, please don't reuse the ID, that will mess up histograms. Please move
this
> to
> > the end of file and use the last id + 1.
> 
> Done.
> 
> A question I was having while doing so is if there is any guarantee that last
id
> + 1 has not been used before. Even though quite unlikely, could it happen that
> someone removes the last entry and then that id gets reused the next time
> someone adds something?

Yes, that's possible but never happened before.  Usually it took sometime before
we can remove an entry, and usually more entries have been added since.

[Zhenyao Mo, 2017-03-09 22:30:56]

On 2017/03/09 22:25:35, chfremer wrote:
>
https://codereview.chromium.org/2744683002/diff/1/gpu/config/software_renderi...
> File gpu/config/software_rendering_list_json.cc (right):
> 
>
https://codereview.chromium.org/2744683002/diff/1/gpu/config/software_renderi...
> gpu/config/software_rendering_list_json.cc:409: "id": 49,
> On 2017/03/09 21:49:32, Zhenyao Mo wrote:
> > No, please don't reuse the ID, that will mess up histograms. Please move
this
> to
> > the end of file and use the last id + 1.
> 
> Done.
> 
> A question I was having while doing so is if there is any guarantee that last
id
> + 1 has not been used before. Even though quite unlikely, could it happen that
> someone removes the last entry and then that id gets reused the next time
> someone adds something?

Yes, that's possible but never happened before.  Usually it took sometime before
we can remove an entry, and usually more entries have been added since.

[Zhenyao Mo, 2017-03-09 22:31:04]

On 2017/03/09 22:30:56, Zhenyao Mo wrote:
> On 2017/03/09 22:25:35, chfremer wrote:
> >
>
https://codereview.chromium.org/2744683002/diff/1/gpu/config/software_renderi...
> > File gpu/config/software_rendering_list_json.cc (right):
> > 
> >
>
https://codereview.chromium.org/2744683002/diff/1/gpu/config/software_renderi...
> > gpu/config/software_rendering_list_json.cc:409: "id": 49,
> > On 2017/03/09 21:49:32, Zhenyao Mo wrote:
> > > No, please don't reuse the ID, that will mess up histograms. Please move
> this
> > to
> > > the end of file and use the last id + 1.
> > 
> > Done.
> > 
> > A question I was having while doing so is if there is any guarantee that
last
> id
> > + 1 has not been used before. Even though quite unlikely, could it happen
that
> > someone removes the last entry and then that id gets reused the next time
> > someone adds something?
> 
> Yes, that's possible but never happened before.  Usually it took sometime
before
> we can remove an entry, and usually more entries have been added since.

lgtm

[chfremer, 2017-03-09 22:37:04]

The CQ bit was checked by chfremer@chromium.org

[chfremer, 2017-03-09 22:37:05]

The patchset sent to the CQ was uploaded after l-g-t-m from emircan@chromium.org
Link to the patchset: https://codereview.chromium.org/2744683002/#ps20001
(title: "Moved blacklist entry to end of file")

[commit-bot: I haz the power, 2017-03-09 22:37:37]

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-09 22:41:02]

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

[commit-bot: I haz the power, 2017-03-09 22:41:03]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  mac_optional_gpu_tests_rel on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_...)

[chfremer, 2017-03-10 18:56:33]

The CQ bit was checked by chfremer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-10 18:57:11]

Dry run: 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-10 19:51:55]

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

[commit-bot: I haz the power, 2017-03-10 19:51:56]

Dry run: This issue passed the CQ dry run.

[chfremer, 2017-03-10 20:41:14]

The CQ bit was checked by chfremer@chromium.org

[chfremer, 2017-03-10 20:41:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
emircan@chromium.org, zmo@chromium.org
Link to the patchset: https://codereview.chromium.org/2744683002/#ps40001
(title: "Rebase to Feb 10")

[commit-bot: I haz the power, 2017-03-10 20:41:48]

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-10 20:47:47]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489178474972890,
"parent_rev": "300b08741f54d52bbdf1c6e400f54dcf64b0da34", "commit_rev":
"31ec638091fc05bd13c41f8cd19cae7479cd5a1c"}

[commit-bot: I haz the power, 2017-03-10 20:48:30]

Description was changed from

==========
Add Gpu blacklist entry for video encoding on Linux.

On Linux, Video encoding is currently erroneously reported on chrome://gpu as
"Hardware 
accelerated". This CL fixes this by adding a blacklist entry.

BUG=641485
TEST=Open chrome://gpu on Linux and check that Video Encode is reported as
     Software only, hardware acceleration unavailable
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

==========
Add Gpu blacklist entry for video encoding on Linux.

On Linux, Video encoding is currently erroneously reported on chrome://gpu as
"Hardware 
accelerated". This CL fixes this by adding a blacklist entry.

BUG=641485
TEST=Open chrome://gpu on Linux and check that Video Encode is reported as
     Software only, hardware acceleration unavailable
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/2744683002
Cr-Commit-Position: refs/heads/master@{#456153}
Committed:
https://chromium.googlesource.com/chromium/src/+/31ec638091fc05bd13c41f8cd19c...
==========

[commit-bot: I haz the power, 2017-03-10 20:48:31]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/31ec638091fc05bd13c41f8cd19c...