Disable unified media pipeline (Spitzer) for Webview for PowerVR

Disable unified media pipeline (Spitzer) for Webview for PowerVR

The function eglCreateImageKHR() seems to fail on this GPU
for L8 textures which results in YUV->RGB conversion failures.
EGLImage is used for Spitzer in WebView.

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

Committed: https://crrev.com/3f7abca63f1e71713d6c26747093d331bd3299f0
Cr-Commit-Position: refs/heads/master@{#414629}

[Tima Vaisburd, 2016-08-25 17:55:30]

Description was changed from

==========
Disable Spitzer for Webview for PowerVR GPU

The function eglCreateImageKHR() seems to fail on this GPU
for L8 textures which results in YUV->RGB conversion failures.
EGLImage is used for Spitzer in WebView.

BUG=632461
==========

to

==========
Disable Spitzer for Webview for PowerVR GPU

The function eglCreateImageKHR() seems to fail on this GPU
for L8 textures which results in YUV->RGB conversion failures.
EGLImage is used for Spitzer in WebView.

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

[Tima Vaisburd, 2016-08-25 17:55:49]

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

[commit-bot: I haz the power, 2016-08-25 17:56:45]

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

[Tima Vaisburd, 2016-08-25 17:57:46]

timav@chromium.org changed reviewers:
+ boliu@chromium.org

[Tima Vaisburd, 2016-08-25 17:57:46]

Finally I found a place in code that seems correct. Still testing on the device.
PTAL.

[commit-bot: I haz the power, 2016-08-25 18:11:41]

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

[commit-bot: I haz the power, 2016-08-25 18:11:42]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_optional_gpu_tests_rel on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_...)

[Tima Vaisburd, 2016-08-25 18:32:05]

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

[commit-bot: I haz the power, 2016-08-25 18:33:41]

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

[DaleCurtis, 2016-08-25 19:00:18]

dalecurtis@chromium.org changed reviewers:
+ dalecurtis@chromium.org

[DaleCurtis, 2016-08-25 19:00:18]

Do you want to disable UMP or just AcceleratedVideoDecode? This will allow the
use of vp8/vp9 through Spitzer then.

[commit-bot: I haz the power, 2016-08-25 19:21:11]

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

[commit-bot: I haz the power, 2016-08-25 19:21:12]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Tima Vaisburd, 2016-08-25 19:25:24]

On 2016/08/25 19:00:18, DaleCurtis wrote:
> Do you want to disable UMP or just AcceleratedVideoDecode? This will allow the
> use of vp8/vp9 through Spitzer then.

I thought the former because of shared texture mailboxes. Are they not required
for Spitzer for software
decoders?

[DaleCurtis, 2016-08-25 19:26:17]

I don't know the original bug, I'm just the peanut gallery :)

[boliu, 2016-08-25 19:35:44]

On 2016/08/25 19:25:24, Tima Vaisburd wrote:
> On 2016/08/25 19:00:18, DaleCurtis wrote:
> > Do you want to disable UMP or just AcceleratedVideoDecode? This will allow
the
> > use of vp8/vp9 through Spitzer then.
> 
> I thought the former because of shared texture mailboxes. Are they not
required
> for Spitzer for software
> decoders?

software decode does *not* mean mailbox sharing is not used. Actually I think
it's precisely software decoding (into separate yuv textures) that's broken on
this driver

[boliu, 2016-08-25 20:59:18]

On 2016/08/25 19:35:44, boliu wrote:
> On 2016/08/25 19:25:24, Tima Vaisburd wrote:
> > On 2016/08/25 19:00:18, DaleCurtis wrote:
> > > Do you want to disable UMP or just AcceleratedVideoDecode? This will allow
> the
> > > use of vp8/vp9 through Spitzer then.
> > 
> > I thought the former because of shared texture mailboxes. Are they not
> required
> > for Spitzer for software
> > decoders?
> 
> software decode does *not* mean mailbox sharing is not used. Actually I think
> it's precisely software decoding (into separate yuv textures) that's broken on
> this driver

I thought spitzer had a way to turn itself off on specific devices. Dale?

[DaleCurtis, 2016-08-25 21:00:42]

We have a couple, this blacklist is one of them -- another is in MediaCodecUtil.
If this affects software decoders then we have to disable all of UMP on these
devices though. We only have support for disabling the hardware decoders, we
always expect software codecs to work fine.

[DaleCurtis, 2016-08-25 21:01:41]

If the YT videos that are showing green are vp8/vp9 then it's as you say,
software codecs are broken some how.

[Tima Vaisburd, 2016-08-25 22:50:44]

On 2016/08/25 21:01:41, DaleCurtis wrote:
> If the YT videos that are showing green are vp8/vp9 then it's as you say,
> software codecs are broken some how.

I'm confirming that it is like Bo said :) This is vp9 video and VpxVideoDecoder
took charge.

[DaleCurtis, 2016-08-25 22:54:30]

lgtm me then :)

[boliu, 2016-08-25 23:05:20]

You should get an actual owner here

https://codereview.chromium.org/2281663002/diff/20001/content/browser/gpu/gpu...
File content/browser/gpu/gpu_data_manager_impl_private.cc (right):

https://codereview.chromium.org/2281663002/diff/20001/content/browser/gpu/gpu...
content/browser/gpu/gpu_data_manager_impl_private.cc:753:
command_line->AppendSwitch(switches::kDisableUnifiedMediaPipeline);
It's not thread safe to be modifying the global command line here. Also it's not
clear where the switch is read and if this is the right command line to modify.

Normally this kind of thing is passed through GpuCapabilities or GpuPreferences,
if you need it on the command buffer client side.

[Tima Vaisburd, 2016-08-25 23:52:41]

