Let only seeks reset Android MSE stream playback completion

media/base/android/media_source_player_unittest.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/stringprintf.h"
 #include "media/base/android/media_codec_bridge.h"
@@ skipping 995 matching lines @@
   SeekPlayer(true, base::TimeDelta());
   EXPECT_EQ(2, demuxer_->num_data_requests());
   player_.OnDemuxerDataAvailable(CreateEOSAck(true));
   EXPECT_FALSE(manager_.playback_completed());
 
   message_loop_.Run();
   EXPECT_TRUE(manager_.playback_completed());
   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();
 
   // Test that the decoder will not request new data after receiving an aborted
   // access unit.
   StartAudioDecoderJob();
   EXPECT_EQ(1, demuxer_->num_data_requests());
 
   // Send an aborted access unit.
   player_.OnDemuxerDataAvailable(CreateAbortedAck(true));
@@ skipping 874 matching lines @@
 
   std::vector<std::string> codec_avc(1, "avc1");
   EXPECT_FALSE(IsTypeSupported(invalid_uuid, "L3", kVideoMp4, codec_avc));
   EXPECT_FALSE(IsTypeSupported(invalid_uuid, "L1", kVideoMp4, codec_avc));
 }
 
 // TODO(xhwang): Are these IsTypeSupported tests device specific?
 // TODO(xhwang): Add more IsTypeSupported tests.
 
 }  // namespace media