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 f12cb1a86a1311975deee6fb209c87aad660e399..def321c21cdd7edfc4a2efee403fe76b223ec753 100644
--- a/media/base/android/media_source_player_unittest.cc
+++ b/media/base/android/media_source_player_unittest.cc
@@ -1013,6 +1013,91 @@ TEST_F(MediaSourcePlayerTest, FirstDataAfterSeekIsEOS) {
   EXPECT_EQ(2, demuxer_->num_data_requests());
 }
 
+TEST_F(MediaSourcePlayerTest, 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.
+  Start(CreateAudioVideoDemuxerConfigs());
+  CreateNextTextureAndSetVideoSurface();
+  EXPECT_TRUE(GetMediaDecoderJob(true) && GetMediaDecoderJob(false));
+  EXPECT_EQ(2, demuxer_->num_data_requests());
+  player_.OnDemuxerDataAvailable(CreateEOSAck(true));  // Audio EOS
+  EXPECT_EQ(0, demuxer_->num_config_requests());
+  player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckWithConfigChanged(
+      false, 0));  // Video |kConfigChanged| as first unit.
+
+  while (GetMediaDecoderJob(true)->is_decoding() ||
+         GetMediaDecoderJob(false)->is_decoding()) {
+    message_loop_.RunUntilIdle();
+  }
+
+  EXPECT_EQ(1, 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 audio EOS decode, above, should the
+  // audio decoder job resume decoding (except after a seek).

[acolwell GONE FROM CHROMIUM, 2013/11/25 21:43:59]

nit: The "(except after a seek)" comment here seems out of place since we don't
actually issue a seek in this test. I think it should be removed.?

[wolenetz, 2013/12/04 00:13:29]

Done.

+  EXPECT_FALSE(GetMediaDecoderJob(true)->is_decoding());
+  player_.OnDemuxerDataAvailable(CreateEOSAck(false));  // Video EOS
+  EXPECT_FALSE(GetMediaDecoderJob(true)->is_decoding());
+
+  // Decode the video EOS. Spot-check that audio decode doesn't resume.
+  EXPECT_FALSE(manager_.playback_completed());
+  do {
+    message_loop_.RunUntilIdle();
+    EXPECT_FALSE(GetMediaDecoderJob(true)->is_decoding());
+  } while (GetMediaDecoderJob(false)->is_decoding());
+
+  EXPECT_TRUE(manager_.playback_completed());
+}
+
+TEST_F(MediaSourcePlayerTest, NoPrefetchForAlreadyFinishedStreamOnStarvation) {
+  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
+
+  // 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.
+  Start(CreateAudioVideoDemuxerConfigs());
+  CreateNextTextureAndSetVideoSurface();
+  EXPECT_TRUE(GetMediaDecoderJob(true) && GetMediaDecoderJob(false));
+  EXPECT_EQ(2, demuxer_->num_data_requests());
+  player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForAudio(0));
+  player_.OnDemuxerDataAvailable(CreateEOSAck(false));  // Video EOS
+
+  // Wait until video EOS is processed and more data (assumed to be audio) is
+  // requested.
+  while (demuxer_->num_data_requests() < 3 ||
+         GetMediaDecoderJob(false)->is_decoding()) {
+    message_loop_.RunUntilIdle();
+  }
+
+  // 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 (except after a seek).

[acolwell GONE FROM CHROMIUM, 2013/11/25 21:43:59]

ditto

[wolenetz, 2013/12/04 00:13:29]

Done.

+  while (GetMediaDecoderJob(true)->is_decoding()) {
+    EXPECT_FALSE(GetMediaDecoderJob(false)->is_decoding());
+    message_loop_.RunUntilIdle();
+  }
+  EXPECT_FALSE(GetMediaDecoderJob(false)->is_decoding());
+  EXPECT_EQ(4, demuxer_->num_data_requests());
+
+  player_.OnDemuxerDataAvailable(CreateEOSAck(true));  // Audio EOS
+  EXPECT_FALSE(GetMediaDecoderJob(false)->is_decoding());
+  EXPECT_FALSE(manager_.playback_completed());
+  message_loop_.Run();
+  EXPECT_TRUE(manager_.playback_completed());
+}
+

[acolwell GONE FROM CHROMIUM, 2013/11/25 21:43:59]

Are audio-only and video-only playback_completed cases covered by other tests?

[wolenetz, 2013/12/04 00:13:29]

Existing tests had partial coverage:
1) ReplayInputAfterEOS (renamed to
SecondVideoDataIsEOSAndSeekPlusStartResumesPlay and added playback_completed
checks and refactored to use SeekPlayer()): video-only EOS completes playback +
seek+start resumes play. I also dropped the test that was already a strict
subset of this, called "NoRequestForDataAfterInputEOS"
2) First(Audio)DataIsEOS(AndSeekPlusStartResumesPlay): parts in parentheses
added to this audio-only test
3) First(Audio)DataAfterSeekIsEOS: audio-only test

I've also added cases:
1) FirstVideoDataAfterSeekIsEOS: video-only test
2) One case each for {audio,video}-only to verify edge-case behavior when
starvation occurs during EOS decode. The A+V test already exists as
(AV_)NoPrefetchForFinishedVideoOnAudioStarvation.
(StarvationDuring{Audio,Video}EOSDecode)
3) An alternative case with A/V streams swapped, similar to
PlaybackCompletionAcrossConfigChange
4) Added "SeekPlusStartResumesPlay" verification to end of
{AV_,VA_}PlaybackCompletionAcrossConfigChange tests.

Note that a helper to consolidate the very common code across
{AV_,VA_}PlaybackCompletionAcrossConfigChange is not yet included, because one
of the two tests currently exposes another bug (http://crbug.com/325528) and is
adjusted to not fail until that bug is fixed.

 TEST_F(MediaSourcePlayerTest, NoRequestForDataAfterAbort) {
   SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();