gpu: merge gpu/ipc/host/gpu_memory_buffer_support.cc to gpu/ipc/common/

gpu: merge gpu/ipc/host/gpu_memory_buffer_support.cc to gpu/ipc/common/

Renderer needs to check if native GBM is supported. So this CL changes
* notify kEnableNativeGpuMemoryBuffer command line to renderer
* merge gpu/ipc/host/gpu_memory_buffer_support.cc to gpu/ipc/common/gpu_memory_buffer_support.cc

ChromeOS will copy a decoded YUV video frame to a YUYV GBM only if YUYV native GBM is supported.
It's because there is not regular texture fallback in OpenGL ES, while EGL
EXT_image_dma_buf_import extensions support YUYV EGL image. It's why Renderer
needs to know.

BUG=683347, 675431
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

[dshwang, 2017-01-20 23:32:37]

Description was changed from

==========
gpu: merge gpu/ipc/host/gpu_memory_buffer_support.cc to gpu/ipc/common/

Renderer needs to check if native GBM is supported. So this CL changes
* notify kEnableNativeGpuMemoryBuffer command line to renderer
* merge gpu/ipc/host/gpu_memory_buffer_support.cc to
gpu/ipc/common/gpu_memory_buffer_support.cc

ChromeOS will decode YUV video on YUYV buffer only if native GBM is supported.
It's because there is not regular texture fallback in OpenGL ES, while EGL
EXT_image_dma_buf_import extensions support YUYV EGL image. It's why Renderer
needs to know.

Actually, current code violates dependency policy, because child process code
uses gpu/ipc/host/
e.g.
services/ui/common/server_gpu_memory_buffer_manager.h
content/renderer/render_thread_impl_browsertest.cc
This change fixes these violation.

BUG=683347
==========

to

==========
gpu: merge gpu/ipc/host/gpu_memory_buffer_support.cc to gpu/ipc/common/

Renderer needs to check if native GBM is supported. So this CL changes
* notify kEnableNativeGpuMemoryBuffer command line to renderer
* merge gpu/ipc/host/gpu_memory_buffer_support.cc to
gpu/ipc/common/gpu_memory_buffer_support.cc

ChromeOS will decode YUV video on YUYV buffer only if native GBM is supported.
It's because there is not regular texture fallback in OpenGL ES, while EGL
EXT_image_dma_buf_import extensions support YUYV EGL image. It's why Renderer
needs to know.

Actually, current code violates dependency policy, because child process code
uses gpu/ipc/host/
e.g.
services/ui/common/server_gpu_memory_buffer_manager.h
content/renderer/render_thread_impl_browsertest.cc
This change fixes these violation.

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

[inactive_dshwang_plz_cc_intel, 2017-01-20 23:32:46]

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

[commit-bot: I haz the power, 2017-01-20 23:33:06]

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-01-20 23:36:39]

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

[commit-bot: I haz the power, 2017-01-20 23:36:40]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_optional_gpu_tests_rel on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_...)

[inactive_dshwang_plz_cc_intel, 2017-01-20 23:40:11]

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

[dshwang, 2017-01-20 23:40:19]

dongseong.hwang@intel.com changed reviewers:
+ ccameron@chromium.org, sadrul@chromium.org, sky@chromium.org,
tsepez@chromium.org, zmo@chromium.org

[dshwang, 2017-01-20 23:40:19]

It's follow-up CL of https://codereview.chromium.org/2551743002

zmo@chromium.org: Please review changes in gpu/
ccameron@chromium.org: Please review changes in content/browser
sky@chromium.org: Please review changes in service/

[commit-bot: I haz the power, 2017-01-20 23:40:37]

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-01-20 23:44:26]

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

[commit-bot: I haz the power, 2017-01-20 23:44:27]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_optional_gpu_tests_rel on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_optional_gpu_...)

[sadrul, 2017-01-21 02:59:39]

sadrul@chromium.org changed reviewers:
+ reveman@chromium.org

[sadrul, 2017-01-21 02:59:40]

+reveman@

We have discussed some of this in
https://bugs.chromium.org/p/chromium/issues/detail?id=675431 Perhaps we can have
a better solution for both from reveman@'s suggestions in that bug?

From this CL description:

> Actually, current code violates dependency policy, because child process code
uses gpu/ipc/host/
> e.g.
> services/ui/common/server_gpu_memory_buffer_manager.h
> content/renderer/render_thread_impl_browsertest.cc
> This change fixes these violation.

Not really. The code in services/ui is the gpu-host in mus. So that dependency
is OK. And test-code having additional dependencies (especially for switches) is
also OK.

