Send overlay promotion hints from to GLStreamTextureImage.

Send overlay promotion hints from to GLStreamTextureImage.

ResourceProvider knows if a resource has requested hints about
overlay promotion.  This CL closes the loop, allowing
ResourceProvider to notify a GLStreamTextureImage that's bound to
the resource's texture about the promotion hint.

This is supported only on Android.  The purpose is to let
AndroidVideoDecodeAccelerator figure out if video frames should be
backed by SurfaceTexture or SurfaceView.

BUG=671357
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Committed: https://crrev.com/6408f27c941a3f90119958301b16f3c7cf2ebced
Cr-Commit-Position: refs/heads/master@{#437906}

[liberato (no reviews please), 2016-12-06 17:06:03]

Description was changed from

==========
Send overlay promotion hints from to GLStreamTextureImage.

ResourceProvider knows if a resource has requested hints about
overlay promotion.  This CL closes the loop, allowing
ResourceProvider to notify a GLStreamTextureImage that's bound to
the resource's texture about the promotion hint.

This is supported only on Android.  The purpose is to let
AndroidVideoDecodeAccelerator figure out if video frames should be
backed by SurfaceTexture or SurfaceView.

BUG=671357
==========

to

==========
Send overlay promotion hints from to GLStreamTextureImage.

ResourceProvider knows if a resource has requested hints about
overlay promotion.  This CL closes the loop, allowing
ResourceProvider to notify a GLStreamTextureImage that's bound to
the resource's texture about the promotion hint.

This is supported only on Android.  The purpose is to let
AndroidVideoDecodeAccelerator figure out if video frames should be
backed by SurfaceTexture or SurfaceView.

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

[liberato (no reviews please), 2016-12-06 17:06:04]

Description was changed from

==========
Send overlay promotion hints from to GLStreamTextureImage.

ResourceProvider knows if a resource has requested hints about
overlay promotion.  This CL closes the loop, allowing
ResourceProvider to notify a GLStreamTextureImage that's bound to
the resource's texture about the promotion hint.

This is supported only on Android.  The purpose is to let
AndroidVideoDecodeAccelerator figure out if video frames should be
backed by SurfaceTexture or SurfaceView.

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

==========
Send overlay promotion hints from to GLStreamTextureImage.

ResourceProvider knows if a resource has requested hints about
overlay promotion.  This CL closes the loop, allowing
ResourceProvider to notify a GLStreamTextureImage that's bound to
the resource's texture about the promotion hint.

This is supported only on Android.  The purpose is to let
AndroidVideoDecodeAccelerator figure out if video frames should be
backed by SurfaceTexture or SurfaceView.

BUG=671357
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[liberato (no reviews please), 2016-12-06 17:42:01]

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

[commit-bot: I haz the power, 2016-12-06 17:42:27]

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

[liberato (no reviews please), 2016-12-06 18:02:03]

Description was changed from

==========
Send overlay promotion hints from to GLStreamTextureImage.

ResourceProvider knows if a resource has requested hints about
overlay promotion.  This CL closes the loop, allowing
ResourceProvider to notify a GLStreamTextureImage that's bound to
the resource's texture about the promotion hint.

This is supported only on Android.  The purpose is to let
AndroidVideoDecodeAccelerator figure out if video frames should be
backed by SurfaceTexture or SurfaceView.

BUG=671357
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Send overlay promotion hints from to GLStreamTextureImage.

ResourceProvider knows if a resource has requested hints about
overlay promotion.  This CL closes the loop, allowing
ResourceProvider to notify a GLStreamTextureImage that's bound to
the resource's texture about the promotion hint.

This is supported only on Android.  The purpose is to let
AndroidVideoDecodeAccelerator figure out if video frames should be
backed by SurfaceTexture or SurfaceView.

BUG=671357
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[liberato (no reviews please), 2016-12-06 18:02:03]

liberato@chromium.org changed reviewers:
+ piman@chromium.org

[liberato (no reviews please), 2016-12-06 18:14:16]

Hi

Here are some interesting bits about this CL:

- i modified how hints are stored in OverlayCandidateList to include the drawing
rect origin.  quite handy, so that AVDA can tell if the video frame is being
scrolled around.

- started checking for |marked_for_deletion| before sending hints in
ResourceProvider::SendPromotionHints .  It wasn't clear to me in the previous
CL, before plumbing these through to GLStreamTextureImage, that there were
extras being sent sometimes.  I didn't try to remove things from
|wants_promotion_hints_set_| when setting |marked_for_deletion|, since there are
several paths that do that.  Worried it would get out of sync if we relied on it
when sending hints.

- included a new GL command, SendOverlayPromotionHintCHROMIUM, in the stream
texture matrix GL extension.  wasn't sure if this was the right place, but
seemed like a reasonable first guess.

manual testing showed that the right info is getting to AVDACodecImage, though
all of media/ was reverted in the most recent CL.

thanks
-fl

[piman, 2016-12-06 21:40:57]

LGTM, just a bunch of nits.

https://codereview.chromium.org/2559523002/diff/80001/gpu/GLES2/extensions/CH...
File gpu/GLES2/extensions/CHROMIUM/CHROMIUM_stream_texture_matrix.txt (right):

https://codereview.chromium.org/2559523002/diff/80001/gpu/GLES2/extensions/CH...
gpu/GLES2/extensions/CHROMIUM/CHROMIUM_stream_texture_matrix.txt:51: void
SendOverlayPromotionHintCHROMIUM(
naming nit: no "Send". The naming logic is that you're setting state, and the gl
names just contain the name of the state you set, not any of "Set" "Send" etc.
See e.g. above, UniformMatrix4fvStreamTextureMatrixCHROMIUM "sets" a uniform
matrix, but we leave "set" out of the name.

https://codereview.chromium.org/2559523002/diff/80001/gpu/command_buffer/serv...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):

https://codereview.chromium.org/2559523002/diff/80001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:1675: // Wrapper for
glOverlayPromotabilityHintSteramTextureMatrixCHROMIUM
nit: wrong GL name? glOverlayPromotionHintCHROMIUM sounds right (given naming
comment in extension doc)

https://codereview.chromium.org/2559523002/diff/80001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:8566:
"glOverlayPromotionHintStreamTextureMatrix",
nit: fix gl name

https://codereview.chromium.org/2559523002/diff/80001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:8574:
LOCAL_SET_GL_ERROR(GL_INVALID_VALUE,
nit: make this a GL_INVALID_OPERATION, which is more consistent with
cross-validation of state.

https://codereview.chromium.org/2559523002/diff/80001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:8575:
"glOverlayPromotionHintStreamTextureMatrix",
nit: ditto

[liberato (no reviews please), 2016-12-09 23:15:10]

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

[commit-bot: I haz the power, 2016-12-09 23:15:58]

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, 2016-12-10 00:00:46]

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

[commit-bot: I haz the power, 2016-12-10 00:00:46]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[liberato (no reviews please), 2016-12-10 00:28:44]

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

[commit-bot: I haz the power, 2016-12-10 00:29:51]

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, 2016-12-10 03:59:46]

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

[commit-bot: I haz the power, 2016-12-10 03:59:47]

Dry run: This issue passed the CQ dry run.

[liberato (no reviews please), 2016-12-12 16:44:38]

thanks for the help!

-fl

https://codereview.chromium.org/2559523002/diff/80001/gpu/GLES2/extensions/CH...
File gpu/GLES2/extensions/CHROMIUM/CHROMIUM_stream_texture_matrix.txt (right):

https://codereview.chromium.org/2559523002/diff/80001/gpu/GLES2/extensions/CH...
gpu/GLES2/extensions/CHROMIUM/CHROMIUM_stream_texture_matrix.txt:51: void
SendOverlayPromotionHintCHROMIUM(
On 2016/12/06 21:40:56, piman - On leave - No reviews wrote:
> naming nit: no "Send". The naming logic is that you're setting state, and the
gl
> names just contain the name of the state you set, not any of "Set" "Send" etc.
> See e.g. above, UniformMatrix4fvStreamTextureMatrixCHROMIUM "sets" a uniform
> matrix, but we leave "set" out of the name.

Done.

https://codereview.chromium.org/2559523002/diff/80001/gpu/command_buffer/serv...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):

https://codereview.chromium.org/2559523002/diff/80001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:1675: // Wrapper for
glOverlayPromotabilityHintSteramTextureMatrixCHROMIUM
On 2016/12/06 21:40:56, piman wrote:
> nit: wrong GL name? glOverlayPromotionHintCHROMIUM sounds right (given naming
> comment in extension doc)

Done.

https://codereview.chromium.org/2559523002/diff/80001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:8566:
"glOverlayPromotionHintStreamTextureMatrix",
On 2016/12/06 21:40:56, piman wrote:
> nit: fix gl name

Done.

https://codereview.chromium.org/2559523002/diff/80001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:8574:
LOCAL_SET_GL_ERROR(GL_INVALID_VALUE,
On 2016/12/06 21:40:56, piman wrote:
> nit: make this a GL_INVALID_OPERATION, which is more consistent with
> cross-validation of state.

Done.

https://codereview.chromium.org/2559523002/diff/80001/gpu/command_buffer/serv...
gpu/command_buffer/service/gles2_cmd_decoder.cc:8575:
"glOverlayPromotionHintStreamTextureMatrix",
On 2016/12/06 21:40:56, piman wrote:
> nit: ditto

Done.

[liberato (no reviews please), 2016-12-12 16:44:43]

The CQ bit was checked by liberato@chromium.org

[liberato (no reviews please), 2016-12-12 16:44:43]

The patchset sent to the CQ was uploaded after l-g-t-m from piman@chromium.org
Link to the patchset: https://codereview.chromium.org/2559523002/#ps140001
(title: "fiddled with GLboolean for windows compiler")

[commit-bot: I haz the power, 2016-12-12 16:45:10]

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-12-12 18:52:40]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1481561083103420,
"parent_rev": "ece1a1ceebc0321d282e2861f40012b12f4563f4", "commit_rev":
"a49901bb0c5116650d676c3a9533db3e0be0c5f6"}

[commit-bot: I haz the power, 2016-12-12 18:53:07]

Description was changed from

==========
Send overlay promotion hints from to GLStreamTextureImage.

ResourceProvider knows if a resource has requested hints about
overlay promotion.  This CL closes the loop, allowing
ResourceProvider to notify a GLStreamTextureImage that's bound to
the resource's texture about the promotion hint.

This is supported only on Android.  The purpose is to let
AndroidVideoDecodeAccelerator figure out if video frames should be
backed by SurfaceTexture or SurfaceView.

BUG=671357
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Send overlay promotion hints from to GLStreamTextureImage.

ResourceProvider knows if a resource has requested hints about
overlay promotion.  This CL closes the loop, allowing
ResourceProvider to notify a GLStreamTextureImage that's bound to
the resource's texture about the promotion hint.

This is supported only on Android.  The purpose is to let
AndroidVideoDecodeAccelerator figure out if video frames should be
backed by SurfaceTexture or SurfaceView.

BUG=671357
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2559523002
==========

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

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-12-12 18:56:21]

