Tried on N7v2 several times and cannot repro.

timav: I remember you have a N5. Could you please help check whether you can repro? I suspect that MediaCodec on N5 doesn't handle frame size change well.

Sure, I can deal with this later though.

A link to bot failure related to this:

https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/45604

I can reproduce the problem with N5 and KitKat (hammerhead-userdebug 4.4.4 KTU84P 1227136 dev-keys)

03-31 13:48:52.875   182 22467 E OMX-VDEC-1080P: 
03-31 13:48:52.875   182 22467 E OMX-VDEC-1080P:  SYS Error Recieved 
03-31 13:48:52.875   182 22468 E OMX-VDEC-1080P: 
03-31 13:48:52.875   182 22468 E OMX-VDEC-1080P:  OMX_COMPONENT_GENERATE_HARDWARE_ERROR
03-31 13:48:52.875 22366 22436 E ACodec  : [OMX.qcom.video.decoder.vp8] ERROR(0x80001009)
03-31 13:48:52.875   182 22453 E OMX-VDEC-1080P: 
03-31 13:48:52.875   182 22453 E OMX-VDEC-1080P:  OMX_COMPONENT_GENERATE_HARDWARE_ERROR
03-31 13:48:52.875 22366 22435 E MediaCodec: Codec reported an error. (omx error 0x80001009, internalError -2147483648)

cc:chcunningham in case he saw anything similar when working on the MSE path.

Could this be related to IsAdaptivePlaybackSupported()?

https://code.google.com/p/chromium/codesearch#search/&q=IsAdaptivePlaybackSupported&sq=package:chromium&type=cs

Not according to my testing. I tested that before filing the bug and it made no difference.

(see c#0)

This will trigger a decoder reconfig. We probably should do the same in AVDA.

https://code.google.com/p/chromium/codesearch#chromium/src/media/base/android/media_decoder_job.cc&l=345

To be clearer, allowAdaptivePlayback flag is only used when IsAdaptivePlaybackSupported() is true [1]. On devices that doesn't support adaptive playback, we should reconfig the decoder, similar to what MediaDecoderJob is doing.

[1] https://code.google.com/p/chromium/codesearch#chromium/src/media/base/android/java/src/org/chromium/media/MediaCodecBridge.java&l=458

N5 definitely supports adaptive playback though, so it shouldn't be the issue we're seeing here.

Fair enough. Can someone check what IsAdaptivePlaybackSupported() returns on devices that can repro this?

I'm looking into what's going on.

c#12: I believe this is not device, but Android version specific.

Thanks! 

This is introduced from KK.

It seems it could be device related:

https://code.google.com/p/chromium/codesearch#chromium/src/media/base/android/java/src/org/chromium/media/MediaCodecUtil.java&l=243

This is related Android doc: http://developer.android.com/reference/android/media/MediaCodecInfo.CodecCapabilities.html#FEATURE_AdaptivePlayback

No matter what the root cause of this particular issue is, it seems we should:
1) Enable AdaptivePlayback in AVDA
2) Handle config change by explicit reconfig if AdaptivePlayback is not supported. (Same as what we are doing in MediaDecoderJob.)

No we do not want 1. That's intentionally disabled since the feature is bonkers and allocates crazy amounts of decoder resources.

See https://codereview.chromium.org/1726103002/

Also I can repro these issues on a marshmallow N5, so it's not limited to KitKat.

c#17 (2): how can we do it for src=... playback? With MSE the demuxer tells us in advance that the reconfiguration is coming, for src=... it is the decoder who tells after it has processed a new encoded frame.

Re #19/#20: Thanks for the info! Agreed that we shouldn't do (1). But aren't we 

I wasn't aware of the issue. Maybe we should have a comment about the magical "false" in AVDA::ConfigureMediaCodec().

Actually do you know how these tests pass magically on N7v2 given AdaptivePlayback is disabled in all cases? Looking at the code, it seems this flag would only affect the format [1]. If we are switching from higher resolution to lower resolution maybe the adaptive playback will still be triggered and will just work.

Re #21: This makes sense. Since the code in MediaCodecUtil will behave the same for KK+.

[1] https://code.google.com/p/chromium/codesearch#chromium/src/media/base/android/java/src/org/chromium/media/MediaCodecBridge.java&l=459

Re #22: Good question. I guess for SRC we don't have to support this case. But for MSE this should work. In theory the config change should handle this. Now I am confused...

