Store AVDACodecImage list in shared state, cleanup callers.

Store AVDACodecImage list in shared state, cleanup callers.

Going through the texture manager every time we want to look up an
AVDACodecImage is a bit onerous, instead track these within the
shared state and clear them when images are destructed.

This allows a couple simplificiations and cleanups and paves the
way for higher frequency calls.

BUG=601066
TEST=manual playback tests.

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

[DaleCurtis, 2016-04-21 19:28:35]

Patchset #2 (id:20001) has been deleted

[DaleCurtis, 2016-04-21 19:28:39]

Patchset #1 (id:1) has been deleted

[DaleCurtis, 2016-04-21 19:29:18]

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

[DaleCurtis, 2016-04-21 19:29:19]

WDYT?

[watk, 2016-04-21 20:45:01]

I don't see why not, lgtm

https://codereview.chromium.org/1910063005/diff/40001/content/common/gpu/medi...
File content/common/gpu/media/avda_codec_image.cc (right):

https://codereview.chromium.org/1910063005/diff/40001/content/common/gpu/medi...
content/common/gpu/media/avda_codec_image.cc:179: codec_buffer_index_ = -1;
kInvalidCodecBufferIndex?

[liberato (no reviews please), 2016-04-21 23:39:54]

lgtm.

https://codereview.chromium.org/1910063005/diff/40001/content/common/gpu/medi...
File content/common/gpu/media/android_deferred_rendering_backing_strategy.cc
(right):

