From 583edfcdcf3ea74f98d8bf0d2cedfbb95486ed70 Mon Sep 17 00:00:00 2001 From: "qinmin@chromium.org" Date: Wed, 18 Dec 2013 02:28:05 +0000 Subject: Fix a crash that prefetch can happen while decoder job is null. Here is why the problem can happen: 1. a surface change happens, and the decoder is still decoding. (surface change event pending) 2. decoder starved, so a pending prefetch request is added.(surface change, prefetch event pending) 3. decoder finished decoding, surface change event is cleaned first since the decoder will be recreated (prefetch event pending). 4. Because we are not at an I-frame, a seek is initiated before the decoder job gets created. (seek event, prefetch event pending) 5. seek finishes, start to handle the prefetch event. However, the current prefetch event handling assumes the decoder job is non-null. This will cause a crash. To fix the problem, we can either: 1. not clearing the pending surface change event before decoder job gets created. Or 2. Clear the prefetch event after seek is finished. This is because StartInternal() will automatically trigger a prefetch if playback is not paused. Or 3. When surface changes, create the decoder job first even if we are not at an iframe. Seek is sent after decoder job gets created. This change implements solution 2. BUG=328900 Review URL: https://codereview.chromium.org/117233004 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@241451 0039d316-1c4b-4281-b951-d872f2087c98 --- media/base/android/media_source_player.cc | 2 ++ media/base/android/media_source_player_unittest.cc | 31 ++++++++++++++++++++-- 2 files changed, 31 insertions(+), 2 deletions(-) (limited to 'media') diff --git a/media/base/android/media_source_player.cc b/media/base/android/media_source_player.cc index e5b518e..ee84528 100644 --- a/media/base/android/media_source_player.cc +++ b/media/base/android/media_source_player.cc @@ -423,6 +423,8 @@ void MediaSourcePlayer::OnDemuxerSeekDone( DVLOG(1) << __FUNCTION__; ClearPendingEvent(SEEK_EVENT_PENDING); + if (IsEventPending(PREFETCH_REQUEST_EVENT_PENDING)) + ClearPendingEvent(PREFETCH_REQUEST_EVENT_PENDING); next_video_data_is_iframe_ = true; diff --git a/media/base/android/media_source_player_unittest.cc b/media/base/android/media_source_player_unittest.cc index 50fdd5f..7970acc 100644 --- a/media/base/android/media_source_player_unittest.cc +++ b/media/base/android/media_source_player_unittest.cc @@ -571,8 +571,10 @@ class MediaSourcePlayerTest : public testing::Test { void WaitForDecodeDone(bool wait_for_audio, bool wait_for_video) { DCHECK(wait_for_audio || wait_for_video); - while ((wait_for_audio && GetMediaDecoderJob(true)->is_decoding()) || - (wait_for_video && GetMediaDecoderJob(false)->is_decoding())) { + while ((wait_for_audio && GetMediaDecoderJob(true) && + GetMediaDecoderJob(true)->is_decoding()) || + (wait_for_video && GetMediaDecoderJob(false) && + GetMediaDecoderJob(false)->is_decoding())) { message_loop_.RunUntilIdle(); } } @@ -1746,6 +1748,31 @@ TEST_F(MediaSourcePlayerTest, NewSurfaceWhileChangingConfigs) { EXPECT_EQ(0, demuxer_->num_seek_requests()); } +TEST_F(MediaSourcePlayerTest, + BrowserSeek_DecoderStarvationWhilePendingSurfaceChange) { + SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE(); + + // Test video decoder starvation while handling a pending surface change + // should not cause any crashes. + CreateNextTextureAndSetVideoSurface(); + StartVideoDecoderJob(true); + DemuxerData data = CreateReadFromDemuxerAckForVideo(); + player_.OnDemuxerDataAvailable(data); + + // Trigger a surface change and decoder starvation. + CreateNextTextureAndSetVideoSurface(); + TriggerPlayerStarvation(); + WaitForVideoDecodeDone(); + + // Surface change should trigger a seek. + EXPECT_EQ(1, demuxer_->num_browser_seek_requests()); + player_.OnDemuxerSeekDone(base::TimeDelta()); + EXPECT_TRUE(GetMediaDecoderJob(false)); + + // A new data request should be sent. + EXPECT_EQ(2, demuxer_->num_data_requests()); +} + TEST_F(MediaSourcePlayerTest, ReleaseWithOnPrefetchDoneAlreadyPosted) { SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE(); -- cgit v1.1