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 262 matching lines @@
     configs.video_codec = kCodecVP8;
     configs.video_size = gfx::Size(320, 240);
     configs.is_video_encrypted = false;
     return configs;
   }
 
   void StartVideoDecoderJob() {
     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);
     player_.Start();
     EXPECT_TRUE(player_.IsPlaying());
   }
 
   AccessUnit CreateAccessUnitWithData(bool is_audio, int audio_packet_id) {
     AccessUnit unit;
 
@@ skipping 65 matching lines @@
     EXPECT_FALSE(GetMediaDecoderJob(true)->is_decoding());
     player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForAudio(0));
     EXPECT_TRUE(GetMediaDecoderJob(true)->is_decoding());
     player_.SeekTo(seek_time);
     EXPECT_EQ(0.0, GetPrerollTimestamp().InMillisecondsF());
     EXPECT_EQ(1, demuxer_->num_data_requests());
     EXPECT_EQ(0, demuxer_->num_seek_requests());
   }
 
   // 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();
 
     // Initiate a seek. Skip the round-trip of requesting seek from renderer.
     // 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;
     data.type = is_audio ? DemuxerStream::AUDIO : DemuxerStream::VIDEO;
     data.access_units.resize(config_unit_index + 1);
 
     for (int i = 0; i < config_unit_index; ++i)
       data.access_units[i] = CreateAccessUnitWithData(is_audio, i);
 
     data.access_units[config_unit_index].status = DemuxerStream::kConfigChanged;
@@ skipping 60 matching lines @@
     EXPECT_FALSE(GetMediaDecoderJob(false));
     EXPECT_TRUE(player_.IsPlaying());
 
     // Only one browser seek should have been initiated, and no further data
     // should have been requested.
     expected_num_seek_requests++;
     expected_num_browser_seek_requests++;
     EXPECT_EQ(expected_num_seek_requests, demuxer_->num_seek_requests());
     EXPECT_EQ(expected_num_browser_seek_requests,
               demuxer_->num_browser_seek_requests());
     EXPECT_EQ(expected_num_data_requests, demuxer_->num_seek_requests());

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

Oops. This typo (num_*seek?!*_requests) is fixed in next patch set.

   }
 
   // Creates a new decoder job and feeds it data ending with a |kConfigChanged|
   // access unit. If |config_unit_in_prefetch| is true, sends feeds the config
   // change AU in response to the job's first read request (prefetch). If
   // 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,
@@ skipping 28 matching lines @@
       EXPECT_EQ(expected_num_data_requests, demuxer_->num_data_requests());
       EXPECT_EQ(expected_num_config_requests, demuxer_->num_config_requests());
     }
 
     // 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));
