Let audio wait for video to finish prerolling after seek

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/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/stringprintf.h"
@@ skipping 466 matching lines @@
     EXPECT_EQ(original_num_seeks + 1, demuxer_->num_seek_requests());
   }
 
   // Preroll the decoder job to |target_timestamp|. The first access unit
   // to decode will have a timestamp equal to |start_timestamp|.
   // 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());
+    // 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) :
         CreateReadFromDemuxerAckForVideo();
     int current_timestamp = start_timestamp.InMilliseconds();
 
     // Send some data with access unit timestamps before the |target_timestamp|,
     // and continue sending the data until preroll finishes.
     // This simulates the common condition that AUs received after browser
     // seek begin with timestamps before the seek target, and don't
     // immediately complete preroll.
     while (IsPrerolling(is_audio)) {
       data.access_units[0].timestamp =
           base::TimeDelta::FromMilliseconds(current_timestamp);
       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);
     }
     EXPECT_LE(target_timestamp, player_.GetCurrentTime());
   }
 
   DemuxerData CreateReadFromDemuxerAckWithConfigChanged(
       bool is_audio,
       int config_unit_index,
       const DemuxerConfigs& configs) {
@@ skipping 1090 matching lines @@
   EXPECT_EQ(100.0, player_.GetCurrentTime().InMillisecondsF());
   EXPECT_TRUE(IsPrerolling(true));
 
   // Send data after the seek position.
   PrerollDecoderToTime(true, target_timestamp, target_timestamp);
 }
 
 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));
 }
 
 TEST_F(MediaSourcePlayerTest, PrerollContinuesAfterUnchangedConfigs) {
   SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
 
   // Test decoder job will resume media prerolling if interrupted by a config
   // change access unit with unchanged configs.
   StartAudioDecoderJob();
 
   SeekPlayerWithAbort(true, base::TimeDelta::FromMilliseconds(100));
   EXPECT_TRUE(IsPrerolling(true));
   EXPECT_EQ(100.0, GetPrerollTimestamp().InMillisecondsF());
 
   DemuxerConfigs configs = CreateAudioDemuxerConfigs(kCodecVorbis, false);
 
   // 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));
 }
 
+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();
 
   // Test that the player allows simultaneous audio and video config change,
   // such as might occur during OnPrefetchDone() if next access unit for both
   // audio and video jobs is |kConfigChanged|.
   CreateNextTextureAndSetVideoSurface();
   Start(CreateAudioVideoDemuxerConfigs());
   player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForAudio(0));
   player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForVideo());
@@ skipping 524 matching lines @@
 
   DemuxerConfigs configs = CreateAudioDemuxerConfigs(kCodecVorbis, true);
   DemuxerData data = CreateReadFromDemuxerAckWithConfigChanged(
       true, 0, configs);
   player_.OnDemuxerDataAvailable(data);
   WaitForAudioDecodeDone();
   DecodeAudioDataUntilOutputBecomesAvailable();
 }
 
 }  // namespace media