Plumb allow_overlay flag for video path into cc

reland: Plumb allow_overlay flag for video path into cc

This will allow hardware video overlays to be enabled in the compositor

Committed: https://crrev.com/91d8e7f8c58a1278ae3ae8e721d4bd0c9d68ff72
Cr-Commit-Position: refs/heads/master@{#313806}

[achaulk, 2015-01-19 18:11:37]

achaulk@chromium.org changed reviewers:
+ jschuh@chromium.org, piman@chromium.org, vrk@chromium.org

[achaulk, 2015-01-19 18:22:47]

Need security review for IPC change

vrk for media/
piman for cc/ & content/

[achaulk, 2015-01-19 18:36:06]

Patchset #3 (id:40001) has been deleted

[achaulk, 2015-01-19 18:36:11]

Patchset #2 (id:20001) has been deleted

[alexst (slow to review), 2015-01-19 19:59:31]

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

[alexst (slow to review), 2015-01-19 19:59:31]

https://codereview.chromium.org/806413004/diff/1/content/common/gpu/gpu_messa...
File content/common/gpu/gpu_messages.h (right):

https://codereview.chromium.org/806413004/diff/1/content/common/gpu/gpu_messa...
content/common/gpu/gpu_messages.h:695: bool)      /* Is overlay buffer */
Driveby nit: "Is HW overlay capable" or something to that extent might be  more
descriptive.

[DaleCurtis, 2015-01-20 18:32:05]

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

[DaleCurtis, 2015-01-20 18:32:06]

Can you explain what this is in more detail? As well as how  it is different
than the VIDEO_HOLE path for VideoFrames?

[DaleCurtis, 2015-01-20 18:32:25]

dalecurtis@chromium.org changed reviewers:
- vrk@chromium.org

[achaulk, 2015-01-20 18:36:19]

This is intended to eventually replace the VIDEO_HOLE mechanism.

This ties into the overlay mechanisms already present in cc, which allows us to
split certain quads off and send them directly to the HW, bypassing the GPU
compositing, but we can only do it to specially allocated textures, this plumbs
that capability flag through

[DaleCurtis, 2015-01-20 18:40:04]

That's great to hear! Can you elaborate on why you chose to use getters/setters
vs making this part of the Picture and WrapNativeTexture constructor? Is this a
value we expect to change?

[piman, 2015-01-20 19:05:48]

LGTM for content/ and cc/

[achaulk, 2015-01-20 19:16:48]

On 2015/01/20 18:40:04, DaleCurtis wrote:
> That's great to hear! Can you elaborate on why you chose to use
getters/setters
> vs making this part of the Picture and WrapNativeTexture constructor? Is this
a
> value we expect to change?

It won't change over the life, no, it's either supported or not and we should
know that at create time. I mostly did that to get things running quicker. I can
make them params instead if you want

[DaleCurtis, 2015-01-20 19:19:17]

Yes, they should be constructor flags then. Thanks!

[achaulk, 2015-01-20 19:39:30]

On 2015/01/20 19:19:17, DaleCurtis wrote:
> Yes, they should be constructor flags then. Thanks!

Done

[DaleCurtis, 2015-01-20 19:52:26]

One last question, are NATIVE_TEXTURE and allow_overlay synonymous?

[achaulk, 2015-01-20 19:55:26]

On 2015/01/20 19:52:26, DaleCurtis wrote:
> One last question, are NATIVE_TEXTURE and allow_overlay synonymous?

From a quick look, NATIVE_TEXTURE simply specifies a hardware buffer, while
allow_overlay is a special kind of hardware buffer. So allow_overlay probably
implies NATIVE_TEXTURE, but not the other way around

[DaleCurtis, 2015-01-20 20:33:26]

media/ lgtm then, thanks!

[jschuh, 2015-01-21 00:21:58]

ipc security lgtm. (notes: opaque bool)

[Pawel Osciak, 2015-01-21 13:09:02]

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

[Pawel Osciak, 2015-01-21 13:09:03]

https://chromiumcodereview.appspot.com/806413004/diff/60001/media/video/pictu...
File media/video/picture.h (right):

https://chromiumcodereview.appspot.com/806413004/diff/60001/media/video/pictu...
media/video/picture.h:61: bool allow_overlay);
Could we get this property from PictureBuffer? Do we need it on both Picture and
PictureBuffer?

[achaulk, 2015-01-21 17:12:04]

https://chromiumcodereview.appspot.com/806413004/diff/1/content/common/gpu/gp...
File content/common/gpu/gpu_messages.h (right):

https://chromiumcodereview.appspot.com/806413004/diff/1/content/common/gpu/gp...
content/common/gpu/gpu_messages.h:695: bool)      /* Is overlay buffer */
On 2015/01/19 19:59:31, alexst wrote:
> Driveby nit: "Is HW overlay capable" or something to that extent might be 
more
> descriptive.

Done.

https://chromiumcodereview.appspot.com/806413004/diff/60001/media/video/pictu...
File media/video/picture.h (right):

https://chromiumcodereview.appspot.com/806413004/diff/60001/media/video/pictu...
media/video/picture.h:61: bool allow_overlay);
On 2015/01/21 13:09:03, Pawel Osciak wrote:
> Could we get this property from PictureBuffer? Do we need it on both Picture
and
> PictureBuffer?

It doesn't seem to be required on the PictureBuffer, at least for the current
use

[achaulk, 2015-01-23 20:27:46]

The CQ bit was checked by achaulk@chromium.org

[commit-bot: I haz the power, 2015-01-23 20:28:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/806413004/60001

[commit-bot: I haz the power, 2015-01-23 20:34:01]

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

[commit-bot: I haz the power, 2015-01-23 20:34:03]

Try jobs failed on following builders:
  ios_rel_device_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ng...)

Try jobs failed on following builders:
  ios_rel_device_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ng...)

[achaulk, 2015-01-27 17:54:33]

The CQ bit was checked by achaulk@chromium.org

[commit-bot: I haz the power, 2015-01-27 17:54:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/806413004/80001

[commit-bot: I haz the power, 2015-01-27 18:24:42]

Committed patchset #3 (id:80001)

[commit-bot: I haz the power, 2015-01-27 18:27:23]

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

[James Cook, 2015-01-27 20:21:20]

A revert of this CL (patchset #3 id:80001) has been created in
https://codereview.chromium.org/881893002/ by jamescook@chromium.org.

The reason for reverting is: This breaks the Chrome OS daisy (32-bit ARM)
builder:

http://build.chromium.org/p/chromiumos.chromium/builders/Daisy%20%28chromium%...

It just looks like a missing callsite.
.

[achaulk, 2015-01-27 21:21:47]

The CQ bit was checked by achaulk@chromium.org

[commit-bot: I haz the power, 2015-01-27 21:22:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/806413004/100001

[achaulk, 2015-01-27 21:29:30]

The CQ bit was unchecked by achaulk@chromium.org

[achaulk, 2015-01-29 22:12:03]

The CQ bit was checked by achaulk@chromium.org

[commit-bot: I haz the power, 2015-01-29 22:13:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/806413004/100001

[commit-bot: I haz the power, 2015-01-29 22:16:37]

Committed patchset #4 (id:100001)

[commit-bot: I haz the power, 2015-01-29 22:18:41]

Patchset 4 (id:??) landed as
https://crrev.com/91d8e7f8c58a1278ae3ae8e721d4bd0c9d68ff72
Cr-Commit-Position: refs/heads/master@{#313806}