Send metadata change to renderer after decoder is drained

Send metadata change to renderer after decoder is drained

MediaSourcePlayer sends a metadata change to renderer whenever it receives a config change.
However, the decoder might still be in the draining process when this happens.
To fix this, we should wait for a OUTPUT_FORMAT_CHANGED status before sending out the IPC.

BUG=381975
Committed: https://crrev.com/07cdc5cc953f92e8c494375faf5e408f3e04ead9
Cr-Commit-Position: refs/heads/master@{#299338}

[qinmin, 2014-10-04 00:01:24]

Patchset #1 (id:1) has been deleted

[qinmin, 2014-10-04 00:02:33]

qinmin@chromium.org changed reviewers:
+ wolenetz@chromium.org

[qinmin, 2014-10-04 00:02:34]

PTAL

[wolenetz, 2014-10-08 00:59:44]

On 2014/10/04 00:02:34, qinmin wrote:
> PTAL

Sorry for delay, this is on my list for CR for no later than tomorrow morning.

[wolenetz, 2014-10-08 19:41:33]

looking good, just a couple questions"

https://codereview.chromium.org/623363002/diff/20001/media/base/android/media...
File media/base/android/media_decoder_job.cc (right):

https://codereview.chromium.org/623363002/diff/20001/media/base/android/media...
media/base/android/media_decoder_job.cc:350: 
Is there a possibility that the first AU is kConfigChanged? If so, would
|drain_decoder| be false and the config change not be signalled to the manager?

https://codereview.chromium.org/623363002/diff/20001/media/base/android/media...
File media/base/android/media_source_player_unittest.cc (right):

https://codereview.chromium.org/623363002/diff/20001/media/base/android/media...
media/base/android/media_source_player_unittest.cc:2289:
while(manager_.num_metadata_changes() == 1) {
Do we need this while()? Or is WaitForVideoDecodeDone() enough to ensure the
metadata change has been signalled to the manager?

[qinmin, 2014-10-08 22:52:42]

https://codereview.chromium.org/623363002/diff/20001/media/base/android/media...
File media/base/android/media_decoder_job.cc (right):

https://codereview.chromium.org/623363002/diff/20001/media/base/android/media...
media/base/android/media_decoder_job.cc:350: 
MEDIA_CODEC_OUTPUT_FORMAT_CHANGED is reported when decoder detects the output
format changed, and it includes the situation that decoder detects output
becomes available after creation.
So even if kConfigChanged is the first AU and MediaCodecBridge gets recreated,
we will still receive MEDIA_CODEC_OUTPUT_FORMAT_CHANGED once decoder detects
there are decoded data in the output.

On 2014/10/08 19:41:33, wolenetz wrote:
> Is there a possibility that the first AU is kConfigChanged? If so, would
> |drain_decoder| be false and the config change not be signalled to the
manager?

https://codereview.chromium.org/623363002/diff/20001/media/base/android/media...
File media/base/android/media_source_player_unittest.cc (right):

https://codereview.chromium.org/623363002/diff/20001/media/base/android/media...
media/base/android/media_source_player_unittest.cc:2289:
while(manager_.num_metadata_changes() == 1) {
On 2014/10/08 19:41:33, wolenetz wrote:
> Do we need this while()? Or is WaitForVideoDecodeDone() enough to ensure the
> metadata change has been signalled to the manager?

When the format changes, we don't know how long the draining will take, and 2nd
decoder may output status like MEDIA_CODEC_DEQUEUE_INPUT_AGAIN_LATER and
MEDIA_CODEC_OUTPUT_BUFFERS_CHANGED at the begining if adaptive playback is not
supported.

[wolenetz, 2014-10-08 23:23:19]

lgtm. ship it! (once network outage is fixed...)

[qinmin, 2014-10-08 23:30:42]

The CQ bit was checked by qinmin@chromium.org

[commit-bot: I haz the power, 2014-10-08 23:32:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/623363002/20001

[commit-bot: I haz the power, 2014-10-09 02:43:04]

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

[commit-bot: I haz the power, 2014-10-09 02:43:05]

Try jobs failed on following builders:
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[qinmin, 2014-10-11 00:33:18]

The CQ bit was checked by qinmin@chromium.org

[commit-bot: I haz the power, 2014-10-11 00:34:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/623363002/40001

[commit-bot: I haz the power, 2014-10-11 01:25:17]

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

[commit-bot: I haz the power, 2014-10-11 01:25:18]

Try jobs failed on following builders:
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[qinmin, 2014-10-13 17:00:52]

The CQ bit was checked by qinmin@chromium.org

[commit-bot: I haz the power, 2014-10-13 17:01:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/623363002/280001

[commit-bot: I haz the power, 2014-10-13 19:09:46]

Committed patchset #3 (id:280001)

[commit-bot: I haz the power, 2014-10-13 19:10:26]

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