Description was changed from

==========
Send overlay promotion hints from to GLStreamTextureImage.

ResourceProvider knows if a resource has requested hints about
overlay promotion.  This CL closes the loop, allowing
ResourceProvider to notify a GLStreamTextureImage that's bound to
the resource's texture about the promotion hint.

This is supported only on Android.  The purpose is to let
AndroidVideoDecodeAccelerator figure out if video frames should be
backed by SurfaceTexture or SurfaceView.

BUG=671357
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2559523002
==========

to

==========
Send overlay promotion hints from to GLStreamTextureImage.

ResourceProvider knows if a resource has requested hints about
overlay promotion.  This CL closes the loop, allowing
ResourceProvider to notify a GLStreamTextureImage that's bound to
the resource's texture about the promotion hint.

This is supported only on Android.  The purpose is to let
AndroidVideoDecodeAccelerator figure out if video frames should be
backed by SurfaceTexture or SurfaceView.

BUG=671357
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Committed: https://crrev.com/6408f27c941a3f90119958301b16f3c7cf2ebced
Cr-Commit-Position: refs/heads/master@{#437906}
==========

[commit-bot: I haz the power, 2016-12-12 18:56:22]

Patchset 8 (id:??) landed as
https://crrev.com/6408f27c941a3f90119958301b16f3c7cf2ebced
Cr-Commit-Position: refs/heads/master@{#437906}