[inactive_dshwang_plz_cc_intel, 2017-01-23 18:57:19]

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

[commit-bot: I haz the power, 2017-01-23 18:58:11]

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

[dshwang, 2017-01-23 19:07:03]

Description was changed from

==========
gpu: merge gpu/ipc/host/gpu_memory_buffer_support.cc to gpu/ipc/common/

Renderer needs to check if native GBM is supported. So this CL changes
* notify kEnableNativeGpuMemoryBuffer command line to renderer
* merge gpu/ipc/host/gpu_memory_buffer_support.cc to
gpu/ipc/common/gpu_memory_buffer_support.cc

ChromeOS will decode YUV video on YUYV buffer only if native GBM is supported.
It's because there is not regular texture fallback in OpenGL ES, while EGL
EXT_image_dma_buf_import extensions support YUYV EGL image. It's why Renderer
needs to know.

Actually, current code violates dependency policy, because child process code
uses gpu/ipc/host/
e.g.
services/ui/common/server_gpu_memory_buffer_manager.h
content/renderer/render_thread_impl_browsertest.cc
This change fixes these violation.

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

==========
gpu: merge gpu/ipc/host/gpu_memory_buffer_support.cc to gpu/ipc/common/

Renderer needs to check if native GBM is supported. So this CL changes
* notify kEnableNativeGpuMemoryBuffer command line to renderer
* merge gpu/ipc/host/gpu_memory_buffer_support.cc to
gpu/ipc/common/gpu_memory_buffer_support.cc

ChromeOS will decode YUV video on YUYV buffer only if native GBM is supported.
It's because there is not regular texture fallback in OpenGL ES, while EGL
EXT_image_dma_buf_import extensions support YUYV EGL image. It's why Renderer
needs to know.

Actually, current code violates dependency policy, because child process code
uses gpu/ipc/host/
e.g.
services/ui/common/server_gpu_memory_buffer_manager.h
content/renderer/render_thread_impl_browsertest.cc
This change fixes these violation.

BUG=683347, 675431
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
==========

[dshwang, 2017-01-23 19:07:26]

Description was changed from

==========
gpu: merge gpu/ipc/host/gpu_memory_buffer_support.cc to gpu/ipc/common/

Renderer needs to check if native GBM is supported. So this CL changes
* notify kEnableNativeGpuMemoryBuffer command line to renderer
* merge gpu/ipc/host/gpu_memory_buffer_support.cc to
gpu/ipc/common/gpu_memory_buffer_support.cc

ChromeOS will decode YUV video on YUYV buffer only if native GBM is supported.
It's because there is not regular texture fallback in OpenGL ES, while EGL
EXT_image_dma_buf_import extensions support YUYV EGL image. It's why Renderer
needs to know.

Actually, current code violates dependency policy, because child process code
uses gpu/ipc/host/
e.g.
services/ui/common/server_gpu_memory_buffer_manager.h
content/renderer/render_thread_impl_browsertest.cc
This change fixes these violation.

BUG=683347, 675431
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

==========
gpu: merge gpu/ipc/host/gpu_memory_buffer_support.cc to gpu/ipc/common/

Renderer needs to check if native GBM is supported. So this CL changes
* notify kEnableNativeGpuMemoryBuffer command line to renderer
* merge gpu/ipc/host/gpu_memory_buffer_support.cc to
gpu/ipc/common/gpu_memory_buffer_support.cc

ChromeOS will decode YUV video on YUYV buffer only if native GBM is supported.
It's because there is not regular texture fallback in OpenGL ES, while EGL
EXT_image_dma_buf_import extensions support YUYV EGL image. It's why Renderer
needs to know.

BUG=683347, 675431
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
==========

[dshwang, 2017-01-23 19:09:10]

On 2017/01/21 02:59:40, sadrul wrote:
> +reveman@
> 
> We have discussed some of this in
> https://bugs.chromium.org/p/chromium/issues/detail?id=675431 Perhaps we can
have
> a better solution for both from reveman@'s suggestions in that bug?

Thanks for pointing out. I think this CL is kind of preparation CL for
BUG=675431, instead of another solution or conflict.
I commented my opinion in the bug.

> From this CL description:
> 
> > Actually, current code violates dependency policy, because child process
code
> uses gpu/ipc/host/
> > e.g.
> > services/ui/common/server_gpu_memory_buffer_manager.h
> > content/renderer/render_thread_impl_browsertest.cc
> > This change fixes these violation.
> 
> Not really. The code in services/ui is the gpu-host in mus. So that dependency
> is OK. And test-code having additional dependencies (especially for switches)
is
> also OK.

