Let only seeks reset Android MSE stream playback completion

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 1da6641b83485f9240aca0355057cb990e7de8ca..c4893094992e494a321a29495522b1d5ead5bc1f 100644
--- a/media/base/android/media_source_player_unittest.cc
+++ b/media/base/android/media_source_player_unittest.cc
@@ -280,6 +280,18 @@ class MediaSourcePlayerTest : public testing::Test {
     Start(CreateVideoDemuxerConfigs());
   }
 
+  DemuxerConfigs CreateDemuxerConfigs(bool have_audio, bool have_video) {
+    DCHECK(have_audio || have_video);
+
+    if (have_audio && !have_video)
+      return CreateAudioDemuxerConfigs(kCodecVorbis);
+
+    if (have_video && !have_audio)
+      return CreateVideoDemuxerConfigs();
+
+    return CreateAudioVideoDemuxerConfigs();
+  }
+
   // Starts decoding the data.
   void Start(const DemuxerConfigs& configs) {
     player_.OnDemuxerConfigsAvailable(configs);
@@ -365,11 +377,12 @@ class MediaSourcePlayerTest : public testing::Test {
   }
 
   // Seek, including simulated receipt of |kAborted| read between SeekTo()
-  // and OnDemuxerSeekDone(). Use this helper method only when the player
-  // already has created the decoder job.
-  void SeekPlayer(bool is_audio, const base::TimeDelta& seek_time) {
-    EXPECT_TRUE(GetMediaDecoderJob(is_audio));
-
+  // and OnDemuxerSeekDone() if |abort| is true. Use this helper method only
+  // when the player already has created the decoder job. If |abort| is false,
+  // |is_audio| is ignored. |expected_new_data_requests| is compared to the
+  // actual increase in data request count due to the seek.
+  void SeekPlayer(bool is_audio, const base::TimeDelta& seek_time, bool abort,
+                  int expected_new_data_requests) {
     int original_num_seeks = demuxer_->num_seek_requests();
     int original_num_data_requests = demuxer_->num_data_requests();
 
@@ -377,28 +390,37 @@ class MediaSourcePlayerTest : public testing::Test {
     // Instead behave as if the renderer has asked us to seek.
     player_.SeekTo(seek_time);
 
-    // Verify that the seek does not occur until previously outstanding data
-    // request is satisfied.
-    EXPECT_EQ(original_num_seeks, demuxer_->num_seek_requests());
+    if (abort) {
+      // Verify that the seek does not occur until previously outstanding data
+      // request is satisfied.
+      EXPECT_EQ(original_num_seeks, demuxer_->num_seek_requests());
 
-    // Simulate seeking causes the demuxer to abort the outstanding read caused
-    // by the seek.
-    player_.OnDemuxerDataAvailable(CreateAbortedAck(is_audio));
+      // Simulate seeking causes the demuxer to abort the outstanding read
+      // caused by the seek.
+      player_.OnDemuxerDataAvailable(CreateAbortedAck(is_audio));
+    }
 
-    // Verify that the seek is requested now that the outstanding read is
-    // completed by aborted access unit.
+    // Verify that the seek is requested.
     EXPECT_EQ(original_num_seeks + 1, demuxer_->num_seek_requests());
 
     // Send back the seek done notification. This should trigger the player to
     // call OnReadFromDemuxer() again.
     EXPECT_EQ(original_num_data_requests, demuxer_->num_data_requests());
     player_.OnDemuxerSeekDone(kNoTimestamp());
-    EXPECT_EQ(original_num_data_requests + 1, demuxer_->num_data_requests());
+    EXPECT_EQ(original_num_data_requests + expected_new_data_requests,
+              demuxer_->num_data_requests());
 
     // No other seek should have been requested.
     EXPECT_EQ(original_num_seeks + 1, demuxer_->num_seek_requests());
   }
 
+  // Seek, assuming player has exactly 1 stream configured (corresponding to
+  // |is_audio|), has created the decoder job, and has an outstanding request
+  // for demuxed data.
+  void SeekPlayer(bool is_audio, const base::TimeDelta& seek_time) {
+    SeekPlayer(is_audio, seek_time, true, 1);
+  }
+
   DemuxerData CreateReadFromDemuxerAckWithConfigChanged(bool is_audio,
                                                         int config_unit_index) {
     DemuxerData data;
@@ -528,8 +550,7 @@ class MediaSourcePlayerTest : public testing::Test {
     // new demuxer configs.
     player_.OnDemuxerDataAvailable(
         CreateReadFromDemuxerAckWithConfigChanged(is_audio, config_unit_index));
-    while (GetMediaDecoderJob(is_audio)->is_decoding())
-      message_loop_.RunUntilIdle();
+    WaitForDecodeDone(is_audio, !is_audio, false, false);
 
     expected_num_config_requests++;
     EXPECT_TRUE(player_.IsPlaying());
@@ -553,6 +574,156 @@ class MediaSourcePlayerTest : public testing::Test {
     player_.SetVideoSurface(surface.Pass());
   }
 
+  // Wait for one or both of the jobs to complete decoding, and optionally
+  // spot-check that one or more of the jobs do not resume decoding. Due to
+  // RunUntilIdle()'s large granularity, these spot-checks do not guarantee the
+  // absence of decode resumption. Decoder jobs are assumed to exist for any
+  // stream whose decode completion is awaited or whose lack of decode
+  // resumption is spot-checked.
+  void WaitForDecodeDone(bool wait_for_audio,
+                         bool wait_for_video,
+                         bool spot_check_no_audio_decoding,
+                         bool spot_check_no_video_decoding) {
+    DCHECK(wait_for_audio || wait_for_video);
+    DCHECK(!(wait_for_audio && spot_check_no_audio_decoding));
+    DCHECK(!(wait_for_video && spot_check_no_video_decoding));
+
+    while ((wait_for_audio && GetMediaDecoderJob(true)->is_decoding()) ||
+           (wait_for_video && GetMediaDecoderJob(false)->is_decoding())) {
+      EXPECT_FALSE(spot_check_no_audio_decoding &&
+                   GetMediaDecoderJob(true)->is_decoding());
+      EXPECT_FALSE(spot_check_no_video_decoding &&
+                   GetMediaDecoderJob(false)->is_decoding());
+      message_loop_.RunUntilIdle();
+    }
+
+    EXPECT_FALSE(spot_check_no_audio_decoding &&
+                 GetMediaDecoderJob(true)->is_decoding());
+    EXPECT_FALSE(spot_check_no_video_decoding &&
+                 GetMediaDecoderJob(false)->is_decoding());
+  }
+
+  void WaitForAudioDecodeDone() {
+    WaitForDecodeDone(true, false, false, false);
+  }
+
+  void WaitForVideoDecodeDone() {
+    WaitForDecodeDone(false, true, false, false);
+  }
+
+  void WaitForAudioVideoDecodeDone() {
+    WaitForDecodeDone(true, true, false, false);
+  }
+
+  // If |send_eos| is true, generates EOS for the stream corresponding to
+  // |eos_for_audio|. Verifies that playback completes. If |send_eos| is false,
+  // then it is assumed that caller previously arranged for player to receive
+  // EOS for the stream corresponding to |eos_is_audio|, but the player has not
+  // yet decoded that EOS.
+  void VerifyPlaybackCompletesOnEOSDecode(bool send_eos, bool eos_for_audio) {
+    int original_num_data_requests = demuxer_->num_data_requests();
+    if (send_eos)
+      player_.OnDemuxerDataAvailable(CreateEOSAck(eos_for_audio));
+    EXPECT_FALSE(manager_.playback_completed());
+    message_loop_.Run();
+    EXPECT_TRUE(manager_.playback_completed());
+    EXPECT_EQ(original_num_data_requests, demuxer_->num_data_requests());
+  }
+
+  // Similar to VerifyPlaybackCompletesOnEOSDecode(), this helper also
+  // spot-checks that the stream corresponding to !|eos_is_audio| does not
+  // resume decoding while the EOS for the stream corresponding to
+  // |eos_is_audio|. By example, if |eos_is_audio| is false, then video EOS is
+  // expected to be decoded but audio decoder should not resume decoding.
+  void AV_VerifyPlaybackCompletesOnEOSDecode(bool send_eos, bool eos_is_audio) {
+    int original_num_data_requests = demuxer_->num_data_requests();
+    if (send_eos) {
+      EXPECT_FALSE(GetMediaDecoderJob(!eos_is_audio)->is_decoding());
+      player_.OnDemuxerDataAvailable(CreateEOSAck(eos_is_audio));
+    }
+
+    EXPECT_FALSE(manager_.playback_completed());
+    EXPECT_FALSE(GetMediaDecoderJob(!eos_is_audio)->is_decoding());
+    message_loop_.RunUntilIdle();  // This lets OnPrefetchDone() run if pending.
+    WaitForDecodeDone(eos_is_audio, !eos_is_audio, !eos_is_audio, eos_is_audio);

[acolwell GONE FROM CHROMIUM, 2013/12/11 21:20:22]

This is really confusing looking. Do you really need the spot checks. If a
decoderjob gets called after it signals EOS won't it just DCHECK? If so then I
don't think you actually need to have this spot check logic because the test
will crash if the wrong thing happens. I'd rather rely on that then have the
spot checking here in the tests.

[wolenetz, 2013/12/12 03:34:46]

Yeah, this was confusing because it was more of a black-box approach that
actually found issues with an earlier patch set. With current patch set, yes,
MDJ would just DCHECK (unless a new MDJ were somehow created, resetting MDJ's
output_eos detection flag), so I've simplified this test helper. Done.

+    EXPECT_TRUE(manager_.playback_completed());
+    EXPECT_EQ(original_num_data_requests, demuxer_->num_data_requests());
+  }
+
+  void VerifyCompletedPlaybackResumesOnSeekPlusStart(bool have_audio,
+                                                     bool have_video) {
+    DCHECK(have_audio || have_video);
+
+    EXPECT_TRUE(manager_.playback_completed());
+    int original_num_data_requests = demuxer_->num_data_requests();
+    SeekPlayer(true /* ignored, since not aborting */,

[acolwell GONE FROM CHROMIUM, 2013/12/11 21:20:22]

nit: I think this just makes this unnecessarily confusing. Can't you just put
the following here and revert the SeekPlayer() changes.

player_.SeekTo( base::TimeDelta());
player_.OnDemuxerSeekDone(kNoTimestamp());

I think this would be much easier to understand.

[wolenetz, 2013/12/12 03:34:46]

Yes, done :)

+               base::TimeDelta(), false, 0);
+
+    int data_request_delta = (have_audio ? 1 : 0) + (have_video ? 1 : 0);
+    DemuxerConfigs configs = CreateDemuxerConfigs(have_audio, have_video);
+    Start(configs);
+
+    EXPECT_TRUE((GetMediaDecoderJob(true) != NULL) == have_audio &&

[acolwell GONE FROM CHROMIUM, 2013/12/11 21:20:22]

Shouldn't this type of check be in Start() instead of here?

[wolenetz, 2013/12/12 03:34:46]

Start() calls MSP::Start(), which does not guarantee that jobs are created. For
instance, video decoder job creation could be pending a valid surface, or
audio/video decoder job(s) creation could be pending OnDemuxerSeekDone().
Perhaps a parameterized version of the Start() helper could simplify this; I'll
take a stab at this to see what it might look like. I've also filed
https://crbug.com/327839 to try to get much of this clutter moved into mocks,
and also to get some flaky pointer-based MDJ inequality testing fixed.

+                (GetMediaDecoderJob(false) != NULL) == have_video);
+    EXPECT_EQ(original_num_data_requests + data_request_delta,
+              demuxer_->num_data_requests());

[acolwell GONE FROM CHROMIUM, 2013/12/11 21:20:22]

Can this be moved into start as well?

[wolenetz, 2013/12/12 03:34:46]

Along the lines of similar comment, I'll take a stab at this.

+  }
+
+  // Starts the appropriate decoder jobs according to |have_audio| and
+  // |have_video|. Then starts seek during decode of EOS or non-EOS according to
+  // |eos_audio| and |eos_video|. Simulates seek completion and verifies that
+  // playback never completed.
+  void VerifySeekDuringEOSDecodePreventsPlaybackCompletion(bool have_audio,
+                                                           bool have_video,
+                                                           bool eos_audio,
+                                                           bool eos_video) {
+    DCHECK(have_audio || have_video);
+    DCHECK(have_audio || !eos_audio);
+    DCHECK(have_video || !eos_video);
+
+    int data_request_delta = (have_audio ? 1 : 0) + (have_video ? 1 : 0);
+    DemuxerConfigs configs = CreateDemuxerConfigs(have_audio, have_video);
+
+    if (have_video)
+      CreateNextTextureAndSetVideoSurface();
+    Start(configs);
+    EXPECT_TRUE((GetMediaDecoderJob(true) != NULL) == have_audio &&

[acolwell GONE FROM CHROMIUM, 2013/12/11 21:20:22]

ditto. here and elsewhere.

[wolenetz, 2013/12/12 03:34:46]

Done.

+                (GetMediaDecoderJob(false) != NULL) == have_video);
+    EXPECT_EQ(data_request_delta, demuxer_->num_data_requests());
+
+    if (have_audio)
+      player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForAudio(0));
+    if (have_video)
+      player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForVideo());
+    for (int i = data_request_delta + 1; i <= 2 * data_request_delta; i++) {
+      message_loop_.Run();  // Loop Quit() occurs on each data request.
+      EXPECT_EQ(i, demuxer_->num_data_requests());
+    }
+
+    // Simulate seek while decoding EOS or non-EOS for the appropriate
+    // stream(s).
+    if (eos_audio) {
+      player_.OnDemuxerDataAvailable(CreateEOSAck(true));
+    } else if (have_audio) {
+      player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForAudio(1));
+    }
+    if (eos_video) {
+      player_.OnDemuxerDataAvailable(CreateEOSAck(false));
+    } else if (have_video) {
+      player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForVideo());
+    }
+    EXPECT_TRUE(!have_audio || GetMediaDecoderJob(true)->is_decoding());
+    EXPECT_TRUE(!have_video || GetMediaDecoderJob(false)->is_decoding());
+    player_.SeekTo(base::TimeDelta());
+    EXPECT_EQ(0, demuxer_->num_seek_requests());
+    WaitForDecodeDone(have_audio, have_video, false, false);
+
+    EXPECT_EQ(1, demuxer_->num_seek_requests());
+    player_.OnDemuxerSeekDone(kNoTimestamp());
+    EXPECT_EQ(3 * data_request_delta, demuxer_->num_data_requests());
+    EXPECT_FALSE(manager_.playback_completed());
+  }
+
   base::TimeTicks StartTimeTicks() {
     return player_.start_time_ticks_;
   }