-    while (GetMediaDecoderJob(is_audio)->is_decoding())
-      message_loop_.RunUntilIdle();
+    WaitForDecodeDone(is_audio, !is_audio, false, false);
 
     expected_num_config_requests++;
     EXPECT_TRUE(player_.IsPlaying());
     EXPECT_TRUE(GetMediaDecoderJob(is_audio));
     EXPECT_EQ(expected_num_data_requests, demuxer_->num_data_requests());
     EXPECT_EQ(expected_num_config_requests, demuxer_->num_config_requests());
   }
 
   void CreateNextTextureAndSetVideoSurface() {
     gfx::SurfaceTexture* surface_texture;
     if (surface_texture_a_is_next_) {
       surface_texture_a_ = new gfx::SurfaceTexture(next_texture_id_++);
       surface_texture = surface_texture_a_.get();
     } else {
       surface_texture_b_ = new gfx::SurfaceTexture(next_texture_id_++);
       surface_texture = surface_texture_b_.get();
     }
 
     surface_texture_a_is_next_ = !surface_texture_a_is_next_;
     gfx::ScopedJavaSurface surface = gfx::ScopedJavaSurface(surface_texture);
     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_;
   }
 
   bool IsTypeSupported(const std::vector<uint8>& scheme_uuid,
                        const std::string& security_level,
                        const std::string& container,
                        const std::vector<std::string>& codecs) {
     return MediaSourcePlayer::IsTypeSupported(
         scheme_uuid, security_level, container, codecs);
@@ skipping 73 matching lines @@
   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.
 }
 
 TEST_F(MediaSourcePlayerTest, StartVideoCodecWithInvalidSurface) {
   SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
 
   // Test video decoder job will be created when surface is valid.

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

This comment is incorrect. s/will be/will not be/, and s/valid/invalid/ will be
included in next patch set.

   scoped_refptr<gfx::SurfaceTexture> surface_texture(
       new gfx::SurfaceTexture(0));
   gfx::ScopedJavaSurface surface(surface_texture.get());
   StartVideoDecoderJob();
   // Video decoder job will not be created until surface is available.
   EXPECT_FALSE(GetMediaDecoderJob(false));
   EXPECT_EQ(0, demuxer_->num_data_requests());
 
   // Release the surface texture.
   surface_texture = NULL;
@@ skipping 190 matching lines @@
 
   // Player should not seek the demuxer on setting initial surface.
   EXPECT_EQ(0, demuxer_->num_seek_requests());
 
   MediaDecoderJob* audio_decoder_job = GetMediaDecoderJob(true);
   MediaDecoderJob* video_decoder_job = GetMediaDecoderJob(false);
   EXPECT_EQ(2, demuxer_->num_data_requests());
   EXPECT_FALSE(audio_decoder_job->is_decoding());
   EXPECT_FALSE(video_decoder_job->is_decoding());
 
   // Sending video data to player, audio decoder should not start.

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

This comment is incorrect. s/audio/video/ is included in next patch set.

   player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForVideo());
   EXPECT_FALSE(video_decoder_job->is_decoding());
 
   // Sending audio data to player, both decoders should start now.
   player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForAudio(0));
   EXPECT_TRUE(audio_decoder_job->is_decoding());
   EXPECT_TRUE(video_decoder_job->is_decoding());
 
   // Reconfirm no seek occurred.
   EXPECT_EQ(0, demuxer_->num_seek_requests());
@@ skipping 23 matching lines @@
   // Let the decoder timeout and execute the OnDecoderStarved() callback.
   base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(100));
 
   EXPECT_TRUE(GetMediaDecoderJob(true)->is_decoding());
   EXPECT_TRUE(StartTimeTicks() != base::TimeTicks());
   message_loop_.RunUntilIdle();
 
   // 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.
   EXPECT_TRUE(StartTimeTicks() == base::TimeTicks());
 
   // 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());
 
   base::TimeTicks current = StartTimeTicks();
   EXPECT_LE(100.0, (current - previous).InMillisecondsF());
 }
 
