Destroy egl image handles in the end of playback.

Destroy egl image handles in the end of playback.
Clean up a couple of unused variables.

BUG=NONE
TEST=omx_video_decode_accelerator_unittest

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

[vhiremath, 2011-07-06 12:58:17]

Ami,

We were not destroying egl image handles in the end of playback.

Please review.

Right now i'm doing this in FreeOutputBuffers.
Does this place ensure that the last frame rendering is done before clearing the
egl-handles ?

[vhiremath, 2011-07-06 13:11:20]

Tested with omx_video_decode_accelerator_unittest

[Ami GONE FROM CHROMIUM, 2011-07-06 15:52:38]

http://codereview.chromium.org/7311004/diff/1003/content/common/gpu/media/omx...
File content/common/gpu/media/omx_video_decode_accelerator.cc (right):

http://codereview.chromium.org/7311004/diff/1003/content/common/gpu/media/omx...
content/common/gpu/media/omx_video_decode_accelerator.cc:757:
texture2eglImage_translator.DestroyEglImage(egl_display_, egl_images_[i]);
This is a bit late to delete the image, as the underlying texture might already
be freed through the call to DismissPictureBuffer above.

IMO this should be changed to:
- OmxVideoDecodeAccelerator::OutputPicture gets a new field 
EGLImageKHR egl_image;
- Move the TranslateToEglImage call from AllocateOutputBuffers to
AssignGLESBuffers right before constructing the OutputPicture to be added to
pictures_
- Call DestroyEglImage(it->egl_image) right above the call to
client_->DismissPictureBuffer(it->first) above.

This has the benefits of:
- Keeping the book-keeping data together; all the "views" of a particular
picture are in the same place
- Makes destruction order mirror construction order.

[vhiremath, 2011-07-07 06:00:37]

http://codereview.chromium.org/7311004/diff/1003/content/common/gpu/media/omx...
File content/common/gpu/media/omx_video_decode_accelerator.cc (right):

http://codereview.chromium.org/7311004/diff/1003/content/common/gpu/media/omx...
content/common/gpu/media/omx_video_decode_accelerator.cc:757:
texture2eglImage_translator.DestroyEglImage(egl_display_, egl_images_[i]);
On 2011/07/06 15:52:38, Ami Fischman wrote:
> This is a bit late to delete the image, as the underlying texture might
already
> be freed through the call to DismissPictureBuffer above.
> 
> IMO this should be changed to:
> - OmxVideoDecodeAccelerator::OutputPicture gets a new field 
> EGLImageKHR egl_image;
> - Move the TranslateToEglImage call from AllocateOutputBuffers to
> AssignGLESBuffers right before constructing the OutputPicture to be added to
> pictures_
> - Call DestroyEglImage(it->egl_image) right above the call to
> client_->DismissPictureBuffer(it->first) above.
> 
> This has the benefits of:
> - Keeping the book-keeping data together; all the "views" of a particular
> picture are in the same place
> - Makes destruction order mirror construction order.

Done.

[Ami GONE FROM CHROMIUM, 2011-07-07 06:13:33]

LGTM
Thanks for doing this!

http://codereview.chromium.org/7311004/diff/5004/content/common/gpu/media/omx...
File content/common/gpu/media/omx_video_decode_accelerator.cc (right):

http://codereview.chromium.org/7311004/diff/5004/content/common/gpu/media/omx...
content/common/gpu/media/omx_video_decode_accelerator.cc:396: void* egl_image =
texture2eglImage_translator.TranslateToEglImage(
s/void*/EGLImageKHR/ here and below.

http://codereview.chromium.org/7311004/diff/5004/content/common/gpu/media/omx...
File content/common/gpu/media/omx_video_decode_accelerator.h (right):

http://codereview.chromium.org/7311004/diff/5004/content/common/gpu/media/omx...
content/common/gpu/media/omx_video_decode_accelerator.h:59: void* egl_image;
s/void*/EGLImageKHR/ here and above?

[vhiremath, 2011-07-07 06:30:10]

On 2011/07/07 06:13:33, Ami Fischman wrote:
> LGTM
> Thanks for doing this!
> 
>
http://codereview.chromium.org/7311004/diff/5004/content/common/gpu/media/omx...
> File content/common/gpu/media/omx_video_decode_accelerator.cc (right):
> 
>
http://codereview.chromium.org/7311004/diff/5004/content/common/gpu/media/omx...
> content/common/gpu/media/omx_video_decode_accelerator.cc:396: void* egl_image
=
> texture2eglImage_translator.TranslateToEglImage(
> s/void*/EGLImageKHR/ here and below.
> 
>
http://codereview.chromium.org/7311004/diff/5004/content/common/gpu/media/omx...
> File content/common/gpu/media/omx_video_decode_accelerator.h (right):
> 
>
http://codereview.chromium.org/7311004/diff/5004/content/common/gpu/media/omx...
> content/common/gpu/media/omx_video_decode_accelerator.h:59: void* egl_image;
> s/void*/EGLImageKHR/ here and above?

Done.
Need to include
#include "third_party/angle/include/EGL/eglext.h"

[commit-bot: I haz the power, 2011-07-07 07:55:24]

Change committed as 91677