On 2016/08/25 23:05:20, boliu wrote:
> You should get an actual owner here
> 
>
https://codereview.chromium.org/2281663002/diff/20001/content/browser/gpu/gpu...
> File content/browser/gpu/gpu_data_manager_impl_private.cc (right):
> 
>
https://codereview.chromium.org/2281663002/diff/20001/content/browser/gpu/gpu...
> content/browser/gpu/gpu_data_manager_impl_private.cc:753:
> command_line->AppendSwitch(switches::kDisableUnifiedMediaPipeline);
> It's not thread safe to be modifying the global command line here. Also it's
not
> clear where the switch is read and if this is the right command line to
modify.
> 
> Normally this kind of thing is passed through GpuCapabilities or
GpuPreferences,
> if you need it on the command buffer client side.

Sorry, this time I do not quite understand.

Spitzer pipeline is controlled by sole function
media::IsUnifiedMediaPipelineEnabled()
which is used in the Renderer process and, as far as I can tell, in the browser
process
through MimeUtil(). In turn, this IsUnifiedMediaPipelineEnabled() is controlled
by one
cli switch: https://cs.chromium.org/chromium/src/media/base/media.cc?rcl=0&l=99

My goal was to set this switch early enough so the Renderer thread or process
will pick it up.
How would GpuPreferences help me?

In short, what am I missing?

> It's not thread safe to be modifying the global command line here

It happens on UI thread, why? Also, the code above calls AppendSwitch().

[Tima Vaisburd, 2016-08-26 00:03:13]

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

[commit-bot: I haz the power, 2016-08-26 00:03:46]

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

[Tima Vaisburd, 2016-08-26 00:05:15]

timav@chromium.org changed reviewers:
+ kbr@chromium.org

[Tima Vaisburd, 2016-08-26 00:05:16]

Adding Ken for OWNERS review.

[Ken Russell (switch to Gerrit), 2016-08-26 00:21:07]

LGTM

Could you please at least mention "unified media pipeline" in the CL description
instead of just the codeword for the project?

[Ken Russell (switch to Gerrit), 2016-08-26 00:22:15]

https://codereview.chromium.org/2281663002/diff/40001/content/browser/gpu/gpu...
File content/browser/gpu/gpu_data_manager_impl_private.cc (right):

https://codereview.chromium.org/2281663002/diff/40001/content/browser/gpu/gpu...
content/browser/gpu/gpu_data_manager_impl_private.cc:751: if
(command_line->HasSwitch(switches::kEnableThreadedTextureMailboxes) &&
Question: is this the key flag which distinguishes WebView from Chromium?

[boliu, 2016-08-26 01:45:41]

lgtm

[Tima Vaisburd, 2016-08-26 01:49:18]

Description was changed from

==========
Disable Spitzer for Webview for PowerVR GPU

The function eglCreateImageKHR() seems to fail on this GPU
for L8 textures which results in YUV->RGB conversion failures.
EGLImage is used for Spitzer in WebView.

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

==========
Disable unified media pipeline (Spitzer) for Webview for PowerVR

The function eglCreateImageKHR() seems to fail on this GPU
for L8 textures which results in YUV->RGB conversion failures.
EGLImage is used for Spitzer in WebView.

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

[Tima Vaisburd, 2016-08-26 01:57:19]

On 2016/08/26 00:22:15, Ken Russell wrote:
> Question: is this the key flag which distinguishes WebView from Chromium?

Yes.

The relation with threaded texture mailboxes seems appropriate:
without the fix the code breaks in TextureDefiniton
https://cs.chromium.org/chromium/src/gpu/command_buffer/service/texture_defin...
which is used because WebView needs threaded texture mailboxes.

[commit-bot: I haz the power, 2016-08-26 02:13:21]

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

[commit-bot: I haz the power, 2016-08-26 02:13:22]

Dry run: This issue passed the CQ dry run.

[Tima Vaisburd, 2016-08-26 02:13:36]

The CQ bit was checked by timav@chromium.org

[Tima Vaisburd, 2016-08-26 02:13:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
dalecurtis@chromium.org
Link to the patchset: https://codereview.chromium.org/2281663002/#ps40001
(title: "rebased, updated bug list version")

[commit-bot: I haz the power, 2016-08-26 02:14:02]

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, 2016-08-26 02:18:52]

Description was changed from

==========
Disable unified media pipeline (Spitzer) for Webview for PowerVR

The function eglCreateImageKHR() seems to fail on this GPU
for L8 textures which results in YUV->RGB conversion failures.
EGLImage is used for Spitzer in WebView.

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

==========
Disable unified media pipeline (Spitzer) for Webview for PowerVR

The function eglCreateImageKHR() seems to fail on this GPU
for L8 textures which results in YUV->RGB conversion failures.
EGLImage is used for Spitzer in WebView.

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

[commit-bot: I haz the power, 2016-08-26 02:18:53]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-26 02:20:42]

Description was changed from

==========
Disable unified media pipeline (Spitzer) for Webview for PowerVR

The function eglCreateImageKHR() seems to fail on this GPU
for L8 textures which results in YUV->RGB conversion failures.
EGLImage is used for Spitzer in WebView.

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

==========
Disable unified media pipeline (Spitzer) for Webview for PowerVR

The function eglCreateImageKHR() seems to fail on this GPU
for L8 textures which results in YUV->RGB conversion failures.
EGLImage is used for Spitzer in WebView.

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

Committed: https://crrev.com/3f7abca63f1e71713d6c26747093d331bd3299f0
Cr-Commit-Position: refs/heads/master@{#414629}
==========

[commit-bot: I haz the power, 2016-08-26 02:20:44]

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