-TEST_F(MediaSourcePlayerTest, NoRequestForDataAfterInputEOS) {
-  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
-
-  // Test MediaSourcePlayer will not request for new data after input EOS is
-  // reached.
+TEST_F(MediaSourcePlayerTest, V_SecondAccessUnitIsEOSAndResumePlayAfterSeek) {
+  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
+
+  // 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();
-  EXPECT_EQ(2, demuxer_->num_data_requests());
-
-  // Send EOS.
-  player_.OnDemuxerDataAvailable(CreateEOSAck(false));
-  message_loop_.Run();
-  // No more request for data should be made.
-  EXPECT_EQ(2, demuxer_->num_data_requests());
-
-  // Reconfirm no seek request has occurred.
-  EXPECT_EQ(0, demuxer_->num_seek_requests());
-}
-
-TEST_F(MediaSourcePlayerTest, ReplayAfterInputEOS) {
-  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
-
-  // Test MediaSourcePlayer can replay after input EOS is
-  // reached.
+
+  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();
-
-  // 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.
+  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());
+
+  // 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.
+
+  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, AV_NoPrefetchForFinishedVideoOnAudioStarvation) {
+  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, even if
+  // another starvation occurs during the latter EOS's decode.
+  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)
+    message_loop_.RunUntilIdle();
+  WaitForVideoDecodeDone();
+
+  // 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());
-
-  // Reconfirm only 1 seek request has occurred.
-  EXPECT_EQ(1, demuxer_->num_seek_requests());
-}
-
-TEST_F(MediaSourcePlayerTest, FirstDataIsEOS) {
-  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));
-
-  EXPECT_EQ(1, demuxer_->num_data_requests());
-  player_.OnDemuxerDataAvailable(CreateEOSAck(true));
-  EXPECT_FALSE(manager_.playback_completed());
-
-  message_loop_.Run();
-  EXPECT_TRUE(manager_.playback_completed());
-  EXPECT_EQ(1, demuxer_->num_data_requests());
-}
-
-TEST_F(MediaSourcePlayerTest, FirstDataAfterSeekIsEOS) {
-  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));
-
-  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());
+  EXPECT_EQ(2, 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);
+}
+
+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, AV_SeekDuringAudioEOSDecodePreventsCompletion) {
+  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
+
+  // 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);
+}
+
+TEST_F(MediaSourcePlayerTest, AV_SeekDuringVideoEOSDecodePreventsCompletion) {
+  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
+
+  // 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, V_SeekDuringEOSDecodePreventsCompletion) {
+  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
+
+  // Test that seek supercedes video-only playback completion on EOS decode, if
+  // SeekTo() occurs during EOS decode.
+  VerifySeekDuringEOSDecodePreventsPlaybackCompletion(false, true, false, true);
+}
+
+TEST_F(MediaSourcePlayerTest, A_SeekDuringEOSDecodePreventsCompletion) {
+  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
+
+  // 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) {
   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));
 
   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());
 
   // No seek requests should have occurred.
   EXPECT_EQ(0, demuxer_->num_seek_requests());
 }
 
 TEST_F(MediaSourcePlayerTest, DemuxerDataArrivesAfterRelease) {
   SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
@@ skipping 610 matching lines @@
 }
 
 TEST_F(MediaSourcePlayerTest, SeekToThenDemuxerSeekThenReleaseThenSeekDone) {
   SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
 
   // Test if Release() occurs after a SeekTo()'s subsequent DemuxerSeek IPC
   // request and OnDemuxerSeekDone() arrives prior to the next Start(), then the
   // 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();
   player_.OnDemuxerSeekDone(kNoTimestamp());
   EXPECT_FALSE(player_.IsPlaying());
   EXPECT_FALSE(GetMediaDecoderJob(true));
   EXPECT_EQ(100.0, GetPrerollTimestamp().InMillisecondsF());
 
   // Player should begin prefetch and resume preroll upon Start().
   EXPECT_EQ(1, demuxer_->num_data_requests());
   StartAudioDecoderJob();
   EXPECT_TRUE(GetMediaDecoderJob(true));
   EXPECT_TRUE(IsPrerolling(true));
   EXPECT_EQ(100.0, GetPrerollTimestamp().InMillisecondsF());
   EXPECT_EQ(2, demuxer_->num_data_requests());
 
   // No further seek should have been requested since before Release(), above.
   EXPECT_EQ(1, demuxer_->num_seek_requests());
 }
 
 TEST_F(MediaSourcePlayerTest, SeekToThenReleaseThenStart) {
   SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
 
   // Test if Release() occurs after a SeekTo()'s subsequent DemuxerSeeK IPC
   // request OnDemuxerSeekDone() does not occur until after the next Start(),

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

s/request OnDemux/request and OnDemux/ comment fix is included in next patch
set.

   // then the player remains pending seek done until (and resumes correct
   // 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();
   StartAudioDecoderJob();
   EXPECT_FALSE(GetMediaDecoderJob(true));
   EXPECT_EQ(1, demuxer_->num_data_requests());
 
   player_.OnDemuxerSeekDone(kNoTimestamp());
   EXPECT_TRUE(GetMediaDecoderJob(true));
   EXPECT_TRUE(IsPrerolling(true));
@@ skipping 196 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