Implement "hole" video frame.

Implement "hole" video frame.

In order to support out-of-band compositing of protected video
content in Chrome, it is necessary to have a transparent hole in
Chrome's HTML rendering so that video playing "beneath" the HTML
elements can be seen through.
    
To accomplish the scenario stated above, define a "hole" video frame
which would clear the region where video would have been composited.
In CC, this is done by sending a transparent black SolidColorDrawQuad
with blending disabled.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=185841

[wonsik2, 2013-02-14 14:31:28]

This change is separated from https://codereview.chromium.org/11442056 to
discuss the impact on core rendering with more focus. Please take a look.

[Tom Hudson, 2013-02-14 14:38:44]

In the comments on 11442056, there was mention of a thread being started to
discuss options. Is that thread archived anywhere? The early draft design doc
was informative, but incomplete, and the comments there petered out in 2011.

[wonsik2, 2013-02-14 14:55:27]

On 2013/02/14 14:38:44, Tom Hudson wrote:
> In the comments on 11442056, there was mention of a thread being started to
> discuss options. Is that thread archived anywhere? The early draft design doc
> was informative, but incomplete, and the comments there petered out in 2011.

The thread is archived in
https://groups.google.com/a/google.com/d/topic/encrypted-media/aNE8IN01SJA/di...
. The thread is slightly outdated, and you can contact darin@ for the latest
status of the discussion.

[jamesr, 2013-02-18 05:06:31]

This seems to have the exact same problem that the previous version had.  cc
uses intermediate framebuffers and readbacks/texture copies for correctness and
effects.  This won't work when those are in effect.  This also restricts
blending possibilities in a way that won't be feasible to maintain.

Stepping back, there are two possibilities for any scene involving a video
intended to be rendered with this technique:

1.) There is no content 'below' the video and the video is not transformed in
any way that would preclude hole-punching from working.  In this situation, you
should either arrange for there to be no content rendering in the area you want
to video to show up in or render the scene in the normal way and clear out the
video area as a post-compositing step.

2.) There is significant content below the video, complex transforms,
clipping/effects that require intermediate fbos, or any other thing that breaks
hole punching.  In this case, there's no way to display the video no matter
what's done in cc or elsewhere.

Neither case can be improved with this patch.

[Vangelis Kokkevis, 2013-02-21 18:52:54]

https://codereview.chromium.org/12255032/diff/1/cc/gl_renderer.cc
File cc/gl_renderer.cc (right):

https://codereview.chromium.org/12255032/diff/1/cc/gl_renderer.cc#newcode992
cc/gl_renderer.cc:992: if (quad->texture_id == 0) {
I'm not too fond of the idea of using texture_id == 0 as a signal to punch a
hole, as it's also a possible valid failure condition. I'm thinking that we
should create a new quad type for holes that also uses a much simpler shader (no
need to do any sort of blending, just blast zeros to the backbuffer).

[Vangelis Kokkevis, 2013-02-21 19:00:39]

There's a couple more things I would like to see to make this change safer
(which may apply really to the other patch with the Android code):
1. Verify that the video layer is rendering into the default render surface.
2. Check that the draw transform of the video layer is indeed axis aligned

If the two conditions above don't apply then we have to figure out what we want
to do.  We can either have some sort of standing texture to alert the developer
of what's going on or just skip it altogether (but not punch a hole in the
page).

[danakj, 2013-02-21 19:01:51]

https://codereview.chromium.org/12255032/diff/1/cc/gl_renderer.cc
File cc/gl_renderer.cc (right):

