Delete AVDACopyingBackingStrategy and rename AVDADeferredRenderingBackingStrategy

Delete AVDACopyingBackingStrategy and rename AVDADeferredRenderingBackingStrategy

AVDACopyingBackingStrategy is unused so it's now deleted. Since we only have a single
strategy left, it's renamed to AVDAPictureBufferManager, since its role is mostly
to associate PictureBuffers with MediaCodec buffers.

BUG=637463
Committed: https://crrev.com/4b2ccc21abd7383301000507b1cdbf7696deb659
Cr-Commit-Position: refs/heads/master@{#415803}

[watk, 2016-08-30 00:54:18]

The CQ bit was checked by watk@chromium.org to run a CQ dry run

[watk, 2016-08-30 01:00:32]

watk@chromium.org changed reviewers:
+ liberato@chromium.org

[commit-bot: I haz the power, 2016-08-30 02:20:41]

Dry run: 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-08-30 03:43:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-30 03:43:59]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[liberato (no reviews please), 2016-08-30 14:45:58]

lgtm % all the android bots failed :)

nice.

-fl

https://codereview.chromium.org/2296513003/diff/1/media/gpu/avda_picture_buff...
File media/gpu/avda_picture_buffer_manager.cc (left):

https://codereview.chromium.org/2296513003/diff/1/media/gpu/avda_picture_buff...
media/gpu/avda_picture_buffer_manager.cc:68: if (!shared_state_)
was the deletion of the |shared_state_| check intentional?

[watk, 2016-08-31 20:22:18]

Patchset #2 (id:20001) has been deleted

[watk, 2016-08-31 20:32:55]

The CQ bit was checked by watk@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-31 20:33:16]

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

[watk, 2016-08-31 20:35:35]

Fixed the unittest so that it would have caught the bug you noticed :)

I think I'll move it out of vda_unittest into to media_unittests in a followup.

https://codereview.chromium.org/2296513003/diff/1/media/gpu/avda_picture_buff...
File media/gpu/avda_picture_buffer_manager.cc (left):

https://codereview.chromium.org/2296513003/diff/1/media/gpu/avda_picture_buff...
media/gpu/avda_picture_buffer_manager.cc:68: if (!shared_state_)
On 2016/08/30 14:45:58, liberato wrote:
> was the deletion of the |shared_state_| check intentional?

Oh, I don't think so, no. Thanks for catching that.

[watk, 2016-08-31 21:22:02]

BTW, PTAL for changes to avda_unittest

[commit-bot: I haz the power, 2016-08-31 23:15:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-31 23:15:59]

Dry run: This issue passed the CQ dry run.

[watk, 2016-08-31 23:23:20]

The CQ bit was checked by watk@chromium.org

[watk, 2016-08-31 23:23:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
liberato@chromium.org
Link to the patchset: https://codereview.chromium.org/2296513003/#ps40001
(title: "Undelete & fix unittest")

[commit-bot: I haz the power, 2016-08-31 23:24:00]

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-08-31 23:29:01]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-08-31 23:32:19]

Description was changed from

==========
Delete AVDACopyingBackingStrategy and rename
AVDADeferredRenderingBackingStrategy

AVDACopyingBackingStrategy is unused so it's now deleted. Since we only have a
single
strategy left, it's renamed to AVDAPictureBufferManager, since its role is
mostly
to associate PictureBuffers with MediaCodec buffers.


BUG=637463
==========

to

==========
Delete AVDACopyingBackingStrategy and rename
AVDADeferredRenderingBackingStrategy

AVDACopyingBackingStrategy is unused so it's now deleted. Since we only have a
single
strategy left, it's renamed to AVDAPictureBufferManager, since its role is
mostly
to associate PictureBuffers with MediaCodec buffers.

BUG=637463

Committed: https://crrev.com/4b2ccc21abd7383301000507b1cdbf7696deb659
Cr-Commit-Position: refs/heads/master@{#415803}
==========

[commit-bot: I haz the power, 2016-08-31 23:32:20]

Patchset 2 (id:??) landed as
https://crrev.com/4b2ccc21abd7383301000507b1cdbf7696deb659
Cr-Commit-Position: refs/heads/master@{#415803}

[Pawel Osciak, 2016-09-01 01:04:28]

posciak@chromium.org changed reviewers:
+ posciak@chromium.org

[Pawel Osciak, 2016-09-01 01:04:29]

https://codereview.chromium.org/2296513003/diff/40001/media/gpu/android_video...
File media/gpu/android_video_decode_accelerator.cc (right):

https://codereview.chromium.org/2296513003/diff/40001/media/gpu/android_video...
media/gpu/android_video_decode_accelerator.cc:496: auto gles_decoder =
get_gles2_decoder_cb_.Run();
I think gles_decoder is not needed anymore? Could it be removed please? I think
we should also be able to remove it from GpuVideoDecodeAcceleratorFactory as
well then...?

[watk, 2016-09-01 17:03:33]

https://codereview.chromium.org/2296513003/diff/40001/media/gpu/android_video...
File media/gpu/android_video_decode_accelerator.cc (right):

https://codereview.chromium.org/2296513003/diff/40001/media/gpu/android_video...
media/gpu/android_video_decode_accelerator.cc:496: auto gles_decoder =
get_gles2_decoder_cb_.Run();
On 2016/09/01 01:04:29, Pawel Osciak wrote:
> I think gles_decoder is not needed anymore? Could it be removed please? I
think
> we should also be able to remove it from GpuVideoDecodeAcceleratorFactory as
> well then...?

I would love to, but we still use it. We pass it to AVDAPictureBufferManager
which mainly uses it for interacting with the TextureManager. The way we attach
MediaCodec buffers to PictureBuffers without copying means we have to be able to
attach GLImages through the TextureManager.

[Pawel Osciak, 2016-09-02 06:03:03]

https://codereview.chromium.org/2296513003/diff/40001/media/gpu/android_video...
File media/gpu/android_video_decode_accelerator.cc (right):

https://codereview.chromium.org/2296513003/diff/40001/media/gpu/android_video...
media/gpu/android_video_decode_accelerator.cc:496: auto gles_decoder =
get_gles2_decoder_cb_.Run();
On 2016/09/01 17:03:33, watk wrote:
> On 2016/09/01 01:04:29, Pawel Osciak wrote:
> > I think gles_decoder is not needed anymore? Could it be removed please? I
> think
> > we should also be able to remove it from GpuVideoDecodeAcceleratorFactory as
> > well then...?
> 
> I would love to, but we still use it. We pass it to AVDAPictureBufferManager
> which mainly uses it for interacting with the TextureManager. The way we
attach
> MediaCodec buffers to PictureBuffers without copying means we have to be able
to
> attach GLImages through the TextureManager. 

I see, thanks!