https://codereview.chromium.org/1910063005/diff/40001/content/common/gpu/medi...
content/common/gpu/media/android_deferred_rendering_backing_strategy.cc:196:
shared_state_->SetImageForPicture(
perhaps pass in the picture buffer id into the codec image constructor, and let
it register itself with the shared state.  then it's symmetric with destruction.
 would also get rid of the iteration in sharedState::Erase, since it could
erase(id).

also helps to enforce that no image is stored twice in the shared state map with
different ids.

https://codereview.chromium.org/1910063005/diff/40001/content/common/gpu/medi...
File content/common/gpu/media/avda_codec_image.cc (right):

https://codereview.chromium.org/1910063005/diff/40001/content/common/gpu/medi...
content/common/gpu/media/avda_codec_image.cc:177: void
AVDACodecImage::SetMediaCodec(media::MediaCodecBridge* codec) {
maybe rename to CodecChanged, since it also resets |codec_buffer_index_|.  it
has the same semantics as CodecChanged elsewhere, and also doesn't sound like a
simple setter.

[DaleCurtis, 2016-04-22 00:05:03]

Description was changed from

==========
Store AVDACodecImage list in shared state, cleanup callers.

Going through the texture manager every time we want to look up an
AVDACodecImage is a bit onerous, instead track these within the
shared state and clear them when images are destructed.

This allows a couple simplificiations and cleanups and paves the
way for higher frequency calls.

BUG=603358
TEST=tbd
==========

to

==========
Store AVDACodecImage list in shared state, cleanup callers.

Going through the texture manager every time we want to look up an
AVDACodecImage is a bit onerous, instead track these within the
shared state and clear them when images are destructed.

This allows a couple simplificiations and cleanups and paves the
way for higher frequency calls.

BUG=603358
TEST=manual playback tests.
==========

[DaleCurtis, 2016-04-22 00:11:21]

Will rebase and land after timav@'s flush CL lands.

https://codereview.chromium.org/1910063005/diff/40001/content/common/gpu/medi...
File content/common/gpu/media/android_deferred_rendering_backing_strategy.cc
(right):

https://codereview.chromium.org/1910063005/diff/40001/content/common/gpu/medi...
content/common/gpu/media/android_deferred_rendering_backing_strategy.cc:196:
shared_state_->SetImageForPicture(
On 2016/04/21 at 23:39:54, liberato-ooo wrote:
> perhaps pass in the picture buffer id into the codec image constructor, and
let it register itself with the shared state.  then it's symmetric with
destruction.  would also get rid of the iteration in sharedState::Erase, since
it could erase(id).
> 
> also helps to enforce that no image is stored twice in the shared state map
with different ids.

Yup looks much better. Also removed Erase() entirely in favor of
SetImageForPicture(nullptr) since that a paradigm we use elsewhere.

https://codereview.chromium.org/1910063005/diff/40001/content/common/gpu/medi...
File content/common/gpu/media/avda_codec_image.cc (right):

https://codereview.chromium.org/1910063005/diff/40001/content/common/gpu/medi...
content/common/gpu/media/avda_codec_image.cc:177: void
AVDACodecImage::SetMediaCodec(media::MediaCodecBridge* codec) {
On 2016/04/21 at 23:39:54, liberato-ooo wrote:
> maybe rename to CodecChanged, since it also resets |codec_buffer_index_|.  it
has the same semantics as CodecChanged elsewhere, and also doesn't sound like a
simple setter.

Done.

https://codereview.chromium.org/1910063005/diff/40001/content/common/gpu/medi...
content/common/gpu/media/avda_codec_image.cc:179: codec_buffer_index_ = -1;
On 2016/04/21 at 20:45:01, watk wrote:
> kInvalidCodecBufferIndex?

Done.

[DaleCurtis, 2016-04-22 00:31:35]

The CQ bit was checked by dalecurtis@chromium.org

[DaleCurtis, 2016-04-22 00:31:36]

The patchset sent to the CQ was uploaded after l-g-t-m from watk@chromium.org,
liberato@chromium.org
Link to the patchset: https://codereview.chromium.org/1910063005/#ps80001
(title: "Fix comment.")

[DaleCurtis, 2016-04-22 00:31:57]

Actually timav@'s CL needs some test fixes, so going ahead.

[commit-bot: I haz the power, 2016-04-22 00:32:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1910063005/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1910063005/80001

[DaleCurtis, 2016-04-22 00:37:07]

Description was changed from

==========
Store AVDACodecImage list in shared state, cleanup callers.

Going through the texture manager every time we want to look up an
AVDACodecImage is a bit onerous, instead track these within the
shared state and clear them when images are destructed.

This allows a couple simplificiations and cleanups and paves the
way for higher frequency calls.

BUG=603358
TEST=manual playback tests.
==========

to

==========
Store AVDACodecImage list in shared state, cleanup callers.

Going through the texture manager every time we want to look up an
AVDACodecImage is a bit onerous, instead track these within the
shared state and clear them when images are destructed.

This allows a couple simplificiations and cleanups and paves the
way for higher frequency calls.

BUG=601066
TEST=manual playback tests.
==========

[commit-bot: I haz the power, 2016-04-22 01:28:48]

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

[commit-bot: I haz the power, 2016-04-22 01:28:49]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[DaleCurtis, 2016-04-22 01:34:36]

The CQ bit was checked by dalecurtis@chromium.org

[commit-bot: I haz the power, 2016-04-22 01:35:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1910063005/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1910063005/80001

[commit-bot: I haz the power, 2016-04-22 02:35:08]

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

[commit-bot: I haz the power, 2016-04-22 02:35:09]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[DaleCurtis, 2016-04-22 04:14:53]

The CQ bit was checked by dalecurtis@chromium.org

[commit-bot: I haz the power, 2016-04-22 04:14:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1910063005/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1910063005/80001

[commit-bot: I haz the power, 2016-04-22 04:58:35]

Committed patchset #3 (id:80001)

[commit-bot: I haz the power, 2016-04-22 19:44:34]

Description was changed from

==========
Store AVDACodecImage list in shared state, cleanup callers.

Going through the texture manager every time we want to look up an
AVDACodecImage is a bit onerous, instead track these within the
shared state and clear them when images are destructed.

This allows a couple simplificiations and cleanups and paves the
way for higher frequency calls.

BUG=601066
TEST=manual playback tests.
==========

to

==========
Store AVDACodecImage list in shared state, cleanup callers.

Going through the texture manager every time we want to look up an
AVDACodecImage is a bit onerous, instead track these within the
shared state and clear them when images are destructed.

This allows a couple simplificiations and cleanups and paves the
way for higher frequency calls.

BUG=601066
TEST=manual playback tests.

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

[commit-bot: I haz the power, 2016-04-22 19:44:35]

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