Allow putting NV12 software GMBs into overlays on windows

Allow putting NV12 software GMBs into overlays on windows

Copy the shared-memory NV12 GpuMemoryBuffer into an NV12 D3D11 dynamic
texture, which can be used as the source for the video processing that
goes to the backbuffer.

This still needs --enable-gpu-memory-buffer-video-frames to be enabled.
In my testing it reduces power on a 1080p60 VP9 video from 4W to 3.1W.

BUG=716286
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/2849963003
Cr-Commit-Position: refs/heads/master@{#469558}
Committed: https://chromium.googlesource.com/chromium/src/+/60b4d8b24e7096491e00b1745ec64129a5234e06

[jbauman, 2017-04-29 01:13:59]

Description was changed from

==========
Allow putting NV12 software GMBs into overlays on windows

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

to

==========
Allow putting NV12 software GMBs into overlays on windows

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

[jbauman, 2017-04-29 01:14:04]

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

[commit-bot: I haz the power, 2017-04-29 01:14:35]

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-04-29 03:00:01]

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

[commit-bot: I haz the power, 2017-04-29 03:00:02]

Dry run: This issue passed the CQ dry run.

[jbauman, 2017-05-04 02:51:57]

Description was changed from

==========
Allow putting NV12 software GMBs into overlays on windows

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

==========
Allow putting NV12 software GMBs into overlays on windows

Copy the shared-memory NV12 GpuMemoryBuffer into an NV12 D3D11 dynamic
texture, which can be used as the source for the video processing that
goes to the backbuffer.

This still needs --enable-gpu-memory-buffer-video-frames to be enabled.
In my testing it reduces power on a 1080p60 VP9 video from 4W to 3.1W.

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

[jbauman, 2017-05-04 02:54:04]

jbauman@chromium.org changed reviewers:
+ ccameron@chromium.org, sunnyps@chromium.org

[DaleCurtis, 2017-05-04 16:05:05]

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

[DaleCurtis, 2017-05-04 16:05:05]

media/ portion lgtm

[sunnyps1, 2017-05-04 22:26:55]

sunnyps@google.com changed reviewers:
+ sunnyps@google.com

[sunnyps1, 2017-05-04 22:26:56]

https://codereview.chromium.org/2849963003/diff/1/gpu/ipc/service/direct_comp...
File gpu/ipc/service/direct_composition_surface_win.cc (right):