c#24: does avda actually see the expected eos before the config change?

c#22: Lets focus on MSE only playback for now. I don't think we need to worry about src= since this is a both a rare case and doesn't work well with MediaPlayer src= anyways (I.e. I see the same artifacts in MediaPlayer as Spitzer).

As far as I've been able to tell that flag doesn't do anything useful and to be clear I have tested with and without that bit flipped. So I think it's a bit of a red herring. I agree that the typical flush -> reset -> decode cycle imposed by DecoderStream should avoid any issues here.

I'd verify that Flush is actually coming as we expect it, then a Reset(), then a decode.

Thinking wildly...

Is it possible that we have frame size change within a media segment (before the MSE config change happans)? I think that's the purpose of this test (think about the SRC case).

Re: eos -> flush - this log line should tell you
https://code.google.com/p/chromium/codesearch#chromium/src/content/common/gpu/media/android_video_decode_accelerator.cc&l=639

Re: comment #6, sorry, I've not seen these particular errors before.

Re: 27 - afaik, in MSE it is not possible to (legitimately) change the frame size without a config change. Matt to confirm.

@#29: Some bytestreams (like h264 in ISO BMFF) in the MSE bytestream format registry, allow as a "quality of implementation" "should" support inband codec reconfig outside of the init segment. WebM MSE bytestream spec, doesn't have any such language (config changes must be explicit in webm track/info elements, not inband in encoded blocks).

MSE ISO-BMFF bytestream spec snippet:
The user agent must support parameter sets (e.g., PPS/SPS) stored in the sample entry (as defined for avc1/avc2), and should support parameter sets stored inband in the samples themselves (as defined for avc3/avc4).

NOTE
For maximum content interoperability, user agents are strongly advised to support both inband and out-of-band storage of the SPS and PPS.

Actually, I think xhwang is right in comment 27. 

IIUC, this test is using the file frame_size_change-av_enc-v.webm. The README[0] says this derives from frame_size_change.webm. That file does not appear to be a valid webm file and I expect it will change frame sizes without properly signalling a config change (no new MSE initialization segment).

The test is whack. It should be re-written to use two files with two init segments. Like this one [1].

[0] https://code.google.com/p/chromium/codesearch#chromium/src/media/test/data/README&sq=package:chromium&type=cs&q=frame_size_change-av_enc-v.webm&l=78

[1]https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/LayoutTests/http/tests/media/media-source/mediasource-config-change-webm-v-framesize.html&sq=package:chromium&type=cs&q=framesize%20f:layout

c#31: yes, I see neither
https://code.google.com/p/chromium/codesearch#chromium/src/content/common/gpu/media/android_video_decode_accelerator.cc&l=639
nor
https://code.google.com/p/chromium/codesearch#chromium/src/content/common/gpu/media/android_video_decode_accelerator.cc&l=857,

but I do see Reset() https://code.google.com/p/chromium/codesearch#chromium/src/content/common/gpu/media/android_video_decode_accelerator.cc&l=961

This Reset() calls MediaCodec.flush() and it results in OMX error.

BTW - can I ask one of you who are running the test to confirm the above by checking that these log line do NOT appear in your logs:

https://code.google.com/p/chromium/codesearch#chromium/src/media/filters/chunk_demuxer.cc&q=chunk_demuxer&sq=package:chromium&type=cs&l=332

https://code.google.com/p/chromium/codesearch#chromium/src/content/common/gpu/media/android_video_decode_accelerator.cc&l=639

My checkout is not in a state that makes it easy for me to do so.

Er, ignore my 33. 32 came it at the same time

Re: 32, my guess is the Reset isn't related to frame size change. Can you say whats triggering it? DestroyVDA?

Re #31: Given #30, isn't "inband codec reconfig" a valid test case that we should cover? Note that we have config change tests. This test is specifically trying to do inband frame size change without using MSE config change.

Re 36: Hand wavy answer, no, at least not for webm (which this test is), and I wouldn't bother adding a test for mp4 at this point either.

The byte stream spec Matt referenced in #30 is only for mp4. The Webm bytstream spec makes no mention of inband signaling (so assume not allowed) and I'm pretty sure its not possible to create a valid WebM file that does this for a single video track. 

Even for mp4, while the spec might allow inband singalling, Chrome's parser does not actually implement it and no one seems to be asking for it.

