OmxVideoDecodeAcceleratorTest improvements:

OmxVideoDecodeAcceleratorTest improvements: 
- Added support for decoding multiple NALUs per Decode() call.  Found & filed
  bug to nvidia 
  (http://code.google.com/p/chrome-os-partner/issues/detail?id=4378)
- Added support for running multiple decoders in parallel.
- Added support for tracking decoding frames-per-second & asserting reasonable
  performance.

FPS numbers from a random run, for flavor:
- Single decoder swapping to display yields ~54fps
- Three decoders swapping to display yield ~18-19fps each
- Single decoder *not* swapping to display yields 195-220fps in different chunking configurations.

BUG=none
TEST=run the test

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

[Ami GONE FROM CHROMIUM, 2011-06-07 21:55:41]

brettw: OWNERS please
vrk: please review

[vrk (LEFT CHROMIUM), 2011-06-09 20:36:11]

LGTM with a few nits!

http://codereview.chromium.org/7046031/diff/1/content/common/gpu/omx_video_de...
File content/common/gpu/omx_video_decode_accelerator_unittest.cc (left):

http://codereview.chromium.org/7046031/diff/1/content/common/gpu/omx_video_de...
content/common/gpu/omx_video_decode_accelerator_unittest.cc:583: // - Test
multiple decodes; sequentially & concurrently.
In what way did you want to test decodes sequentially? (I guess the other
question I'm implying is, are we testing multiple decodes sequentially in this
unit test now?)

http://codereview.chromium.org/7046031/diff/1/content/common/gpu/omx_video_de...
File content/common/gpu/omx_video_decode_accelerator_unittest.cc (right):

http://codereview.chromium.org/7046031/diff/1/content/common/gpu/omx_video_de...
content/common/gpu/omx_video_decode_accelerator_unittest.cc:198: // Arrange X
windows side by side whimsically.
:)

http://codereview.chromium.org/7046031/diff/1/content/common/gpu/omx_video_de...
content/common/gpu/omx_video_decode_accelerator_unittest.cc:216:
CHECK(eglMakeCurrent(egl_display_, egl_surface, egl_surface, egl_context_))
What's the reason for this eglMakeCurrent call here?

http://codereview.chromium.org/7046031/diff/1/content/common/gpu/omx_video_de...
content/common/gpu/omx_video_decode_accelerator_unittest.cc:427: // Compute &
return in |*end_pos| the end position for the next batch of NALs
teeny tiny nit: should all these NALs be NALUs?

http://codereview.chromium.org/7046031/diff/1/content/common/gpu/omx_video_de...
content/common/gpu/omx_video_decode_accelerator_unittest.cc:499: void
EglRenderingVDAClient::DismissPictureBuffer(int32 picture_buffer_id) {
Didn't catch this before, but I think you also have to delete textures after you
dismiss a picture buffer to free the texture memory. This is done by
glDeleteTextures (http://www.opengl.org/sdk/docs/man/xhtml/glDeleteTextures.xml)

http://codereview.chromium.org/7046031/diff/1/content/common/gpu/omx_video_de...
content/common/gpu/omx_video_decode_accelerator_unittest.cc:623:
base::ShadowingAtExitManager at_exit_manager_;
nit: no ending _

http://codereview.chromium.org/7046031/diff/1/content/common/gpu/omx_video_de...
content/common/gpu/omx_video_decode_accelerator_unittest.cc:628: // Suppress EGL
surface swapping in all but one test, to cut down overall test
Looks like 2 tests have num_NALs_per_decode == 1?

[Ami GONE FROM CHROMIUM, 2011-06-09 23:22:25]

http://codereview.chromium.org/7046031/diff/1/content/common/gpu/omx_video_de...
File content/common/gpu/omx_video_decode_accelerator_unittest.cc (left):

http://codereview.chromium.org/7046031/diff/1/content/common/gpu/omx_video_de...
content/common/gpu/omx_video_decode_accelerator_unittest.cc:583: // - Test
multiple decodes; sequentially & concurrently.
On 2011/06/09 20:36:11, Victoria Kirst wrote:
> In what way did you want to test decodes sequentially?

Play one stream, then play another, with the same decoder.  I realized the
previous phrasing wasn't super-clear which is why I replaced it with
seek/stop/pause/play/replay.  The interesting bit isn't the different streams
per-se, but rather making sure the state machine works the way we expect it to.

> (I guess the other
> question I'm implying is, are we testing multiple decodes sequentially in this
> unit test now?)

Nope.

http://codereview.chromium.org/7046031/diff/1/content/common/gpu/omx_video_de...
File content/common/gpu/omx_video_decode_accelerator_unittest.cc (right):

http://codereview.chromium.org/7046031/diff/1/content/common/gpu/omx_video_de...
content/common/gpu/omx_video_decode_accelerator_unittest.cc:198: // Arrange X
windows side by side whimsically.
On 2011/06/09 20:36:11, Victoria Kirst wrote:
> :)

Done.

http://codereview.chromium.org/7046031/diff/1/content/common/gpu/omx_video_de...
content/common/gpu/omx_video_decode_accelerator_unittest.cc:216:
CHECK(eglMakeCurrent(egl_display_, egl_surface, egl_surface, egl_context_))
On 2011/06/09 20:36:11, Victoria Kirst wrote:
> What's the reason for this eglMakeCurrent call here?

Without it the shaders fail to compile, presumably b/c GL needs an
active/current context to do much of anything.

http://codereview.chromium.org/7046031/diff/1/content/common/gpu/omx_video_de...
content/common/gpu/omx_video_decode_accelerator_unittest.cc:427: // Compute &
return in |*end_pos| the end position for the next batch of NALs
On 2011/06/09 20:36:11, Victoria Kirst wrote:
> teeny tiny nit: should all these NALs be NALUs?

Done.

http://codereview.chromium.org/7046031/diff/1/content/common/gpu/omx_video_de...
content/common/gpu/omx_video_decode_accelerator_unittest.cc:499: void
EglRenderingVDAClient::DismissPictureBuffer(int32 picture_buffer_id) {
On 2011/06/09 20:36:11, Victoria Kirst wrote:
> Didn't catch this before, but I think you also have to delete textures after
you
> dismiss a picture buffer to free the texture memory. This is done by
> glDeleteTextures
(http://www.opengl.org/sdk/docs/man/xhtml/glDeleteTextures.xml)

Done.

http://codereview.chromium.org/7046031/diff/1/content/common/gpu/omx_video_de...
content/common/gpu/omx_video_decode_accelerator_unittest.cc:623:
base::ShadowingAtExitManager at_exit_manager_;
On 2011/06/09 20:36:11, Victoria Kirst wrote:
> nit: no ending _

Done.

http://codereview.chromium.org/7046031/diff/1/content/common/gpu/omx_video_de...
content/common/gpu/omx_video_decode_accelerator_unittest.cc:628: // Suppress EGL
surface swapping in all but one test, to cut down overall test
On 2011/06/09 20:36:11, Victoria Kirst wrote:
> Looks like 2 tests have num_NALs_per_decode == 1?

Done.

[Ami GONE FROM CHROMIUM, 2011-06-09 23:24:54]

+kbr for OWNERS approval
-brettw (who is on vacation; d'oh!)

[Ken Russell (switch to Gerrit), 2011-06-10 00:25:00]

LGTM from OWNERS standpoint. Yay tests.