@@ -901,8 +1072,7 @@ TEST_F(MediaSourcePlayerTest, StartTimeTicksResetAfterDecoderUnderruns) {
   // Send new data to the decoder so it can finish the currently
   // pending decode.
   player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForAudio(3));
-  while (GetMediaDecoderJob(true)->is_decoding())
-    message_loop_.RunUntilIdle();
+  WaitForAudioDecodeDone();
 
   // Verify the start time ticks is cleared at this point because the
   // player is prefetching.
@@ -917,102 +1087,243 @@ TEST_F(MediaSourcePlayerTest, StartTimeTicksResetAfterDecoderUnderruns) {
   EXPECT_LE(100.0, (current - previous).InMillisecondsF());
 }
 
-TEST_F(MediaSourcePlayerTest, NoRequestForDataAfterInputEOS) {
+TEST_F(MediaSourcePlayerTest, V_SecondAccessUnitIsEOSAndResumePlayAfterSeek) {
   SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
 
-  // Test MediaSourcePlayer will not request for new data after input EOS is
-  // reached.
+  // Test MediaSourcePlayer can replay video after input EOS is reached.
   CreateNextTextureAndSetVideoSurface();
   StartVideoDecoderJob();
+
   // Player should not seek the demuxer on setting initial surface.
   EXPECT_EQ(0, demuxer_->num_seek_requests());
 
-  EXPECT_EQ(1, demuxer_->num_data_requests());
   // Send the first input chunk.
   player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForVideo());
   message_loop_.Run();
+
+  VerifyPlaybackCompletesOnEOSDecode(true, false);
+  VerifyCompletedPlaybackResumesOnSeekPlusStart(false, true);
+}
+
+TEST_F(MediaSourcePlayerTest, A_FirstAccessUnitIsEOSAndResumePlayAfterSeek) {
+  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
+
+  // Test decode of audio EOS buffer without any prior decode. See also
+  // http://b/11696552.
+  // Also tests that seeking+Start() after completing audio playback resumes
+  // playback.
+  Start(CreateAudioDemuxerConfigs(kCodecAAC));
+  EXPECT_TRUE(GetMediaDecoderJob(true));
+
+  VerifyPlaybackCompletesOnEOSDecode(true, true);
+  VerifyCompletedPlaybackResumesOnSeekPlusStart(true, false);
+}
+
+TEST_F(MediaSourcePlayerTest, V_FirstAccessUnitAfterSeekIsEOS) {
+  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
+
+  // Test decode of video EOS buffer, just after seeking, without any prior
+  // decode (other than the simulated |kAborted| resulting from the seek
+  // process.)
+  CreateNextTextureAndSetVideoSurface();
+  StartVideoDecoderJob();
+  EXPECT_TRUE(GetMediaDecoderJob(false));

[acolwell GONE FROM CHROMIUM, 2013/12/11 21:20:22]

ditto

[wolenetz, 2013/12/12 03:34:46]

Done.

+  SeekPlayer(false, base::TimeDelta());
+  VerifyPlaybackCompletesOnEOSDecode(true, false);
+}
+
+TEST_F(MediaSourcePlayerTest, A_FirstAccessUnitAfterSeekIsEOS) {
+  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
+
+  // Test decode of audio EOS buffer, just after seeking, without any prior
+  // decode (other than the simulated |kAborted| resulting from the seek
+  // process.) See also http://b/11696552.
+  Start(CreateAudioDemuxerConfigs(kCodecAAC));
+  EXPECT_TRUE(GetMediaDecoderJob(true));
+  SeekPlayer(true, base::TimeDelta());
+  VerifyPlaybackCompletesOnEOSDecode(true, true);
+}
+
+TEST_F(MediaSourcePlayerTest, AV_PlaybackCompletionAcrossConfigChange) {
+  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
+
+  // Test that if one stream (audio) has completed decode of EOS and the other
+  // stream (video) processes config change, that subsequent video EOS completes
+  // A/V playback.
+  // Also tests that seeking+Start() after completing playback resumes playback.
+  Start(CreateAudioVideoDemuxerConfigs());
+  CreateNextTextureAndSetVideoSurface();
+  EXPECT_TRUE(GetMediaDecoderJob(true) && GetMediaDecoderJob(false));

[acolwell GONE FROM CHROMIUM, 2013/12/11 21:20:22]

ditto.

[wolenetz, 2013/12/12 03:34:46]

Done.

+  player_.OnDemuxerDataAvailable(CreateEOSAck(true));  // Audio EOS
+  EXPECT_EQ(0, demuxer_->num_config_requests());
+  player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckWithConfigChanged(
+      false, 0));  // Video |kConfigChanged| as first unit.
+
+  WaitForAudioVideoDecodeDone();
+
+  EXPECT_EQ(1, demuxer_->num_config_requests());
   EXPECT_EQ(2, demuxer_->num_data_requests());