Explicit signal via new init segment is the most common and the most straightforward approach, so I vote we focus on that instead.

c#35: From RendererImpl::Flush()
https://code.google.com/p/chromium/codesearch#chromium/src/media/renderers/renderer_impl.cc&l=147

+1 to #37. Thanks for recording our chat results here, Chris :)

The old pipeline does not call MediaCodec.flush(). No flush - no crash?

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb

commit 7defc181e284d5e24a00a9254d4dc7ac3f2c15cb
Author: dalecurtis <dalecurtis@chromium.org>
Date: Fri Apr 01 00:46:57 2016

Flip unified media pipeline to default-on w/ disabled holdback.

Will be merged back to M50 pending launch-approval.

Experiment config will be updated to target M50 stable only at a 95%
disabled rate and slowly ramped down to 0-1% over a couple weeks
to ensure there are no major regressions.

Since there are still some WebView issues to sort out, WebView
has been disabled in this patch set.

Notably on M51 this will also enable 100% of MSE/EME playbacks
onto the unified pipeline. Since MSE only makes up ~9% of
Android playbacks (EME < 1%) and have always been codec restricted,
this is an acceptable risk.

BUG= 597495 , 598840,  598963 
TEST=passes bots.

Review URL: https://codereview.chromium.org/1840173004