https://codereview.chromium.org/2849963003/diff/1/gpu/ipc/service/direct_comp...
gpu/ipc/service/direct_composition_surface_win.cc:212: bool
UploadMemoryImages(gl::GLImageMemory* y_image_memory,
nit: UploadVideoImages

https://codereview.chromium.org/2849963003/diff/1/gpu/ipc/service/direct_comp...
gpu/ipc/service/direct_composition_surface_win.cc:386: CHECK(SUCCEEDED(hr)) <<
std::hex << hr;
nit: can you write a more descriptive error message? or omit it completely?

https://codereview.chromium.org/2849963003/diff/1/gpu/ipc/service/direct_comp...
gpu/ipc/service/direct_composition_surface_win.cc:394: CHECK(SUCCEEDED(hr)) <<
std::hex << hr;
nit: same here

https://codereview.chromium.org/2849963003/diff/1/gpu/ipc/service/direct_comp...
gpu/ipc/service/direct_composition_surface_win.cc:408: for (int y = 0; y <
texture_size.height() / 2; y++) {
nit: uv_image_size.height() instead of texture_size.height() / 2

https://codereview.chromium.org/2849963003/diff/1/gpu/ipc/service/direct_comp...
gpu/ipc/service/direct_composition_surface_win.cc:460: } else if (last_gl_image_
== params.image[0]) {
Does this check also work for YUV? Can't UV change but not Y?

https://codereview.chromium.org/2849963003/diff/1/gpu/ipc/service/direct_comp...
gpu/ipc/service/direct_composition_surface_win.cc:476: DCHECK(!!y_image_memory);
nit: DCHECK(y_image_memory)

[ccameron, 2017-05-04 22:33:20]

ui/ lgtm

[jbauman, 2017-05-04 22:51:07]

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

[jbauman, 2017-05-04 22:51:45]

PTAL.

On 2017/05/04 22:26:56, sunnyps1 wrote:
>
https://codereview.chromium.org/2849963003/diff/1/gpu/ipc/service/direct_comp...
> File gpu/ipc/service/direct_composition_surface_win.cc (right):
> 
>
https://codereview.chromium.org/2849963003/diff/1/gpu/ipc/service/direct_comp...
> gpu/ipc/service/direct_composition_surface_win.cc:212: bool
> UploadMemoryImages(gl::GLImageMemory* y_image_memory,
> nit: UploadVideoImages
> 
>
https://codereview.chromium.org/2849963003/diff/1/gpu/ipc/service/direct_comp...
> gpu/ipc/service/direct_composition_surface_win.cc:386: CHECK(SUCCEEDED(hr)) <<
> std::hex << hr;
> nit: can you write a more descriptive error message? or omit it completely?
> 
>
https://codereview.chromium.org/2849963003/diff/1/gpu/ipc/service/direct_comp...
> gpu/ipc/service/direct_composition_surface_win.cc:394: CHECK(SUCCEEDED(hr)) <<
> std::hex << hr;
> nit: same here
> 
>
https://codereview.chromium.org/2849963003/diff/1/gpu/ipc/service/direct_comp...
> gpu/ipc/service/direct_composition_surface_win.cc:408: for (int y = 0; y <
> texture_size.height() / 2; y++) {
> nit: uv_image_size.height() instead of texture_size.height() / 2
> 
>
https://codereview.chromium.org/2849963003/diff/1/gpu/ipc/service/direct_comp...
> gpu/ipc/service/direct_composition_surface_win.cc:460: } else if
(last_gl_image_
> == params.image[0]) {
> Does this check also work for YUV? Can't UV change but not Y?
> 
>
https://codereview.chromium.org/2849963003/diff/1/gpu/ipc/service/direct_comp...
> gpu/ipc/service/direct_composition_surface_win.cc:476:
DCHECK(!!y_image_memory);
> nit: DCHECK(y_image_memory)

[commit-bot: I haz the power, 2017-05-04 22:52:12]

Dry run: CQ is trying da patch.

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

[sunnyps1, 2017-05-04 22:53:39]

lgtm

[sunnyps, 2017-05-04 22:54:16]

lgtm with the right account

[commit-bot: I haz the power, 2017-05-04 23:57:24]

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

[commit-bot: I haz the power, 2017-05-04 23:57:25]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jbauman, 2017-05-05 00:15:36]

The CQ bit was checked by jbauman@chromium.org

[jbauman, 2017-05-05 00:15:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
dalecurtis@chromium.org, ccameron@chromium.org
Link to the patchset: https://codereview.chromium.org/2849963003/#ps20001
(title: "post-review changes")

[commit-bot: I haz the power, 2017-05-05 00:16: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, 2017-05-05 01:07:19]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493943336995170,
"parent_rev": "ab52b62684a4634d2d5e7fdc2a8a137af9aa8647", "commit_rev":
"60b4d8b24e7096491e00b1745ec64129a5234e06"}

[commit-bot: I haz the power, 2017-05-05 01:07:30]

Description was changed from

==========
Allow putting NV12 software GMBs into overlays on windows

Copy the shared-memory NV12 GpuMemoryBuffer into an NV12 D3D11 dynamic
texture, which can be used as the source for the video processing that
goes to the backbuffer.

This still needs --enable-gpu-memory-buffer-video-frames to be enabled.
In my testing it reduces power on a 1080p60 VP9 video from 4W to 3.1W.

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

==========
Allow putting NV12 software GMBs into overlays on windows

Copy the shared-memory NV12 GpuMemoryBuffer into an NV12 D3D11 dynamic
texture, which can be used as the source for the video processing that
goes to the backbuffer.

This still needs --enable-gpu-memory-buffer-video-frames to be enabled.
In my testing it reduces power on a 1080p60 VP9 video from 4W to 3.1W.

BUG=716286
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/2849963003
Cr-Commit-Position: refs/heads/master@{#469558}
Committed:
https://chromium.googlesource.com/chromium/src/+/60b4d8b24e7096491e00b1745ec6...
==========

[commit-bot: I haz the power, 2017-05-05 01:07:31]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/60b4d8b24e7096491e00b1745ec6...