+  player_.OnDemuxerConfigsAvailable(CreateAudioVideoDemuxerConfigs());
+  EXPECT_EQ(3, demuxer_->num_data_requests());
 
-  // Send EOS.
-  player_.OnDemuxerDataAvailable(CreateEOSAck(false));
-  message_loop_.Run();
-  // No more request for data should be made.
+  // At no time after completing audio EOS decode, above, should the
+  // audio decoder job resume decoding. Send and decode video EOS. Spot-check
+  // that audio decode doesn't resume.
+  AV_VerifyPlaybackCompletesOnEOSDecode(true, false);
+  VerifyCompletedPlaybackResumesOnSeekPlusStart(true, true);
+}
+
+TEST_F(MediaSourcePlayerTest, VA_PlaybackCompletionAcrossConfigChange) {
+  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
+
+  // Test that if one stream (video) has completed decode of EOS and the other
+  // stream (audio) processes config change, that subsequent audio EOS completes
+  // A/V playback.
+  // Also tests that seeking+Start() after completing playback resumes playback.
+  Start(CreateAudioVideoDemuxerConfigs());
+  CreateNextTextureAndSetVideoSurface();
+  EXPECT_TRUE(GetMediaDecoderJob(true) && GetMediaDecoderJob(false));

[acolwell GONE FROM CHROMIUM, 2013/12/11 21:20:22]

ditto. here and elsewhere

[wolenetz, 2013/12/12 03:34:46]

Done.

   EXPECT_EQ(2, demuxer_->num_data_requests());
+  player_.OnDemuxerDataAvailable(CreateEOSAck(false));  // Video EOS
+  EXPECT_EQ(0, demuxer_->num_config_requests());
+  player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckWithConfigChanged(
+      true, 0));  // Audio |kConfigChanged| as first unit.
 
