Let audio wait for video to finish prerolling after seek

After a seek, both audio and video decoder job will start prerolling.
However, audio decoder job can finish prerolling much earlier than video decoder job.
As a result, audio will start playing before video starts.
And we will see some fast motion when video try to catch up with audio after it starts rendering.

internal b/17527385

Review URL: https://codereview.chromium.org/596093002

Cr-Commit-Position: refs/heads/master@{#296607}

media/base/android/media_decoder_job.h

@@ -101,6 +101,8 @@ class MediaDecoderJob {
 
   bool is_content_encrypted() const { return is_content_encrypted_; }
 
+  bool prerolling() const { return prerolling_; }
+
  protected:
   // Creates a new MediaDecoderJob instance.
   // |decoder_task_runner| - Thread on which the decoder task will run.

media/base/android/media_source_player.cc

@@ -44,6 +44,7 @@ MediaSourcePlayer::MediaSourcePlayer(
       is_waiting_for_key_(false),
       is_waiting_for_audio_decoder_(false),
       is_waiting_for_video_decoder_(false),
+      prerolling_(false),
       weak_factory_(this) {
   audio_decoder_job_.reset(new AudioDecoderJob(
       base::Bind(&DemuxerAndroid::RequestDemuxerData,
@@ -346,6 +347,7 @@ void MediaSourcePlayer::OnDemuxerSeekDone(
     audio_decoder_job_->BeginPrerolling(preroll_timestamp_);
   if (HasVideo())
     video_decoder_job_->BeginPrerolling(preroll_timestamp_);
+  prerolling_ = true;
 
   if (!doing_browser_seek_)
     manager()->OnSeekComplete(player_id(), current_time);
@@ -509,6 +511,14 @@ void MediaSourcePlayer::MediaDecoderCallback(
   if (status == MEDIA_CODEC_STOPPED)
     return;
 
+  if (prerolling_ && IsPrerollFinished(is_audio)) {
+    if (IsPrerollFinished(!is_audio)) {
+      prerolling_ = false;
+      StartInternal();
+    }
+    return;
+  }
+
   if (is_clock_manager) {
     // If we have a valid timestamp, start the starvation callback. Otherwise,
     // reset the |start_time_ticks_| so that the next frame will not suffer
@@ -526,6 +536,12 @@ void MediaSourcePlayer::MediaDecoderCallback(
     DecodeMoreVideo();
 }
 
+bool MediaSourcePlayer::IsPrerollFinished(bool is_audio) const {
+  if (is_audio)
+    return !HasAudio() || !audio_decoder_job_->prerolling();
+  return !HasVideo() || !video_decoder_job_->prerolling();
+}
+
 void MediaSourcePlayer::DecodeMoreAudio() {
   DVLOG(1) << __FUNCTION__;
   DCHECK(!audio_decoder_job_->is_decoding());
@@ -589,11 +605,11 @@ void MediaSourcePlayer::ClearDecodingData() {
   start_time_ticks_ = base::TimeTicks();
 }
 
-bool MediaSourcePlayer::HasVideo() {
+bool MediaSourcePlayer::HasVideo() const {
   return video_decoder_job_->HasStream();
 }
 
-bool MediaSourcePlayer::HasAudio() {
+bool MediaSourcePlayer::HasAudio() const {
   return audio_decoder_job_->HasStream();
 }
 

media/base/android/media_source_player.h

@@ -90,6 +90,8 @@ class MEDIA_EXPORT MediaSourcePlayer : public MediaPlayerAndroid,
         base::TimeDelta current_presentation_timestamp,
         base::TimeDelta max_presentation_timestamp);
 
+  bool IsPrerollFinished(bool is_audio) const;
+
   // Gets MediaCrypto object from |drm_bridge_|.
   base::android::ScopedJavaLocalRef<jobject> GetMediaCrypto();
 
@@ -107,8 +109,8 @@ class MEDIA_EXPORT MediaSourcePlayer : public MediaPlayerAndroid,
   void DecodeMoreVideo();
 
   // Functions check whether audio/video is present.
-  bool HasVideo();
-  bool HasAudio();
+  bool HasVideo() const;
+  bool HasAudio() const;
 
   // Functions that check whether audio/video stream has reached end of output
   // or are not present in player configuration.
@@ -258,6 +260,9 @@ class MEDIA_EXPORT MediaSourcePlayer : public MediaPlayerAndroid,
   // Test-only callback for hooking the completion of the next decode cycle.
   base::Closure decode_callback_for_testing_;
 
+  // Whether audio or video decoder is in the process of prerolling.
+  bool prerolling_;
+
   // Weak pointer passed to media decoder jobs for callbacks.
   // NOTE: Weak pointers must be invalidated before all other member variables.
   base::WeakPtrFactory<MediaSourcePlayer> weak_factory_;

media/base/android/media_source_player_unittest.cc

@@ -479,12 +479,19 @@ class MediaSourcePlayerTest : public testing::Test {
 
   // Preroll the decoder job to |target_timestamp|. The first access unit
   // to decode will have a timestamp equal to |start_timestamp|.
+  // |is_clock_manager| indicates whether the decoder serves as the clock
+  // manager for the player.
   // TODO(qinmin): Add additional test cases for out-of-order decodes.
   // See http://crbug.com/331421.
   void PrerollDecoderToTime(bool is_audio,
                             const base::TimeDelta& start_timestamp,
-                            const base::TimeDelta& target_timestamp) {
-    EXPECT_EQ(target_timestamp, player_.GetCurrentTime());
+                            const base::TimeDelta& target_timestamp,
+                            bool is_clock_manager) {
+    // For streams with both audio and video, 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_clock_manager ||
+                target_timestamp == player_.GetCurrentTime());
     // |start_timestamp| must be smaller than |target_timestamp|.
     EXPECT_LE(start_timestamp, target_timestamp);
     DemuxerData data = is_audio ? CreateReadFromDemuxerAckForAudio(1) :
@@ -502,7 +509,8 @@ 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_clock_manager ||
+                  target_timestamp == player_.GetCurrentTime());
       current_timestamp += 30;
       WaitForDecodeDone(is_audio, !is_audio);
     }
@@ -1516,7 +1524,7 @@ TEST_F(MediaSourcePlayerTest, PrerollAudioAfterSeek) {
   SeekPlayerWithAbort(true, base::TimeDelta::FromMilliseconds(100));
   EXPECT_TRUE(IsPrerolling(true));
   PrerollDecoderToTime(
-      true, base::TimeDelta(), base::TimeDelta::FromMilliseconds(100));
+      true, base::TimeDelta(), base::TimeDelta::FromMilliseconds(100), true);
 }
 
 TEST_F(MediaSourcePlayerTest, PrerollVideoAfterSeek) {
@@ -1529,7 +1537,7 @@ TEST_F(MediaSourcePlayerTest, PrerollVideoAfterSeek) {
   SeekPlayerWithAbort(false, base::TimeDelta::FromMilliseconds(100));
   EXPECT_TRUE(IsPrerolling(false));
   PrerollDecoderToTime(
-      false, base::TimeDelta(), base::TimeDelta::FromMilliseconds(100));
+      false, base::TimeDelta(), base::TimeDelta::FromMilliseconds(100), true);
 }
 
 TEST_F(MediaSourcePlayerTest, SeekingAfterCompletingPrerollRestartsPreroll) {
@@ -1607,7 +1615,7 @@ TEST_F(MediaSourcePlayerTest, PrerollContinuesAcrossReleaseAndStart) {
   EXPECT_TRUE(IsPrerolling(true));
 
   // Send data after the seek position.
-  PrerollDecoderToTime(true, target_timestamp, target_timestamp);
+  PrerollDecoderToTime(true, target_timestamp, target_timestamp, true);
 }
 
 TEST_F(MediaSourcePlayerTest, PrerollContinuesAcrossConfigChange) {
@@ -1628,8 +1636,9 @@ TEST_F(MediaSourcePlayerTest, PrerollContinuesAcrossConfigChange) {
   DemuxerData data = CreateReadFromDemuxerAckWithConfigChanged(
       true, 0, configs);
   player_.OnDemuxerDataAvailable(data);
+
   PrerollDecoderToTime(
-      true, base::TimeDelta(), base::TimeDelta::FromMilliseconds(100));
+      true, base::TimeDelta(), base::TimeDelta::FromMilliseconds(100), true);
 }
 
 TEST_F(MediaSourcePlayerTest, PrerollContinuesAfterUnchangedConfigs) {
@@ -1651,7 +1660,59 @@ TEST_F(MediaSourcePlayerTest, PrerollContinuesAfterUnchangedConfigs) {
       true, 0, configs);
   player_.OnDemuxerDataAvailable(data);
   PrerollDecoderToTime(
-      true, base::TimeDelta(), base::TimeDelta::FromMilliseconds(100));
+      true, base::TimeDelta(), base::TimeDelta::FromMilliseconds(100), true);
+}
+
+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, true);
+  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, false);
+  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) {
@@ -1777,7 +1838,7 @@ TEST_F(MediaSourcePlayerTest, BrowserSeek_PrerollAfterBrowserSeek) {
   EXPECT_EQ(3, demuxer_->num_data_requests());
 
   PrerollDecoderToTime(
-      false, base::TimeDelta(), base::TimeDelta::FromMilliseconds(100));
+      false, base::TimeDelta(), base::TimeDelta::FromMilliseconds(100), true);
 }
 
 TEST_F(MediaSourcePlayerTest, VideoDemuxerConfigChange) {