hook up new MacVideoDecodeAccelerator

hook up new MacVideoDecodeAccelerator

This CL hooks up the new MacVideoDecodeAccelerator so that it can be instantiated by the HTML5 and pepper code. Currently only the pepper code uses it.

I also updated the pepper video_decode example app to be able to draw the GL_TEXTURE_RECTANGLE_ARB textures that the Mac decoder generates.

BUG=127414
TEST=

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=141302

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=141977

[jochen (gone - plz use gerrit), 2012-05-19 20:16:09]

https://chromiumcodereview.appspot.com/10411042/diff/12001/content/common/gpu...
File content/common/gpu/media/gpu_video_decode_accelerator.cc (right):

https://chromiumcodereview.appspot.com/10411042/diff/12001/content/common/gpu...
content/common/gpu/media/gpu_video_decode_accelerator.cc:26: #include
"base/win/windows_version.h"
nit. alphabetical ordering

[Ami GONE FROM CHROMIUM, 2012-05-19 20:46:11]

I'm not qualified to review the changes to gpu/* in this CL, please add an
appropriate reviewer for those bits.

This CL also depends on other unlanded CL(s).  IWBN to note them.

https://chromiumcodereview.appspot.com/10411042/diff/12001/content/common/gpu...
File content/common/gpu/media/gpu_video_decode_accelerator.cc (right):

https://chromiumcodereview.appspot.com/10411042/diff/12001/content/common/gpu...
content/common/gpu/media/gpu_video_decode_accelerator.cc:26: #include
"base/win/windows_version.h"
sort order