-  // Reconfirm no seek request has occurred.
-  EXPECT_EQ(0, demuxer_->num_seek_requests());
+  WaitForAudioVideoDecodeDone();
+
+  // TODO(wolenetz/qinmin): Prevent redundant demuxer config request and change
+  // expectation to 1 here. See http://crbug.com/325528.
+  EXPECT_EQ(2, demuxer_->num_config_requests());
+  EXPECT_EQ(2, demuxer_->num_data_requests());
+  player_.OnDemuxerConfigsAvailable(CreateAudioVideoDemuxerConfigs());
+  EXPECT_EQ(3, demuxer_->num_data_requests());
+
+  // At no time after completing video EOS decode, above, should the
+  // video decoder job resume decoding. Send and decode audio EOS. Spot-check
+  // that video decode doesn't resume.
+  AV_VerifyPlaybackCompletesOnEOSDecode(true, true);
+  VerifyCompletedPlaybackResumesOnSeekPlusStart(true, true);
 }
 
-TEST_F(MediaSourcePlayerTest, ReplayAfterInputEOS) {
+TEST_F(MediaSourcePlayerTest, AV_NoPrefetchForFinishedVideoOnAudioStarvation) {
   SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
 
-  // Test MediaSourcePlayer can replay after input EOS is
-  // reached.
+  // Test that if one stream (video) has completed decode of EOS, prefetch
+  // resulting from player starvation occurs only for the other stream (audio),
+  // and responding to that prefetch with EOS completes A/V playback, even if
+  // another starvation occurs during the latter EOS's decode.
+  Start(CreateAudioVideoDemuxerConfigs());
   CreateNextTextureAndSetVideoSurface();
-  StartVideoDecoderJob();
+  EXPECT_TRUE(GetMediaDecoderJob(true) && GetMediaDecoderJob(false));
+  EXPECT_EQ(2, demuxer_->num_data_requests());
+  player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForAudio(0));
+  player_.OnDemuxerDataAvailable(CreateEOSAck(false));  // Video EOS
 
-  // Player should not seek the demuxer on setting initial surface.
-  EXPECT_EQ(0, demuxer_->num_seek_requests());
+  // Wait until video EOS is processed and more data (assumed to be audio) is
+  // requested.
+  while (demuxer_->num_data_requests() < 3)
+    message_loop_.RunUntilIdle();
+  WaitForVideoDecodeDone();
 
-  EXPECT_EQ(1, demuxer_->num_data_requests());
-  // Send the first input chunk.
+  // Simulate decoder underrun to trigger prefetch while still decoding audio.
+  EXPECT_EQ(3, demuxer_->num_data_requests());
+  player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForAudio(1));
+  EXPECT_TRUE(GetMediaDecoderJob(true)->is_decoding() &&
+              !GetMediaDecoderJob(false)->is_decoding());
+  TriggerPlayerStarvation();
+
+  // Complete the audio decode that was in progress when simulated player
+  // starvation was triggered. At no time after completing video EOS decode,
+  // above, should the video decoder job resume decoding.
+  WaitForDecodeDone(true, false, false, true);
+  EXPECT_EQ(4, demuxer_->num_data_requests());
+
+  player_.OnDemuxerDataAvailable(CreateEOSAck(true));  // Audio EOS
+  EXPECT_FALSE(GetMediaDecoderJob(false)->is_decoding());
+  EXPECT_TRUE(GetMediaDecoderJob(true)->is_decoding());
+
+  // Simulate another decoder underrun to trigger prefetch while decoding EOS.
+  TriggerPlayerStarvation();
+
+  AV_VerifyPlaybackCompletesOnEOSDecode(false, true);
+}
+
+TEST_F(MediaSourcePlayerTest, V_StarvationDuringEOSDecode) {
+  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
+
+  // Test that video-only playback completes without further data requested when
+  // starvation occurs during EOS decode.
+  CreateNextTextureAndSetVideoSurface();
+  StartVideoDecoderJob();
   player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForVideo());
   message_loop_.Run();
   EXPECT_EQ(2, demuxer_->num_data_requests());
 
