Fix an issue that audio and video may run out of sync

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 a3e69e5bd3f5f91e0b1bb50c73fb9d20b3a8811c..95711d11fae023dfd4f0298259e4da128355025d 100644
--- a/media/base/android/media_source_player_unittest.cc
+++ b/media/base/android/media_source_player_unittest.cc
@@ -731,6 +731,10 @@ class MediaSourcePlayerTest : public testing::Test {
     return player_.start_time_ticks_;
   }
 
+  AudioTimestampHelper* GetAudioTimestampHelper() {
+    return player_.audio_timestamp_helper_.get();
+  }
+
   base::MessageLoop message_loop_;
   MockMediaPlayerManager manager_;
   MockDemuxerAndroid* demuxer_;  // Owned by |player_|.
@@ -2064,4 +2068,48 @@ TEST_F(MediaSourcePlayerTest, SurfaceChangeClearedEvenIfMediaCryptoAbsent) {
   EXPECT_FALSE(GetMediaDecoderJob(false));
 }
 
+TEST_F(MediaSourcePlayerTest, CurrentTimeUpdatedWhileDecoderStarved) {
+  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
+
+  // Test that current time is updated while decoder is starved.
+  StartAudioDecoderJob(true);
+  for (int i = 0; i < 3; ++i) {
+    player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForAudio(i));
+    WaitForAudioDecodeDone();
+  }
+  base::TimeDelta current_time = player_.GetCurrentTime();
+
+  // Trigger starvation while the decoder is decoding.
+  player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForAudio(3));
+  TriggerPlayerStarvation();
+  WaitForAudioDecodeDone();
+
+  // Current time should be updated.
+  EXPECT_LT(current_time.InMillisecondsF(),
+            player_.GetCurrentTime().InMillisecondsF());
+}
+
+TEST_F(MediaSourcePlayerTest,
+       AudioTimestampHelperNotRecreatedIfOnlyVideoConfigChanges) {
+  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
+
+  // Test that AudioTimestampHelper is not recreated if only video config
+  // changes.
+  CreateNextTextureAndSetVideoSurface();
+  Start(CreateAudioVideoDemuxerConfigs(), true);
+  AudioTimestampHelper* helper = GetAudioTimestampHelper();
+  for (int i = 0; i < 3; ++i) {
+    player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForAudio(i));
+    WaitForAudioDecodeDone();
+  }
+  player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForAudio(3));
+  player_.OnDemuxerDataAvailable(
+      CreateReadFromDemuxerAckWithConfigChanged(false, 0));
+  WaitForDecodeDone(true, true);
+  EXPECT_EQ(1, demuxer_->num_config_requests());
+  player_.OnDemuxerConfigsAvailable(CreateAudioVideoDemuxerConfigs());
+
+  EXPECT_EQ(helper, GetAudioTimestampHelper());

[wolenetz, 2014/04/04 17:27:43]

nit: Is there a better way than pointer comparison to ensure we haven't created
a new helper? I would prefer we don't further proliferate such a check that may
not always catch that the instance has changed, if the memory allocator re-used
the same location for the new instance. There's other locations in other tests
in this file that do this (e.g. to check if decoder job has/hasn't changed) that
I'm also concerned with. File bug + a TODO?

[qinmin, 2014/04/04 17:58:19]

Done.

+}
+
 }  // namespace media