Ok, I removed the description.

[commit-bot: I haz the power, 2017-01-23 20:01:24]

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

[commit-bot: I haz the power, 2017-01-23 20:01:25]

Dry run: This issue passed the CQ dry run.

[dshwang, 2017-01-26 22:13:34]

reveman@, could you review overall of this CL?

[dshwang, 2017-02-08 00:33:18]

reveman@, could you review overall of this CL?

[dshwang, 2017-02-10 23:34:21]

reveman@, could you review overall of this CL?

zmo@chromium.org: Please review changes in gpu/
ccameron@chromium.org: Please review changes in content/browser
sky@chromium.org: Please review changes in service/

[reveman, 2017-02-11 00:06:24]

Not sure this is the right approach. I'd rather we could enable decode to YUYV
independently of native GMB support and handle that correctly or leave it up to
how we configure a specific device and only enable this on a devices where we
know it's supported.

[dshwang, 2017-02-11 01:34:39]

On 2017/02/11 00:06:24, reveman wrote:
> Not sure this is the right approach. I'd rather we could enable decode to YUYV
> independently of native GMB support and handle that correctly or leave it up
to
> how we configure a specific device and only enable this on a devices where we
> know it's supported.

Sorry for confusing. This CL is for GpuMemoryBufferVideoFramePool, which copies
I420 to GMB supported format (e.g. NV12 or YUYV). It doesn't decode video
directly to YUYV plane.
GpuMemoryBufferVideoFramePool just depends on native GMB. It's why
--enable-gpu-memory-buffer-video-frames is enabled by default in linux only if
--enable-native-gpu-memory-buffers is enabled.
GpuMemoryBufferVideoFramePool just wants to query YUVY native GMB is supported
in renderer process.

[dshwang, 2017-02-11 01:36:09]

Description was changed from

==========
gpu: merge gpu/ipc/host/gpu_memory_buffer_support.cc to gpu/ipc/common/

Renderer needs to check if native GBM is supported. So this CL changes
* notify kEnableNativeGpuMemoryBuffer command line to renderer
* merge gpu/ipc/host/gpu_memory_buffer_support.cc to
gpu/ipc/common/gpu_memory_buffer_support.cc

ChromeOS will decode YUV video on YUYV buffer only if native GBM is supported.
It's because there is not regular texture fallback in OpenGL ES, while EGL
EXT_image_dma_buf_import extensions support YUYV EGL image. It's why Renderer
needs to know.

BUG=683347, 675431
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

==========
gpu: merge gpu/ipc/host/gpu_memory_buffer_support.cc to gpu/ipc/common/

Renderer needs to check if native GBM is supported. So this CL changes
* notify kEnableNativeGpuMemoryBuffer command line to renderer
* merge gpu/ipc/host/gpu_memory_buffer_support.cc to
gpu/ipc/common/gpu_memory_buffer_support.cc

ChromeOS will copy decoded YUV video to YUYV GBM only if YUYV native GBM is
supported.
It's because there is not regular texture fallback in OpenGL ES, while EGL
EXT_image_dma_buf_import extensions support YUYV EGL image. It's why Renderer
needs to know.

BUG=683347, 675431
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
==========

[dshwang, 2017-02-11 01:36:32]

Description was changed from

==========
gpu: merge gpu/ipc/host/gpu_memory_buffer_support.cc to gpu/ipc/common/

Renderer needs to check if native GBM is supported. So this CL changes
* notify kEnableNativeGpuMemoryBuffer command line to renderer
* merge gpu/ipc/host/gpu_memory_buffer_support.cc to
gpu/ipc/common/gpu_memory_buffer_support.cc

ChromeOS will copy decoded YUV video to YUYV GBM only if YUYV native GBM is
supported.
It's because there is not regular texture fallback in OpenGL ES, while EGL
EXT_image_dma_buf_import extensions support YUYV EGL image. It's why Renderer
needs to know.

BUG=683347, 675431
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

==========
gpu: merge gpu/ipc/host/gpu_memory_buffer_support.cc to gpu/ipc/common/

Renderer needs to check if native GBM is supported. So this CL changes
* notify kEnableNativeGpuMemoryBuffer command line to renderer
* merge gpu/ipc/host/gpu_memory_buffer_support.cc to
gpu/ipc/common/gpu_memory_buffer_support.cc

ChromeOS will copy a decoded YUV video frame to a YUYV GBM only if YUYV native
GBM is supported.
It's because there is not regular texture fallback in OpenGL ES, while EGL
EXT_image_dma_buf_import extensions support YUYV EGL image. It's why Renderer
needs to know.

BUG=683347, 675431
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
==========

