Let audio wait for video to finish prerolling after seek

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 8050e34c2725bad12b3933dfbe42e2096f1cb0c1..a0e4d9ef26fbdea967f8caba67420da8439f17e1 100644
--- a/media/base/android/media_source_player_unittest.cc
+++ b/media/base/android/media_source_player_unittest.cc
@@ -484,7 +484,10 @@ class MediaSourcePlayerTest : public testing::Test {
   void PrerollDecoderToTime(bool is_audio,
                             const base::TimeDelta& start_timestamp,
                             const base::TimeDelta& target_timestamp) {
-    EXPECT_EQ(target_timestamp, player_.GetCurrentTime());
+    // It is possible that audio rolls past the |target_timestamp|. As a result,
+    // the current time may be larger than the |target_timestamp| for video as
+    // it may not be the clock manager.
+    EXPECT_TRUE(!is_audio || target_timestamp == player_.GetCurrentTime());

[wolenetz, 2014/09/24 20:39:04]

nit: Can you more precisely capture the expectation of player currenttime for a
video-only player that is prerolling?

[qinmin, 2014/09/24 22:49:50]

added a boolean to indicate whether the decoder is the clock manager. So for
video only player, the boolean should be true.

     // |start_timestamp| must be smaller than |target_timestamp|.
     EXPECT_LE(start_timestamp, target_timestamp);
     DemuxerData data = is_audio ? CreateReadFromDemuxerAckForAudio(1) :
@@ -502,7 +505,7 @@ class MediaSourcePlayerTest : public testing::Test {
       player_.OnDemuxerDataAvailable(data);
       EXPECT_TRUE(GetMediaDecoderJob(is_audio)->is_decoding());
       EXPECT_TRUE(GetMediaCodecBridge(is_audio));
-      EXPECT_EQ(target_timestamp, player_.GetCurrentTime());
+      EXPECT_TRUE(!is_audio || target_timestamp == player_.GetCurrentTime());

[wolenetz, 2014/09/24 20:39:04]

nit: ditto

[qinmin, 2014/09/24 22:49:50]

is_clock_manager should handle the case.

On 2014/09/24 20:39:04, wolenetz wrote:

       current_timestamp += 30;
       WaitForDecodeDone(is_audio, !is_audio);
     }
@@ -1613,21 +1616,11 @@ TEST_F(MediaSourcePlayerTest, PrerollContinuesAcrossReleaseAndStart) {
 TEST_F(MediaSourcePlayerTest, PrerollContinuesAcrossConfigChange) {
   SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
 
-  // Test decoder job will resume media prerolling if interrupted by
-  // |kConfigChanged| and OnDemuxerConfigsAvailable().
+  // Test decoder job will preroll the media to the seek position.
   StartAudioDecoderJob();
 
   SeekPlayerWithAbort(true, base::TimeDelta::FromMilliseconds(100));
   EXPECT_TRUE(IsPrerolling(true));
-  EXPECT_EQ(100.0, GetPrerollTimestamp().InMillisecondsF());
-
-  DemuxerConfigs configs = CreateAudioDemuxerConfigs(kCodecVorbis, true);

[wolenetz, 2014/09/24 20:39:04]

Do we have another test case that covers the case where a config change occurs
between the result of the seek and the preroll target? (Why was this piece of
this test removed?) Granted, this scenario is fairly unlikely, since without any
intervening changes to the underlying audio and video source_buffer_streams,
they will begin with buffers from the last keyframe with dts <= the seek target
(and config changes should be followed by keyframe, so there shouldn't be an
intervening config change). I'm uncertain if an overlapped appendBuffer for a
different config during preroll could cause config change during preroll, but I
suspect it could.

[qinmin, 2014/09/24 22:49:50]

Wierd, I didn't remember I made any change to this function.
Reverted all changes here.

On 2014/09/24 20:39:04, wolenetz wrote:

-
-  // In response to data request, simulate that demuxer signals config change by
-  // sending an AU with |kConfigChanged|.
-  DemuxerData data = CreateReadFromDemuxerAckWithConfigChanged(
-      true, 0, configs);
-  player_.OnDemuxerDataAvailable(data);
   PrerollDecoderToTime(
       true, base::TimeDelta(), base::TimeDelta::FromMilliseconds(100));
 }
@@ -1654,6 +1647,58 @@ TEST_F(MediaSourcePlayerTest, PrerollContinuesAfterUnchangedConfigs) {
       true, base::TimeDelta(), base::TimeDelta::FromMilliseconds(100));
 }
 
+TEST_F(MediaSourcePlayerTest, AudioPrerollFinishesBeforeVideo) {
+  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
+
+  // Test that after audio finishes prerolling, it will wait for video to finish
+  // prerolling before advancing together.
+  CreateNextTextureAndSetVideoSurface();
+  Start(CreateAudioVideoDemuxerConfigs());
+
+  // Initiate a seek.
+  base::TimeDelta seek_position = base::TimeDelta::FromMilliseconds(100);
+  player_.SeekTo(seek_position);
+  player_.OnDemuxerDataAvailable(CreateAbortedAck(true));
+  player_.OnDemuxerDataAvailable(CreateAbortedAck(false));
+  WaitForDecodeDone(true, true);
+
+  // Verify that the seek is requested.
+  EXPECT_EQ(1, demuxer_->num_seek_requests());
+  player_.OnDemuxerSeekDone(kNoTimestamp());
+  EXPECT_EQ(4, demuxer_->num_data_requests());
+  EXPECT_EQ(player_.GetCurrentTime().InMillisecondsF(), 100.0);
+  EXPECT_EQ(GetPrerollTimestamp().InMillisecondsF(), 100.0);
+
+  // Send both audio and video data to finish prefetching.
+  base::TimeDelta seek_ack_position = base::TimeDelta::FromMilliseconds(70);
+  DemuxerData audio_data = CreateReadFromDemuxerAckForAudio(0);
+  audio_data.access_units[0].timestamp = seek_ack_position;
+  DemuxerData video_data = CreateReadFromDemuxerAckForVideo();
+  video_data.access_units[0].timestamp = seek_ack_position;
+  player_.OnDemuxerDataAvailable(audio_data);
+  player_.OnDemuxerDataAvailable(video_data);
+  WaitForAudioDecodeDone();
+  WaitForVideoDecodeDone();
+
+  // Send audio data at and after the seek position. Audio should finish
+  // prerolling and stop decoding.
+  EXPECT_EQ(6, demuxer_->num_data_requests());
+  PrerollDecoderToTime(true, seek_position, seek_position);
+  EXPECT_FALSE(GetMediaDecoderJob(true)->is_decoding());
+  EXPECT_FALSE(IsPrerolling(true));
+  EXPECT_TRUE(IsPrerolling(false));
+
+  // Send video data to let video finish prerolling.
+  PrerollDecoderToTime(false, seek_position, seek_position);
+  EXPECT_FALSE(IsPrerolling(false));
+
+  // Both audio and video decoders should start decoding again.
+  player_.OnDemuxerDataAvailable(audio_data);
+  player_.OnDemuxerDataAvailable(video_data);
+  EXPECT_TRUE(GetMediaDecoderJob(true)->is_decoding());
+  EXPECT_TRUE(GetMediaDecoderJob(false)->is_decoding());
+}
+
 TEST_F(MediaSourcePlayerTest, SimultaneousAudioVideoConfigChange) {
   SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();