Use cached Key frames to avoid browser seek

Use cached Key frames to avoid browser seek

This change allows MediaSourcePlayer to skip browser seek if a key frame is found in cache.
Key frames can be identified by an empty data buffer, a config change or is_key_frame field.

BUG=304234
Committed: https://crrev.com/ae2b37fe7550f276ccf01d84cb88b2cb913d441e
Cr-Commit-Position: refs/heads/master@{#314699}

[qinmin, 2015-02-04 03:30:26]

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

[qinmin, 2015-02-04 03:30:27]

PTAL

[wolenetz, 2015-02-04 21:07:23]

Mostly nits.
Also, why is the std::min used in SetCurrentFrameToPreviouslyCachedKeyFrame()?
I'm also curious: where is MDJ::is_video() ever used, or overridden?

https://codereview.chromium.org/898843002/diff/1/media/base/android/media_dec...
File media/base/android/media_decoder_job.h (right):

https://codereview.chromium.org/898843002/diff/1/media/base/android/media_dec...
media/base/android/media_decoder_job.h:68: // STATUS_KEY_FRAME_REQUIRED if a
browser seek is required.
nit: retain comment around |callback| usage

https://codereview.chromium.org/898843002/diff/1/media/base/android/media_sou...
File media/base/android/media_source_player.cc (right):

https://codereview.chromium.org/898843002/diff/1/media/base/android/media_sou...
media/base/android/media_source_player.cc:567: if
(MediaDecoderJob::STATUS_SUCCESS == audio_decoder_job_->Decode(
nit: here and in DecodeMoreVideo(), use a switch to cover all possible return
statuses for clarity?

https://codereview.chromium.org/898843002/diff/1/media/base/android/media_sou...
File media/base/android/media_source_player_unittest.cc (right):

https://codereview.chromium.org/898843002/diff/1/media/base/android/media_sou...
media/base/android/media_source_player_unittest.cc:1556:
WaitForVideoDecodeDone();
nit: after waiting here, confirm explicitly that the video decoder job has no
data?

[qinmin, 2015-02-04 23:11:23]

For SetCurrentFrameToPreviouslyCachedKeyFrame(), I use std::min() because the
access_unit_index can be equal to the size of the access_units.
Whenenver an AU is decoded, we increment the access_unit_index by 1. When the
last AU in the DemuxerData is decoded, the access_unit_index will be identical
to access_unit.size(). That will cause an OOB error if we use it as an array
index, so adding a std::min() solves the issue.

I removed is_video() function. I want to add it to DLOG messages to separate
audio decoder logs from video decoder logs, but we can leave it in a separate
change

https://codereview.chromium.org/898843002/diff/1/media/base/android/media_dec...
File media/base/android/media_decoder_job.h (right):

https://codereview.chromium.org/898843002/diff/1/media/base/android/media_dec...
media/base/android/media_decoder_job.h:68: // STATUS_KEY_FRAME_REQUIRED if a
browser seek is required.
On 2015/02/04 21:07:23, wolenetz wrote:
> nit: retain comment around |callback| usage

Done.

https://codereview.chromium.org/898843002/diff/1/media/base/android/media_sou...
File media/base/android/media_source_player.cc (right):

https://codereview.chromium.org/898843002/diff/1/media/base/android/media_sou...
media/base/android/media_source_player.cc:567: if
(MediaDecoderJob::STATUS_SUCCESS == audio_decoder_job_->Decode(
On 2015/02/04 21:07:23, wolenetz wrote:
> nit: here and in DecodeMoreVideo(), use a switch to cover all possible return
> statuses for clarity?

Done.

https://codereview.chromium.org/898843002/diff/1/media/base/android/media_sou...
File media/base/android/media_source_player_unittest.cc (right):

https://codereview.chromium.org/898843002/diff/1/media/base/android/media_sou...
media/base/android/media_source_player_unittest.cc:1556:
WaitForVideoDecodeDone();
On 2015/02/04 21:07:23, wolenetz wrote:
> nit: after waiting here, confirm explicitly that the video decoder job has no
> data?

Done.

[wolenetz, 2015-02-04 23:59:20]

lgtm

[qinmin, 2015-02-05 00:03:20]

The CQ bit was checked by qinmin@chromium.org

[commit-bot: I haz the power, 2015-02-05 00:03:56]

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

[commit-bot: I haz the power, 2015-02-05 00:59:45]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-02-05 01:00:59]

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