-  // Send EOS.
-  player_.OnDemuxerDataAvailable(CreateEOSAck(false));
+  // Simulate decoder underrun to trigger prefetch while decoding EOS.
+  player_.OnDemuxerDataAvailable(CreateEOSAck(false));  // Video EOS
+  EXPECT_TRUE(GetMediaDecoderJob(false)->is_decoding());
+  TriggerPlayerStarvation();
+  VerifyPlaybackCompletesOnEOSDecode(false, false);
+}
+
+TEST_F(MediaSourcePlayerTest, A_StarvationDuringEOSDecode) {
+  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
+
+  // Test that audio-only playback completes without further data requested when
+  // starvation occurs during EOS decode.
+  StartAudioDecoderJob();
+  EXPECT_TRUE(GetMediaDecoderJob(true));
+  player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForAudio(0));
   message_loop_.Run();
-  // No more request for data should be made.
   EXPECT_EQ(2, demuxer_->num_data_requests());
 
-  // Initiate a seek. Skip requesting element seek of renderer.
-  // Instead behave as if the renderer has asked us to seek.
-  player_.SeekTo(base::TimeDelta());
-  StartVideoDecoderJob();
-  EXPECT_EQ(1, demuxer_->num_seek_requests());
-  player_.OnDemuxerSeekDone(kNoTimestamp());
-  // Seek/Play after EOS should request more data.
-  EXPECT_EQ(3, demuxer_->num_data_requests());
+  // Simulate decoder underrun to trigger prefetch while decoding EOS.
+  player_.OnDemuxerDataAvailable(CreateEOSAck(true));  // Audio EOS
+  EXPECT_TRUE(GetMediaDecoderJob(true)->is_decoding());
+  TriggerPlayerStarvation();
+  VerifyPlaybackCompletesOnEOSDecode(false, true);
+}
 
