V4L2VideoDecodeAccelerator: destroy buffers in decoder thread.

V4L2VideoDecodeAccelerator: destroy buffers in decoder thread.

Move destroying input and output buffers from child thread to
decoder thread has two benefits. (1) Book accounting variables
like output_buffer_map_ are only accessed in decoder thread.
(2) kChangingResolution used to mean waiting for output buffers
to destroy or waiting for processor to return frames. Now it
only means waiting for processor. The code is simpler.

BUG=b/29059119
TEST=Run VDA unittest, run video_VideoSeek test and play video on
elm and peach pit.

Committed: https://crrev.com/a9fcabff6db188adb5ef1a16803e7a7a5d8a05eb
Cr-Commit-Position: refs/heads/master@{#420027}

[wuchengli, 2016-09-12 09:09:46]

Patchset #1 (id:1) has been deleted

[wuchengli, 2016-09-12 09:10:52]

wuchengli@chromium.org changed reviewers:
+ kcwu@chromium.org, posciak@chromium.org

[wuchengli, 2016-09-12 09:10:53]

PTAL.

[kcwu, 2016-09-12 10:59:48]

https://codereview.chromium.org/2335573002/diff/20001/media/gpu/v4l2_video_de...
File media/gpu/v4l2_video_decode_accelerator.cc (right):

https://codereview.chromium.org/2335573002/diff/20001/media/gpu/v4l2_video_de...
media/gpu/v4l2_video_decode_accelerator.cc:2161:
base::Bind(base::IgnoreResult(&V4L2Device::DestroyEGLImage), device_,
Add log if destroy failed at least.

https://codereview.chromium.org/2335573002/diff/20001/media/gpu/v4l2_video_de...
media/gpu/v4l2_video_decode_accelerator.cc:2166: if
(eglDestroySyncKHR(egl_display_, output_record.egl_sync) != EGL_TRUE) {
Is it okay to call eglDestroySyncKHR on non-child thread?

[wuchengli, 2016-09-12 11:15:36]

https://codereview.chromium.org/2335573002/diff/20001/media/gpu/v4l2_video_de...
File media/gpu/v4l2_video_decode_accelerator.cc (right):

https://codereview.chromium.org/2335573002/diff/20001/media/gpu/v4l2_video_de...
media/gpu/v4l2_video_decode_accelerator.cc:2161:
base::Bind(base::IgnoreResult(&V4L2Device::DestroyEGLImage), device_,
On 2016/09/12 10:59:48, kcwu wrote:
> Add log if destroy failed at least.
V4L2SVDA has three base::IgnoreResult(&V4L2Device::DestroyEGLImage). I'll add in
GenericV4L2Device::DestroyEGLImage and see what Pawel thinks.

https://codereview.chromium.org/2335573002/diff/20001/media/gpu/v4l2_video_de...
media/gpu/v4l2_video_decode_accelerator.cc:2166: if
(eglDestroySyncKHR(egl_display_, output_record.egl_sync) != EGL_TRUE) {
On 2016/09/12 10:59:48, kcwu wrote:
> Is it okay to call eglDestroySyncKHR on non-child thread?
Yes. It's OK. Also see line 1296. eglDestroySyncKHR is called on decode thread.

[wuchengli, 2016-09-13 09:26:15]

PTAL

[kcwu, 2016-09-13 09:31:26]

lgtm

[wuchengli, 2016-09-14 08:32:29]

Patchset #3 (id:60001) has been deleted

[wuchengli, 2016-09-14 08:47:01]

Patchset #4 (id:100001) has been deleted

[Pawel Osciak, 2016-09-15 07:51:43]

lgtm % nits

https://codereview.chromium.org/2335573002/diff/120001/media/gpu/v4l2_video_d...
File media/gpu/v4l2_video_decode_accelerator.h (right):

https://codereview.chromium.org/2335573002/diff/120001/media/gpu/v4l2_video_d...
media/gpu/v4l2_video_decode_accelerator.h:136: // Performing resolution change
and waiting image processor to return all
s/waiting/waiting for/

https://codereview.chromium.org/2335573002/diff/120001/media/gpu/v4l2_video_d...
media/gpu/v4l2_video_decode_accelerator.h:334: // In contrast to
DestroyInputBuffers, which is called only from destructor,
Nit: s/from destructor/on destruction/

[wuchengli, 2016-09-21 06:44:58]

https://codereview.chromium.org/2335573002/diff/120001/media/gpu/v4l2_video_d...
File media/gpu/v4l2_video_decode_accelerator.h (right):

https://codereview.chromium.org/2335573002/diff/120001/media/gpu/v4l2_video_d...
media/gpu/v4l2_video_decode_accelerator.h:136: // Performing resolution change
and waiting image processor to return all
On 2016/09/15 07:51:42, Pawel Osciak wrote:
> s/waiting/waiting for/

Done.

https://codereview.chromium.org/2335573002/diff/120001/media/gpu/v4l2_video_d...
media/gpu/v4l2_video_decode_accelerator.h:334: // In contrast to
DestroyInputBuffers, which is called only from destructor,
On 2016/09/15 07:51:42, Pawel Osciak wrote:
> Nit: s/from destructor/on destruction/

Done.

[wuchengli, 2016-09-21 06:45:54]

Description was changed from

==========
V4L2VideoDecodeAccelerator: destroy buffers in decoder thread.

Move destroying input and output buffers from child thread to
decoder thread has two benefits. (1) Book accounting variables
like output_buffer_map_ are only accessed in decoder thread.
(2) kChangingResolution used to mean waiting output buffers to
destroy or waiting processor to return frames. Now it only means
waiting processor. The code is simpler.

BUG=b/29059119
TEST=Run VDA unittest, run video_VideoSeek test and play video on
elm and peach pit.
==========

to

==========
V4L2VideoDecodeAccelerator: destroy buffers in decoder thread.

Move destroying input and output buffers from child thread to
decoder thread has two benefits. (1) Book accounting variables
like output_buffer_map_ are only accessed in decoder thread.
(2) kChangingResolution used to mean waiting for output buffers to
destroy or waiting for processor to return frames. Now it only means
waiting for processor. The code is simpler.

BUG=b/29059119
TEST=Run VDA unittest, run video_VideoSeek test and play video on
elm and peach pit.
==========

[wuchengli, 2016-09-21 06:46:07]

Description was changed from

==========
V4L2VideoDecodeAccelerator: destroy buffers in decoder thread.

Move destroying input and output buffers from child thread to
decoder thread has two benefits. (1) Book accounting variables
like output_buffer_map_ are only accessed in decoder thread.
(2) kChangingResolution used to mean waiting for output buffers to
destroy or waiting for processor to return frames. Now it only means
waiting for processor. The code is simpler.

BUG=b/29059119
TEST=Run VDA unittest, run video_VideoSeek test and play video on
elm and peach pit.
==========

to

==========
V4L2VideoDecodeAccelerator: destroy buffers in decoder thread.

Move destroying input and output buffers from child thread to
decoder thread has two benefits. (1) Book accounting variables
like output_buffer_map_ are only accessed in decoder thread.
(2) kChangingResolution used to mean waiting for output buffers
to destroy or waiting for processor to return frames. Now it only
means waiting for processor. The code is simpler.

BUG=b/29059119
TEST=Run VDA unittest, run video_VideoSeek test and play video on
elm and peach pit.
==========

[wuchengli, 2016-09-21 06:46:17]

Description was changed from

==========
V4L2VideoDecodeAccelerator: destroy buffers in decoder thread.

Move destroying input and output buffers from child thread to
decoder thread has two benefits. (1) Book accounting variables
like output_buffer_map_ are only accessed in decoder thread.
(2) kChangingResolution used to mean waiting for output buffers
to destroy or waiting for processor to return frames. Now it only
means waiting for processor. The code is simpler.

BUG=b/29059119
TEST=Run VDA unittest, run video_VideoSeek test and play video on
elm and peach pit.
==========

to

==========
V4L2VideoDecodeAccelerator: destroy buffers in decoder thread.

Move destroying input and output buffers from child thread to
decoder thread has two benefits. (1) Book accounting variables
like output_buffer_map_ are only accessed in decoder thread.
(2) kChangingResolution used to mean waiting for output buffers
to destroy or waiting for processor to return frames. Now it
only means waiting for processor. The code is simpler.

BUG=b/29059119
TEST=Run VDA unittest, run video_VideoSeek test and play video on
elm and peach pit.
==========

[wuchengli, 2016-09-21 09:00:50]

The CQ bit was checked by wuchengli@chromium.org

[wuchengli, 2016-09-21 09:00:50]

The patchset sent to the CQ was uploaded after l-g-t-m from kcwu@chromium.org,
posciak@chromium.org
Link to the patchset: https://codereview.chromium.org/2335573002/#ps160001
(title: "address Pawel's nits in PS4")

[commit-bot: I haz the power, 2016-09-21 09:01:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-21 11:46:06]

Description was changed from

==========
V4L2VideoDecodeAccelerator: destroy buffers in decoder thread.

Move destroying input and output buffers from child thread to
decoder thread has two benefits. (1) Book accounting variables
like output_buffer_map_ are only accessed in decoder thread.
(2) kChangingResolution used to mean waiting for output buffers
to destroy or waiting for processor to return frames. Now it
only means waiting for processor. The code is simpler.

BUG=b/29059119
TEST=Run VDA unittest, run video_VideoSeek test and play video on
elm and peach pit.
==========

to

==========
V4L2VideoDecodeAccelerator: destroy buffers in decoder thread.

Move destroying input and output buffers from child thread to
decoder thread has two benefits. (1) Book accounting variables
like output_buffer_map_ are only accessed in decoder thread.
(2) kChangingResolution used to mean waiting for output buffers
to destroy or waiting for processor to return frames. Now it
only means waiting for processor. The code is simpler.

BUG=b/29059119
TEST=Run VDA unittest, run video_VideoSeek test and play video on
elm and peach pit.
==========

[commit-bot: I haz the power, 2016-09-21 11:46:07]

Committed patchset #6 (id:160001)

[commit-bot: I haz the power, 2016-09-21 11:48:00]

Description was changed from

==========
V4L2VideoDecodeAccelerator: destroy buffers in decoder thread.

Move destroying input and output buffers from child thread to
decoder thread has two benefits. (1) Book accounting variables
like output_buffer_map_ are only accessed in decoder thread.
(2) kChangingResolution used to mean waiting for output buffers
to destroy or waiting for processor to return frames. Now it
only means waiting for processor. The code is simpler.

BUG=b/29059119
TEST=Run VDA unittest, run video_VideoSeek test and play video on
elm and peach pit.
==========

to

==========
V4L2VideoDecodeAccelerator: destroy buffers in decoder thread.

Move destroying input and output buffers from child thread to
decoder thread has two benefits. (1) Book accounting variables
like output_buffer_map_ are only accessed in decoder thread.
(2) kChangingResolution used to mean waiting for output buffers
to destroy or waiting for processor to return frames. Now it
only means waiting for processor. The code is simpler.

BUG=b/29059119
TEST=Run VDA unittest, run video_VideoSeek test and play video on
elm and peach pit.

Committed: https://crrev.com/a9fcabff6db188adb5ef1a16803e7a7a5d8a05eb
Cr-Commit-Position: refs/heads/master@{#420027}
==========

[commit-bot: I haz the power, 2016-09-21 11:48:01]

Patchset 6 (id:??) landed as
https://crrev.com/a9fcabff6db188adb5ef1a16803e7a7a5d8a05eb
Cr-Commit-Position: refs/heads/master@{#420027}