From 7299d813506cfcd197927b7a630e424770366339 Mon Sep 17 00:00:00 2001 From: "qinmin@chromium.org" Date: Mon, 17 Jun 2013 20:23:49 +0000 Subject: Don't perform seek when surface changes if we are already seeking Currently when surface changes, we always perform a seek so that the decoder will get I-frame first. However, if we are already seeking, there is no need to do that. BUG=233420 Review URL: https://chromiumcodereview.appspot.com/16955011 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@206788 0039d316-1c4b-4281-b951-d872f2087c98 --- media/base/android/media_source_player.cc | 4 + media/base/android/media_source_player_unittest.cc | 86 +++++++++++++--------- 2 files changed, 56 insertions(+), 34 deletions(-) (limited to 'media/base') diff --git a/media/base/android/media_source_player.cc b/media/base/android/media_source_player.cc index 95d0660..7e65b7d 100644 --- a/media/base/android/media_source_player.cc +++ b/media/base/android/media_source_player.cc @@ -291,6 +291,10 @@ MediaSourcePlayer::~MediaSourcePlayer() { void MediaSourcePlayer::SetVideoSurface(gfx::ScopedJavaSurface surface) { surface_ = surface.Pass(); pending_event_ |= SURFACE_CHANGE_EVENT_PENDING; + if (pending_event_ & SEEK_EVENT_PENDING) { + // Waiting for the seek to finish. + return; + } // Setting a new surface will require a new MediaCodec to be created. // Request a seek so that the new decoder will decode an I-frame first. // Or otherwise, the new MediaCodec might crash. See b/8950387. diff --git a/media/base/android/media_source_player_unittest.cc b/media/base/android/media_source_player_unittest.cc index 713a574..ee03c30 100644 --- a/media/base/android/media_source_player_unittest.cc +++ b/media/base/android/media_source_player_unittest.cc @@ -93,22 +93,30 @@ class MediaSourcePlayerTest : public testing::Test { return player_->video_decoder_job_.get(); } + // Starts an audio decoder job. + void StartAudioDecoderJob() { + MediaPlayerHostMsg_DemuxerReady_Params params; + params.audio_codec = kCodecVorbis; + params.audio_channels = 2; + params.audio_sampling_rate = 44100; + params.is_audio_encrypted = false; + Start(params); + } + + void StartVideoDecoderJob() { + MediaPlayerHostMsg_DemuxerReady_Params params; + params.video_codec = kCodecVP8; + params.video_size = gfx::Size(320, 240); + params.is_video_encrypted = false; + Start(params); + } + // Starts decoding the data. void Start(const MediaPlayerHostMsg_DemuxerReady_Params& params) { player_->DemuxerReady(params); player_->Start(); } - // Set the surface to the player. - void SetVideoSurface(gfx::ScopedJavaSurface surface) { - unsigned last_seek_request_id = manager_->last_seek_request_id(); - // Calling SetVideoSurface will trigger a seek. - player_->SetVideoSurface(surface.Pass()); - EXPECT_EQ(last_seek_request_id + 1, manager_->last_seek_request_id()); - // Sending back the seek ACK. - player_->OnSeekRequestAck(manager_->last_seek_request_id()); - } - protected: scoped_ptr manager_; scoped_ptr player_; @@ -122,12 +130,7 @@ TEST_F(MediaSourcePlayerTest, StartAudioDecoderWithValidConfig) { return; // Test audio decoder job will be created when codec is successfully started. - MediaPlayerHostMsg_DemuxerReady_Params params; - params.audio_codec = kCodecVorbis; - params.audio_channels = 2; - params.audio_sampling_rate = 44100; - params.is_audio_encrypted = false; - Start(params); + StartAudioDecoderJob(); EXPECT_TRUE(NULL != GetMediaDecoderJob(true)); EXPECT_EQ(1, manager_->num_requests()); } @@ -158,16 +161,14 @@ TEST_F(MediaSourcePlayerTest, StartVideoCodecWithValidSurface) { scoped_refptr surface_texture( new gfx::SurfaceTextureBridge(0)); gfx::ScopedJavaSurface surface(surface_texture.get()); - MediaPlayerHostMsg_DemuxerReady_Params params; - params.video_codec = kCodecVP8; - params.video_size = gfx::Size(320, 240); - params.is_video_encrypted = false; - Start(params); + StartVideoDecoderJob(); // Video decoder job will not be created until surface is available. EXPECT_TRUE(NULL == GetMediaDecoderJob(false)); EXPECT_EQ(0, manager_->num_requests()); - SetVideoSurface(surface.Pass()); + player_->SetVideoSurface(surface.Pass()); + EXPECT_EQ(1u, manager_->last_seek_request_id()); + player_->OnSeekRequestAck(manager_->last_seek_request_id()); // The decoder job should be ready now. EXPECT_TRUE(NULL != GetMediaDecoderJob(false)); EXPECT_EQ(1, manager_->num_requests()); @@ -181,18 +182,16 @@ TEST_F(MediaSourcePlayerTest, StartVideoCodecWithInvalidSurface) { scoped_refptr surface_texture( new gfx::SurfaceTextureBridge(0)); gfx::ScopedJavaSurface surface(surface_texture.get()); - MediaPlayerHostMsg_DemuxerReady_Params params; - params.video_codec = kCodecVP8; - params.video_size = gfx::Size(320, 240); - params.is_video_encrypted = false; - Start(params); + StartVideoDecoderJob(); // Video decoder job will not be created until surface is available. EXPECT_TRUE(NULL == GetMediaDecoderJob(false)); EXPECT_EQ(0, manager_->num_requests()); // Release the surface texture. surface_texture = NULL; - SetVideoSurface(surface.Pass()); + player_->SetVideoSurface(surface.Pass()); + EXPECT_EQ(1u, manager_->last_seek_request_id()); + player_->OnSeekRequestAck(manager_->last_seek_request_id()); EXPECT_TRUE(NULL == GetMediaDecoderJob(false)); EXPECT_EQ(0, manager_->num_requests()); } @@ -202,12 +201,7 @@ TEST_F(MediaSourcePlayerTest, ReadFromDemuxerAfterSeek) { return; // Test decoder job will resend a ReadFromDemuxer request after seek. - MediaPlayerHostMsg_DemuxerReady_Params params; - params.audio_codec = kCodecVorbis; - params.audio_channels = 2; - params.audio_sampling_rate = 44100; - params.is_audio_encrypted = false; - Start(params); + StartAudioDecoderJob(); EXPECT_TRUE(NULL != GetMediaDecoderJob(true)); EXPECT_EQ(1, manager_->num_requests()); @@ -220,5 +214,29 @@ TEST_F(MediaSourcePlayerTest, ReadFromDemuxerAfterSeek) { EXPECT_EQ(2, manager_->num_requests()); } +TEST_F(MediaSourcePlayerTest, SetSurfaceWhileSeeking) { + if (!MediaCodecBridge::IsAvailable()) + return; + + // Test SetVideoSurface() will not cause an extra seek while the player is + // waiting for a seek ACK. + scoped_refptr surface_texture( + new gfx::SurfaceTextureBridge(0)); + gfx::ScopedJavaSurface surface(surface_texture.get()); + StartVideoDecoderJob(); + // Player is still waiting for SetVideoSurface(), so no request is sent. + EXPECT_EQ(0, manager_->num_requests()); + player_->SeekTo(base::TimeDelta()); + EXPECT_EQ(1u, manager_->last_seek_request_id()); + + player_->SetVideoSurface(surface.Pass()); + EXPECT_TRUE(NULL == GetMediaDecoderJob(false)); + EXPECT_EQ(1u, manager_->last_seek_request_id()); + + // Send the seek ack, player should start requesting data afterwards. + player_->OnSeekRequestAck(manager_->last_seek_request_id()); + EXPECT_TRUE(NULL != GetMediaDecoderJob(false)); + EXPECT_EQ(1, manager_->num_requests()); +} } // namespace media -- cgit v1.1