-  // Reconfirm only 1 seek request has occurred.
-  EXPECT_EQ(1, demuxer_->num_seek_requests());
+TEST_F(MediaSourcePlayerTest, AV_SeekDuringEOSDecodePreventsCompletion) {
+  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
+
+  // Test that seek supercedes audio+video playback completion on simultaneous
+  // audio and video EOS decode, if SeekTo() occurs during these EOS decodes.
+  VerifySeekDuringEOSDecodePreventsPlaybackCompletion(true, true, true, true);
 }
 
-TEST_F(MediaSourcePlayerTest, FirstDataIsEOS) {
+TEST_F(MediaSourcePlayerTest, AV_SeekDuringAudioEOSDecodePreventsCompletion) {
   SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
 
-  // Test decode of EOS buffer without any prior decode. See also
-  // http://b/11696552.
-  Start(CreateAudioDemuxerConfigs(kCodecAAC));
-  EXPECT_TRUE(GetMediaDecoderJob(true));
+  // Test that seek supercedes audio+video playback completion on simultaneous
+  // audio EOS and video non-EOS decode, if SeekTo() occurs during these
+  // decodes.
+  VerifySeekDuringEOSDecodePreventsPlaybackCompletion(true, true, true, false);
+}
 
-  EXPECT_EQ(1, demuxer_->num_data_requests());
-  player_.OnDemuxerDataAvailable(CreateEOSAck(true));
-  EXPECT_FALSE(manager_.playback_completed());
+TEST_F(MediaSourcePlayerTest, AV_SeekDuringVideoEOSDecodePreventsCompletion) {
+  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
 
-  message_loop_.Run();
-  EXPECT_TRUE(manager_.playback_completed());
-  EXPECT_EQ(1, demuxer_->num_data_requests());
+  // Test that seek supercedes audio+video playback completion on simultaneous
+  // audio non-EOS and video EOS decode, if SeekTo() occurs during these
+  // decodes.
+  VerifySeekDuringEOSDecodePreventsPlaybackCompletion(true, true, false, true);
 }
 
-TEST_F(MediaSourcePlayerTest, FirstDataAfterSeekIsEOS) {
+TEST_F(MediaSourcePlayerTest, V_SeekDuringEOSDecodePreventsCompletion) {
   SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
 
-  // Test decode of EOS buffer, just after seeking, without any prior decode
-  // (other than the simulated |kAborted| resulting from the seek process.)
-  // See also http://b/11696552.
-  Start(CreateAudioDemuxerConfigs(kCodecAAC));
-  EXPECT_TRUE(GetMediaDecoderJob(true));
+  // Test that seek supercedes video-only playback completion on EOS decode, if
+  // SeekTo() occurs during EOS decode.
+  VerifySeekDuringEOSDecodePreventsPlaybackCompletion(false, true, false, true);
+}
 
-  SeekPlayer(true, base::TimeDelta());
-  EXPECT_EQ(2, demuxer_->num_data_requests());
-  player_.OnDemuxerDataAvailable(CreateEOSAck(true));
-  EXPECT_FALSE(manager_.playback_completed());
+TEST_F(MediaSourcePlayerTest, A_SeekDuringEOSDecodePreventsCompletion) {
+  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
 
-  message_loop_.Run();
-  EXPECT_TRUE(manager_.playback_completed());
-  EXPECT_EQ(2, demuxer_->num_data_requests());
+  // Test that seek supercedes audio-only playback completion on EOS decode, if
+  // SeekTo() occurs during EOS decode.
+  VerifySeekDuringEOSDecodePreventsPlaybackCompletion(true, false, true, false);
 }
 
 TEST_F(MediaSourcePlayerTest, NoRequestForDataAfterAbort) {
@@ -1027,9 +1338,8 @@ TEST_F(MediaSourcePlayerTest, NoRequestForDataAfterAbort) {
   player_.OnDemuxerDataAvailable(CreateAbortedAck(true));
 
   EXPECT_TRUE(GetMediaDecoderJob(true)->is_decoding());
-  // Wait for the decoder job to finish decoding.
-  while (GetMediaDecoderJob(true)->is_decoding())
-    message_loop_.RunUntilIdle();
+
+  WaitForAudioDecodeDone();
 
   // No request will be sent for new data.
   EXPECT_EQ(1, demuxer_->num_data_requests());
@@ -1660,8 +1970,7 @@ TEST_F(MediaSourcePlayerTest, SeekToThenDemuxerSeekThenReleaseThenSeekDone) {
   // player will resume correct post-seek preroll upon Start().
   StartAudioDecoderJobAndSeekToWhileDecoding(
       base::TimeDelta::FromMilliseconds(100));
-  while (GetMediaDecoderJob(true)->is_decoding())
-    message_loop_.RunUntilIdle();
+  WaitForAudioDecodeDone();
   EXPECT_EQ(1, demuxer_->num_seek_requests());
 
   ReleasePlayer();
@@ -1691,8 +2000,7 @@ TEST_F(MediaSourcePlayerTest, SeekToThenReleaseThenStart) {
   // post-seek preroll after) OnDemuxerSeekDone().
   StartAudioDecoderJobAndSeekToWhileDecoding(
       base::TimeDelta::FromMilliseconds(100));
-  while (GetMediaDecoderJob(true)->is_decoding())
-    message_loop_.RunUntilIdle();
+  WaitForAudioDecodeDone();
   EXPECT_EQ(1, demuxer_->num_seek_requests());
 
   ReleasePlayer();