Refactor MSE implementation on Android to simplify the logic and improve the performance

media/base/android/media_source_player_unittest.cc

Index: media/base/android/media_source_player_unittest.cc
diff --git a/media/base/android/media_source_player_unittest.cc b/media/base/android/media_source_player_unittest.cc
index db09b544a6cc608c2a4d893b37dea0eb419e16c9..5dec69e048bb8f05fa2adf2894b9b87f2b2e216e 100644
--- a/media/base/android/media_source_player_unittest.cc
+++ b/media/base/android/media_source_player_unittest.cc
@@ -8,10 +8,12 @@
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/stringprintf.h"
+#include "media/base/android/audio_decoder_job.h"
 #include "media/base/android/media_codec_bridge.h"
 #include "media/base/android/media_drm_bridge.h"
 #include "media/base/android/media_player_manager.h"
 #include "media/base/android/media_source_player.h"
+#include "media/base/android/video_decoder_job.h"
 #include "media/base/bind_to_current_loop.h"
 #include "media/base/decoder_buffer.h"
 #include "media/base/test_data_util.h"
@@ -186,7 +188,8 @@ class MediaSourcePlayerTest : public testing::Test {
   virtual ~MediaSourcePlayerTest() {}
 
  protected:
-  // Get the decoder job from the MediaSourcePlayer.
+  // Get the decoder job from the MediaSourcePlayer. The return value must not
+  // be NULL.
   MediaDecoderJob* GetMediaDecoderJob(bool is_audio) {
     if (is_audio) {
       return reinterpret_cast<MediaDecoderJob*>(
@@ -196,10 +199,18 @@ class MediaSourcePlayerTest : public testing::Test {
         player_.video_decoder_job_.get());
   }
 
+  // Get the MediaCodecBridge from the decoder job. The return value could be
+  // NULL if the decoder is not yet created.
+  MediaCodecBridge* GetMediaCodecBridge(bool is_audio) {
+    if (is_audio)
+      return player_.audio_decoder_job_->media_codec_bridge_.get();
+    return player_.video_decoder_job_->media_codec_bridge_.get();
+  }
+
   // Get the per-job prerolling status from the MediaSourcePlayer's job matching
   // |is_audio|. Caller must guard against NPE if the player's job is NULL.
   bool IsPrerolling(bool is_audio) {
-    return GetMediaDecoderJob(is_audio)->prerolling();
+    return GetMediaDecoderJob(is_audio)->prerolling_;
   }
 
   // Get the preroll timestamp from the MediaSourcePlayer.
@@ -218,8 +229,6 @@ class MediaSourcePlayerTest : public testing::Test {
     EXPECT_TRUE(player_.IsPlaying());
     player_.Release();
     EXPECT_FALSE(player_.IsPlaying());
-    EXPECT_FALSE(GetMediaDecoderJob(true));
-    EXPECT_FALSE(GetMediaDecoderJob(false));
   }
 
   // Upon the next successful decode callback, post a task to call Release()
@@ -236,22 +245,17 @@ class MediaSourcePlayerTest : public testing::Test {
   }
 
   // Asynch test callback posted upon decode completion to verify that a pending
-  // prefetch done event is cleared across |player_|'s Release(). This helps
+  // prefetch done event is not cleared across |player_|'s Release(). This helps
   // ensure the ReleaseWithOnPrefetchDoneAlreadyPosted test scenario is met.
   void ReleaseWithPendingPrefetchDoneVerification() {
     EXPECT_TRUE(player_.IsEventPending(player_.PREFETCH_DONE_EVENT_PENDING));
     ReleasePlayer();
-    EXPECT_FALSE(player_.IsEventPending(player_.PREFETCH_DONE_EVENT_PENDING));
+    EXPECT_TRUE(player_.IsEventPending(player_.PREFETCH_DONE_EVENT_PENDING));
     EXPECT_FALSE(decoder_callback_hook_executed_);
+    EXPECT_FALSE(GetMediaCodecBridge(true));
     decoder_callback_hook_executed_ = true;
   }
 
-  // Inspect internal pending_event_ state of |player_|. This is for infrequent
-  // use by tests, only where required.
-  bool IsPendingSurfaceChange() {
-    return player_.IsEventPending(player_.SURFACE_CHANGE_EVENT_PENDING);
-  }
-
   DemuxerConfigs CreateAudioDemuxerConfigs(AudioCodec audio_codec) {
     DemuxerConfigs configs;
     configs.audio_codec = audio_codec;
@@ -280,9 +284,9 @@ class MediaSourcePlayerTest : public testing::Test {
     return configs;
   }
 
-  DemuxerConfigs CreateVideoDemuxerConfigs() {
+  DemuxerConfigs CreateVideoDemuxerConfigs(VideoCodec video_codec) {
     DemuxerConfigs configs;
-    configs.video_codec = kCodecVP8;
+    configs.video_codec = video_codec;
     configs.video_size = gfx::Size(320, 240);
     configs.is_video_encrypted = false;
     configs.duration = kDefaultDuration;
@@ -304,7 +308,7 @@ class MediaSourcePlayerTest : public testing::Test {
       return CreateAudioDemuxerConfigs(kCodecVorbis);
 
     if (have_video && !have_audio)
-      return CreateVideoDemuxerConfigs();
+      return CreateVideoDemuxerConfigs(kCodecVP8);
 
     return CreateAudioVideoDemuxerConfigs();
   }
@@ -316,7 +320,7 @@ class MediaSourcePlayerTest : public testing::Test {
 
   // Starts a video decoder job.
   void StartVideoDecoderJob() {
-    Start(CreateVideoDemuxerConfigs());
+    Start(CreateVideoDemuxerConfigs(kCodecVP8));
   }
 
   // Starts decoding the data.
@@ -326,8 +330,8 @@ class MediaSourcePlayerTest : public testing::Test {
     player_.Start();
 
     EXPECT_TRUE(player_.IsPlaying());
-    int expected_num_requests = (GetMediaDecoderJob(true) ? 1 : 0) +
-        (GetMediaDecoderJob(false) ? 1 : 0);
+    int expected_num_requests = (player_.HasAudio() ? 1 : 0) +

[wolenetz, 2014/06/04 00:21:47]

nit: Do all calls to this helper supply valid configs? If so, maybe we shouldn't
trust |player_| Has*() implementation, and should check them in this helper
versus the supplied configs? Something like
EXPECT_TRUE(player_.Has[Audio/Video]() == (configs.[video/audio]_codec is a
known codec)?

[qinmin, 2014/06/04 19:29:15]

No, StartAudioDecoderWithInvalidConfig test will pass invalid configs to the
decoder job. However, HasAudio()/HasVideo() only checks whether the stream has
audio/video part. It doesn't have any impact on whether data request are sent.
Since the decoder is created lazily when Decode() is called, prefetch() will
fetch the data even if the config is invalid. 

On 2014/06/04 00:21:47, wolenetz wrote:

+        (player_.HasVideo() ? 1 : 0);
     EXPECT_EQ(expected_num_requests, demuxer_->num_data_requests());
   }
 
@@ -349,9 +353,9 @@ class MediaSourcePlayerTest : public testing::Test {
     EXPECT_EQ(original_num_data_requests + expected_request_delta,
               demuxer_->num_data_requests());
     EXPECT_EQ(expect_player_requests_audio_data,

[wolenetz, 2014/06/04 00:21:47]

These are no-ops now. Remove? Can we deterministically expect that the decoder
bridges are created based on some input here?

[qinmin, 2014/06/04 19:29:15]

Done. Start() won't create the decoder job because it will call Prefetch()
first. So at this moment we cannot determine whether the MediaCodecBridges are
created. Tests that call this function should check the decoder bridge by
themselves

On 2014/06/04 00:21:47, wolenetz wrote:

-              expect_player_requests_audio_data && GetMediaDecoderJob(true));
+              expect_player_requests_audio_data);
     EXPECT_EQ(expect_player_requests_video_data,
-              expect_player_requests_video_data && GetMediaDecoderJob(false));
+              expect_player_requests_video_data);
   }
 
   // Keeps decoding audio data until the decoder starts to output samples.
@@ -439,7 +443,7 @@ class MediaSourcePlayerTest : public testing::Test {
   // decoding.
   void StartAudioDecoderJobAndSeekToWhileDecoding(
       const base::TimeDelta& seek_time) {
-    EXPECT_FALSE(GetMediaDecoderJob(true));
+    EXPECT_FALSE(GetMediaCodecBridge(true));
     EXPECT_FALSE(player_.IsPlaying());
     EXPECT_EQ(0, demuxer_->num_data_requests());
     EXPECT_EQ(0.0, GetPrerollTimestamp().InMillisecondsF());
@@ -476,6 +480,9 @@ class MediaSourcePlayerTest : public testing::Test {
     // caused by the seek.
     player_.OnDemuxerDataAvailable(CreateAbortedAck(is_audio));
 
+    // Wait for the decode job to finish so we can process the seek request.
+    WaitForDecodeDone(is_audio, !is_audio);
+
     // Verify that the seek is requested.
     EXPECT_EQ(original_num_seeks + 1, demuxer_->num_seek_requests());
 
@@ -520,8 +527,10 @@ class MediaSourcePlayerTest : public testing::Test {
     EXPECT_LE(target_timestamp, player_.GetCurrentTime());
   }
 
-  DemuxerData CreateReadFromDemuxerAckWithConfigChanged(bool is_audio,
-                                                        int config_unit_index) {
+  DemuxerData CreateReadFromDemuxerAckWithConfigChanged(
+      bool is_audio,
+      int config_unit_index,
+      const DemuxerConfigs& configs) {
     DemuxerData data;
     data.type = is_audio ? DemuxerStream::AUDIO : DemuxerStream::VIDEO;
     data.access_units.resize(config_unit_index + 1);
@@ -531,7 +540,7 @@ class MediaSourcePlayerTest : public testing::Test {
 
     data.access_units[config_unit_index].status = DemuxerStream::kConfigChanged;
     data.demuxer_configs.resize(1);
-    data.demuxer_configs[0] = CreateDemuxerConfigs(is_audio, !is_audio);
+    data.demuxer_configs[0] = configs;
     return data;
   }
 
@@ -543,27 +552,34 @@ class MediaSourcePlayerTest : public testing::Test {
   // browser seek results once decode completes and surface change processing
   // begins.
   void BrowserSeekPlayer(bool trigger_with_release_start) {
-    int expected_num_data_requests = demuxer_->num_data_requests() +
-        (trigger_with_release_start ? 1 : 2);
+    int expected_num_data_requests = demuxer_->num_data_requests() + 2;
     int expected_num_seek_requests = demuxer_->num_seek_requests();
     int expected_num_browser_seek_requests =
         demuxer_->num_browser_seek_requests();
 
-    EXPECT_FALSE(GetMediaDecoderJob(false));
     CreateNextTextureAndSetVideoSurface();
     StartVideoDecoderJob();
     if (trigger_with_release_start) {
+      // Consume the first frame, so that the next VideoDecoderJob will not
+      // inherit the I-frame from the previous decoder.
+      player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForVideo());
       ReleasePlayer();
+      WaitForVideoDecodeDone();
+
       // Simulate demuxer's response to the video data request. The data will be
-      // discarded.
+      // passed to the next MediaCodecBridge.
       player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForVideo());
-      EXPECT_FALSE(GetMediaDecoderJob(false));
+      EXPECT_FALSE(GetMediaCodecBridge(false));
       EXPECT_FALSE(player_.IsPlaying());
       EXPECT_EQ(expected_num_seek_requests, demuxer_->num_seek_requests());
 
       CreateNextTextureAndSetVideoSurface();
       Resume(false, false);
-      EXPECT_FALSE(GetMediaDecoderJob(false));
+      EXPECT_FALSE(GetMediaCodecBridge(false));
+
+      // Run the message loop so that prefetch will complete.
+      while (expected_num_seek_requests == demuxer_->num_seek_requests())
+        message_loop_.RunUntilIdle();
     } else {
       // Simulate demuxer's response to the video data request.
       player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForVideo());
@@ -576,7 +592,7 @@ class MediaSourcePlayerTest : public testing::Test {
       CreateNextTextureAndSetVideoSurface();
 
       // Browser seek should not begin until decoding has completed.
-      EXPECT_TRUE(GetMediaDecoderJob(false));
+      EXPECT_TRUE(GetMediaCodecBridge(false));
       EXPECT_EQ(expected_num_seek_requests, demuxer_->num_seek_requests());
 
       // Wait for the decoder job to finish decoding and be reset pending the

[wolenetz, 2014/06/04 00:21:46]

nit: here and multiple other places in this file, the decoder job is not 'reset'
any longer. Perhaps s/decoder job/media codec bridge/?

[qinmin, 2014/06/04 19:29:15]

Done.

@@ -601,10 +617,10 @@ class MediaSourcePlayerTest : public testing::Test {
   // false, regular data is fed and decoded prior to feeding the config change
   // AU in response to the second data request (after prefetch completed).
   // |config_unit_index| controls which access unit is |kConfigChanged|.
-  void StartConfigChange(bool is_audio,
-                         bool config_unit_in_prefetch,
-                         int config_unit_index) {
-    EXPECT_FALSE(GetMediaDecoderJob(is_audio));
+  void SendConfigChangeToDecoder(bool is_audio,
+                                 bool config_unit_in_prefetch,
+                                 int config_unit_index) {
+    EXPECT_FALSE(GetMediaCodecBridge(is_audio));
     if (is_audio) {
       StartAudioDecoderJob();
     } else {
@@ -613,7 +629,6 @@ class MediaSourcePlayerTest : public testing::Test {
     }
 
     int expected_num_data_requests = demuxer_->num_data_requests();
-
     // Feed and decode a standalone access unit so the player exits prefetch.
     if (!config_unit_in_prefetch) {
       if (is_audio)
@@ -628,12 +643,31 @@ class MediaSourcePlayerTest : public testing::Test {
       EXPECT_EQ(expected_num_data_requests, demuxer_->num_data_requests());
     }
 
+    DemuxerConfigs configs = is_audio ? CreateAudioDemuxerConfigs(kCodecAAC) :
+        CreateVideoDemuxerConfigs(kCodecVP9);
     // Feed and decode access units with data for any units prior to
     // |config_unit_index|, and a |kConfigChanged| unit at that index.
     // Player should prepare to reconfigure the decoder job, and should request
     // new demuxer configs.
-    player_.OnDemuxerDataAvailable(
-        CreateReadFromDemuxerAckWithConfigChanged(is_audio, config_unit_index));
+    player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckWithConfigChanged(
+        is_audio, config_unit_index, configs));
+
+    expected_num_data_requests++;
+    EXPECT_EQ(expected_num_data_requests, demuxer_->num_data_requests());
+    if (is_audio)
+      player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForAudio(0));
+    else
+      player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForVideo());
+  }
+
+  // Send a config change to the decoder job and drain the decoder so that the
+  // config change is processed.
+  void StartConfigChange(bool is_audio,
+                         bool config_unit_in_prefetch,
+                         int config_unit_index) {
+    SendConfigChangeToDecoder(is_audio, config_unit_in_prefetch,
+                              config_unit_index);
+    int expected_num_data_requests = demuxer_->num_data_requests();
     // Run until decoder starts to request new data.

[wolenetz, 2014/06/04 00:21:47]

Should IsDraining(is_audio) be true here, and false after the while loop, below?
Seems like a good thing to add checks, if so.

[qinmin, 2014/06/04 19:29:15]

it really depends on config_unit_in_prefetch and config_unit_index.
If config_unit_in_prefetch is true, then IsDraining() will be false since there
is no need to drain the decoder.
Otherwise, only if config_unit_index==0, then IsDraining() will be true.
added a EXPECT_EQ here and a EXPECT_FALSE(IsDrainging(is_audio)) after the while
statement

On 2014/06/04 00:21:47, wolenetz wrote:

     while (demuxer_->num_data_requests() == expected_num_data_requests)
       message_loop_.RunUntilIdle();
@@ -658,10 +692,10 @@ class MediaSourcePlayerTest : public testing::Test {
   // assumed to exist for any stream whose decode completion is awaited.
   void WaitForDecodeDone(bool wait_for_audio, bool wait_for_video) {
     DCHECK(wait_for_audio || wait_for_video);
-    while ((wait_for_audio && GetMediaDecoderJob(true) &&
+    while ((wait_for_audio && GetMediaCodecBridge(true) &&
                GetMediaDecoderJob(true)->HasData() &&
                GetMediaDecoderJob(true)->is_decoding()) ||
-           (wait_for_video && GetMediaDecoderJob(false) &&
+           (wait_for_video && GetMediaCodecBridge(false) &&
                GetMediaDecoderJob(false)->HasData() &&
                GetMediaDecoderJob(false)->is_decoding())) {
       message_loop_.RunUntilIdle();
@@ -764,6 +798,14 @@ class MediaSourcePlayerTest : public testing::Test {
     return player_.start_time_ticks_;
   }
 
+  bool IsRequestingDemuxerData(bool is_audio) {
+    return GetMediaDecoderJob(is_audio)->is_requesting_demuxer_data_;
+  }
+
+  bool IsDrainingDecoder(bool is_audio) {
+    return GetMediaDecoderJob(is_audio)->drain_decoder_;
+  }
+
   base::MessageLoop message_loop_;
   MockMediaPlayerManager manager_;
   MockDemuxerAndroid* demuxer_;  // Owned by |player_|.
@@ -812,28 +854,23 @@ TEST_F(MediaSourcePlayerTest, StartAudioDecoderWithInvalidConfig) {
 TEST_F(MediaSourcePlayerTest, StartVideoCodecWithValidSurface) {
   SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
 
-  // Test video decoder job will be created when surface is valid.
-  // Video decoder job will not be created until surface is available.
+  // Test video decoder job will not be created until data is received.
   StartVideoDecoderJob();
 
   // Set both an initial and a later video surface without receiving any
   // demuxed data yet.
   CreateNextTextureAndSetVideoSurface();
-  MediaDecoderJob* first_job = GetMediaDecoderJob(false);
-  EXPECT_TRUE(first_job);
+  EXPECT_FALSE(GetMediaCodecBridge(false));
   CreateNextTextureAndSetVideoSurface();
-
-  // Setting another surface will not create a new job until any pending
-  // read is satisfied (and job is no longer decoding).
-  EXPECT_EQ(first_job, GetMediaDecoderJob(false));
+  EXPECT_FALSE(GetMediaCodecBridge(false));
 
   // No seeks, even on setting surface, should have occurred. (Browser seeks can
   // occur on setting surface, but only after previously receiving video data.)
   EXPECT_EQ(0, demuxer_->num_seek_requests());
 
-  // Note, the decoder job for the second surface set, above, will be created
-  // only after the pending read is satisfied and decoded, and the resulting
-  // browser seek is done. See BrowserSeek_* tests for this coverage.
+  // Send the first input chunk and verify that decoder will be created.
+  player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForVideo());
+  EXPECT_TRUE(GetMediaCodecBridge(false));
 }
 
 TEST_F(MediaSourcePlayerTest, StartVideoCodecWithInvalidSurface) {
@@ -851,9 +888,7 @@ TEST_F(MediaSourcePlayerTest, StartVideoCodecWithInvalidSurface) {
 
   // Player should not seek the demuxer on setting initial surface.
   EXPECT_EQ(0, demuxer_->num_seek_requests());
-
-  EXPECT_FALSE(GetMediaDecoderJob(false));
-  EXPECT_EQ(0, demuxer_->num_data_requests());
+  EXPECT_FALSE(GetMediaCodecBridge(false));

[wolenetz, 2014/06/04 00:21:46]

nit: We should expect prefetch of 1 read at this point, IIUC. Check this?

[qinmin, 2014/06/04 19:29:15]

Done.

 }
 
 TEST_F(MediaSourcePlayerTest, ReadFromDemuxerAfterSeek) {
@@ -869,26 +904,23 @@ TEST_F(MediaSourcePlayerTest, SetSurfaceWhileSeeking) {
 
   // Test SetVideoSurface() will not cause an extra seek while the player is
   // waiting for demuxer to indicate seek is done.
-  // Player is still waiting for SetVideoSurface(), so no request is sent.
-  StartVideoDecoderJob();  // Verifies no data requested.
+  player_.OnDemuxerConfigsAvailable(CreateVideoDemuxerConfigs(kCodecVP8));
 
   // Initiate a seek. Skip requesting element seek of renderer.
   // Instead behave as if the renderer has asked us to seek.
-  EXPECT_EQ(0, demuxer_->num_seek_requests());
   player_.SeekTo(base::TimeDelta());
   EXPECT_EQ(1, demuxer_->num_seek_requests());
 
   CreateNextTextureAndSetVideoSurface();
-  EXPECT_FALSE(GetMediaDecoderJob(false));
   EXPECT_EQ(1, demuxer_->num_seek_requests());
-
-  // Reconfirm player has not yet requested data.
-  EXPECT_EQ(0, demuxer_->num_data_requests());
+  player_.Start();
 
   // Send the seek done notification. The player should start requesting data.
   player_.OnDemuxerSeekDone(kNoTimestamp());
-  EXPECT_TRUE(GetMediaDecoderJob(false));
+  EXPECT_FALSE(GetMediaCodecBridge(false));
   EXPECT_EQ(1, demuxer_->num_data_requests());
+  player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForVideo());
+  EXPECT_TRUE(GetMediaCodecBridge(false));
 
   // Reconfirm exactly 1 seek request has been made of demuxer, and that it
   // was not a browser seek request.
@@ -917,7 +949,7 @@ TEST_F(MediaSourcePlayerTest, ChangeMultipleSurfaceWhileDecoding) {
   // Wait for the decoder job to finish decoding and be reset pending a browser
   // seek.
   WaitForVideoDecodeDone();
-  player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForVideo());
+  player_.OnDemuxerDataAvailable(CreateAbortedAck(false));

[wolenetz, 2014/06/04 00:21:47]

Why send kAborted instead of simulating the asynch response to a data request
that renderer processed before browser seek commenced?

[qinmin, 2014/06/04 19:29:15]

If i understand correctly, browser seek can result in 2 behaviors:
1. if chunk demuxer is processing a pending read request, it will send abort and
then the seek ack
2. Otherwise, chunk demuxer just sends the seek ack

So this is behavior 1.
Anyway, changed this to send a data response.

On 2014/06/04 00:21:47, wolenetz wrote:

[wolenetz, 2014/06/04 21:42:24]

You're correct. Either behavior would have been fine here. Sorry.

 
   // Only one browser seek should have been initiated. No further data request
   // should have been processed on |message_loop_| before surface change event
@@ -928,9 +960,12 @@ TEST_F(MediaSourcePlayerTest, ChangeMultipleSurfaceWhileDecoding) {
   // Simulate browser seek is done and confirm player requests more data for new
   // video decoder job.
   player_.OnDemuxerSeekDone(player_.GetCurrentTime());
-  EXPECT_TRUE(GetMediaDecoderJob(false));
+  EXPECT_FALSE(GetMediaCodecBridge(false));
   EXPECT_EQ(3, demuxer_->num_data_requests());
   EXPECT_EQ(1, demuxer_->num_seek_requests());
+
+  player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForVideo());
+  EXPECT_TRUE(GetMediaCodecBridge(false));
 }
 
 TEST_F(MediaSourcePlayerTest, SetEmptySurfaceAndStarveWhileDecoding) {
@@ -951,7 +986,7 @@ TEST_F(MediaSourcePlayerTest, SetEmptySurfaceAndStarveWhileDecoding) {
   // this case.
   TriggerPlayerStarvation();
   // Wait for the decoder job to finish decoding and be reset.
-  while (GetMediaDecoderJob(false))
+  while (GetMediaDecoderJob(false)->is_decoding())
     message_loop_.RunUntilIdle();
 
   // No further seek or data requests should have been received since the
@@ -972,24 +1007,24 @@ TEST_F(MediaSourcePlayerTest, ReleaseVideoDecoderResourcesWhileDecoding) {
   // not be immediately released.
   CreateNextTextureAndSetVideoSurface();
   StartVideoDecoderJob();
-  EXPECT_EQ(1, manager_.num_resources_requested());
+  // No resource is requested since there is no data to decode.
+  EXPECT_EQ(0, manager_.num_resources_requested());
   ReleasePlayer();
-  // The resources will be immediately released since the decoder is idle.
-  EXPECT_EQ(1, manager_.num_resources_released());
+  EXPECT_EQ(0, manager_.num_resources_released());
   player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForVideo());
 
   // Recreate the video decoder.
   CreateNextTextureAndSetVideoSurface();
   player_.Start();
-  EXPECT_EQ(1, demuxer_->num_browser_seek_requests());
-  player_.OnDemuxerSeekDone(base::TimeDelta());
-  EXPECT_EQ(2, manager_.num_resources_requested());
-  player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForVideo());
+  while (!GetMediaDecoderJob(false)->is_decoding())
+    message_loop_.RunUntilIdle();
+  EXPECT_EQ(0, demuxer_->num_browser_seek_requests());
+  EXPECT_EQ(1, manager_.num_resources_requested());
   ReleasePlayer();
   // The resource is still held by the video decoder until it finishes decoding.
-  EXPECT_EQ(1, manager_.num_resources_released());
+  EXPECT_EQ(0, manager_.num_resources_released());
   // Wait for the decoder job to finish decoding and be reset.
-  while (manager_.num_resources_released() != 2)
+  while (manager_.num_resources_released() != 1)
     message_loop_.RunUntilIdle();
 }
 
@@ -999,7 +1034,6 @@ TEST_F(MediaSourcePlayerTest, AudioOnlyStartAfterSeekFinish) {
   // Test audio decoder job will not start until pending seek event is handled.
   DemuxerConfigs configs = CreateAudioDemuxerConfigs(kCodecVorbis);
   player_.OnDemuxerConfigsAvailable(configs);
-  EXPECT_FALSE(GetMediaDecoderJob(true));
 
   // Initiate a seek. Skip requesting element seek of renderer.
   // Instead behave as if the renderer has asked us to seek.
@@ -1007,16 +1041,19 @@ TEST_F(MediaSourcePlayerTest, AudioOnlyStartAfterSeekFinish) {
   EXPECT_EQ(1, demuxer_->num_seek_requests());
 
   player_.Start();
-  EXPECT_FALSE(GetMediaDecoderJob(true));
   EXPECT_EQ(0, demuxer_->num_data_requests());
 
   // Sending back the seek done notification.
   player_.OnDemuxerSeekDone(kNoTimestamp());
-  EXPECT_TRUE(GetMediaDecoderJob(true));
+  EXPECT_FALSE(GetMediaCodecBridge(true));
   EXPECT_EQ(1, demuxer_->num_data_requests());
 
   // Reconfirm exactly 1 seek request has been made of demuxer.
   EXPECT_EQ(1, demuxer_->num_seek_requests());
+
+  // Decoder is created after data is received.
+  player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForAudio(0));
+  EXPECT_TRUE(GetMediaCodecBridge(true));
 }
 
 TEST_F(MediaSourcePlayerTest, VideoOnlyStartAfterSeekFinish) {
@@ -1024,9 +1061,8 @@ TEST_F(MediaSourcePlayerTest, VideoOnlyStartAfterSeekFinish) {
 
   // Test video decoder job will not start until pending seek event is handled.
   CreateNextTextureAndSetVideoSurface();
-  DemuxerConfigs configs = CreateVideoDemuxerConfigs();
+  DemuxerConfigs configs = CreateVideoDemuxerConfigs(kCodecVP8);
   player_.OnDemuxerConfigsAvailable(configs);
-  EXPECT_FALSE(GetMediaDecoderJob(false));
 
   // Initiate a seek. Skip requesting element seek of renderer.
   // Instead behave as if the renderer has asked us to seek.
@@ -1034,16 +1070,19 @@ TEST_F(MediaSourcePlayerTest, VideoOnlyStartAfterSeekFinish) {
   EXPECT_EQ(1, demuxer_->num_seek_requests());
 
   player_.Start();
-  EXPECT_FALSE(GetMediaDecoderJob(false));
   EXPECT_EQ(0, demuxer_->num_data_requests());
 
   // Sending back the seek done notification.
   player_.OnDemuxerSeekDone(kNoTimestamp());
-  EXPECT_TRUE(GetMediaDecoderJob(false));
+  EXPECT_FALSE(GetMediaCodecBridge(false));
   EXPECT_EQ(1, demuxer_->num_data_requests());
 
   // Reconfirm exactly 1 seek request has been made of demuxer.
   EXPECT_EQ(1, demuxer_->num_seek_requests());
+
+  // Decoder is created after data is received.
+  player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForVideo());
+  EXPECT_TRUE(GetMediaCodecBridge(false));
 }
 
 TEST_F(MediaSourcePlayerTest, StartImmediatelyAfterPause) {
@@ -1075,7 +1114,7 @@ TEST_F(MediaSourcePlayerTest, StartImmediatelyAfterPause) {
     message_loop_.RunUntilIdle();
   // The decoder job should finish and wait for data.
   EXPECT_EQ(2, demuxer_->num_data_requests());
-  EXPECT_TRUE(GetMediaDecoderJob(true)->is_requesting_demuxer_data());
+  EXPECT_TRUE(IsRequestingDemuxerData(true));
 }
 
 TEST_F(MediaSourcePlayerTest, DecoderJobsCannotStartWithoutAudio) {
@@ -1119,15 +1158,12 @@ TEST_F(MediaSourcePlayerTest, StartTimeTicksResetAfterDecoderUnderruns) {
   // Let the decoder starve.
   TriggerPlayerStarvation();
   WaitForAudioDecodeDone();
-
-  // Verify the start time ticks is cleared at this point because the
-  // player is prefetching.
-  EXPECT_TRUE(StartTimeTicks() == base::TimeTicks());
+  EXPECT_TRUE(StartTimeTicks() == previous);

[wolenetz, 2014/06/04 00:21:47]

aside: Does this (and the change below) fix the infrequent flakiness of this
test?

[qinmin, 2014/06/04 19:29:15]

I haven't seen this tests failing with this patch, so I think we are ok. The
previous flakiness are mostly solved by the introduction of
DecodeAudioDataUntilOutputBecomesAvailable() i think

On 2014/06/04 00:21:47, wolenetz wrote:

 
   // Send new data to the decoder so it can finish prefetching. This should
   // reset the start time ticks.
   player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForAudio(3));
-  EXPECT_TRUE(StartTimeTicks() != base::TimeTicks());
+  EXPECT_TRUE(StartTimeTicks() != previous);
 
   base::TimeTicks current = StartTimeTicks();
   EXPECT_LE(0, (current - previous).InMillisecondsF());
@@ -1194,8 +1230,9 @@ TEST_F(MediaSourcePlayerTest, AV_PlaybackCompletionAcrossConfigChange) {
   Start(CreateAudioVideoDemuxerConfigs());
 
   player_.OnDemuxerDataAvailable(CreateEOSAck(true));  // Audio EOS
+  DemuxerConfigs configs = CreateVideoDemuxerConfigs(kCodecVP9);
   player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckWithConfigChanged(
-      false, 0));  // Video |kConfigChanged| as first unit.
+      false, 0, configs));  // Video |kConfigChanged| as first unit.
 
   WaitForAudioVideoDecodeDone();
 
@@ -1218,8 +1255,9 @@ TEST_F(MediaSourcePlayerTest, VA_PlaybackCompletionAcrossConfigChange) {
   Start(CreateAudioVideoDemuxerConfigs());
 
   player_.OnDemuxerDataAvailable(CreateEOSAck(false));  // Video EOS
+  // Audio |kConfigChanged| as first unit.
   player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckWithConfigChanged(
-      true, 0));  // Audio |kConfigChanged| as first unit.
+      true, 0, CreateAudioDemuxerConfigs(kCodecVorbis)));
 
   WaitForAudioVideoDecodeDone();
 
@@ -1391,7 +1429,6 @@ TEST_F(MediaSourcePlayerTest, BrowserSeek_RegularSeekPendsBrowserSeekDone) {
 
   // Simulate renderer requesting a regular seek while browser seek in progress.
   player_.SeekTo(base::TimeDelta());
-  EXPECT_FALSE(GetMediaDecoderJob(false));
 
   // Simulate browser seek is done. Confirm player requests the regular seek,
   // still has no video decoder job configured, and has not requested any
@@ -1399,16 +1436,17 @@ TEST_F(MediaSourcePlayerTest, BrowserSeek_RegularSeekPendsBrowserSeekDone) {
   // BrowserSeekPlayer().
   EXPECT_EQ(1, demuxer_->num_seek_requests());
   player_.OnDemuxerSeekDone(base::TimeDelta());
-  EXPECT_FALSE(GetMediaDecoderJob(false));
   EXPECT_EQ(2, demuxer_->num_seek_requests());
   EXPECT_EQ(1, demuxer_->num_browser_seek_requests());
 
   // Simulate regular seek is done and confirm player requests more data for
   // new video decoder job.
   player_.OnDemuxerSeekDone(kNoTimestamp());
-  EXPECT_TRUE(GetMediaDecoderJob(false));
+  EXPECT_FALSE(GetMediaCodecBridge(false));
   EXPECT_EQ(3, demuxer_->num_data_requests());
   EXPECT_EQ(2, demuxer_->num_seek_requests());
+  player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForVideo());
+  EXPECT_TRUE(GetMediaCodecBridge(false));
 }
 
 TEST_F(MediaSourcePlayerTest, BrowserSeek_InitialReleaseAndStart) {
@@ -1425,15 +1463,14 @@ TEST_F(MediaSourcePlayerTest, BrowserSeek_InitialReleaseAndStart) {
 
   player_.Start();
 
-  // The new player won't be created until the pending data request is
-  // processed.
+  // No data request is issued since there is still one pending.
   EXPECT_EQ(1, demuxer_->num_data_requests());
-  EXPECT_FALSE(GetMediaDecoderJob(false));
+  EXPECT_FALSE(GetMediaCodecBridge(false));
 
-  // A browser seek should be requested.
+  // No browser seek is needed.
   player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForVideo());
-  EXPECT_EQ(1, demuxer_->num_browser_seek_requests());
-  EXPECT_EQ(1, demuxer_->num_data_requests());
+  EXPECT_EQ(0, demuxer_->num_browser_seek_requests());
+  EXPECT_EQ(2, demuxer_->num_data_requests());
 }
 
 TEST_F(MediaSourcePlayerTest, BrowserSeek_MidStreamReleaseAndStart) {
@@ -1445,8 +1482,7 @@ TEST_F(MediaSourcePlayerTest, BrowserSeek_MidStreamReleaseAndStart) {
 
   // Simulate browser seek is done and confirm player requests more data.
   player_.OnDemuxerSeekDone(base::TimeDelta());
-  EXPECT_TRUE(GetMediaDecoderJob(false));
-  EXPECT_EQ(2, demuxer_->num_data_requests());
+  EXPECT_EQ(3, demuxer_->num_data_requests());
   EXPECT_EQ(1, demuxer_->num_seek_requests());
 }
 
@@ -1544,10 +1580,9 @@ TEST_F(MediaSourcePlayerTest, PrerollContinuesAcrossReleaseAndStart) {
       // seen by decoder, this assumption allows preroll continuation
       // verification and prevents multiple in-flight data requests.
       ReleasePlayer();
-      player_.OnDemuxerDataAvailable(data);
-      WaitForAudioDecodeDone();
-      EXPECT_FALSE(GetMediaDecoderJob(true));
-      Resume(true, false);
+      // The decoder is still decoding and will not be immediately released.
+      EXPECT_TRUE(GetMediaCodecBridge(true));
+      Resume(false, false);
     } else {
       player_.OnDemuxerDataAvailable(data);
       EXPECT_TRUE(GetMediaDecoderJob(true)->is_decoding());
@@ -1573,11 +1608,13 @@ TEST_F(MediaSourcePlayerTest, PrerollContinuesAcrossConfigChange) {
   EXPECT_TRUE(IsPrerolling(true));
   EXPECT_EQ(100.0, GetPrerollTimestamp().InMillisecondsF());
 
+  DemuxerConfigs configs = CreateAudioDemuxerConfigs(kCodecVorbis);
+  configs.audio_sampling_rate = 11025;

[wolenetz, 2014/06/04 00:21:46]

nit: Add tests that no actual config change occurs if kConfigChanged AU provides
unchanged config for each of audio and video?

[qinmin, 2014/06/04 19:29:15]

Done.

 
   // In response to data request, simulate that demuxer signals config change by
-  // sending an AU with |kConfigChanged|. Player should reconfigure the
-  // audio decoder job with the supplied configs.
-  DemuxerData data = CreateReadFromDemuxerAckWithConfigChanged(true, 0);
+  // sending an AU with |kConfigChanged|.
+  DemuxerData data = CreateReadFromDemuxerAckWithConfigChanged(
+      true, 0, configs);
   player_.OnDemuxerDataAvailable(data);
   PrerollDecoderToTime(
       true, base::TimeDelta(), base::TimeDelta::FromMilliseconds(100));
@@ -1591,24 +1628,29 @@ TEST_F(MediaSourcePlayerTest, SimultaneousAudioVideoConfigChange) {
   // audio and video jobs is |kConfigChanged|.
   CreateNextTextureAndSetVideoSurface();
   Start(CreateAudioVideoDemuxerConfigs());
-  MediaDecoderJob* first_audio_job = GetMediaDecoderJob(true);
-  MediaDecoderJob* first_video_job = GetMediaDecoderJob(false);
+  player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForAudio(0));
+  player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForVideo());
+  EXPECT_TRUE(GetMediaCodecBridge(true));
+  EXPECT_TRUE(GetMediaCodecBridge(false));
+  WaitForAudioVideoDecodeDone();
 
   // Simulate audio |kConfigChanged| prefetched as standalone access unit.
+  DemuxerConfigs audio_configs = CreateAudioDemuxerConfigs(kCodecVorbis);
+  audio_configs.audio_sampling_rate = 11025;
   player_.OnDemuxerDataAvailable(
-      CreateReadFromDemuxerAckWithConfigChanged(true, 0));
+      CreateReadFromDemuxerAckWithConfigChanged(true, 0, audio_configs));
 
   // Simulate video |kConfigChanged| prefetched as standalone access unit.
   player_.OnDemuxerDataAvailable(
-      CreateReadFromDemuxerAckWithConfigChanged(false, 0));
-  EXPECT_EQ(4, demuxer_->num_data_requests());
-
-  // Both jobs should have been reconfigured by now.
-  // TODO(qinmin): Fix flaky pointer-based MDJ inequality testing.
-  // See http://crbug.com/327839.
-  EXPECT_NE(first_audio_job, GetMediaDecoderJob(true));
-  EXPECT_NE(first_video_job, GetMediaDecoderJob(false));
-  EXPECT_TRUE(GetMediaDecoderJob(true) && GetMediaDecoderJob(false));
+      CreateReadFromDemuxerAckWithConfigChanged(
+          false, 0, CreateVideoDemuxerConfigs(kCodecVP9)));
+  EXPECT_EQ(6, demuxer_->num_data_requests());
+  EXPECT_TRUE(IsDrainingDecoder(true));
+  EXPECT_TRUE(IsDrainingDecoder(false));
+
+  // Waiting for decoder to finish draining.
+  while (IsDrainingDecoder(true) || IsDrainingDecoder(false))
+    message_loop_.RunUntilIdle();
 }
 
 TEST_F(MediaSourcePlayerTest, DemuxerConfigRequestedIfInPrefetchUnit0) {
@@ -1661,7 +1703,8 @@ TEST_F(MediaSourcePlayerTest, BrowserSeek_PrerollAfterBrowserSeek) {
   // Simulate browser seek is done, but to a later time than was requested.
   EXPECT_LT(player_.GetCurrentTime().InMillisecondsF(), 100);
   player_.OnDemuxerSeekDone(base::TimeDelta::FromMilliseconds(100));
-  EXPECT_TRUE(GetMediaDecoderJob(false));
+  // Because next AU is not I-frame, MediaCodecBridge will not be recreated.
+  EXPECT_FALSE(GetMediaCodecBridge(false));
   EXPECT_EQ(100.0, player_.GetCurrentTime().InMillisecondsF());
   EXPECT_EQ(100.0, GetPrerollTimestamp().InMillisecondsF());
   EXPECT_EQ(3, demuxer_->num_data_requests());
@@ -1676,25 +1719,71 @@ TEST_F(MediaSourcePlayerTest, VideoDemuxerConfigChange) {
   // Test that video config change notification results in creating a new
   // video decoder job results without any browser seek.

[wolenetz, 2014/06/04 00:21:47]

nit: fix comment. no new job is created, just a new bridge.

[qinmin, 2014/06/04 19:29:15]

Done.

   StartConfigChange(false, true, 1);
-  EXPECT_TRUE(GetMediaDecoderJob(false));
-  EXPECT_EQ(2, demuxer_->num_data_requests());
+
+  // New video decoder job should have been created and configured, without any

[wolenetz, 2014/06/04 00:21:47]

nit: ditto

[qinmin, 2014/06/04 19:29:15]

Done.

+  // browser seek.
+  EXPECT_TRUE(GetMediaCodecBridge(false));
+  EXPECT_EQ(3, demuxer_->num_data_requests());
   EXPECT_EQ(0, demuxer_->num_seek_requests());
 }
 
-TEST_F(MediaSourcePlayerTest, NewSurfaceAfterChangingConfigs) {
+TEST_F(MediaSourcePlayerTest, DecoderDrainInterruptedBySeek) {
   SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
 
-  // Test that no seek results from a SetVideoSurface() that occurs after
-  // the player processes new demuxer configs. This test may be good to keep
-  // beyond browser seek hack.
-  StartConfigChange(false, false, 1);
-  EXPECT_TRUE(GetMediaDecoderJob(false));
+  // Test if a decoder is being drained while receiving a seek request, draining
+  // is canceled.
+  SendConfigChangeToDecoder(true, false, 0);
+  EXPECT_TRUE(IsDrainingDecoder(true));
+
+  player_.SeekTo(base::TimeDelta::FromMilliseconds(100));
+  WaitForAudioDecodeDone();
+  player_.OnDemuxerSeekDone(kNoTimestamp());

[wolenetz, 2014/06/04 00:21:46]

Just prior to this line, should IsDrainingDecoder(true) be false? Or only
sometime later? If the former, please check that before OnDemuxerSeekDone().

[qinmin, 2014/06/04 19:29:15]

Done. Yes, WaitForAudioDecodeDone() should make IsDrainingDecoder(true) to
become false.

On 2014/06/04 00:21:46, wolenetz wrote:

+
+  EXPECT_EQ(1, demuxer_->num_seek_requests());
+  EXPECT_EQ(4, demuxer_->num_data_requests());
+  EXPECT_FALSE(IsDrainingDecoder(true));
+}
+
+TEST_F(MediaSourcePlayerTest, DecoderDrainInterruptedByRelease) {
+  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
+
+  // Test if a decoder is being drained while receiving a release request,
+  // draining is canceled.
+  SendConfigChangeToDecoder(true, false, 0);
+  EXPECT_TRUE(IsDrainingDecoder(true));
+
+  ReleasePlayer();
+  WaitForAudioDecodeDone();
+  EXPECT_EQ(3, demuxer_->num_data_requests());
+  EXPECT_FALSE(IsDrainingDecoder(true));
+
+  EXPECT_FALSE(GetMediaCodecBridge(true));
+  EXPECT_FALSE(player_.IsPlaying());
+
+  player_.Start();
+  EXPECT_TRUE(player_.IsPlaying());
   EXPECT_EQ(3, demuxer_->num_data_requests());
+}
+
+TEST_F(MediaSourcePlayerTest, DecoderDrainInterruptedBySurfaceChange) {
+  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
+
+  // Test if a video decoder is being drained while surface changes, draining
+  // is canceled.
+  SendConfigChangeToDecoder(false, false, 0);
+  EXPECT_TRUE(IsDrainingDecoder(false));
 
   CreateNextTextureAndSetVideoSurface();
-  EXPECT_TRUE(GetMediaDecoderJob(false));
+  WaitForVideoDecodeDone();
+
+  EXPECT_FALSE(IsDrainingDecoder(false));
+  EXPECT_FALSE(GetMediaCodecBridge(false));
+  EXPECT_TRUE(player_.IsPlaying());
   EXPECT_EQ(3, demuxer_->num_data_requests());
-  EXPECT_EQ(0, demuxer_->num_seek_requests());
+
+  // Finish the browser seek introduced by surface change.
+  player_.OnDemuxerSeekDone(base::TimeDelta());
+  EXPECT_EQ(4, demuxer_->num_data_requests());
 }
 
 TEST_F(MediaSourcePlayerTest,
@@ -1718,7 +1807,9 @@ TEST_F(MediaSourcePlayerTest,
   player_.OnDemuxerDataAvailable(data);
   EXPECT_EQ(1, demuxer_->num_browser_seek_requests());
   player_.OnDemuxerSeekDone(base::TimeDelta());
-  EXPECT_TRUE(GetMediaDecoderJob(false));
+  // After seek is done, prefetch is handled first. MediaCodecBridge is not
+  // created at this moment.
+  EXPECT_FALSE(GetMediaCodecBridge(false));
 
   // A new data request should be sent.
   EXPECT_EQ(3, demuxer_->num_data_requests());
@@ -1743,7 +1834,8 @@ TEST_F(MediaSourcePlayerTest, ReleaseWithOnPrefetchDoneAlreadyPosted) {
   // triggered by starvation to simulate decoder underrun, can trivially
   // post task to run OnPrefetchDone().
   player_.OnDemuxerDataAvailable(
-      CreateReadFromDemuxerAckWithConfigChanged(true, 4));
+      CreateReadFromDemuxerAckWithConfigChanged(
+          true, 4, CreateAudioDemuxerConfigs(kCodecVorbis)));
   EXPECT_TRUE(GetMediaDecoderJob(true)->is_decoding());
 
   // Simulate decoder underrun, so trivial prefetch starts while still decoding.
@@ -1757,11 +1849,12 @@ TEST_F(MediaSourcePlayerTest, ReleaseWithOnPrefetchDoneAlreadyPosted) {
   OnNextTestDecodeCallbackPostTaskToReleasePlayer();
 
   WaitForAudioDecodeDone();
-  EXPECT_TRUE(decoder_callback_hook_executed_);
-  EXPECT_EQ(2, demuxer_->num_data_requests());
 

[wolenetz, 2014/06/04 00:21:46]

Why is |decoder_callback_hook_executed_| no longer expected to be true here?

[qinmin, 2014/06/04 19:29:15]

it should still be true, added the EXPECT_TRUE() back.

On 2014/06/04 00:21:46, wolenetz wrote:

-  // Player should have no decoder job until after Start().
-  Resume(true, false);
+  EXPECT_EQ(3, demuxer_->num_data_requests());
+
+  // Player should not request any new data since the access units haven't
+  // been fully decoded yet.
+  Resume(false, false);
 }
 
 TEST_F(MediaSourcePlayerTest, SeekToThenReleaseThenDemuxerSeekAndDone) {
@@ -1774,11 +1867,13 @@ TEST_F(MediaSourcePlayerTest, SeekToThenReleaseThenDemuxerSeekAndDone) {
   StartAudioDecoderJobAndSeekToWhileDecoding(
       base::TimeDelta::FromMilliseconds(100));
   ReleasePlayer();
+  EXPECT_EQ(0, demuxer_->num_seek_requests());
+  WaitForAudioDecodeDone();
   EXPECT_EQ(1, demuxer_->num_seek_requests());
 
   player_.OnDemuxerSeekDone(kNoTimestamp());
   EXPECT_EQ(100.0, GetPrerollTimestamp().InMillisecondsF());
-  EXPECT_FALSE(GetMediaDecoderJob(true));
+  EXPECT_FALSE(GetMediaCodecBridge(true));
   EXPECT_FALSE(player_.IsPlaying());
 
   // Player should begin prefetch and resume preroll upon Start().
@@ -1802,13 +1897,15 @@ TEST_F(MediaSourcePlayerTest, SeekToThenReleaseThenDemuxerSeekThenStart) {
   StartAudioDecoderJobAndSeekToWhileDecoding(
       base::TimeDelta::FromMilliseconds(100));
   ReleasePlayer();
-  EXPECT_EQ(1, demuxer_->num_seek_requests());
+  EXPECT_EQ(0, demuxer_->num_seek_requests());
 
   // Player should not prefetch upon Start() nor create the decoder job, due to
   // awaiting DemuxerSeekDone.
   EXPECT_EQ(2, demuxer_->num_data_requests());
   Resume(false, false);
 
+  WaitForAudioDecodeDone();
+  EXPECT_EQ(1, demuxer_->num_seek_requests());
   player_.OnDemuxerSeekDone(kNoTimestamp());
   EXPECT_TRUE(GetMediaDecoderJob(true));
   EXPECT_TRUE(IsPrerolling(true));
@@ -1833,7 +1930,7 @@ TEST_F(MediaSourcePlayerTest, SeekToThenDemuxerSeekThenReleaseThenSeekDone) {
   ReleasePlayer();
   player_.OnDemuxerSeekDone(kNoTimestamp());
   EXPECT_FALSE(player_.IsPlaying());
-  EXPECT_FALSE(GetMediaDecoderJob(true));
+  EXPECT_FALSE(GetMediaCodecBridge(true));
   EXPECT_EQ(100.0, GetPrerollTimestamp().InMillisecondsF());
 
   // Player should begin prefetch and resume preroll upon Start().
@@ -1863,7 +1960,7 @@ TEST_F(MediaSourcePlayerTest, SeekToThenReleaseThenStart) {
   Resume(false, false);
 
   player_.OnDemuxerSeekDone(kNoTimestamp());
-  EXPECT_TRUE(GetMediaDecoderJob(true));
+  EXPECT_FALSE(GetMediaCodecBridge(true));
   EXPECT_TRUE(IsPrerolling(true));
   EXPECT_EQ(100.0, GetPrerollTimestamp().InMillisecondsF());
   EXPECT_EQ(3, demuxer_->num_data_requests());
@@ -1875,21 +1972,27 @@ TEST_F(MediaSourcePlayerTest, SeekToThenReleaseThenStart) {
 TEST_F(MediaSourcePlayerTest, ConfigChangedThenReleaseThenStart) {
   SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
 
-  // Test if Release() occurs after |kConfigChanged| detected, and then Start()
-  // is called, then the player does not issue any new data request since it is
-  // still waiting for the previous one.
+  // Test if Release() occurs after |kConfigChanged| is processed, new data
+  // requested of demuxer, and the requested data arrive before the next
+  // Start(), then the player starts to decode the new data without any browser

[wolenetz, 2014/06/04 00:21:46]

nit: s/browser//

[qinmin, 2014/06/04 19:29:15]

Done.

+  // seek.
   StartConfigChange(true, true, 0);
   ReleasePlayer();
 
-  EXPECT_FALSE(GetMediaDecoderJob(true));
+  EXPECT_TRUE(GetMediaCodecBridge(true));
   EXPECT_FALSE(player_.IsPlaying());
-  EXPECT_EQ(2, demuxer_->num_data_requests());
+  EXPECT_EQ(3, demuxer_->num_data_requests());
+  player_.OnDemuxerDataAvailable(
+        CreateReadFromDemuxerAckWithConfigChanged(
+            true, 4, CreateAudioDemuxerConfigs(kCodecVorbis)));
+  WaitForAudioDecodeDone();
+  EXPECT_FALSE(GetMediaCodecBridge(true));
 
-  // Player should resume upon Start().
-  Resume(false, false);
-  EXPECT_TRUE(GetMediaDecoderJob(true));
+  // Player should resume upon Start(), even without further configs supplied.
+  player_.Start();
   EXPECT_TRUE(player_.IsPlaying());
-  EXPECT_EQ(2, demuxer_->num_data_requests());
+  EXPECT_EQ(3, demuxer_->num_data_requests());
+  EXPECT_EQ(0, demuxer_->num_seek_requests());
 }
 
 TEST_F(MediaSourcePlayerTest, BrowserSeek_ThenReleaseThenDemuxerSeekDone) {
@@ -1905,7 +2008,7 @@ TEST_F(MediaSourcePlayerTest, BrowserSeek_ThenReleaseThenDemuxerSeekDone) {
 
   player_.OnDemuxerSeekDone(expected_preroll_timestamp);
   EXPECT_FALSE(player_.IsPlaying());
-  EXPECT_FALSE(GetMediaDecoderJob(false));
+  EXPECT_FALSE(GetMediaCodecBridge(false));
   EXPECT_EQ(expected_preroll_timestamp, GetPrerollTimestamp());
 
   // Player should begin prefetch and resume preroll upon Start().
@@ -1937,7 +2040,8 @@ TEST_F(MediaSourcePlayerTest, BrowserSeek_ThenReleaseThenStart) {
   Resume(false, false);
 
   player_.OnDemuxerSeekDone(expected_preroll_timestamp);
-  EXPECT_TRUE(GetMediaDecoderJob(false));
+  // Prefetch takes place first, and the decoder is not created yet.
+  EXPECT_FALSE(GetMediaCodecBridge(false));
   EXPECT_TRUE(IsPrerolling(false));
   EXPECT_EQ(expected_preroll_timestamp, GetPrerollTimestamp());
   EXPECT_EQ(expected_preroll_timestamp, player_.GetCurrentTime());
@@ -1945,6 +2049,11 @@ TEST_F(MediaSourcePlayerTest, BrowserSeek_ThenReleaseThenStart) {
 
   // No further seek should have been requested since BrowserSeekPlayer().
   EXPECT_EQ(1, demuxer_->num_seek_requests());
+
+  // Decoder will be created once data is received.
+  player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForVideo());
+  while (!GetMediaCodecBridge(false))
+    message_loop_.RunUntilIdle();
 }
 
 // TODO(xhwang): Once we add tests to cover DrmBridge, update this test to
@@ -1958,13 +2067,12 @@ TEST_F(MediaSourcePlayerTest, SurfaceChangeClearedEvenIfMediaCryptoAbsent) {
   // Test that |SURFACE_CHANGE_EVENT_PENDING| is not pending after
   // SetVideoSurface() for a player configured for encrypted video, when the
   // player has not yet received media crypto.
-  DemuxerConfigs configs = CreateVideoDemuxerConfigs();
+  DemuxerConfigs configs = CreateVideoDemuxerConfigs(kCodecVP8);
   configs.is_video_encrypted = true;
 
   player_.OnDemuxerConfigsAvailable(configs);
   CreateNextTextureAndSetVideoSurface();
-  EXPECT_FALSE(IsPendingSurfaceChange());
-  EXPECT_FALSE(GetMediaDecoderJob(false));
+  EXPECT_FALSE(GetMediaCodecBridge(false));
 }
 
 TEST_F(MediaSourcePlayerTest, CurrentTimeUpdatedWhileDecoderStarved) {
@@ -1993,7 +2101,10 @@ TEST_F(MediaSourcePlayerTest, CurrentTimeKeepsIncreasingAfterConfigChange) {
 
   DecodeAudioDataUntilOutputBecomesAvailable();
 
-  DemuxerData data = CreateReadFromDemuxerAckWithConfigChanged(true, 0);
+  DemuxerConfigs configs = CreateAudioDemuxerConfigs(kCodecVorbis);

[wolenetz, 2014/06/04 00:21:46]

nit: Can this 11025 alternate config (and similar for kCodecVP9 alternate
config) be abstracted into helpers and the result be simpler?

[qinmin, 2014/06/04 19:29:15]

ok, changed the CreateAudioDemuxerConfigs() to accept a bool value for low
sampling rate, CreateVideoDemuxerConfigs already accepts a codec param for
video.

On 2014/06/04 00:21:46, wolenetz wrote:

+  configs.audio_sampling_rate = 11025;
+  DemuxerData data = CreateReadFromDemuxerAckWithConfigChanged(
+      true, 0, configs);
   player_.OnDemuxerDataAvailable(data);
   WaitForAudioDecodeDone();
   DecodeAudioDataUntilOutputBecomesAvailable();