https://chromiumcodereview.appspot.com/10411042/diff/12001/content/common/gpu...
content/common/gpu/media/gpu_video_decode_accelerator.cc:133:
scoped_refptr<DXVAVideoDecodeAccelerator> video_decoder(
Since you're already duplicating the assignment to video_decode_accelerator_ in
each of the #if clauses, might as well drop the stack-local scoped_refptr
video_decoder in each of the clauses and just assign to v_d_a_ directly.

https://chromiumcodereview.appspot.com/10411042/diff/12001/content/common/gpu...
content/common/gpu/media/gpu_video_decode_accelerator.cc:148: #else  // Update
RenderViewImpl::createMediaPlayer when adding clauses.
I'm pretty sure you need to rebase this CL to pick up the newly landed VDA for
libVA.

https://chromiumcodereview.appspot.com/10411042/diff/12001/ppapi/examples/vid...
File ppapi/examples/video_decode/video_decode.cc (right):

https://chromiumcodereview.appspot.com/10411042/diff/12001/ppapi/examples/vid...
ppapi/examples/video_decode/video_decode.cc:43: struct PictureBufferInfo {
You could revert most of the changes to this file, and end up with much simpler
code, if instead of parameterizing the texture target you made it
platform-specific.

#if defined(OS_MAC)  // Or whatever the real define is
#define kTextureTarget GL_TEXTURE_RECTANGLE_ARB
#else
#define kTextureTarget GL_TEXTURE_2D
#endif

and similarly for defining the shader/program.

Basically there's no need to runtime-parameterize something that's compile-time
resolvable.

https://chromiumcodereview.appspot.com/10411042/diff/12001/ppapi/examples/vid...
ppapi/examples/video_decode/video_decode.cc:91: kNumDecoders = 1 };  // Baked
into viewport rendering.
Why these two changes?
Both of these are >1 to demonstrate that various things work correctly.

https://chromiumcodereview.appspot.com/10411042/diff/12001/ppapi/examples/vid...
ppapi/examples/video_decode/video_decode.cc:203: assert(false);
s/assert(false)/NOTREACHED()/

https://chromiumcodereview.appspot.com/10411042/diff/12001/ppapi/examples/vid...
ppapi/examples/video_decode/video_decode.cc:523:
gles2_if_->ClearColor(context_->pp_resource(), 1, 1, 0, 1);
why?

[sail, 2012-05-29 04:21:28]

https://chromiumcodereview.appspot.com/10411042/diff/12001/content/common/gpu...
File content/common/gpu/media/gpu_video_decode_accelerator.cc (right):

https://chromiumcodereview.appspot.com/10411042/diff/12001/content/common/gpu...
content/common/gpu/media/gpu_video_decode_accelerator.cc:133:
scoped_refptr<DXVAVideoDecodeAccelerator> video_decoder(
On 2012/05/19 20:46:12, Ami Fischman wrote:
> Since you're already duplicating the assignment to video_decode_accelerator_
in
> each of the #if clauses, might as well drop the stack-local scoped_refptr
> video_decoder in each of the clauses and just assign to v_d_a_ directly.

Unfortunately, the code still needs to be able to call the platform specific
functions for each decoder after creation. For example, SetEglState().

https://chromiumcodereview.appspot.com/10411042/diff/12001/content/common/gpu...
content/common/gpu/media/gpu_video_decode_accelerator.cc:148: #else  // Update
RenderViewImpl::createMediaPlayer when adding clauses.
On 2012/05/19 20:46:12, Ami Fischman wrote:
> I'm pretty sure you need to rebase this CL to pick up the newly landed VDA for
> libVA.

Yea definitely. I'll rebase all the CLs as things start to land.

https://chromiumcodereview.appspot.com/10411042/diff/12001/ppapi/examples/vid...
File ppapi/examples/video_decode/video_decode.cc (right):

https://chromiumcodereview.appspot.com/10411042/diff/12001/ppapi/examples/vid...
ppapi/examples/video_decode/video_decode.cc:43: struct PictureBufferInfo {
On 2012/05/19 20:46:12, Ami Fischman wrote:
> You could revert most of the changes to this file, and end up with much
simpler
> code, if instead of parameterizing the texture target you made it
> platform-specific.
> 
> #if defined(OS_MAC)  // Or whatever the real define is
> #define kTextureTarget GL_TEXTURE_RECTANGLE_ARB
> #else
> #define kTextureTarget GL_TEXTURE_2D
> #endif
> 
> and similarly for defining the shader/program.
> 
> Basically there's no need to runtime-parameterize something that's
compile-time
> resolvable.

As we discussed, this may need to be platform independent at some point.

https://chromiumcodereview.appspot.com/10411042/diff/12001/ppapi/examples/vid...
ppapi/examples/video_decode/video_decode.cc:91: kNumDecoders = 1 };  // Baked
into viewport rendering.
On 2012/05/19 20:46:12, Ami Fischman wrote:
> Why these two changes?
> Both of these are >1 to demonstrate that various things work correctly.

Ooops, reverted.

https://chromiumcodereview.appspot.com/10411042/diff/12001/ppapi/examples/vid...
ppapi/examples/video_decode/video_decode.cc:203: assert(false);
On 2012/05/19 20:46:12, Ami Fischman wrote:
> s/assert(false)/NOTREACHED()/

This code base uses assert instead of NOTREACHED().

https://chromiumcodereview.appspot.com/10411042/diff/12001/ppapi/examples/vid...
ppapi/examples/video_decode/video_decode.cc:523:
gles2_if_->ClearColor(context_->pp_resource(), 1, 1, 0, 1);
On 2012/05/19 20:46:12, Ami Fischman wrote:
> why?

Argh, more testing code. Reverted.

[Ami GONE FROM CHROMIUM, 2012-05-29 17:42:38]

lgtm

https://chromiumcodereview.appspot.com/10411042/diff/13001/ppapi/examples/vid...
File ppapi/examples/video_decode/video_decode.cc (right):

https://chromiumcodereview.appspot.com/10411042/diff/13001/ppapi/examples/vid...
ppapi/examples/video_decode/video_decode.cc:251:
gles2_if_->DeleteProgram(context_->pp_resource(), shader_2d_.program);
Should these DeleteProgram calls live in ~Shader(), instead?

https://chromiumcodereview.appspot.com/10411042/diff/13001/ppapi/examples/vid...
ppapi/examples/video_decode/video_decode.cc:453: // The video frame is upside
down so scale it by -1 and offset it by the
This is a bug, so I don't think you should have this workaround as part of the
example code.
http://crbug.com/130106

[sail, 2012-05-30 03:51:33]

https://chromiumcodereview.appspot.com/10411042/diff/12001/content/common/gpu...
File content/common/gpu/media/gpu_video_decode_accelerator.cc (right):

https://chromiumcodereview.appspot.com/10411042/diff/12001/content/common/gpu...
content/common/gpu/media/gpu_video_decode_accelerator.cc:26: #include
"base/win/windows_version.h"
On 2012/05/19 20:46:12, Ami Fischman wrote:
> sort order

Done.

https://chromiumcodereview.appspot.com/10411042/diff/12001/content/common/gpu...
content/common/gpu/media/gpu_video_decode_accelerator.cc:26: #include
"base/win/windows_version.h"
On 2012/05/19 20:16:11, jochen wrote:
> nit. alphabetical ordering

Done.

https://chromiumcodereview.appspot.com/10411042/diff/13001/ppapi/examples/vid...
File ppapi/examples/video_decode/video_decode.cc (right):

https://chromiumcodereview.appspot.com/10411042/diff/13001/ppapi/examples/vid...
ppapi/examples/video_decode/video_decode.cc:251:
gles2_if_->DeleteProgram(context_->pp_resource(), shader_2d_.program);
On 2012/05/29 17:42:38, Ami Fischman wrote:
> Should these DeleteProgram calls live in ~Shader(), instead?

Currently Shader is just a POD and gets copied around.

https://chromiumcodereview.appspot.com/10411042/diff/13001/ppapi/examples/vid...
ppapi/examples/video_decode/video_decode.cc:453: // The video frame is upside
down so scale it by -1 and offset it by the
On 2012/05/29 17:42:38, Ami Fischman wrote:
> This is a bug, so I don't think you should have this workaround as part of the
> example code.
> http://crbug.com/130106

Done.

[commit-bot: I haz the power, 2012-06-08 19:45:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sail@chromium.org/10411042/18002

[commit-bot: I haz the power, 2012-06-08 22:14:14]

Change committed as 141302

[commit-bot: I haz the power, 2012-06-09 00:54:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sail@chromium.org/10411042/32002

[commit-bot: I haz the power, 2012-06-09 01:13:10]

Try job failure for 10411042-32002 (retry) on mac_rel for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-06-13 19:52:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sail@chromium.org/10411042/38001

[commit-bot: I haz the power, 2012-06-13 21:18:12]

Change committed as 141977