Cr-Commit-Position: refs/heads/master@{#384451}

[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/android_webview/lib/main/aw_main_delegate.cc
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/chrome/app/generated_resources.grd
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/chrome/browser/about_flags.cc
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/content/browser/gpu/gpu_process_host.cc
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/content/browser/media/encrypted_media_browsertest.cc
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/content/browser/media/media_canplaytype_browsertest.cc
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/media/base/key_systems_unittest.cc
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/media/base/media.cc
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/media/base/media_switches.cc
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/media/base/media_switches.h
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/media/base/mime_util_internal.cc
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/media/base/mime_util_unittest.cc
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/media/base/run_all_unittests.cc
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/media/filters/chunk_demuxer_unittest.cc
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/tools/metrics/histograms/histograms.xml

I see double release of the output buffer in the log.

03-31 18:54:47.373  9736  9751 V timav   : [9736/9751]   18:54:47.380 UpdateSurfaceTexture: releasing output buffer, index:5
03-31 18:54:47.373  9736  9751 V timav   : [9736/9751]   18:54:47.380 ReleaseOutputBuffer: 5
03-31 18:54:47.373   182  1013 E OMX-VDEC-1080P: 
03-31 18:54:47.373   182  1013 E OMX-VDEC-1080P:  ALL output buffers are freed/released
03-31 18:54:47.373   182  9829 E OMX-VDEC-1080P: 
03-31 18:54:47.373   182  9829 E OMX-VDEC-1080P:  OMX_CommandPortDisable complete for port [1]
03-31 18:54:47.373   182  9829 E OMX-VDEC-1080P: Streaming off 0 port
03-31 18:54:47.373  9736  9751 V timav   : [9736/9751]   18:54:47.384 DequeueInputBuffer: status: 1, index: -1
03-31 18:54:47.373  9736  9751 V timav   : [9736/9751]   18:54:47.384 DequeueOutputBuffer: status: 2, index:-1, offset: 0, size: 0, flags: 0
03-31 18:54:47.383  9736  9751 V timav   : [9736/9751]   18:54:47.387 DequeueInputBuffer: status: 1, index: -1
03-31 18:54:47.383  9736  9751 V timav   : [9736/9751]   18:54:47.388 DequeueOutputBuffer: status: 2, index:-1, offset: 0, size: 0, flags: 0
03-31 18:54:47.383  9736  9751 V timav   : [9736/9751]   18:54:47.388 DequeueInputBuffer: status: 1, index: -1
03-31 18:54:47.383  9736  9751 V timav   : [9736/9751]   18:54:47.388 DequeueOutputBuffer: status: 2, index:-1, offset: 0, size: 0, flags: 0
03-31 18:54:47.383  9736  9751 V timav   : [9736/9751]   18:54:47.396 UpdateSurfaceTexture: releasing output buffer, index:5
03-31 18:54:47.383  9736  9751 V timav   : [9736/9751]   18:54:47.396 ReleaseOutputBuffer: 5

According to the log, between these two releases no output buffer was dequeued.

I do not understand how can this happen though, so it may be red herring.

can you add logging to find out if it is the same codec image instance that does the release both times?  the only way i can even remotely see that happening is if MC returns 5 twice(!), and avda puts it into two different PictureBuffers.

c#43: You are right, I have two AvdaCodecImage objects. Keep looking. Btw, MediaCodec.flush() restores ownership on all buffers:

"Upon return, all indices previously returned in calls to dequeueInputBuffer and dequeueOutputBuffer — or obtained via onInputBufferAvailable or onOutputBufferAvailable callbacks — become invalid, and all buffers are owned by the codec."

c#42: Please ignore. There are two video elements on the page, and thus there are two MediaCodecs that work in parallel (on the same thread, I hope it is ok).
I haven't noticed anything wrong in the way we call MediaCodec methods.

I also confirmed that allowAdaptivePlayback flag does not matter in this case, and that codec reports that it allows adaptive playback.

If you have some ideas, please share.

In a private conversation Min also pointed out that for Vp8 in Spitzer we might be using Vpx instead of MediaCodec. The encryption here is ClearKey and the stream is coming to MediaCodec unencrypted.

Changing initial video size in AVDA::ConfigureMediaCodec() to 1920x1080 fixes the problem.
https://code.google.com/p/chromium/codesearch#chromium/src/content/common/gpu/media/android_video_decode_accelerator.cc&l=859

After that change is done, setting the MediaFormat keys for adaptive playback
https://code.google.com/p/chromium/codesearch#chromium/src/media/base/android/java/src/org/chromium/media/MediaCodecBridge.java&l=459
seems to have no effect.

Hmm, if change that setting to 1920x1080 do you see any memory usage increases in the mediaserver process? On an N4 device are you still able to create multiple codec instances?

I do not know. I believe your findings about N4 are quite correct. The problem is there are various bugs that, I think, push implementation in the opposite directions at the same time.
So far my guess is that we should say that N5 + vp8 does not support adaptive playback, i.e. do an adaptive playback blacklisting.

Meh, it's probably not worth doing anything about this since implicit signaling is rare. When we have VDAv2 architecture we can always reconfigure with the right size.

FWIW, setting to 1920x1080 is equivalent to hard coding mAdaptivePlaybackSupported to be true.

https://b.corp.google.com/u/0/issues/27976666

Can you elaborate on that Xiaohan? Setting 1920x1080 sets KEY_WIDTH, KEY_HEIGHT not KEY_MAX_WIDTH, KEY_MAX_HEIGHT. Are you guessing that they mean the same thing or does it say that somewhere?

C#52: I'm trying to say the opposite, i.e. setting adaptive media keys to that size is not enough or maybe has no effect. It is initial video size that we have to set to large one, the one that goes to MediaCodec.configure()

M-mm, sorry, I meant KEY_WIDTH and KEY_HEIGHT (that go to configure())

c51: did you mean recreating MediaCodec each time demuxer reports config change, and not using flush()?

Yes, we could go that route just like the media source player does today.

Yes, I thought this would require adaptive blacklisting.

Re #54: You are right. I got confused with KEY_WIDTH and KEY_MAX_WIDTH.

Recent Android M (MMB30D) seems to work fine.

c#51: I'm somewhat worried that malformed stream can lead to a crash, but I guess I should not. I will close this bug as "won't fix" if I do not hear any objections.

Additional observation: the gpu_video_decoder has the initial size 1920x1080. So, if we propagate this size to the AVDA and use it, we might avoid the crash in this particular test. Still, this would not solve the problem with malformed stream.

After updating to the latest ToT (Clankium 51.0.2700.0) and applying the fix from c#47 + allow_adaptive_playback=true I see corrupted frames and the test often hangs up. That is, we are at the initial state if the bug.

The stall happens in MediaCodec.flush(), https://b.corp.google.com/u/0/issues/28007844.

An attempt to replace the flush() with  release and recreate pair caused MediaCodec.release() to hand up on KitKat, it looks like it works on later Android versions.

If we instead fail on implicit config change signaling and release the codec before providing any more buffers, is it too late to flush/release the codec without issue? AVDA used to have code to kill the codec when this occurred. 

c#64: Sorry, did not quite understand, please elaborate. "implicit config change signaling": did you mean receiving configuration change from dequeueOutputBuffer()? If so, how can we fail, in other words, what would be a condition to release MediaCodec?

I meant: is it too late to avoid the bugs if we fail out and tear down the codec as soon as the output_format_changed message is received?

I see. I think we would need to keep the resolution changing buffer and several following buffers in the cache and re-submit them after we recreate the codec. My brute-force attempt to just release and recreate MediaCodec did not work.

Oh, I'm saying throw a decode error since this is a rare case that we likely don't need to support. Forget about re-configuring, just error out.

Posting an error after detecting an unexpected resolution change worked for me (MediaCodec.release() did not hang) as long as the new resolution did not exceed the initial one (so it cannot be a replacement of #47).
We can do it as a precaution. I'm curious what it is we are doing different than before, the corrupted output frames indicate that we skip some encoded (input) frames...

After comparison between MediaCodecPlayer and AVDA in how they call MediaCodec API and a private conversation with Min I tend to thisnk that MediaCodecPlayer and MediaSourcePlayer implicitly drain the codec before they call flash, and this step is crucial for flush() to succeed (although it is not needed then).

The quick AVDA experiment https://codereview.chromium.org/1862303002 that does the drain seems to fix the flush() stall.

It does not fix the unit test though, because the test started hanging in MediaCodec.release(). At this point I'm waiting for a word from Android team.

Hmm, we can probably switch to implicit drain if we have to. It will slow down seeks and exit-fullscreen transitions though. The fullscreen transitions we can fix by not calling Reset() in the future, but that's a bit of work.

What happens if take the "slow reset" path if we detect any implicit configuration changes? I.e. whenever a format change does not follow an Initialize() or Flush() take the .reset() path here:

https://code.google.com/p/chromium/codesearch#chromium/src/content/common/gpu/media/android_video_decode_accelerator.cc&l=919

I guess that doesn't help your .release() hang though. Just the .flush() one.

If you are talking about replacing the flush() with release and recreate pair for all cases, I tried that in the very beginning of and it did not help because then MediaCodec.release() - the one that came instead of flush - hangs forever.

I haven't figure out why .release() hangs or what would unblock it.

Not all cases, just when implicit changes are detected. I believe you confirmed offline that .release() still hung irrespective of adaptive playback setting.

I tried the unencrypted vp8 and h264 files Dale sent to me. With vp8 the flush() hangs as reported before, this is not related to the test itself. h264 seems to have no issues.

With vp8, reporting the pipeline error after the decoder detected the new resolution seems to be not enough: subsequent user actions might force AVDA::Reset() and it hangs, both if using MediaCodec.release() or MediaCodec.flush().

If this is vp8 only we can use media::VP8Parser to extract the resolution information from bitstream buffers and bail out when this is detected.

RTC video decoder tries to avoid frame size changes on Android too FWIW:
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/media/rtc_video_decoder.cc&l=183

Right now it seems the only way out! Btw the render flag in ReleaseOutputBuffer() has no effect as well.

Hm. It did not help. It seems enqueueing just one input frame is enough to suspend .relese() in the codec. One frame has only one resolution by definition.

04-08 18:46:59.190 27352 27552 V timav   : [27352/27552] 18:46:59.195 VideoCodecBridge::CreateDecoder size:1920x1080 allow_adaptive_playback:1
04-08 18:46:59.190 27352 27552 V timav   : [27352/27552] 18:46:59.199 SdkMediaCodecBridge::Start
04-08 18:46:59.200 27352 27369 V timav   : [27352/27369] 18:46:59.209 SdkMediaCodecBridge::DequeueInputBuffer index:0 result:0
04-08 18:46:59.200 27352 27369 V timav   : [27352/27369] 18:46:59.210 SdkMediaCodecBridge::QueueInputBuffer index:0 data_size:3648 pts:0s result:0

<here gpu_video_decoder.cc detects the resolution change and posts an error>

04-08 18:46:59.210 27352 27369 V timav   : [27352/27369] 18:46:59.212 AVDA::Destroy
04-08 18:46:59.210 27352 27369 V timav   : [27352/27369] 18:46:59.213 SdkMediaCodecBridge::~SdkMediaCodecBridge
04-08 18:46:59.210 27352 27369 W cr_timav: calling MediaCodec.release()

This MediaCodec.release() does not return.

I tried several other VP8 files (but they had constant resolution) and never saw .flush() or .release() blocked.

If I explicitly drain decoder (DequeueOutputBuffer / ReleaseOutputBuffer(no_render)) it does not hang. Maybe we should do this...

Very strange, maybe it has nothing to do with this file but more to do with any pending configuration change being outstanding before flush is called(). If you take a normal vp8 file and force an error after the first frame is queued but before we deque the OUTPUT_FORMAT_CHANGED message, can you trigger the same hang?

How long does it take if instead of doing an async dequeu+release you just sit in a sync flush loop, i.e.  queue eos, while(!flushed) { dequeue + release }.

Dale, you caught me working on a solution that seems to work (i.e. file plays, unit test passes) in my limited testing. Although I think it's complicated, I wanted to record it (https://codereview.chromium.org/1862303002/), now back to your questions.

c#80: If I force an error after the first frame is queues but before we dequeue anything from the output (except TRY_AGAIN_LATER) any other VP8 file plays as expected, but the original file often stuck.

Maybe there is something wrong with the file itself? ffprobe says "encoder:  libwebm-0.0.0.1" while for other files it is a later version or e.g. Lavf56.1.100.

I think we're pretty sure something is wrong with the file :) That said we can't have it hanging the gpu process.

c#80: Answering the second question: sync drain typically takes 10ms or less, and I observed 20ms on a regular file and 60ms on our problematic file.

To do the drain one needs to enqueue EOS, for that we need to ask the input buffer, but it does not have to be immediately available. So we have to process output until the input buffer appears, and then process more output until eos comes out.

maybe we should have another thread in GPU process for media? I think right now  all players are processed on one thread, with EME audio and video are on the same thread as well?

How about just translating all VDA::Reset() calls into VDA::Flush() for VP8 as well as call VDA::Reset() during destruction for VP8? That allows us to continue using the current async approach without too much pain. We can speed up this process by dropping all pending inputs and releasing all output buffers (w/o render) before queuing the eos.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e69b434c2d5c6fd5415a772cda5e5de2f140996c

commit e69b434c2d5c6fd5415a772cda5e5de2f140996c
Author: timav <timav@chromium.org>
Date: Fri Apr 22 22:30:47 2016

Delay actual flush and release in AVDA

Postpone the actual call to MediaCodec.flush() and .release()
to let the AVDA drain the MediaCodec. This seems necessary to
at least one VP8 file, otherwise .flash() and .release() hangs.

The CL attempts to generalize the codec draining mechanism
and uses it in 2 additional places: Reset() for Vp8 and
Destroy() for Vp8.

This CL also release all pending output buffer in Release()
and Destroy() for all codecs.

BUG= 598963 

Review URL: https://codereview.chromium.org/1862303002

Cr-Commit-Position: refs/heads/master@{#389273}

[modify] https://crrev.com/e69b434c2d5c6fd5415a772cda5e5de2f140996c/content/common/gpu/media/android_deferred_rendering_backing_strategy.cc
[modify] https://crrev.com/e69b434c2d5c6fd5415a772cda5e5de2f140996c/content/common/gpu/media/android_deferred_rendering_backing_strategy.h
[modify] https://crrev.com/e69b434c2d5c6fd5415a772cda5e5de2f140996c/content/common/gpu/media/android_video_decode_accelerator.cc
[modify] https://crrev.com/e69b434c2d5c6fd5415a772cda5e5de2f140996c/content/common/gpu/media/android_video_decode_accelerator.h

We should enable the test disabled in c#41 now too.

Tima, can I ask you to run the mp4 frame size change layout tests to make sure this doesn't regress anything there?

https://docs.google.com/document/d/1UwpyGbCqs9lSUgBQn9G8ztdCkARugTgixVgdgVmNqk0/edit

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6aef14f05bd5535f49f117d380d41b1faad16420

commit 6aef14f05bd5535f49f117d380d41b1faad16420
Author: timav <timav@chromium.org>
Date: Mon Apr 25 19:26:07 2016

media: Enable FrameSizeChangeVideo test

After the VP8 frame size change fix for Spitzer
https://codereview.chromium.org/1862303002/
is landed we can re-enable this test.

BUG= 598963 

Review URL: https://codereview.chromium.org/1914923005

Cr-Commit-Position: refs/heads/master@{#389535}

[modify] https://crrev.com/6aef14f05bd5535f49f117d380d41b1faad16420/content/browser/media/encrypted_media_browsertest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6aef14f05bd5535f49f117d380d41b1faad16420

commit 6aef14f05bd5535f49f117d380d41b1faad16420
Author: timav <timav@chromium.org>
Date: Mon Apr 25 19:26:07 2016

media: Enable FrameSizeChangeVideo test

After the VP8 frame size change fix for Spitzer
https://codereview.chromium.org/1862303002/
is landed we can re-enable this test.

BUG= 598963 

Review URL: https://codereview.chromium.org/1914923005

Cr-Commit-Position: refs/heads/master@{#389535}

[modify] https://crrev.com/6aef14f05bd5535f49f117d380d41b1faad16420/content/browser/media/encrypted_media_browsertest.cc

Had a flake of this test here:

https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/65243

But can't find any evidence that this is flaking often.

Shall I disable the test again? Wait?

marking merge request for https://codereview.chromium.org/1862303002 so I can merge  issue 601066  more easily.

Your change meets the bar and is auto-approved for M51 (branch: 2704)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b6b2141b0683cad4d1f7ccfcaa6350c31437cdf2

commit b6b2141b0683cad4d1f7ccfcaa6350c31437cdf2
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Tue May 10 04:12:38 2016

Merge to M51: Various fixes to prevent playback hang on MotoX.

e886ba2 Avoid waiting on the SurfaceTexture until we need to.
cecc8a4 Increase cases where back/front buffering early rendering occur.
9b07026 Queue more input buffers and process more output if possible.
9771835 Avoid unnecessary post task during frame delivery.
fb00bd3 ReleaseOutputBuffer to surface back and front buffers where possible.
6c5e1df Delay actual flush and release in AVDA
4ddcd18 Store AVDACodecImage list in shared state, cleanup callers.

BUG= 598963 ,  601066 
TBR=liberato, timav

(cherry picked from commit 5cda4bb2b9d1a7b7821f87df58337928361ce2c3)

Review URL: https://codereview.chromium.org/1963773003 .

Cr-Commit-Position: refs/branch-heads/2704@{#462}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/b6b2141b0683cad4d1f7ccfcaa6350c31437cdf2/content/common/gpu/media/android_copying_backing_strategy.cc
[modify] https://crrev.com/b6b2141b0683cad4d1f7ccfcaa6350c31437cdf2/content/common/gpu/media/android_copying_backing_strategy.h
[modify] https://crrev.com/b6b2141b0683cad4d1f7ccfcaa6350c31437cdf2/content/common/gpu/media/android_deferred_rendering_backing_strategy.cc
[modify] https://crrev.com/b6b2141b0683cad4d1f7ccfcaa6350c31437cdf2/content/common/gpu/media/android_deferred_rendering_backing_strategy.h
[modify] https://crrev.com/b6b2141b0683cad4d1f7ccfcaa6350c31437cdf2/content/common/gpu/media/android_video_decode_accelerator.cc
[modify] https://crrev.com/b6b2141b0683cad4d1f7ccfcaa6350c31437cdf2/content/common/gpu/media/android_video_decode_accelerator.h
[modify] https://crrev.com/b6b2141b0683cad4d1f7ccfcaa6350c31437cdf2/content/common/gpu/media/avda_codec_image.cc
[modify] https://crrev.com/b6b2141b0683cad4d1f7ccfcaa6350c31437cdf2/content/common/gpu/media/avda_codec_image.h
[modify] https://crrev.com/b6b2141b0683cad4d1f7ccfcaa6350c31437cdf2/content/common/gpu/media/avda_shared_state.cc
[modify] https://crrev.com/b6b2141b0683cad4d1f7ccfcaa6350c31437cdf2/content/common/gpu/media/avda_shared_state.h
[modify] https://crrev.com/b6b2141b0683cad4d1f7ccfcaa6350c31437cdf2/content/common/gpu/media/gpu_video_decode_accelerator.cc
[modify] https://crrev.com/b6b2141b0683cad4d1f7ccfcaa6350c31437cdf2/media/base/video_decoder.h
[modify] https://crrev.com/b6b2141b0683cad4d1f7ccfcaa6350c31437cdf2/media/filters/ffmpeg_video_decoder.cc
[modify] https://crrev.com/b6b2141b0683cad4d1f7ccfcaa6350c31437cdf2/media/filters/gpu_video_decoder.cc
[modify] https://crrev.com/b6b2141b0683cad4d1f7ccfcaa6350c31437cdf2/media/filters/vpx_video_decoder.cc