https://codereview.chromium.org/12255032/diff/1/cc/gl_renderer.cc#newcode992
cc/gl_renderer.cc:992: if (quad->texture_id == 0) {
On 2013/02/21 18:52:54, Vangelis Kokkevis wrote:
> I'm not too fond of the idea of using texture_id == 0 as a signal to punch a
> hole, as it's also a possible valid failure condition. I'm thinking that we
> should create a new quad type for holes that also uses a much simpler shader
(no
> need to do any sort of blending, just blast zeros to the backbuffer). 

That sounds like a solid color quad with needs blending forced off?

[wonsik2, 2013-02-26 04:52:24]

https://codereview.chromium.org/12255032/diff/1/cc/gl_renderer.cc
File cc/gl_renderer.cc (right):

https://codereview.chromium.org/12255032/diff/1/cc/gl_renderer.cc#newcode992
cc/gl_renderer.cc:992: if (quad->texture_id == 0) {
On 2013/02/21 18:52:54, Vangelis Kokkevis wrote:
> I'm not too fond of the idea of using texture_id == 0 as a signal to punch a
> hole, as it's also a possible valid failure condition. I'm thinking that we
> should create a new quad type for holes that also uses a much simpler shader
(no
> need to do any sort of blending, just blast zeros to the backbuffer). 

Created a new quad type; PTAL

https://codereview.chromium.org/12255032/diff/1/cc/gl_renderer.cc#newcode992
cc/gl_renderer.cc:992: if (quad->texture_id == 0) {
On 2013/02/21 19:01:51, danakj wrote:
> On 2013/02/21 18:52:54, Vangelis Kokkevis wrote:
> > I'm not too fond of the idea of using texture_id == 0 as a signal to punch a
> > hole, as it's also a possible valid failure condition. I'm thinking that we
> > should create a new quad type for holes that also uses a much simpler shader
> (no
> > need to do any sort of blending, just blast zeros to the backbuffer). 
> 
> That sounds like a solid color quad with needs blending forced off?

I tried SolidColorDrawQuad with needs_blending == false, but it didn't work as
expected.

I ended up implementing a new quad. PTAL.

[wonsik2, 2013-02-26 06:47:03]

https://codereview.chromium.org/12255032/diff/1/cc/gl_renderer.cc
File cc/gl_renderer.cc (right):

https://codereview.chromium.org/12255032/diff/1/cc/gl_renderer.cc#newcode992
cc/gl_renderer.cc:992: if (quad->texture_id == 0) {
On 2013/02/26 04:52:24, wonsik wrote:
> On 2013/02/21 19:01:51, danakj wrote:
> > On 2013/02/21 18:52:54, Vangelis Kokkevis wrote:
> > > I'm not too fond of the idea of using texture_id == 0 as a signal to punch
a
> > > hole, as it's also a possible valid failure condition. I'm thinking that
we
> > > should create a new quad type for holes that also uses a much simpler
shader
> > (no
> > > need to do any sort of blending, just blast zeros to the backbuffer). 
> > 
> > That sounds like a solid color quad with needs blending forced off?
> 
> I tried SolidColorDrawQuad with needs_blending == false, but it didn't work as
> expected.
> 
> I ended up implementing a new quad. PTAL.

With further investigation, it turned out that an approach with
SolidColorDrawQuad worked.

[jamesr, 2013-02-26 07:31:35]

https://codereview.chromium.org/12255032/diff/9002/cc/video_layer_impl.cc
File cc/video_layer_impl.cc (right):

https://codereview.chromium.org/12255032/diff/9002/cc/video_layer_impl.cc#new...
cc/video_layer_impl.cc:235: #if defined(GOOGLE_TV)
please put a comment in here below the #if defined() with a list of contact
email addresses for this code and explaining that this block is only maintained
by those people and not the general compositor team

[wonsik2, 2013-02-27 02:16:41]

https://codereview.chromium.org/12255032/diff/9002/cc/video_layer_impl.cc
File cc/video_layer_impl.cc (right):

https://codereview.chromium.org/12255032/diff/9002/cc/video_layer_impl.cc#new...
cc/video_layer_impl.cc:235: #if defined(GOOGLE_TV)
On 2013/02/26 07:31:35, jamesr wrote:
> please put a comment in here below the #if defined() with a list of contact
> email addresses for this code and explaining that this block is only
maintained
> by those people and not the general compositor team

Done.

[jamesr, 2013-02-27 02:59:31]

lgtm for cc/.

I don't have OWNERS for the rest.  I'd suggest scherkus@ for the media/ changes.

[vangelis, 2013-02-27 04:56:45]

https://codereview.chromium.org/12255032/diff/12001/cc/video_layer_impl.cc
File cc/video_layer_impl.cc (right):

https://codereview.chromium.org/12255032/diff/12001/cc/video_layer_impl.cc#ne...
cc/video_layer_impl.cc:244: scoped_ptr<SolidColorDrawQuad> solidColorDrawQuad =
SolidColorDrawQuad::Create();
80 col limit. Here and below.

[scherkus (not reviewing), 2013-02-27 06:54:20]

wonsik: code by itself is looking fine but do you have a CL in progress that
contains the changes that use the new VideoFrame::HOLE code path?

If not, could you describe how https://codereview.chromium.org/11442056 would
change?

https://codereview.chromium.org/12255032/diff/12001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_controller.cc (right):

https://codereview.chromium.org/12255032/diff/12001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_controller.cc:426:
NOTREACHED();
do you need to duplicate the { NOTREACHED(); break; } block?

what about letting it fall through to the one below?

[wonsik2, 2013-02-27 10:19:29]

scherkus: I am working on a CL to use VideoFrame::HOLE, but it's not ready for
upload yet. For a brief summary, the CL will change
webkit_media::WebMediaPlayerAndroid::ReallocateVideoFrame() to allocate
WebVideoFrameImpl with the following code, when the video is flagged as
out-of-band compositing:

    video_frame_.reset(new
WebVideoFrameImpl(VideoFrame::CreateHoleFrame(natural_size_)));

I will try to complete and upload the CL soon.

https://codereview.chromium.org/12255032/diff/12001/cc/video_layer_impl.cc
File cc/video_layer_impl.cc (right):

https://codereview.chromium.org/12255032/diff/12001/cc/video_layer_impl.cc#ne...
cc/video_layer_impl.cc:244: scoped_ptr<SolidColorDrawQuad> solidColorDrawQuad =
SolidColorDrawQuad::Create();
On 2013/02/27 04:56:46, vangelis wrote:
> 80 col limit. Here and below.

Done.

https://codereview.chromium.org/12255032/diff/12001/content/browser/renderer_...
File content/browser/renderer_host/media/video_capture_controller.cc (right):

https://codereview.chromium.org/12255032/diff/12001/content/browser/renderer_...
content/browser/renderer_host/media/video_capture_controller.cc:426:
NOTREACHED();
On 2013/02/27 06:54:20, scherkus wrote:
> do you need to duplicate the { NOTREACHED(); break; } block?
> 
> what about letting it fall through to the one below?

Done. Added a short comment for a little safeguard against possible mistakes
around ifdef's and switch fall-through.

[scherkus (not reviewing), 2013-02-27 18:47:35]

lgtm

[commit-bot: I haz the power, 2013-03-04 02:06:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wonsik@google.com/12255032/7002

[commit-bot: I haz the power, 2013-03-04 02:08:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wonsik@google.com/12255032/7002

[commit-bot: I haz the power, 2013-03-04 02:11:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wonsik@google.com/12255032/7002

[commit-bot: I haz the power, 2013-03-04 03:37:51]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win7_aura.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-03-04 05:52:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wonsik@google.com/12255032/7002

[commit-bot: I haz the power, 2013-03-04 06:31:22]

Change committed as 185841