[reveman, 2017-02-13 18:49:38]

On 2017/02/11 at 01:34:39, dongseong.hwang wrote:
> On 2017/02/11 00:06:24, reveman wrote:
> > Not sure this is the right approach. I'd rather we could enable decode to
YUYV
> > independently of native GMB support and handle that correctly or leave it up
to
> > how we configure a specific device and only enable this on a devices where
we
> > know it's supported.
> 
> Sorry for confusing. This CL is for GpuMemoryBufferVideoFramePool, which
copies I420 to GMB supported format (e.g. NV12 or YUYV). It doesn't decode video
directly to YUYV plane.
> GpuMemoryBufferVideoFramePool just depends on native GMB. It's why
--enable-gpu-memory-buffer-video-frames is enabled by default in linux only if
--enable-native-gpu-memory-buffers is enabled.
> GpuMemoryBufferVideoFramePool just wants to query YUVY native GMB is supported
in renderer process.

Still, I'd rather not have implicit dependencies on native GMBs. Ideally we'd
have a sw fallback for all cases. We should at least use a design that expects
that, even if short term we might just implement it for the native case. GMB
video frames should be the default with or without native GMBs as there's no
reason for that to be less efficient than a normal upload.

[dshwang, 2017-02-21 22:51:22]

On 2017/02/13 18:49:38, reveman wrote:
> On 2017/02/11 at 01:34:39, dongseong.hwang wrote:
> > On 2017/02/11 00:06:24, reveman wrote:
> > > Not sure this is the right approach. I'd rather we could enable decode to
> YUYV
> > > independently of native GMB support and handle that correctly or leave it
up
> to
> > > how we configure a specific device and only enable this on a devices where
> we
> > > know it's supported.
> > 
> > Sorry for confusing. This CL is for GpuMemoryBufferVideoFramePool, which
> copies I420 to GMB supported format (e.g. NV12 or YUYV). It doesn't decode
video
> directly to YUYV plane.
> > GpuMemoryBufferVideoFramePool just depends on native GMB. It's why
> --enable-gpu-memory-buffer-video-frames is enabled by default in linux only if
> --enable-native-gpu-memory-buffers is enabled.
> > GpuMemoryBufferVideoFramePool just wants to query YUVY native GMB is
supported
> in renderer process.
> 
> Still, I'd rather not have implicit dependencies on native GMBs. Ideally we'd
> have a sw fallback for all cases. We should at least use a design that expects
> that, even if short term we might just implement it for the native case. GMB
> video frames should be the default with or without native GMBs as there's no
> reason for that to be less efficient than a normal upload.

as time goes on, GMB has been evolved that there are some formats which can not
be supported by sw fallback.
e.g. YVU_420, YUV_420_BIPLANAR, UYVY_422
https://cs.chromium.org/chromium/src/ui/gl/gl_image_memory.cc?type=cs&q=Valid...

This is the root reason why GpuMemoryBufferVideoFramePool want to know
--enable-native-gpu-memory-buffers

In theory, it is possible to implement sw fallback for those exceptional formats
and GpuMemoryBufferVideoFramePool uses them.
However, the implementation is too expensive to be used in practice. For
example, YUYV sw fallback requires "RGB conversion to temporary memory" and
"upload texture". In native case, GPU driver binds YUYV GMB to RGB texture
somehow. e.g. EGL_EXT_image_dma_buf_import
In the case, there is no point that GpuMemoryBufferVideoFramePool uses YUYV
format, because 3 GMBs of R_8 (i.e. by default) are much cheaper. It's why
GpuMemoryBufferVideoFramePool want to know native YUYV GMB is supported.

It's kind of problem between code simpliciy and optimization. The code path is
hotspot for video decoding, so I think we want to choose optimization. What do
you think?

[dshwang, 2017-03-07 21:50:03]

https://codereview.chromium.org/2649553003/diff/40001/gpu/ipc/common/gpu_memo...
File gpu/ipc/common/gpu_memory_buffer_support.cc (right):

https://codereview.chromium.org/2649553003/diff/40001/gpu/ipc/common/gpu_memo...
gpu/ipc/common/gpu_memory_buffer_support.cc:50: if
(!(AreNativeGpuMemoryBuffersEnabled() || never_cpu_read))
reveman@, besides video GMB support, I think this CL makes code more make sense.

common/gpu_memory_buffer_support.cc can be used by either renderer and browser,
and IsNativeGpuMemoryBufferConfigurationSupported() cannot confirm
"IsNativeGpuMemoryBufferConfigurationSupported" to renderer, because renderer
cannot know if native GMB is supported.