Drop idle MediaCodec instances after a few seconds within AVDA.

Drop idle MediaCodec instances after a few seconds within AVDA.

Idle in this case means we have not seen a Decode() call from the
renderer yet. These cases are not caught by our normal suspend
pipeline in WMPI because the rendere has not reached the "have
future data" state. I.e. a site has appended just initialization
segments to the demuxer, but is stalled waiting for the first
frame.

BUG=612909
TEST=idle codecs release fast enough to prevent device fallover.

[DaleCurtis, 2016-07-13 00:02:52]

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

[liberato (no reviews please), 2016-07-13 05:25:55]

nice.

https://codereview.chromium.org/2147843002/diff/1/media/gpu/android_video_dec...
File media/gpu/android_video_decode_accelerator.cc (right):

https://codereview.chromium.org/2147843002/diff/1/media/gpu/android_video_dec...
media/gpu/android_video_decode_accelerator.cc:966: no_decode_timeout_.Stop();
i think that you also need to stop the timer in ReleaseMediaCodec.

for example, if the timer fires while async codec config is pending, then we
don't want to take any action.

https://codereview.chromium.org/2147843002/diff/1/media/gpu/android_video_dec...
media/gpu/android_video_decode_accelerator.cc:1206:
no_decode_timeout_.Start(FROM_HERE, base::TimeDelta::FromSeconds(2), this,
2 => kSomethingSomething

https://codereview.chromium.org/2147843002/diff/1/media/gpu/android_video_dec...
media/gpu/android_video_decode_accelerator.cc:1792: if (media_codec_) {
please consider early out.

https://codereview.chromium.org/2147843002/diff/1/media/gpu/android_video_dec...
File media/gpu/android_video_decode_accelerator.h (right):

https://codereview.chromium.org/2147843002/diff/1/media/gpu/android_video_dec...
media/gpu/android_video_decode_accelerator.h:492: // a seconds of idle activity.
This prevents older devices from falling over
"a seconds"

[DaleCurtis, 2016-07-13 21:12:21]

watk@ can you test this again on your device? I reduced the timeout to 1 second.
I don't think anything other than sites intentionally stalling media source
should hit this timeout.

for src= we will already have a few packets queued and ready to go. for mse,
this can only happen if post initialization segment is delayed.

https://codereview.chromium.org/2147843002/diff/1/media/gpu/android_video_dec...
File media/gpu/android_video_decode_accelerator.cc (right):

https://codereview.chromium.org/2147843002/diff/1/media/gpu/android_video_dec...
media/gpu/android_video_decode_accelerator.cc:966: no_decode_timeout_.Stop();
On 2016/07/13 at 05:25:55, liberato wrote:
> i think that you also need to stop the timer in ReleaseMediaCodec.
> 
> for example, if the timer fires while async codec config is pending, then we
don't want to take any action.

Done.

https://codereview.chromium.org/2147843002/diff/1/media/gpu/android_video_dec...
media/gpu/android_video_decode_accelerator.cc:1206:
no_decode_timeout_.Start(FROM_HERE, base::TimeDelta::FromSeconds(2), this,
On 2016/07/13 at 05:25:55, liberato wrote:
> 2 => kSomethingSomething

Done.

https://codereview.chromium.org/2147843002/diff/1/media/gpu/android_video_dec...
media/gpu/android_video_decode_accelerator.cc:1792: if (media_codec_) {
On 2016/07/13 at 05:25:55, liberato wrote:
> please consider early out.

Done.

[liberato (no reviews please), 2016-07-13 21:16:21]

lgtm

[DaleCurtis, 2016-07-13 21:17:34]

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

[commit-bot: I haz the power, 2016-07-13 21:18:40]

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-07-13 22:35:19]

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

[commit-bot: I haz the power, 2016-07-13 22:35:20]

Dry run: This issue passed the CQ dry run.

[watk, 2016-07-13 22:38:25]

Title still says "few seconds".

lgtm if you want to land this. The crash is still triggerable after hitting
"load more" a few times. It feels less common than before, but it varies enough
that I can't say.

I think it's because vimeo is still creating so  many elements at once.

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

https://codereview.chromium.org/2147843002/diff/20001/media/gpu/android_video...
media/gpu/android_video_decode_accelerator.cc:1804: codec_needs_reset_ = true;
Do we need to call strategy_->CodecChanged(nullptr)?

I feel like the answer is no, but just wanted to double check you'd thought
about it.

[liberato (no reviews please), 2016-07-14 15:33:57]

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

https://codereview.chromium.org/2147843002/diff/20001/media/gpu/android_video...
media/gpu/android_video_decode_accelerator.cc:1804: codec_needs_reset_ = true;
On 2016/07/13 22:38:25, watk wrote:
> Do we need to call strategy_->CodecChanged(nullptr)?
> 
> I feel like the answer is no, but just wanted to double check you'd thought
> about it.

i don't think that we do, since no decode has been issued => no frames
outstanding.  however, it's probably a good idea since it's safe to call
CodecChanged here, and more robust.

good catch.